Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

On Tue, 8 May 2018 11:35:00 +0100
Jean-Philippe Brucker  wrote:

> Hi Jacob,
> 
> Looks mostly good to me, I just have a couple more comments
> 
> On 04/05/18 19:07, Jacob Pan wrote:
> > Now the passdown invalidation granularities look like:
> > (sorted by coarseness), will send out in v5 patchset soon if no
> > issues.
> > 
> > /**
> >  * enum iommu_inv_granularity - Generic invalidation granularity
> >  *
> >  * @IOMMU_INV_GRANU_DOMAIN: Device context cache
> > associated with a
> >  *  domain ID.
> >  * @IOMMU_INV_GRANU_DEVICE: Device context cache
> > associated with a
> >  *  device ID
> >  * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID:   TLB entries or PASID
> > caches of all
> >  *  PASIDs associated with a
> > domain ID
> >  * @IOMMU_INV_GRANU_PASID_SEL:  TLB entries or PASID
> > cache associated
> >  *  with a PASID and a domain
> >  * @IOMMU_INV_GRANU_PAGE_PASID: TLB entries of
> > selected page range
> >  *  within a PASID
> >  *
> >  * When an invalidation request is passed down to IOMMU to flush
> > translation
> >  * caches, it may carry different granularity levels, which can be
> > specific
> >  * to certain types of translation caches. For an example, PASID
> > selective
> >  * granularity is only applicable PASID cache and IOTLB
> > invalidation but for
> >  * device context caches.  
> 
> Should it be "PASID selective granularity is only applicable to PASID
> cache and IOTLB but not device context caches"?
> 
right, thanks!
> >  * This enum is a collection of granularities for all types of
> > translation
> >  * caches. The idea is to make it easy for IOMMU model specific
> > driver to
> >  * convert from generic to model specific value. Not all
> > combinations between
> >  * translation caches and granularity levels are valid. Each IOMMU
> > driver
> >  * can enforce check based on its own conversion table. The
> > conversion is
> >  * based on 2D look-up with inputs as follows:
> >  * - translation cache types
> >  * - granularity
> >  * No global granularity is allowed in that passdown invalidation
> > for an
> >  * assigned device should only impact the device or domain itself.  
> 
> That last sentence is a bit confusing, because "global granularity"
> might also refer to the "global" TLB flag which is allowed. In my
> opinion you can leave this rationale out, I doubt userspace will ever
> demand a mechanism for global invalidation.
> 
yes, i can leave the last sentence out.
> >  *
> >  * type |   DTLB|TLB|   PASID   |  CONTEXT
> >  *  granule |   |   |   |
> >  * -+---+---+---+---
> >  *  DOMAIN  |   |   |   | Y
> >  *  DEVICE  |   |   |   | Y  
> 
> I can't really see a use-case for DOMAIN and DEVICE. It might make
> more sense to keep only DN_ALL_PASID, which would then also
> invalidate the device context cache. But since they will be very rare
> events, factoring them doesn't seem important.
> 
ok. we have no use for now either, was there for completeness. i will
remove for now.
> >  *  DN_ALL_PASID|   Y   |   Y   |   Y   |
> >  *  PASID_SEL   |   Y   |   Y   |   Y   |
> >  *  PAGE_PASID  |   |   Y   |   |  
> 
> Why not allow PAGE_PASID+DTLB? We need a way to invalidate individual
> DTLB entries
> 
I was thinking PAGE_PASID+TLB includes PAGE_PASID+DTLB, but you are
right, DTLB should be a 'Y' here.
> Thanks,
> Jean

[Jacob Pan]

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

On Tue, 8 May 2018 11:35:00 +0100
Jean-Philippe Brucker  wrote:

> Hi Jacob,
> 
> Looks mostly good to me, I just have a couple more comments
> 
> On 04/05/18 19:07, Jacob Pan wrote:
> > Now the passdown invalidation granularities look like:
> > (sorted by coarseness), will send out in v5 patchset soon if no
> > issues.
> > 
> > /**
> >  * enum iommu_inv_granularity - Generic invalidation granularity
> >  *
> >  * @IOMMU_INV_GRANU_DOMAIN: Device context cache
> > associated with a
> >  *  domain ID.
> >  * @IOMMU_INV_GRANU_DEVICE: Device context cache
> > associated with a
> >  *  device ID
> >  * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID:   TLB entries or PASID
> > caches of all
> >  *  PASIDs associated with a
> > domain ID
> >  * @IOMMU_INV_GRANU_PASID_SEL:  TLB entries or PASID
> > cache associated
> >  *  with a PASID and a domain
> >  * @IOMMU_INV_GRANU_PAGE_PASID: TLB entries of
> > selected page range
> >  *  within a PASID
> >  *
> >  * When an invalidation request is passed down to IOMMU to flush
> > translation
> >  * caches, it may carry different granularity levels, which can be
> > specific
> >  * to certain types of translation caches. For an example, PASID
> > selective
> >  * granularity is only applicable PASID cache and IOTLB
> > invalidation but for
> >  * device context caches.  
> 
> Should it be "PASID selective granularity is only applicable to PASID
> cache and IOTLB but not device context caches"?
> 
right, thanks!
> >  * This enum is a collection of granularities for all types of
> > translation
> >  * caches. The idea is to make it easy for IOMMU model specific
> > driver to
> >  * convert from generic to model specific value. Not all
> > combinations between
> >  * translation caches and granularity levels are valid. Each IOMMU
> > driver
> >  * can enforce check based on its own conversion table. The
> > conversion is
> >  * based on 2D look-up with inputs as follows:
> >  * - translation cache types
> >  * - granularity
> >  * No global granularity is allowed in that passdown invalidation
> > for an
> >  * assigned device should only impact the device or domain itself.  
> 
> That last sentence is a bit confusing, because "global granularity"
> might also refer to the "global" TLB flag which is allowed. In my
> opinion you can leave this rationale out, I doubt userspace will ever
> demand a mechanism for global invalidation.
> 
yes, i can leave the last sentence out.
> >  *
> >  * type |   DTLB|TLB|   PASID   |  CONTEXT
> >  *  granule |   |   |   |
> >  * -+---+---+---+---
> >  *  DOMAIN  |   |   |   | Y
> >  *  DEVICE  |   |   |   | Y  
> 
> I can't really see a use-case for DOMAIN and DEVICE. It might make
> more sense to keep only DN_ALL_PASID, which would then also
> invalidate the device context cache. But since they will be very rare
> events, factoring them doesn't seem important.
> 
ok. we have no use for now either, was there for completeness. i will
remove for now.
> >  *  DN_ALL_PASID|   Y   |   Y   |   Y   |
> >  *  PASID_SEL   |   Y   |   Y   |   Y   |
> >  *  PAGE_PASID  |   |   Y   |   |  
> 
> Why not allow PAGE_PASID+DTLB? We need a way to invalidate individual
> DTLB entries
> 
I was thinking PAGE_PASID+TLB includes PAGE_PASID+DTLB, but you are
right, DTLB should be a 'Y' here.
> Thanks,
> Jean

[Jacob Pan]

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jean-Philippe Brucker]

Hi Jacob,

Looks mostly good to me, I just have a couple more comments

On 04/05/18 19:07, Jacob Pan wrote:
> Now the passdown invalidation granularities look like:
> (sorted by coarseness), will send out in v5 patchset soon if no issues.
> 
> /**
>  * enum iommu_inv_granularity - Generic invalidation granularity
>  *
>  * @IOMMU_INV_GRANU_DOMAIN:   Device context cache associated with a
>  *domain ID.
>  * @IOMMU_INV_GRANU_DEVICE:   Device context cache associated with a
>  *device ID
>  * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID: TLB entries or PASID caches of all
>  *PASIDs associated with a domain ID
>  * @IOMMU_INV_GRANU_PASID_SEL:TLB entries or PASID cache 
> associated
>  *with a PASID and a domain
>  * @IOMMU_INV_GRANU_PAGE_PASID:   TLB entries of selected page 
> range
>  *within a PASID
>  *
>  * When an invalidation request is passed down to IOMMU to flush translation
>  * caches, it may carry different granularity levels, which can be specific
>  * to certain types of translation caches. For an example, PASID selective
>  * granularity is only applicable PASID cache and IOTLB invalidation but for
>  * device context caches.

Should it be "PASID selective granularity is only applicable to PASID
cache and IOTLB but not device context caches"?

>  * This enum is a collection of granularities for all types of translation
>  * caches. The idea is to make it easy for IOMMU model specific driver to
>  * convert from generic to model specific value. Not all combinations between
>  * translation caches and granularity levels are valid. Each IOMMU driver
>  * can enforce check based on its own conversion table. The conversion is
>  * based on 2D look-up with inputs as follows:
>  * - translation cache types
>  * - granularity
>  * No global granularity is allowed in that passdown invalidation for an
>  * assigned device should only impact the device or domain itself.

That last sentence is a bit confusing, because "global granularity"
might also refer to the "global" TLB flag which is allowed. In my
opinion you can leave this rationale out, I doubt userspace will ever
demand a mechanism for global invalidation.

>  *
>  * type |   DTLB|TLB|   PASID   |  CONTEXT
>  *  granule |   |   |   |
>  * -+---+---+---+---
>  *  DOMAIN  |   |   |   | Y
>  *  DEVICE  |   |   |   | Y

I can't really see a use-case for DOMAIN and DEVICE. It might make more
sense to keep only DN_ALL_PASID, which would then also invalidate the
device context cache. But since they will be very rare events, factoring
them doesn't seem important.

>  *  DN_ALL_PASID|   Y   |   Y   |   Y   |
>  *  PASID_SEL   |   Y   |   Y   |   Y   |
>  *  PAGE_PASID  |   |   Y   |   |

Why not allow PAGE_PASID+DTLB? We need a way to invalidate individual
DTLB entries

Thanks,
Jean

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jean-Philippe Brucker]

Hi Jacob,

Looks mostly good to me, I just have a couple more comments

On 04/05/18 19:07, Jacob Pan wrote:
> Now the passdown invalidation granularities look like:
> (sorted by coarseness), will send out in v5 patchset soon if no issues.
> 
> /**
>  * enum iommu_inv_granularity - Generic invalidation granularity
>  *
>  * @IOMMU_INV_GRANU_DOMAIN:   Device context cache associated with a
>  *domain ID.
>  * @IOMMU_INV_GRANU_DEVICE:   Device context cache associated with a
>  *device ID
>  * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID: TLB entries or PASID caches of all
>  *PASIDs associated with a domain ID
>  * @IOMMU_INV_GRANU_PASID_SEL:TLB entries or PASID cache 
> associated
>  *with a PASID and a domain
>  * @IOMMU_INV_GRANU_PAGE_PASID:   TLB entries of selected page 
> range
>  *within a PASID
>  *
>  * When an invalidation request is passed down to IOMMU to flush translation
>  * caches, it may carry different granularity levels, which can be specific
>  * to certain types of translation caches. For an example, PASID selective
>  * granularity is only applicable PASID cache and IOTLB invalidation but for
>  * device context caches.

Should it be "PASID selective granularity is only applicable to PASID
cache and IOTLB but not device context caches"?

>  * This enum is a collection of granularities for all types of translation
>  * caches. The idea is to make it easy for IOMMU model specific driver to
>  * convert from generic to model specific value. Not all combinations between
>  * translation caches and granularity levels are valid. Each IOMMU driver
>  * can enforce check based on its own conversion table. The conversion is
>  * based on 2D look-up with inputs as follows:
>  * - translation cache types
>  * - granularity
>  * No global granularity is allowed in that passdown invalidation for an
>  * assigned device should only impact the device or domain itself.

That last sentence is a bit confusing, because "global granularity"
might also refer to the "global" TLB flag which is allowed. In my
opinion you can leave this rationale out, I doubt userspace will ever
demand a mechanism for global invalidation.

>  *
>  * type |   DTLB|TLB|   PASID   |  CONTEXT
>  *  granule |   |   |   |
>  * -+---+---+---+---
>  *  DOMAIN  |   |   |   | Y
>  *  DEVICE  |   |   |   | Y

I can't really see a use-case for DOMAIN and DEVICE. It might make more
sense to keep only DN_ALL_PASID, which would then also invalidate the
device context cache. But since they will be very rare events, factoring
them doesn't seem important.

>  *  DN_ALL_PASID|   Y   |   Y   |   Y   |
>  *  PASID_SEL   |   Y   |   Y   |   Y   |
>  *  PAGE_PASID  |   |   Y   |   |

Why not allow PAGE_PASID+DTLB? We need a way to invalidate individual
DTLB entries

Thanks,
Jean

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

On Sat, 5 May 2018 15:19:43 -0700
Jerry Snitselaar  wrote:

> >
> >+static inline int iommu_sva_invalidate(struct iommu_domain *domain,
> >+struct device *dev, struct tlb_invalidate_info
> >*inv_info) +{
> >+return -EINVAL;
> >+}
> >+  
> 
> Would -ENODEV make more sense here?
> 
yes, make sense. thanks!
>  [...]  

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

On Sat, 5 May 2018 15:19:43 -0700
Jerry Snitselaar  wrote:

> >
> >+static inline int iommu_sva_invalidate(struct iommu_domain *domain,
> >+struct device *dev, struct tlb_invalidate_info
> >*inv_info) +{
> >+return -EINVAL;
> >+}
> >+  
> 
> Would -ENODEV make more sense here?
> 
yes, make sense. thanks!
>  [...]  

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jerry Snitselaar]

On Mon Apr 16 18, Jacob Pan wrote:

From: "Liu, Yi L" 

When an 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 passing them down to the
host/physical IOMMU.
Since the invalidation data are obtained from user space and will be
written into physical IOMMU, we must allow security check at various
layers. Therefore, generic invalidation data format are proposed here,
model specific IOMMU drivers need to convert them into their own format.

Signed-off-by: Liu, Yi L 
Signed-off-by: Jean-Philippe Brucker 
Signed-off-by: Jacob Pan 
Signed-off-by: Ashok Raj 
---
drivers/iommu/iommu.c  | 14 
include/linux/iommu.h  | 12 +++
include/uapi/linux/iommu.h | 79 ++
3 files changed, 105 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3a69620..784e019 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1344,6 +1344,20 @@ void iommu_unbind_pasid_table(struct iommu_domain 
*domain, struct device *dev)
}
EXPORT_SYMBOL_GPL(iommu_unbind_pasid_table);

+int iommu_sva_invalidate(struct iommu_domain *domain,
+   struct device *dev, struct tlb_invalidate_info *inv_info)
+{
+   int ret = 0;
+
+   if (unlikely(!domain->ops->sva_invalidate))
+   return -ENODEV;
+
+   ret = domain->ops->sva_invalidate(domain, dev, inv_info);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_sva_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 8ad111f..e963dbd 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -190,6 +190,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
+ * @sva_invalidate: invalidate translation caches of shared virtual address
 */
