Skip to contentSkip to conversation

x86_64: make amd quad core 8 socket system not be clustered_box

PATCH In review Yinghai Lu 7 participants 48 messages Feb 21, 2008 – Apr 28, 2008

Subsystems (3)
lore
Share & tools
lore

Yinghai Lu

author
#0 lore raw

PATCH 1/1 x86_64: make amd quad core 8 socket system not be clustered_box

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. or [8, 0x27]. apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

and will get

Marking TSC unstable due to TSCs unsynchronized

even the CPUs have X86_FEATURE_CONSTANT_TSC set.

this patch offset back the apic before get apic clusterid.

or use dmi to get apic_is_clustered?

arch/x86/kernel/apic_64.c +4 -1
index 9b2be3d..5bdf1bc 100644
@@ -1223,8 +1223,11 @@ __cpuinit int apic_is_clustered_box(void)
 		else
 			break;
 
-		if (id != BAD_APICID)
+		if (id != BAD_APICID) {
+			if (id >= boot_cpu_id)
+				id -= boot_cpu_id;
 			__set_bit(APIC_CLUSTERID(id), clustermap);
+		}
 	}
 
 	/* Problem:  Partially populated chassis may not have CPUs in some of
#1 lore raw
Quoted 6 lines · Yinghai Lu

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. or [8, 0x27]. apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

and will get

Quoted 7 lines

Marking TSC unstable due to TSCs unsynchronized

even the CPUs have X86_FEATURE_CONSTANT_TSC set.

this patch offset back the apic before get apic clusterid.

or use dmi to get apic_is_clustered?

The clustered check is for Summit and es7000 systems On 64bit systems it might be actually possible to trigger this based on SLIT instead. But you'll need to check with the IBM Summit/Unisys es7000 developers if that works or not

If you don't want to do that the safer way would be probably the check if there are holes between the CPUs APIC numbers. If yes then it's likely clustered mode. I think that would be better than to disable it unconditionally for apic lifting like your patches does.

-Andi

author
#2 lore raw
Quoted 21 lines · Andi Kleen
Quoted 6 lines · Yinghai Lu

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. or [8, 0x27]. apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

and will get

Quoted 7 lines

Marking TSC unstable due to TSCs unsynchronized

even the CPUs have X86_FEATURE_CONSTANT_TSC set.

this patch offset back the apic before get apic clusterid.

or use dmi to get apic_is_clustered?

The clustered check is for Summit and es7000 systems On 64bit systems it might be actually possible to trigger this based on SLIT instead. But you'll need to check with the IBM Summit/Unisys es7000 developers if that works or not

Quoted 5 lines

If you don't want to do that the safer way would be probably the check if there are holes between the CPUs APIC numbers. If yes then it's likely clustered mode. I think that would be better than to disable it unconditionally for apic lifting like your patches does.

so for that box [4, 0x23] still could be apic clustered? there is a hole [0,3]..

is their box using AMD cpu or not?

YH

author
#3 lore raw
Quoted 28 lines · Yinghai Lu
Quoted 21 lines · Andi Kleen
Quoted 6 lines · Yinghai Lu

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. or [8, 0x27]. apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

and will get

Quoted 7 lines

Marking TSC unstable due to TSCs unsynchronized

even the CPUs have X86_FEATURE_CONSTANT_TSC set.

this patch offset back the apic before get apic clusterid.

or use dmi to get apic_is_clustered?

The clustered check is for Summit and es7000 systems On 64bit systems it might be actually possible to trigger this based on SLIT instead. But you'll need to check with the IBM Summit/Unisys es7000 developers if that works or not

Quoted 5 lines

If you don't want to do that the safer way would be probably the check if there are holes between the CPUs APIC numbers. If yes then it's likely clustered mode. I think that would be better than to disable it unconditionally for apic lifting like your patches does.

Quoted 3 lines

so for that box [4, 0x23] still could be apic clustered? there is a hole [0,3]..

is their box using AMD cpu or not?

or move
CPUs have X86_FEATURE_CONSTANT_TSC check before apic_clustered check?

YH

#4 lore raw
Quoted 3 lines

or move
CPUs have X86_FEATURE_CONSTANT_TSC check before apic_clustered check?

No that would be not correct on Intel boxes.

-Andi

#5 lore raw
In reply to Yinghai Lu
Quoted 2 lines

so for that box [4, 0x23] still could be apic clustered? there is a hole [0,3]..

I meant holes between the CPUs only, not including the IO-APICs.

Quoted 1 line

is their box using AMD cpu or not?

Intel. AMD boxes don't really need clustered mode because they support bigflat mode.

-Andi

#6 lore raw
Quoted 10 lines · Andi Kleen
Quoted 2 lines

so for that box [4, 0x23] still could be apic clustered? there is a hole [0,3]..

I meant holes between the CPUs only, not including the IO-APICs.

Quoted 1 line

is their box using AMD cpu or not?

Intel. AMD boxes don't really need clustered mode because they support bigflat mode.

So DMI or exclude AMD CPU?

YH

#7 lore raw
Quoted 6 lines · Yinghai Lu
Quoted 5 lines · Andi Kleen
Quoted 1 line

so for that box [4, 0x23] still could be apic clustered? there is a hole [0,3]..

I meant holes between the CPUs only, not including the IO-APICs.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

is their box using AMD cpu or not?

Intel. AMD boxes don't really need clustered mode because they support bigflat mode.

So DMI or exclude AMD CPU?

Just check for holes between the cpus as I suggested earlier

-Andi

#8 lore raw
Quoted 8 lines · Andi Kleen
Quoted 6 lines · Yinghai Lu
Quoted 5 lines · Andi Kleen
Quoted 1 line

so for that box [4, 0x23] still could be apic clustered? there is a hole [0,3]..

I meant holes between the CPUs only, not including the IO-APICs.

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

is their box using AMD cpu or not?

Intel. AMD boxes don't really need clustered mode because they support bigflat mode.

So DMI or exclude AMD CPU?

Just check for holes between the cpus as I suggested earlier

how about their system that is not full populated with CPU?

YH

#9 lore raw
Quoted 10 lines · Yinghai Lu
Quoted 9 lines · Andi Kleen
Quoted 8 lines · Yinghai Lu
Quoted 7 lines · Andi Kleen
Quoted 1 line

so for that box [4, 0x23] still could be apic clustered? there is a hole [0,3]..

I meant holes between the CPUs only, not including the IO-APICs.

Quoted 1 line

is their box using AMD cpu or not?

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

Intel. AMD boxes don't really need clustered mode because they support bigflat mode.

So DMI or exclude AMD CPU?

Just check for holes between the cpus as I suggested earlier

how about their system that is not full populated with CPU?

I would expect the APIC IDs to be continuous then.

-Andi

#10 lore raw
Quoted 14 lines · Andi Kleen
Quoted 13 lines · Yinghai Lu
Quoted 12 lines · Andi Kleen
Quoted 11 lines · Yinghai Lu
Quoted 9 lines · Andi Kleen
Quoted 1 line

so for that box [4, 0x23] still could be apic clustered? there is a hole [0,3]..

I meant holes between the CPUs only, not including the IO-APICs.

Quoted 1 line

is their box using AMD cpu or not?

Intel. AMD boxes don't really need clustered mode because they support bigflat mode.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

So DMI or exclude AMD CPU?

Just check for holes between the cpus as I suggested earlier

how about their system that is not full populated with CPU?

I would expect the APIC IDs to be continuous then.

so those box will have apic id hole even they are fully poluated with cpu...?

YH

author
#11 lore raw

PATCH 1/1 x86_64: make amd quad core 8 socket system not be clustered_box v2

In reply to Andi Kleen

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. and apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

and will get

Marking TSC unstable due to TSCs unsynchronized

even the CPUs have X86_FEATURE_CONSTANT_TSC set.

this patch will check if the cpu is from AMD.

arch/x86/kernel/apic_64.c +11 -1
index d8d03e0..7d8ffda 100644
@@ -1180,9 +1180,19 @@ __cpuinit int apic_is_clustered_box(void)
 {
 	int i, clusters, zeros;
 	unsigned id;
-	u16 *bios_cpu_apicid = x86_bios_cpu_apicid_early_ptr;
+	u16 *bios_cpu_apicid;
 	DECLARE_BITMAP(clustermap, NUM_APIC_CLUSTERS);
 
+	/*
+	 * there is not this kind of box with AMD CPU yet.
+	 * Some AMD box with quadcore cpu and 8 sockets apicid
+	 * will be [4, 0x23] or [8, 0x27] could be thought to
+	 * have three apic_clusters. So go out early.
+	 */
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+		return 0;
+
+	bios_cpu_apicid = x86_bios_cpu_apicid_early_ptr;
 	bitmap_zero(clustermap, NUM_APIC_CLUSTERS);
 
 	for (i = 0; i < NR_CPUS; i++) {
#12 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 5 lines · Yinghai Lu

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. and apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

Quoted 7 lines

and will get

Marking TSC unstable due to TSCs unsynchronized

even the CPUs have X86_FEATURE_CONSTANT_TSC set.

this patch will check if the cpu is from AMD.

thanks Yinghai, applied.

Ingo

#13 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

In reply to Yinghai Lu
Quoted 4 lines · Yinghai Lu

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. and apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

Ok I see you chose the quick hack over doing it properly ...

Quoted 6 lines

and will get

Marking TSC unstable due to TSCs unsynchronized

even the CPUs have X86_FEATURE_CONSTANT_TSC set.

I doubt that will do the right thing on AMD based vSMP, which also required the cluster check on AMD iirc.

Cc'ed Kiran/Shai. damage has already hit x86 tree I believe.

-Andi

arch/x86/kernel/apic_64.c +8 -1
index d8d03e0..7d8ffda 100644
@@ -1180,9 +1180,19 @@ __cpuinit int apic_is_clustered_box(void)
 {
        int i, clusters, zeros;
        unsigned id;
-       u16 *bios_cpu_apicid = x86_bios_cpu_apicid_early_ptr;
+       u16 *bios_cpu_apicid;
        DECLARE_BITMAP(clustermap, NUM_APIC_CLUSTERS);
+       /*
+        * Some AMD box with quadcore cpu and 8 sockets apicid
+        * will be [4, 0x23] or [8, 0x27] could be thought to
+        * have three apic_clusters. So go out early.
+        */
+       if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+               return 0;
 
author
#14 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 11 lines · Andi Kleen
Quoted 4 lines · Yinghai Lu

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. and apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

Ok I see you chose the quick hack over doing it properly ...

Quoted 3 lines

and will get

Quoted 8 lines
Quoted 3 lines

Marking TSC unstable due to TSCs unsynchronized

even the CPUs have X86_FEATURE_CONSTANT_TSC set.

I doubt that will do the right thing on AMD based vSMP, which also required the cluster check on AMD iirc.

Cc'ed Kiran/Shai. damage has already hit x86 tree I believe.

do you have bootlog for these box?

IBM summit2, Unisys es70000, and system from scalemp..

DMI could tell the difference?

YH

author
#15 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 19 lines · Yinghai Lu
Quoted 18 lines · Andi Kleen
Quoted 4 lines · Yinghai Lu

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. and apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

Ok I see you chose the quick hack over doing it properly ...

Quoted 6 lines

and will get

Marking TSC unstable due to TSCs unsynchronized

even the CPUs have X86_FEATURE_CONSTANT_TSC set.

I doubt that will do the right thing on AMD based vSMP, which also required the cluster check on AMD iirc.

Quoted 7 lines
Quoted 1 line

Cc'ed Kiran/Shai. damage has already hit x86 tree I believe.

do you have bootlog for these box?

IBM summit2, Unisys es70000, and system from scalemp..

DMI could tell the difference?

i produced one patch against linus tree. but it can not be applied to x86/testing

x86/testing has
obj-$(CONFIG_PARAVIRT) += vsmp_64.o

so my question: is the vsmp the real HW or just in paravirt?

YH

----

[PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v3

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. and apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

and will get

Marking TSC unstable due to TSCs unsynchronized

even the CPUs have X86_FEATURE_CONSTANT_TSC set.

this patch will check if the cpu is from AMD.

also vsmp still need that checking...

arch/x86/kernel/Makefile +1 -1
index 4eb5ce8..2bec799 100644
@@ -60,7 +60,7 @@ obj-$(CONFIG_KEXEC)		+= relocate_kernel_$(BITS).o crash.o
 obj-$(CONFIG_CRASH_DUMP)	+= crash_dump_$(BITS).o
 obj-$(CONFIG_X86_NUMAQ)		+= numaq_32.o
 obj-$(CONFIG_X86_SUMMIT_NUMA)	+= summit_32.o
-obj-$(CONFIG_X86_VSMP)		+= vsmp_64.o
+obj-y				+= vsmp_64.o
 obj-$(CONFIG_KPROBES)		+= kprobes.o
 obj-$(CONFIG_MODULES)		+= module_$(BITS).o
 obj-$(CONFIG_ACPI_SRAT) 	+= srat_32.o
arch/x86/kernel/apic_64.c +12 -1
index d8d03e0..1427ec3 100644
@@ -1180,9 +1180,20 @@ __cpuinit int apic_is_clustered_box(void)
 {
 	int i, clusters, zeros;
 	unsigned id;
-	u16 *bios_cpu_apicid = x86_bios_cpu_apicid_early_ptr;
+	u16 *bios_cpu_apicid;
 	DECLARE_BITMAP(clustermap, NUM_APIC_CLUSTERS);
 
+	/*
+	 * there is not this kind of box with AMD CPU yet.
+	 * Some AMD box with quadcore cpu and 8 sockets apicid
+	 * will be [4, 0x23] or [8, 0x27] could be thought to
+	 * have three apic_clusters. So go out early.
+	 * vsmp box still need checking...
+	 */
+	if (!is_vsmp_box() && (boot_cpu_data.x86_vendor == X86_VENDOR_AMD))
+		return 0;
+
+	bios_cpu_apicid = x86_bios_cpu_apicid_early_ptr;
 	bitmap_zero(clustermap, NUM_APIC_CLUSTERS);
 
 	for (i = 0; i < NR_CPUS; i++) {
arch/x86/kernel/vsmp_64.c +23 -6
index d971210..2780df1 100644
@@ -16,19 +16,35 @@
 #include <asm/pci-direct.h>
 #include <asm/io.h>
 
+static int vsmp = -1;
+
+__cpuinit int is_vsmp_box(void)
+{
+	if (vsmp != -1)
+		return vsmp;
+
+	vsmp = 0;
+	if (!early_pci_allowed())
+		return vsmp;
+
+	/* Check if we are running on a ScaleMP vSMP box */
+	if (read_pci_config(0, 0x1f, 0, PCI_VENDOR_ID) ==
+	     (PCI_VENDOR_ID_SCALEMP || (PCI_DEVICE_ID_SCALEMP_VSMP_CTL << 16)))
+		vsmp = 1;
+
+	return vsmp;
+}
+
+#ifdef CONFIG_X86_VSMP
 static int __init vsmp_init(void)
 {
 	void *address;
 	unsigned int cap, ctl;
 
-	if (!early_pci_allowed())
+	if (!vsmp)
 		return 0;
 
-	/* Check if we are running on a ScaleMP vSMP box */
-	if ((read_pci_config_16(0, 0x1f, 0, PCI_VENDOR_ID) !=
-	     PCI_VENDOR_ID_SCALEMP) ||
-	    (read_pci_config_16(0, 0x1f, 0, PCI_DEVICE_ID) !=
-	     PCI_DEVICE_ID_SCALEMP_VSMP_CTL))
+	if (!early_pci_allowed())
 		return 0;
 
 	/* set vSMP magic bits to indicate vSMP capable kernel */
@@ -50,3 +66,4 @@ static int __init vsmp_init(void)
 }
 
 core_initcall(vsmp_init);
+#endif
include/asm-x86/apic.h +1 -0
index bcfc07f..d69f596 100644
@@ -130,6 +130,7 @@ extern u8 setup_APIC_eilvt_mce(u8 vector, u8 msg_type, u8 mask);
 extern u8 setup_APIC_eilvt_ibs(u8 vector, u8 msg_type, u8 mask);
 
 extern int apic_is_clustered_box(void);
+extern int is_vsmp_box(void);
 
 #else /* !CONFIG_X86_LOCAL_APIC */
 static inline void lapic_shutdown(void) { }
author
#16 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

In reply to Yinghai Lu

please check the fix for v2.

this one can be applied to x86.git#testing

YH

---
[PATCH] x86_64: apic_is_clustered_box fix for vsmp

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. and apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

and will get

Marking TSC unstable due to TSCs unsynchronized

even the CPUs have X86_FEATURE_CONSTANT_TSC set.

v2 patch will check if the cpu is from AMD.

vsmp still need that checking...

this patch is fix for vsmp

arch/x86/kernel/apic_64.c +2 -2
index 8f6c45e..8a47579 100644
@@ -1206,9 +1206,9 @@ __cpuinit int apic_is_clustered_box(void)
 	 * there is not this kind of box with AMD CPU yet.
 	 * Some AMD box with quadcore cpu and 8 sockets apicid
 	 * will be [4, 0x23] or [8, 0x27] could be thought to
-	 * have three apic_clusters. So go out early.
+	 * vsmp box still need checking...
 	 */
-	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+	if (!is_vsmp_box() && (boot_cpu_data.x86_vendor == X86_VENDOR_AMD))
 		return 0;
 
 	bios_cpu_apicid = x86_bios_cpu_apicid_early_ptr;
arch/x86/kernel/vsmp_64.c +21 -6
index 54202b1..09e16ff 100644
@@ -72,19 +72,34 @@ static unsigned __init vsmp_patch(u8 type, u16 clobbers, void *ibuf,
 
 }
 
+static int vsmp = -1;
+
+int is_vsmp_box(void)
+{
+	if (vsmp != -1)
+		return vsmp;
+
+	vsmp = 0;
+	if (!early_pci_allowed())
+		return vsmp;
+
+	/* Check if we are running on a ScaleMP vSMP box */
+	if (read_pci_config(0, 0x1f, 0, PCI_VENDOR_ID) ==
+	     (PCI_VENDOR_ID_SCALEMP || (PCI_DEVICE_ID_SCALEMP_VSMP_CTL << 16)))
+		vsmp = 1;
+
+	return vsmp;
+}
+
 void __init vsmp_init(void)
 {
 	void *address;
 	unsigned int cap, ctl, cfg;
 
-	if (!early_pci_allowed())
+	if (!vsmp)
 		return;
 
-	/* Check if we are running on a ScaleMP vSMP box */
-	if ((read_pci_config_16(0, 0x1f, 0, PCI_VENDOR_ID) !=
-	     PCI_VENDOR_ID_SCALEMP) ||
-	    (read_pci_config_16(0, 0x1f, 0, PCI_DEVICE_ID) !=
-	     PCI_DEVICE_ID_SCALEMP_VSMP_CTL))
+	if (!early_pci_allowed())
 		return;
 
 	/* If we are, use the distinguished irq functions */
include/asm-x86/apic.h +5 -0
index a68dc6b..24f7a05 100644
@@ -51,12 +51,17 @@ extern unsigned boot_cpu_id;
  */
 #ifdef CONFIG_PARAVIRT
 #include <asm/paravirt.h>
+extern int is_vsmp_box(void);
 #else
 #define apic_write native_apic_write
 #define apic_write_atomic native_apic_write_atomic
 #define apic_read native_apic_read
 #define setup_boot_clock setup_boot_APIC_clock
 #define setup_secondary_clock setup_secondary_APIC_clock
+int inline is_vsmp_box(void)
+{
+	return 0;
+}
 #endif
 
 static inline void native_apic_write(unsigned long reg, u32 v)
author
#17 lore raw

PATCH 1/1 x86_64: for apic_is_clustered_box for vsmp v2

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. and apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

and will get

Marking TSC unstable due to TSCs unsynchronized

even the CPUs have X86_FEATURE_CONSTANT_TSC set.

quick fixwill check if the cpu is from AMD.

but vsmp still need that checking...

this patch is fix to make sure that vsmp not to be passed.

and need to be applied after
x86_64: make amd quad core 8 socket system not be clustered_box v2
in x86/testing

arch/x86/kernel/apic_64.c +2 -2
Index: linux-2.6/arch/x86/kernel/apic_64.c
===================================================================
@@ -1206,9 +1206,9 @@ __cpuinit int apic_is_clustered_box(void
 	 * there is not this kind of box with AMD CPU yet.
 	 * Some AMD box with quadcore cpu and 8 sockets apicid
 	 * will be [4, 0x23] or [8, 0x27] could be thought to
-	 * have three apic_clusters. So go out early.
+	 * vsmp box still need checking...
 	 */
-	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+	if (!is_vsmp_box() && (boot_cpu_data.x86_vendor == X86_VENDOR_AMD))
 		return 0;
 
 	bios_cpu_apicid = x86_bios_cpu_apicid_early_ptr;
arch/x86/kernel/vsmp_64.c +21 -6
Index: linux-2.6/arch/x86/kernel/vsmp_64.c
===================================================================
@@ -72,19 +72,34 @@ static unsigned __init vsmp_patch(u8 typ
 
 }
 
+static int vsmp = -1;
+
+int is_vsmp_box(void)
+{
+	if (vsmp != -1)
+		return vsmp;
+
+	vsmp = 0;
+	if (!early_pci_allowed())
+		return vsmp;
+
+	/* Check if we are running on a ScaleMP vSMP box */
+	if (read_pci_config(0, 0x1f, 0, PCI_VENDOR_ID) ==
+	     (PCI_VENDOR_ID_SCALEMP || (PCI_DEVICE_ID_SCALEMP_VSMP_CTL << 16)))
+		vsmp = 1;
+
+	return vsmp;
+}
+
 void __init vsmp_init(void)
 {
 	void *address;
 	unsigned int cap, ctl, cfg;
 
-	if (!early_pci_allowed())
+	if (!is_vsmp_box())
 		return;
 
-	/* Check if we are running on a ScaleMP vSMP box */
-	if ((read_pci_config_16(0, 0x1f, 0, PCI_VENDOR_ID) !=
-	     PCI_VENDOR_ID_SCALEMP) ||
-	    (read_pci_config_16(0, 0x1f, 0, PCI_DEVICE_ID) !=
-	     PCI_DEVICE_ID_SCALEMP_VSMP_CTL))
+	if (!early_pci_allowed())
 		return;
 
 	/* If we are, use the distinguished irq functions */
include/asm-x86/apic.h +5 -0
Index: linux-2.6/include/asm-x86/apic.h
===================================================================
@@ -51,12 +51,17 @@ extern unsigned boot_cpu_id;
  */
 #ifdef CONFIG_PARAVIRT
 #include <asm/paravirt.h>
+extern int is_vsmp_box(void);
 #else
 #define apic_write native_apic_write
 #define apic_write_atomic native_apic_write_atomic
 #define apic_read native_apic_read
 #define setup_boot_clock setup_boot_APIC_clock
 #define setup_secondary_clock setup_secondary_APIC_clock
+static int inline is_vsmp_box(void)
+{
+	return 0;
+}
 #endif
 
 static inline void native_apic_write(unsigned long reg, u32 v)
author
#18 lore raw

PATCH 1/1 x86: vSMP selection in config

find out vSMP setting is going away in config after make oldconfig

vSMP need to PARAVIRT and PCI.
so move PARAVIRT out of if PARAVIRT_GUEST, and make vSMP select PCI instead of depends on PCI

after patch vSMP could stick there.

arch/x86/Kconfig +4 -3
Index: linux-2.6/arch/x86/Kconfig
===================================================================
@@ -330,8 +330,9 @@ config X86_RDC321X
 
 config X86_VSMP
 	bool "Support for ScaleMP vSMP"
-	depends on X86_64 && PCI
+	depends on X86_64
 	select PARAVIRT
+	select PCI
 	help
 	  Support for ScaleMP vSMP systems.  Say 'Y' here if this kernel is
 	  supposed to run on these EM64T-based machines.  Only choose this option
@@ -376,6 +377,8 @@ config VMI
 
 source "arch/x86/lguest/Kconfig"
 
+endif
+
 config PARAVIRT
 	bool "Enable paravirtualization code"
 	depends on !(X86_VISWS || X86_VOYAGER)
@@ -385,8 +388,6 @@ config PARAVIRT
 	  over full virtualization.  However, when run without a hypervisor
 	  the kernel is theoretically slower and slightly larger.
 
-endif
-
 config ACPI_SRAT
 	def_bool y
 	depends on X86_32 && ACPI && NUMA && (X86_SUMMIT || X86_GENERICARCH)
#19 lore raw

Kconfig configuration restore bug [Was: x86: vSMP selection in config]

Hi Roman.

We discovered a situation where we could set a choice value in menuconfig but later when we either was running menuconfig or oldconfig the value were changed.

I have created a minimal config that exhibit the error. It was created in a pure mechanical trial-and-error fashion.

First the minimal Kconfig file:

# x86 configuration

choice
prompt "Subarchitecture Type"

config X86_PC
bool "PC-compatible"

config X86_VOYAGER
bool "Voyager (NCR)"

config X86_VSMP
bool "Support for ScaleMP vSMP"
depends on PCI

endchoice

config PCI
bool "PCI support" if !X86_VISWS
depends on !X86_VOYAGER
default y

config USB_ARCH_HAS_HCD
bool
default PCI

config USB
bool "Support for Host-side USB"
depends on USB_ARCH_HAS_HCD

config USB_PHIDGET
bool "USB Phidgets drivers"
depends on USB

config USB_PHIDGETMOTORCONTROL
bool "USB PhidgetMotorControl support"
depends on USB_PHIDGET

Next the saved .config that is used:
#
# Automatically generated make config: don't edit
# Linux kernel version: KERNELVERSION
# Tue Feb 26 20:27:09 2008
#
# CONFIG_X86_PC is not set
# CONFIG_X86_VOYAGER is not set
CONFIG_X86_VSMP=y
CONFIG_PCI=y
CONFIG_USB_ARCH_HAS_HCD=y
CONFIG_USB=y
# CONFIG_USB_PHIDGET is not set

When we enter menuconfig or are running oldconfig then we can see that CONFIG_X86_PC is set to 'y' and CONFIG_X86_VSMP is set to 'n'.

If I in menuconfig select VSMP this setting is saved but then oldconfig kicks in and we loose the setting again.

If I delete any of the config variables in the sample above then we no longer change the values and we keep the VSMP equals 'y'.

Can you please take a look at this.

Thanks,
Sam

#20 lore raw

Re: Kconfig configuration restore bug [Was: x86: vSMP selection in config]

Hi,

Quoted 13 lines · Sam Ravnborg

choice
prompt "Subarchitecture Type"

config X86_PC
bool "PC-compatible"

config X86_VOYAGER
bool "Voyager (NCR)"

config X86_VSMP
bool "Support for ScaleMP vSMP"
depends on PCI

Quoted 6 lines

endchoice

config PCI
bool "PCI support" if !X86_VISWS
depends on !X86_VOYAGER
default y

The basic problem is that this is a recursive dependency - PCI depends on the choice and the choice depends on PCI. IMO X86_VSMP cannot depend on PCI.
I'm looking into why this hasn't been picked up by the dependency check...

bye, Roman

#21 lore raw

PATCH 1/3 fix recursive dependencies

In reply to Sam Ravnborg

Hi,

Quoted 3 lines · Sam Ravnborg

We discovered a situation where we could set a choice value in menuconfig but later when we either was running menuconfig or oldconfig the value were changed.

The patch fixes these dependency problems.

bye, Roman

The proper dependency check uncovered a few dependency problems, the subarchitecture used a mixture of selects and depends on SMP and PCI dependency was messed up.

---
 arch/x86/Kconfig |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
arch/x86/Kconfig +4 -6
Index: linux-2.6/arch/x86/Kconfig
===================================================================
@@ -243,8 +243,7 @@ config X86_ELAN
 
 config X86_VOYAGER
 	bool "Voyager (NCR)"
-	depends on X86_32
-	select SMP if !BROKEN
+	depends on X86_32 && (SMP || BROKEN)
 	help
 	  Voyager is an MCA-based 32-way capable SMP architecture proprietary
 	  to NCR Corp.  Machine classes 345x/35xx/4100/51xx are Voyager-based.
@@ -256,9 +255,8 @@ config X86_VOYAGER
 
 config X86_NUMAQ
 	bool "NUMAQ (IBM/Sequent)"
-	select SMP
+	depends on SMP && X86_32
 	select NUMA
-	depends on X86_32
 	help
 	  This option is used for getting Linux to run on a (IBM/Sequent) NUMA
 	  multiquad box. This changes the way that processors are bootstrapped,
@@ -329,7 +327,7 @@ config X86_RDC321X
 
 config X86_VSMP
 	bool "Support for ScaleMP vSMP"
-	depends on X86_64 && PCI
+	depends on X86_64
 	 help
 	  Support for ScaleMP vSMP systems.  Say 'Y' here if this kernel is
 	  supposed to run on these EM64T-based machines.  Only choose this option
@@ -1381,7 +1379,7 @@ endmenu
 menu "Bus options (PCI etc.)"
 
 config PCI
-	bool "PCI support" if !X86_VISWS
+	bool "PCI support" if !X86_VISWS && !X86_VSMP
 	depends on !X86_VOYAGER
 	default y
 	select ARCH_SUPPORTS_MSI if (X86_LOCAL_APIC && X86_IO_APIC)
#22 lore raw

Re: [PATCH 1/3] fix recursive dependencies

Quoted 13 lines · Roman Zippel

Hi,

Quoted 3 lines · Sam Ravnborg

We discovered a situation where we could set a choice value in menuconfig but later when we either was running menuconfig or oldconfig the value were changed.

The patch fixes these dependency problems.

bye, Roman

Quoted 7 lines

The proper dependency check uncovered a few dependency problems, the subarchitecture used a mixture of selects and depends on SMP and PCI dependency was messed up.

---

Ingo,

please drop
x86: PARAVIRT needed by PARAVIRT_GUEST or X86_VSMP
x86: vSMP selection in config
in x86.git#testing to use Roman's three patches.

Thanks

Yinghai Lu

#23 lore raw

Re: [PATCH 1/3] fix recursive dependencies

Hi,

Quoted 2 lines · Yinghai Lu

x86: PARAVIRT needed by PARAVIRT_GUEST or X86_VSMP
x86: vSMP selection in config

Sorry, I hadn't seen there already was a followup.

Quoted 1 line

in x86.git#testing to use Roman's three patches.

Only the first patch needs merging, the other two can wait a little.

bye, Roman

#24 lore raw

Re: [PATCH 1/3] fix recursive dependencies

Quoted 4 lines · Roman Zippel

Hi,

On Thu, 28 Feb 2008, Yinghai Lu wrote:

Quoted 8 lines
Quoted 2 lines

x86: PARAVIRT needed by PARAVIRT_GUEST or X86_VSMP
x86: vSMP selection in config

Sorry, I hadn't seen there already was a followup.

Quoted 1 line

in x86.git#testing to use Roman's three patches.

Only the first patch needs merging, the other two can wait a little.

Agreed. I will take the latter two in kbuild.git.
The first one belong in x86.git.

Sam

#25 lore raw

Re: [PATCH 1/3] fix recursive dependencies

Quoted 5 lines · Sam Ravnborg
Quoted 3 lines
Quoted 1 line

in x86.git#testing to use Roman's three patches.

Only the first patch needs merging, the other two can wait a little.

Agreed. I will take the latter two in kbuild.git.
The first one belong in x86.git.

i've picked up Roman's fix for 2.6.25 merging.

Ingo

#26 lore raw

Re: [PATCH 1/3] fix recursive dependencies

In reply to Roman Zippel
Quoted 3 lines · Roman Zippel

The proper dependency check uncovered a few dependency problems, the subarchitecture used a mixture of selects and depends on SMP and PCI dependency was messed up.

thanks Roman, applied.

Ingo

#27 lore raw

PATCH 2/3 fix choice dependency check

In reply to Sam Ravnborg

Properly check the dependency of choices as a group.
Also fix that sym_check_deps() correctly terminates the dependency loop error check (otherwise it would continue printing the dependency chain).

---
 scripts/kconfig/symbol.c |   94 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 76 insertions(+), 18 deletions(-)
scripts/kconfig/symbol.c +76 -18
Index: linux-2.6/scripts/kconfig/symbol.c
===================================================================
@@ -762,8 +762,6 @@ struct symbol **sym_re_search(const char
 }
 
 
-struct symbol *sym_check_deps(struct symbol *sym);
-
 static struct symbol *sym_check_expr_deps(struct expr *e)
 {
 	struct symbol *sym;
@@ -795,40 +793,100 @@ static struct symbol *sym_check_expr_dep
 }
 
 /* return NULL when dependencies are OK */
-struct symbol *sym_check_deps(struct symbol *sym)
+static struct symbol *sym_check_sym_deps(struct symbol *sym)
 {
 	struct symbol *sym2;
 	struct property *prop;
 
-	if (sym->flags & SYMBOL_CHECK) {
-		fprintf(stderr, "%s:%d:error: found recursive dependency: %s",
-		        sym->prop->file->name, sym->prop->lineno, sym->name);
-		return sym;
-	}
-	if (sym->flags & SYMBOL_CHECKED)
-		return NULL;
-
-	sym->flags |= (SYMBOL_CHECK | SYMBOL_CHECKED);
 	sym2 = sym_check_expr_deps(sym->rev_dep.expr);
 	if (sym2)
-		goto out;
+		return sym2;
 
 	for (prop = sym->prop; prop; prop = prop->next) {
 		if (prop->type == P_CHOICE || prop->type == P_SELECT)
 			continue;
 		sym2 = sym_check_expr_deps(prop->visible.expr);
 		if (sym2)
-			goto out;
+			break;
 		if (prop->type != P_DEFAULT || sym_is_choice(sym))
 			continue;
 		sym2 = sym_check_expr_deps(prop->expr);
 		if (sym2)
-			goto out;
+			break;
 	}
-out:
+
+	return sym2;
+}
+
+static struct symbol *sym_check_choice_deps(struct symbol *choice)
+{
+	struct symbol *sym, *sym2;
+	struct property *prop;
+	struct expr *e;
+
+	prop = sym_get_choice_prop(choice);
+	expr_list_for_each_sym(prop->expr, e, sym)
+		sym->flags |= (SYMBOL_CHECK | SYMBOL_CHECKED);
+
+	choice->flags |= (SYMBOL_CHECK | SYMBOL_CHECKED);
+	sym2 = sym_check_sym_deps(choice);
+	choice->flags &= ~SYMBOL_CHECK;
 	if (sym2)
-		fprintf(stderr, " -> %s%s", sym->name, sym2 == sym? "\n": "");
-	sym->flags &= ~SYMBOL_CHECK;
+		goto out;
+
+	expr_list_for_each_sym(prop->expr, e, sym) {
+		sym2 = sym_check_sym_deps(sym);
+		if (sym2) {
+			fprintf(stderr, " -> %s", sym->name);
+			break;
+		}
+	}
+out:
+	expr_list_for_each_sym(prop->expr, e, sym)
+		sym->flags &= ~SYMBOL_CHECK;
+
+	if (sym2 && sym_is_choice_value(sym2) &&
+	    prop_get_symbol(sym_get_choice_prop(sym2)) == choice)
+		sym2 = choice;
+
+	return sym2;
+}
+
+struct symbol *sym_check_deps(struct symbol *sym)
+{
+	struct symbol *sym2;
+	struct property *prop;
+
+	if (sym->flags & SYMBOL_CHECK) {
+		fprintf(stderr, "%s:%d:error: found recursive dependency: %s",
+		        sym->prop->file->name, sym->prop->lineno,
+			sym->name ? sym->name : "<choice>");
+		return sym;
+	}
+	if (sym->flags & SYMBOL_CHECKED)
+		return NULL;
+
+	if (sym_is_choice_value(sym)) {
+		/* for choice groups start the check with main choice symbol */
+		prop = sym_get_choice_prop(sym);
+		sym2 = sym_check_deps(prop_get_symbol(prop));
+	} else if (sym_is_choice(sym)) {
+		sym2 = sym_check_choice_deps(sym);
+	} else {
+		sym->flags |= (SYMBOL_CHECK | SYMBOL_CHECKED);
+		sym2 = sym_check_sym_deps(sym);
+		sym->flags &= ~SYMBOL_CHECK;
+	}
+
+	if (sym2) {
+		fprintf(stderr, " -> %s", sym->name ? sym->name : "<choice>");
+		if (sym2 == sym) {
+			fprintf(stderr, "\n");
+			zconfnerrs++;
+			sym2 = NULL;
+		}
+	}
+
 	return sym2;
 }
 
#28 lore raw

Re: [PATCH 2/3] fix choice dependency check

Quoted 6 lines · Roman Zippel

Properly check the dependency of choices as a group.
Also fix that sym_check_deps() correctly terminates the dependency loop error check (otherwise it would continue printing the dependency chain).

Hi Roman.

I realised this was never applied - so I did so now. Same with the one with named choice groups.

Sam

#29 lore raw

PATCH 3/3 add named choice group

In reply to Sam Ravnborg

As choice dependency are now fully checked, it's quite easy to add support for named choices. This lifts the restriction that a choice value can only appear once, although it still has to be within the same group, but multiple choices can be joined by giving them a name. While at it I cleaned up a little the choice type logic to simplify it a bit.

---
 scripts/kconfig/lex.zconf.c_shipped |   25 --
 scripts/kconfig/lkc_proto.h         |    2 
 scripts/kconfig/menu.c              |   64 +++----
 scripts/kconfig/symbol.c            |   24 +-
 scripts/kconfig/zconf.tab.c_shipped |  301 ++++++++++++++++++------------------
 scripts/kconfig/zconf.y             |   13 -
 6 files changed, 206 insertions(+), 223 deletions(-)
scripts/kconfig/lex.zconf.c_shipped +3 -22
Index: linux-2.6/scripts/kconfig/lex.zconf.c_shipped
===================================================================
@@ -5,25 +5,6 @@
 
 /* A lexical scanner generated by flex */
 
-#define yy_create_buffer zconf_create_buffer
-#define yy_delete_buffer zconf_delete_buffer
-#define yy_flex_debug zconf_flex_debug
-#define yy_init_buffer zconf_init_buffer
-#define yy_flush_buffer zconf_flush_buffer
-#define yy_load_buffer_state zconf_load_buffer_state
-#define yy_switch_to_buffer zconf_switch_to_buffer
-#define yyin zconfin
-#define yyleng zconfleng
-#define yylex zconflex
-#define yylineno zconflineno
-#define yyout zconfout
-#define yyrestart zconfrestart
-#define yytext zconftext
-#define yywrap zconfwrap
-#define yyalloc zconfalloc
-#define yyrealloc zconfrealloc
-#define yyfree zconffree
-
 #define FLEX_SCANNER
 #define YY_FLEX_MAJOR_VERSION 2
 #define YY_FLEX_MINOR_VERSION 5
@@ -354,7 +335,7 @@ void zconffree (void *  );
 
 /* Begin user sect3 */
 
-#define zconfwrap(n) 1
+#define zconfwrap() 1
 #define YY_SKIP_YYWRAP
 
 typedef unsigned char YY_CHAR;
@@ -1535,7 +1516,7 @@ static int yy_get_next_buffer (void)
 
 		/* Read in more data. */
 		YY_INPUT( (&YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[number_to_move]),
-			(yy_n_chars), num_to_read );
+			(yy_n_chars), (size_t) num_to_read );
 
 		YY_CURRENT_BUFFER_LVALUE->yy_n_chars = (yy_n_chars);
 		}
@@ -2007,7 +1988,7 @@ YY_BUFFER_STATE zconf_scan_buffer  (char
 
 /** Setup the input buffer state to scan a string. The next call to zconflex() will
  * scan from a @e copy of @a str.
- * @param str a NUL-terminated string to scan
+ * @param yystr a NUL-terminated string to scan
  * 
  * @return the newly allocated buffer state object.
  * @note If you want to scan bytes that may contain NUL values, then use
scripts/kconfig/lkc_proto.h +1 -1
Index: linux-2.6/scripts/kconfig/lkc_proto.h
===================================================================
@@ -21,7 +21,7 @@ P(menu_get_help,const char *,(struct men
 /* symbol.c */
 P(symbol_hash,struct symbol *,[SYMBOL_HASHSIZE]);
 
-P(sym_lookup,struct symbol *,(const char *name, int isconst));
+P(sym_lookup,struct symbol *,(const char *name, int flags));
 P(sym_find,struct symbol *,(const char *name));
 P(sym_re_search,struct symbol **,(const char *pattern));
 P(sym_type_name,const char *,(enum symbol_type type));
scripts/kconfig/menu.c +27 -37

95 lines not on this page · raw message

scripts/kconfig/symbol.c +11 -13
Index: linux-2.6/scripts/kconfig/symbol.c
===================================================================
@@ -40,7 +40,7 @@ void sym_add_default(struct symbol *sym,
 {
 	struct property *prop = prop_alloc(P_DEFAULT, sym);
 
-	prop->expr = expr_alloc_symbol(sym_lookup(def, 1));
+	prop->expr = expr_alloc_symbol(sym_lookup(def, SYMBOL_CONST));
 }
 
 void sym_init(void)
@@ -350,9 +350,6 @@ void sym_calc_value(struct symbol *sym)
 		;
 	}
 
-	if (sym->flags & SYMBOL_AUTO)
-		sym->flags &= ~SYMBOL_WRITE;
-
 	sym->curr = newval;
 	if (sym_is_choice(sym) && newval.tri == yes)
 		sym->curr.val = sym_calc_choice(sym);
@@ -377,6 +374,9 @@ void sym_calc_value(struct symbol *sym)
 				sym_set_changed(choice_sym);
 		}
 	}
+
+	if (sym->flags & SYMBOL_AUTO)
+		sym->flags &= ~SYMBOL_WRITE;
 }
 
 void sym_clear_all_valid(void)
@@ -651,7 +651,7 @@ bool sym_is_changable(struct symbol *sym
 	return sym->visible > sym->rev_dep.tri;
 }
 
-struct symbol *sym_lookup(const char *name, int isconst)
+struct symbol *sym_lookup(const char *name, int flags)
 {
 	struct symbol *symbol;
 	const char *ptr;
@@ -671,11 +671,10 @@ struct symbol *sym_lookup(const char *na
 		hash &= 0xff;
 
 		for (symbol = symbol_hash[hash]; symbol; symbol = symbol->next) {
-			if (!strcmp(symbol->name, name)) {
-				if ((isconst && symbol->flags & SYMBOL_CONST) ||
-				    (!isconst && !(symbol->flags & SYMBOL_CONST)))
-					return symbol;
-			}
+			if (!strcmp(symbol->name, name) &&
+			    (flags ? symbol->flags & flags
+				   : !(symbol->flags & (SYMBOL_CONST|SYMBOL_CHOICE))))
+				return symbol;
 		}
 		new_name = strdup(name);
 	} else {
@@ -687,8 +686,7 @@ struct symbol *sym_lookup(const char *na
 	memset(symbol, 0, sizeof(*symbol));
 	symbol->name = new_name;
 	symbol->type = S_UNKNOWN;
-	if (isconst)
-		symbol->flags |= SYMBOL_CONST;
+	symbol->flags |= flags;
 
 	symbol->next = symbol_hash[hash];
 	symbol_hash[hash] = symbol;
@@ -962,7 +960,7 @@ void prop_add_env(const char *env)
 	}
 
 	prop = prop_alloc(P_ENV, sym);
-	prop->expr = expr_alloc_symbol(sym_lookup(env, 1));
+	prop->expr = expr_alloc_symbol(sym_lookup(env, SYMBOL_CONST));
 
 	sym_env_list = expr_alloc_one(E_LIST, sym_env_list);
 	sym_env_list->right.sym = sym;
scripts/kconfig/zconf.tab.c_shipped +156 -145

408 lines not on this page · raw message

scripts/kconfig/zconf.y +8 -5
Index: linux-2.6/scripts/kconfig/zconf.y
===================================================================
@@ -91,7 +91,7 @@ static struct menu *current_menu, *curre
 %type <id> end
 %type <id> option_name
 %type <menu> if_entry menu_entry choice_entry
-%type <string> symbol_option_arg
+%type <string> symbol_option_arg word_opt
 
 %destructor {
 	fprintf(stderr, "%s:%d: missing end statement for this entry\n",
@@ -239,10 +239,10 @@ symbol_option_arg:
 
 /* choice entry */
 
-choice: T_CHOICE T_EOL
+choice: T_CHOICE word_opt T_EOL
 {
-	struct symbol *sym = sym_lookup(NULL, 0);
-	sym->flags |= SYMBOL_CHOICE;
+	struct symbol *sym = sym_lookup($2, SYMBOL_CHOICE);
+	sym->flags |= SYMBOL_AUTO;
 	menu_add_entry(sym);
 	menu_add_expr(P_CHOICE, NULL, NULL);
 	printd(DEBUG_PARSE, "%s:%d:choice\n", zconf_curname(), zconf_lineno());
@@ -456,9 +456,12 @@ expr:	  symbol				{ $$ = expr_alloc_symb
 ;
 
 symbol:	  T_WORD	{ $$ = sym_lookup($1, 0); free($1); }
-	| T_WORD_QUOTE	{ $$ = sym_lookup($1, 1); free($1); }
+	| T_WORD_QUOTE	{ $$ = sym_lookup($1, SYMBOL_CONST); free($1); }
 ;
 
+word_opt: /* empty */			{ $$ = NULL; }
+	| T_WORD
+
 %%
 
 void conf_parse(const char *name)
#30 lore raw

Re: [PATCH] x86: vSMP selection in config

In reply to Yinghai Lu
Quoted 8 lines · Yinghai Lu

find out vSMP setting is going away in config after make oldconfig

vSMP need to PARAVIRT and PCI.
so move PARAVIRT out of if PARAVIRT_GUEST, and make vSMP select PCI instead of depends on PCI

after patch vSMP could stick there.

I'm certain that we have hit a Kconfig bug / limitation here and the patch below is just a workaround for that issue.

I tracked it down to a minimal case and hope Roman can provide either an explanation or a fix.

IMO VSMP can wait until this is resolved properly and we should not apply this patch.

Sam

author
#31 lore raw

Re: [PATCH] x86: vSMP selection in config

Quoted 10 lines · Sam Ravnborg
Quoted 8 lines · Yinghai Lu

find out vSMP setting is going away in config after make oldconfig

vSMP need to PARAVIRT and PCI.
so move PARAVIRT out of if PARAVIRT_GUEST, and make vSMP select PCI instead of depends on PCI

after patch vSMP could stick there.

Quoted 8 lines

I'm certain that we have hit a Kconfig bug / limitation here and the patch below is just a workaround for that issue.

I tracked it down to a minimal case and hope Roman can provide either an explanation or a fix.

IMO VSMP can wait until this is resolved properly and we should not apply this patch.

also Kconfig seems ignore
if PARAVIRT_GUEST

endif

and
to use config PARAVIRT in that if block.

YH

#32 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

In reply to Andi Kleen
Quoted 7 lines · Andi Kleen
Quoted 4 lines · Yinghai Lu

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. and apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

Ok I see you chose the quick hack over doing it properly ...

you didn't answer my question:

for IBM Summit2, Unisys ES7000, and ScaleMP vSMP,
if call cpus sockets are fully populated with quadcore cpus, do they still have hole between apic id?

YH

#33 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

In reply to Andi Kleen
Quoted 9 lines · Andi Kleen
Quoted 4 lines · Yinghai Lu

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. and apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

Ok I see you chose the quick hack over doing it properly ...

Quoted 1 line
Quoted 9 lines
Quoted 5 lines

and will get

Marking TSC unstable due to TSCs unsynchronized

even the CPUs have X86_FEATURE_CONSTANT_TSC set.

I doubt that will do the right thing on AMD based vSMP, which also required the cluster check on AMD iirc.

Andi, Yes. AMD based vSMPowered systems uses clustered APICs, and this check base on cpu vendor id is not good :(.

Thanks,
Kiran

#34 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 14 lines · Ravikiran Thirumalai
Quoted 13 lines · Andi Kleen
Quoted 4 lines · Yinghai Lu

quad core 8 socket system will have apic id lifting.the apic id range could be [4, 0x23]. and apic_is_clustered_box will think that need to three clusters and that is large than 2. So it is treated as clustered_box.

Ok I see you chose the quick hack over doing it properly ...

Quoted 5 lines

and will get

Marking TSC unstable due to TSCs unsynchronized

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

even the CPUs have X86_FEATURE_CONSTANT_TSC set.

I doubt that will do the right thing on AMD based vSMP, which also required the cluster check on AMD iirc.

Andi, Yes. AMD based vSMPowered systems uses clustered APICs, and this check base on cpu vendor id is not good :(.

please check if you happy with

http://lkml.org/lkml/2008/2/24/273

Thanks

Yinghai Lu

#35 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 10 lines · Yinghai Lu
Quoted 3 lines · Ravikiran Thirumalai

...
Andi, Yes. AMD based vSMPowered systems uses clustered APICs, and this check base on cpu vendor id is not good :(.

please check if you happy with

http://lkml.org/lkml/2008/2/24/273

I don't quite understand the purpose of the patch to begin with. Looking at the current x86 git tree, apic_is_clustered_box is used only to determine if tsc is synchronized on the platform. The AMD docs imply that TSC's are not guaranteed to be synced across cores between nodes -- Opteron BKDG for family 10h, Section 2.9.4:

<quote>
Note: Timers associated with different CPU cores in the same processor increment at the same rate. Timers associated with different CPU cores in different processors increment at slightly different rates if (1) they are located on different nodes and (2) CLKIN for these nodes is derived from different, non-synchronized oscillator sources.
</quote>

But that is not what the x86 tree does (with your patches) -- it looks for the X86_FEATURE_CONSTANT_TSC at unsynchronized_tsc() and assumes a synchronized clock. Huh!?? Am i missing something here? X86_FEATURE_CONSTANT_TSC is set from CPUID Fn8000_0007 -- TscInvariant bit, which implies TSC is not affected by change in PM states. This does not talk about whether CLKIN for different packages are from synchronized/non synchronized oscillator sources in the above quote.

Thanks,
Kiran

#36 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 5 lines

I don't quite understand the purpose of the patch to begin with. Looking at the current x86 git tree, apic_is_clustered_box is used only to determine if tsc is synchronized on the platform. The AMD docs imply that TSC's are not guaranteed to be synced across cores between nodes -- Opteron BKDG for family 10h, Section 2.9.4:

After long discussions with AMD they determined the CPUID flag for sync RDTSC will imply synchronization between nodes.

If you can't support that in your hardware you're supposed to clear it.

-Andi

#37 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 8 lines · Andi Kleen
Quoted 5 lines

I don't quite understand the purpose of the patch to begin with. Looking at the current x86 git tree, apic_is_clustered_box is used only to determine if tsc is synchronized on the platform. The AMD docs imply that TSC's are not guaranteed to be synced across cores between nodes -- Opteron BKDG for family 10h, Section 2.9.4:

After long discussions with AMD they determined the CPUID flag for sync RDTSC will imply synchronization between nodes.

Ah!

Quoted 3 lines

If you can't support that in your hardware you're supposed to clear it.

Hmm! How would a hardware vendor do that? That doesn't seem to be clear in the BKDG. (Well, this is the problem with undocumented features :()

Thanks,
Kiran

#38 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 14 lines · Ravikiran Thirumalai
Quoted 8 lines · Andi Kleen
Quoted 5 lines

I don't quite understand the purpose of the patch to begin with. Looking at the current x86 git tree, apic_is_clustered_box is used only to determine if tsc is synchronized on the platform. The AMD docs imply that TSC's are not guaranteed to be synced across cores between nodes -- Opteron BKDG for family 10h, Section 2.9.4:

After long discussions with AMD they determined the CPUID flag for sync RDTSC will imply synchronization between nodes.

Ah!

Quoted 1 line
Quoted 6 lines
Quoted 2 lines

If you can't support that in your hardware you're supposed to clear it.

Hmm! How would a hardware vendor do that? That doesn't seem to be clear in the BKDG. (Well, this is the problem with undocumented features :()

any good sign for APIC_clustered box? there is apicid between cpus even all cpu are quadcore and fully populated?

YH

#39 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 4 lines · Yinghai Lu
Quoted 3 lines · Ravikiran Thirumalai
Quoted 1 line · Andi Kleen
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

If you can't support that in your hardware you're supposed to clear it.

Hmm! How would a hardware vendor do that? That doesn't seem to be clear in the BKDG. (Well, this is the problem with undocumented features :()

any good sign for APIC_clustered box? there is apicid between cpus even all cpu are quadcore and fully populated?

I would suggest checking the SLIT distances -- On AMD boxes, if you have three different distances between nodes, then that system would be multiboard, and there is no way TSCs can be synced. On Intel boxes, if there are two different distances between nodes, then this would be a multi board/multi chassi box and TSCs won't be synced. This is a more generic solution and should work on Summit/Unisys boxes as well. (I am ignoring Intel CSI for now. It might need the same treatment as AMD)

Comments?

Kiran

#40 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 15 lines · Ravikiran Thirumalai
Quoted 5 lines · Yinghai Lu
Quoted 3 lines · Ravikiran Thirumalai
Quoted 1 line · Andi Kleen
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

If you can't support that in your hardware you're supposed to clear it.

Hmm! How would a hardware vendor do that? That doesn't seem to be clear in the BKDG. (Well, this is the problem with undocumented features :()

any good sign for APIC_clustered box? there is apicid between cpus even all cpu are quadcore and fully populated?

Quoted 7 lines

I would suggest checking the SLIT distances -- On AMD boxes, if you have three different distances between nodes, then that system would be multiboard, and there is no way TSCs can be synced. On Intel boxes, if there are two different distances between nodes, then this would be a multi board/multi chassi box and TSCs won't be synced. This is a more generic solution and should work on Summit/Unisys boxes as well. (I am ignoring Intel CSI for now. It might need the same treatment as AMD)

1. if acpi=off ?
2. some system will be treated wrong.
my four sockets system
ACPI: SLIT: nodes = 4
10 13 13 16
13 10 16 13
13 16 10 13
16 13 13 10
my eight sockets system
ACPI: SLIT: nodes = 8
10 12 12 14 14 14 14 16
12 10 14 12 14 14 12 14
12 14 10 14 12 12 14 14
14 12 14 10 12 12 14 14
14 14 12 12 10 14 12 14
14 14 12 12 14 10 14 12
14 12 14 14 12 14 10 12
16 14 14 14 14 12 12 10

YH

#41 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 2 lines · Yinghai Lu

1. if acpi=off ?

Well that is not a realistic scenario for any multi chassi NUMA machine, since the proximity information is very important and turning acpi off deprives the OS of this information.

Quoted 8 lines

2. some system will be treated wrong.
my four sockets system
ACPI: SLIT: nodes = 4
10 13 13 16
13 10 16 13
13 16 10 13
16 13 13 10
my eight sockets system

This makes it difficult to use SLIT for multi chassi detection then :(. Did not know such SLIT tables existed on single board machines.

Thanks,
Kiran

#42 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 18 lines · Ravikiran Thirumalai
Quoted 2 lines · Yinghai Lu

1. if acpi=off ?

Well that is not a realistic scenario for any multi chassi NUMA machine, since the proximity information is very important and turning acpi off deprives the OS of this information.

Quoted 8 lines

2. some system will be treated wrong.
my four sockets system
ACPI: SLIT: nodes = 4
10 13 13 16
13 10 16 13
13 16 10 13
16 13 13 10
my eight sockets system

Quoted 2 lines

This makes it difficult to use SLIT for multi chassi detection then :(. Did not know such SLIT tables existed on single board machines.

Yes

YH

#43 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

In reply to Ravikiran Thirumalai
Quoted 7 lines · Ravikiran Thirumalai
Quoted 2 lines · Yinghai Lu

1. if acpi=off ?

Well that is not a realistic scenario for any multi chassi NUMA machine, since the proximity information is very important and turning acpi off deprives the OS of this information.

actually OS should detect the distance instead of rely on SLIT.

YH

#44 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 11 lines · Yinghai Lu
Quoted 7 lines · Ravikiran Thirumalai
Quoted 2 lines · Yinghai Lu

1. if acpi=off ?

Well that is not a realistic scenario for any multi chassi NUMA machine, since the proximity information is very important and turning acpi off deprives the OS of this information.

actually OS should detect the distance instead of rely on SLIT.

How does it do that? ACPI SRAT is needed to determine this, and it wouldn't
work if acpi=off.

#45 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 8 lines · Ravikiran Thirumalai
Quoted 6 lines · Yinghai Lu
Quoted 4 lines · Ravikiran Thirumalai
Quoted 2 lines · Yinghai Lu

1. if acpi=off ?

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

Well that is not a realistic scenario for any multi chassi NUMA machine, since the proximity information is very important and turning acpi off deprives the OS of this information.

actually OS should detect the distance instead of rely on SLIT.

How does it do that? ACPI SRAT is needed to determine this, and it wouldn't work if acpi=off.

srat only have apicid to node mapping, and ram to node mapping.

for amd64 system, when acpi=off
ram to node mapping can be retrieved from pci conf space apicid to node mapping could be calculated too... with boot_cpu_id ...plus coreid bits.

YH

#46 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 13 lines · Yinghai Lu
Quoted 12 lines · Ravikiran Thirumalai
Quoted 10 lines · Yinghai Lu
Quoted 7 lines · Ravikiran Thirumalai
Quoted 2 lines · Yinghai Lu

1. if acpi=off ?

Well that is not a realistic scenario for any multi chassi NUMA machine, since the proximity information is very important and turning acpi off deprives the OS of this information.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

actually OS should detect the distance instead of rely on SLIT.

How does it do that? ACPI SRAT is needed to determine this, and it wouldn't work if acpi=off.

srat only have apicid to node mapping, and ram to node mapping.

Which is very important for performance.

Quoted 3 lines

for amd64 system, when acpi=off
ram to node mapping can be retrieved from pci conf space

I was referring to all types of numa systems, not just AMD. The pci config space is amd specific. Multi chassi systems in particular would be acpi SRAT based. Anyways.

Kiran

#47 lore raw

Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2

Quoted 25 lines · Ravikiran Thirumalai
Quoted 13 lines · Yinghai Lu
Quoted 12 lines · Ravikiran Thirumalai
Quoted 10 lines · Yinghai Lu
Quoted 7 lines · Ravikiran Thirumalai
Quoted 2 lines · Yinghai Lu

1. if acpi=off ?

Well that is not a realistic scenario for any multi chassi NUMA machine, since the proximity information is very important and turning acpi off deprives the OS of this information.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

actually OS should detect the distance instead of rely on SLIT.

How does it do that? ACPI SRAT is needed to determine this, and it wouldn't work if acpi=off.

srat only have apicid to node mapping, and ram to node mapping.

Which is very important for performance.

Quoted 1 line
Quoted 6 lines
Quoted 2 lines

for amd64 system, when acpi=off
ram to node mapping can be retrieved from pci conf space

I was referring to all types of numa systems, not just AMD. The pci config space is amd specific. Multi chassi systems in particular would be acpi SRAT based. Anyways.

can you try x86.git#testing your vsmp to see if is_vsmp_box is working?

http://people.redhat.com/mingo/x86.git/README

YH