x86: make NR_IRQS on 32bit is same to 64bit

PATCH Applied Yinghai Lu 6 participants 15 messages Nov 4, 2008 – Nov 6, 2008 x86

lore

Share & tools

lore

Yinghai Lu

author

2008-11-04 22:12 UTC #0 lore raw

PATCH 1/1 x86: make NR_IRQS on 32bit is same to 64bit

Impact: so NR_IRQS is bigger enough for system with lots of apic/pins

Now: if IO_APIC is there, will have big NR_IRQS

otherwise still use 224

Signed-off-by Yinghai <[email protected]>

---
 arch/x86/include/asm/irq_vectors.h |   20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

arch/x86/include/asm/irq_vectors.h +6 ~~-14~~

Index: linux-2.6/arch/x86/include/asm/irq_vectors.h
===================================================================
@@ -101,30 +101,22 @@
 #define LAST_VM86_IRQ		15
 #define invalid_vm86_irq(irq)	((irq) < 3 || (irq) > 15)
 
-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_IO_APIC) && !defined(CONFIG_PARAVIRT) && !defined(CONFIG_X86_VISWS) && !defined(CONFIG_X86_VOYAGER)
 # if NR_CPUS < MAX_IO_APICS
 #  define NR_IRQS (NR_VECTORS + (32 * NR_CPUS))
 # else
 #  define NR_IRQS (NR_VECTORS + (32 * MAX_IO_APICS))
 # endif
 
-#elif !defined(CONFIG_X86_VOYAGER)
+#elif defined(CONFIG_PARAVIRT) || defined(CONFIG_X86_VISWS) || defined(CONFIG_X86_VOYAGER)
 
-# if defined(CONFIG_X86_IO_APIC) || defined(CONFIG_PARAVIRT) || defined(CONFIG_X86_VISWS)
-
-#  define NR_IRQS		224
-
-# else /* IO_APIC || PARAVIRT */
-
-#  define NR_IRQS		16
-
-# endif
+# define NR_IRQS		224
 
-#else /* !VISWS && !VOYAGER */
+#else /* IO_APIC || PARAVIRT */
 
-# define NR_IRQS		224
+# define NR_IRQS		16
 
-#endif /* VISWS */
+#endif
 
 /* Voyager specific defines */
 /* These define the CPIs we use in linux */

Ben Hutchings

2008-11-04 22:29 UTC #1 lore raw

Quoted 6 lines · Yinghai Lu

Impact: so NR_IRQS is bigger enough for system with lots of apic/pins
Now: if IO_APIC is there, will have big NR_IRQS
otherwise still use 224

[...]

I have no idea whether this is useful, but it doesn't solve the general problem that probe_nr_irqs() can return a value > NR_IRQS. So far as I can see it can return up to 2 * 24 * MAX_IO_APICS which may be greater than NR_VECTORS + (32 * max(MAX_IO_APICS, NR_CPUS)).

Ben.

Signature · 3 lines

Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Yinghai Lu

author

2008-11-04 22:36 UTC #2 lore raw

Quoted 5 lines · Ben Hutchings

Quoted 4 lines · Yinghai Lu
Impact: so NR_IRQS is bigger enough for system with lots of apic/pins
Now: if IO_APIC is there, will have big NR_IRQS

Quoted 7 lines

Quoted 1 line
otherwise still use 224
[...]
I have no idea whether this is useful, but it doesn't solve the general problem that probe_nr_irqs() can return a value > NR_IRQS. So far as I can see it can return up to 2 * 24 * MAX_IO_APICS which may be greater than NR_VECTORS + (32 * max(MAX_IO_APICS, NR_CPUS)).

your patch is still needed...
also please add one WARN_ON when nr > NR_IRQS.

Ingo Molnar

Tested-by Ben Hutchings Acked-by Yinghai Lu on v1 1/1 Applied

2008-11-06 06:26 UTC #3 lore raw

In reply to Yinghai Lu

Quoted 7 lines · Yinghai Lu

Impact: so NR_IRQS is bigger enough for system with lots of apic/pins
Now: if IO_APIC is there, will have big NR_IRQS
otherwise still use 224
Signed-off-by Yinghai <[email protected]>

applied to tip/x86/urgent, thanks Yinghai! Below are the final form of the two commit.

Clean-up sidenote: i think we can now remove the VISWS #ifdef portion for good? Mind sending a patch for that too?

Ingo

---------------->
commit 1b4897688011cd05e07f00dcfe6af3331eb36a3c

Author: Yinghai Lu <[email protected]>
Date:   Tue Nov 4 14:10:13 2008 -0800

x86: size NR_IRQS on 32-bit systems the same way as 64-bit

Impact: make NR_IRQS big enough for system with lots of apic/pins

If lots of IO_APIC's are there (or can be there), size the same way as 64-bit, depending on MAX_IO_APICS and NR_CPUS.

This fixes the boot problem reported by Ben Hutchings on a 32-bit server with 5 IO-APICs and 240 IO-APIC pins.

Signed-off-by Yinghai <[email protected]>

Tested-by Ben Hutchings <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>

arch/x86/include/asm/irq_vectors.h +6 ~~-14~~

index d843ed0..503aadc 100644
@@ -101,30 +101,22 @@
 #define LAST_VM86_IRQ		15
 #define invalid_vm86_irq(irq)	((irq) < 3 || (irq) > 15)
 
-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_IO_APIC) && !defined(CONFIG_PARAVIRT) && !defined(CONFIG_X86_VISWS) && !defined(CONFIG_X86_VOYAGER)
 # if NR_CPUS < MAX_IO_APICS
 #  define NR_IRQS (NR_VECTORS + (32 * NR_CPUS))
 # else
 #  define NR_IRQS (NR_VECTORS + (32 * MAX_IO_APICS))
 # endif
 
-#elif !defined(CONFIG_X86_VOYAGER)
+#elif defined(CONFIG_PARAVIRT) || defined(CONFIG_X86_VISWS) || defined(CONFIG_X86_VOYAGER)
 
-# if defined(CONFIG_X86_IO_APIC) || defined(CONFIG_PARAVIRT) || defined(CONFIG_X86_VISWS)
-
-#  define NR_IRQS		224
-
-# else /* IO_APIC || PARAVIRT */
-
-#  define NR_IRQS		16
-
-# endif
+# define NR_IRQS		224
 
-#else /* !VISWS && !VOYAGER */
+#else /* IO_APIC || PARAVIRT */
 
-# define NR_IRQS		224
+# define NR_IRQS		16
 
-#endif /* VISWS */
+#endif
 
 /* Voyager specific defines */
 /* These define the CPIs we use in linux */

commit c78d0cf2925bffae8a6f00e7d9b8e971b0392edd

Author: Ben Hutchings <[email protected]>
Date:   Wed Nov 5 12:04:46 2008 +0000

x86: don't allow nr_irqs > NR_IRQS

Impact: fix boot hang on 32-bit systems with more than 224 IO-APIC pins

On some 32-bit systems with a lot of IO-APICs probe_nr_irqs() can return a value larger than NR_IRQS. This will lead to probe_irq_on() overrunning the irq_desc array.

I hit this when running net-next-2.6 (close to 2.6.28-rc3) on a Supermicro dual Xeon system. NR_IRQS is 224 but probe_nr_irqs() detects 5 IOAPICs and returns 240. Here are the log messages:

Tue Nov 4 16:53:47 2008 ACPI: IOAPIC (id[0x01] address[0xfec00000] gsi_base[0]) Tue Nov 4 16:53:47 2008 IOAPIC[0]: apic_id 1, version 32, address 0xfec00000, GSI 0-23 Tue Nov 4 16:53:47 2008 ACPI: IOAPIC (id[0x02] address[0xfec81000] gsi_base[24]) Tue Nov 4 16:53:47 2008 IOAPIC[1]: apic_id 2, version 32, address 0xfec81000, GSI 24-47 Tue Nov 4 16:53:47 2008 ACPI: IOAPIC (id[0x03] address[0xfec81400] gsi_base[48]) Tue Nov 4 16:53:47 2008 IOAPIC[2]: apic_id 3, version 32, address 0xfec81400, GSI 48-71 Tue Nov 4 16:53:47 2008 ACPI: IOAPIC (id[0x04] address[0xfec82000] gsi_base[72]) Tue Nov 4 16:53:47 2008 IOAPIC[3]: apic_id 4, version 32, address 0xfec82000, GSI 72-95 Tue Nov 4 16:53:47 2008 ACPI: IOAPIC (id[0x05] address[0xfec82400] gsi_base[96]) Tue Nov 4 16:53:47 2008 IOAPIC[4]: apic_id 5, version 32, address 0xfec82400, GSI 96-119 Tue Nov 4 16:53:47 2008 ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 high edge) Tue Nov 4 16:53:47 2008 ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level) Tue Nov 4 16:53:47 2008 Enabling APIC mode: Flat. Using 5 I/O APICs

Signed-off-by Ben Hutchings <[email protected]>

Acked-by Yinghai Lu <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>

arch/x86/kernel/io_apic.c +2 -0

index b764d74..7a3f202 100644
@@ -3611,6 +3611,8 @@ int __init probe_nr_irqs(void)
 	/* something wrong ? */
 	if (nr < nr_min)
 		nr = nr_min;
+	if (WARN_ON(nr > NR_IRQS))
+		nr = NR_IRQS;
 
 	return nr;
 }

Yinghai Lu

author

2008-11-06 06:43 UTC #4 lore raw

Quoted 9 lines · Ingo Molnar

Quoted 6 lines · Yinghai Lu
Impact: so NR_IRQS is bigger enough for system with lots of apic/pins
Now: if IO_APIC is there, will have big NR_IRQS
otherwise still use 224

Quoted 7 lines

Quoted 1 line
Signed-off-by Yinghai <[email protected]>
applied to tip/x86/urgent, thanks Yinghai! Below are the final form of the two commit.
Clean-up sidenote: i think we can now remove the VISWS #ifdef portion for good? Mind sending a patch for that too?

could remove CONFIG_PARAVIRT too.. Jeremy?

H. Peter Anvin

reviewer

2008-11-06 06:56 UTC #5 lore raw

Quoted 6 lines · Yinghai Lu

Quoted 3 lines
Clean-up sidenote: i think we can now remove the VISWS #ifdef portion for good? Mind sending a patch for that too?
could remove CONFIG_PARAVIRT too.. Jeremy?

This seems like a bad idea, especially for the embedded guys.

-hpa

Yinghai Lu

author

2008-11-06 07:00 UTC #6 lore raw

Quoted 9 lines · H. Peter Anvin

Quoted 6 lines · Yinghai Lu
Quoted 3 lines
Clean-up sidenote: i think we can now remove the VISWS #ifdef portion for good? Mind sending a patch for that too?
could remove CONFIG_PARAVIRT too.. Jeremy?
This seems like a bad idea, especially for the embedded guys.

make one CONFIG_NR_IRQS for them before working out one dyna_array is get merged?

H. Peter Anvin

reviewer

2008-11-06 07:07 UTC #7 lore raw

Quoted 11 lines · Yinghai Lu

Quoted 6 lines · H. Peter Anvin
Quoted 4 lines · Yinghai Lu
Quoted 2 lines
Clean-up sidenote: i think we can now remove the VISWS #ifdef portion for good? Mind sending a patch for that too?
could remove CONFIG_PARAVIRT too.. Jeremy?
This seems like a bad idea, especially for the embedded guys.
make one CONFIG_NR_IRQS for them before working out one dyna_array is get merged?

Ah, sorry... I missed the context and thought you meant removing CONFIG_PARAVIRT in general.

-hpa

Yinghai Lu

author

2008-11-06 07:10 UTC #8 lore raw

Quoted 11 lines · H. Peter Anvin

Quoted 10 lines · Yinghai Lu
Quoted 8 lines · H. Peter Anvin
Quoted 6 lines · Yinghai Lu
Quoted 3 lines
Clean-up sidenote: i think we can now remove the VISWS #ifdef portion for good? Mind sending a patch for that too?
could remove CONFIG_PARAVIRT too.. Jeremy?

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
This seems like a bad idea, especially for the embedded guys.
make one CONFIG_NR_IRQS for them before working out one dyna_array is get merged?
Ah, sorry... I missed the context and thought you meant removing CONFIG_PARAVIRT in general.

I mean make NR_IRQS bigger like IO_APIC is set when CONFIG_PARAVIRT is set.

Yinghai Lu

2008-11-06 07:28 UTC #9 lore raw

Quoted 14 lines · Yinghai Lu

Quoted 13 lines · H. Peter Anvin
Quoted 12 lines · Yinghai Lu
Quoted 9 lines · H. Peter Anvin
Quoted 6 lines · Yinghai Lu
Quoted 3 lines
Clean-up sidenote: i think we can now remove the VISWS #ifdef portion for good? Mind sending a patch for that too?
could remove CONFIG_PARAVIRT too.. Jeremy?
This seems like a bad idea, especially for the embedded guys.

Quoted 9 lines

Quoted 6 lines
Quoted 3 lines
make one CONFIG_NR_IRQS for them before working out one dyna_array is get merged?
Ah, sorry... I missed the context and thought you meant removing CONFIG_PARAVIRT in general.
I mean make NR_IRQS bigger like IO_APIC is set when CONFIG_PARAVIRT is set.

(quoted diff) +0 -0

b/arch/x86/include/asm/irq_vectors.h
index 503aadc..0005adb 100644

arch/x86/include/asm/irq_vectors.h +0 -1

@@ -101,18 +101,18 @@
 #define LAST_VM86_IRQ          15
 #define invalid_vm86_irq(irq)  ((irq) < 3 || (irq) > 15)
 
-#if defined(CONFIG_X86_IO_APIC) && !defined(CONFIG_PARAVIRT) &&

!defined(CONFIG_X86_VISWS) && !defined(CONFIG_X86_VOYAGER) +#if defined(CONFIG_X86_IO_APIC) && !defined(CONFIG_X86_VOYAGER)

 # if NR_CPUS < MAX_IO_APICS
 #  define NR_IRQS (NR_VECTORS + (32 * NR_CPUS))
 # else
 #  define NR_IRQS (NR_VECTORS + (32 * MAX_IO_APICS))
 # endif

-#elif defined(CONFIG_PARAVIRT) || defined(CONFIG_X86_VISWS) || defined(CONFIG_X86_VOYAGER)
+#elif defined(CONFIG_X86_VOYAGER)

 # define NR_IRQS               224

-#else /* IO_APIC || PARAVIRT */
+#else /* IO_APIC || VOYAGER */

 # define NR_IRQS               16

Yinghai Lu

author

2008-11-06 07:37 UTC #10 lore raw

In reply to H. Peter Anvin

Quoted 7 lines · H. Peter Anvin

Quoted 6 lines · Yinghai Lu
Quoted 5 lines · H. Peter Anvin
Quoted 4 lines · Yinghai Lu
Quoted 2 lines
Clean-up sidenote: i think we can now remove the VISWS #ifdef portion for good? Mind sending a patch for that too?
could remove CONFIG_PARAVIRT too.. Jeremy?

Quoted 9 lines

Quoted 5 lines
Quoted 1 line
This seems like a bad idea, especially for the embedded guys.
make one CONFIG_NR_IRQS for them before working out one dyna_array is get merged?
Ah, sorry... I missed the context and thought you meant removing CONFIG_PARAVIRT in general.

---
[PATCH] x86: remove VISWS and PARAVIRT around NR_IRQS puzzle

Impact: fix warn when PARAVIRT is set in config

Signed-off-by Yinghai Lu <[email protected]>

arch/x86/include/asm/irq_vectors.h +3 -3

index 503aadc..0005adb 100644
@@ -101,18 +101,18 @@
 #define LAST_VM86_IRQ		15
 #define invalid_vm86_irq(irq)	((irq) < 3 || (irq) > 15)
 
-#if defined(CONFIG_X86_IO_APIC) && !defined(CONFIG_PARAVIRT) && !defined(CONFIG_X86_VISWS) && !defined(CONFIG_X86_VOYAGER)
+#if defined(CONFIG_X86_IO_APIC) && !defined(CONFIG_X86_VOYAGER)
 # if NR_CPUS < MAX_IO_APICS
 #  define NR_IRQS (NR_VECTORS + (32 * NR_CPUS))
 # else
 #  define NR_IRQS (NR_VECTORS + (32 * MAX_IO_APICS))
 # endif
 
-#elif defined(CONFIG_PARAVIRT) || defined(CONFIG_X86_VISWS) || defined(CONFIG_X86_VOYAGER)
+#elif defined(CONFIG_X86_VOYAGER)
 
 # define NR_IRQS		224
 
-#else /* IO_APIC || PARAVIRT */
+#else /* IO_APIC || VOYAGER */
 
 # define NR_IRQS		16

Ingo Molnar

Applied

2008-11-06 08:36 UTC #11 lore raw

Quoted 5 lines · Yinghai Lu

[PATCH] x86: remove VISWS and PARAVIRT around NR_IRQS puzzle
Impact: fix warn when PARAVIRT is set in config
Signed-off-by Yinghai Lu <[email protected]>

applied to tip/x86/urgent, thanks Yinghai!

Ingo

Matt Mackall

2008-11-06 22:01 UTC #12 lore raw

In reply to H. Peter Anvin

Quoted 9 lines · H. Peter Anvin

Quoted 6 lines · Yinghai Lu
Quoted 3 lines
Clean-up sidenote: i think we can now remove the VISWS #ifdef portion for good? Mind sending a patch for that too?
could remove CONFIG_PARAVIRT too.. Jeremy?
This seems like a bad idea, especially for the embedded guys.

I'd actually really like to see us move to dynamically-allocated irq tables as they're currently huge. But I haven't dug into it deeply enough to see what sort of gotchas are involved.

Signature · 1 line

Mathematics is the supreme nostalgia of our time.

Ingo Molnar

2008-11-06 22:24 UTC #13 lore raw

Quoted 6 lines · Matt Mackall

Quoted 5 lines · H. Peter Anvin
Quoted 4 lines · Yinghai Lu
Quoted 3 lines
Clean-up sidenote: i think we can now remove the VISWS #ifdef portion for good? Mind sending a patch for that too?

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
could remove CONFIG_PARAVIRT too.. Jeremy?
This seems like a bad idea, especially for the embedded guys.
I'd actually really like to see us move to dynamically-allocated irq tables as they're currently huge. But I haven't dug into it deeply enough to see what sort of gotchas are involved.

sparseirq (== dynamic IRQ tables) is all implemented already, and most of the details except the final feature-enabler are upstream already.

Ingo

Matt Mackall

2008-11-06 22:37 UTC #14 lore raw

Quoted 11 lines · Ingo Molnar

Quoted 9 lines · Matt Mackall
Quoted 8 lines · H. Peter Anvin
Quoted 6 lines · Yinghai Lu
Quoted 3 lines
Clean-up sidenote: i think we can now remove the VISWS #ifdef portion for good? Mind sending a patch for that too?
could remove CONFIG_PARAVIRT too.. Jeremy?

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
This seems like a bad idea, especially for the embedded guys.
I'd actually really like to see us move to dynamically-allocated irq tables as they're currently huge. But I haven't dug into it deeply enough to see what sort of gotchas are involved.
sparseirq (== dynamic IRQ tables) is all implemented already, and most of the details except the final feature-enabler are upstream already.

Thanks for the pointer, I'd missed this going by.

Signature · 1 line

Mathematics is the supreme nostalgia of our time.