Hidden devices are associated with DomXEN but usable by the
hardware domain. Hence they need flushing as well when all devices are
to have flushes invoked.

While there drop a redundant ATS-enabled check and constify the first
parameter of the involved function.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/passthrough/amd/iommu_cmd.c
+++ b/xen/drivers/passthrough/amd/iommu_cmd.c
@@ -308,14 +308,11 @@ void amd_iommu_flush_iotlb(u8 devfn, con
     flush_command_buffer(iommu, iommu_dev_iotlb_timeout);
 }
 
-static void amd_iommu_flush_all_iotlbs(struct domain *d, daddr_t daddr,
+static void amd_iommu_flush_all_iotlbs(const struct domain *d, daddr_t daddr,
                                        unsigned int order)
 {
     struct pci_dev *pdev;
 
-    if ( !ats_enabled )
-        return;
-
     for_each_pdev( d, pdev )
     {
         u8 devfn = pdev->devfn;
@@ -343,7 +340,16 @@ static void _amd_iommu_flush_pages(struc
     }
 
     if ( ats_enabled )
+    {
         amd_iommu_flush_all_iotlbs(d, daddr, order);
+
+        /*
+         * Hidden devices are associated with DomXEN but usable by the
+         * hardware domain. Hence they need dealing with here as well.
+         */
+        if ( is_hardware_domain(d) )
+            amd_iommu_flush_all_iotlbs(dom_xen, daddr, order);
+    }
 }
 
 void amd_iommu_flush_all_pages(struct domain *d)

On 17.09.2021 13:00, Jan Beulich wrote:
> Hidden devices are associated with DomXEN but usable by the
> hardware domain. Hence they need flushing as well when all devices are
> to have flushes invoked.
> 
> While there drop a redundant ATS-enabled check and constify the first
> parameter of the involved function.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

The VT-d side equivalent having gone in a while ago, I think it would
be good to have the AMD side on par.

Jan

> --- a/xen/drivers/passthrough/amd/iommu_cmd.c
> +++ b/xen/drivers/passthrough/amd/iommu_cmd.c
> @@ -308,14 +308,11 @@ void amd_iommu_flush_iotlb(u8 devfn, con
>      flush_command_buffer(iommu, iommu_dev_iotlb_timeout);
>  }
>  
> -static void amd_iommu_flush_all_iotlbs(struct domain *d, daddr_t daddr,
> +static void amd_iommu_flush_all_iotlbs(const struct domain *d, daddr_t daddr,
>                                         unsigned int order)
>  {
>      struct pci_dev *pdev;
>  
> -    if ( !ats_enabled )
> -        return;
> -
>      for_each_pdev( d, pdev )
>      {
>          u8 devfn = pdev->devfn;
> @@ -343,7 +340,16 @@ static void _amd_iommu_flush_pages(struc
>      }
>  
>      if ( ats_enabled )
> +    {
>          amd_iommu_flush_all_iotlbs(d, daddr, order);
> +
> +        /*
> +         * Hidden devices are associated with DomXEN but usable by the
> +         * hardware domain. Hence they need dealing with here as well.
> +         */
> +        if ( is_hardware_domain(d) )
> +            amd_iommu_flush_all_iotlbs(dom_xen, daddr, order);
> +    }
>  }
>  
>  void amd_iommu_flush_all_pages(struct domain *d)
> 
> 

On 11/10/2021 09:04, Jan Beulich wrote:
> On 17.09.2021 13:00, Jan Beulich wrote:
>> Hidden devices are associated with DomXEN but usable by the
>> hardware domain. Hence they need flushing as well when all devices are
>> to have flushes invoked.
>>
>> While there drop a redundant ATS-enabled check and constify the first
>> parameter of the involved function.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> The VT-d side equivalent having gone in a while ago, I think it would
> be good to have the AMD side on par.
> 

Agreed.

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