[patch v2 0/3] ioapic kdump, eoi fixes

[patch v2 0/3] ioapic kdump, eoi fixes

[Suresh Siddha]

v2 of the original series
http://marc.info/?l=linux-kernel&m=131309907720703&w=2

Changes in v2:
* Make sure that the IO-APIC RTE trigger mode is set to level trigger
  in clear_IO_APIC_pin() before doing EOI to clear the remoteIRR.

thanks,
suresh

[patch v2 1/3] x86, kdump, ioapic: reset remote IRR in clear_IO_APIC

[Suresh Siddha]

[-- Attachment #1: reset_IRR_in_clear_IO_APIC_pin.patch --]
[-- Type: text/plain, Size: 3287 bytes --]

In the kdump scenario mentioned below, we can have a case where the device
using level triggered interrupt will not generate any interrupts in the
kdump kernel.

1. IO-APIC sends a level triggered interrupt to the CPU's local APIC
2. Kernel crashed before the CPU services this interrupt, leaving the remoteIRR
   in the IO-APIC set
3. kdump kernel boot sequence does clear_IO_APIC() as part of IO-APIC
   initialization. But this fails to reset remoteIRR bit of the IO-APIC RTE
   as the remoteIRR bit is read-only.
4. Device using that level triggered entry can't generate any more interrupts
   because of the remoteIRR bit.

In clear_IO_APIC_pin(), check if the remoteIRR bit is set and if so
do an explicit attempt to clear it (by doing EOI write on modern io-apic's
and changing trigger mode to edge/level on older io-apic's).
Also before doing the explicit EOI to the io-apic, ensure that the trigger
mode is indeed set to level. This will enable the explicit EOI to the
io-apic to reset the remoteIRR bit.

Fixes: https://bugzilla.novell.com/show_bug.cgi?id=701686   

Root-caused-by: Thomas Renninger <trenn@suse.de>
Tested-by: Leonardo Chiquitto <lchiquitto@novell.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Rafael Wysocki <rjw@novell.com>
Cc: Maciej W. Rozycki <macro@linux-mips.org>
---
 arch/x86/kernel/apic/io_apic.c |   48 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

Index: linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-2.6-tip.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
@@ -593,10 +593,56 @@ static void clear_IO_APIC_pin(unsigned i
 	entry = ioapic_read_entry(apic, pin);
 	if (entry.delivery_mode == dest_SMI)
 		return;
+
 	/*
-	 * Disable it in the IO-APIC irq-routing table:
+	 * Make sure the entry is masked and re-read the contents to check
+	 * if it is a level triggered pin and if the remoteIRR is set.
+	 */
+	if (!entry.mask) {
+		entry.mask = 1;
+		ioapic_write_entry(apic, pin, entry);
+		entry = ioapic_read_entry(apic, pin);
+	}
+
+	if (entry.irr) {
+		/*
+		 * Make sure the trigger mode is set to level. Explicit EOI
+		 * doesn't clear the remoteIRR if the trigger mode is not
+		 * set to level.
+		 */
+		if (!entry.trigger) {
+			entry.trigger = IOAPIC_LEVEL;
+			ioapic_write_entry(apic, pin, entry);
+		}
+
+		if (mpc_ioapic_ver(apic) >= 0x20) {
+			unsigned long flags;
+
+			raw_spin_lock_irqsave(&ioapic_lock, flags);
+			io_apic_eoi(apic, entry.vector);
+			raw_spin_unlock_irqrestore(&ioapic_lock, flags);
+		} else {
+			/*
+			 * Mechanism by which we clear remoteIRR in this
+			 * case is by changing the trigger mode to edge and
+			 * back to level.
+			 */
+			entry.trigger = IOAPIC_EDGE;
+			ioapic_write_entry(apic, pin, entry);
+			entry.trigger = IOAPIC_LEVEL;
+			ioapic_write_entry(apic, pin, entry);
+		}
+	}
+
+	/*
+	 * Clear the rest of the bits in the IO-APIC RTE except for the mask
+	 * bit.
 	 */
 	ioapic_mask_entry(apic, pin);
+	entry = ioapic_read_entry(apic, pin);
+	if (entry.irr)
+		printk(KERN_ERR "Unable to reset IRR for apic: %d, pin :%d\n",
+		       mpc_ioapic_id(apic), pin);
 }
 
 static void clear_IO_APIC (void)

[patch v2 2/3] x86, ioapic: restore the mask bit correctly in eoi_ioapic_irq()

[Suresh Siddha]

[-- Attachment #1: fix_eoi_ioapic_irq_mask_unmask.patch --]
[-- Type: text/plain, Size: 3023 bytes --]

For older io-apic's, we were clearing the remote IRR by changing the RTE
trigger mode to edge and then back to level. We wanted to mask the RTE during
this process, so we were essentially doing mask+edge and then to unmask+level.

As part of the commit ca64c47cecd0321b2e0dcbd7aaff44b68ce20654, we
moved this EOI process earlier where the IO-APIC RTE is masked. So we were
wrongly unmasking it in the eoi_ioapic_irq().

So change the remoteIRR clear sequence in eoi_ioapic_irq() to mask + edge and
then restore the previous RTE entry which will restore the mask status aswell
as the level trigger.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Maciej W. Rozycki <macro@linux-mips.org>
Cc: Thomas Renninger <trenn@suse.de>
Cc: Rafael Wysocki <rjw@novell.com>
---
 arch/x86/kernel/apic/io_apic.c |   43 +++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)

Index: linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-2.6-tip.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
@@ -394,13 +394,21 @@ union entry_union {
 	struct IO_APIC_route_entry entry;
 };
 
+static struct IO_APIC_route_entry __ioapic_read_entry(int apic, int pin)
+{
+	union entry_union eu;
+
+	eu.w1 = io_apic_read(apic, 0x10 + 2 * pin);
+	eu.w2 = io_apic_read(apic, 0x11 + 2 * pin);
+	return eu.entry;
+}
+
 static struct IO_APIC_route_entry ioapic_read_entry(int apic, int pin)
 {
 	union entry_union eu;
 	unsigned long flags;
 	raw_spin_lock_irqsave(&ioapic_lock, flags);
-	eu.w1 = io_apic_read(apic, 0x10 + 2 * pin);
-	eu.w2 = io_apic_read(apic, 0x11 + 2 * pin);
+	eu.entry = __ioapic_read_entry(apic, pin);
 	raw_spin_unlock_irqrestore(&ioapic_lock, flags);
 	return eu.entry;
 }
@@ -529,18 +537,6 @@ static void io_apic_modify_irq(struct ir
 		__io_apic_modify_irq(entry, mask_and, mask_or, final);
 }
 
-static void __mask_and_edge_IO_APIC_irq(struct irq_pin_list *entry)
-{
-	__io_apic_modify_irq(entry, ~IO_APIC_REDIR_LEVEL_TRIGGER,
-			     IO_APIC_REDIR_MASKED, NULL);
-}
-
-static void __unmask_and_level_IO_APIC_irq(struct irq_pin_list *entry)
-{
-	__io_apic_modify_irq(entry, ~IO_APIC_REDIR_MASKED,
-			     IO_APIC_REDIR_LEVEL_TRIGGER, NULL);
-}
-
 static void io_apic_sync(struct irq_pin_list *entry)
 {
 	/*
@@ -2488,8 +2484,23 @@ static void eoi_ioapic_irq(unsigned int 
 			else
 				io_apic_eoi(entry->apic, cfg->vector);
 		} else {
-			__mask_and_edge_IO_APIC_irq(entry);
-			__unmask_and_level_IO_APIC_irq(entry);
+			struct IO_APIC_route_entry rte, rte1;
+
+			rte = rte1 =
+				__ioapic_read_entry(entry->apic, entry->pin);
+
+			/*
+			 * Mask the entry and change the trigger mode to edge.
+			 */
+			rte1.mask = 1;
+			rte1.trigger = IOAPIC_EDGE;
+
+			__ioapic_write_entry(apic, pin, rte1);
+
+			/*
+			 * Restore the previous level triggered entry.
+			 */
+			__ioapic_write_entry(apic, pin, rte);
 		}
 	}
 	raw_spin_unlock_irqrestore(&ioapic_lock, flags);

[patch v2 3/3] x86, ioapic: consolidate the explicit EOI code

[Suresh Siddha]

[-- Attachment #1: consolidate_eoi_ioapic_irq.patch --]
[-- Type: text/plain, Size: 5724 bytes --]

Consolidate the io-apic EOI code in clear_IO_APIC_pin() and eoi_ioapic_irq().

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Thomas Renninger <trenn@suse.de>
Cc: Rafael Wysocki <rjw@novell.com>
Cc: Maciej W. Rozycki <macro@linux-mips.org>
---
 arch/x86/kernel/apic/io_apic.c |  139 +++++++++++++++++++----------------------
 1 file changed, 65 insertions(+), 74 deletions(-)

Index: linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-2.6-tip.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
@@ -581,6 +581,66 @@ static void unmask_ioapic_irq(struct irq
 	unmask_ioapic(data->chip_data);
 }
 
+/*
+ * IO-APIC versions below 0x20 don't support EOI register.
+ * For the record, here is the information about various versions:
+ *     0Xh     82489DX
+ *     1Xh     I/OAPIC or I/O(x)APIC which are not PCI 2.2 Compliant
+ *     2Xh     I/O(x)APIC which is PCI 2.2 Compliant
+ *     30h-FFh Reserved
+ *
+ * Some of the Intel ICH Specs (ICH2 to ICH5) documents the io-apic
+ * version as 0x2. This is an error with documentation and these ICH chips
+ * use io-apic's of version 0x20.
+ *
+ * For IO-APIC's with EOI register, we use that to do an explicit EOI.
+ * Otherwise, we simulate the EOI message manually by changing the trigger
+ * mode to edge and then back to level, with RTE being masked during this.
+ */
+static void __eoi_ioapic_pin(int apic, int pin, int vector, struct irq_cfg *cfg)
+{
+	if (mpc_ioapic_ver(apic) >= 0x20) {
+		/*
+		 * Intr-remapping uses pin number as the virtual vector
+		 * in the RTE. Actual vector is programmed in
+		 * intr-remapping table entry. Hence for the io-apic
+		 * EOI we use the pin number.
+		 */
+		if (cfg && irq_remapped(cfg))
+			io_apic_eoi(apic, pin);
+		else
+			io_apic_eoi(apic, vector);
+	} else {
+		struct IO_APIC_route_entry entry, entry1;
+
+		entry = entry1 = __ioapic_read_entry(apic, pin);
+
+		/*
+		 * Mask the entry and change the trigger mode to edge.
+		 */
+		entry1.mask = 1;
+		entry1.trigger = IOAPIC_EDGE;
+
+		__ioapic_write_entry(apic, pin, entry1);
+
+		/*
+		 * Restore the previous level triggered entry.
+		 */
+		__ioapic_write_entry(apic, pin, entry);
+	}
+}
+
+static void eoi_ioapic_irq(unsigned int irq, struct irq_cfg *cfg)
+{
+	struct irq_pin_list *entry;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&ioapic_lock, flags);
+	for_each_irq_pin(entry, cfg->irq_2_pin)
+		__eoi_ioapic_pin(entry->apic, entry->pin, cfg->vector, cfg);
+	raw_spin_unlock_irqrestore(&ioapic_lock, flags);
+}
+
 static void clear_IO_APIC_pin(unsigned int apic, unsigned int pin)
 {
 	struct IO_APIC_route_entry entry;
@@ -601,6 +661,8 @@ static void clear_IO_APIC_pin(unsigned i
 	}
 
 	if (entry.irr) {
+		unsigned long flags;
+
 		/*
 		 * Make sure the trigger mode is set to level. Explicit EOI
 		 * doesn't clear the remoteIRR if the trigger mode is not
@@ -611,23 +673,9 @@ static void clear_IO_APIC_pin(unsigned i
 			ioapic_write_entry(apic, pin, entry);
 		}
 
-		if (mpc_ioapic_ver(apic) >= 0x20) {
-			unsigned long flags;
-
-			raw_spin_lock_irqsave(&ioapic_lock, flags);
-			io_apic_eoi(apic, entry.vector);
-			raw_spin_unlock_irqrestore(&ioapic_lock, flags);
-		} else {
-			/*
-			 * Mechanism by which we clear remoteIRR in this
-			 * case is by changing the trigger mode to edge and
-			 * back to level.
-			 */
-			entry.trigger = IOAPIC_EDGE;
-			ioapic_write_entry(apic, pin, entry);
-			entry.trigger = IOAPIC_LEVEL;
-			ioapic_write_entry(apic, pin, entry);
-		}
+		raw_spin_lock_irqsave(&ioapic_lock, flags);
+		__eoi_ioapic_pin(apic, pin, entry.vector, NULL);
+		raw_spin_unlock_irqrestore(&ioapic_lock, flags);
 	}
 
 	/*
@@ -2449,63 +2497,6 @@ static void ack_apic_edge(struct irq_dat
 
 atomic_t irq_mis_count;
 
-/*
- * IO-APIC versions below 0x20 don't support EOI register.
- * For the record, here is the information about various versions:
- *     0Xh     82489DX
- *     1Xh     I/OAPIC or I/O(x)APIC which are not PCI 2.2 Compliant
- *     2Xh     I/O(x)APIC which is PCI 2.2 Compliant
- *     30h-FFh Reserved
- *
- * Some of the Intel ICH Specs (ICH2 to ICH5) documents the io-apic
- * version as 0x2. This is an error with documentation and these ICH chips
- * use io-apic's of version 0x20.
- *
- * For IO-APIC's with EOI register, we use that to do an explicit EOI.
- * Otherwise, we simulate the EOI message manually by changing the trigger
- * mode to edge and then back to level, with RTE being masked during this.
-*/
-static void eoi_ioapic_irq(unsigned int irq, struct irq_cfg *cfg)
-{
-	struct irq_pin_list *entry;
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&ioapic_lock, flags);
-	for_each_irq_pin(entry, cfg->irq_2_pin) {
-		if (mpc_ioapic_ver(entry->apic) >= 0x20) {
-			/*
-			 * Intr-remapping uses pin number as the virtual vector
-			 * in the RTE. Actual vector is programmed in
-			 * intr-remapping table entry. Hence for the io-apic
-			 * EOI we use the pin number.
-			 */
-			if (irq_remapped(cfg))
-				io_apic_eoi(entry->apic, entry->pin);
-			else
-				io_apic_eoi(entry->apic, cfg->vector);
-		} else {
-			struct IO_APIC_route_entry rte, rte1;
-
-			rte = rte1 =
-				__ioapic_read_entry(entry->apic, entry->pin);
-
-			/*
-			 * Mask the entry and change the trigger mode to edge.
-			 */
-			rte1.mask = 1;
-			rte1.trigger = IOAPIC_EDGE;
-
-			__ioapic_write_entry(apic, pin, rte1);
-
-			/*
-			 * Restore the previous level triggered entry.
-			 */
-			__ioapic_write_entry(apic, pin, rte);
-		}
-	}
-	raw_spin_unlock_irqrestore(&ioapic_lock, flags);
-}
-
 static void ack_apic_level(struct irq_data *data)
 {
 	struct irq_cfg *cfg = data->chip_data;