[PATCH] VT-d: fix caching mode IOTLB flushing

[PATCH] VT-d: fix caching mode IOTLB flushing

[Jan Beulich]

While for context cache entry flushing use of did 0 is indeed correct
(after all upon reading the context entry the IOMMU wouldn't know any
domain ID if the entry is not present, and hence a surrogate one needs
to be used), for IOTLB entries the normal domain ID (from the [present]
context entry) gets used. See sub-section "IOTLB" of section "Address
Translation Caches" in the VT-d spec.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Luckily this is supposed to be an issue only when running on emulated
IOMMUs; hardware implementations are expected to have CAP.CM=0.

--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -474,17 +474,10 @@ int vtd_flush_iotlb_reg(struct vtd_iommu
 
     /*
      * In the non-present entry flush case, if hardware doesn't cache
-     * non-present entry we do nothing and if hardware cache non-present
-     * entry, we flush entries of domain 0 (the domain id is used to cache
-     * any non-present entries)
+     * non-present entries we do nothing.
      */
-    if ( flush_non_present_entry )
-    {
-        if ( !cap_caching_mode(iommu->cap) )
-            return 1;
-        else
-            did = 0;
-    }
+    if ( flush_non_present_entry && !cap_caching_mode(iommu->cap) )
+        return 1;
 
     /* use register invalidation */
     switch ( type )
--- a/xen/drivers/passthrough/vtd/qinval.c
+++ b/xen/drivers/passthrough/vtd/qinval.c
@@ -362,17 +362,10 @@ static int __must_check flush_iotlb_qi(s
 
     /*
      * In the non-present entry flush case, if hardware doesn't cache
-     * non-present entry we do nothing and if hardware cache non-present
-     * entry, we flush entries of domain 0 (the domain id is used to cache
-     * any non-present entries)
+     * non-present entries we do nothing.
      */
-    if ( flush_non_present_entry )
-    {
-        if ( !cap_caching_mode(iommu->cap) )
-            return 1;
-        else
-            did = 0;
-    }
+    if ( flush_non_present_entry && !cap_caching_mode(iommu->cap) )
+        return 1;
 
     /* use queued invalidation */
     if (cap_write_drain(iommu->cap))

Re: [PATCH] VT-d: fix caching mode IOTLB flushing

[Paul Durrant]

On 19/08/2021 09:05, Jan Beulich wrote:
> While for context cache entry flushing use of did 0 is indeed correct
> (after all upon reading the context entry the IOMMU wouldn't know any
> domain ID if the entry is not present, and hence a surrogate one needs
> to be used), for IOTLB entries the normal domain ID (from the [present]
> context entry) gets used. See sub-section "IOTLB" of section "Address
> Translation Caches" in the VT-d spec.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Paul Durrant <paul@xen.org>

RE: [PATCH] VT-d: fix caching mode IOTLB flushing

[Tian, Kevin]

> From: Jan Beulich <jbeulich@suse.com>
> Sent: Thursday, August 19, 2021 4:06 PM
> 
> While for context cache entry flushing use of did 0 is indeed correct
> (after all upon reading the context entry the IOMMU wouldn't know any
> domain ID if the entry is not present, and hence a surrogate one needs
> to be used), for IOTLB entries the normal domain ID (from the [present]
> context entry) gets used. See sub-section "IOTLB" of section "Address
> Translation Caches" in the VT-d spec.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

> ---
> Luckily this is supposed to be an issue only when running on emulated
> IOMMUs; hardware implementations are expected to have CAP.CM=0.
> 
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -474,17 +474,10 @@ int vtd_flush_iotlb_reg(struct vtd_iommu
> 
>      /*
>       * In the non-present entry flush case, if hardware doesn't cache
> -     * non-present entry we do nothing and if hardware cache non-present
> -     * entry, we flush entries of domain 0 (the domain id is used to cache
> -     * any non-present entries)
> +     * non-present entries we do nothing.
>       */
> -    if ( flush_non_present_entry )
> -    {
> -        if ( !cap_caching_mode(iommu->cap) )
> -            return 1;
> -        else
> -            did = 0;
> -    }
> +    if ( flush_non_present_entry && !cap_caching_mode(iommu->cap) )
> +        return 1;
> 
>      /* use register invalidation */
>      switch ( type )
> --- a/xen/drivers/passthrough/vtd/qinval.c
> +++ b/xen/drivers/passthrough/vtd/qinval.c
> @@ -362,17 +362,10 @@ static int __must_check flush_iotlb_qi(s
> 
>      /*
>       * In the non-present entry flush case, if hardware doesn't cache
> -     * non-present entry we do nothing and if hardware cache non-present
> -     * entry, we flush entries of domain 0 (the domain id is used to cache
> -     * any non-present entries)
> +     * non-present entries we do nothing.
>       */
> -    if ( flush_non_present_entry )
> -    {
> -        if ( !cap_caching_mode(iommu->cap) )
> -            return 1;
> -        else
> -            did = 0;
> -    }
> +    if ( flush_non_present_entry && !cap_caching_mode(iommu->cap) )
> +        return 1;
> 
>      /* use queued invalidation */
>      if (cap_write_drain(iommu->cap))