[xen stable-4.16] iommu/amd-vi: flush IOMMU TLB when flushing the DTE

[patchbot@xxxxxxx]

commit 35217b78048e91a0f4d0f14b31a474cc59ec1388
Author:     Roger Pau Monne <roger.pau@xxxxxxxxxx>
AuthorDate: Tue Jun 13 15:01:05 2023 +0200
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Wed Sep 27 16:29:50 2023 +0100

    iommu/amd-vi: flush IOMMU TLB when flushing the DTE
    
    The caching invalidation guidelines from the AMD-Vi specification 
(48882â??Rev
    3.07-PUBâ??Oct 2022) seem to be misleading on some hardware, as devices will
    malfunction (see stale DMA mappings) if some fields of the DTE are updated 
but
    the IOMMU TLB is not flushed. This has been observed in practice on AMD
    systems.  Due to the lack of guidance from the currently published
    specification this patch aims to increase the flushing done in order to 
prevent
    device malfunction.
    
    In order to fix, issue an INVALIDATE_IOMMU_PAGES command from
    amd_iommu_flush_device(), flushing all the address space.  Note this 
requires
    callers to be adjusted in order to pass the DomID on the DTE previous to the
    modification.
    
    Some call sites don't provide a valid DomID to amd_iommu_flush_device() in
    order to avoid the flush.  That's because the device had address 
translations
    disabled and hence the previous DomID on the DTE is not valid.  Note the
    current logic relies on the entity disabling address translations to also 
flush
    the TLB of the in use DomID.
    
    Device I/O TLB flushing when ATS are enabled is not covered by the current
    change, as ATS usage is not security supported.
    
    This is XSA-442 / CVE-2023-34326
    
    Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
    (cherry picked from commit 5fc98b97084a46884acef9320e643faf40d42212)
---
 xen/drivers/passthrough/amd/iommu.h         |  3 ++-
 xen/drivers/passthrough/amd/iommu_cmd.c     | 10 +++++++++-
 xen/drivers/passthrough/amd/iommu_guest.c   |  5 +++--
 xen/drivers/passthrough/amd/iommu_init.c    |  6 +++++-
 xen/drivers/passthrough/amd/pci_amd_iommu.c | 14 ++++++++++----
 5 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu.h 
b/xen/drivers/passthrough/amd/iommu.h
index 3c702eb517..6dd24593a0 100644
--- a/xen/drivers/passthrough/amd/iommu.h
+++ b/xen/drivers/passthrough/amd/iommu.h
@@ -280,7 +280,8 @@ void amd_iommu_flush_pages(struct domain *d, unsigned long 
dfn,
                            unsigned int order);
 void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev,
                            uint64_t gaddr, unsigned int order);
-void amd_iommu_flush_device(struct amd_iommu *iommu, uint16_t bdf);
+void amd_iommu_flush_device(struct amd_iommu *iommu, uint16_t bdf,
+                            domid_t domid);
 void amd_iommu_flush_intremap(struct amd_iommu *iommu, uint16_t bdf);
 void amd_iommu_flush_all_caches(struct amd_iommu *iommu);
 
diff --git a/xen/drivers/passthrough/amd/iommu_cmd.c 
b/xen/drivers/passthrough/amd/iommu_cmd.c
index 809d93b89f..41a32c757b 100644
--- a/xen/drivers/passthrough/amd/iommu_cmd.c
+++ b/xen/drivers/passthrough/amd/iommu_cmd.c
@@ -362,10 +362,18 @@ void amd_iommu_flush_pages(struct domain *d,
     _amd_iommu_flush_pages(d, __dfn_to_daddr(dfn), order);
 }
 
-void amd_iommu_flush_device(struct amd_iommu *iommu, uint16_t bdf)
+void amd_iommu_flush_device(struct amd_iommu *iommu, uint16_t bdf,
+                            domid_t domid)
 {
     invalidate_dev_table_entry(iommu, bdf);
     flush_command_buffer(iommu, 0);
+
+    /* Also invalidate IOMMU TLB entries when flushing the DTE. */
+    if ( domid != DOMID_INVALID )
+    {
+        invalidate_iommu_pages(iommu, INV_IOMMU_ALL_PAGES_ADDRESS, domid, 0);
+        flush_command_buffer(iommu, 0);
+    }
 }
 
 void amd_iommu_flush_intremap(struct amd_iommu *iommu, uint16_t bdf)
diff --git a/xen/drivers/passthrough/amd/iommu_guest.c 
b/xen/drivers/passthrough/amd/iommu_guest.c
index 85828490ff..38c7b4d979 100644
--- a/xen/drivers/passthrough/amd/iommu_guest.c
+++ b/xen/drivers/passthrough/amd/iommu_guest.c
@@ -385,7 +385,7 @@ static int do_completion_wait(struct domain *d, cmd_entry_t 
*cmd)
 
 static int do_invalidate_dte(struct domain *d, cmd_entry_t *cmd)
 {
-    uint16_t gbdf, mbdf, req_id, gdom_id, hdom_id;
+    uint16_t gbdf, mbdf, req_id, gdom_id, hdom_id, prev_domid;
     struct amd_iommu_dte *gdte, *mdte, *dte_base;
     struct amd_iommu *iommu = NULL;
     struct guest_iommu *g_iommu;
@@ -445,13 +445,14 @@ static int do_invalidate_dte(struct domain *d, 
cmd_entry_t *cmd)
     req_id = get_dma_requestor_id(iommu->seg, mbdf);
     dte_base = iommu->dev_table.buffer;
     mdte = &dte_base[req_id];
+    prev_domid = mdte->domain_id;
 
     spin_lock_irqsave(&iommu->lock, flags);
     dte_set_gcr3_table(mdte, hdom_id, gcr3_mfn << PAGE_SHIFT, gv, glx);
 
     spin_unlock_irqrestore(&iommu->lock, flags);
 
-    amd_iommu_flush_device(iommu, req_id);
+    amd_iommu_flush_device(iommu, req_id, prev_domid);
 
     return 0;
 }
diff --git a/xen/drivers/passthrough/amd/iommu_init.c 
b/xen/drivers/passthrough/amd/iommu_init.c
index ca791d4e54..7dfe4b15dc 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -1556,7 +1556,11 @@ static int _invalidate_all_devices(
         req_id = ivrs_mappings[bdf].dte_requestor_id;
         if ( iommu )
         {
-            amd_iommu_flush_device(iommu, req_id);
+            /*
+             * IOMMU TLB flush performed separately (see
+             * invalidate_all_domain_pages()).
+             */
+            amd_iommu_flush_device(iommu, req_id, DOMID_INVALID);
             amd_iommu_flush_intremap(iommu, req_id);
         }
     }
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c 
b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index e5e0f00402..7b6dbf546a 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -192,10 +192,13 @@ static int __must_check amd_iommu_setup_domain_device(
 
         spin_unlock_irqrestore(&iommu->lock, flags);
 
-        amd_iommu_flush_device(iommu, req_id);
+        /* DTE didn't have DMA translations enabled, do not flush the TLB. */
+        amd_iommu_flush_device(iommu, req_id, DOMID_INVALID);
     }
     else if ( dte->pt_root != mfn_x(page_to_mfn(root_pg)) )
     {
+        domid_t prev_domid = dte->domain_id;
+
         /*
          * Strictly speaking if the device is the only one with this requestor
          * ID, it could be allowed to be re-assigned regardless of unity map
@@ -252,7 +255,7 @@ static int __must_check amd_iommu_setup_domain_device(
 
         spin_unlock_irqrestore(&iommu->lock, flags);
 
-        amd_iommu_flush_device(iommu, req_id);
+        amd_iommu_flush_device(iommu, req_id, prev_domid);
     }
     else
         spin_unlock_irqrestore(&iommu->lock, flags);
@@ -421,6 +424,8 @@ static void amd_iommu_disable_domain_device(const struct 
domain *domain,
     spin_lock_irqsave(&iommu->lock, flags);
     if ( dte->tv || dte->v )
     {
+        domid_t prev_domid = dte->domain_id;
+
         /* See the comment in amd_iommu_setup_device_table(). */
         dte->int_ctl = IOMMU_DEV_TABLE_INT_CONTROL_ABORTED;
         smp_wmb();
@@ -439,7 +444,7 @@ static void amd_iommu_disable_domain_device(const struct 
domain *domain,
 
         spin_unlock_irqrestore(&iommu->lock, flags);
 
-        amd_iommu_flush_device(iommu, req_id);
+        amd_iommu_flush_device(iommu, req_id, prev_domid);
 
         AMD_IOMMU_DEBUG("Disable: device id = %#x, "
                         "domain = %d, paging mode = %d\n",
@@ -611,7 +616,8 @@ static int amd_iommu_add_device(u8 devfn, struct pci_dev 
*pdev)
 
         spin_unlock_irqrestore(&iommu->lock, flags);
 
-        amd_iommu_flush_device(iommu, bdf);
+        /* DTE didn't have DMA translations enabled, do not flush the TLB. */
+        amd_iommu_flush_device(iommu, bdf, DOMID_INVALID);
     }
 
     if ( amd_iommu_reserve_domain_unity_map(
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.16