[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH] Actually clear IO-APIC pins on boot and shutdown when used with an IOMMU



When booted with iommu=on, io_apic_read/write functions call into the
interrupt remapping code to update the IRTEs.  Unfortunately, on boot
and shutdown, we really want clear_IO_APIC() to sanitize the actual
IOAPIC RTE, and not just the bits that are active when interrupt
remapping is enabled.  This is particularly a problem on older versions
of Xen which used the IOAPIC RTE as the canonical source for the IRTE
index.  In that case, clear_IO_APIC() actually causes whatever happens
to be stored in the RTEs to be used as an IRTE index, which can come
back and bite us in ioapic_guest_write() if we attempt to remove an
interrupt that didn't actually exist.  Current upstream appears less
susceptible to errors since the IRTE index is stored in an array, but
it's still a good idea to sanitize the IOAPIC state.

Signed-off-by: Alex Williamson <alex.williamson@xxxxxx>
--

diff -r 1e9441f4dcbd xen/arch/x86/io_apic.c
--- a/xen/arch/x86/io_apic.c    Mon Dec 14 11:58:45 2009 +0000
+++ b/xen/arch/x86/io_apic.c    Tue Dec 15 16:05:46 2009 -0700
@@ -221,15 +221,20 @@
     spin_unlock_irqrestore(&ioapic_lock, flags);
 }
 
-static void clear_IO_APIC_pin(unsigned int apic, unsigned int pin)
+static void clear_IO_APIC_pin(unsigned int apic, unsigned int pin, int raw)
 {
     struct IO_APIC_route_entry entry;
     unsigned long flags;
     
     /* Check delivery_mode to be sure we're not clearing an SMI pin */
     spin_lock_irqsave(&ioapic_lock, flags);
-    *(((int*)&entry) + 0) = io_apic_read(apic, 0x10 + 2 * pin);
-    *(((int*)&entry) + 1) = io_apic_read(apic, 0x11 + 2 * pin);
+    if (raw) {
+        *(((int*)&entry) + 0) = __io_apic_read(apic, 0x10 + 2 * pin);
+        *(((int*)&entry) + 1) = __io_apic_read(apic, 0x11 + 2 * pin);
+    } else {
+        *(((int*)&entry) + 0) = io_apic_read(apic, 0x10 + 2 * pin);
+        *(((int*)&entry) + 1) = io_apic_read(apic, 0x11 + 2 * pin);
+    }
     spin_unlock_irqrestore(&ioapic_lock, flags);
     if (entry.delivery_mode == dest_SMI)
         return;
@@ -240,8 +245,13 @@
     memset(&entry, 0, sizeof(entry));
     entry.mask = 1;
     spin_lock_irqsave(&ioapic_lock, flags);
-    io_apic_write(apic, 0x10 + 2 * pin, *(((int *)&entry) + 0));
-    io_apic_write(apic, 0x11 + 2 * pin, *(((int *)&entry) + 1));
+    if (raw) {
+        __io_apic_write(apic, 0x10 + 2 * pin, *(((int *)&entry) + 0));
+        __io_apic_write(apic, 0x11 + 2 * pin, *(((int *)&entry) + 1));
+    } else {
+        io_apic_write(apic, 0x10 + 2 * pin, *(((int *)&entry) + 0));
+        io_apic_write(apic, 0x11 + 2 * pin, *(((int *)&entry) + 1));
+    }
     spin_unlock_irqrestore(&ioapic_lock, flags);
 }
 
@@ -250,8 +260,11 @@
     int apic, pin;
 
     for (apic = 0; apic < nr_ioapics; apic++)
-        for (pin = 0; pin < nr_ioapic_registers[apic]; pin++)
-            clear_IO_APIC_pin(apic, pin);
+        for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {
+            clear_IO_APIC_pin(apic, pin, 1);
+            if (ioapic_reg_remapped(0x10 + 2 * pin))
+                clear_IO_APIC_pin(apic, pin, 0);
+        }
 }
 
 #ifdef CONFIG_SMP
@@ -1739,7 +1752,7 @@
     *(((int *)&entry0) + 1) = io_apic_read(apic, 0x11 + 2 * pin);
     *(((int *)&entry0) + 0) = io_apic_read(apic, 0x10 + 2 * pin);
     spin_unlock_irqrestore(&ioapic_lock, flags);
-    clear_IO_APIC_pin(apic, pin);
+    clear_IO_APIC_pin(apic, pin, 0);
 
     memset(&entry1, 0, sizeof(entry1));
 
@@ -1772,7 +1785,7 @@
 
     CMOS_WRITE(save_control, RTC_CONTROL);
     CMOS_WRITE(save_freq_select, RTC_FREQ_SELECT);
-    clear_IO_APIC_pin(apic, pin);
+    clear_IO_APIC_pin(apic, pin, 0);
 
     spin_lock_irqsave(&ioapic_lock, flags);
     io_apic_write(apic, 0x11 + 2 * pin, *(((int *)&entry0) + 1));
@@ -1842,10 +1855,10 @@
         if (timer_irq_works()) {
             local_irq_restore(flags);
             if (disable_timer_pin_1 > 0)
-                clear_IO_APIC_pin(apic1, pin1);
+                clear_IO_APIC_pin(apic1, pin1, 0);
             return;
         }
-        clear_IO_APIC_pin(apic1, pin1);
+        clear_IO_APIC_pin(apic1, pin1, 0);
         printk(KERN_ERR "..MP-BIOS bug: 8254 timer not connected to "
                "IO-APIC\n");
     }
@@ -1869,7 +1882,7 @@
         /*
          * Cleanup, just in case ...
          */
-        clear_IO_APIC_pin(apic2, pin2);
+        clear_IO_APIC_pin(apic2, pin2, 0);
     }
     printk(" failed.\n");
 
diff -r 1e9441f4dcbd xen/include/asm-x86/io_apic.h
--- a/xen/include/asm-x86/io_apic.h     Mon Dec 14 11:58:45 2009 +0000
+++ b/xen/include/asm-x86/io_apic.h     Tue Dec 15 16:05:46 2009 -0700
@@ -131,20 +131,30 @@
 /* Only need to remap ioapic RTE (reg: 10~3Fh) */
 #define ioapic_reg_remapped(reg) (iommu_enabled && ((reg) >= 0x10))
 
+static inline unsigned int __io_apic_read(unsigned int apic, unsigned int reg)
+{
+       *IO_APIC_BASE(apic) = reg;
+       return *(IO_APIC_BASE(apic)+4);
+}
+
 static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
 {
        if (ioapic_reg_remapped(reg))
                return iommu_read_apic_from_ire(apic, reg);
+       return __io_apic_read(apic, reg);
+}
+
+static inline void __io_apic_write(unsigned int apic, unsigned int reg, 
unsigned int value)
+{
        *IO_APIC_BASE(apic) = reg;
-       return *(IO_APIC_BASE(apic)+4);
+       *(IO_APIC_BASE(apic)+4) = value;
 }
 
 static inline void io_apic_write(unsigned int apic, unsigned int reg, unsigned 
int value)
 {
        if (ioapic_reg_remapped(reg))
                return iommu_update_ire_from_apic(apic, reg, value);
-       *IO_APIC_BASE(apic) = reg;
-       *(IO_APIC_BASE(apic)+4) = value;
+       __io_apic_write(apic, reg, value);
 }
 
 static inline void io_apic_eoi(unsigned int apic, unsigned int vector)



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.