Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1676108 > unrolled thread

[PATCH 3/9] iommu: Introduce iommu do invalidate API function

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 3/9] iommu: Introduce iommu do invalidate API function Jacob Pan <jacob.jun.pan@linux.intel.com> - 2017-06-27 21:50 +0200
    Re: [PATCH 3/9] iommu: Introduce iommu do invalidate API function Joerg Roedel <joro@8bytes.org> - 2017-06-28 12:10 +0200
      Re: [PATCH 3/9] iommu: Introduce iommu do invalidate API function Jacob Pan <jacob.jun.pan@linux.intel.com> - 2017-06-28 18:10 +0200
        Re: [PATCH 3/9] iommu: Introduce iommu do invalidate API function Jean-Philippe Brucker <jean-philippe.brucker@arm.com> - 2017-06-28 20:00 +0200

#1676108 — [PATCH 3/9] iommu: Introduce iommu do invalidate API function

From: "Liu, Yi L" <yi.l.liu@linux.intel.com>

When a SVM capable device is assigned to a guest, the first level page
tables are owned by the guest and the guest PASID table pointer is
linked to the device context entry of the physical IOMMU.

Host IOMMU driver has no knowledge of caching structure updates unless
the guest invalidation activities are passed down to the host. The
primary usage is derived from emulated IOMMU in the guest, where QEMU
can trap invalidation activities before pass them down the
host/physical IOMMU. There are IOMMU architectural specific actions
need to be taken which requires the generic APIs introduced in this
patch to have opaque data in the tlb_invalidate_info argument.

Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
---
 drivers/iommu/iommu.c      | 14 ++++++++++++++
 include/linux/iommu.h      | 12 ++++++++++++
 include/uapi/linux/iommu.h | 13 +++++++++++++
 3 files changed, 39 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 494309b..d973555 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1347,6 +1347,20 @@ int iommu_unbind_pasid_table(struct iommu_domain *domain, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_unbind_pasid_table);
 
+int iommu_invalidate(struct iommu_domain *domain,
+		struct device *dev, struct tlb_invalidate_info *inv_info)
+{
+	int ret = 0;
+
+	if (unlikely(!domain->ops->invalidate))
+		return -ENODEV;
+
+	ret = domain->ops->invalidate(domain, dev, inv_info);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_invalidate);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
 				  struct device *dev)
 {
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 7122add..fbc08ae 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -186,6 +186,7 @@ struct iommu_resv_region {
  * @pgsize_bitmap: bitmap of all possible supported page sizes
  * @bind_pasid_table: bind pasid table pointer for guest SVM
  * @unbind_pasid_table: unbind pasid table pointer and restore defaults
+ * @invalidate: invalidate translation caches
  */
 struct iommu_ops {
 	bool (*capable)(enum iommu_cap);
@@ -232,6 +233,8 @@ struct iommu_ops {
 				struct pasid_table_info *pasidt_binfo);
 	int (*unbind_pasid_table)(struct iommu_domain *domain,
 				struct device *dev);
+	int (*invalidate)(struct iommu_domain *domain,
+		struct device *dev, struct tlb_invalidate_info *inv_info);
 
 	unsigned long pgsize_bitmap;
 };
@@ -293,6 +296,9 @@ extern int iommu_bind_pasid_table(struct iommu_domain *domain,
 		struct device *dev, struct pasid_table_info *pasidt_binfo);
 extern int iommu_unbind_pasid_table(struct iommu_domain *domain,
 				struct device *dev);
+extern int iommu_do_invalidate(struct iommu_domain *domain,
+		struct device *dev, struct tlb_invalidate_info *inv_info);
+
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
 		     phys_addr_t paddr, size_t size, int prot);
@@ -660,6 +666,12 @@ int iommu_unbind_pasid_table(struct iommu_domain *domain, struct device *dev)
 	return -EINVAL;
 }
 
+static inline int iommu_do_invalidate(struct iommu_domain *domain,
+		struct device *dev, struct tlb_invalidate_info *inv_info)
+{
+	return -EINVAL;
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #endif /* __LINUX_IOMMU_H */
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index 9089a30..f077353 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -34,5 +34,18 @@ struct pasid_table_info {
 	__u8	opaque[];
 };
 
+/*
+ * Translation cache invalidation information, contains IOMMU model specific
+ * data which can be parsed based on model ID by model specific drivers.
+ *
+ * @model	iommu_model number
+ * @length	length of the opaque data in bytes
+ * @opaque	model specific IOMMU data
+ */
+struct tlb_invalidate_info {
+	__u32	model;
+	__u32	length;
+	__u8	opaque[];
+};
 
 #endif /* _UAPI_IOMMU_H */
-- 
2.7.4

[toc] | [next] | [standalone]

#1676506

On Tue, Jun 27, 2017 at 12:47:57PM -0700, Jacob Pan wrote:
> From: "Liu, Yi L" <yi.l.liu@linux.intel.com>
> 
> When a SVM capable device is assigned to a guest, the first level page
> tables are owned by the guest and the guest PASID table pointer is
> linked to the device context entry of the physical IOMMU.
> 
> Host IOMMU driver has no knowledge of caching structure updates unless
> the guest invalidation activities are passed down to the host. The
> primary usage is derived from emulated IOMMU in the guest, where QEMU
> can trap invalidation activities before pass them down the
> host/physical IOMMU. There are IOMMU architectural specific actions
> need to be taken which requires the generic APIs introduced in this
> patch to have opaque data in the tlb_invalidate_info argument.

Which "IOMMU architectural specific actions" are you thinking of?

> +int iommu_invalidate(struct iommu_domain *domain,
> +		struct device *dev, struct tlb_invalidate_info *inv_info)
> +{
> +	int ret = 0;
> +
> +	if (unlikely(!domain->ops->invalidate))
> +		return -ENODEV;
> +
> +	ret = domain->ops->invalidate(domain, dev, inv_info);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(iommu_invalidate);

[...]

> +struct tlb_invalidate_info {
> +	__u32	model;
> +	__u32	length;
> +	__u8	opaque[];
> +};

This interface is aweful. It requires the user of a generic api to know
details about the implementation behind to do anything useful.

Please explain in more detail why this is needed. My feeling is that we
can make this more generic with a small set of invalidation functions in
the iommu-api.



	Joerg

[toc] | [prev] | [next] | [standalone]

#1676900

On Wed, 28 Jun 2017 12:08:23 +0200
Joerg Roedel <joro@8bytes.org> wrote:

> On Tue, Jun 27, 2017 at 12:47:57PM -0700, Jacob Pan wrote:
> > From: "Liu, Yi L" <yi.l.liu@linux.intel.com>
> > 
> > When a SVM capable device is assigned to a guest, the first level
> > page tables are owned by the guest and the guest PASID table
> > pointer is linked to the device context entry of the physical IOMMU.
> > 
> > Host IOMMU driver has no knowledge of caching structure updates
> > unless the guest invalidation activities are passed down to the
> > host. The primary usage is derived from emulated IOMMU in the
> > guest, where QEMU can trap invalidation activities before pass them
> > down the host/physical IOMMU. There are IOMMU architectural
> > specific actions need to be taken which requires the generic APIs
> > introduced in this patch to have opaque data in the
> > tlb_invalidate_info argument.  
> 
> Which "IOMMU architectural specific actions" are you thinking of?
> 
construction of queued invalidation descriptors, then submit them to
the IOMMU QI interface.
> > +int iommu_invalidate(struct iommu_domain *domain,
> > +		struct device *dev, struct tlb_invalidate_info
> > *inv_info) +{
> > +	int ret = 0;
> > +
> > +	if (unlikely(!domain->ops->invalidate))
> > +		return -ENODEV;
> > +
> > +	ret = domain->ops->invalidate(domain, dev, inv_info);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_invalidate);  
> 
> [...]
> 
> > +struct tlb_invalidate_info {
> > +	__u32	model;
> > +	__u32	length;
> > +	__u8	opaque[];
> > +};  
> 
> This interface is aweful. It requires the user of a generic api to
> know details about the implementation behind to do anything useful.
> 
> Please explain in more detail why this is needed. My feeling is that
> we can make this more generic with a small set of invalidation
> functions in the iommu-api.
> 
My thinking was that via configuration control, there will be unlikely
any mixed IOMMU models between pIOMMU and vIOMMU. We could have just
model specific data pass through layers of SW (QEMU, VFIO) for
performance reasons. We do have an earlier hybrid version that has
generic data and opaque raw data. Would the below work for all IOMMU
models?

https://www.spinics.net/lists/kvm/msg148798.html

struct tlb_invalidate_info
{
        __u32   model;  /* Vendor number */
        __u8 granularity
#define DEVICE_SELECTVIE_INV    (1 << 0)
#define PAGE_SELECTIVE_INV      (1 << 0)
#define PASID_SELECTIVE_INV     (1 << 1)
        __u32 pasid;
        __u64 addr;
        __u64 size;

        /* Since IOMMU format has already been validated for this table,
           the IOMMU driver knows that the following structure is in a
           format it knows */
        __u8 opaque[];
};

> 
> 
> 	Joerg
> 

[Jacob Pan]

[toc] | [prev] | [next] | [standalone]

#1676991

On 28/06/17 17:09, Jacob Pan wrote:
> On Wed, 28 Jun 2017 12:08:23 +0200
> Joerg Roedel <joro@8bytes.org> wrote:
> 
>> On Tue, Jun 27, 2017 at 12:47:57PM -0700, Jacob Pan wrote:
>>> From: "Liu, Yi L" <yi.l.liu@linux.intel.com>
>>>
>>> When a SVM capable device is assigned to a guest, the first level
>>> page tables are owned by the guest and the guest PASID table
>>> pointer is linked to the device context entry of the physical IOMMU.
>>>
>>> Host IOMMU driver has no knowledge of caching structure updates
>>> unless the guest invalidation activities are passed down to the
>>> host. The primary usage is derived from emulated IOMMU in the
>>> guest, where QEMU can trap invalidation activities before pass them
>>> down the host/physical IOMMU. There are IOMMU architectural
>>> specific actions need to be taken which requires the generic APIs
>>> introduced in this patch to have opaque data in the
>>> tlb_invalidate_info argument.  
>>
>> Which "IOMMU architectural specific actions" are you thinking of?
>>
> construction of queued invalidation descriptors, then submit them to
> the IOMMU QI interface.
>>> +int iommu_invalidate(struct iommu_domain *domain,
>>> +		struct device *dev, struct tlb_invalidate_info
>>> *inv_info) +{
>>> +	int ret = 0;
>>> +
>>> +	if (unlikely(!domain->ops->invalidate))
>>> +		return -ENODEV;
>>> +
>>> +	ret = domain->ops->invalidate(domain, dev, inv_info);
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(iommu_invalidate);  
>>
>> [...]
>>
>>> +struct tlb_invalidate_info {
>>> +	__u32	model;
>>> +	__u32	length;
>>> +	__u8	opaque[];
>>> +};  
>>
>> This interface is aweful. It requires the user of a generic api to
>> know details about the implementation behind to do anything useful.
>>
>> Please explain in more detail why this is needed. My feeling is that
>> we can make this more generic with a small set of invalidation
>> functions in the iommu-api.
>>
> My thinking was that via configuration control, there will be unlikely
> any mixed IOMMU models between pIOMMU and vIOMMU. We could have just
> model specific data pass through layers of SW (QEMU, VFIO) for
> performance reasons. We do have an earlier hybrid version that has
> generic data and opaque raw data. Would the below work for all IOMMU
> models?

For reference, this was also discussed in the initial posting of the series:
https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03452.html

At least for ARM SMMUv2 and v3, I think the invalidation format you
propose should be sufficient, although "device_selective" should probably
be "domain_selective". And maybe a flag field could contain relatively
generic hints such as "only invalidate leaf table when page_selective".

Thanks,
Jean

> https://www.spinics.net/lists/kvm/msg148798.html
> 
> struct tlb_invalidate_info
> {
>         __u32   model;  /* Vendor number */
>         __u8 granularity
> #define DEVICE_SELECTVIE_INV    (1 << 0)
> #define PAGE_SELECTIVE_INV      (1 << 0)
> #define PASID_SELECTIVE_INV     (1 << 1)
>         __u32 pasid;
>         __u64 addr;
>         __u64 size;
> 
>         /* Since IOMMU format has already been validated for this table,
>            the IOMMU driver knows that the following structure is in a
>            format it knows */
>         __u8 opaque[];
> };
> 
>>
>>
>> 	Joerg
>>
> 
> [Jacob Pan]
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web