Subject: [PATCH] x86: detect use of extended APIC ID for AMD CPUs


Booting a 32-bit kernel on Magny-Cours results in the following panic

...
Using APIC driver default
...
Overriding APIC driver with bigsmp
...
Getting VERSION: 80050010
Getting VERSION: 80050010
Getting ID: 10000000
Getting ID: ef000000
Getting LVT0: 700
Getting LVT1: 10000
Kernel panic - not syncing: Boot APIC ID in local APIC unexpected (16 vs 0)
Pid: 1, comm: swapper Not tainted 2.6.30-rcX #2
Call Trace:
[<c05194da>] ? panic+0x38/0xd3
[<c0743102>] ? native_smp_prepare_cpus+0x259/0x31f
[<c073b19d>] ? kernel_init+0x3e/0x141
[<c073b15f>] ? kernel_init+0x0/0x141
[<c020325f>] ? kernel_thread_helper+0x7/0x10

The reason is that default_get_apic_id handled extension of local APIC
ID field just in case of XAPIC.
Thus for this AMD CPU, default_get_apic_id() returns 0 and
bigsmp_get_apic_id() returns 16 which leads to the respective kernel
panic.

This patch introduces a Linux specific feature flag to indicate
support for extended APIC id (8 bits instead of 4 bits width) and sets
the flag on AMD CPUs if applicable.

Signed-off-by: Andreas Herrmann <[email protected]>
---
arch/x86/include/asm/apic.h | 2 +-
arch/x86/include/asm/cpufeature.h | 1 +
arch/x86/kernel/cpu/amd.c | 9 +++++++++
3 files changed, 11 insertions(+), 1 deletions(-)

Normally you would run 64-bit kernel on such a CPU which correctly
handles 8-bit APIC id. So this is not a big issue, but it's still a
bug in the 32-bit case.

Patch is against tip/master plus 5 patches from
"[PATCH 0/5 v3] x86: adapt CPU topology detection for AMD Magny-Cours"
Please apply.


Thanks,

Andreas

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 229d0be..bb7d479 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -402,7 +402,7 @@ static inline unsigned default_get_apic_id(unsigned long x)
{
unsigned int ver = GET_APIC_VERSION(apic_read(APIC_LVR));

- if (APIC_XAPIC(ver))
+ if (APIC_XAPIC(ver) || boot_cpu_has(X86_FEATURE_EXTD_APICID))
return (x >> 24) & 0xFF;
else
return (x >> 24) & 0x0F;
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 7abe596..6143bfb 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -95,6 +95,7 @@
#define X86_FEATURE_NONSTOP_TSC (3*32+24) /* TSC does not stop in C states */
#define X86_FEATURE_CLFLUSH_MONITOR (3*32+25) /* "" clflush reqd with monitor */
#define X86_FEATURE_AMD_DCM (3*32+26) /* multi-node processor */
+#define X86_FEATURE_EXTD_APICID (3*32+27) /* has extended APICID (8 bits) */

/* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
#define X86_FEATURE_XMM3 (4*32+ 0) /* "pni" SSE-3 */
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 8c27925..551adb8 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -409,6 +409,15 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
(c->x86_model == 8 && c->x86_mask >= 8))
set_cpu_cap(c, X86_FEATURE_K6_MTRR);
#endif
+#ifdef CONFIG_X86_LOCAL_APIC
+ /* check CPU config space for extended APIC ID */
+ if (c->x86 >= 0xf) {
+ unsigned int val;
+ val = read_pci_config(0, 24, 0, 0x68);
+ if ((val & ((1 << 17) | (1 << 18))) == ((1 << 17) | (1 << 18)))
+ set_cpu_cap(c, X86_FEATURE_EXTD_APICID);
+ }
+#endif
}

static void __cpuinit init_amd(struct cpuinfo_x86 *c)
--
1.6.0.4



Subject: [tip:x86/urgent] x86: Detect use of extended APIC ID for AMD CPUs

Commit-ID: 37f36c1017f0032fa7a1fb85a0cddfc0d526d17f
Gitweb: http://git.kernel.org/tip/37f36c1017f0032fa7a1fb85a0cddfc0d526d17f
Author: Andreas Herrmann <[email protected]>
AuthorDate: Thu, 4 Jun 2009 12:40:16 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Sun, 7 Jun 2009 16:39:38 +0200

x86: Detect use of extended APIC ID for AMD CPUs

Booting a 32-bit kernel on Magny-Cours results in the following panic

...
Using APIC driver default
...
Overriding APIC driver with bigsmp
...
Getting VERSION: 80050010
Getting VERSION: 80050010
Getting ID: 10000000
Getting ID: ef000000
Getting LVT0: 700
Getting LVT1: 10000
Kernel panic - not syncing: Boot APIC ID in local APIC unexpected (16 vs 0)
Pid: 1, comm: swapper Not tainted 2.6.30-rcX #2
Call Trace:
[<c05194da>] ? panic+0x38/0xd3
[<c0743102>] ? native_smp_prepare_cpus+0x259/0x31f
[<c073b19d>] ? kernel_init+0x3e/0x141
[<c073b15f>] ? kernel_init+0x0/0x141
[<c020325f>] ? kernel_thread_helper+0x7/0x10

The reason is that default_get_apic_id handled extension of local APIC
ID field just in case of XAPIC.

Thus for this AMD CPU, default_get_apic_id() returns 0 and
bigsmp_get_apic_id() returns 16 which leads to the respective kernel
panic.

This patch introduces a Linux specific feature flag to indicate
support for extended APIC id (8 bits instead of 4 bits width) and sets
the flag on AMD CPUs if applicable.

Signed-off-by: Andreas Herrmann <[email protected]>
Cc: <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>


---
arch/x86/include/asm/apic.h | 2 +-
arch/x86/include/asm/cpufeature.h | 1 +
arch/x86/kernel/cpu/amd.c | 10 ++++++++++
3 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 42f2f83..9b2c049 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -410,7 +410,7 @@ static inline unsigned default_get_apic_id(unsigned long x)
{
unsigned int ver = GET_APIC_VERSION(apic_read(APIC_LVR));

- if (APIC_XAPIC(ver))
+ if (APIC_XAPIC(ver) || boot_cpu_has(X86_FEATURE_EXTD_APICID))
return (x >> 24) & 0xFF;
else
return (x >> 24) & 0x0F;
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index bb83b1c..78dee4f 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -94,6 +94,7 @@
#define X86_FEATURE_TSC_RELIABLE (3*32+23) /* TSC is known to be reliable */
#define X86_FEATURE_NONSTOP_TSC (3*32+24) /* TSC does not stop in C states */
#define X86_FEATURE_CLFLUSH_MONITOR (3*32+25) /* "" clflush reqd with monitor */
+#define X86_FEATURE_EXTD_APICID (3*32+26) /* has extended APICID (8 bits) */

/* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
#define X86_FEATURE_XMM3 (4*32+ 0) /* "pni" SSE-3 */
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 7e4a459..fae8be8 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -351,6 +351,16 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
(c->x86_model == 8 && c->x86_mask >= 8))
set_cpu_cap(c, X86_FEATURE_K6_MTRR);
#endif
+#ifdef CONFIG_X86_LOCAL_APIC
+ /* check CPU config space for extended APIC ID */
+ if (c->x86 >= 0xf) {
+ unsigned int val;
+
+ val = read_pci_config(0, 24, 0, 0x68);
+ if ((val & ((1 << 17) | (1 << 18))) == ((1 << 17) | (1 << 18)))
+ set_cpu_cap(c, X86_FEATURE_EXTD_APICID);
+ }
+#endif
}

static void __cpuinit init_amd(struct cpuinfo_x86 *c)

2009-06-07 15:34:06

by Ingo Molnar

[permalink] [raw]
Subject: Re: [tip:x86/urgent] x86: Detect use of extended APIC ID for AMD CPUs


* tip-bot for Andreas Herrmann <[email protected]> wrote:

> +#ifdef CONFIG_X86_LOCAL_APIC
> + /* check CPU config space for extended APIC ID */
> + if (c->x86 >= 0xf) {
> + unsigned int val;
> +
> + val = read_pci_config(0, 24, 0, 0x68);
> + if ((val & ((1 << 17) | (1 << 18))) == ((1 << 17) | (1 << 18)))
> + set_cpu_cap(c, X86_FEATURE_EXTD_APICID);
> + }
> +#endif

It's possible to enable local APIC but not CONFIG_PCI - in that case
this sequence will fail to build.

Ingo

2009-06-07 18:47:51

by Yinghai Lu

[permalink] [raw]
Subject: Re: [tip:x86/urgent] x86: Detect use of extended APIC ID for AMD CPUs

tip-bot for Andreas Herrmann wrote:
> Commit-ID: 37f36c1017f0032fa7a1fb85a0cddfc0d526d17f
> Gitweb: http://git.kernel.org/tip/37f36c1017f0032fa7a1fb85a0cddfc0d526d17f
> Author: Andreas Herrmann <[email protected]>
> AuthorDate: Thu, 4 Jun 2009 12:40:16 +0200
> Committer: Ingo Molnar <[email protected]>
> CommitDate: Sun, 7 Jun 2009 16:39:38 +0200
>
> x86: Detect use of extended APIC ID for AMD CPUs
>
> Booting a 32-bit kernel on Magny-Cours results in the following panic
>
> ...
> Using APIC driver default
> ...
> Overriding APIC driver with bigsmp
> ...
> Getting VERSION: 80050010
> Getting VERSION: 80050010
> Getting ID: 10000000
> Getting ID: ef000000
> Getting LVT0: 700
> Getting LVT1: 10000
> Kernel panic - not syncing: Boot APIC ID in local APIC unexpected (16 vs 0)
> Pid: 1, comm: swapper Not tainted 2.6.30-rcX #2
> Call Trace:
> [<c05194da>] ? panic+0x38/0xd3
> [<c0743102>] ? native_smp_prepare_cpus+0x259/0x31f
> [<c073b19d>] ? kernel_init+0x3e/0x141
> [<c073b15f>] ? kernel_init+0x0/0x141
> [<c020325f>] ? kernel_thread_helper+0x7/0x10
>
> The reason is that default_get_apic_id handled extension of local APIC
> ID field just in case of XAPIC.
>
> Thus for this AMD CPU, default_get_apic_id() returns 0 and
> bigsmp_get_apic_id() returns 16 which leads to the respective kernel
> panic.
>
> This patch introduces a Linux specific feature flag to indicate
> support for extended APIC id (8 bits instead of 4 bits width) and sets
> the flag on AMD CPUs if applicable.
>
> Signed-off-by: Andreas Herrmann <[email protected]>
> Cc: <[email protected]>
> LKML-Reference: <[email protected]>
> Signed-off-by: Ingo Molnar <[email protected]>
>
>
> ---
> arch/x86/include/asm/apic.h | 2 +-
> arch/x86/include/asm/cpufeature.h | 1 +
> arch/x86/kernel/cpu/amd.c | 10 ++++++++++
> 3 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> index 42f2f83..9b2c049 100644
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -410,7 +410,7 @@ static inline unsigned default_get_apic_id(unsigned long x)
> {
> unsigned int ver = GET_APIC_VERSION(apic_read(APIC_LVR));
>
> - if (APIC_XAPIC(ver))
> + if (APIC_XAPIC(ver) || boot_cpu_has(X86_FEATURE_EXTD_APICID))
> return (x >> 24) & 0xFF;
> else
> return (x >> 24) & 0x0F;

looks like we should fix APIC_XAPIC() or have one global apic_ver.

YH

Subject: Re: [tip:x86/urgent] x86: Detect use of extended APIC ID for AMD CPUs

On Sun, Jun 07, 2009 at 05:33:27PM +0200, Ingo Molnar wrote:
>
> * tip-bot for Andreas Herrmann <[email protected]> wrote:
>
> > +#ifdef CONFIG_X86_LOCAL_APIC
> > + /* check CPU config space for extended APIC ID */
> > + if (c->x86 >= 0xf) {
> > + unsigned int val;
> > +
> > + val = read_pci_config(0, 24, 0, 0x68);
> > + if ((val & ((1 << 17) | (1 << 18))) == ((1 << 17) | (1 << 18)))
> > + set_cpu_cap(c, X86_FEATURE_EXTD_APICID);
> > + }
> > +#endif
>
> It's possible to enable local APIC but not CONFIG_PCI - in that case
> this sequence will fail to build.

Arrgh, thought that the stuff in early.c is always compiled in. Just
checked arch/x86/pci/Makefile showing
obj-y += common.o early.o
and I didn't check arch/x86/Makefile which contains
drivers-$(CONFIG_PCI) += arch/x86/pci/

Sorry.
I'll send an updated patch.


Andreas

Subject: Re: [tip:x86/urgent] x86: Detect use of extended APIC ID for AMD CPUs

On Sun, Jun 07, 2009 at 11:46:51AM -0700, Yinghai Lu wrote:
> tip-bot for Andreas Herrmann wrote:
> > diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> > index 42f2f83..9b2c049 100644
> > --- a/arch/x86/include/asm/apic.h
> > +++ b/arch/x86/include/asm/apic.h
> > @@ -410,7 +410,7 @@ static inline unsigned default_get_apic_id(unsigned long x)
> > {
> > unsigned int ver = GET_APIC_VERSION(apic_read(APIC_LVR));
> >
> > - if (APIC_XAPIC(ver))
> > + if (APIC_XAPIC(ver) || boot_cpu_has(X86_FEATURE_EXTD_APICID))
> > return (x >> 24) & 0xFF;
> > else
> > return (x >> 24) & 0x0F;
>
> looks like we should fix APIC_XAPIC() or have one global apic_ver.

I thought to set the new flag for Intel as well and to replace
APIC_XAPIC with checks for that flag. But for some reason this was
ugly as lapic access must be configured before you can check the
APIC_VERSION and so I decided to keep Intel code as is and not to
potentially introduce regresions.


Regards,
Andreas

Subject: [PATCH v2] x86: detect use of extended APIC ID for AMD CPUs

Booting a 32-bit kernel on Magny-Cours results in the following panic

...
Using APIC driver default
...
Overriding APIC driver with bigsmp
...
Getting VERSION: 80050010
Getting VERSION: 80050010
Getting ID: 10000000
Getting ID: ef000000
Getting LVT0: 700
Getting LVT1: 10000
Kernel panic - not syncing: Boot APIC ID in local APIC unexpected (16
vs 0)
Pid: 1, comm: swapper Not tainted 2.6.30-rc3-numa-fixup-nonuma #2
Call Trace:
[<c05194da>] ? panic+0x38/0xd3
[<c0743102>] ? native_smp_prepare_cpus+0x259/0x31f
[<c073b19d>] ? kernel_init+0x3e/0x141
[<c073b15f>] ? kernel_init+0x0/0x141
[<c020325f>] ? kernel_thread_helper+0x7/0x10

The reason is that default_get_apic_id handled extension of local APIC
ID field just in case of XAPIC.
Thus for this AMD CPU, default_get_apic_id() returns 0 and
bigsmp_get_apic_id() returns 16 which leads to the respective kernel
panic.

This patch introduces a Linux specific feature flag to indicate
support for extended APIC id (8 bits instead of 4 bits width) and sets
the flag on AMD CPUs if applicable.

Signed-off-by: Andreas Herrmann <[email protected]>
---
arch/x86/include/asm/apic.h | 2 +-
arch/x86/include/asm/cpufeature.h | 1 +
arch/x86/kernel/cpu/amd.c | 10 ++++++++++
3 files changed, 12 insertions(+), 1 deletions(-)

New patch version
- is against tip/x86/urgent (as you have tried to integrate the previous
patch version into this branch)
- avoids linker error in case of CONFIG_PCI=n

Regards,
Andreas

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 42f2f83..9b2c049 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -410,7 +410,7 @@ static inline unsigned default_get_apic_id(unsigned long x)
{
unsigned int ver = GET_APIC_VERSION(apic_read(APIC_LVR));

- if (APIC_XAPIC(ver))
+ if (APIC_XAPIC(ver) || boot_cpu_has(X86_FEATURE_EXTD_APICID))
return (x >> 24) & 0xFF;
else
return (x >> 24) & 0x0F;
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index bb83b1c..78dee4f 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -94,6 +94,7 @@
#define X86_FEATURE_TSC_RELIABLE (3*32+23) /* TSC is known to be reliable */
#define X86_FEATURE_NONSTOP_TSC (3*32+24) /* TSC does not stop in C states */
#define X86_FEATURE_CLFLUSH_MONITOR (3*32+25) /* "" clflush reqd with monitor */
+#define X86_FEATURE_EXTD_APICID (3*32+26) /* has extended APICID (8 bits) */

/* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
#define X86_FEATURE_XMM3 (4*32+ 0) /* "pni" SSE-3 */
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 7e4a459..841ed9f 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -6,6 +6,7 @@
#include <asm/processor.h>
#include <asm/apic.h>
#include <asm/cpu.h>
+#include <asm/pci-direct.h>

#ifdef CONFIG_X86_64
# include <asm/numa_64.h>
@@ -351,6 +352,15 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
(c->x86_model == 8 && c->x86_mask >= 8))
set_cpu_cap(c, X86_FEATURE_K6_MTRR);
#endif
+#if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_PCI)
+ /* check CPU config space for extended APIC ID */
+ if (c->x86 >= 0xf) {
+ unsigned int val;
+ val = read_pci_config(0, 24, 0, 0x68);
+ if ((val & ((1 << 17) | (1 << 18))) == ((1 << 17) | (1 << 18)))
+ set_cpu_cap(c, X86_FEATURE_EXTD_APICID);
+ }
+#endif
}

static void __cpuinit init_amd(struct cpuinfo_x86 *c)
--
1.6.3.1


Subject: [PATCH v3] x86: detect use of extended APIC ID for AMD CPUs

Booting a 32-bit kernel on Magny-Cours results in the following panic

...
Using APIC driver default
...
Overriding APIC driver with bigsmp
...
Getting VERSION: 80050010
Getting VERSION: 80050010
Getting ID: 10000000
Getting ID: ef000000
Getting LVT0: 700
Getting LVT1: 10000
Kernel panic - not syncing: Boot APIC ID in local APIC unexpected (16 vs 0)
Pid: 1, comm: swapper Not tainted 2.6.30-rcX #2
Call Trace:
[<c05194da>] ? panic+0x38/0xd3
[<c0743102>] ? native_smp_prepare_cpus+0x259/0x31f
[<c073b19d>] ? kernel_init+0x3e/0x141
[<c073b15f>] ? kernel_init+0x0/0x141
[<c020325f>] ? kernel_thread_helper+0x7/0x10

The reason is that default_get_apic_id handled extension of local APIC
ID field just in case of XAPIC.

Thus for this AMD CPU, default_get_apic_id() returns 0 and
bigsmp_get_apic_id() returns 16 which leads to the respective kernel
panic.

This patch introduces a Linux specific feature flag to indicate
support for extended APIC id (8 bits instead of 4 bits width) and sets
the flag on AMD CPUs if applicable.

Signed-off-by: Andreas Herrmann <[email protected]>
---
arch/x86/include/asm/apic.h | 2 +-
arch/x86/include/asm/cpufeature.h | 1 +
arch/x86/kernel/cpu/amd.c | 10 ++++++++++
3 files changed, 12 insertions(+), 1 deletions(-)

Slightly modified changelog in comparision to v2.

Regards,
Andreas


diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 42f2f83..9b2c049 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -410,7 +410,7 @@ static inline unsigned default_get_apic_id(unsigned long x)
{
unsigned int ver = GET_APIC_VERSION(apic_read(APIC_LVR));

- if (APIC_XAPIC(ver))
+ if (APIC_XAPIC(ver) || boot_cpu_has(X86_FEATURE_EXTD_APICID))
return (x >> 24) & 0xFF;
else
return (x >> 24) & 0x0F;
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index bb83b1c..78dee4f 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -94,6 +94,7 @@
#define X86_FEATURE_TSC_RELIABLE (3*32+23) /* TSC is known to be reliable */
#define X86_FEATURE_NONSTOP_TSC (3*32+24) /* TSC does not stop in C states */
#define X86_FEATURE_CLFLUSH_MONITOR (3*32+25) /* "" clflush reqd with monitor */
+#define X86_FEATURE_EXTD_APICID (3*32+26) /* has extended APICID (8 bits) */

/* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
#define X86_FEATURE_XMM3 (4*32+ 0) /* "pni" SSE-3 */
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 7e4a459..0802e15 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -6,6 +6,7 @@
#include <asm/processor.h>
#include <asm/apic.h>
#include <asm/cpu.h>
+#include <asm/pci-direct.h>

#ifdef CONFIG_X86_64
# include <asm/numa_64.h>
@@ -351,6 +352,15 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
(c->x86_model == 8 && c->x86_mask >= 8))
set_cpu_cap(c, X86_FEATURE_K6_MTRR);
#endif
+#if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_PCI)
+ /* check CPU config space for extended APIC ID */
+ if (c->x86 >= 0xf) {
+ unsigned int val;
+ val = read_pci_config(0, 24, 0, 0x68);
+ if ((val & ((1 << 17) | (1 << 18))) == ((1 << 17) | (1 << 18)))
+ set_cpu_cap(c, X86_FEATURE_EXTD_APICID);
+ }
+#endif
}

static void __cpuinit init_amd(struct cpuinfo_x86 *c)
--
1.6.3.1


Subject: [tip:x86/urgent] x86: Detect use of extended APIC ID for AMD CPUs

Commit-ID: 42937e81a82b6bbc51a309c83da140b3a7ca5945
Gitweb: http://git.kernel.org/tip/42937e81a82b6bbc51a309c83da140b3a7ca5945
Author: Andreas Herrmann <[email protected]>
AuthorDate: Mon, 8 Jun 2009 15:55:09 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 9 Jun 2009 15:28:46 +0200

x86: Detect use of extended APIC ID for AMD CPUs

Booting a 32-bit kernel on Magny-Cours results in the following panic:

...
Using APIC driver default
...
Overriding APIC driver with bigsmp
...
Getting VERSION: 80050010
Getting VERSION: 80050010
Getting ID: 10000000
Getting ID: ef000000
Getting LVT0: 700
Getting LVT1: 10000
Kernel panic - not syncing: Boot APIC ID in local APIC unexpected (16 vs 0)
Pid: 1, comm: swapper Not tainted 2.6.30-rcX #2
Call Trace:
[<c05194da>] ? panic+0x38/0xd3
[<c0743102>] ? native_smp_prepare_cpus+0x259/0x31f
[<c073b19d>] ? kernel_init+0x3e/0x141
[<c073b15f>] ? kernel_init+0x0/0x141
[<c020325f>] ? kernel_thread_helper+0x7/0x10

The reason is that default_get_apic_id handled extension of local APIC
ID field just in case of XAPIC.

Thus for this AMD CPU, default_get_apic_id() returns 0 and
bigsmp_get_apic_id() returns 16 which leads to the respective kernel
panic.

This patch introduces a Linux specific feature flag to indicate
support for extended APIC id (8 bits instead of 4 bits width) and sets
the flag on AMD CPUs if applicable.

Signed-off-by: Andreas Herrmann <[email protected]>
Cc: <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>


---
arch/x86/include/asm/apic.h | 2 +-
arch/x86/include/asm/cpufeature.h | 1 +
arch/x86/kernel/cpu/amd.c | 10 ++++++++++
3 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 42f2f83..9b2c049 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -410,7 +410,7 @@ static inline unsigned default_get_apic_id(unsigned long x)
{
unsigned int ver = GET_APIC_VERSION(apic_read(APIC_LVR));

- if (APIC_XAPIC(ver))
+ if (APIC_XAPIC(ver) || boot_cpu_has(X86_FEATURE_EXTD_APICID))
return (x >> 24) & 0xFF;
else
return (x >> 24) & 0x0F;
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index bb83b1c..78dee4f 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -94,6 +94,7 @@
#define X86_FEATURE_TSC_RELIABLE (3*32+23) /* TSC is known to be reliable */
#define X86_FEATURE_NONSTOP_TSC (3*32+24) /* TSC does not stop in C states */
#define X86_FEATURE_CLFLUSH_MONITOR (3*32+25) /* "" clflush reqd with monitor */
+#define X86_FEATURE_EXTD_APICID (3*32+26) /* has extended APICID (8 bits) */

/* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
#define X86_FEATURE_XMM3 (4*32+ 0) /* "pni" SSE-3 */
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 7e4a459..0802e15 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -6,6 +6,7 @@
#include <asm/processor.h>
#include <asm/apic.h>
#include <asm/cpu.h>
+#include <asm/pci-direct.h>

#ifdef CONFIG_X86_64
# include <asm/numa_64.h>
@@ -351,6 +352,15 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
(c->x86_model == 8 && c->x86_mask >= 8))
set_cpu_cap(c, X86_FEATURE_K6_MTRR);
#endif
+#if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_PCI)
+ /* check CPU config space for extended APIC ID */
+ if (c->x86 >= 0xf) {
+ unsigned int val;
+ val = read_pci_config(0, 24, 0, 0x68);
+ if ((val & ((1 << 17) | (1 << 18))) == ((1 << 17) | (1 << 18)))
+ set_cpu_cap(c, X86_FEATURE_EXTD_APICID);
+ }
+#endif
}

static void __cpuinit init_amd(struct cpuinfo_x86 *c)

2009-07-21 10:43:20

by Bastian Blank

[permalink] [raw]
Subject: Re: [PATCH] x86: detect use of extended APIC ID for AMD CPUs

On Thu, Jun 04, 2009 at 12:40:16PM +0200, Andreas Herrmann wrote:
> Booting a 32-bit kernel on Magny-Cours results in the following panic

This patch breaks Xen really bad. It uses read_pci_config without
knowing that there is something accessible. Also read_pci_config does
not define fault handlers.

| (XEN) traps.c:413:d375 Unhandled general protection fault fault/trap [#13] on VCPU 0 [ec=0000]
| (XEN) domain_crash_sync called from entry.S
| (XEN) Domain 375 (vcpu#0) crashed on cpu#1:
| (XEN) ----[ Xen-3.2-1 x86_64 debug=n Not tainted ]----
| (XEN) CPU: 1
| (XEN) RIP: e033:[<ffffffff80401d6b>]
| (XEN) RFLAGS: 0000000000000282 CONTEXT: guest
| (XEN) rax: 000000008000c068 rbx: ffffffff80618a40 rcx: 0000000000000068
| (XEN) rdx: 0000000000000cf8 rsi: 000000000000c000 rdi: 0000000000000000
| (XEN) rbp: 0000000000000018 rsp: ffffffff80621eb0 r8: ffffffff80621efc
| (XEN) r9: 00000000ffffffff r10: ffffffff80621ef8 r11: 00000000ffffffff
| (XEN) r12: ffffffffffffffff r13: ffffffff80621fd8 r14: 0000002040404030
| (XEN) r15: 00000000015bb600 cr0: 000000008005003b cr4: 00000000000006f0
| (XEN) cr3: 00000002077be000 cr2: 000000204316b000
| (XEN) ds: 0000 es: 0000 fs: 0000 gs: 0000 ss: e02b cs: e033
| (XEN) Guest stack trace from rsp=ffffffff80621eb0:
| (XEN) 0000000000000068 00000000ffffffff 0000000000000000 ffffffff80401d6b
| (XEN) 000000010000e030 0000000000010082 ffffffff80621ef0 000000000000e02b
| (XEN) ffffffff804aeb3a 0000000100000000 0000302800000000 ffffffff805bcb20
| (XEN) ffffffff80651552 0000000000000000 0000000000000000 0000000000fdfd88
| (XEN) ffffffff8064e47c 0000000001fb9000 ffffffff80545875 0000000000000800
| (XEN) 0000000000002c00 ffffffff81fb9000 ffffffff80650026 ffffffff805ca1c0

| ffffffff80401d44 T read_pci_config
| ffffffff804aeb41 t early_init_amd

I see several ways to fix this:
- Define fault handlers for *_pci_config
- Check for Xen
- Disable early PCI access from Xen if running unpriviledged and check
that in either in *_pci_config or early_init_amd

Bastian

--
Insults are effective only where emotion is present.
-- Spock, "Who Mourns for Adonais?" stardate 3468.1

2009-07-21 16:56:49

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [PATCH] x86: detect use of extended APIC ID for AMD CPUs

On 07/21/09 03:36, Bastian Blank wrote:
> On Thu, Jun 04, 2009 at 12:40:16PM +0200, Andreas Herrmann wrote:
>
>> Booting a 32-bit kernel on Magny-Cours results in the following panic
>>
>
> This patch breaks Xen really bad. It uses read_pci_config without
> knowing that there is something accessible. Also read_pci_config does
> not define fault handlers.
>
> | (XEN) traps.c:413:d375 Unhandled general protection fault fault/trap [#13] on VCPU 0 [ec=0000]
> | (XEN) domain_crash_sync called from entry.S
> | (XEN) Domain 375 (vcpu#0) crashed on cpu#1:
> | (XEN) ----[ Xen-3.2-1 x86_64 debug=n Not tainted ]----
> | (XEN) CPU: 1
> | (XEN) RIP: e033:[<ffffffff80401d6b>]
> | (XEN) RFLAGS: 0000000000000282 CONTEXT: guest
> | (XEN) rax: 000000008000c068 rbx: ffffffff80618a40 rcx: 0000000000000068
> | (XEN) rdx: 0000000000000cf8 rsi: 000000000000c000 rdi: 0000000000000000
> | (XEN) rbp: 0000000000000018 rsp: ffffffff80621eb0 r8: ffffffff80621efc
> | (XEN) r9: 00000000ffffffff r10: ffffffff80621ef8 r11: 00000000ffffffff
> | (XEN) r12: ffffffffffffffff r13: ffffffff80621fd8 r14: 0000002040404030
> | (XEN) r15: 00000000015bb600 cr0: 000000008005003b cr4: 00000000000006f0
> | (XEN) cr3: 00000002077be000 cr2: 000000204316b000
> | (XEN) ds: 0000 es: 0000 fs: 0000 gs: 0000 ss: e02b cs: e033
> | (XEN) Guest stack trace from rsp=ffffffff80621eb0:
> | (XEN) 0000000000000068 00000000ffffffff 0000000000000000 ffffffff80401d6b
> | (XEN) 000000010000e030 0000000000010082 ffffffff80621ef0 000000000000e02b
> | (XEN) ffffffff804aeb3a 0000000100000000 0000302800000000 ffffffff805bcb20
> | (XEN) ffffffff80651552 0000000000000000 0000000000000000 0000000000fdfd88
> | (XEN) ffffffff8064e47c 0000000001fb9000 ffffffff80545875 0000000000000800
> | (XEN) 0000000000002c00 ffffffff81fb9000 ffffffff80650026 ffffffff805ca1c0
>
> | ffffffff80401d44 T read_pci_config
> | ffffffff804aeb41 t early_init_amd
>
> I see several ways to fix this:
> - Define fault handlers for *_pci_config
> - Check for Xen
> - Disable early PCI access from Xen if running unpriviledged and check
> that in either in *_pci_config or early_init_amd
>
>

I assume "d375" is a domU. cpu_has_apic should be false, and this code
shouldn't be running if the CPU has no (visible) apic.

Does this work? (Completely untested.)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 28e5f59..e2485b0 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -356,7 +356,7 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
#endif
#if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_PCI)
/* check CPU config space for extended APIC ID */
- if (c->x86 >= 0xf) {
+ if (cpu_has_apic && c->x86 >= 0xf) {
unsigned int val;
val = read_pci_config(0, 24, 0, 0x68);
if ((val & ((1 << 17) | (1 << 18))) == ((1 << 17) | (1 << 18)))


J

2009-07-22 09:53:37

by Bastian Blank

[permalink] [raw]
Subject: Re: [PATCH] x86: detect use of extended APIC ID for AMD CPUs

On Tue, Jul 21, 2009 at 09:56:45AM -0700, Jeremy Fitzhardinge wrote:
> Does this work? (Completely untested.)

Yes, it works. It just looks like a workaround.

Bastian

--
Each kiss is as the first.
-- Miramanee, Kirk's wife, "The Paradise Syndrome",
stardate 4842.6

2009-07-22 14:29:27

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH] x86: detect use of extended APIC ID for AMD CPUs

On Wed, 22 Jul 2009, Bastian Blank wrote:

> On Tue, Jul 21, 2009 at 09:56:45AM -0700, Jeremy Fitzhardinge wrote:
> > Does this work? (Completely untested.)
>
> Yes, it works. It just looks like a workaround.

No, it looks fine. Jeremy, can you please send it again with a proper
changelog.

Thanks,

tglx

2009-07-22 16:59:38

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [PATCH] x86/amd: don't probe for extended APIC ID if APICs are disabled

If we've logically disabled apics, don't probe the PCI space for the
AMD extended APIC ID.

[ Impact: prevent boot crash under Xen. ]

Signed-off-by: Jeremy Fitzhardinge <[email protected]>
Reported-by: Bastian Blank <[email protected]>

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 28e5f59..e2485b0 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -356,7 +356,7 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
#endif
#if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_PCI)
/* check CPU config space for extended APIC ID */
- if (c->x86 >= 0xf) {
+ if (cpu_has_apic && c->x86 >= 0xf) {
unsigned int val;
val = read_pci_config(0, 24, 0, 0x68);
if ((val & ((1 << 17) | (1 << 18))) == ((1 << 17) | (1 << 18)))