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>

diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c
index b9914aa45b4946399818c94d3dfde54be25ff0fd..a5ad1192f7a1ddc0447b18c94bb0edaae62204da 100644 (file)
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -22,6 +22,7 @@
 #include <xen/errno.h>
 #include <xen/acpi.h>
 #include <asm/apicdef.h>
+#include <asm/io_apic.h>
 #include <asm/amd-iommu.h>
 #include <asm/hvm/svm/amd-iommu-proto.h>
 
@@ -637,6 +638,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(*special);
     if ( header_length < (block_length + dev_length) )
@@ -657,9 +659,53 @@ static u16 __init parse_ivhd_device_special(
     switch ( special->variety )
     {
     case ACPI_IVHD_IOAPIC:
-    /* set device id of ioapic */
-        ioapic_sbdf[special->handle].bdf = bdf;
-        ioapic_sbdf[special->handle].seg = seg;
+        /*
+         * 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) != special->handle )
+                continue;
+
+            if ( ioapic_sbdf[special->handle].pin_setup )
+            {
+                if ( ioapic_sbdf[special->handle].bdf == bdf &&
+                     ioapic_sbdf[special->handle].seg == seg )
+                    AMD_IOMMU_DEBUG("IVHD Warning: Duplicate IO-APIC %#x entries\n",
+                                    special->handle);
+                else
+                {
+                    printk(XENLOG_ERR "IVHD Error: Conflicting IO-APIC %#x entries\n",
+                           special->handle);
+                    if ( amd_iommu_perdev_intremap )
+                        return 0;
+                }
+            }
+            else
+            {
+                /* set device id of ioapic */
+                ioapic_sbdf[special->handle].bdf = bdf;
+                ioapic_sbdf[special->handle].seg = seg;
+
+                ioapic_sbdf[special->handle].pin_setup = xzalloc_array(
+                    unsigned long, BITS_TO_LONGS(nr_ioapic_entries[apic]));
+                if ( nr_ioapic_entries[apic] &&
+                     !ioapic_sbdf[IO_APIC_ID(apic)].pin_setup )
+                {
+                    printk(XENLOG_ERR "IVHD Error: Out of memory\n");
+                    return 0;
+                }
+            }
+            break;
+        }
+        if ( apic == nr_ioapics )
+        {
+            printk(XENLOG_ERR "IVHD Error: Invalid IO-APIC %#x\n",
+                   special->handle);
+            return 0;
+        }
         break;
     case ACPI_IVHD_HPET:
         /* set device id of hpet */

diff --git a/xen/drivers/passthrough/amd/iommu_intr.c b/xen/drivers/passthrough/amd/iommu_intr.c
index 7d1dc6a7b4c563c9ac744126c2f23f3341d80eac..5629cc7712bb1c3152eaa9ee4fb9df40c12aa7cb 100644 (file)
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -100,12 +100,12 @@ static void update_intremap_entry(u32* entry, u8 vector, u8 int_type,
 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;
@@ -121,6 +121,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(iommu->seg, bdf, old_offset);
+    }
     entry = (u32*)get_intremap_entry(iommu->seg, req_id, offset);
     update_intremap_entry(entry, vector, delivery_mode, dest_mode, dest);
 
@@ -189,6 +197,7 @@ int __init amd_iommu_setup_ioapic_remapping(void)
                 amd_iommu_flush_intremap(iommu, req_id);
                 spin_unlock_irqrestore(&iommu->lock, flags);
             }
+            set_bit(pin, ioapic_sbdf[IO_APIC_ID(apic)].pin_setup);
         }
     }
     return 0;
@@ -200,6 +209,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, seg, bdf;
     struct amd_iommu *iommu;
 
@@ -237,6 +247,14 @@ void amd_iommu_ioapic_update_ire(
         *(((u32 *)&new_rte) + 1) = value;
     }
 
+    if ( new_rte.mask &&
+         !test_bit(pin, ioapic_sbdf[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 )
     {
@@ -245,7 +263,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_sbdf[IO_APIC_ID(apic)].pin_setup) ? &old_rte
+                                                                  : NULL);
 
     /* Forward write access to IO-APIC RTE */
     __io_apic_write(apic, reg, value);
@@ -356,6 +378,12 @@ void amd_iommu_msi_msg_update_ire(
         return;
     }
 
+    if ( msi_desc->remap_index >= 0 )
+        update_intremap_entry_from_msi_msg(iommu, bdf, msi_desc, NULL);
+
+    if ( !msg )
+        return;
+
     update_intremap_entry_from_msi_msg(iommu, bdf, 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 72cc455053043319bc6e0ceac89f05afba7606d6..06789bc148ddb7cb13d40ee5e88b42da59f61f65 100644 (file)
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
@@ -101,6 +101,7 @@ int amd_setup_hpet_msi(struct msi_desc *msi_desc);
 
 extern struct ioapic_sbdf {
     u16 bdf, seg;
+    unsigned long *pin_setup;
 } ioapic_sbdf[MAX_IO_APICS];
 extern void *shared_intremap_table;