struct iommu_ops {
bool (*capable)(enum iommu_cap);
@@ -243,6 +244,8 @@ struct iommu_ops {
struct pasid_table_config *pasidt_binfo);
void (*unbind_pasid_table)(struct iommu_domain *domain,
struct device *dev);
+   int (*sva_invalidate)(struct iommu_domain *domain,
+   struct device *dev, struct tlb_invalidate_info *inv_info);

unsigned long pgsize_bitmap;
};
@@ -309,6 +312,9 @@ extern int iommu_bind_pasid_table(struct iommu_domain 
*domain,
struct device *dev, struct pasid_table_config *pasidt_binfo);
extern void iommu_unbind_pasid_table(struct iommu_domain *domain,
struct device *dev);
+extern int iommu_sva_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);
@@ -720,6 +726,12 @@ void iommu_unbind_pasid_table(struct iommu_domain *domain, 
struct device *dev)
{
}

+static inline int iommu_sva_invalidate(struct iommu_domain *domain,
+   struct device *dev, struct tlb_invalidate_info *inv_info)
+{
+   return -EINVAL;
+}
+


Would -ENODEV make more sense here?


#endif /* CONFIG_IOMMU_API */

#endif /* __LINUX_IOMMU_H */
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index 9f7a6bf..4447943 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -29,4 +29,83 @@ struct pasid_table_config {
__u8 pasid_bits;
};

+/**
+ * enum iommu_inv_granularity - Generic invalidation granularity
+ *
+ * When an invalidation request is sent to IOMMU to flush translation caches,
+ * it may carry different granularity. These granularity levels are not 
specific
+ * to a type of translation cache. For an example, PASID selective granularity
+ * is only applicable to PASID cache invalidation.
+ * This enum is a collection of granularities for all types of translation
+ * caches. The idea is to make it easy for IOMMU model specific driver do
+ * conversion from generic to model specific value.
+ */
+enum iommu_inv_granularity {
+   IOMMU_INV_GRANU_DOMAIN = 1, /* all TLBs associated with a domain */
+   IOMMU_INV_GRANU_DEVICE, /* caching 

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jerry Snitselaar]

On Mon Apr 16 18, Jacob Pan wrote:

From: "Liu, Yi L" 

When an 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 passing them down to the
host/physical IOMMU.
Since the invalidation data are obtained from user space and will be
written into physical IOMMU, we must allow security check at various
layers. Therefore, generic invalidation data format are proposed here,
model specific IOMMU drivers need to convert them into their own format.

Signed-off-by: Liu, Yi L 
Signed-off-by: Jean-Philippe Brucker 
Signed-off-by: Jacob Pan 
Signed-off-by: Ashok Raj 
---
drivers/iommu/iommu.c  | 14 
include/linux/iommu.h  | 12 +++
include/uapi/linux/iommu.h | 79 ++
3 files changed, 105 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3a69620..784e019 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1344,6 +1344,20 @@ void iommu_unbind_pasid_table(struct iommu_domain 
*domain, struct device *dev)
}
EXPORT_SYMBOL_GPL(iommu_unbind_pasid_table);

+int iommu_sva_invalidate(struct iommu_domain *domain,
+   struct device *dev, struct tlb_invalidate_info *inv_info)
+{
+   int ret = 0;
+
+   if (unlikely(!domain->ops->sva_invalidate))
+   return -ENODEV;
+
+   ret = domain->ops->sva_invalidate(domain, dev, inv_info);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_sva_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 8ad111f..e963dbd 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -190,6 +190,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
+ * @sva_invalidate: invalidate translation caches of shared virtual address
 */
struct iommu_ops {
bool (*capable)(enum iommu_cap);
@@ -243,6 +244,8 @@ struct iommu_ops {
struct pasid_table_config *pasidt_binfo);
void (*unbind_pasid_table)(struct iommu_domain *domain,
struct device *dev);
+   int (*sva_invalidate)(struct iommu_domain *domain,
+   struct device *dev, struct tlb_invalidate_info *inv_info);

unsigned long pgsize_bitmap;
};
@@ -309,6 +312,9 @@ extern int iommu_bind_pasid_table(struct iommu_domain 
*domain,
struct device *dev, struct pasid_table_config *pasidt_binfo);
extern void iommu_unbind_pasid_table(struct iommu_domain *domain,
struct device *dev);
+extern int iommu_sva_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);
@@ -720,6 +726,12 @@ void iommu_unbind_pasid_table(struct iommu_domain *domain, 
struct device *dev)
{
}

+static inline int iommu_sva_invalidate(struct iommu_domain *domain,
+   struct device *dev, struct tlb_invalidate_info *inv_info)
+{
+   return -EINVAL;
+}
+


Would -ENODEV make more sense here?


#endif /* CONFIG_IOMMU_API */

#endif /* __LINUX_IOMMU_H */
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index 9f7a6bf..4447943 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -29,4 +29,83 @@ struct pasid_table_config {
__u8 pasid_bits;
};

+/**
+ * enum iommu_inv_granularity - Generic invalidation granularity
+ *
+ * When an invalidation request is sent to IOMMU to flush translation caches,
+ * it may carry different granularity. These granularity levels are not 
specific
+ * to a type of translation cache. For an example, PASID selective granularity
+ * is only applicable to PASID cache invalidation.
+ * This enum is a collection of granularities for all types of translation
+ * caches. The idea is to make it easy for IOMMU model specific driver do
+ * conversion from generic to model specific value.
+ */
+enum iommu_inv_granularity {
+   IOMMU_INV_GRANU_DOMAIN = 1, /* all TLBs associated with a domain */
+   IOMMU_INV_GRANU_DEVICE, /* caching structure associated with a
+* device ID
+*/
+   

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

On Thu, 3 May 2018 21:46:16 -0700
Jacob Pan  wrote:

> On Wed, 2 May 2018 10:31:50 +0100
> Jean-Philippe Brucker  wrote:
> 
> > On 01/05/18 23:58, Jacob Pan wrote:  
> >  Maybe this should be called "NG_PAGE_PASID",  
> > >>> Sure. I was thinking page range already implies non-global
> > >>> pages.  
> >  and "DOMAIN_PAGE" should
> >  instead be "PAGE_PASID". If I understood their meaning
> >  correctly, it would be more consistent with the rest.
> >   
> > >>> I am trying not to mix granu between request w/ PASID and w/o.
> > >>> DOMAIN_PAGE meant to be for request w/o PASID.  
> > >>
> > >> Is the distinction necessary? I understand the IOMMU side might
> > >> offer many possibilities for invalidation, but the user probably
> > >> doesn't need all of them. It might be easier to document,
> > >> upstream and maintain if we only specify what's currently needed
> > >> by users (what does QEMU VT-d use?) Others can always extend it
> > >> by increasing the version.
> > >>
> > >> Do you think that this invalidation message will be used outside
> > >> of BIND_PASID_TABLE context? I can't see an other use but who
> > >> knows. At the moment requests w/o PASID are managed with
> > >> VFIO_IOMMU_MAP/UNMAP_DMA, which doesn't require invalidation. And
> > >> in a BIND_PASID_TABLE context, IOMMUs requests w/o PASID are
> > >> just a special case using PASID 0 (for Arm and AMD) so I suppose
> > >> they'll use the same invalidation commands as requests w/ PASID.
> > >>
> > > My understanding is that for GIOVA use case, VT-d vIOMMU creates
> > > GIOVA-GPA mapping and the host shadows the 2nd level page tables
> > > to create GIOVA-HPA mapping. So when assigned device in the guest
> > > can do both DMA map/unmap and VFIO map/unmap, VFIO unmap is one
> > > time deal (I guess invalidation can be captured in other code
> > > path), but guest kernel use of DMA unmap could will trigger
> > > invalidation. QEMU needs to trap those invalidation and passdown
> > > to physical IOMMU. So we do need invalidation w/o PASID.
> > 
> > Hm, isn't this all done by host userspace? Whether guest does DMA
> > map/unmap or VFIO map/unmap, it creates/removes IOVA-GPA mappings in
> > the vIOMMU. QEMU captures invalidation requests for these mappings
> > from the guest, finds GPA-HVA in the shadow map and sends a VFIO
> > map/unmap request for IOVA-HVA.
> >   
> Sorry for the delay but you are right, I have also confirmed with Yi
> that we don't need second level invalidation. I will remove IOTLB
> invalidation w/o PASID case from the API.
> 
Now the passdown invalidation granularities look like:
(sorted by coarseness), will send out in v5 patchset soon if no issues.

/**
 * enum iommu_inv_granularity - Generic invalidation granularity
 *
 * @IOMMU_INV_GRANU_DOMAIN: Device context cache associated with a
 *  domain ID.
 * @IOMMU_INV_GRANU_DEVICE: Device context cache associated with a
 *  device ID
 * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID:   TLB entries or PASID caches of all
 *  PASIDs associated with a domain ID
 * @IOMMU_INV_GRANU_PASID_SEL:  TLB entries or PASID cache associated
 *  with a PASID and a domain
 * @IOMMU_INV_GRANU_PAGE_PASID: TLB entries of selected page range
 *  within a PASID
 *
 * When an invalidation request is passed down to IOMMU to flush translation
 * caches, it may carry different granularity levels, which can be specific
 * to certain types of translation caches. For an example, PASID selective
 * granularity is only applicable PASID cache and IOTLB invalidation but for
 * device context caches.
 * This enum is a collection of granularities for all types of translation
 * caches. The idea is to make it easy for IOMMU model specific driver to
 * convert from generic to model specific value. Not all combinations between
 * translation caches and granularity levels are valid. Each IOMMU driver
 * can enforce check based on its own conversion table. The conversion is
 * based on 2D look-up with inputs as follows:
 * - translation cache types
 * - granularity
 * No global granularity is allowed in that passdown invalidation for an
 * assigned device should only impact the device or domain itself.
 *
 * type |   DTLB|TLB|   PASID   |  CONTEXT
 *  granule |   |   |   |
 * -+---+---+---+---
 *  DOMAIN  |   |   |   | Y
 *  DEVICE  |   |   |   | Y
 *  DN_ALL_PASID|   Y   |   Y   |   Y   |
 *  PASID_SEL   |   Y   |   Y   |   Y   |
 *  PAGE_PASID  |   |   Y   |   |
 *
 */
enum 

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

On Thu, 3 May 2018 21:46:16 -0700
Jacob Pan  wrote:

> On Wed, 2 May 2018 10:31:50 +0100
> Jean-Philippe Brucker  wrote:
> 
> > On 01/05/18 23:58, Jacob Pan wrote:  
> >  Maybe this should be called "NG_PAGE_PASID",  
> > >>> Sure. I was thinking page range already implies non-global
> > >>> pages.  
> >  and "DOMAIN_PAGE" should
> >  instead be "PAGE_PASID". If I understood their meaning
> >  correctly, it would be more consistent with the rest.
> >   
> > >>> I am trying not to mix granu between request w/ PASID and w/o.
> > >>> DOMAIN_PAGE meant to be for request w/o PASID.  
> > >>
> > >> Is the distinction necessary? I understand the IOMMU side might
> > >> offer many possibilities for invalidation, but the user probably
> > >> doesn't need all of them. It might be easier to document,
> > >> upstream and maintain if we only specify what's currently needed
> > >> by users (what does QEMU VT-d use?) Others can always extend it
> > >> by increasing the version.
> > >>
> > >> Do you think that this invalidation message will be used outside
> > >> of BIND_PASID_TABLE context? I can't see an other use but who
> > >> knows. At the moment requests w/o PASID are managed with
> > >> VFIO_IOMMU_MAP/UNMAP_DMA, which doesn't require invalidation. And
> > >> in a BIND_PASID_TABLE context, IOMMUs requests w/o PASID are
> > >> just a special case using PASID 0 (for Arm and AMD) so I suppose
> > >> they'll use the same invalidation commands as requests w/ PASID.
> > >>
> > > My understanding is that for GIOVA use case, VT-d vIOMMU creates
> > > GIOVA-GPA mapping and the host shadows the 2nd level page tables
> > > to create GIOVA-HPA mapping. So when assigned device in the guest
> > > can do both DMA map/unmap and VFIO map/unmap, VFIO unmap is one
> > > time deal (I guess invalidation can be captured in other code
> > > path), but guest kernel use of DMA unmap could will trigger
> > > invalidation. QEMU needs to trap those invalidation and passdown
> > > to physical IOMMU. So we do need invalidation w/o PASID.
> > 
> > Hm, isn't this all done by host userspace? Whether guest does DMA
> > map/unmap or VFIO map/unmap, it creates/removes IOVA-GPA mappings in
> > the vIOMMU. QEMU captures invalidation requests for these mappings
> > from the guest, finds GPA-HVA in the shadow map and sends a VFIO
> > map/unmap request for IOVA-HVA.
> >   
> Sorry for the delay but you are right, I have also confirmed with Yi
> that we don't need second level invalidation. I will remove IOTLB
> invalidation w/o PASID case from the API.
> 
Now the passdown invalidation granularities look like:
(sorted by coarseness), will send out in v5 patchset soon if no issues.

/**
 * enum iommu_inv_granularity - Generic invalidation granularity
 *
 * @IOMMU_INV_GRANU_DOMAIN: Device context cache associated with a
 *  domain ID.
 * @IOMMU_INV_GRANU_DEVICE: Device context cache associated with a
 *  device ID
 * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID:   TLB entries or PASID caches of all
 *  PASIDs associated with a domain ID
 * @IOMMU_INV_GRANU_PASID_SEL:  TLB entries or PASID cache associated
 *  with a PASID and a domain
 * @IOMMU_INV_GRANU_PAGE_PASID: TLB entries of selected page range
 *  within a PASID
 *
 * When an invalidation request is passed down to IOMMU to flush translation
 * caches, it may carry different granularity levels, which can be specific
 * to certain types of translation caches. For an example, PASID selective
 * granularity is only applicable PASID cache and IOTLB invalidation but for
 * device context caches.
 * This enum is a collection of granularities for all types of translation
 * caches. The idea is to make it easy for IOMMU model specific driver to
 * convert from generic to model specific value. Not all combinations between
 * translation caches and granularity levels are valid. Each IOMMU driver
 * can enforce check based on its own conversion table. The conversion is
 * based on 2D look-up with inputs as follows:
 * - translation cache types
 * - granularity
 * No global granularity is allowed in that passdown invalidation for an
 * assigned device should only impact the device or domain itself.
 *
 * type |   DTLB|TLB|   PASID   |  CONTEXT
 *  granule |   |   |   |
 * -+---+---+---+---
 *  DOMAIN  |   |   |   | Y
 *  DEVICE  |   |   |   | Y
 *  DN_ALL_PASID|   Y   |   Y   |   Y   |
 *  PASID_SEL   |   Y   |   Y   |   Y   |
 *  PAGE_PASID  |   |   Y   |   |
 *
 */
enum iommu_inv_granularity {
IOMMU_INV_GRANU_DOMAIN,

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

On Wed, 2 May 2018 10:31:50 +0100
Jean-Philippe Brucker  wrote:

> On 01/05/18 23:58, Jacob Pan wrote:
>  Maybe this should be called "NG_PAGE_PASID",
> >>> Sure. I was thinking page range already implies non-global
> >>> pages.
>  and "DOMAIN_PAGE" should
>  instead be "PAGE_PASID". If I understood their meaning correctly,
>  it would be more consistent with the rest.
> 
> >>> I am trying not to mix granu between request w/ PASID and w/o.
> >>> DOMAIN_PAGE meant to be for request w/o PASID.
> >>
> >> Is the distinction necessary? I understand the IOMMU side might
> >> offer many possibilities for invalidation, but the user probably
> >> doesn't need all of them. It might be easier to document, upstream
> >> and maintain if we only specify what's currently needed by users
> >> (what does QEMU VT-d use?) Others can always extend it by
> >> increasing the version.
> >>
> >> Do you think that this invalidation message will be used outside of
> >> BIND_PASID_TABLE context? I can't see an other use but who knows.
> >> At the moment requests w/o PASID are managed with
> >> VFIO_IOMMU_MAP/UNMAP_DMA, which doesn't require invalidation. And
> >> in a BIND_PASID_TABLE context, IOMMUs requests w/o PASID are just a
> >> special case using PASID 0 (for Arm and AMD) so I suppose they'll
> >> use the same invalidation commands as requests w/ PASID.
> >>  
> > My understanding is that for GIOVA use case, VT-d vIOMMU creates
> > GIOVA-GPA mapping and the host shadows the 2nd level page tables to
> > create GIOVA-HPA mapping. So when assigned device in the guest can
> > do both DMA map/unmap and VFIO map/unmap, VFIO unmap is one time
> > deal (I guess invalidation can be captured in other code path), but
> > guest kernel use of DMA unmap could will trigger invalidation. QEMU
> > needs to trap those invalidation and passdown to physical IOMMU. So
> > we do need invalidation w/o PASID.  
> 
> Hm, isn't this all done by host userspace? Whether guest does DMA
> map/unmap or VFIO map/unmap, it creates/removes IOVA-GPA mappings in
> the vIOMMU. QEMU captures invalidation requests for these mappings
> from the guest, finds GPA-HVA in the shadow map and sends a VFIO
> map/unmap request for IOVA-HVA.
> 
Sorry for the delay but you are right, I have also confirmed with Yi
that we don't need second level invalidation. I will remove IOTLB
invalidation w/o PASID case from the API.

Thanks,

> Thanks,
> Jean
> 

[Jacob Pan]

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

On Wed, 2 May 2018 10:31:50 +0100
Jean-Philippe Brucker  wrote:

> On 01/05/18 23:58, Jacob Pan wrote:
>  Maybe this should be called "NG_PAGE_PASID",
> >>> Sure. I was thinking page range already implies non-global
> >>> pages.
>  and "DOMAIN_PAGE" should
>  instead be "PAGE_PASID". If I understood their meaning correctly,
>  it would be more consistent with the rest.
> 
> >>> I am trying not to mix granu between request w/ PASID and w/o.
> >>> DOMAIN_PAGE meant to be for request w/o PASID.
> >>
> >> Is the distinction necessary? I understand the IOMMU side might
> >> offer many possibilities for invalidation, but the user probably
> >> doesn't need all of them. It might be easier to document, upstream
> >> and maintain if we only specify what's currently needed by users
> >> (what does QEMU VT-d use?) Others can always extend it by
> >> increasing the version.
> >>
> >> Do you think that this invalidation message will be used outside of
> >> BIND_PASID_TABLE context? I can't see an other use but who knows.
> >> At the moment requests w/o PASID are managed with
> >> VFIO_IOMMU_MAP/UNMAP_DMA, which doesn't require invalidation. And
> >> in a BIND_PASID_TABLE context, IOMMUs requests w/o PASID are just a
> >> special case using PASID 0 (for Arm and AMD) so I suppose they'll
> >> use the same invalidation commands as requests w/ PASID.
> >>  
> > My understanding is that for GIOVA use case, VT-d vIOMMU creates
> > GIOVA-GPA mapping and the host shadows the 2nd level page tables to
> > create GIOVA-HPA mapping. So when assigned device in the guest can
> > do both DMA map/unmap and VFIO map/unmap, VFIO unmap is one time
> > deal (I guess invalidation can be captured in other code path), but
> > guest kernel use of DMA unmap could will trigger invalidation. QEMU
> > needs to trap those invalidation and passdown to physical IOMMU. So
> > we do need invalidation w/o PASID.  
> 
> Hm, isn't this all done by host userspace? Whether guest does DMA
> map/unmap or VFIO map/unmap, it creates/removes IOVA-GPA mappings in
> the vIOMMU. QEMU captures invalidation requests for these mappings
> from the guest, finds GPA-HVA in the shadow map and sends a VFIO
> map/unmap request for IOVA-HVA.
> 
Sorry for the delay but you are right, I have also confirmed with Yi
that we don't need second level invalidation. I will remove IOTLB
invalidation w/o PASID case from the API.

Thanks,

> Thanks,
> Jean
> 

[Jacob Pan]

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jean-Philippe Brucker]

On 01/05/18 23:58, Jacob Pan wrote:
 Maybe this should be called "NG_PAGE_PASID",  
>>> Sure. I was thinking page range already implies non-global pages.  
 and "DOMAIN_PAGE" should
 instead be "PAGE_PASID". If I understood their meaning correctly,
 it would be more consistent with the rest.
  
>>> I am trying not to mix granu between request w/ PASID and w/o.
>>> DOMAIN_PAGE meant to be for request w/o PASID.  
>>
>> Is the distinction necessary? I understand the IOMMU side might offer
>> many possibilities for invalidation, but the user probably doesn't
>> need all of them. It might be easier to document, upstream and
>> maintain if we only specify what's currently needed by users (what
>> does QEMU VT-d use?) Others can always extend it by increasing the
>> version.
>>
>> Do you think that this invalidation message will be used outside of
>> BIND_PASID_TABLE context? I can't see an other use but who knows. At
>> the moment requests w/o PASID are managed with
>> VFIO_IOMMU_MAP/UNMAP_DMA, which doesn't require invalidation. And in
>> a BIND_PASID_TABLE context, IOMMUs requests w/o PASID are just a
>> special case using PASID 0 (for Arm and AMD) so I suppose they'll use
>> the same invalidation commands as requests w/ PASID.
>>
> My understanding is that for GIOVA use case, VT-d vIOMMU creates
> GIOVA-GPA mapping and the host shadows the 2nd level page tables to
> create GIOVA-HPA mapping. So when assigned device in the guest can do
> both DMA map/unmap and VFIO map/unmap, VFIO unmap is one time deal
> (I guess invalidation can be captured in other code path), but guest
> kernel use of DMA unmap could will trigger invalidation. QEMU needs to
> trap those invalidation and passdown to physical IOMMU. So we do need
> invalidation w/o PASID.

Hm, isn't this all done by host userspace? Whether guest does DMA
map/unmap or VFIO map/unmap, it creates/removes IOVA-GPA mappings in the
vIOMMU. QEMU captures invalidation requests for these mappings from the
guest, finds GPA-HVA in the shadow map and sends a VFIO map/unmap
request for IOVA-HVA.

Thanks,
Jean

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jean-Philippe Brucker]

On 01/05/18 23:58, Jacob Pan wrote:
 Maybe this should be called "NG_PAGE_PASID",  
>>> Sure. I was thinking page range already implies non-global pages.  
 and "DOMAIN_PAGE" should
 instead be "PAGE_PASID". If I understood their meaning correctly,
 it would be more consistent with the rest.
  
>>> I am trying not to mix granu between request w/ PASID and w/o.
>>> DOMAIN_PAGE meant to be for request w/o PASID.  
>>
>> Is the distinction necessary? I understand the IOMMU side might offer
>> many possibilities for invalidation, but the user probably doesn't
>> need all of them. It might be easier to document, upstream and
>> maintain if we only specify what's currently needed by users (what
>> does QEMU VT-d use?) Others can always extend it by increasing the
>> version.
>>
>> Do you think that this invalidation message will be used outside of
>> BIND_PASID_TABLE context? I can't see an other use but who knows. At
>> the moment requests w/o PASID are managed with
>> VFIO_IOMMU_MAP/UNMAP_DMA, which doesn't require invalidation. And in
>> a BIND_PASID_TABLE context, IOMMUs requests w/o PASID are just a
>> special case using PASID 0 (for Arm and AMD) so I suppose they'll use
>> the same invalidation commands as requests w/ PASID.
>>
> My understanding is that for GIOVA use case, VT-d vIOMMU creates
> GIOVA-GPA mapping and the host shadows the 2nd level page tables to
> create GIOVA-HPA mapping. So when assigned device in the guest can do
> both DMA map/unmap and VFIO map/unmap, VFIO unmap is one time deal
> (I guess invalidation can be captured in other code path), but guest
> kernel use of DMA unmap could will trigger invalidation. QEMU needs to
> trap those invalidation and passdown to physical IOMMU. So we do need
> invalidation w/o PASID.

Hm, isn't this all done by host userspace? Whether guest does DMA
map/unmap or VFIO map/unmap, it creates/removes IOVA-GPA mappings in the
vIOMMU. QEMU captures invalidation requests for these mappings from the
guest, finds GPA-HVA in the shadow map and sends a VFIO map/unmap
request for IOVA-HVA.

Thanks,
Jean

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

On Fri, 27 Apr 2018 19:07:43 +0100
Jean-Philippe Brucker  wrote:

> On 23/04/18 21:43, Jacob Pan wrote:
> [...]
> >> The last name is a bit unfortunate. Since the Arm architecture uses
> >> the name "context" for what a PASID points to, "Device cache" would
> >> suit us better but it's not important.
> >>  
> > or call it device context cache. actually so far context cache is
> > here only for completeness purpose. the expected use case is that
> > QEMU traps guest device context cache flush and call
> > bind_pasid_table.   
> 
> Right, makes sense
> 
> [...]
> >> If this corresponds to QI_GRAN_ALL_ALL in patch 9, the comment
> >> should be "Cache of all PASIDs"? Or maybe "all entries for all
> >> PASIDs"? Is it different from GRANU_DOMAIN then?  
> > QI_GRAN_ALL_ALL maps to VT-d spec 6.5.2.4, which invalidates all ext
> > TLB cache within a domain. It could reuse GRANU_DOMAIN but I was
> > also trying to match the naming convention in the spec.  
> 
> Sorry I don't quite understand the difference between TLB and ext TLB
> invalidation. Can an ext TLB invalidation do everything a TLB can do
> plus some additional parameters (introduced in more recent version of
> the spec), or do they have distinct purposes? I'm trying to understand
> why it needs to be user-visible
> 
> >>> + IOMMU_INV_GRANU_PASID_SEL,  /* only invalidate
> >>> specified PASID */ +
> >>> + IOMMU_INV_GRANU_NG_ALL_PASID,   /* non-global within
> >>> all PASIDs */
> >>> + IOMMU_INV_GRANU_NG_PASID,   /* non-global within a
> >>> PASIDs */
> >>
> >> Are the "NG" variant needed since there is a
> >> IOMMU_INVALIDATE_GLOBAL_PAGE below? We should drop either flag or
> >> granule.
> >>
> >> FWIW I'm starting to think more granule options is actually better
> >> than flags, because it flattens the combinations and keeps them to
> >> two dimensions, that we can understand and explain with a table.
> >>  
> >>> + IOMMU_INV_GRANU_PAGE_PASID, /* page-selective
> >>> within a PASID */
> >>
> >> Maybe this should be called "NG_PAGE_PASID",  
> > Sure. I was thinking page range already implies non-global pages.  
> >> and "DOMAIN_PAGE" should
> >> instead be "PAGE_PASID". If I understood their meaning correctly,
> >> it would be more consistent with the rest.
> >>  
> > I am trying not to mix granu between request w/ PASID and w/o.
> > DOMAIN_PAGE meant to be for request w/o PASID.  
> 
> Is the distinction necessary? I understand the IOMMU side might offer
> many possibilities for invalidation, but the user probably doesn't
> need all of them. It might be easier to document, upstream and
> maintain if we only specify what's currently needed by users (what
> does QEMU VT-d use?) Others can always extend it by increasing the
> version.
> 
> Do you think that this invalidation message will be used outside of
> BIND_PASID_TABLE context? I can't see an other use but who knows. At
> the moment requests w/o PASID are managed with
> VFIO_IOMMU_MAP/UNMAP_DMA, which doesn't require invalidation. And in
> a BIND_PASID_TABLE context, IOMMUs requests w/o PASID are just a
> special case using PASID 0 (for Arm and AMD) so I suppose they'll use
> the same invalidation commands as requests w/ PASID.
> 
My understanding is that for GIOVA use case, VT-d vIOMMU creates
GIOVA-GPA mapping and the host shadows the 2nd level page tables to
create GIOVA-HPA mapping. So when assigned device in the guest can do
both DMA map/unmap and VFIO map/unmap, VFIO unmap is one time deal
(I guess invalidation can be captured in other code path), but guest
kernel use of DMA unmap could will trigger invalidation. QEMU needs to
trap those invalidation and passdown to physical IOMMU. So we do need
invalidation w/o PASID.

> >>> + IOMMU_INV_NR_GRANU,
> >>> +};
> >>> +
> >>> +/** enum iommu_inv_type - Generic translation cache types for
> >>> invalidation
> >>> + *
> >>> + * Invalidation requests sent to IOMMU may indicate which
> >>> translation cache
> >>> + * to be operated on.
> >>> + * Combined with enum iommu_inv_granularity, model specific
> >>> driver can do a
> >>> + * simple lookup to convert generic type to model specific value.
> >>> + */
> >>> +enum iommu_inv_type {
> >>
> >> These should be flags (1 << 0), (1 << 1) etc, since IOMMUs will
> >> want to invalidate multiple caches at once (at least DTLB and
> >> TLB). You could then do for_each_set_bit in the driver
> >>  
> > I was thinking the invalidation to be inclusive as we discussed
> > earlier ,last year :).
> > TLB includes DLTB
> > PASID cache includes TLB and DTLB. I need to document it better.  
> 
> Ah right, I guess I was stuck on an old version :) Then the current
> values make sense
> 
> >>> + IOMMU_INV_TYPE_DTLB,/* device IOTLB */
> >>> + IOMMU_INV_TYPE_TLB, /* IOMMU paging structure
> >>> cache */
> >>> + IOMMU_INV_TYPE_PASID,   /* PASID cache */
> >>> + IOMMU_INV_TYPE_CONTEXT, /* device context entry
> >>> cache */
> >>> + IOMMU_INV_NR_TYPE
> >>> +};
> 

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

On Fri, 27 Apr 2018 19:07:43 +0100
Jean-Philippe Brucker  wrote:

> On 23/04/18 21:43, Jacob Pan wrote:
> [...]
> >> The last name is a bit unfortunate. Since the Arm architecture uses
> >> the name "context" for what a PASID points to, "Device cache" would
> >> suit us better but it's not important.
> >>  
> > or call it device context cache. actually so far context cache is
> > here only for completeness purpose. the expected use case is that
> > QEMU traps guest device context cache flush and call
> > bind_pasid_table.   
> 
> Right, makes sense
> 
> [...]
> >> If this corresponds to QI_GRAN_ALL_ALL in patch 9, the comment
> >> should be "Cache of all PASIDs"? Or maybe "all entries for all
> >> PASIDs"? Is it different from GRANU_DOMAIN then?  
> > QI_GRAN_ALL_ALL maps to VT-d spec 6.5.2.4, which invalidates all ext
> > TLB cache within a domain. It could reuse GRANU_DOMAIN but I was
> > also trying to match the naming convention in the spec.  
> 
> Sorry I don't quite understand the difference between TLB and ext TLB
> invalidation. Can an ext TLB invalidation do everything a TLB can do
> plus some additional parameters (introduced in more recent version of
> the spec), or do they have distinct purposes? I'm trying to understand
> why it needs to be user-visible
> 
> >>> + IOMMU_INV_GRANU_PASID_SEL,  /* only invalidate
> >>> specified PASID */ +
> >>> + IOMMU_INV_GRANU_NG_ALL_PASID,   /* non-global within
> >>> all PASIDs */
> >>> + IOMMU_INV_GRANU_NG_PASID,   /* non-global within a
> >>> PASIDs */
> >>
> >> Are the "NG" variant needed since there is a
> >> IOMMU_INVALIDATE_GLOBAL_PAGE below? We should drop either flag or
> >> granule.
> >>
> >> FWIW I'm starting to think more granule options is actually better
> >> than flags, because it flattens the combinations and keeps them to
> >> two dimensions, that we can understand and explain with a table.
> >>  
> >>> + IOMMU_INV_GRANU_PAGE_PASID, /* page-selective
> >>> within a PASID */
> >>
> >> Maybe this should be called "NG_PAGE_PASID",  
> > Sure. I was thinking page range already implies non-global pages.  
> >> and "DOMAIN_PAGE" should
> >> instead be "PAGE_PASID". If I understood their meaning correctly,
> >> it would be more consistent with the rest.
> >>  
> > I am trying not to mix granu between request w/ PASID and w/o.
> > DOMAIN_PAGE meant to be for request w/o PASID.  
> 
> Is the distinction necessary? I understand the IOMMU side might offer
> many possibilities for invalidation, but the user probably doesn't
> need all of them. It might be easier to document, upstream and
> maintain if we only specify what's currently needed by users (what
> does QEMU VT-d use?) Others can always extend it by increasing the
> version.
> 
> Do you think that this invalidation message will be used outside of
> BIND_PASID_TABLE context? I can't see an other use but who knows. At
> the moment requests w/o PASID are managed with
> VFIO_IOMMU_MAP/UNMAP_DMA, which doesn't require invalidation. And in
> a BIND_PASID_TABLE context, IOMMUs requests w/o PASID are just a
> special case using PASID 0 (for Arm and AMD) so I suppose they'll use
> the same invalidation commands as requests w/ PASID.
> 
My understanding is that for GIOVA use case, VT-d vIOMMU creates
GIOVA-GPA mapping and the host shadows the 2nd level page tables to
create GIOVA-HPA mapping. So when assigned device in the guest can do
both DMA map/unmap and VFIO map/unmap, VFIO unmap is one time deal
(I guess invalidation can be captured in other code path), but guest
kernel use of DMA unmap could will trigger invalidation. QEMU needs to
trap those invalidation and passdown to physical IOMMU. So we do need
invalidation w/o PASID.

> >>> + IOMMU_INV_NR_GRANU,
> >>> +};
> >>> +
> >>> +/** enum iommu_inv_type - Generic translation cache types for
> >>> invalidation
> >>> + *
> >>> + * Invalidation requests sent to IOMMU may indicate which
> >>> translation cache
> >>> + * to be operated on.
> >>> + * Combined with enum iommu_inv_granularity, model specific
> >>> driver can do a
> >>> + * simple lookup to convert generic type to model specific value.
> >>> + */
> >>> +enum iommu_inv_type {
> >>
> >> These should be flags (1 << 0), (1 << 1) etc, since IOMMUs will
> >> want to invalidate multiple caches at once (at least DTLB and
> >> TLB). You could then do for_each_set_bit in the driver
> >>  
> > I was thinking the invalidation to be inclusive as we discussed
> > earlier ,last year :).
> > TLB includes DLTB
> > PASID cache includes TLB and DTLB. I need to document it better.  
> 
> Ah right, I guess I was stuck on an old version :) Then the current
> values make sense
> 
> >>> + IOMMU_INV_TYPE_DTLB,/* device IOTLB */
> >>> + IOMMU_INV_TYPE_TLB, /* IOMMU paging structure
> >>> cache */
> >>> + IOMMU_INV_TYPE_PASID,   /* PASID cache */
> >>> + IOMMU_INV_TYPE_CONTEXT, /* device context entry
> >>> cache */
> >>> + IOMMU_INV_NR_TYPE
> >>> +};
> >>
> >> We need to summarize 

RE: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Tian, Kevin]

> From: Jean-Philippe Brucker [mailto:jean-philippe.bruc...@arm.com]
> Sent: Saturday, April 28, 2018 2:08 AM
> 
> [...]
> >> If this corresponds to QI_GRAN_ALL_ALL in patch 9, the comment should
> >> be "Cache of all PASIDs"? Or maybe "all entries for all PASIDs"? Is it
> >> different from GRANU_DOMAIN then?
> > QI_GRAN_ALL_ALL maps to VT-d spec 6.5.2.4, which invalidates all ext
> > TLB cache within a domain. It could reuse GRANU_DOMAIN but I was
> > also trying to match the naming convention in the spec.
> 
> Sorry I don't quite understand the difference between TLB and ext TLB
> invalidation. Can an ext TLB invalidation do everything a TLB can do
> plus some additional parameters (introduced in more recent version of
> the spec), or do they have distinct purposes? I'm trying to understand
> why it needs to be user-visible

distinct purpose though some overlapped effect:

IOTLB invalidate is more for 2nd-level cache on granularity (global/
domain/PSI), with side effect on 1st-level and nested caches (global/
domain).

Extended IOTLB invalidate is specifically for 1st-level and nested
caches on granularity (per-domain: all PASIDs/per PASID/PSI). 

Thanks
Kevin

RE: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Tian, Kevin]

> From: Jean-Philippe Brucker [mailto:jean-philippe.bruc...@arm.com]
> Sent: Saturday, April 28, 2018 2:08 AM
> 
> [...]
> >> If this corresponds to QI_GRAN_ALL_ALL in patch 9, the comment should
> >> be "Cache of all PASIDs"? Or maybe "all entries for all PASIDs"? Is it
> >> different from GRANU_DOMAIN then?
> > QI_GRAN_ALL_ALL maps to VT-d spec 6.5.2.4, which invalidates all ext
> > TLB cache within a domain. It could reuse GRANU_DOMAIN but I was
> > also trying to match the naming convention in the spec.
> 
> Sorry I don't quite understand the difference between TLB and ext TLB
> invalidation. Can an ext TLB invalidation do everything a TLB can do
> plus some additional parameters (introduced in more recent version of
> the spec), or do they have distinct purposes? I'm trying to understand
> why it needs to be user-visible

distinct purpose though some overlapped effect:

IOTLB invalidate is more for 2nd-level cache on granularity (global/
domain/PSI), with side effect on 1st-level and nested caches (global/
domain).

Extended IOTLB invalidate is specifically for 1st-level and nested
caches on granularity (per-domain: all PASIDs/per PASID/PSI). 

Thanks
Kevin

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jean-Philippe Brucker]

On 23/04/18 21:43, Jacob Pan wrote:
[...]
>> The last name is a bit unfortunate. Since the Arm architecture uses
>> the name "context" for what a PASID points to, "Device cache" would
>> suit us better but it's not important.
>>
> or call it device context cache. actually so far context cache is here
> only for completeness purpose. the expected use case is that QEMU traps
> guest device context cache flush and call bind_pasid_table. 

Right, makes sense

[...]
>> If this corresponds to QI_GRAN_ALL_ALL in patch 9, the comment should
>> be "Cache of all PASIDs"? Or maybe "all entries for all PASIDs"? Is it
>> different from GRANU_DOMAIN then?
> QI_GRAN_ALL_ALL maps to VT-d spec 6.5.2.4, which invalidates all ext
> TLB cache within a domain. It could reuse GRANU_DOMAIN but I was
> also trying to match the naming convention in the spec.

Sorry I don't quite understand the difference between TLB and ext TLB
invalidation. Can an ext TLB invalidation do everything a TLB can do
plus some additional parameters (introduced in more recent version of
the spec), or do they have distinct purposes? I'm trying to understand
why it needs to be user-visible

>>> +   IOMMU_INV_GRANU_PASID_SEL,  /* only invalidate
>>> specified PASID */ +
>>> +   IOMMU_INV_GRANU_NG_ALL_PASID,   /* non-global within
>>> all PASIDs */
>>> +   IOMMU_INV_GRANU_NG_PASID,   /* non-global within a
>>> PASIDs */  
>>
>> Are the "NG" variant needed since there is a
>> IOMMU_INVALIDATE_GLOBAL_PAGE below? We should drop either flag or
>> granule.
>>
>> FWIW I'm starting to think more granule options is actually better
>> than flags, because it flattens the combinations and keeps them to two
>> dimensions, that we can understand and explain with a table.
>>
>>> +   IOMMU_INV_GRANU_PAGE_PASID, /* page-selective
>>> within a PASID */  
>>
>> Maybe this should be called "NG_PAGE_PASID",
> Sure. I was thinking page range already implies non-global pages.
>> and "DOMAIN_PAGE" should
>> instead be "PAGE_PASID". If I understood their meaning correctly, it
>> would be more consistent with the rest.
>>
> I am trying not to mix granu between request w/ PASID and w/o.
> DOMAIN_PAGE meant to be for request w/o PASID.

Is the distinction necessary? I understand the IOMMU side might offer
many possibilities for invalidation, but the user probably doesn't need
all of them. It might be easier to document, upstream and maintain if we
only specify what's currently needed by users (what does QEMU VT-d use?)
Others can always extend it by increasing the version.

Do you think that this invalidation message will be used outside of
BIND_PASID_TABLE context? I can't see an other use but who knows. At the
moment requests w/o PASID are managed with VFIO_IOMMU_MAP/UNMAP_DMA,
which doesn't require invalidation. And in a BIND_PASID_TABLE context,
IOMMUs requests w/o PASID are just a special case using PASID 0 (for Arm
and AMD) so I suppose they'll use the same invalidation commands as
requests w/ PASID.

>>> +   IOMMU_INV_NR_GRANU,
>>> +};
>>> +
>>> +/** enum iommu_inv_type - Generic translation cache types for
>>> invalidation
>>> + *
>>> + * Invalidation requests sent to IOMMU may indicate which
>>> translation cache
>>> + * to be operated on.
>>> + * Combined with enum iommu_inv_granularity, model specific driver
>>> can do a
>>> + * simple lookup to convert generic type to model specific value.
>>> + */
>>> +enum iommu_inv_type {  
>>
>> These should be flags (1 << 0), (1 << 1) etc, since IOMMUs will want
>> to invalidate multiple caches at once (at least DTLB and TLB). You
>> could then do for_each_set_bit in the driver
>>
> I was thinking the invalidation to be inclusive as we discussed earlier
> ,last year :).
> TLB includes DLTB
> PASID cache includes TLB and DTLB. I need to document it better.

Ah right, I guess I was stuck on an old version :) Then the current
values make sense

>>> +   IOMMU_INV_TYPE_DTLB,/* device IOTLB */
>>> +   IOMMU_INV_TYPE_TLB, /* IOMMU paging structure cache
>>> */
>>> +   IOMMU_INV_TYPE_PASID,   /* PASID cache */
>>> +   IOMMU_INV_TYPE_CONTEXT, /* device context entry
>>> cache */
>>> +   IOMMU_INV_NR_TYPE
>>> +};  
>>
>> We need to summarize and explain valid combinations, because reading
>> inv_type_granu_map and inv_type_granu_table is a bit tedious. I tried
>> to reproduce inv_type_granu_map here (Cell format is PASID_TAGGED /
>> !PASID_TAGGED). Could you check if this matches your model?
> great summary. thanks
>>
>> type |   DTLB|TLB|   PASID   |  CONTEXT
>>  granule |   |   |   |
>> -+---+---+---+---
>>- | / Y   | / Y   |   | / Y
> what is this row?

Hm, the arrays in patch 9 have 9 entries, this is entry 0 (for which I
asked if it corresponded to "invalidate all caches" in my previous
reply).

>>  DOMAIN  |   | / Y   |   | / Y
>>  DEVICE  

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jean-Philippe Brucker]

On 23/04/18 21:43, Jacob Pan wrote:
[...]
>> The last name is a bit unfortunate. Since the Arm architecture uses
>> the name "context" for what a PASID points to, "Device cache" would
>> suit us better but it's not important.
>>
> or call it device context cache. actually so far context cache is here
> only for completeness purpose. the expected use case is that QEMU traps
> guest device context cache flush and call bind_pasid_table. 

Right, makes sense

[...]
>> If this corresponds to QI_GRAN_ALL_ALL in patch 9, the comment should
>> be "Cache of all PASIDs"? Or maybe "all entries for all PASIDs"? Is it
>> different from GRANU_DOMAIN then?
> QI_GRAN_ALL_ALL maps to VT-d spec 6.5.2.4, which invalidates all ext
> TLB cache within a domain. It could reuse GRANU_DOMAIN but I was
> also trying to match the naming convention in the spec.

Sorry I don't quite understand the difference between TLB and ext TLB
invalidation. Can an ext TLB invalidation do everything a TLB can do
plus some additional parameters (introduced in more recent version of
the spec), or do they have distinct purposes? I'm trying to understand
why it needs to be user-visible

>>> +   IOMMU_INV_GRANU_PASID_SEL,  /* only invalidate
>>> specified PASID */ +
>>> +   IOMMU_INV_GRANU_NG_ALL_PASID,   /* non-global within
>>> all PASIDs */
>>> +   IOMMU_INV_GRANU_NG_PASID,   /* non-global within a
>>> PASIDs */  
>>
>> Are the "NG" variant needed since there is a
>> IOMMU_INVALIDATE_GLOBAL_PAGE below? We should drop either flag or
>> granule.
>>
>> FWIW I'm starting to think more granule options is actually better
>> than flags, because it flattens the combinations and keeps them to two
>> dimensions, that we can understand and explain with a table.
>>
>>> +   IOMMU_INV_GRANU_PAGE_PASID, /* page-selective
>>> within a PASID */  
>>
>> Maybe this should be called "NG_PAGE_PASID",
> Sure. I was thinking page range already implies non-global pages.
>> and "DOMAIN_PAGE" should
>> instead be "PAGE_PASID". If I understood their meaning correctly, it
>> would be more consistent with the rest.
>>
> I am trying not to mix granu between request w/ PASID and w/o.
> DOMAIN_PAGE meant to be for request w/o PASID.

