[Xen-devel] [PATCH V2] x86/i8259: Handle bogus spurious interrupts more quietly

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

c/s 25336:edd7c7ad1ad2 introduced the concept of a bogus vector, for in irqs
delivered through the i8259 PIC after IO-APICs had been set up.

However, if supurious PIC vectors are received, many "No irq handler for vector"
log messages can be seen on the console.

This patch adds to the bogus vector logic to detect spurious PIC vectors and
simply ignore them.  _mask_and_ack_8259A_irq() has been modified to return a
boolean indicating whether the irq is real or not, and in the case of a spurious
vector, the error in do_IRQ() is not printed.

One complication is that now, _mask_and_ack_8259A_irq() can get called whatever
the ack mode is, so has been altered to work out whether it should EOI the irq
or not.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

--
Changes since v1
    * Modify _mask_and_ack_8259A_irq() rather than basically duplicating it

diff -r 1126b3079bef -r 3cacac4d8a4d xen/arch/x86/i8259.c
--- a/xen/arch/x86/i8259.c
+++ b/xen/arch/x86/i8259.c
@@ -85,10 +85,12 @@ BUILD_16_IRQS(0xc) BUILD_16_IRQS(0xd) BU
 
 static DEFINE_SPINLOCK(i8259A_lock);
 
-static void _mask_and_ack_8259A_irq(unsigned int irq);
+static bool_t _mask_and_ack_8259A_irq(unsigned int irq);
 
-void (*__read_mostly bogus_8259A_irq)(unsigned int irq) =
-    _mask_and_ack_8259A_irq;
+bool_t bogus_8259A_irq(unsigned int irq)
+{
+    return _mask_and_ack_8259A_irq(irq);
+}
 
 static void mask_and_ack_8259A_irq(struct irq_desc *desc)
 {
@@ -239,12 +241,15 @@ static inline int i8259A_irq_real(unsign
  * Careful! The 8259A is a fragile beast, it pretty
  * much _has_ to be done exactly like this (mask it
  * first, _then_ send the EOI, and the order of EOI
- * to the two 8259s is important!
+ * to the two 8259s is important!  Return a boolean
+ * indicating whether the irq was genuine or spurious.
  */
-static void _mask_and_ack_8259A_irq(unsigned int irq)
+static bool_t _mask_and_ack_8259A_irq(unsigned int irq)
 {
     unsigned int irqmask = 1 << irq;
     unsigned long flags;
+    bool_t real_irq = 1; /* Assume real unless spurious */
+    bool_t need_eoi = i8259A_irq_type.ack != disable_8259A_irq;
 
     spin_lock_irqsave(&i8259A_lock, flags);
     /*
@@ -270,15 +275,19 @@ static void _mask_and_ack_8259A_irq(unsi
     if (irq & 8) {
         inb(0xA1);              /* DUMMY - (do we need this?) */
         outb(cached_A1,0xA1);
-        outb(0x60 + (irq & 7), 0xA0);/* 'Specific EOI' to slave */
-        outb(0x62,0x20);        /* 'Specific EOI' to master-IRQ2 */
+        if ( need_eoi )
+        {
+            outb(0x60 + (irq & 7), 0xA0);/* 'Specific EOI' to slave */
+            outb(0x62,0x20);        /* 'Specific EOI' to master-IRQ2 */
+        }
     } else {
         inb(0x21);              /* DUMMY - (do we need this?) */
         outb(cached_21,0x21);
-        outb(0x60 + irq, 0x20);/* 'Specific EOI' to master */
+        if ( need_eoi )
+            outb(0x60 + irq, 0x20);/* 'Specific EOI' to master */
     }
     spin_unlock_irqrestore(&i8259A_lock, flags);
-    return;
+    return real_irq;
 
  spurious_8259A_irq:
     /*
@@ -293,6 +302,7 @@ static void _mask_and_ack_8259A_irq(unsi
 
     {
         static int spurious_irq_mask;
+        real_irq = 0;
         /*
          * At this point we can be sure the IRQ is spurious,
          * lets ACK and report it. [once per IRQ]
@@ -367,19 +377,13 @@ void __devinit init_8259A(int auto_eoi)
                                is to be investigated) */
 
     if (auto_eoi)
-    {
         /*
          * in AEOI mode we just have to mask the interrupt
          * when acking.
          */
         i8259A_irq_type.ack = disable_8259A_irq;
-        bogus_8259A_irq = _disable_8259A_irq;
-    }
     else
-    {
         i8259A_irq_type.ack = mask_and_ack_8259A_irq;
-        bogus_8259A_irq = _mask_and_ack_8259A_irq;
-    }
 
     udelay(100);            /* wait for 8259A to initialize */
 
diff -r 1126b3079bef -r 3cacac4d8a4d xen/arch/x86/irq.c
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -817,11 +817,11 @@ void do_IRQ(struct cpu_user_regs *regs)
                 ack_APIC_irq();
             else
                 kind = "";
-            if ( vector >= FIRST_LEGACY_VECTOR &&
-                 vector <= LAST_LEGACY_VECTOR )
-                bogus_8259A_irq(vector - FIRST_LEGACY_VECTOR);
-            printk("CPU%u: No irq handler for vector %02x (IRQ %d%s)\n",
-                   smp_processor_id(), vector, irq, kind);
+            if ( ! ( vector >= FIRST_LEGACY_VECTOR &&
+                     vector <= LAST_LEGACY_VECTOR &&
+                     bogus_8259A_irq(vector - FIRST_LEGACY_VECTOR) ) )
+                printk("CPU%u: No irq handler for vector %02x (IRQ %d%s)\n",
+                       smp_processor_id(), vector, irq, kind);
             TRACE_1D(TRC_HW_IRQ_UNMAPPED_VECTOR, vector);
         }
         goto out_no_unlock;
diff -r 1126b3079bef -r 3cacac4d8a4d xen/include/asm-x86/irq.h
--- a/xen/include/asm-x86/irq.h
+++ b/xen/include/asm-x86/irq.h
@@ -104,7 +104,7 @@ void mask_8259A(void);
 void unmask_8259A(void);
 void init_8259A(int aeoi);
 void make_8259A_irq(unsigned int irq);
-extern void (*bogus_8259A_irq)(unsigned int irq);
+bool_t bogus_8259A_irq(unsigned int irq);
 int i8259A_suspend(void);
 int i8259A_resume(void);
 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel