x86_64: Support for new UV apic

Index: linux/arch/x86/kernel/apic_64.c
===================================================================

arch/x86/kernel/apic_64.c +1 -0

@@ -738,6 +738,7 @@ void __cpuinit setup_local_APIC(void)
 	unsigned int value;
 	int i, j;
 
+	preempt_disable();
 	value = apic_read(APIC_LVR);
 
 	BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f);

Quoted 11 lines · Jack Steiner

Index: linux/arch/x86/kernel/apic_64.c
===================================================================

arch/x86/kernel/apic_64.c +1 -0

@@ -738,6 +738,7 @@ void __cpuinit setup_local_APIC(void)
 	unsigned int value;
 	int i, j;
 
+	preempt_disable();
 	value = apic_read(APIC_LVR);
 

arch/x86/kernel/apic_64.c void __cpuinit setup_local_APIC(void) · line 836

Quoted 7 lines

arch/x86/kernel/apic_64.c +1 -0

 	BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f);

@@ -831,6 +832,7 @@ void __cpuinit setup_local_APIC(void)
 	else
 		value = APIC_DM_NMI | APIC_LVT_MASKED;
 	apic_write(APIC_LVT1, value);
+	preempt_enable();
 }

hm, this looks a bit weird - why are all the preempt-disable/enable calls needed?

Quoted 14 lines · Ingo Molnar

Quoted 11 lines · Jack Steiner

Index: linux/arch/x86/kernel/apic_64.c
===================================================================

arch/x86/kernel/apic_64.c +1 -0

@@ -738,6 +738,7 @@ void __cpuinit setup_local_APIC(void)
 	unsigned int value;
 	int i, j;
 
+	preempt_disable();
 	value = apic_read(APIC_LVR);
 

Quoted 10 lines

arch/x86/kernel/apic_64.c void __cpuinit setup_local_APIC(void) · line 836

Quoted 7 lines

arch/x86/kernel/apic_64.c +1 -0

 	BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f);

@@ -831,6 +832,7 @@ void __cpuinit setup_local_APIC(void)
 	else
 		value = APIC_DM_NMI | APIC_LVT_MASKED;
 	apic_write(APIC_LVT1, value);
+	preempt_enable();
 }

hm, this looks a bit weird - why are all the preempt-disable/enable calls needed?

The first patch had a preempt disable/enable in the function that reads apicid (see read_apic_id() in arch/x86/kernel/genapic_64.c). This seemed necessary since large system generate an apicid by reading the live id & concatenating it with extra bits.

One of the review comments suggested that I change the preempt to a WARN() since reading apic_id really should be done with preemtion disabled. The added code eliminates the warnings.

Quoted 26 lines · Jack Steiner

Quoted 14 lines · Ingo Molnar

Quoted 11 lines · Jack Steiner

Index: linux/arch/x86/kernel/apic_64.c
===================================================================

arch/x86/kernel/apic_64.c +1 -0

@@ -738,6 +738,7 @@ void __cpuinit setup_local_APIC(void)
 	unsigned int value;
 	int i, j;
 
+	preempt_disable();
 	value = apic_read(APIC_LVR);
 

Quoted 10 lines

arch/x86/kernel/apic_64.c void __cpuinit setup_local_APIC(void) · line 836

Quoted 7 lines

arch/x86/kernel/apic_64.c +1 -0

 	BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f);

@@ -831,6 +832,7 @@ void __cpuinit setup_local_APIC(void)
 	else
 		value = APIC_DM_NMI | APIC_LVT_MASKED;
 	apic_write(APIC_LVT1, value);
+	preempt_enable();
 }

hm, this looks a bit weird - why are all the preempt-disable/enable calls needed?

Quoted 8 lines

The first patch had a preempt disable/enable in the function that reads apicid (see read_apic_id() in arch/x86/kernel/genapic_64.c). This seemed necessary since large system generate an apicid by reading the live id & concatenating it with extra bits.

One of the review comments suggested that I change the preempt to a WARN() since reading apic_id really should be done with preemtion disabled. The added code eliminates the warnings.

could you post the stacktraces of the warnings as they occured? All those codepaths should be running with preemption disabled in some natural way.

Ingo

UV supports really big systems. So big, in fact, that the APICID register does not contain enough bits to contain an APICID that is unique across all cpus.

The UV BIOS supports 3 APICID modes:

- legacy mode. This mode uses the old APIC mode where APICID is in bits [31:24] of the APICID register.

- x2apic mode. This mode is whitebox-compatible. APICIDs are unique across all cpus. Standard x2apic APIC operations (Intel-defined) can be used for IPIs. The node identifier fits within the Intel-defined portion of the APICID register.

- x2apic-uv mode. In this mode, the APICIDs on each node have unique IDs, but IDs on different node are not unique. For example, if each mode has 32 cpus, the APICIDs on each node might be 0 - 31. Every node has the same set of IDs.
The UV hub is used to route IPIs/interrupts to the correct node. Traditional APIC operations WILL NOT WORK.

In x2apic-uv mode, the ACPI tables all contain a full unique ID (note:
exact bit layout still changing but the following is close):

nnnnnnnnnnlc0cch
n = unique node number
l = socket number on board
c = core
h = hyperthread

Only the "lc0cch" bits are written to the APICID register. The remaining bits are supplied by having the get_apic_id() function "OR" the extra bits into the value read from the APICID register. (Hmmm.. why not keep the ENTIRE APICID register in per-cpu data....)

The x2apic-uv mode is recognized by the MADT table containing:

          oem_id = "SGI"
          oem_table_id = "UV-X"

Based on:

        git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86.git

Signed-off-by Jack Steiner <[email protected]>

---

  arch/x86/kernel/Makefile         |    2
  arch/x86/kernel/apic_64.c        |    2
  arch/x86/kernel/genapic_64.c     |   18 ++
  arch/x86/kernel/genx2apic_uv_x.c |  245 +++++++++++++++++++++++++++++++++++++++
  arch/x86/kernel/setup64.c        |    4
  arch/x86/kernel/smpboot.c        |    5
  include/asm-x86/genapic_64.h     |    5
  include/asm-x86/smp.h            |    5
  8 files changed, 285 insertions(+), 1 deletion(-)

Index: linux/arch/x86/kernel/genapic_64.c
===================================================================
--- linux.orig/arch/x86/kernel/genapic_64.c 2008-03-28 13:06:07.000000000 -0500
+++ linux/arch/x86/kernel/genapic_64.c 2008-03-28 13:06:12.000000000 -0500 @@ -15,6 +15,7 @@
#include <linux/kernel.h>
#include <linux/ctype.h>
#include <linux/init.h>
+#include <linux/hardirq.h>

#include <asm/smp.h>
#include <asm/ipi.h>
@@ -32,6 +33,7 @@ void *x86_cpu_to_apicid_early_ptr;
#endif
DEFINE_PER_CPU(u16, x86_cpu_to_apicid) = BAD_APICID; EXPORT_PER_CPU_SYMBOL(x86_cpu_to_apicid);
+DEFINE_PER_CPU(int, x2apic_extra_bits);

  struct genapic __read_mostly *genapic = &apic_flat;

