Skip to contentSkip to conversation

irq nobody cared workaround for i386

PATCH Applied Michal Schmidt 3 participants 5 messages Jun 19, 2007 – Jun 22, 2007 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Michal Schmidt

author
#1 lore raw

PATCH 1/1 irq nobody cared workaround for i386

Quoted 8 lines · Steven Rostedt

This is the final "design" for the nobody cared bug. For all IO-APICS other than the first one (the chained IO-APICS) we use the PCIX version of the mask and unmask interrupt routines. This changes the interrupt from level to edge for mask and edge to level for unmask. This keeps the PCI-E from thinking it's in legacy mode and assert an old fashion INT# interrupt which might spread to other interrupts.

Here's a port of the workaround to i386. I tested it successfully on IBM LS21.
Notice I had to disable the quirk handling in ack_ioapic_quirk_irq. The code path was triggering on LS21 and because it plays with the Interrupt Mask bit, it produced the doubled interrupts again. I don't like it and I need to think about a solution which would handle both quirks correctly.

Michal

i386/kernel/io_apic.c renamed from i386/kernel/io_apic.c.orig +58 -9
@@ -261,6 +261,18 @@ static void __unmask_IO_APIC_irq (unsign
 	__modify_IO_APIC_irq(irq, 0, 0x00010000);
 }
 
+/* trigger = 0 (edge mode) */
+static void __pcix_mask_IO_APIC_irq (unsigned int irq)
+{
+	__modify_IO_APIC_irq(irq, 0, 0x00008000);
+}
+
+/* mask = 0, trigger = 1 (level mode) */
+static void __pcix_unmask_IO_APIC_irq (unsigned int irq)
+{
+	__modify_IO_APIC_irq(irq, 0x00008000, 0x00010000);
+}
+
 static void mask_IO_APIC_irq (unsigned int irq)
 {
 	unsigned long flags;

@@ -279,6 +291,24 @@ static void unmask_IO_APIC_irq (unsigned
 	spin_unlock_irqrestore(&ioapic_lock, flags);
 }
 
+static void pcix_mask_IO_APIC_irq (unsigned int irq)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ioapic_lock, flags);
+	__pcix_mask_IO_APIC_irq(irq);
+	spin_unlock_irqrestore(&ioapic_lock, flags);
+}
+
+static void pcix_unmask_IO_APIC_irq (unsigned int irq)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ioapic_lock, flags);
+	__pcix_unmask_IO_APIC_irq(irq);
+	spin_unlock_irqrestore(&ioapic_lock, flags);
+}
+
 static void clear_IO_APIC_pin(unsigned int apic, unsigned int pin)
 {
 	struct IO_APIC_route_entry entry;

@@ -1257,22 +1287,27 @@ static int assign_irq_vector(int irq)
 
 	return vector;
 }
+
 static struct irq_chip ioapic_chip;
+static struct irq_chip pcix_ioapic_chip;
 
 #define IOAPIC_AUTO	-1
 #define IOAPIC_EDGE	0
 #define IOAPIC_LEVEL	1
 
-static void ioapic_register_intr(int irq, int vector, unsigned long trigger)
+static void ioapic_register_intr(int irq, int vector, unsigned long trigger,
+				 int pcix)
 {
+	struct irq_chip *chip = pcix ? &pcix_ioapic_chip : &ioapic_chip;
+
 	if ((trigger == IOAPIC_AUTO && IO_APIC_irq_trigger(irq)) ||
 			trigger == IOAPIC_LEVEL)
-		set_irq_chip_and_handler_name(irq, &ioapic_chip,
-					 handle_fasteoi_irq, "fasteoi");
-	else {
-		set_irq_chip_and_handler_name(irq, &ioapic_chip,
-					 handle_edge_irq, "edge");
-	}
+		set_irq_chip_and_handler_name(irq, chip, handle_fasteoi_irq,
+					      pcix ? "pcix-fasteoi" : "fasteoi");
+	else
+		set_irq_chip_and_handler_name(irq, chip, handle_edge_irq,
+					      pcix ? "pcix-edge" : "edge");
+	
 	set_intr_gate(vector, interrupt[irq]);
 }
 

@@ -1336,7 +1371,7 @@ static void __init setup_IO_APIC_irqs(vo
 		if (IO_APIC_IRQ(irq)) {
 			vector = assign_irq_vector(irq);
 			entry.vector = vector;
-			ioapic_register_intr(irq, vector, IOAPIC_AUTO);
+			ioapic_register_intr(irq, vector, IOAPIC_AUTO, apic>0);
 		
 			if (!apic && (irq < 16))
 				disable_8259A_irq(irq);

@@ -2027,6 +2062,7 @@ static void ack_ioapic_quirk_irq(unsigne
 
 	ack_APIC_irq();
 
+#if 0
 	if (!(v & (1 << (i & 0x1f)))) {
 		atomic_inc(&irq_mis_count);
 		spin_lock(&ioapic_lock);

@@ -2036,6 +2072,7 @@ static void ack_ioapic_quirk_irq(unsigne
 		__modify_IO_APIC_irq(irq, 0x00008000, 0x00010000);
 		spin_unlock(&ioapic_lock);
 	}
+#endif
 }
 
 static int ioapic_retrigger_irq(unsigned int irq)

@@ -2058,6 +2095,18 @@ static struct irq_chip ioapic_chip __rea
 	.retrigger	= ioapic_retrigger_irq,
 };
 
+static struct irq_chip pcix_ioapic_chip __read_mostly = {
+	.name 		= "IO-APIC",
+	.startup 	= startup_ioapic_irq,
+	.mask	 	= pcix_mask_IO_APIC_irq,
+	.unmask	 	= pcix_unmask_IO_APIC_irq,
+	.ack 		= ack_ioapic_irq,
+	.eoi 		= ack_ioapic_quirk_irq,
+#ifdef CONFIG_SMP
+	.set_affinity 	= set_ioapic_affinity_irq,
+#endif
+	.retrigger	= ioapic_retrigger_irq,
+};
 
 static inline void init_IO_APIC_traps(void)
 {

@@ -2858,7 +2907,7 @@ int io_apic_set_pci_routing (int ioapic,
 		mp_ioapics[ioapic].mpc_apicid, pin, entry.vector, irq,
 		edge_level, active_high_low);
 
-	ioapic_register_intr(irq, entry.vector, edge_level);
+	ioapic_register_intr(irq, entry.vector, edge_level, ioapic>0);
 
 	if (!ioapic && (irq < 16))
 		disable_8259A_irq(irq);
author
#2 lore raw
Quoted 12 lines · Michal Schmidt
Quoted 9 lines · Steven Rostedt

This is the final "design" for the nobody cared bug. For all IO-APICS other than the first one (the chained IO-APICS) we use the PCIX version of the mask and unmask interrupt routines. This changes the interrupt from level to edge for mask and edge to level for unmask. This keeps the PCI-E from thinking it's in legacy mode and assert an old fashion INT# interrupt which might spread to other interrupts.

Quoted 7 lines

Here's a port of the workaround to i386. I tested it successfully on IBM LS21.
Notice I had to disable the quirk handling in ack_ioapic_quirk_irq. The code path was triggering on LS21 and because it plays with the Interrupt Mask bit, it produced the doubled interrupts again. I don't like it and I need to think about a solution which would handle both quirks correctly.

I came to the conclusion that the IO-APICs which need the fix for the nobody cared bug don't have the issue ack_ioapic_quirk_irq is designed to work-around. It should be safe simply to use the normal ack_ioapic_irq as the .eoi method in pcix_ioapic_chip.
So this is the port of Steven's fix for the nobody cared bug to i386. It works fine on IBM LS21 I have access to.

Signed-off-by Michal Schmidt <[email protected]>
i386/kernel/io_apic.c renamed from i386/kernel/io_apic.c.orig +56 -9
@@ -261,6 +261,18 @@ static void __unmask_IO_APIC_irq (unsign
 	__modify_IO_APIC_irq(irq, 0, 0x00010000);
 }
 
+/* trigger = 0 (edge mode) */
+static void __pcix_mask_IO_APIC_irq (unsigned int irq)
+{
+	__modify_IO_APIC_irq(irq, 0, 0x00008000);
+}
+
+/* mask = 0, trigger = 1 (level mode) */
+static void __pcix_unmask_IO_APIC_irq (unsigned int irq)
+{
+	__modify_IO_APIC_irq(irq, 0x00008000, 0x00010000);
+}
+
 static void mask_IO_APIC_irq (unsigned int irq)
 {
 	unsigned long flags;

@@ -279,6 +291,24 @@ static void unmask_IO_APIC_irq (unsigned
 	spin_unlock_irqrestore(&ioapic_lock, flags);
 }
 
+static void pcix_mask_IO_APIC_irq (unsigned int irq)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ioapic_lock, flags);
+	__pcix_mask_IO_APIC_irq(irq);
+	spin_unlock_irqrestore(&ioapic_lock, flags);
+}
+
+static void pcix_unmask_IO_APIC_irq (unsigned int irq)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ioapic_lock, flags);
+	__pcix_unmask_IO_APIC_irq(irq);
+	spin_unlock_irqrestore(&ioapic_lock, flags);
+}
+
 static void clear_IO_APIC_pin(unsigned int apic, unsigned int pin)
 {
 	struct IO_APIC_route_entry entry;

@@ -1257,22 +1287,27 @@ static int assign_irq_vector(int irq)
 
 	return vector;
 }
+
 static struct irq_chip ioapic_chip;
+static struct irq_chip pcix_ioapic_chip;
 
 #define IOAPIC_AUTO	-1
 #define IOAPIC_EDGE	0
 #define IOAPIC_LEVEL	1
 
-static void ioapic_register_intr(int irq, int vector, unsigned long trigger)
+static void ioapic_register_intr(int irq, int vector, unsigned long trigger,
+				 int pcix)
 {
+	struct irq_chip *chip = pcix ? &pcix_ioapic_chip : &ioapic_chip;
+
 	if ((trigger == IOAPIC_AUTO && IO_APIC_irq_trigger(irq)) ||
 			trigger == IOAPIC_LEVEL)
-		set_irq_chip_and_handler_name(irq, &ioapic_chip,
-					 handle_fasteoi_irq, "fasteoi");
-	else {
-		set_irq_chip_and_handler_name(irq, &ioapic_chip,
-					 handle_edge_irq, "edge");
-	}
+		set_irq_chip_and_handler_name(irq, chip, handle_fasteoi_irq,
+					      pcix ? "pcix-fasteoi" : "fasteoi");
+	else
+		set_irq_chip_and_handler_name(irq, chip, handle_edge_irq,
+					      pcix ? "pcix-edge" : "edge");
+	
 	set_intr_gate(vector, interrupt[irq]);
 }
 

@@ -1336,7 +1371,7 @@ static void __init setup_IO_APIC_irqs(vo
 		if (IO_APIC_IRQ(irq)) {
 			vector = assign_irq_vector(irq);
 			entry.vector = vector;
-			ioapic_register_intr(irq, vector, IOAPIC_AUTO);
+			ioapic_register_intr(irq, vector, IOAPIC_AUTO, apic>0);
 		
 			if (!apic && (irq < 16))
 				disable_8259A_irq(irq);

@@ -2058,6 +2093,18 @@ static struct irq_chip ioapic_chip __rea
 	.retrigger	= ioapic_retrigger_irq,
 };
 
+static struct irq_chip pcix_ioapic_chip __read_mostly = {
+	.name 		= "IO-APIC",
+	.startup 	= startup_ioapic_irq,
+	.mask	 	= pcix_mask_IO_APIC_irq,
+	.unmask	 	= pcix_unmask_IO_APIC_irq,
+	.ack 		= ack_ioapic_irq,
+	.eoi 		= ack_ioapic_irq,
+#ifdef CONFIG_SMP
+	.set_affinity 	= set_ioapic_affinity_irq,
+#endif
+	.retrigger	= ioapic_retrigger_irq,
+};
 
 static inline void init_IO_APIC_traps(void)
 {

@@ -2858,7 +2905,7 @@ int io_apic_set_pci_routing (int ioapic,
 		mp_ioapics[ioapic].mpc_apicid, pin, entry.vector, irq,
 		edge_level, active_high_low);
 
-	ioapic_register_intr(irq, entry.vector, edge_level);
+	ioapic_register_intr(irq, entry.vector, edge_level, ioapic>0);
 
 	if (!ioapic && (irq < 16))
 		disable_8259A_irq(irq);
#3 lore raw
Quoted 9 lines · Michal Schmidt

Michal Schmidt wrote:

I came to the conclusion that the IO-APICs which need the fix for the nobody cared bug don't have the issue ack_ioapic_quirk_irq is designed to work-around. It should be safe simply to use the normal ack_ioapic_irq as the .eoi method in pcix_ioapic_chip.
So this is the port of Steven's fix for the nobody cared bug to i386. It works fine on IBM LS21 I have access to.

Thanks Michal for doing this!

i386/kernel/io_apic.c static void __init setup_IO_APIC_irqs(vo · line 1374

Quoted 9 lines
Signed-off-by Michal Schmidt <[email protected]>
i386/kernel/io_apic.c renamed from i386/kernel/io_apic.c.orig +1 -1
@@ -1336,7 +1371,7 @@ static void __init setup_IO_APIC_irqs(vo
 		if (IO_APIC_IRQ(irq)) {
 			vector = assign_irq_vector(irq);
 			entry.vector = vector;
-			ioapic_register_intr(irq, vector, IOAPIC_AUTO);
+			ioapic_register_intr(irq, vector, IOAPIC_AUTO, apic>0);

You want to make that "apic > 0". Note the spacing. If it breaks 80 characters, then simply put it to a new line.

Quoted 7 lines
i386/kernel/io_apic.c renamed from i386/kernel/io_apic.c.orig +0 -0
 			if (!apic && (irq < 16))
 				disable_8259A_irq(irq);

@@ -2058,6 +2093,18 @@ static struct irq_chip ioapic_chip __rea
 	.retrigger	= ioapic_retrigger_irq,
 };

[...]

i386/kernel/io_apic.c int io_apic_set_pci_routing (int ioapic, · line 2908

Quoted 8 lines

static inline void init_IO_APIC_traps(void)
{

i386/kernel/io_apic.c renamed from i386/kernel/io_apic.c.orig +1 -1
@@ -2858,7 +2905,7 @@ int io_apic_set_pci_routing (int ioapic,
 		mp_ioapics[ioapic].mpc_apicid, pin, entry.vector, irq,
 		edge_level, active_high_low);
 
-	ioapic_register_intr(irq, entry.vector, edge_level);
+	ioapic_register_intr(irq, entry.vector, edge_level, ioapic>0);

Again, add the spaces.

i386/kernel/io_apic.c int io_apic_set_pci_routing (int ioapic,

Quoted 5 lines
i386/kernel/io_apic.c renamed from i386/kernel/io_apic.c.orig +0 -0
 	if (!ioapic && (irq < 16))
 		disable_8259A_irq(irq);

ACK

-- Steve

author
#4 lore raw
Quoted 10 lines · Steven Rostedt
Quoted 8 lines · Michal Schmidt

I came to the conclusion that the IO-APICs which need the fix for the nobody cared bug don't have the issue ack_ioapic_quirk_irq is designed to work-around. It should be safe simply to use the normal ack_ioapic_irq as the .eoi method in pcix_ioapic_chip.
So this is the port of Steven's fix for the nobody cared bug to i386. It works fine on IBM LS21 I have access to.

Quoted 8 lines

You want to make that "apic > 0". Note the spacing. If it breaks 80 characters, then simply put it to a new line.

[...]
ACK

-- Steve

OK, I fixed the spacing in both occurences.

Signed-off-by Michal Schmidt <[email protected]>
i386/kernel/io_apic.c renamed from i386/kernel/io_apic.c.orig +57 -9
@@ -261,6 +261,18 @@ static void __unmask_IO_APIC_irq (unsign
 	__modify_IO_APIC_irq(irq, 0, 0x00010000);
 }
 
+/* trigger = 0 (edge mode) */
+static void __pcix_mask_IO_APIC_irq (unsigned int irq)
+{
+	__modify_IO_APIC_irq(irq, 0, 0x00008000);
+}
+
+/* mask = 0, trigger = 1 (level mode) */
+static void __pcix_unmask_IO_APIC_irq (unsigned int irq)
+{
+	__modify_IO_APIC_irq(irq, 0x00008000, 0x00010000);
+}
+
 static void mask_IO_APIC_irq (unsigned int irq)
 {
 	unsigned long flags;

@@ -279,6 +291,24 @@ static void unmask_IO_APIC_irq (unsigned
 	spin_unlock_irqrestore(&ioapic_lock, flags);
 }
 
+static void pcix_mask_IO_APIC_irq (unsigned int irq)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ioapic_lock, flags);
+	__pcix_mask_IO_APIC_irq(irq);
+	spin_unlock_irqrestore(&ioapic_lock, flags);
+}
+
+static void pcix_unmask_IO_APIC_irq (unsigned int irq)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ioapic_lock, flags);
+	__pcix_unmask_IO_APIC_irq(irq);
+	spin_unlock_irqrestore(&ioapic_lock, flags);
+}
+
 static void clear_IO_APIC_pin(unsigned int apic, unsigned int pin)
 {
 	struct IO_APIC_route_entry entry;

@@ -1257,22 +1287,27 @@ static int assign_irq_vector(int irq)
 
 	return vector;
 }
+
 static struct irq_chip ioapic_chip;
+static struct irq_chip pcix_ioapic_chip;
 
 #define IOAPIC_AUTO	-1
 #define IOAPIC_EDGE	0
 #define IOAPIC_LEVEL	1
 
-static void ioapic_register_intr(int irq, int vector, unsigned long trigger)
+static void ioapic_register_intr(int irq, int vector, unsigned long trigger,
+				 int pcix)
 {
+	struct irq_chip *chip = pcix ? &pcix_ioapic_chip : &ioapic_chip;
+
 	if ((trigger == IOAPIC_AUTO && IO_APIC_irq_trigger(irq)) ||
 			trigger == IOAPIC_LEVEL)
-		set_irq_chip_and_handler_name(irq, &ioapic_chip,
-					 handle_fasteoi_irq, "fasteoi");
-	else {
-		set_irq_chip_and_handler_name(irq, &ioapic_chip,
-					 handle_edge_irq, "edge");
-	}
+		set_irq_chip_and_handler_name(irq, chip, handle_fasteoi_irq,
+					      pcix ? "pcix-fasteoi" : "fasteoi");
+	else
+		set_irq_chip_and_handler_name(irq, chip, handle_edge_irq,
+					      pcix ? "pcix-edge" : "edge");
+	
 	set_intr_gate(vector, interrupt[irq]);
 }
 

@@ -1336,7 +1371,8 @@ static void __init setup_IO_APIC_irqs(vo
 		if (IO_APIC_IRQ(irq)) {
 			vector = assign_irq_vector(irq);
 			entry.vector = vector;
-			ioapic_register_intr(irq, vector, IOAPIC_AUTO);
+			ioapic_register_intr(irq, vector, IOAPIC_AUTO,
+					     apic > 0);
 		
 			if (!apic && (irq < 16))
 				disable_8259A_irq(irq);

@@ -2058,6 +2094,18 @@ static struct irq_chip ioapic_chip __rea
 	.retrigger	= ioapic_retrigger_irq,
 };
 
+static struct irq_chip pcix_ioapic_chip __read_mostly = {
+	.name 		= "IO-APIC",
+	.startup 	= startup_ioapic_irq,
+	.mask	 	= pcix_mask_IO_APIC_irq,
+	.unmask	 	= pcix_unmask_IO_APIC_irq,
+	.ack 		= ack_ioapic_irq,
+	.eoi 		= ack_ioapic_irq,
+#ifdef CONFIG_SMP
+	.set_affinity 	= set_ioapic_affinity_irq,
+#endif
+	.retrigger	= ioapic_retrigger_irq,
+};
 
 static inline void init_IO_APIC_traps(void)
 {

@@ -2858,7 +2906,7 @@ int io_apic_set_pci_routing (int ioapic,
 		mp_ioapics[ioapic].mpc_apicid, pin, entry.vector, irq,
 		edge_level, active_high_low);
 
-	ioapic_register_intr(irq, entry.vector, edge_level);
+	ioapic_register_intr(irq, entry.vector, edge_level, ioapic > 0);
 
 	if (!ioapic && (irq < 16))
 		disable_8259A_irq(irq);
Applied
#5 lore raw

i386/kernel/io_apic.c

Quoted 6 lines · Michal Schmidt

OK, I fixed the spacing in both occurences.

Signed-off-by Michal Schmidt <[email protected]>
i386/kernel/io_apic.c renamed from i386/kernel/io_apic.c.orig +0 -0

Please create -p1 patches next time.

Applied. Thanks,

tglx