WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] Actually clear IO-APIC pins on boot and shutdown whe

To: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] Actually clear IO-APIC pins on boot and shutdown when used with an IOMMU
From: Alex Williamson <alex.williamson@xxxxxx>
Date: Tue, 15 Dec 2009 16:16:18 -0700
Delivery-date: Tue, 15 Dec 2009 15:16:29 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: HP OSLO R&D
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH] Actually clear IO-APIC pins on boot and shutdown when used with an IOMMU, Alex Williamson <=