Is the distinction necessary? I understand the IOMMU side might offer
many possibilities for invalidation, but the user probably doesn't need
all of them. It might be easier to document, upstream and maintain if we
only specify what's currently needed by users (what does QEMU VT-d use?)
Others can always extend it by increasing the version.

Do you think that this invalidation message will be used outside of
BIND_PASID_TABLE context? I can't see an other use but who knows. At the
moment requests w/o PASID are managed with VFIO_IOMMU_MAP/UNMAP_DMA,
which doesn't require invalidation. And in a BIND_PASID_TABLE context,
IOMMUs requests w/o PASID are just a special case using PASID 0 (for Arm
and AMD) so I suppose they'll use the same invalidation commands as
requests w/ PASID.

>>> +   IOMMU_INV_NR_GRANU,
>>> +};
>>> +
>>> +/** enum iommu_inv_type - Generic translation cache types for
>>> invalidation
>>> + *
>>> + * Invalidation requests sent to IOMMU may indicate which
>>> translation cache
>>> + * to be operated on.
>>> + * Combined with enum iommu_inv_granularity, model specific driver
>>> can do a
>>> + * simple lookup to convert generic type to model specific value.
>>> + */
>>> +enum iommu_inv_type {  
>>
>> These should be flags (1 << 0), (1 << 1) etc, since IOMMUs will want
>> to invalidate multiple caches at once (at least DTLB and TLB). You
>> could then do for_each_set_bit in the driver
>>
> I was thinking the invalidation to be inclusive as we discussed earlier
> ,last year :).
> TLB includes DLTB
> PASID cache includes TLB and DTLB. I need to document it better.

Ah right, I guess I was stuck on an old version :) Then the current
values make sense

>>> +   IOMMU_INV_TYPE_DTLB,/* device IOTLB */
>>> +   IOMMU_INV_TYPE_TLB, /* IOMMU paging structure cache
>>> */
>>> +   IOMMU_INV_TYPE_PASID,   /* PASID cache */
>>> +   IOMMU_INV_TYPE_CONTEXT, /* device context entry
>>> cache */
>>> +   IOMMU_INV_NR_TYPE
>>> +};  
>>
>> We need to summarize and explain valid combinations, because reading
>> inv_type_granu_map and inv_type_granu_table is a bit tedious. I tried
>> to reproduce inv_type_granu_map here (Cell format is PASID_TAGGED /
>> !PASID_TAGGED). Could you check if this matches your model?
> great summary. thanks
>>
>> type |   DTLB|TLB|   PASID   |  CONTEXT
>>  granule |   |   |   |
>> -+---+---+---+---
>>- | / Y   | / Y   |   | / Y
> what is this row?

Hm, the arrays in patch 9 have 9 entries, this is entry 0 (for which I
asked if it corresponded to "invalidate all caches" in my previous
reply).

>>  DOMAIN  |   | / Y   |   | / Y
>>  DEVICE  

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

On Fri, 20 Apr 2018 19:19:54 +0100
Jean-Philippe Brucker  wrote:

> Hi Jacob,
> 
> On Mon, Apr 16, 2018 at 10:48:54PM +0100, Jacob Pan wrote:
> [...]
> > +/**
> > + * enum iommu_inv_granularity - Generic invalidation granularity
> > + *
> > + * When an invalidation request is sent to IOMMU to flush
> > translation caches,
> > + * it may carry different granularity. These granularity levels
> > are not specific
> > + * to a type of translation cache. For an example, PASID selective
> > granularity
> > + * is only applicable to PASID cache invalidation.  
> 
> I'm still confused by this, I think we should add more definitions
> because architectures tend to use different names. What you call
> "Translations caches" encompasses all caches that can be invalidated
> with this request, right? So all of:
> 
yes correct.
> * "TLB" and "DTLB" that cache IOVA->GPA and GPA->PA (TLB is in the
>   IOMMU, DTLB is an ATC in an endpoint),
> * "PASID cache" that cache PASID->Translation Table,
> * "Context cache" that cache RID->PASID table
> 
> Does this match the model you're using?
> 
yes. PASID cache and context caches are in the IOMMU.
> The last name is a bit unfortunate. Since the Arm architecture uses
> the name "context" for what a PASID points to, "Device cache" would
> suit us better but it's not important.
> 
or call it device context cache. actually so far context cache is here
only for completeness purpose. the expected use case is that QEMU traps
guest device context cache flush and call bind_pasid_table.

> I don't understand what you mean by "PASID selective granularity is
> only applicable to PASID cache invalidation", it seems to contradict
> the preceding sentence.
You are right. That was a mistake. I meant to say "These granularity
levels are specific to a type of"

> What if user sends an invalidation with
> IOMMU_INV_TYPE_TLB and IOMMU_INV_GRANU_ALL_PASID? Doesn't this remove
> from the TLBs all entries with the given PASID?
> 
No, this meant to invalidate all PASID of a given domain ID. I need to
correct the description.
The dilemma here is to map model specific fields into generic list. not
all combinations are legal.
> > + * This enum is a collection of granularities for all types of
> > translation
> > + * caches. The idea is to make it easy for IOMMU model specific
> > driver do
> > + * conversion from generic to model specific value.
> > + */
> > +enum iommu_inv_granularity {  
> 
> In patch 9, inv_type_granu_map has some valid fields with granularity
> == 0. Does it mean "invalidate all caches"?
> 
> I don't think user should ever be allowed to invalidate caches entries
> of devices and domains it doesn't own.
> 
Agreed, I removed global granu to avoid device invalidation beyond
device itself. But I missed some of the fields in
inv_type_granu_map{}. 
> > +   IOMMU_INV_GRANU_DOMAIN = 1, /* all TLBs associated
> > with a domain */
> > +   IOMMU_INV_GRANU_DEVICE, /* caching
> > structure associated with a
> > +* device ID
> > +*/
> > +   IOMMU_INV_GRANU_DOMAIN_PAGE,/* address range with
> > a domain */  
> 
> > +   IOMMU_INV_GRANU_ALL_PASID,  /* cache of a given
> > PASID */  
> 
> If this corresponds to QI_GRAN_ALL_ALL in patch 9, the comment should
> be "Cache of all PASIDs"? Or maybe "all entries for all PASIDs"? Is it
> different from GRANU_DOMAIN then?
QI_GRAN_ALL_ALL maps to VT-d spec 6.5.2.4, which invalidates all ext
TLB cache within a domain. It could reuse GRANU_DOMAIN but I was
also trying to match the naming convention in the spec.

> > +   IOMMU_INV_GRANU_PASID_SEL,  /* only invalidate
> > specified PASID */ +
> > +   IOMMU_INV_GRANU_NG_ALL_PASID,   /* non-global within
> > all PASIDs */
> > +   IOMMU_INV_GRANU_NG_PASID,   /* non-global within a
> > PASIDs */  
> 
> Are the "NG" variant needed since there is a
> IOMMU_INVALIDATE_GLOBAL_PAGE below? We should drop either flag or
> granule.
> 
> FWIW I'm starting to think more granule options is actually better
> than flags, because it flattens the combinations and keeps them to two
> dimensions, that we can understand and explain with a table.
> 
> > +   IOMMU_INV_GRANU_PAGE_PASID, /* page-selective
> > within a PASID */  
> 
> Maybe this should be called "NG_PAGE_PASID",
Sure. I was thinking page range already implies non-global pages.
> and "DOMAIN_PAGE" should
> instead be "PAGE_PASID". If I understood their meaning correctly, it
> would be more consistent with the rest.
> 
I am trying not to mix granu between request w/ PASID and w/o.
DOMAIN_PAGE meant to be for request w/o PASID.

> > +   IOMMU_INV_NR_GRANU,
> > +};
> > +
> > +/** enum iommu_inv_type - Generic translation cache types for
> > invalidation
> > + *
> > + * Invalidation requests sent to IOMMU may indicate which
> > translation cache
> > + * to be operated on.
> > + * Combined with enum iommu_inv_granularity, model specific driver
> > 

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

On Fri, 20 Apr 2018 19:19:54 +0100
Jean-Philippe Brucker  wrote:

> Hi Jacob,
> 
> On Mon, Apr 16, 2018 at 10:48:54PM +0100, Jacob Pan wrote:
> [...]
> > +/**
> > + * enum iommu_inv_granularity - Generic invalidation granularity
> > + *
> > + * When an invalidation request is sent to IOMMU to flush
> > translation caches,
> > + * it may carry different granularity. These granularity levels
> > are not specific
> > + * to a type of translation cache. For an example, PASID selective
> > granularity
> > + * is only applicable to PASID cache invalidation.  
> 
> I'm still confused by this, I think we should add more definitions
> because architectures tend to use different names. What you call
> "Translations caches" encompasses all caches that can be invalidated
> with this request, right? So all of:
> 
yes correct.
> * "TLB" and "DTLB" that cache IOVA->GPA and GPA->PA (TLB is in the
>   IOMMU, DTLB is an ATC in an endpoint),
> * "PASID cache" that cache PASID->Translation Table,
> * "Context cache" that cache RID->PASID table
> 
> Does this match the model you're using?
> 
yes. PASID cache and context caches are in the IOMMU.
> The last name is a bit unfortunate. Since the Arm architecture uses
> the name "context" for what a PASID points to, "Device cache" would
> suit us better but it's not important.
> 
or call it device context cache. actually so far context cache is here
only for completeness purpose. the expected use case is that QEMU traps
guest device context cache flush and call bind_pasid_table.

> I don't understand what you mean by "PASID selective granularity is
> only applicable to PASID cache invalidation", it seems to contradict
> the preceding sentence.
You are right. That was a mistake. I meant to say "These granularity
levels are specific to a type of"

> What if user sends an invalidation with
> IOMMU_INV_TYPE_TLB and IOMMU_INV_GRANU_ALL_PASID? Doesn't this remove
> from the TLBs all entries with the given PASID?
> 
No, this meant to invalidate all PASID of a given domain ID. I need to
correct the description.
The dilemma here is to map model specific fields into generic list. not
all combinations are legal.
> > + * This enum is a collection of granularities for all types of
> > translation
> > + * caches. The idea is to make it easy for IOMMU model specific
> > driver do
> > + * conversion from generic to model specific value.
> > + */
> > +enum iommu_inv_granularity {  
> 
> In patch 9, inv_type_granu_map has some valid fields with granularity
> == 0. Does it mean "invalidate all caches"?
> 
> I don't think user should ever be allowed to invalidate caches entries
> of devices and domains it doesn't own.
> 
Agreed, I removed global granu to avoid device invalidation beyond
device itself. But I missed some of the fields in
inv_type_granu_map{}. 
> > +   IOMMU_INV_GRANU_DOMAIN = 1, /* all TLBs associated
> > with a domain */
> > +   IOMMU_INV_GRANU_DEVICE, /* caching
> > structure associated with a
> > +* device ID
> > +*/
> > +   IOMMU_INV_GRANU_DOMAIN_PAGE,/* address range with
> > a domain */  
> 
> > +   IOMMU_INV_GRANU_ALL_PASID,  /* cache of a given
> > PASID */  
> 
> If this corresponds to QI_GRAN_ALL_ALL in patch 9, the comment should
> be "Cache of all PASIDs"? Or maybe "all entries for all PASIDs"? Is it
> different from GRANU_DOMAIN then?
QI_GRAN_ALL_ALL maps to VT-d spec 6.5.2.4, which invalidates all ext
TLB cache within a domain. It could reuse GRANU_DOMAIN but I was
also trying to match the naming convention in the spec.

> > +   IOMMU_INV_GRANU_PASID_SEL,  /* only invalidate
> > specified PASID */ +
> > +   IOMMU_INV_GRANU_NG_ALL_PASID,   /* non-global within
> > all PASIDs */
> > +   IOMMU_INV_GRANU_NG_PASID,   /* non-global within a
> > PASIDs */  
> 
> Are the "NG" variant needed since there is a
> IOMMU_INVALIDATE_GLOBAL_PAGE below? We should drop either flag or
> granule.
> 
> FWIW I'm starting to think more granule options is actually better
> than flags, because it flattens the combinations and keeps them to two
> dimensions, that we can understand and explain with a table.
> 
> > +   IOMMU_INV_GRANU_PAGE_PASID, /* page-selective
> > within a PASID */  
> 
> Maybe this should be called "NG_PAGE_PASID",
Sure. I was thinking page range already implies non-global pages.
> and "DOMAIN_PAGE" should
> instead be "PAGE_PASID". If I understood their meaning correctly, it
> would be more consistent with the rest.
> 
I am trying not to mix granu between request w/ PASID and w/o.
DOMAIN_PAGE meant to be for request w/o PASID.

> > +   IOMMU_INV_NR_GRANU,
> > +};
> > +
> > +/** enum iommu_inv_type - Generic translation cache types for
> > invalidation
> > + *
> > + * Invalidation requests sent to IOMMU may indicate which
> > translation cache
> > + * to be operated on.
> > + * Combined with enum iommu_inv_granularity, model specific driver
> > can do a
> > + * simple lookup 

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jean-Philippe Brucker]

Hi Jacob,

On Mon, Apr 16, 2018 at 10:48:54PM +0100, Jacob Pan wrote:
[...]
> +/**
> + * enum iommu_inv_granularity - Generic invalidation granularity
> + *
> + * When an invalidation request is sent to IOMMU to flush translation caches,
> + * it may carry different granularity. These granularity levels are not 
> specific
> + * to a type of translation cache. For an example, PASID selective 
> granularity
> + * is only applicable to PASID cache invalidation.

I'm still confused by this, I think we should add more definitions
because architectures tend to use different names. What you call
"Translations caches" encompasses all caches that can be invalidated
with this request, right? So all of:

* "TLB" and "DTLB" that cache IOVA->GPA and GPA->PA (TLB is in the
  IOMMU, DTLB is an ATC in an endpoint),
* "PASID cache" that cache PASID->Translation Table,
* "Context cache" that cache RID->PASID table

Does this match the model you're using?

The last name is a bit unfortunate. Since the Arm architecture uses the
name "context" for what a PASID points to, "Device cache" would suit us
better but it's not important.

I don't understand what you mean by "PASID selective granularity is only
applicable to PASID cache invalidation", it seems to contradict the
preceding sentence. What if user sends an invalidation with
IOMMU_INV_TYPE_TLB and IOMMU_INV_GRANU_ALL_PASID? Doesn't this remove
from the TLBs all entries with the given PASID?

> + * This enum is a collection of granularities for all types of translation
> + * caches. The idea is to make it easy for IOMMU model specific driver do
> + * conversion from generic to model specific value.
> + */
> +enum iommu_inv_granularity {

In patch 9, inv_type_granu_map has some valid fields with granularity ==
0. Does it mean "invalidate all caches"?

I don't think user should ever be allowed to invalidate caches entries
of devices and domains it doesn't own.

> + IOMMU_INV_GRANU_DOMAIN = 1, /* all TLBs associated with a domain */
> + IOMMU_INV_GRANU_DEVICE, /* caching structure associated with a
> +  * device ID
> +  */
> + IOMMU_INV_GRANU_DOMAIN_PAGE,/* address range with a domain */

> + IOMMU_INV_GRANU_ALL_PASID,  /* cache of a given PASID */

If this corresponds to QI_GRAN_ALL_ALL in patch 9, the comment should be
"Cache of all PASIDs"? Or maybe "all entries for all PASIDs"? Is it
different from GRANU_DOMAIN then?

> + IOMMU_INV_GRANU_PASID_SEL,  /* only invalidate specified PASID */
> +
> + IOMMU_INV_GRANU_NG_ALL_PASID,   /* non-global within all PASIDs */
> + IOMMU_INV_GRANU_NG_PASID,   /* non-global within a PASIDs */

Are the "NG" variant needed since there is a IOMMU_INVALIDATE_GLOBAL_PAGE
below? We should drop either flag or granule.

FWIW I'm starting to think more granule options is actually better than
flags, because it flattens the combinations and keeps them to two
dimensions, that we can understand and explain with a table.

> + IOMMU_INV_GRANU_PAGE_PASID, /* page-selective within a PASID */

Maybe this should be called "NG_PAGE_PASID", and "DOMAIN_PAGE" should
instead be "PAGE_PASID". If I understood their meaning correctly, it
would be more consistent with the rest.

> + IOMMU_INV_NR_GRANU,
> +};
> +
> +/** enum iommu_inv_type - Generic translation cache types for invalidation
> + *
> + * Invalidation requests sent to IOMMU may indicate which translation cache
> + * to be operated on.
> + * Combined with enum iommu_inv_granularity, model specific driver can do a
> + * simple lookup to convert generic type to model specific value.
> + */
> +enum iommu_inv_type {

These should be flags (1 << 0), (1 << 1) etc, since IOMMUs will want to
invalidate multiple caches at once (at least DTLB and TLB). You could
then do for_each_set_bit in the driver

> + IOMMU_INV_TYPE_DTLB,/* device IOTLB */
> + IOMMU_INV_TYPE_TLB, /* IOMMU paging structure cache */
> + IOMMU_INV_TYPE_PASID,   /* PASID cache */
> + IOMMU_INV_TYPE_CONTEXT, /* device context entry cache */
> + IOMMU_INV_NR_TYPE
> +};

