AMD,IOMMU: Clean up old entries in remapping tables when creating new one

When changing the affinity of an IRQ associated with a passed
through PCI device, clear previous mapping.

This is XSA-36 / CVE-2013-0153.

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

In addition, because some BIOSes may incorrectly program IVRS
entries for IOAPIC try to check for entry's consistency. Specifically,
if conflicting entries are found disable IOMMU if per-device
remapping table is used. If entries refer to bogus IOAPIC IDs
disable IOMMU unconditionally

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com>
xen-unstable changeset: 26517:601139e2b0db
xen-unstable date: Tue Feb  5 14:20:47 UTC 2013

3 files changed, 93 insertions, 10 deletions

diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c
index 8e0868fabd..f9d980327a 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -21,6 +21,7 @@
 #include <xen/config.h>
 #include <xen/errno.h>
 #include <asm/apicdef.h>
+#include <asm/io_apic.h>
 #include <asm/amd-iommu.h>
 #include <asm/hvm/svm/amd-iommu-proto.h>
 #include <asm/hvm/svm/amd-iommu-acpi.h>
@@ -29,7 +30,6 @@ extern unsigned long amd_iommu_page_entries;
 extern unsigned short ivrs_bdf_entries;
 extern struct ivrs_mappings *ivrs_mappings;
 extern unsigned short last_bdf;
-extern int ioapic_bdf[MAX_IO_APICS];
 extern void *shared_intremap_table;
 
 static void add_ivrs_mapping_entry(
@@ -636,6 +636,7 @@ static u16 __init parse_ivhd_device_special(
     u16 header_length, u16 block_length, struct amd_iommu *iommu)
 {
     u16 dev_length, bdf;
+    int apic;
 
     dev_length = sizeof(struct acpi_ivhd_device_special);
     if ( header_length < (block_length + dev_length) )
@@ -652,9 +653,58 @@ static u16 __init parse_ivhd_device_special(
     }
 
     add_ivrs_mapping_entry(bdf, bdf, ivhd_device->header.flags, iommu);
-    /* set device id of ioapic */
-    ioapic_bdf[ivhd_device->special.handle] = bdf;
-    return dev_length;
+
+    if ( ivhd_device->special.variety != 1 /* ACPI_IVHD_IOAPIC */ )
+    {
+        if ( ivhd_device->special.variety != 2 /* ACPI_IVHD_HPET */ )
+            printk(XENLOG_ERR "Unrecognized IVHD special variety %#x\n",
+                   ivhd_device->special.variety);
+        return dev_length;
+    }
+
+    /*
+     * Some BIOSes have IOAPIC broken entries so we check for IVRS
+     * consistency here --- whether entry's IOAPIC ID is valid and
+     * whether there are conflicting/duplicated entries.
+     */
+    for ( apic = 0; apic < nr_ioapics; apic++ )
+    {
+        if ( IO_APIC_ID(apic) != ivhd_device->special.handle )
+            continue;
+
+        if ( ioapic_bdf[ivhd_device->special.handle].pin_setup )
+        {
+            if ( ioapic_bdf[ivhd_device->special.handle].bdf == bdf )
+                AMD_IOMMU_DEBUG("IVHD Warning: Duplicate IO-APIC %#x entries\n",
+                                ivhd_device->special.handle);
+            else
+            {
+                printk(XENLOG_ERR "IVHD Error: Conflicting IO-APIC %#x entries\n",
+                       ivhd_device->special.handle);
+                if ( amd_iommu_perdev_intremap )
+                    return 0;
+            }
+        }
+        else
+        {
+            /* set device id of ioapic */
+            ioapic_bdf[ivhd_device->special.handle].bdf = bdf;
+
+            ioapic_bdf[ivhd_device->special.handle].pin_setup = xzalloc_array(
+                unsigned long, BITS_TO_LONGS(nr_ioapic_registers[apic]));
+            if ( nr_ioapic_registers[apic] &&
+                 !ioapic_bdf[IO_APIC_ID(apic)].pin_setup )
+            {
+                printk(XENLOG_ERR "IVHD Error: Out of memory\n");
+                return 0;
+            }
+        }
+        return dev_length;
+    }
+
+    printk(XENLOG_ERR "IVHD Error: Invalid IO-APIC %#x\n",
+           ivhd_device->special.handle);
+    return 0;
 }
 
 static int __init parse_ivhd_block(struct acpi_ivhd_block_header *ivhd_block)
diff --git a/xen/drivers/passthrough/amd/iommu_intr.c b/xen/drivers/passthrough/amd/iommu_intr.c
index 07ab37df61..551bc77ace 100644
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -27,7 +27,7 @@
 #define INTREMAP_LENGTH 0xB
 #define INTREMAP_ENTRIES (1 << INTREMAP_LENGTH)
 
-int ioapic_bdf[MAX_IO_APICS];
+struct ioapic_bdf ioapic_bdf[MAX_IO_APICS];
 extern struct ivrs_mappings *ivrs_mappings;
 extern unsigned short ivrs_bdf_entries;
 void *shared_intremap_table;
@@ -117,12 +117,12 @@ void invalidate_interrupt_table(struct amd_iommu *iommu, u16 device_id)
 static void update_intremap_entry_from_ioapic(
     int bdf,
     struct amd_iommu *iommu,
-    struct IO_APIC_route_entry *ioapic_rte)
+    const struct IO_APIC_route_entry *rte,
+    const struct IO_APIC_route_entry *old_rte)
 {
     unsigned long flags;
     u32* entry;
     u8 delivery_mode, dest, vector, dest_mode;
-    struct IO_APIC_route_entry *rte = ioapic_rte;
     int req_id;
     spinlock_t *lock;
     int offset;
@@ -138,6 +138,14 @@ static void update_intremap_entry_from_ioapic(
     spin_lock_irqsave(lock, flags);
 
     offset = get_intremap_offset(vector, delivery_mode);
+    if ( old_rte )
+    {
+        int old_offset = get_intremap_offset(old_rte->vector,
+                                             old_rte->delivery_mode);
+
+        if ( offset != old_offset )
+            free_intremap_entry(bdf, old_offset);
+    }
     entry = (u32*)get_intremap_entry(req_id, offset);
     update_intremap_entry(entry, vector, delivery_mode, dest_mode, dest);
 
@@ -176,7 +184,7 @@ int __init amd_iommu_setup_ioapic_remapping(void)
                 continue;
 
             /* get device id of ioapic devices */
-            bdf = ioapic_bdf[IO_APIC_ID(apic)];
+            bdf = ioapic_bdf[IO_APIC_ID(apic)].bdf;
             iommu = find_iommu_for_device(bdf);
             if ( !iommu )
             {
@@ -207,6 +215,7 @@ int __init amd_iommu_setup_ioapic_remapping(void)
                 flush_command_buffer(iommu);
                 spin_unlock_irqrestore(&iommu->lock, flags);
             }
+            set_bit(pin, ioapic_bdf[IO_APIC_ID(apic)].pin_setup);
         }
     }
     return 0;
