[Xen-changelog] [xen master] AMD/IOMMU: correct IRTE updating

[patchbot@xxxxxxx]

commit 9e0e225a3aeccd807a8db88ba4669f8ab30ecc99
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Wed Jul 31 13:27:52 2019 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Wed Jul 31 13:27:52 2019 +0200

    AMD/IOMMU: correct IRTE updating
    
    Flushing didn't get done along the lines of what the specification says.
    Mark entries to be updated as not remapped (which will result in
    interrupt requests to get target aborted, but the interrupts should be
    masked anyway at that point in time), issue the flush, and only then
    write the new entry.
    
    In update_intremap_entry_from_msi_msg() also fold the duplicate initial
    lock determination and acquire into just a single instance.
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Acked-by: Brian Woods <brian.woods@xxxxxxx>
---
 xen/drivers/passthrough/amd/iommu_intr.c | 75 ++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 28 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_intr.c 
b/xen/drivers/passthrough/amd/iommu_intr.c
index 9690f75c6c..f5685506c9 100644
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -213,15 +213,13 @@ static void update_intremap_entry(const struct amd_iommu 
*iommu,
             },
         };
 
-        ACCESS_ONCE(entry.ptr128->raw[0]) = 0;
+        ASSERT(!entry.ptr128->full.remap_en);
+        entry.ptr128->raw[1] = irte.raw[1];
         /*
-         * Low half, in particular RemapEn, needs to be cleared first.  See
+         * High half needs to be set before low one (containing RemapEn).  See
          * comment in free_intremap_entry() regarding the choice of barrier.
          */
         smp_wmb();
-        entry.ptr128->raw[1] = irte.raw[1];
-        /* High half needs to be set before low one (containing RemapEn). */
-        smp_wmb();
         ACCESS_ONCE(entry.ptr128->raw[0]) = irte.raw[0];
     }
     else
@@ -296,6 +294,20 @@ static int update_intremap_entry_from_ioapic(
     }
 
     entry = get_intremap_entry(iommu, req_id, offset);
+
+    /* The RemapEn fields match for all formats. */
+    while ( iommu->enabled && entry.ptr32->flds.remap_en )
+    {
+        entry.ptr32->flds.remap_en = false;
+        spin_unlock(lock);
+
+        spin_lock(&iommu->lock);
+        amd_iommu_flush_intremap(iommu, req_id);
+        spin_unlock(&iommu->lock);
+
+        spin_lock(lock);
+    }
+
     if ( fresh )
         /* nothing */;
     else if ( !lo_update )
@@ -325,13 +337,6 @@ static int update_intremap_entry_from_ioapic(
 
     spin_unlock_irqrestore(lock, flags);
 
-    if ( iommu->enabled && !fresh )
-    {
-        spin_lock_irqsave(&iommu->lock, flags);
-        amd_iommu_flush_intremap(iommu, req_id);
-        spin_unlock_irqrestore(&iommu->lock, flags);
-    }
-
     set_rte_index(rte, offset);
 
     return 0;
@@ -587,19 +592,27 @@ static int update_intremap_entry_from_msi_msg(
     req_id = get_dma_requestor_id(iommu->seg, bdf);
     alias_id = get_intremap_requestor_id(iommu->seg, bdf);
 
+    lock = get_intremap_lock(iommu->seg, req_id);
+    spin_lock_irqsave(lock, flags);
+
     if ( msg == NULL )
     {
-        lock = get_intremap_lock(iommu->seg, req_id);
-        spin_lock_irqsave(lock, flags);
         for ( i = 0; i < nr; ++i )
             free_intremap_entry(iommu, req_id, *remap_index + i);
         spin_unlock_irqrestore(lock, flags);
-        goto done;
-    }
 
-    lock = get_intremap_lock(iommu->seg, req_id);
+        if ( iommu->enabled )
+        {
+            spin_lock_irqsave(&iommu->lock, flags);
+            amd_iommu_flush_intremap(iommu, req_id);
+            if ( alias_id != req_id )
+                amd_iommu_flush_intremap(iommu, alias_id);
+            spin_unlock_irqrestore(&iommu->lock, flags);
+        }
+
+        return 0;
+    }
 
-    spin_lock_irqsave(lock, flags);
     dest_mode = (msg->address_lo >> MSI_ADDR_DESTMODE_SHIFT) & 0x1;
     delivery_mode = (msg->data >> MSI_DATA_DELIVERY_MODE_SHIFT) & 0x1;
     vector = (msg->data >> MSI_DATA_VECTOR_SHIFT) & MSI_DATA_VECTOR_MASK;
@@ -623,6 +636,22 @@ static int update_intremap_entry_from_msi_msg(
     }
 
     entry = get_intremap_entry(iommu, req_id, offset);
+
+    /* The RemapEn fields match for all formats. */
+    while ( iommu->enabled && entry.ptr32->flds.remap_en )
+    {
+        entry.ptr32->flds.remap_en = false;
+        spin_unlock(lock);
+
+        spin_lock(&iommu->lock);
+        amd_iommu_flush_intremap(iommu, req_id);
+        if ( alias_id != req_id )
+            amd_iommu_flush_intremap(iommu, alias_id);
+        spin_unlock(&iommu->lock);
+
+        spin_lock(lock);
+    }
+
     update_intremap_entry(iommu, entry, vector, delivery_mode, dest_mode, 
dest);
     spin_unlock_irqrestore(lock, flags);
 
@@ -642,16 +671,6 @@ static int update_intremap_entry_from_msi_msg(
                get_ivrs_mappings(iommu->seg)[alias_id].intremap_table);
     }
 
-done:
-    if ( iommu->enabled )
-    {
-        spin_lock_irqsave(&iommu->lock, flags);
-        amd_iommu_flush_intremap(iommu, req_id);
-        if ( alias_id != req_id )
-            amd_iommu_flush_intremap(iommu, alias_id);
-        spin_unlock_irqrestore(&iommu->lock, flags);
-    }
-
     return 0;
 }
 
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog