x86: Don't allow nr_irqs > NR_IRQS

On some systems probe_nr_irqs() can return a value larger than NR_IRQS. This will lead to probe_irq_on() overrunning the irq_desc array.

---
 arch/x86/kernel/io_apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)


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

I think this has become possible since:

commit d6c88a507ef0b6afdb013cba4e7804ba7324d99a

Author: Thomas Gleixner <[email protected]>
Date:   Wed Oct 15 15:27:23 2008 +0200

genirq: revert dynarray

Revert the dynarray changes. They need more thought and polishing.

Ben.

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

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.

[Ben Hutchings - Tue, Nov 04, 2008 at 05:18:22PM +0000]

| On some systems probe_nr_irqs() can return a value larger than
| NR_IRQS.  This will lead to probe_irq_on() overrunning the irq_desc
| array.
| 
| Signed-off-by: Ben Hutchings <[email protected]>
| ---
|  arch/x86/kernel/io_apic.c |    2 ++
|  1 files changed, 2 insertions(+), 0 deletions(-)
| 
| 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
| 
| I think this has become possible since:
| 
| commit d6c88a507ef0b6afdb013cba4e7804ba7324d99a
| Author: Thomas Gleixner <[email protected]>
| Date:   Wed Oct 15 15:27:23 2008 +0200
| 
|     genirq: revert dynarray
| 
|     Revert the dynarray changes. They need more thought and polishing.
| 
|     Signed-off-by: Thomas Gleixner <[email protected]>
| 
| Ben.
| 
| diff --git a/arch/x86/kernel/io_apic.c b/arch/x86/kernel/io_apic.c
| index b764d74..c8482fb 100644
| --- a/arch/x86/kernel/io_apic.c
| +++ b/arch/x86/kernel/io_apic.c
| @@ -3611,6 +3611,8 @@ int __init probe_nr_irqs(void)
|  	/* something wrong ? */
|  	if (nr < nr_min)
|  		nr = nr_min;
| +	if (nr > NR_IRQS)
| +		nr = NR_IRQS;
|  
|  	return nr;
|  }
|

Hi Ben,

I don't think that is because of Thomas' commit. If we've got number of pins larger then we expect it means something wrong with our NR_IRQS. Is it possible to get your .config?

- Cyrill -

On Tue, 2008-11-04 at 21:00 +0300, Cyrill Gorcunov wrote:
[...]

[...]

Well there must have been an earlier change that resulted in detecting 5 IOAPICs instead of just 1, but that presumably would work as long as the irq_desc array was dynamically allocated. This reversion breaks that.

You don't really need to see the config; NR_IRQS is *always* 224 on normal x86-32 systems.

Ben.

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.

Yes. it should the same as setting to 64bit. we will cost some extra ram...

We can not make sparseirq/dyn_array to 2.6.28..., maybe 2.6.29 later.

Ingo,
Maybe we can put back dyn_array at first, that will still be 1:1 mapping..., and not waste memory..

YH

[Ben Hutchings - Tue, Nov 04, 2008 at 06:36:47PM +0000]

| On Tue, 2008-11-04 at 21:00 +0300, Cyrill Gorcunov wrote:
| [...]
| > | 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
| > | 
| > | I think this has become possible since:
| > | 
| > | commit d6c88a507ef0b6afdb013cba4e7804ba7324d99a
| > | Author: Thomas Gleixner <[email protected]>
| > | Date:   Wed Oct 15 15:27:23 2008 +0200
| > | 
| > |     genirq: revert dynarray
| > | 
| > |     Revert the dynarray changes. They need more thought and polishing.
| > | 
| > |     Signed-off-by: Thomas Gleixner <[email protected]>
| [...]
| > Hi Ben,
| > 
| > I don't think that is because of Thomas' commit. If we've got
| > number of pins larger then we expect it means something wrong
| > with our NR_IRQS. Is it possible to get your .config?
| 
| Well there must have been an earlier change that resulted in detecting 5
| IOAPICs instead of just 1, but that presumably would work as long as the
| irq_desc array was dynamically allocated.  This reversion breaks that.

Hmm... wich means the problem in area of detecting IOAPICs but this number we got from ACPI... have to check.

| 
| You don't really need to see the config; NR_IRQS is *always* 224 on
| normal x86-32 systems.

Ben, how could I know that you're using 32bit version, unfortunately I don't recite in my mind "NR_IRQS is 224 on x86-32" every minute :)

| 
| Ben.
| 
| -- 
| 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.
| 

- Cyrill -

On Tue, 2008-11-04 at 22:46 +0300, Cyrill Gorcunov wrote:
[...]

It has 2 single-core processors (family 15 model 4). Here's the lspci output:

00:00.0 Host bridge: Intel Corporation E7520 Memory Controller Hub (rev 0c) 00:00.1 Class ff00: Intel Corporation E7525/E7520 Error Reporting Registers (rev 0c) 00:02.0 PCI bridge: Intel Corporation E7525/E7520/E7320 PCI Express Port A (rev 0c) 00:03.0 PCI bridge: Intel Corporation E7525/E7520/E7320 PCI Express Port A1 (rev 0c) 00:04.0 PCI bridge: Intel Corporation E7525/E7520 PCI Express Port B (rev 0c) 00:06.0 PCI bridge: Intel Corporation E7520 PCI Express Port C (rev 0c)
00:1d.0 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI Controller #1 (rev 02) 00:1d.1 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI Controller #2 (rev 02) 00:1d.2 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI Controller #3 (rev 02) 00:1d.3 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI Controller #4 (rev 02) 00:1d.7 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB2 EHCI Controller (rev 02) 00:1e.0 PCI bridge: Intel Corporation 82801 PCI Bridge (rev c2)
00:1f.0 ISA bridge: Intel Corporation 82801EB/ER (ICH5/ICH5R) LPC Interface Bridge (rev 02) 00:1f.1 IDE interface: Intel Corporation 82801EB/ER (ICH5/ICH5R) IDE Controller (rev 02) 00:1f.3 SMBus: Intel Corporation 82801EB/ER (ICH5/ICH5R) SMBus Controller (rev 02) 02:00.0 PCI bridge: Intel Corporation 6700PXH PCI Express-to-PCI Bridge A (rev 09) 02:00.1 PIC: Intel Corporation 6700/6702PXH I/OxAPIC Interrupt Controller A (rev 09) 02:00.2 PCI bridge: Intel Corporation 6700PXH PCI Express-to-PCI Bridge B (rev 09) 02:00.3 PIC: Intel Corporation 6700PXH I/OxAPIC Interrupt Controller B (rev 09) 04:02.0 Ethernet controller: Intel Corporation 82546GB Gigabit Ethernet Controller (rev 03) 04:02.1 Ethernet controller: Intel Corporation 82546GB Gigabit Ethernet Controller (rev 03) 05:00.0 PCI bridge: Intel Corporation 6700PXH PCI Express-to-PCI Bridge A (rev 09) 05:00.1 PIC: Intel Corporation 6700/6702PXH I/OxAPIC Interrupt Controller A (rev 09) 05:00.2 PCI bridge: Intel Corporation 6700PXH PCI Express-to-PCI Bridge B (rev 09) 05:00.3 PIC: Intel Corporation 6700PXH I/OxAPIC Interrupt Controller B (rev 09) 07:02.0 FireWire (IEEE 1394): Texas Instruments TSB43AB23 IEEE-1394a-2000 Controller (PHY/Link) 08:00.0 Ethernet controller: Solarflare Communications SFC4000 rev B [Solarstorm] (rev 02) 09:01.0 VGA compatible controller: ATI Technologies Inc Rage XL (rev 27)

I see 4 PICs there; no idea where the 5th is hiding.

I switched to an RHEL 5 (2.6.18-53) kernel and found that that also detects 5 IOAPICs, so there's no recent change in that.

This is the commit that made nr_irqs dynamic:

commit 71f521bbaf375b685aeea20c6b0ed8600cd6edfe

Author: Yinghai Lu <[email protected]>
Date:   Tue Aug 19 20:50:03 2008 -0700

x86, irq: get nr_irqs from madt

Until now, NR_IRQS was derived from black magic defines that had to be "large enough" to both accomodate NR_CPUS and MAX_NR_IO_APICs.

This resulted in a way too large irq_desc[] array on most x86 systems. Especially with larger CPU masks, the size of irq_desc can spiral out of control quickly.

So be smarter about it and use precise allocation instead: determine the default maximum possible IRQ number from the ACPI MADT. Use a minimum limit of at least 32 IRQs for broken BIOSes.

That was fine at the time because the irq_desc array was allocated dynamically. But now that irq_desc has been reverted to a static array, the probe function needs to apply the upper limit. Alternately nr_irqs could be set to NR_IRQS unconditionally; the probing for nr_irqs is fairly pointless if it isn't used for dynamic allocation.

Ben.

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.

On some systems probe_nr_irqs() can return a value larger than NR_IRQS. This will lead to probe_irq_on() overrunning the irq_desc array.

---
 arch/x86/kernel/io_apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)


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

I have added a WARN_ON() as suggested by Yinghai Lu.

Ben.

@@ -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;
 }

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.

[Ben Hutchings - Wed, Nov 05, 2008 at 12:04:46PM +0000]

| On some systems probe_nr_irqs() can return a value larger than
| NR_IRQS.  This will lead to probe_irq_on() overrunning the irq_desc
| array.
| 
| Signed-off-by: Ben Hutchings <[email protected]>
| ---
|  arch/x86/kernel/io_apic.c |    2 ++
|  1 files changed, 2 insertions(+), 0 deletions(-)
| 
| 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
| 
| I have added a WARN_ON() as suggested by Yinghai Lu.
| 
| Ben.
| 
| diff --git a/arch/x86/kernel/io_apic.c b/arch/x86/kernel/io_apic.c
| --- a/arch/x86/kernel/io_apic.c
| +++ b/arch/x86/kernel/io_apic.c
| @@ -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;
|  }
| 
| -- 
| 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.
| 

My Ack if you need it, Ingo CC'ed

---

Ben I didn't manage to get a look over ACPI related changes (ie to find the reason why the kernel is able to reveal 5 IO-APICs
now but this patch is needed anyway as Yinghai already mentioned and should solve your problem. FWIW, I don't see neither apics on my laptop by lspci except that ICH is notified so I suspect the ACPI parsing is right)

- Cyrill -

On Wed, 2008-11-05 at 22:11 +0300, Cyrill Gorcunov wrote:
[...]

[...]

I was mistaken; that *isn't* a recent change. A default RHEL 5 kernel (based on 2.6.18) also finds 5 IOAPICs.

Ben.

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.

[Ben Hutchings - Wed, Nov 05, 2008 at 07:15:31PM +0000]

| On Wed, 2008-11-05 at 22:11 +0300, Cyrill Gorcunov wrote:
| [...]
| > Ben I didn't manage to get a look over ACPI related changes
| > (ie to find the reason why the kernel is able to reveal 5 IO-APICs
| >  now
| [...]
| 
| I was mistaken; that *isn't* a recent change.  A default RHEL 5 kernel
| (based on 2.6.18) also finds 5 IOAPICs.
| 
| Ben.
|

So, these io-apics was there for a long time :) and recent dyn-irq tricks just bared the problem of NR_IRQS for which Yinghai already sent a patch. Lets wait until they're merged.

- Cyrill -

yes - it's simply a box with a lot of IO-APICs. That's not bad at all, it spreads out IRQ processing amongst devices.

Ingo

applied to tip/x86/urgent, thanks Ben!

Ingo