[PATCH] AMD IOMMU: Fix an interrupt remapping issue

[Wei Wang2 <wei.wang2@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 984 bytes --]

(sorry, Forget the patch)

Some device could generate bogus interrupts if an IO-APIC RTE and an iommu 
interrupt remapping entry are not consistent during 2 adjacent 64bits IO-APIC 
RTE updates. For example, if the 2nd operation updates destination bits in 
RTE for SATA device and unmask it, in some case, SATA device will assert 
ioapic pin to generate interrupt immediately using new destination but iommu 
could still translate it into the old destination, then dom0 would be 
confused. To fix that, we sync up interrupt remapping entry with IO-APIC IRE 
on every 32 bits operation and foward IOAPIC RTE updates after interrupt 
remapping table has been changed. 

Jan, This patch fixes SATA device issue we observed (Bug #680824), please 
review it. Thanks! 
Wei
--
Advanced Micro Devices GmbH
Sitz: Dornach, Gemeinde Aschheim, 
Landkreis München Registergericht München, 
HRB Nr. 43632
WEEE-Reg-Nr: DE 12919551
Geschäftsführer:
Alberto Bozzo, Andrew Bowd

[-- Attachment #2: fix_intremap.patch --]
[-- Type: text/x-diff, Size: 5502 bytes --]

# HG changeset patch
# User Wei Wang <wei.wang2@amd.com>
# Node ID ab2944070ca99790546b34fa04a80103d3e7464f
# Parent  e5a750d1bf9bb021713c6721000e655a4054ebea
Some device could generate bogus interrupts if an IO-APIC RTE and an iommu interrupt remapping entry are not consistent during 2 adjacent 64bits IO-APIC RTE updates. For example, if the 2nd operation updates destination bits in RTE for SATA device and unmask it, in some case, SATA device will assert ioapic pin to generate interrupt immediately using new destination but iommu could still translate it into the old destination, then dom0 would be confused. To fix that, we sync up interrupt remapping entry with IO-APIC IRE on every 32 bits operation and foward IOAPIC RTE updates after interrupt remapping table has been changed.  

Signed-off-by Wei Wang <wei.wang2@amd.com>

diff -r e5a750d1bf9b -r ab2944070ca9 xen/drivers/passthrough/amd/iommu_intr.c
--- a/xen/drivers/passthrough/amd/iommu_intr.c	Thu Apr 07 11:12:55 2011 +0100
+++ b/xen/drivers/passthrough/amd/iommu_intr.c	Fri Apr 08 12:35:48 2011 +0200
@@ -118,7 +118,7 @@ static void update_intremap_entry_from_i
     int bdf,
     struct amd_iommu *iommu,
     struct IO_APIC_route_entry *ioapic_rte,
-    unsigned int rte_upper, unsigned int value)
+    unsigned int value)
 {
     unsigned long flags;
     u32* entry;
@@ -130,28 +130,26 @@ static void update_intremap_entry_from_i
 
     req_id = get_intremap_requestor_id(bdf);
     lock = get_intremap_lock(req_id);
-    /* only remap interrupt vector when lower 32 bits in ioapic ire changed */
-    if ( likely(!rte_upper) )
-    {
-        delivery_mode = rte->delivery_mode;
-        vector = rte->vector;
-        dest_mode = rte->dest_mode;
-        dest = rte->dest.logical.logical_dest;
-
-        spin_lock_irqsave(lock, flags);
-        offset = get_intremap_offset(vector, delivery_mode);
-        entry = (u32*)get_intremap_entry(req_id, offset);
-
-        update_intremap_entry(entry, vector, delivery_mode, dest_mode, dest);
-        spin_unlock_irqrestore(lock, flags);
-
-        if ( iommu->enabled )
-        {
-            spin_lock_irqsave(&iommu->lock, flags);
-            invalidate_interrupt_table(iommu, req_id);
-            flush_command_buffer(iommu);
-            spin_unlock_irqrestore(&iommu->lock, flags);
-        }
+
+    delivery_mode = rte->delivery_mode;
+    vector = rte->vector;
+    dest_mode = rte->dest_mode;
+    dest = rte->dest.logical.logical_dest;
+
+    spin_lock_irqsave(lock, flags);
+
+    offset = get_intremap_offset(vector, delivery_mode);
+    entry = (u32*)get_intremap_entry(req_id, offset);
+    update_intremap_entry(entry, vector, delivery_mode, dest_mode, dest);
+
+    spin_unlock_irqrestore(lock, flags);
+
+    if ( iommu->enabled )
+    {
+        spin_lock_irqsave(&iommu->lock, flags);
+        invalidate_interrupt_table(iommu, req_id);
+        flush_command_buffer(iommu);
+        spin_unlock_irqrestore(&iommu->lock, flags);
     }
 }
 
@@ -199,7 +197,8 @@ int __init amd_iommu_setup_ioapic_remapp
             spin_lock_irqsave(lock, flags);
             offset = get_intremap_offset(vector, delivery_mode);
             entry = (u32*)get_intremap_entry(req_id, offset);
-            update_intremap_entry(entry, vector, delivery_mode, dest_mode, dest);
+            update_intremap_entry(entry, vector,
+                                  delivery_mode, dest_mode, dest);
             spin_unlock_irqrestore(lock, flags);
 
             if ( iommu->enabled )
@@ -218,15 +217,12 @@ void amd_iommu_ioapic_update_ire(
     unsigned int apic, unsigned int reg, unsigned int value)
 {
     struct IO_APIC_route_entry ioapic_rte = { 0 };
-    unsigned int rte_upper = (reg & 1) ? 1 : 0;
+    unsigned int rte_lo;
     int saved_mask, bdf;
     struct amd_iommu *iommu;
 
-    *IO_APIC_BASE(apic) = reg;
-    *(IO_APIC_BASE(apic)+4) = value;
-
     if ( !iommu_intremap )
-        return;
+        goto done;
 
     /* get device id of ioapic devices */
     bdf = ioapic_bdf[IO_APIC_ID(apic)];
@@ -237,28 +233,34 @@ void amd_iommu_ioapic_update_ire(
                         bdf);
         return;
     }
-    if ( rte_upper )
-        return;
+
+    /* get lower 32 bits IO-APIC ire index */
+    rte_lo = (reg & 1) ? reg - 1 : reg;
 
     /* read both lower and upper 32-bits of rte entry */
-    *IO_APIC_BASE(apic) = reg;
+    *IO_APIC_BASE(apic) = rte_lo;
     *(((u32 *)&ioapic_rte) + 0) = *(IO_APIC_BASE(apic)+4);
-    *IO_APIC_BASE(apic) = reg + 1;
+    *IO_APIC_BASE(apic) = rte_lo + 1;
     *(((u32 *)&ioapic_rte) + 1) = *(IO_APIC_BASE(apic)+4);
 
     /* mask the interrupt while we change the intremap table */
     saved_mask = ioapic_rte.mask;
     ioapic_rte.mask = 1;
-    *IO_APIC_BASE(apic) = reg;
+    *IO_APIC_BASE(apic) = rte_lo;
     *(IO_APIC_BASE(apic)+4) = *(((int *)&ioapic_rte)+0);
     ioapic_rte.mask = saved_mask;
 
-    update_intremap_entry_from_ioapic(
-        bdf, iommu, &ioapic_rte, rte_upper, value);
+    /* Update interrupt remapping entry */
+    update_intremap_entry_from_ioapic(bdf, iommu, &ioapic_rte, value);
 
     /* unmask the interrupt after we have updated the intremap table */
+    *IO_APIC_BASE(apic) = rte_lo;
+    *(IO_APIC_BASE(apic)+4) = *(((u32 *)&ioapic_rte)+0);
+
+done:
+    /* Forward write access to IO-APIC */
     *IO_APIC_BASE(apic) = reg;
-    *(IO_APIC_BASE(apic)+4) = *(((u32 *)&ioapic_rte)+0);
+    *(IO_APIC_BASE(apic)+4) = value;
 }
 
 static void update_intremap_entry_from_msi_msg(

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel