[stable 2.6.32..2.6.34] x86, ioapic: initialize nr_ioapic_registers early in mp_register_ioapic()

[stable 2.6.32..2.6.34] x86, ioapic: initialize nr_ioapic_registers early in mp_register_ioapic()

[Suresh Siddha]

Lin Bao reported that one of the HP platforms failed to boot
2.6.32 kernel, when the BIOS enabled interrupt-remapping and
x2apic before handing over the control to the Linux kernel.

During boot, Linux kernel masks all the interrupt sources
(8259, IO-APIC RTE's), setup the interrupt-remapping hardware
with the OS controlled table and unmasks the 8259 interrupts
but not the IO-APIC RTE's (as the newly setup interrupt-remapping
table and the IO-APIC RTE's are not yet programmed by the kernel).

Shortly after this, IO-APIC RTE's and the interrupt-remapping table
entries are programmed based on the ACPI tables etc. So the
expectation is that any interrupt during this window will be dropped
and not see the intermediate configuration.

In the reported problematic case, BIOS has configured the IO-APIC
in virtual wire-B mode. Between the window of the kernel setting up
new interrupt-remapping table  and the IO-APIC RTE's are properly
configured, an interrupt gets routed by the IO-APIC RTE (setup
by the virtual wire-B configuration) and sees the empty
interrupt-remapping table entry, resulting in vt-d fault causing
the platform to generate NMI. And the OS panics on this unexpected NMI.

This problem doesn't happen with more recent kernels and closer
look at the 2.6.32 kernel shows that the code which masks
the IO-APIC RTE's is not working as expected as the nr_ioapic_registers
for each IO-APIC is not yet initialized at this point. In the later
kernels we initialize nr_ioapic_registers much before and
everything works as expected.

For 2.6.[32..34] kernels, fix this issue by initializing
nr_ioapic_registers early in mp_register_ioapic()

Relevant upstream commit info:

commit 7716a5c4ff5f1f3dc5e9edcab125cbf7fceef0af
Author: Eric W. Biederman <ebiederm@xmission.com>
Date:   Tue Mar 30 01:07:12 2010 -0700

    x86, ioapic: Move nr_ioapic_registers calculation to mp_register_ioapic.

Reported-and-tested-by: Zhang, Lin-Bao <linbao.zhang@hp.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: stable@vger.kernel.org [v2.6.32..v2.6.34]
---
 arch/x86/kernel/apic/io_apic.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 8928d97..d256bc3 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -4262,6 +4262,7 @@ static int bad_ioapic(unsigned long address)
 void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 {
 	int idx = 0;
+	int entries;
 
 	if (bad_ioapic(address))
 		return;
@@ -4280,10 +4281,14 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 	 * Build basic GSI lookup table to facilitate gsi->io_apic lookups
 	 * and to prevent reprogramming of IOAPIC pins (PCI GSIs).
 	 */
+	entries = io_apic_get_redir_entries(idx);
 	mp_gsi_routing[idx].gsi_base = gsi_base;
-	mp_gsi_routing[idx].gsi_end = gsi_base +
-	    io_apic_get_redir_entries(idx);
+	mp_gsi_routing[idx].gsi_end = gsi_base + entries;
 
+	/*
+	 * The number of IO-APIC IRQ registers (== #pins):
+	 */
+	nr_ioapic_registers[idx] = entries + 1;
 	printk(KERN_INFO "IOAPIC[%d]: apic_id %d, version %d, address 0x%x, "
 	       "GSI %d-%d\n", idx, mp_ioapics[idx].apicid,
 	       mp_ioapics[idx].apicver, mp_ioapics[idx].apicaddr,

Re: [stable 2.6.32..2.6.34] x86, ioapic: initialize nr_ioapic_registers early in mp_register_ioapic()

[Jonathan Nieder]

Hi Suresh,

Suresh Siddha wrote:

[...]
> This problem doesn't happen with more recent kernels and closer
> look at the 2.6.32 kernel shows that the code which masks
> the IO-APIC RTE's is not working as expected as the nr_ioapic_registers
> for each IO-APIC is not yet initialized at this point. In the later
> kernels we initialize nr_ioapic_registers much before and
> everything works as expected.
>
> For 2.6.[32..34] kernels, fix this issue by initializing
> nr_ioapic_registers early in mp_register_ioapic()
>
> Relevant upstream commit info:
>
> commit 7716a5c4ff5f1f3dc5e9edcab125cbf7fceef0af

Why not cherry-pick 7716a5c4ff5 in full?

Curious,
Jonathan

Re: [stable 2.6.32..2.6.34] x86, ioapic: initialize nr_ioapic_registers early in mp_register_ioapic()

[Suresh Siddha]

On Wed, 2012-10-24 at 11:25 -0700, Jonathan Nieder wrote:
> Hi Suresh,
> 
> Suresh Siddha wrote:
> 
> [...]
> > This problem doesn't happen with more recent kernels and closer
> > look at the 2.6.32 kernel shows that the code which masks
> > the IO-APIC RTE's is not working as expected as the nr_ioapic_registers
> > for each IO-APIC is not yet initialized at this point. In the later
> > kernels we initialize nr_ioapic_registers much before and
> > everything works as expected.
> >
> > For 2.6.[32..34] kernels, fix this issue by initializing
> > nr_ioapic_registers early in mp_register_ioapic()
> >
> > Relevant upstream commit info:
> >
> > commit 7716a5c4ff5f1f3dc5e9edcab125cbf7fceef0af
> 
> Why not cherry-pick 7716a5c4ff5 in full?

As that depends on the other commits like:
commit 4b6b19a1c7302477653d799a53d48063dd53d555
Author: Eric W. Biederman <ebiederm@xmission.com>
Date:   Tue Mar 30 01:07:08 2010 -0700

Wanted to keep the changes as minimal as possible.

thanks,
suresh

Re: [stable 2.6.32..2.6.34] x86, ioapic: initialize nr_ioapic_registers early in mp_register_ioapic()

[Jonathan Nieder]

Suresh Siddha wrote:

> commit 4b6b19a1c7302477653d799a53d48063dd53d555

Oh, yuck.  If that commit had changed io_apic_get_redir_entries()'s
name, it would have avoided some trouble.  Thanks for pointing it out.

> Wanted to keep the changes as minimal as possible.

I don't see how this is conceptually simpler than a version that also
removes the nr_ioapic_registers calculation in enable_IO_APIC().

Either way, please do mention 4b6b19a1c730 in the commit message so
others looking at the patch can see the off-by-one that is being
narrowly avoided.  Thanks for your help and patience.

Jonathan

[RFC/PATCH 2.6.32.y 0/3] Re: [stable 2.6.32..2.6.34] x86, ioapic: initialize nr_ioapic_registers early in mp_register_ioapic()

[Jonathan Nieder]

Suresh Siddha wrote:
> On Wed, 2012-10-24 at 11:25 -0700, Jonathan Nieder wrote:

>> Why not cherry-pick 7716a5c4ff5 in full?
>
> As that depends on the other commits like:
> commit 4b6b19a1c7302477653d799a53d48063dd53d555

More importantly, if I understand correctly it might depend on

 commit cf7500c0ea13
 Author: Eric W. Biederman <ebiederm@xmission.com>
 Date:   Tue Mar 30 01:07:11 2010 -0700

     x86, ioapic: In mpparse use mp_register_ioapic

Here's a series, completely untested, that is closer to what I
expected.  But the approach you took seems reasonable, too, as long
as the commit message is tweaked to explain it.

Thanks again,
Jonathan

Eric W. Biederman (3):
  x86, ioapic: Teach mp_register_ioapic to compute a global gsi_end
  x86, ioapic: In mpparse use mp_register_ioapic
  x86, ioapic: Move nr_ioapic_registers calculation to
    mp_register_ioapic.

 arch/x86/include/asm/io_apic.h |  1 +
 arch/x86/kernel/apic/io_apic.c | 28 ++++++++++++++--------------
 arch/x86/kernel/mpparse.c      | 25 +------------------------
 arch/x86/kernel/sfi.c          |  4 +---
 4 files changed, 17 insertions(+), 41 deletions(-)

[PATCH 1/3] x86, ioapic: Teach mp_register_ioapic to compute a global gsi_end

[Jonathan Nieder]

From: "Eric W. Biederman" <ebiederm@xmission.com>
Date: Tue, 30 Mar 2010 01:07:10 -0700

commit 5777372af5c929b8f3c706ed7b295b7279537c88 upstream.

Add the global variable gsi_end and teach mp_register_ioapic
to keep it uptodate as we add more ioapics into the system.

ioapics can only be added early in boot so the code that
runs later can treat gsi_end as a constant.

Remove the have hacks in sfi.c to second guess mp_register_ioapic
by keeping t's own running total of how many gsi's have been seen,
and instead use the gsi_end.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
LKML-Reference: <1269936436-7039-9-git-send-email-ebiederm@xmission.com>
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 arch/x86/include/asm/io_apic.h | 1 +
 arch/x86/kernel/apic/io_apic.c | 6 ++++++
 arch/x86/kernel/sfi.c          | 4 +---
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 5f61f6e0ffdd..c145253f34fa 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -186,6 +186,7 @@ struct mp_ioapic_gsi{
 	int gsi_end;
 };
 extern struct mp_ioapic_gsi  mp_gsi_routing[];
+extern u32 gsi_end;
 int mp_find_ioapic(int gsi);
 int mp_find_ioapic_pin(int ioapic, int gsi);
 void __init mp_register_ioapic(int id, u32 address, u32 gsi_base);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 8928d9785eb4..7a5087640599 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -90,6 +90,9 @@ int nr_ioapics;
 /* IO APIC gsi routing info */
 struct mp_ioapic_gsi  mp_gsi_routing[MAX_IO_APICS];
 
+/* The last gsi number used */
+u32 gsi_end;
+
 /* MP IRQ source entries */
 struct mpc_intsrc mp_irqs[MAX_IRQ_SOURCES];
 
@@ -4284,6 +4287,9 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 	mp_gsi_routing[idx].gsi_end = gsi_base +
 	    io_apic_get_redir_entries(idx);
 
+	if (mp_gsi_routing[idx].gsi_end > gsi_end)
+		gsi_end = mp_gsi_routing[idx].gsi_end;
+
 	printk(KERN_INFO "IOAPIC[%d]: apic_id %d, version %d, address 0x%x, "
 	       "GSI %d-%d\n", idx, mp_ioapics[idx].apicid,
 	       mp_ioapics[idx].apicver, mp_ioapics[idx].apicaddr,
diff --git a/arch/x86/kernel/sfi.c b/arch/x86/kernel/sfi.c
index 34e099382651..7ded57896c0a 100644
--- a/arch/x86/kernel/sfi.c
+++ b/arch/x86/kernel/sfi.c
@@ -81,7 +81,6 @@ static int __init sfi_parse_cpus(struct sfi_table_header *table)
 #endif /* CONFIG_X86_LOCAL_APIC */
 
 #ifdef CONFIG_X86_IO_APIC
-static u32 gsi_base;
 
 static int __init sfi_parse_ioapic(struct sfi_table_header *table)
 {
@@ -94,8 +93,7 @@ static int __init sfi_parse_ioapic(struct sfi_table_header *table)
 	pentry = (struct sfi_apic_table_entry *)sb->pentry;
 
 	for (i = 0; i < num; i++) {
-		mp_register_ioapic(i, pentry->phys_addr, gsi_base);
-		gsi_base += io_apic_get_redir_entries(i);
+		mp_register_ioapic(i, pentry->phys_addr, gsi_end + 1);
 		pentry++;
 	}
 
-- 
1.8.0

[PATCH 2/3] x86, ioapic: In mpparse use mp_register_ioapic

[Jonathan Nieder]

From: "Eric W. Biederman" <ebiederm@xmission.com>
Date: Tue, 30 Mar 2010 01:07:11 -0700

commit cf7500c0ea133d66f8449d86392d83f84010263 upstream.

Long ago MP_ioapic_info was the primary way of setting up our
ioapic data structures and mp_register_ioapic was a compatibility
shim for acpi code.  Now the situation is reversed and
and mp_register_ioapic is the primary way of setting up our
ioapic data structures.

Keep the setting up of ioapic data structures uniform by
having mp_register_ioapic call mp_register_ioapic.

This changes a few fields:

- type: is now hardset to MP_IOAPIC but type had to
  bey MP_IOAPIC or MP_ioapic_info would not have been called.

- flags: is now hard coded to MPC_APIC_USABLE.
  We require flags to contain at least MPC_APIC_USEBLE in
  MP_ioapic_info and we don't ever examine flags so dropping
  a few flags that might possibly exist that we have never
  used is harmless.

- apicaddr: Unchanged

- apicver: Read from the ioapic instead of using the cached
  hardware value in the MP table.  The real hardware value
  will be more accurate.

- apicid: Now verified to be unique and changed if it is not.
  If the BIOS got this right this is a noop.  If the BIOS did
  not fixing things appears to be the better solution.

This adds gsi_base and gsi_end values to our ioapics defined with
the mpatable, which will make our lives simpler later since
we can always assume gsi_base and gsi_end are valid.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
LKML-Reference: <1269936436-7039-10-git-send-email-ebiederm@xmission.com>
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 arch/x86/kernel/mpparse.c | 25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/arch/x86/kernel/mpparse.c b/arch/x86/kernel/mpparse.c
index 788d6cee0ab8..0d895d5d3031 100644
--- a/arch/x86/kernel/mpparse.c
+++ b/arch/x86/kernel/mpparse.c
@@ -115,21 +115,6 @@ static void __init MP_bus_info(struct mpc_bus *m)
 		printk(KERN_WARNING "Unknown bustype %s - ignoring\n", str);
 }
 
-static int bad_ioapic(unsigned long address)
-{
-	if (nr_ioapics >= MAX_IO_APICS) {
-		printk(KERN_ERR "ERROR: Max # of I/O APICs (%d) exceeded "
-		       "(found %d)\n", MAX_IO_APICS, nr_ioapics);
-		panic("Recompile kernel with bigger MAX_IO_APICS!\n");
-	}
-	if (!address) {
-		printk(KERN_ERR "WARNING: Bogus (zero) I/O APIC address"
-		       " found in table, skipping!\n");
-		return 1;
-	}
-	return 0;
-}
-
 static void __init MP_ioapic_info(struct mpc_ioapic *m)
 {
 	if (!(m->flags & MPC_APIC_USABLE))
@@ -138,15 +123,7 @@ static void __init MP_ioapic_info(struct mpc_ioapic *m)
 	printk(KERN_INFO "I/O APIC #%d Version %d at 0x%X.\n",
 	       m->apicid, m->apicver, m->apicaddr);
 
-	if (bad_ioapic(m->apicaddr))
-		return;
-
-	mp_ioapics[nr_ioapics].apicaddr = m->apicaddr;
-	mp_ioapics[nr_ioapics].apicid = m->apicid;
-	mp_ioapics[nr_ioapics].type = m->type;
-	mp_ioapics[nr_ioapics].apicver = m->apicver;
-	mp_ioapics[nr_ioapics].flags = m->flags;
-	nr_ioapics++;
+	mp_register_ioapic(m->apicid, m->apicaddr, gsi_end + 1);
 }
 
 static void print_MP_intsrc_info(struct mpc_intsrc *m)
-- 
1.8.0

[PATCH 3/3] x86, ioapic: Move nr_ioapic_registers calculation to mp_register_ioapic.

[Jonathan Nieder]

From: "Eric W. Biederman" <ebiederm@xmission.com>
Date: Tue, 30 Mar 2010 01:07:12 -0700

commit 7716a5c4ff5f1f3dc5e9edcab125cbf7fceef0af upstream.

Now that all ioapic registration happens in mp_register_ioapic we can
move the calculation of nr_ioapic_registers there from enable_IO_APIC.
The number of ioapic registers is already calucated in mp_register_ioapic
so all that really needs to be done is to save the caluclated value
in nr_ioapic_registers.

[suresh.b.siddha@intel.com: backport to 2.6.32.y and 2.6.34.y:

 Lin Bao reported that one of the HP platforms failed to boot
 2.6.32 kernel, when the BIOS enabled interrupt-remapping and
 x2apic before handing over the control to the Linux kernel.

 During boot, Linux kernel masks all the interrupt sources
 (8259, IO-APIC RTE's), setup the interrupt-remapping hardware
 with the OS controlled table and unmasks the 8259 interrupts
 but not the IO-APIC RTE's (as the newly setup interrupt-remapping
 table and the IO-APIC RTE's are not yet programmed by the kernel).

 Shortly after this, IO-APIC RTE's and the interrupt-remapping table
 entries are programmed based on the ACPI tables etc. So the
 expectation is that any interrupt during this window will be dropped
 and not see the intermediate configuration.

 In the reported problematic case, BIOS has configured the IO-APIC
 in virtual wire-B mode. Between the window of the kernel setting up
 new interrupt-remapping table  and the IO-APIC RTE's are properly
 configured, an interrupt gets routed by the IO-APIC RTE (setup
 by the virtual wire-B configuration) and sees the empty
 interrupt-remapping table entry, resulting in vt-d fault causing
 the platform to generate NMI. And the OS panics on this unexpected NMI.

 This problem doesn't happen with more recent kernels and closer
 look at the 2.6.32 kernel shows that the code which masks
 the IO-APIC RTE's is not working as expected as the nr_ioapic_registers
 for each IO-APIC is not yet initialized at this point. In the later
 kernels we initialize nr_ioapic_registers much before and
 everything works as expected.]

[jrnieder@gmail.com: include removal of nr_ioapic_registers calculation
 from enable_IO_APIC() to complete the backport]

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
LKML-Reference: <1269936436-7039-11-git-send-email-ebiederm@xmission.com>
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
Reported-by: Zhang, Lin-Bao <linbao.zhang@hp.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 arch/x86/kernel/apic/io_apic.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 7a5087640599..6b7d97f1f5e5 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1945,20 +1945,8 @@ static struct { int pin, apic; } ioapic_i8259 = { -1, -1 };
 
 void __init enable_IO_APIC(void)
 {
-	union IO_APIC_reg_01 reg_01;
 	int i8259_apic, i8259_pin;
 	int apic;
-	unsigned long flags;
-
-	/*
-	 * The number of IO-APIC IRQ registers (== #pins):
-	 */
-	for (apic = 0; apic < nr_ioapics; apic++) {
-		spin_lock_irqsave(&ioapic_lock, flags);
-		reg_01.raw = io_apic_read(apic, 1);
-		spin_unlock_irqrestore(&ioapic_lock, flags);
-		nr_ioapic_registers[apic] = reg_01.bits.entries+1;
-	}
 
 	if (!nr_legacy_irqs)
 		return;
@@ -4265,6 +4253,7 @@ static int bad_ioapic(unsigned long address)
 void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 {
 	int idx = 0;
+	int entries;
 
 	if (bad_ioapic(address))
 		return;
@@ -4283,9 +4272,14 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 	 * Build basic GSI lookup table to facilitate gsi->io_apic lookups
 	 * and to prevent reprogramming of IOAPIC pins (PCI GSIs).
 	 */
+	entries = io_apic_get_redir_entries(idx);
 	mp_gsi_routing[idx].gsi_base = gsi_base;
-	mp_gsi_routing[idx].gsi_end = gsi_base +
-	    io_apic_get_redir_entries(idx);
+	mp_gsi_routing[idx].gsi_end = gsi_base + entries;
+
+	/*
+	 * The number of IO-APIC IRQ registers (== #pins):
+	 */
+	nr_ioapic_registers[idx] = entries + 1;
 
 	if (mp_gsi_routing[idx].gsi_end > gsi_end)
 		gsi_end = mp_gsi_routing[idx].gsi_end;
-- 
1.8.0

Re: [RFC/PATCH 2.6.32.y 0/3] Re: [stable 2.6.32..2.6.34] x86, ioapic: initialize nr_ioapic_registers early in mp_register_ioapic()

[Suresh Siddha]

On Wed, 2012-10-24 at 12:41 -0700, Jonathan Nieder wrote:
> Suresh Siddha wrote:
> > On Wed, 2012-10-24 at 11:25 -0700, Jonathan Nieder wrote:
> 
> >> Why not cherry-pick 7716a5c4ff5 in full?
> >
> > As that depends on the other commits like:
> > commit 4b6b19a1c7302477653d799a53d48063dd53d555
> 
> More importantly, if I understand correctly it might depend on
> 
>  commit cf7500c0ea13
>  Author: Eric W. Biederman <ebiederm@xmission.com>
>  Date:   Tue Mar 30 01:07:11 2010 -0700
> 
>      x86, ioapic: In mpparse use mp_register_ioapic
> 
> Here's a series, completely untested, that is closer to what I
> expected.  But the approach you took seems reasonable, too, as long
> as the commit message is tweaked to explain it.
> 
> Thanks again,
> Jonathan
> 
> Eric W. Biederman (3):
>   x86, ioapic: Teach mp_register_ioapic to compute a global gsi_end
>   x86, ioapic: In mpparse use mp_register_ioapic
>   x86, ioapic: Move nr_ioapic_registers calculation to
>     mp_register_ioapic.
> 
>  arch/x86/include/asm/io_apic.h |  1 +
>  arch/x86/kernel/apic/io_apic.c | 28 ++++++++++++++--------------
>  arch/x86/kernel/mpparse.c      | 25 +------------------------
>  arch/x86/kernel/sfi.c          |  4 +---
>  4 files changed, 17 insertions(+), 41 deletions(-)

hmm, NO.

I am not sure if it is worth spending time validating all these changes
for the stable series and I can't do it on my own, as I don't have all
the relevant HW.

For example, another commit a4384df3e24579d6292a1b3b41d500349948f30b
(which you haven't picked up in your series) fixes some of these issues
introduced by the commits you have picked.

commit a4384df3e24579d6292a1b3b41d500349948f30b
Author: Eric W. Biederman <ebiederm@xmission.com>
Date:   Tue Jun 8 11:44:32 2010 -0700

    x86, irq: Rename gsi_end gsi_top, and fix off by one errors

So I did think about all these things and wanted to really pursue the
smallest and simplest change. Here is the updated patch with just some
more text added to the changelog. Greg, does this look ok to you?

Thanks.
-- 8< --

From: Suresh Siddha <suresh.b.siddha@intel.com>
Subject: x86, ioapic: initialize nr_ioapic_registers early in mp_register_ioapic()

Lin Bao reported that one of the HP platforms failed to boot
2.6.32 kernel, when the BIOS enabled interrupt-remapping and
x2apic before handing over the control to the Linux kernel.

During boot, Linux kernel masks all the interrupt sources
(8259, IO-APIC RTE's), setup the interrupt-remapping hardware
with the OS controlled table and unmasks the 8259 interrupts
but not the IO-APIC RTE's (as the newly setup interrupt-remapping
table and the IO-APIC RTE's are not yet programmed by the kernel).

Shortly after this, IO-APIC RTE's and the interrupt-remapping table
entries are programmed based on the ACPI tables etc. So the
expectation is that any interrupt during this window will be dropped
and not see the intermediate configuration.

In the reported problematic case, BIOS has configured the IO-APIC
in virtual wire-B mode. Between the window of the kernel setting up
new interrupt-remapping table  and the IO-APIC RTE's are properly
configured, an interrupt gets routed by the IO-APIC RTE (setup
by the virtual wire-B configuration) and sees the empty
interrupt-remapping table entry, resulting in vt-d fault causing
the platform to generate NMI. And the OS panics on this unexpected NMI.

This problem doesn't happen with more recent kernels and closer
look at the 2.6.32 kernel shows that the code which masks
the IO-APIC RTE's is not working as expected as the nr_ioapic_registers
for each IO-APIC is not yet initialized at this point. In the later
kernels we initialize nr_ioapic_registers much before and
everything works as expected.

For 2.6.[32..34] kernels, fix this issue by initializing
nr_ioapic_registers early in mp_register_ioapic()

[ Relevant upstream commit info:
  commit 7716a5c4ff5f1f3dc5e9edcab125cbf7fceef0af
  Author: Eric W. Biederman <ebiederm@xmission.com>
  Date:   Tue Mar 30 01:07:12 2010 -0700

    x86, ioapic: Move nr_ioapic_registers calculation to mp_register_ioapic.

  As the upstream commit depends on quite a few prior commits
  and some followup fixes in the mainline, we just picked
  the smallest relevant hunk for fixing the issue at hand.
  Problematic platform uses ACPI for IO-APIC, VT-d enumeration etc
  and this hunk only touches the ACPI based platforms.

  nr_ioapic_reigsters initialization in enable_IO_APIC() is still
  retained, so that other configurations like legacy MPS table based
  enumeration etc works with no change.
]

Reported-and-tested-by: Zhang, Lin-Bao <linbao.zhang@hp.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: stable@vger.kernel.org [v2.6.32..v2.6.34]
---
 arch/x86/kernel/apic/io_apic.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 8928d97..d256bc3 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -4262,6 +4262,7 @@ static int bad_ioapic(unsigned long address)
 void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 {
 	int idx = 0;
+	int entries;
 
 	if (bad_ioapic(address))
 		return;
@@ -4280,10 +4281,14 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 	 * Build basic GSI lookup table to facilitate gsi->io_apic lookups
 	 * and to prevent reprogramming of IOAPIC pins (PCI GSIs).
 	 */
+	entries = io_apic_get_redir_entries(idx);
 	mp_gsi_routing[idx].gsi_base = gsi_base;
-	mp_gsi_routing[idx].gsi_end = gsi_base +
-	    io_apic_get_redir_entries(idx);
+	mp_gsi_routing[idx].gsi_end = gsi_base + entries;
 
+	/*
+	 * The number of IO-APIC IRQ registers (== #pins):
+	 */
+	nr_ioapic_registers[idx] = entries + 1;
 	printk(KERN_INFO "IOAPIC[%d]: apic_id %d, version %d, address 0x%x, "
 	       "GSI %d-%d\n", idx, mp_ioapics[idx].apicid,
 	       mp_ioapics[idx].apicver, mp_ioapics[idx].apicaddr,

Re: [RFC/PATCH 2.6.32.y 0/3] Re: [stable 2.6.32..2.6.34] x86, ioapic: initialize nr_ioapic_registers early in mp_register_ioapic()

[Jonathan Nieder]

Suresh Siddha wrote:

>                               Here is the updated patch with just some
> more text added to the changelog.

Yes, that's clearer.  For what it's worth,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks for your patience.

Re: [RFC/PATCH 2.6.32.y 0/3] Re: [stable 2.6.32..2.6.34] x86, ioapic: initialize nr_ioapic_registers early in mp_register_ioapic()

[Eric W. Biederman]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Suresh Siddha wrote:
>
>>                               Here is the updated patch with just some
>> more text added to the changelog.
>
> Yes, that's clearer.  For what it's worth,
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
> Thanks for your patience.

The strange days when code cleanups get backported as bug fixes.
Just this minimal version of things look good.

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

Eric

Re: [RFC/PATCH 2.6.32.y 0/3] Re: [stable 2.6.32..2.6.34] x86, ioapic: initialize nr_ioapic_registers early in mp_register_ioapic()

[Willy Tarreau]

On Wed, Oct 24, 2012 at 06:25:20PM -0700, Eric W. Biederman wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
> 
> > Suresh Siddha wrote:
> >
> >>                               Here is the updated patch with just some
> >> more text added to the changelog.
> >
> > Yes, that's clearer.  For what it's worth,
> > Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> >
> > Thanks for your patience.
> 
> The strange days when code cleanups get backported as bug fixes.
> Just this minimal version of things look good.
> 
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

Just queued it for 2.6.32, thanks guys.

Willy