@@ -218,6 +227,7 @@ void amd_iommu_ioapic_update_ire(
     struct IO_APIC_route_entry old_rte = { 0 };
     struct IO_APIC_route_entry new_rte = { 0 };
     unsigned int rte_lo = (reg & 1) ? reg - 1 : reg;
+    unsigned int pin = (reg - 0x10) / 2;
     int saved_mask, bdf;
     struct amd_iommu *iommu;
 
@@ -228,7 +238,7 @@ void amd_iommu_ioapic_update_ire(
     }
 
     /* get device id of ioapic devices */
-    bdf = ioapic_bdf[IO_APIC_ID(apic)];
+    bdf = ioapic_bdf[IO_APIC_ID(apic)].bdf;
     iommu = find_iommu_for_device(bdf);
     if ( !iommu )
     {
@@ -254,6 +264,14 @@ void amd_iommu_ioapic_update_ire(
         *(((u32 *)&new_rte) + 1) = value;
     }
 
+    if ( new_rte.mask &&
+         !test_bit(pin, ioapic_bdf[IO_APIC_ID(apic)].pin_setup) )
+    {
+        ASSERT(saved_mask);
+        __io_apic_write(apic, reg, value);
+        return;
+    }
+
     /* mask the interrupt while we change the intremap table */
     if ( !saved_mask )
     {
@@ -262,7 +280,11 @@ void amd_iommu_ioapic_update_ire(
     }
 
     /* Update interrupt remapping entry */
-    update_intremap_entry_from_ioapic(bdf, iommu, &new_rte);
+    update_intremap_entry_from_ioapic(
+        bdf, iommu, &new_rte,
+        test_and_set_bit(pin,
+                         ioapic_bdf[IO_APIC_ID(apic)].pin_setup) ? &old_rte
+                                                                 : NULL);
 
     /* Forward write access to IO-APIC RTE */
     __io_apic_write(apic, reg, value);
@@ -373,6 +395,12 @@ void amd_iommu_msi_msg_update_ire(
         return;
     }
 
+    if ( msi_desc->remap_index >= 0 )
+        update_intremap_entry_from_msi_msg(iommu, pdev, msi_desc, NULL);
+
+    if ( !msg )
+        return;
+
     update_intremap_entry_from_msi_msg(iommu, pdev, msi_desc, msg);
 }
 
diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
index 52ddc721e6..d345f2f58d 100644
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
@@ -88,6 +88,11 @@ void amd_iommu_read_msi_from_ire(
 unsigned int amd_iommu_read_ioapic_from_ire(
     unsigned int apic, unsigned int reg);
 
+extern struct ioapic_bdf {
+    u16 bdf;
+    unsigned long *pin_setup;
+} ioapic_bdf[];
+
 /* power management support */
 void amd_iommu_resume(void);
 void amd_iommu_suspend(void);