We need to summarize and explain valid combinations, because reading
inv_type_granu_map and inv_type_granu_table is a bit tedious. I tried to
reproduce inv_type_granu_map here (Cell format is PASID_TAGGED /
!PASID_TAGGED). Could you check if this matches your model?

type |   DTLB|TLB|   PASID   |  CONTEXT
 granule |   |   |   |
-+---+---+---+---
   - | / Y   | / Y   |   | / Y
 DOMAIN  |   | / Y   |   | / Y
 DEVICE  |   |   |   | / Y
 DOMAIN_PAGE |   | / Y   |   |
 ALL_PASID   |   Y   |   Y   |   |
 PASID_SEL   |   Y   |   |   Y   |
 NG_ALL_PASID|

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jean-Philippe Brucker]

Hi Jacob,

On Mon, Apr 16, 2018 at 10:48:54PM +0100, Jacob Pan wrote:
[...]
> +/**
> + * enum iommu_inv_granularity - Generic invalidation granularity
> + *
> + * When an invalidation request is sent to IOMMU to flush translation caches,
> + * it may carry different granularity. These granularity levels are not 
> specific
> + * to a type of translation cache. For an example, PASID selective 
> granularity
> + * is only applicable to PASID cache invalidation.

I'm still confused by this, I think we should add more definitions
because architectures tend to use different names. What you call
"Translations caches" encompasses all caches that can be invalidated
with this request, right? So all of:

* "TLB" and "DTLB" that cache IOVA->GPA and GPA->PA (TLB is in the
  IOMMU, DTLB is an ATC in an endpoint),
* "PASID cache" that cache PASID->Translation Table,
* "Context cache" that cache RID->PASID table

Does this match the model you're using?

The last name is a bit unfortunate. Since the Arm architecture uses the
name "context" for what a PASID points to, "Device cache" would suit us
better but it's not important.

I don't understand what you mean by "PASID selective granularity is only
applicable to PASID cache invalidation", it seems to contradict the
preceding sentence. What if user sends an invalidation with
IOMMU_INV_TYPE_TLB and IOMMU_INV_GRANU_ALL_PASID? Doesn't this remove
from the TLBs all entries with the given PASID?

> + * This enum is a collection of granularities for all types of translation
> + * caches. The idea is to make it easy for IOMMU model specific driver do
> + * conversion from generic to model specific value.
> + */
> +enum iommu_inv_granularity {

In patch 9, inv_type_granu_map has some valid fields with granularity ==
0. Does it mean "invalidate all caches"?

I don't think user should ever be allowed to invalidate caches entries
of devices and domains it doesn't own.

> + IOMMU_INV_GRANU_DOMAIN = 1, /* all TLBs associated with a domain */
> + IOMMU_INV_GRANU_DEVICE, /* caching structure associated with a
> +  * device ID
> +  */
> + IOMMU_INV_GRANU_DOMAIN_PAGE,/* address range with a domain */

> + IOMMU_INV_GRANU_ALL_PASID,  /* cache of a given PASID */

If this corresponds to QI_GRAN_ALL_ALL in patch 9, the comment should be
"Cache of all PASIDs"? Or maybe "all entries for all PASIDs"? Is it
different from GRANU_DOMAIN then?

> + IOMMU_INV_GRANU_PASID_SEL,  /* only invalidate specified PASID */
> +
> + IOMMU_INV_GRANU_NG_ALL_PASID,   /* non-global within all PASIDs */
> + IOMMU_INV_GRANU_NG_PASID,   /* non-global within a PASIDs */

Are the "NG" variant needed since there is a IOMMU_INVALIDATE_GLOBAL_PAGE
below? We should drop either flag or granule.

FWIW I'm starting to think more granule options is actually better than
flags, because it flattens the combinations and keeps them to two
dimensions, that we can understand and explain with a table.

> + IOMMU_INV_GRANU_PAGE_PASID, /* page-selective within a PASID */

Maybe this should be called "NG_PAGE_PASID", and "DOMAIN_PAGE" should
instead be "PAGE_PASID". If I understood their meaning correctly, it
would be more consistent with the rest.

> + IOMMU_INV_NR_GRANU,
> +};
> +
> +/** enum iommu_inv_type - Generic translation cache types for invalidation
> + *
> + * Invalidation requests sent to IOMMU may indicate which translation cache
> + * to be operated on.
> + * Combined with enum iommu_inv_granularity, model specific driver can do a
> + * simple lookup to convert generic type to model specific value.
> + */
> +enum iommu_inv_type {

These should be flags (1 << 0), (1 << 1) etc, since IOMMUs will want to
invalidate multiple caches at once (at least DTLB and TLB). You could
then do for_each_set_bit in the driver

> + IOMMU_INV_TYPE_DTLB,/* device IOTLB */
> + IOMMU_INV_TYPE_TLB, /* IOMMU paging structure cache */
> + IOMMU_INV_TYPE_PASID,   /* PASID cache */
> + IOMMU_INV_TYPE_CONTEXT, /* device context entry cache */
> + IOMMU_INV_NR_TYPE
> +};

We need to summarize and explain valid combinations, because reading
inv_type_granu_map and inv_type_granu_table is a bit tedious. I tried to
reproduce inv_type_granu_map here (Cell format is PASID_TAGGED /
!PASID_TAGGED). Could you check if this matches your model?

type |   DTLB|TLB|   PASID   |  CONTEXT
 granule |   |   |   |
-+---+---+---+---
   - | / Y   | / Y   |   | / Y
 DOMAIN  |   | / Y   |   | / Y
 DEVICE  |   |   |   | / Y
 DOMAIN_PAGE |   | / Y   |   |
 ALL_PASID   |   Y   |   Y   |   |
 PASID_SEL   |   Y   |   |   Y   |
 NG_ALL_PASID|

[PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

From: "Liu, Yi L" 

When an 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 passing them down to the
host/physical IOMMU.
Since the invalidation data are obtained from user space and will be
written into physical IOMMU, we must allow security check at various
layers. Therefore, generic invalidation data format are proposed here,
model specific IOMMU drivers need to convert them into their own format.

Signed-off-by: Liu, Yi L 
Signed-off-by: Jean-Philippe Brucker 
Signed-off-by: Jacob Pan 
Signed-off-by: Ashok Raj 
---
 drivers/iommu/iommu.c  | 14 
 include/linux/iommu.h  | 12 +++
 include/uapi/linux/iommu.h | 79 ++
 3 files changed, 105 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3a69620..784e019 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1344,6 +1344,20 @@ void iommu_unbind_pasid_table(struct iommu_domain 
*domain, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_unbind_pasid_table);
 
+int iommu_sva_invalidate(struct iommu_domain *domain,
+   struct device *dev, struct tlb_invalidate_info *inv_info)
+{
+   int ret = 0;
+
+   if (unlikely(!domain->ops->sva_invalidate))
+   return -ENODEV;
+
+   ret = domain->ops->sva_invalidate(domain, dev, inv_info);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_sva_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 8ad111f..e963dbd 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -190,6 +190,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
+ * @sva_invalidate: invalidate translation caches of shared virtual address
  */
 struct iommu_ops {
bool (*capable)(enum iommu_cap);
@@ -243,6 +244,8 @@ struct iommu_ops {
struct pasid_table_config *pasidt_binfo);
void (*unbind_pasid_table)(struct iommu_domain *domain,
struct device *dev);
+   int (*sva_invalidate)(struct iommu_domain *domain,
+   struct device *dev, struct tlb_invalidate_info *inv_info);
 
unsigned long pgsize_bitmap;
 };
@@ -309,6 +312,9 @@ extern int iommu_bind_pasid_table(struct iommu_domain 
*domain,
struct device *dev, struct pasid_table_config *pasidt_binfo);
 extern void iommu_unbind_pasid_table(struct iommu_domain *domain,
struct device *dev);
+extern int iommu_sva_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);
@@ -720,6 +726,12 @@ void iommu_unbind_pasid_table(struct iommu_domain *domain, 
struct device *dev)
 {
 }
 
+static inline int iommu_sva_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 9f7a6bf..4447943 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -29,4 +29,83 @@ struct pasid_table_config {
__u8 pasid_bits;
 };
 
+/**
+ * enum iommu_inv_granularity - Generic invalidation granularity
+ *
+ * When an invalidation request is sent to IOMMU to flush translation caches,
+ * it may carry different granularity. These granularity levels are not 
specific
+ * to a type of translation cache. For an example, PASID selective granularity
+ * is only applicable to PASID cache invalidation.
+ * This enum is a collection of granularities for all types of translation
+ * caches. The idea is to make it easy for IOMMU model specific driver do
+ * conversion from generic to model specific value.
+ */
+enum iommu_inv_granularity {
+   IOMMU_INV_GRANU_DOMAIN = 1, /* all TLBs associated with a domain */
+   IOMMU_INV_GRANU_DEVICE, /* caching structure associated with a
+   

[PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

From: "Liu, Yi L" 

When an 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 passing them down to the
host/physical IOMMU.
Since the invalidation data are obtained from user space and will be
written into physical IOMMU, we must allow security check at various
layers. Therefore, generic invalidation data format are proposed here,
model specific IOMMU drivers need to convert them into their own format.

Signed-off-by: Liu, Yi L 
Signed-off-by: Jean-Philippe Brucker 
Signed-off-by: Jacob Pan 
Signed-off-by: Ashok Raj 
---
 drivers/iommu/iommu.c  | 14 
 include/linux/iommu.h  | 12 +++
 include/uapi/linux/iommu.h | 79 ++
 3 files changed, 105 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3a69620..784e019 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1344,6 +1344,20 @@ void iommu_unbind_pasid_table(struct iommu_domain 
*domain, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_unbind_pasid_table);
 
+int iommu_sva_invalidate(struct iommu_domain *domain,
+   struct device *dev, struct tlb_invalidate_info *inv_info)
+{
+   int ret = 0;
+
+   if (unlikely(!domain->ops->sva_invalidate))
+   return -ENODEV;
+
+   ret = domain->ops->sva_invalidate(domain, dev, inv_info);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_sva_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 8ad111f..e963dbd 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -190,6 +190,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
+ * @sva_invalidate: invalidate translation caches of shared virtual address
  */
 struct iommu_ops {
bool (*capable)(enum iommu_cap);
@@ -243,6 +244,8 @@ struct iommu_ops {
struct pasid_table_config *pasidt_binfo);
void (*unbind_pasid_table)(struct iommu_domain *domain,
struct device *dev);
+   int (*sva_invalidate)(struct iommu_domain *domain,
+   struct device *dev, struct tlb_invalidate_info *inv_info);
 
unsigned long pgsize_bitmap;
 };
@@ -309,6 +312,9 @@ extern int iommu_bind_pasid_table(struct iommu_domain 
*domain,
struct device *dev, struct pasid_table_config *pasidt_binfo);
 extern void iommu_unbind_pasid_table(struct iommu_domain *domain,
struct device *dev);
+extern int iommu_sva_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);
@@ -720,6 +726,12 @@ void iommu_unbind_pasid_table(struct iommu_domain *domain, 
struct device *dev)
 {
 }
 
+static inline int iommu_sva_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 9f7a6bf..4447943 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -29,4 +29,83 @@ struct pasid_table_config {
__u8 pasid_bits;
 };
 
+/**
+ * enum iommu_inv_granularity - Generic invalidation granularity
+ *
+ * When an invalidation request is sent to IOMMU to flush translation caches,
+ * it may carry different granularity. These granularity levels are not 
specific
+ * to a type of translation cache. For an example, PASID selective granularity
+ * is only applicable to PASID cache invalidation.
+ * This enum is a collection of granularities for all types of translation
+ * caches. The idea is to make it easy for IOMMU model specific driver do
+ * conversion from generic to model specific value.
+ */
+enum iommu_inv_granularity {
+   IOMMU_INV_GRANU_DOMAIN = 1, /* all TLBs associated with a domain */
+   IOMMU_INV_GRANU_DEVICE, /* caching structure associated with a
+* device ID
+*/
+   IOMMU_INV_GRANU_DOMAIN_PAGE,/* address range with 

[PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

From: "Liu, Yi L" 

When an 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 passing them down to the
host/physical IOMMU.
Since the invalidation data are obtained from user space and will be
written into physical IOMMU, we must allow security check at various
layers. Therefore, generic invalidation data format are proposed here,
model specific IOMMU drivers need to convert them into their own format.

Signed-off-by: Liu, Yi L 
Signed-off-by: Jean-Philippe Brucker 
Signed-off-by: Jacob Pan 
Signed-off-by: Ashok Raj 
---
 drivers/iommu/iommu.c  | 14 
 include/linux/iommu.h  | 12 +++
 include/uapi/linux/iommu.h | 79 ++
 3 files changed, 105 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 734ed09..466f789 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1344,6 +1344,20 @@ void iommu_unbind_pasid_table(struct iommu_domain 
*domain, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_unbind_pasid_table);
 
+int iommu_sva_invalidate(struct iommu_domain *domain,
+   struct device *dev, struct tlb_invalidate_info *inv_info)
+{
+   int ret = 0;
+
+   if (unlikely(!domain->ops->sva_invalidate))
+   return -ENODEV;
+
+   ret = domain->ops->sva_invalidate(domain, dev, inv_info);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_sva_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 0f6f6c5..da684a7 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -190,6 +190,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
+ * @sva_invalidate: invalidate translation caches of shared virtual address
  */
 struct iommu_ops {
bool (*capable)(enum iommu_cap);
@@ -243,6 +244,8 @@ struct iommu_ops {
struct pasid_table_config *pasidt_binfo);
void (*unbind_pasid_table)(struct iommu_domain *domain,
struct device *dev);
+   int (*sva_invalidate)(struct iommu_domain *domain,
+   struct device *dev, struct tlb_invalidate_info *inv_info);
 
unsigned long pgsize_bitmap;
 };
@@ -309,6 +312,9 @@ extern int iommu_bind_pasid_table(struct iommu_domain 
*domain,
struct device *dev, struct pasid_table_config *pasidt_binfo);
 extern void iommu_unbind_pasid_table(struct iommu_domain *domain,
struct device *dev);
+extern int iommu_sva_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);
@@ -720,6 +726,12 @@ void iommu_unbind_pasid_table(struct iommu_domain *domain, 
struct device *dev)
 {
 }
 
+static inline int iommu_sva_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 9f7a6bf..4447943 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -29,4 +29,83 @@ struct pasid_table_config {
__u8 pasid_bits;
 };
 
+/**
+ * enum iommu_inv_granularity - Generic invalidation granularity
+ *
+ * When an invalidation request is sent to IOMMU to flush translation caches,
+ * it may carry different granularity. These granularity levels are not 
specific
+ * to a type of translation cache. For an example, PASID selective granularity
+ * is only applicable to PASID cache invalidation.
+ * This enum is a collection of granularities for all types of translation
+ * caches. The idea is to make it easy for IOMMU model specific driver do
+ * conversion from generic to model specific value.
+ */
+enum iommu_inv_granularity {
+   IOMMU_INV_GRANU_DOMAIN = 1, /* all TLBs associated with a domain */
+   IOMMU_INV_GRANU_DEVICE, /* caching structure associated with a
+   

[PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

From: "Liu, Yi L" 

When an 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 passing them down to the
host/physical IOMMU.
Since the invalidation data are obtained from user space and will be
written into physical IOMMU, we must allow security check at various
layers. Therefore, generic invalidation data format are proposed here,
model specific IOMMU drivers need to convert them into their own format.

Signed-off-by: Liu, Yi L 
Signed-off-by: Jean-Philippe Brucker 
Signed-off-by: Jacob Pan 
Signed-off-by: Ashok Raj 
---
 drivers/iommu/iommu.c  | 14 
 include/linux/iommu.h  | 12 +++
 include/uapi/linux/iommu.h | 79 ++
 3 files changed, 105 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 734ed09..466f789 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1344,6 +1344,20 @@ void iommu_unbind_pasid_table(struct iommu_domain 
*domain, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_unbind_pasid_table);
 
+int iommu_sva_invalidate(struct iommu_domain *domain,
+   struct device *dev, struct tlb_invalidate_info *inv_info)
+{
+   int ret = 0;
+
+   if (unlikely(!domain->ops->sva_invalidate))
+   return -ENODEV;
+
+   ret = domain->ops->sva_invalidate(domain, dev, inv_info);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_sva_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 0f6f6c5..da684a7 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -190,6 +190,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
+ * @sva_invalidate: invalidate translation caches of shared virtual address
  */
 struct iommu_ops {
bool (*capable)(enum iommu_cap);
@@ -243,6 +244,8 @@ struct iommu_ops {
struct pasid_table_config *pasidt_binfo);
void (*unbind_pasid_table)(struct iommu_domain *domain,
struct device *dev);
+   int (*sva_invalidate)(struct iommu_domain *domain,
+   struct device *dev, struct tlb_invalidate_info *inv_info);
 
unsigned long pgsize_bitmap;
 };
@@ -309,6 +312,9 @@ extern int iommu_bind_pasid_table(struct iommu_domain 
*domain,
struct device *dev, struct pasid_table_config *pasidt_binfo);
 extern void iommu_unbind_pasid_table(struct iommu_domain *domain,
struct device *dev);
+extern int iommu_sva_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);
@@ -720,6 +726,12 @@ void iommu_unbind_pasid_table(struct iommu_domain *domain, 
struct device *dev)
 {
 }
 
+static inline int iommu_sva_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 9f7a6bf..4447943 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -29,4 +29,83 @@ struct pasid_table_config {
__u8 pasid_bits;
 };
 
+/**
+ * enum iommu_inv_granularity - Generic invalidation granularity
+ *
+ * When an invalidation request is sent to IOMMU to flush translation caches,
+ * it may carry different granularity. These granularity levels are not 
specific
+ * to a type of translation cache. For an example, PASID selective granularity
+ * is only applicable to PASID cache invalidation.
+ * This enum is a collection of granularities for all types of translation
+ * caches. The idea is to make it easy for IOMMU model specific driver do
+ * conversion from generic to model specific value.
+ */
+enum iommu_inv_granularity {
+   IOMMU_INV_GRANU_DOMAIN = 1, /* all TLBs associated with a domain */
+   IOMMU_INV_GRANU_DEVICE, /* caching structure associated with a
+* device ID
+*/
+   IOMMU_INV_GRANU_DOMAIN_PAGE,/* address range with