[PATCH] i386: irq: Kill IRQ compression

[PATCH] i386: irq: Kill IRQ compression

[Len Brown]

This code makes simple systems complex:

ACPI: PCI Interrupt 0000:03:04.0[A] -> GSI 18 (level, low) -> IRQ 16
ACPI: PCI Interrupt 0000:00:1d.0[A] -> GSI 16 (level, low) -> IRQ 17
ACPI: PCI Interrupt 0000:00:1d.1[B] -> GSI 19 (level, low) -> IRQ 18
ACPI: PCI Interrupt 0000:00:1d.7[D] -> GSI 23 (level, low) -> IRQ 19

The same code was already removed from x86_64

Signed-off-by: Len Brown <len.brown@intel.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Natalie Protasevich <Natalie.Protasevich@UNISYS.com>
---
 arch/i386/kernel/io_apic.c |    5 ----
 arch/i386/kernel/mpparse.c |   41 ---------------------------------
 include/asm-i386/io_apic.h |    1
 3 files changed, 1 insertion(+), 46 deletions(-)

Index: linus/arch/i386/kernel/mpparse.c
===================================================================
--- linus.orig/arch/i386/kernel/mpparse.c
+++ linus/arch/i386/kernel/mpparse.c
@@ -1041,20 +1041,12 @@ void __init mp_config_acpi_legacy_irqs (
 	}
 }
 
-#define MAX_GSI_NUM	4096
-
 int mp_register_gsi(u32 gsi, int triggering, int polarity)
 {
 	int ioapic = -1;
 	int ioapic_pin = 0;
 	int idx, bit = 0;
 	static int pci_irq = 16;
-	/*
-	 * Mapping between Global System Interrups, which
-	 * represent all possible interrupts, and IRQs
-	 * assigned to actual devices.
-	 */
-	static int		gsi_to_irq[MAX_GSI_NUM];
 
 	/* Don't set up the ACPI SCI because it's already set up */
 	if (acpi_gbl_FADT.sci_interrupt == gsi)
@@ -1087,42 +1079,11 @@ int mp_register_gsi(u32 gsi, int trigger
 	if ((1<<bit) & mp_ioapic_routing[ioapic].pin_programmed[idx]) {
 		Dprintk(KERN_DEBUG "Pin %d-%d already programmed\n",
 			mp_ioapic_routing[ioapic].apic_id, ioapic_pin);
-		return gsi_to_irq[gsi];
+		return gsi;
 	}
 
 	mp_ioapic_routing[ioapic].pin_programmed[idx] |= (1<<bit);
 
-	if (triggering == ACPI_LEVEL_SENSITIVE) {
-		/*
-		 * For PCI devices assign IRQs in order, avoiding gaps
-		 * due to unused I/O APIC pins.
-		 */
-		int irq = gsi;
-		if (gsi < MAX_GSI_NUM) {
-			/*
-			 * Retain the VIA chipset work-around (gsi > 15), but
-			 * avoid a problem where the 8254 timer (IRQ0) is setup
-			 * via an override (so it's not on pin 0 of the ioapic),
-			 * and at the same time, the pin 0 interrupt is a PCI
-			 * type.  The gsi > 15 test could cause these two pins
-			 * to be shared as IRQ0, and they are not shareable.
-			 * So test for this condition, and if necessary, avoid
-			 * the pin collision.
-			 */
-			if (gsi > 15 || (gsi == 0 && !timer_uses_ioapic_pin_0))
-				gsi = pci_irq++;
-			/*
-			 * Don't assign IRQ used by ACPI SCI
-			 */
-			if (gsi == acpi_gbl_FADT.sci_interrupt)
-				gsi = pci_irq++;
-			gsi_to_irq[irq] = gsi;
-		} else {
-			printk(KERN_ERR "GSI %u is too high\n", gsi);
-			return gsi;
-		}
-	}
-
 	io_apic_set_pci_routing(ioapic, ioapic_pin, gsi,
 		    triggering == ACPI_EDGE_SENSITIVE ? 0 : 1,
 		    polarity == ACPI_ACTIVE_HIGH ? 0 : 1);
Index: linus/arch/i386/kernel/io_apic.c
===================================================================
--- linus.orig/arch/i386/kernel/io_apic.c
+++ linus/arch/i386/kernel/io_apic.c
@@ -2212,8 +2212,6 @@ static inline void unlock_ExtINT_logic(v
 	ioapic_write_entry(apic, pin, entry0);
 }
 
-int timer_uses_ioapic_pin_0;
-
 /*
  * This code may look a bit paranoid, but it's supposed to cooperate with
  * a wide range of boards and BIOS bugs.  Fortunately only the timer IRQ
@@ -2250,9 +2248,6 @@ static inline void __init check_timer(vo
 	pin2  = ioapic_i8259.pin;
 	apic2 = ioapic_i8259.apic;
 
-	if (pin1 == 0)
-		timer_uses_ioapic_pin_0 = 1;
-
 	printk(KERN_INFO "..TIMER: vector=0x%02X apic1=%d pin1=%d apic2=%d pin2=%d\n",
 		vector, apic1, pin1, apic2, pin2);
 
Index: linus/include/asm-i386/io_apic.h
===================================================================
--- linus.orig/include/asm-i386/io_apic.h
+++ linus/include/asm-i386/io_apic.h
@@ -142,7 +142,6 @@ extern int io_apic_get_unique_id (int io
 extern int io_apic_get_version (int ioapic);
 extern int io_apic_get_redir_entries (int ioapic);
 extern int io_apic_set_pci_routing (int ioapic, int pin, int irq, int edge_level, int active_high_low);
-extern int timer_uses_ioapic_pin_0;
 #endif /* CONFIG_ACPI */
 
 extern int (*ioapic_renumber_irq)(int ioapic, int irq);

Re: [PATCH] i386: irq: Kill IRQ compression

[Eric W. Biederman]

Len Brown <lenb@kernel.org> writes:

> This code makes simple systems complex:
>
> ACPI: PCI Interrupt 0000:03:04.0[A] -> GSI 18 (level, low) -> IRQ 16
> ACPI: PCI Interrupt 0000:00:1d.0[A] -> GSI 16 (level, low) -> IRQ 17
> ACPI: PCI Interrupt 0000:00:1d.1[B] -> GSI 19 (level, low) -> IRQ 18
> ACPI: PCI Interrupt 0000:00:1d.7[D] -> GSI 23 (level, low) -> IRQ 19
>
> The same code was already removed from x86_64

By itself I don't think we are going to observe any real problems
with this patch.

However if we are going to be serious about this we need to do a
few more things.

- kill iopaic_renumber_irq.
- Increase NR_IRQS.

We will still be limited to about 208 interrupts in use at one
time, but we can allow more irq sources to be described.

This reminds me.  I really need to dig up my patch that doesn't
allocate a vector for an irq until request irq time.

Eric

Re: [PATCH] i386: irq: Kill IRQ compression

[Eric W. Biederman]

ebiederm@xmission.com (Eric W. Biederman) writes:

> By itself I don't think we are going to observe any real problems
> with this patch.
>
> However if we are going to be serious about this we need to do a
> few more things.
>
> - kill ioapic_renumber_irq.

Looking closer ioapic_renumber_irq does not appear to be an irq
compress thing.  Rather it appears to be a work around for a weird
acpi implementation where gsi 0 - 15 are not the ISA irqs. 

Natalie is my reading of the code there correct?

If so keeping ioapic_renumber_irq makes sense.  Although giving
it a name that suggests it is working around weird implementation
details would be good.

Eric

[PATCH] i386 irq: Kill NR_IRQ_VECTORS and increase NR_IRQS

[Eric W. Biederman]

Due to hardware and software implementation limits the i386 kernel can
only use 224 or so different IRQs at one time.  However except for
actually having the irqs delivered there are no limits except the
arbitrary number NR_IRQS on how many irqs we can talk about and deal
with.

Frequently not all io_apics inputs are connected to interrupt traces
going to real devices, and since even when they are devices don't
always use all of interrupt traces routed to them.  So it makes sense
to be able to talk about many more irq sources then we can actually
use.

So this patch consolidates NR_IRQS and NR_IRQ_VECTORS into just
NR_IRQS and raises NR_IRQS where appropriate above the number of irqs
we can use at one time. 

Allowing the kernel to work on big machines without complicated
and error prone irq remapping logic.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 arch/i386/kernel/acpi/boot.c                       |    4 ++--
 arch/i386/kernel/io_apic.c                         |    6 +++---
 include/asm-i386/mach-default/irq_vectors_limits.h |    8 +++-----
 include/asm-i386/mach-generic/irq_vectors_limits.h |    3 +--
 include/asm-i386/mach-summit/irq_vectors_limits.h  |    3 +--
 include/asm-i386/mach-visws/irq_vectors.h          |    1 -
 include/asm-i386/mach-voyager/irq_vectors.h        |    1 -
 7 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/arch/i386/kernel/acpi/boot.c b/arch/i386/kernel/acpi/boot.c
index e94aff6..4375abd 100644
--- a/arch/i386/kernel/acpi/boot.c
+++ b/arch/i386/kernel/acpi/boot.c
@@ -826,7 +826,7 @@ static int __init acpi_parse_madt_ioapic_entries(void)
 
 	count =
 	    acpi_table_parse_madt(ACPI_MADT_TYPE_INTERRUPT_OVERRIDE, acpi_parse_int_src_ovr,
-				  NR_IRQ_VECTORS);
+				  NR_IRQS);
 	if (count < 0) {
 		printk(KERN_ERR PREFIX
 		       "Error parsing interrupt source overrides entry\n");
@@ -846,7 +846,7 @@ static int __init acpi_parse_madt_ioapic_entries(void)
 
 	count =
 	    acpi_table_parse_madt(ACPI_MADT_TYPE_NMI_SOURCE, acpi_parse_nmi_src,
-				  NR_IRQ_VECTORS);
+				  NR_IRQS);
 	if (count < 0) {
 		printk(KERN_ERR PREFIX "Error parsing NMI SRC entry\n");
 		/* TBD: Cleanup to allow fallback to MPS */
diff --git a/arch/i386/kernel/io_apic.c b/arch/i386/kernel/io_apic.c
index e3e5940..92cc604 100644
--- a/arch/i386/kernel/io_apic.c
+++ b/arch/i386/kernel/io_apic.c
@@ -1223,14 +1223,14 @@ static inline int IO_APIC_irq_trigger(int irq)
 }
 
 /* irq_vectors is indexed by the sum of all RTEs in all I/O APICs. */
-static u8 irq_vector[NR_IRQ_VECTORS] __read_mostly = { FIRST_DEVICE_VECTOR , 0 };
+static u8 irq_vector[NR_IRQS] __read_mostly = { FIRST_DEVICE_VECTOR , 0 };
 
 static int __assign_irq_vector(int irq)
 {
 	static int current_vector = FIRST_DEVICE_VECTOR, current_offset = 0;
 	int vector, offset, i;
 
-	BUG_ON((unsigned)irq >= NR_IRQ_VECTORS);
+	BUG_ON((unsigned)irq >= NR_IRQS);
 
 	if (irq_vector[irq] > 0)
 		return irq_vector[irq];
@@ -1247,7 +1247,7 @@ next:
 		return -ENOSPC;
 	if (vector == SYSCALL_VECTOR)
 		goto next;
-	for (i = 0; i < NR_IRQ_VECTORS; i++)
+	for (i = 0; i < NR_IRQS; i++)
 		if (irq_vector[i] == vector)
 			goto next;
 
diff --git a/include/asm-i386/mach-default/irq_vectors_limits.h b/include/asm-i386/mach-default/irq_vectors_limits.h
index 7f161e7..ad4e05c 100644
--- a/include/asm-i386/mach-default/irq_vectors_limits.h
+++ b/include/asm-i386/mach-default/irq_vectors_limits.h
@@ -2,15 +2,13 @@
 #define _ASM_IRQ_VECTORS_LIMITS_H
 
 #ifdef CONFIG_X86_IO_APIC
-#define NR_IRQS 224
-# if (224 >= 32 * NR_CPUS)
-# define NR_IRQ_VECTORS NR_IRQS
+# if (200 >= 32 * NR_CPUS)
+#  define NR_IRQS 200
 # else
-# define NR_IRQ_VECTORS (32 * NR_CPUS)
+#  define NR_IRQS (32 * NR_CPUS)
 # endif
 #else
 #define NR_IRQS 16
-#define NR_IRQ_VECTORS NR_IRQS
 #endif
 
 #endif /* _ASM_IRQ_VECTORS_LIMITS_H */
diff --git a/include/asm-i386/mach-generic/irq_vectors_limits.h b/include/asm-i386/mach-generic/irq_vectors_limits.h
index 890ce3f..f16f727 100644
--- a/include/asm-i386/mach-generic/irq_vectors_limits.h
+++ b/include/asm-i386/mach-generic/irq_vectors_limits.h
@@ -8,7 +8,6 @@
  * This value should be the same in both the generic and summit subarches.
  * Change one, change 'em both.
  */
-#define NR_IRQS	224
-#define NR_IRQ_VECTORS	1024
+#define NR_IRQS	1024
 
 #endif /* _ASM_IRQ_VECTORS_LIMITS_H */
diff --git a/include/asm-i386/mach-summit/irq_vectors_limits.h b/include/asm-i386/mach-summit/irq_vectors_limits.h
index 890ce3f..f16f727 100644
--- a/include/asm-i386/mach-summit/irq_vectors_limits.h
+++ b/include/asm-i386/mach-summit/irq_vectors_limits.h
@@ -8,7 +8,6 @@
  * This value should be the same in both the generic and summit subarches.
  * Change one, change 'em both.
  */
-#define NR_IRQS	224
-#define NR_IRQ_VECTORS	1024
+#define NR_IRQS	1024
 
 #endif /* _ASM_IRQ_VECTORS_LIMITS_H */
diff --git a/include/asm-i386/mach-visws/irq_vectors.h b/include/asm-i386/mach-visws/irq_vectors.h
index cb572d8..c32ebdf 100644
--- a/include/asm-i386/mach-visws/irq_vectors.h
+++ b/include/asm-i386/mach-visws/irq_vectors.h
@@ -51,7 +51,6 @@
  */
 #define NR_VECTORS 256
 #define NR_IRQS 224
-#define NR_IRQ_VECTORS NR_IRQS
 
 #define FPU_IRQ			13
 
diff --git a/include/asm-i386/mach-voyager/irq_vectors.h b/include/asm-i386/mach-voyager/irq_vectors.h
index 165421f..abaf5cf 100644
--- a/include/asm-i386/mach-voyager/irq_vectors.h
+++ b/include/asm-i386/mach-voyager/irq_vectors.h
@@ -57,7 +57,6 @@
 
 #define NR_VECTORS 256
 #define NR_IRQS 224
-#define NR_IRQ_VECTORS NR_IRQS
 
 #define FPU_IRQ				13
 
-- 
1.4.4.1.g278f

Re: [PATCH] i386 irq: Kill NR_IRQ_VECTORS and increase NR_IRQS

[Andi Kleen]

> Allowing the kernel to work on big machines without complicated
> and error prone irq remapping logic.

How much memory does this use by default? If it's much there should
be at least a CONFIG_* to make it smaller.

-Andi

Re: [PATCH] i386 irq: Kill NR_IRQ_VECTORS and increase NR_IRQS

[Eric W. Biederman]

Andi Kleen <ak@suse.de> writes:

>> Allowing the kernel to work on big machines without complicated
>> and error prone irq remapping logic.
>
> How much memory does this use by default? If it's much there should
> be at least a CONFIG_* to make it smaller.

If you don't build a generic kernel and are not a big way smp you only
get 200 IRQs so it is less.  

Otherwise it is either 1024 or NR_CPUS*32 depending on the subarch
we compile.  The limits are actually unchanged from what we are doing
today.

I'm in the final stages of figuring out how to kill the stupid
irq_desc array which should make all the memory consumption worries
go away but that is a little ways off, and I'm putting real bug
fixes ahead of that work.

Basically this is just a back port from x86_64 without the
complication of the per cpu vector logic.  So if we run into any
issues I missed before this reaches a stable kernel we can just copy
the fix from x86_64 where we have been doing this for a while.  

So in short the existing CONFIG_ options should give us the control
we need, and before it becomes any kind of an issue I intend to
completely remove the compile time limit and make it dynamic.

Eric