@@ -42,6 +44,9 @@ static enum uv_system_type uv_system_typ
*/
void __init setup_apic_routing(void)
{
+ if (uv_system_type == UV_NON_UNIQUE_APIC)
+ genapic = &apic_x2apic_uv_x;
+ else
#ifdef CONFIG_ACPI
/*
* Quirk: some x86_64 machines can only use physical APIC mode
@@ -82,6 +87,19 @@ int __init acpi_madt_oem_check(char *oem

        return 0;

}

+unsigned int read_apic_id(void)
+{
+ unsigned int id;
+

 +       WARN_ON(preemptible());
 +       id = apic_read(APIC_ID);
 +       if (uv_system_type >= UV_X2APIC)

+ id |= __get_cpu_var(x2apic_extra_bits);
+ else
+ id = (id >> 24) & 0xFFu;;
+ return id;
+}

Quoted 94 lines · Jack Steiner

UV supports really big systems. So big, in fact, that the APICID register does not contain enough bits to contain an APICID that is unique across all cpus.

The UV BIOS supports 3 APICID modes:

- legacy mode. This mode uses the old APIC mode where APICID is in bits [31:24] of the APICID register.

- x2apic mode. This mode is whitebox-compatible. APICIDs are unique across all cpus. Standard x2apic APIC operations (Intel-defined) can be used for IPIs. The node identifier fits within the Intel-defined portion of the APICID register.

- x2apic-uv mode. In this mode, the APICIDs on each node have unique IDs, but IDs on different node are not unique. For example, if each mode has 32 cpus, the APICIDs on each node might be 0 - 31. Every node has the same set of IDs.
The UV hub is used to route IPIs/interrupts to the correct node. Traditional APIC operations WILL NOT WORK.

In x2apic-uv mode, the ACPI tables all contain a full unique ID (note:
exact bit layout still changing but the following is close):

nnnnnnnnnnlc0cch
n = unique node number
l = socket number on board
c = core
h = hyperthread

Only the "lc0cch" bits are written to the APICID register. The remaining bits are supplied by having the get_apic_id() function "OR" the extra bits into the value read from the APICID register. (Hmmm.. why not keep the ENTIRE APICID register in per-cpu data....)

The x2apic-uv mode is recognized by the MADT table containing:

          oem_id = "SGI"
          oem_table_id = "UV-X"

Based on:

        git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86.git

Signed-off-by Jack Steiner <[email protected]>

---

  arch/x86/kernel/Makefile         |    2
  arch/x86/kernel/apic_64.c        |    2
  arch/x86/kernel/genapic_64.c     |   18 ++
  arch/x86/kernel/genx2apic_uv_x.c |  245 +++++++++++++++++++++++++++++++++++++++
  arch/x86/kernel/setup64.c        |    4
  arch/x86/kernel/smpboot.c        |    5
  include/asm-x86/genapic_64.h     |    5
  include/asm-x86/smp.h            |    5
  8 files changed, 285 insertions(+), 1 deletion(-)

Index: linux/arch/x86/kernel/genapic_64.c
===================================================================
--- linux.orig/arch/x86/kernel/genapic_64.c 2008-03-28 13:06:07.000000000 -0500
+++ linux/arch/x86/kernel/genapic_64.c 2008-03-28 13:06:12.000000000 -0500 @@ -15,6 +15,7 @@
#include <linux/kernel.h>
#include <linux/ctype.h>
#include <linux/init.h>
+#include <linux/hardirq.h>

#include <asm/smp.h>
#include <asm/ipi.h>
@@ -32,6 +33,7 @@ void *x86_cpu_to_apicid_early_ptr;
#endif
DEFINE_PER_CPU(u16, x86_cpu_to_apicid) = BAD_APICID; EXPORT_PER_CPU_SYMBOL(x86_cpu_to_apicid);
+DEFINE_PER_CPU(int, x2apic_extra_bits);

  struct genapic __read_mostly *genapic = &apic_flat;

@@ -42,6 +44,9 @@ static enum uv_system_type uv_system_typ
*/
void __init setup_apic_routing(void)
{
+ if (uv_system_type == UV_NON_UNIQUE_APIC)
+ genapic = &apic_x2apic_uv_x;
+ else
#ifdef CONFIG_ACPI
/*
* Quirk: some x86_64 machines can only use physical APIC mode
@@ -82,6 +87,19 @@ int __init acpi_madt_oem_check(char *oem

        return 0;

}

+unsigned int read_apic_id(void)
+{
+ unsigned int id;
+

Quoted 8 lines

 +       WARN_ON(preemptible());
 +       id = apic_read(APIC_ID);
 +       if (uv_system_type >= UV_X2APIC)

+ id |= __get_cpu_var(x2apic_extra_bits);
+ else
+ id = (id >> 24) & 0xFFu;;
+ return id;
+}

so this is "the new one of Friday"?

it still wrong. you can not shit id here. ot broke all x86_64 smp.

Did you test it on non UV_X2APIC box?

Quoted 1 line

Did you test it on non UV_X2APIC box?

anyway the read_apic_id is totally wrong, even for your UV_X2APIC box. because id=apic_read(APIC_ID) will have apic_id at bits [31,24], and id |= __get_cpu_var(x2apic_extra_bits) is assuming that is on bits [5,0]

so you even didn't test in your UV_X2APIC box!

Quoted 6 lines

Quoted 1 line

Did you test it on non UV_X2APIC box?

anyway the read_apic_id is totally wrong, even for your UV_X2APIC box. because id=apic_read(APIC_ID) will have apic_id at bits [31,24], and id |= __get_cpu_var(x2apic_extra_bits) is assuming that is on bits [5,0]

Quoted 2 lines

so you even didn't test in your UV_X2APIC box!

It works fine on UV_X2APIX boxes because the double shift does not occur. However, support for UV_X2APIC is dependent on x2apic code that is not yet in the tree. Once the APIC is switched into x2apic mode, the apicid is located in the LOW bits of the apicid register, not the HIGH bits.

Quoted 9 lines · Jack Steiner

Quoted 8 lines

Quoted 1 line

Did you test it on non UV_X2APIC box?

anyway the read_apic_id is totally wrong, even for your UV_X2APIC box. because id=apic_read(APIC_ID) will have apic_id at bits [31,24], and id |= __get_cpu_var(x2apic_extra_bits) is assuming that is on bits [5,0]

so you even didn't test in your UV_X2APIC box!

Quoted 5 lines

It works fine on UV_X2APIX boxes because the double shift does not occur. However, support for UV_X2APIC is dependent on x2apic code that is not yet in the tree. Once the APIC is switched into x2apic mode, the apicid is located in the LOW bits of the apicid register, not the HIGH bits.

oh, so that will need have new version GET_APIC_ID too.

Quoted 16 lines · Yinghai Lu

Quoted 14 lines · Jack Steiner

Quoted 8 lines

Quoted 1 line

Did you test it on non UV_X2APIC box?

anyway the read_apic_id is totally wrong, even for your UV_X2APIC box. because id=apic_read(APIC_ID) will have apic_id at bits [31,24], and id |= __get_cpu_var(x2apic_extra_bits) is assuming that is on bits [5,0]

so you even didn't test in your UV_X2APIC box!

It works fine on UV_X2APIX boxes because the double shift does not occur. However, support for UV_X2APIC is dependent on x2apic code that is not yet in the tree. Once the APIC is switched into x2apic mode, the apicid is located in the LOW bits of the apicid register, not the HIGH bits.

Quoted 1 line

oh, so that will need have new version GET_APIC_ID too.

Yes, although I think all the changes will be unified into one non-inline function that is a combination of GET_APIC_ID() & read_apic_id().

Quoted 20 lines · Jack Steiner

Quoted 17 lines · Yinghai Lu

Quoted 6 lines · Jack Steiner

Quoted 6 lines

Quoted 1 line

Did you test it on non UV_X2APIC box?

anyway the read_apic_id is totally wrong, even for your UV_X2APIC box. because id=apic_read(APIC_ID) will have apic_id at bits [31,24], and id |= __get_cpu_var(x2apic_extra_bits) is assuming that is on bits [5,0]

Quoted 8 lines

Quoted 2 lines

so you even didn't test in your UV_X2APIC box!

It works fine on UV_X2APIX boxes because the double shift does not occur. However, support for UV_X2APIC is dependent on x2apic code that is not yet in the tree. Once the APIC is switched into x2apic mode, the apicid is located in the LOW bits of the apicid register, not the HIGH bits.

oh, so that will need have new version GET_APIC_ID too.

Quoted 3 lines

Yes, although I think all the changes will be unified into one non-inline function that is a combination of GET_APIC_ID() & read_apic_id().

it seems x2apic patch should be applied before uv patch...

YH

so this is "the new one of Friday"?

Did you test it on non UV_X2APIC box?

Quoted 1 line

so this is "the new one of Friday"?

Yes, and it has the same bug although it is located in a slightly different place.

A few minutes ago, I posted a patch to delete the extra lines.

Quoted 1 line

Did you test it on non UV_X2APIC box?

The code is clearly wrong. I booted on an 8p AMD box and had no problems. Apparently the kernel (at least basic booting) is not too sensitive to incorrect apicids being returned. Most critical-to-boot code must use apicids from the ACPI tables. However, the bug does affect numa node assignment. And probably other places, too.

Quoted 9 lines · Jack Steiner

Quoted 1 line

so this is "the new one of Friday"?

Yes, and it has the same bug although it is located in a slightly different place.

A few minutes ago, I posted a patch to delete the extra lines.

Quoted 8 lines

Quoted 1 line

Did you test it on non UV_X2APIC box?

The code is clearly wrong. I booted on an 8p AMD box and had no problems. Apparently the kernel (at least basic booting) is not too sensitive to incorrect apicids being returned. Most critical-to-boot code must use apicids from the ACPI tables. However, the bug does affect numa node assignment. And probably other places, too.

please consider one global get_apic_id() and bad_apicid to replace GET_APIC_ID and BAD_APICID at this point.

Quoted 19 lines · Yinghai Lu

Quoted 9 lines · Jack Steiner

Quoted 1 line

so this is "the new one of Friday"?

Yes, and it has the same bug although it is located in a slightly different place.

A few minutes ago, I posted a patch to delete the extra lines.

Quoted 8 lines

Quoted 1 line

Did you test it on non UV_X2APIC box?

The code is clearly wrong. I booted on an 8p AMD box and had no problems. Apparently the kernel (at least basic booting) is not too sensitive to incorrect apicids being returned. Most critical-to-boot code must use apicids from the ACPI tables. However, the bug does affect numa node assignment. And probably other places, too.

Quoted 2 lines

please consider one global get_apic_id() and bad_apicid to replace GET_APIC_ID and BAD_APICID at this point.

I think that makes sense.

The x2apic patch that should be posted in the near future also makes significant changes in this area. Once that patch is posted, I'll make the simplifications.

Ok???

Quoted 30 lines · Jack Steiner

Quoted 19 lines · Yinghai Lu

Quoted 9 lines · Jack Steiner

Quoted 1 line

so this is "the new one of Friday"?

Yes, and it has the same bug although it is located in a slightly different place.

A few minutes ago, I posted a patch to delete the extra lines.

Quoted 8 lines

Quoted 1 line

Did you test it on non UV_X2APIC box?

The code is clearly wrong. I booted on an 8p AMD box and had no problems. Apparently the kernel (at least basic booting) is not too sensitive to incorrect apicids being returned. Most critical-to-boot code must use apicids from the ACPI tables. However, the bug does affect numa node assignment. And probably other places, too.

Quoted 2 lines

please consider one global get_apic_id() and bad_apicid to replace GET_APIC_ID and BAD_APICID at this point.

I think that makes sense.

The x2apic patch that should be posted in the near future also makes significant changes in this area. Once that patch is posted, I'll make the simplifications.

Quoted 1 line

Ok???

good.

with the x2apic patch, GET_APIC_ID is read_apic_id, and it will not shift again?

YH

Fix double-shift of apicid in previous patch.

The code is clearly wrong. I booted on an 8p AMD box and had no problems. Apparently the kernel (at least basic booting) is not too sensitive to incorrect apicids being returned. Most critical-to-boot code must use apicids from the ACPI tables.