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

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

[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

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?

Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>

diff --git a/arch/x86/kernel/apic_64.c b/arch/x86/kernel/apic_64.c
index 9b2be3d..5bdf1bc 100644
--- a/arch/x86/kernel/apic_64.c
+++ b/arch/x86/kernel/apic_64.c
@@ -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

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

[Andi Kleen]

Yinghai Lu <Yinghai.Lu@Sun.COM> writes:

> 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?

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

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

[Yinghai Lu]

On Friday 22 February 2008 04:25:18 am Andi Kleen wrote:
> Yinghai Lu <Yinghai.Lu@Sun.COM> writes:
> 
> > 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?
> 
> 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.

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

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

[Andi Kleen]

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

> is their box using AMD cpu or not?

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

-Andi

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

[Yinghai Lu]

On Fri, Feb 22, 2008 at 11:00 AM, Andi Kleen <andi@firstfloor.org> wrote:
>
>  > 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.
>
>
>  > 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

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

[Andi Kleen]

Yinghai Lu wrote:
> On Fri, Feb 22, 2008 at 11:00 AM, Andi Kleen <andi@firstfloor.org> wrote:
>>  > 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.
>>
>>
>>  > 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

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

[Yinghai Lu]

On Fri, Feb 22, 2008 at 11:07 AM, Andi Kleen <andi@firstfloor.org> wrote:
>
> Yinghai Lu wrote:
>  > On Fri, Feb 22, 2008 at 11:00 AM, Andi Kleen <andi@firstfloor.org> wrote:
>  >>  > 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.
>  >>
>  >>
>  >>  > 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

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

[Andi Kleen]

Yinghai Lu wrote:
> On Fri, Feb 22, 2008 at 11:07 AM, Andi Kleen <andi@firstfloor.org> wrote:
>> Yinghai Lu wrote:
>>  > On Fri, Feb 22, 2008 at 11:00 AM, Andi Kleen <andi@firstfloor.org> wrote:
>>  >>  > 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.
>>  >>
>>  >>
>>  >>  > 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?

I would expect the APIC IDs to be continuous then.

-Andi

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

[Yinghai Lu]

On Fri, Feb 22, 2008 at 11:10 AM, Andi Kleen <andi@firstfloor.org> wrote:
> Yinghai Lu wrote:
>  > On Fri, Feb 22, 2008 at 11:07 AM, Andi Kleen <andi@firstfloor.org> wrote:
>  >> Yinghai Lu wrote:
>  >>  > On Fri, Feb 22, 2008 at 11:00 AM, Andi Kleen <andi@firstfloor.org> wrote:
>  >>  >>  > 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.
>  >>  >>
>  >>  >>
>  >>  >>  > 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?
>
>  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

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

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

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.

Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>

diff --git a/arch/x86/kernel/apic_64.c b/arch/x86/kernel/apic_64.c
index d8d03e0..7d8ffda 100644
--- a/arch/x86/kernel/apic_64.c
+++ b/arch/x86/kernel/apic_64.c
@@ -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++) {

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

[Ingo Molnar]

* Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:

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

thanks Yinghai, applied.

	Ingo

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

[Andi Kleen]

On Sat, Feb 23, 2008 at 09:48:42PM -0800, Yinghai Lu wrote:
> 
> 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 ...

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

diff --git a/arch/x86/kernel/apic_64.c b/arch/x86/kernel/apic_64.c
index d8d03e0..7d8ffda 100644
--- a/arch/x86/kernel/apic_64.c
+++ b/arch/x86/kernel/apic_64.c
@@ -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;

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

[Yinghai Lu]

On Sunday 24 February 2008 04:29:03 am Andi Kleen wrote:
> On Sat, Feb 23, 2008 at 09:48:42PM -0800, Yinghai Lu wrote:
> > 
> > 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 ...
> 
> > 
> > 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.

do you have bootlog for these box?

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

DMI could tell the difference?

YH

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

[Yinghai Lu]

On Sunday 24 February 2008 03:00:52 pm Yinghai Lu wrote:
> On Sunday 24 February 2008 04:29:03 am Andi Kleen wrote:
> > On Sat, Feb 23, 2008 at 09:48:42PM -0800, Yinghai Lu wrote:
> > > 
> > > 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 ...
> > 
> > > 
> > > 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.
> 
> 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...

Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 4eb5ce8..2bec799 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -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
diff --git a/arch/x86/kernel/apic_64.c b/arch/x86/kernel/apic_64.c
index d8d03e0..1427ec3 100644
--- a/arch/x86/kernel/apic_64.c
+++ b/arch/x86/kernel/apic_64.c
@@ -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++) {
diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index d971210..2780df1 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -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
diff --git a/include/asm-x86/apic.h b/include/asm-x86/apic.h
index bcfc07f..d69f596 100644
--- a/include/asm-x86/apic.h
+++ b/include/asm-x86/apic.h
@@ -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) { }

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

[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

Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>

diff --git a/arch/x86/kernel/apic_64.c b/arch/x86/kernel/apic_64.c
index 8f6c45e..8a47579 100644
--- a/arch/x86/kernel/apic_64.c
+++ b/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;
diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 54202b1..09e16ff 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -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 */
diff --git a/include/asm-x86/apic.h b/include/asm-x86/apic.h
index a68dc6b..24f7a05 100644
--- a/include/asm-x86/apic.h
+++ b/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
+int inline is_vsmp_box(void)
+{
+	return 0;
+}
 #endif
 
 static inline void native_apic_write(unsigned long reg, u32 v)

[PATCH] x86_64: for apic_is_clustered_box for vsmp v2

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

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

Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>

Index: linux-2.6/arch/x86/kernel/apic_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic_64.c
+++ 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;
Index: linux-2.6/arch/x86/kernel/vsmp_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/vsmp_64.c
+++ 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 */
Index: linux-2.6/include/asm-x86/apic.h
===================================================================
--- linux-2.6.orig/include/asm-x86/apic.h
+++ 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)

[PATCH] x86: vSMP selection in config

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

Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>

Index: linux-2.6/arch/x86/Kconfig
===================================================================
--- linux-2.6.orig/arch/x86/Kconfig
+++ 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)

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

[Sam Ravnborg]

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

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

[Roman Zippel]

Hi,

On Tue, 26 Feb 2008, Sam Ravnborg wrote:

> 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

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

[PATCH 1/3] fix recursive dependencies

[Roman Zippel]

Hi,

On Tue, 26 Feb 2008, Sam Ravnborg wrote:

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

Signed-off-by: Roman Zippel <zippel@linux-m68k.org>

---
 arch/x86/Kconfig |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Index: linux-2.6/arch/x86/Kconfig
===================================================================
--- linux-2.6.orig/arch/x86/Kconfig
+++ 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)

Re: [PATCH 1/3] fix recursive dependencies

[Yinghai Lu]

On Thu, Feb 28, 2008 at 8:09 PM, Roman Zippel <zippel@linux-m68k.org> wrote:
> Hi,
>
>  On Tue, 26 Feb 2008, Sam Ravnborg wrote:
>
>  > 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.
>
>  Signed-off-by: Roman Zippel <zippel@linux-m68k.org>
>
>  ---
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

Re: [PATCH 1/3] fix recursive dependencies

[Roman Zippel]

Hi,

On Thu, 28 Feb 2008, Yinghai Lu wrote:

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

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

> 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

Re: [PATCH 1/3] fix recursive dependencies

[Sam Ravnborg]

On Fri, Feb 29, 2008 at 02:22:01PM +0100, Roman Zippel wrote:
> Hi,
> 
> On Thu, 28 Feb 2008, Yinghai Lu wrote:
> 
> > x86: PARAVIRT needed by PARAVIRT_GUEST or X86_VSMP
> > x86: vSMP selection in config
> 
> Sorry, I hadn't seen there already was a followup.
> 
> > 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

Re: [PATCH 1/3] fix recursive dependencies

[Ingo Molnar]

* Sam Ravnborg <sam@ravnborg.org> wrote:

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

Re: [PATCH 1/3] fix recursive dependencies

[Ingo Molnar]

* Roman Zippel <zippel@linux-m68k.org> wrote:

> 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

[PATCH 2/3] fix choice dependency check

[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).

Signed-off-by: Roman Zippel <zippel@linux-m68k.org>

---
 scripts/kconfig/symbol.c |   94 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 76 insertions(+), 18 deletions(-)

Index: linux-2.6/scripts/kconfig/symbol.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/symbol.c
+++ 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;
 }
 

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

[Sam Ravnborg]

On Fri, Feb 29, 2008 at 05:10:24AM +0100, Roman Zippel wrote:
> 
> 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).
> 
> Signed-off-by: Roman Zippel <zippel@linux-m68k.org>

Hi Roman.

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

	Sam

[PATCH 3/3] add named choice group

[Roman Zippel]

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.

Signed-off-by: Roman Zippel <zippel@linux-m68k.org>

---
 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(-)

Index: linux-2.6/scripts/kconfig/lex.zconf.c_shipped
===================================================================
--- linux-2.6.orig/scripts/kconfig/lex.zconf.c_shipped
+++ 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
Index: linux-2.6/scripts/kconfig/lkc_proto.h
===================================================================
--- linux-2.6.orig/scripts/kconfig/lkc_proto.h
+++ 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));
Index: linux-2.6/scripts/kconfig/menu.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/menu.c
+++ linux-2.6/scripts/kconfig/menu.c
@@ -235,18 +235,22 @@ void menu_finalize(struct menu *parent)
 	sym = parent->sym;
 	if (parent->list) {
 		if (sym && sym_is_choice(sym)) {
-			/* find the first choice value and find out choice type */
-			for (menu = parent->list; menu; menu = menu->next) {
-				if (menu->sym) {
-					current_entry = parent;
-					if (sym->type == S_UNKNOWN)
+			if (sym->type == S_UNKNOWN) {
+				/* find the first choice value to find out choice type */
+				current_entry = parent;
+				for (menu = parent->list; menu; menu = menu->next) {
+					if (menu->sym && menu->sym->type != S_UNKNOWN) {
 						menu_set_type(menu->sym->type);
-					current_entry = menu;
-					if (menu->sym->type == S_UNKNOWN)
-						menu_set_type(sym->type);
-					break;
+						break;
+					}
 				}
 			}
+			/* set the type of the remaining choice values */
+			for (menu = parent->list; menu; menu = menu->next) {
+				current_entry = menu;
+				if (menu->sym && menu->sym->type == S_UNKNOWN)
+					menu_set_type(sym->type);
+			}
 			parentdep = expr_alloc_symbol(sym);
 		} else if (parent->prompt)
 			parentdep = parent->prompt->visible.expr;
@@ -313,50 +317,36 @@ void menu_finalize(struct menu *parent)
 		}
 	}
 	for (menu = parent->list; menu; menu = menu->next) {
-		if (sym && sym_is_choice(sym) && menu->sym) {
+		if (sym && sym_is_choice(sym) &&
+		    menu->sym && !sym_is_choice_value(menu->sym)) {
+			current_entry = menu;
 			menu->sym->flags |= SYMBOL_CHOICEVAL;
 			if (!menu->prompt)
 				menu_warn(menu, "choice value must have a prompt");
 			for (prop = menu->sym->prop; prop; prop = prop->next) {
-				if (prop->type == P_PROMPT && prop->menu != menu) {
-					prop_warn(prop, "choice values "
-					    "currently only support a "
-					    "single prompt");
-				}
 				if (prop->type == P_DEFAULT)
 					prop_warn(prop, "defaults for choice "
-					    "values not supported");
+						  "values not supported");
+				if (prop->menu == menu)
+					continue;
+				if (prop->type == P_PROMPT &&
+				    prop->menu->parent->sym != sym)
+					prop_warn(prop, "choice value used outside its choice group");
 			}
-			current_entry = menu;
-			if (menu->sym->type == S_UNKNOWN)
-				menu_set_type(sym->type);
 			/* Non-tristate choice values of tristate choices must
 			 * depend on the choice being set to Y. The choice
 			 * values' dependencies were propagated to their
 			 * properties above, so the change here must be re-
-			 * propagated. */
+			 * propagated.
+			 */
 			if (sym->type == S_TRISTATE && menu->sym->type != S_TRISTATE) {
 				basedep = expr_alloc_comp(E_EQUAL, sym, &symbol_yes);
-				basedep = expr_alloc_and(basedep, menu->dep);
-				basedep = expr_eliminate_dups(basedep);
-				menu->dep = basedep;
+				menu->dep = expr_alloc_and(basedep, menu->dep);
 				for (prop = menu->sym->prop; prop; prop = prop->next) {
 					if (prop->menu != menu)
 						continue;
-					dep = expr_alloc_and(expr_copy(basedep),
-							     prop->visible.expr);
-					dep = expr_eliminate_dups(dep);
-					dep = expr_trans_bool(dep);
-					prop->visible.expr = dep;
-					if (prop->type == P_SELECT) {
-						struct symbol *es = prop_get_symbol(prop);
-						dep2 = expr_alloc_symbol(menu->sym);
-						dep = expr_alloc_and(dep2,
-								     expr_copy(dep));
-						dep = expr_alloc_or(es->rev_dep.expr, dep);
-						dep = expr_eliminate_dups(dep);
-						es->rev_dep.expr = dep;
-					}
+					prop->visible.expr = expr_alloc_and(expr_copy(basedep),
+									    prop->visible.expr);
 				}
 			}
 			menu_add_symbol(P_CHOICE, sym, NULL);
Index: linux-2.6/scripts/kconfig/symbol.c
===================================================================
--- linux-2.6.orig/scripts/kconfig/symbol.c
+++ 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;
Index: linux-2.6/scripts/kconfig/zconf.tab.c_shipped
===================================================================
--- linux-2.6.orig/scripts/kconfig/zconf.tab.c_shipped
+++ linux-2.6/scripts/kconfig/zconf.tab.c_shipped
@@ -446,16 +446,16 @@ union yyalloc
 /* YYFINAL -- State number of the termination state.  */
 #define YYFINAL  3
 /* YYLAST -- Last index in YYTABLE.  */
-#define YYLAST   258
+#define YYLAST   259
 
 /* YYNTOKENS -- Number of terminals.  */
 #define YYNTOKENS  35
 /* YYNNTS -- Number of nonterminals.  */
-#define YYNNTS  45
+#define YYNNTS  46
 /* YYNRULES -- Number of rules.  */
-#define YYNRULES  108
+#define YYNRULES  110
 /* YYNRULES -- Number of states.  */
-#define YYNSTATES  178
+#define YYNSTATES  180
 
 /* YYTRANSLATE(YYLEX) -- Bison symbol number corresponding to YYLEX.  */
 #define YYUNDEFTOK  2
@@ -507,13 +507,14 @@ static const yytype_uint16 yyprhs[] =
       28,    33,    37,    39,    41,    43,    45,    47,    49,    51,
       53,    55,    57,    59,    61,    63,    67,    70,    74,    77,
       81,    84,    85,    88,    91,    94,    97,   100,   103,   107,
-     112,   117,   122,   128,   132,   133,   137,   138,   141,   144,
-     147,   149,   153,   154,   157,   160,   163,   166,   169,   174,
-     178,   181,   186,   187,   190,   194,   196,   200,   201,   204,
-     207,   210,   214,   217,   219,   223,   224,   227,   230,   233,
-     237,   241,   244,   247,   250,   251,   254,   257,   260,   265,
-     266,   269,   271,   273,   276,   279,   282,   284,   287,   288,
-     291,   293,   297,   301,   305,   308,   312,   316,   318
+     112,   117,   122,   128,   132,   133,   137,   138,   141,   145,
+     148,   150,   154,   155,   158,   161,   164,   167,   170,   175,
+     179,   182,   187,   188,   191,   195,   197,   201,   202,   205,
+     208,   211,   215,   218,   220,   224,   225,   228,   231,   234,
+     238,   242,   245,   248,   251,   252,   255,   258,   261,   266,
+     267,   270,   272,   274,   277,   280,   283,   285,   288,   289,
+     292,   294,   298,   302,   306,   309,   313,   317,   319,   321,
+     322
 };
 
 /* YYRHS -- A `-1'-separated list of the rules' RHS.  */
@@ -533,24 +534,25 @@ static const yytype_int8 yyrhs[] =
       30,    -1,    20,    78,    77,    30,    -1,    21,    25,    77,
       30,    -1,    22,    79,    79,    77,    30,    -1,    23,    48,
       30,    -1,    -1,    48,    25,    49,    -1,    -1,    33,    74,
-      -1,     7,    30,    -1,    50,    54,    -1,    75,    -1,    51,
-      56,    52,    -1,    -1,    54,    55,    -1,    54,    72,    -1,
-      54,    70,    -1,    54,    30,    -1,    54,    40,    -1,    18,
-      74,    77,    30,    -1,    19,    73,    30,    -1,    17,    30,
-      -1,    20,    25,    77,    30,    -1,    -1,    56,    39,    -1,
-      14,    78,    76,    -1,    75,    -1,    57,    60,    58,    -1,
-      -1,    60,    39,    -1,    60,    64,    -1,    60,    53,    -1,
-       4,    74,    30,    -1,    61,    71,    -1,    75,    -1,    62,
-      65,    63,    -1,    -1,    65,    39,    -1,    65,    64,    -1,
-      65,    53,    -1,     6,    74,    30,    -1,     9,    74,    30,
-      -1,    67,    71,    -1,    12,    30,    -1,    69,    13,    -1,
-      -1,    71,    72,    -1,    71,    30,    -1,    71,    40,    -1,
-      16,    24,    78,    30,    -1,    -1,    74,    77,    -1,    25,
-      -1,    26,    -1,     5,    30,    -1,     8,    30,    -1,    15,
-      30,    -1,    30,    -1,    76,    30,    -1,    -1,    14,    78,
-      -1,    79,    -1,    79,    33,    79,    -1,    79,    27,    79,
-      -1,    29,    78,    28,    -1,    34,    78,    -1,    78,    31,
-      78,    -1,    78,    32,    78,    -1,    25,    -1,    26,    -1
+      -1,     7,    80,    30,    -1,    50,    54,    -1,    75,    -1,
+      51,    56,    52,    -1,    -1,    54,    55,    -1,    54,    72,
+      -1,    54,    70,    -1,    54,    30,    -1,    54,    40,    -1,
+      18,    74,    77,    30,    -1,    19,    73,    30,    -1,    17,
+      30,    -1,    20,    25,    77,    30,    -1,    -1,    56,    39,
+      -1,    14,    78,    76,    -1,    75,    -1,    57,    60,    58,
+      -1,    -1,    60,    39,    -1,    60,    64,    -1,    60,    53,
+      -1,     4,    74,    30,    -1,    61,    71,    -1,    75,    -1,
+      62,    65,    63,    -1,    -1,    65,    39,    -1,    65,    64,
+      -1,    65,    53,    -1,     6,    74,    30,    -1,     9,    74,
+      30,    -1,    67,    71,    -1,    12,    30,    -1,    69,    13,
+      -1,    -1,    71,    72,    -1,    71,    30,    -1,    71,    40,
+      -1,    16,    24,    78,    30,    -1,    -1,    74,    77,    -1,
+      25,    -1,    26,    -1,     5,    30,    -1,     8,    30,    -1,
+      15,    30,    -1,    30,    -1,    76,    30,    -1,    -1,    14,
+      78,    -1,    79,    -1,    79,    33,    79,    -1,    79,    27,
+      79,    -1,    29,    78,    28,    -1,    34,    78,    -1,    78,
+      31,    78,    -1,    78,    32,    78,    -1,    25,    -1,    26,
+      -1,    -1,    25,    -1
 };
 
 /* YYRLINE[YYN] -- source line where rule number YYN was defined.  */
@@ -566,7 +568,8 @@ static const yytype_uint16 yyrline[] =
      339,   344,   351,   356,   364,   367,   369,   370,   371,   374,
      382,   389,   396,   402,   409,   411,   412,   413,   416,   424,
      426,   431,   432,   435,   436,   437,   441,   442,   445,   446,
-     449,   450,   451,   452,   453,   454,   455,   458,   459
+     449,   450,   451,   452,   453,   454,   455,   458,   459,   462,
+     463
 };
 #endif
 
@@ -590,7 +593,8 @@ static const char *const yytname[] =
   "if_entry", "if_end", "if_stmt", "if_block", "menu", "menu_entry",
   "menu_end", "menu_stmt", "menu_block", "source_stmt", "comment",
   "comment_stmt", "help_start", "help", "depends_list", "depends",
-  "prompt_stmt_opt", "prompt", "end", "nl", "if_expr", "expr", "symbol", 0
+  "prompt_stmt_opt", "prompt", "end", "nl", "if_expr", "expr", "symbol",
+  "word_opt", 0
 };
 #endif
 
@@ -619,7 +623,8 @@ static const yytype_uint8 yyr1[] =
       60,    61,    62,    63,    64,    65,    65,    65,    65,    66,
       67,    68,    69,    70,    71,    71,    71,    71,    72,    73,
       73,    74,    74,    75,    75,    75,    76,    76,    77,    77,
-      78,    78,    78,    78,    78,    78,    78,    79,    79
+      78,    78,    78,    78,    78,    78,    78,    79,    79,    80,
+      80
 };
 
 /* YYR2[YYN] -- Number of symbols composing right hand side of rule YYN.  */
@@ -629,13 +634,14 @@ static const yytype_uint8 yyr2[] =
        4,     3,     1,     1,     1,     1,     1,     1,     1,     1,
        1,     1,     1,     1,     1,     3,     2,     3,     2,     3,
        2,     0,     2,     2,     2,     2,     2,     2,     3,     4,
-       4,     4,     5,     3,     0,     3,     0,     2,     2,     2,
+       4,     4,     5,     3,     0,     3,     0,     2,     3,     2,
        1,     3,     0,     2,     2,     2,     2,     2,     4,     3,
        2,     4,     0,     2,     3,     1,     3,     0,     2,     2,
        2,     3,     2,     1,     3,     0,     2,     2,     2,     3,
        3,     2,     2,     2,     0,     2,     2,     2,     4,     0,
        2,     1,     1,     2,     2,     2,     1,     2,     0,     2,
-       1,     3,     3,     3,     2,     3,     3,     1,     1
+       1,     3,     3,     3,     2,     3,     3,     1,     1,     0,
+       1
 };
 
 /* YYDEFACT[STATE-NAME] -- Default rule to reduce with in state
@@ -643,69 +649,69 @@ static const yytype_uint8 yyr2[] =
    means the default is an error.  */
 static const yytype_uint8 yydefact[] =
 {
-       3,     0,     0,     1,     0,     0,     0,     0,     0,     0,
+       3,     0,     0,     1,     0,     0,     0,     0,     0,   109,
        0,     0,     0,     0,     0,     0,    12,    16,    13,    14,
       18,    15,    17,     0,    19,     0,     4,    31,    22,    31,
       23,    52,    62,     5,    67,    20,    84,    75,     6,    24,
       84,    21,     8,    11,    91,    92,     0,     0,    93,     0,
-      48,    94,     0,     0,     0,   107,   108,     0,     0,     0,
-     100,    95,     0,     0,     0,     0,     0,     0,     0,     0,
-       0,     0,    96,     7,    71,    79,    80,    27,    29,     0,
-     104,     0,     0,    64,     0,     0,     9,    10,     0,     0,
-       0,     0,    89,     0,     0,     0,    44,     0,    37,    36,
-      32,    33,     0,    35,    34,     0,     0,    89,     0,    56,
-      57,    53,    55,    54,    63,    51,    50,    68,    70,    66,
-      69,    65,    86,    87,    85,    76,    78,    74,    77,    73,
-      97,   103,   105,   106,   102,   101,    26,    82,     0,    98,
-       0,    98,    98,    98,     0,     0,     0,    83,    60,    98,
-       0,    98,     0,     0,     0,    38,    90,     0,     0,    98,
-      46,    43,    25,     0,    59,     0,    88,    99,    39,    40,
-      41,     0,     0,    45,    58,    61,    42,    47
+     110,     0,    94,     0,     0,     0,   107,   108,     0,     0,
+       0,   100,    95,     0,     0,     0,     0,     0,     0,     0,
+       0,     0,     0,    96,     7,    71,    79,    48,    80,    27,
+      29,     0,   104,     0,     0,    64,     0,     0,     9,    10,
+       0,     0,     0,     0,    89,     0,     0,     0,    44,     0,
+      37,    36,    32,    33,     0,    35,    34,     0,     0,    89,
+       0,    56,    57,    53,    55,    54,    63,    51,    50,    68,
+      70,    66,    69,    65,    86,    87,    85,    76,    78,    74,
+      77,    73,    97,   103,   105,   106,   102,   101,    26,    82,
+       0,    98,     0,    98,    98,    98,     0,     0,     0,    83,
+      60,    98,     0,    98,     0,     0,     0,    38,    90,     0,
+       0,    98,    46,    43,    25,     0,    59,     0,    88,    99,
+      39,    40,    41,     0,     0,    45,    58,    61,    42,    47
 };
 
 /* YYDEFGOTO[NTERM-NUM].  */
 static const yytype_int16 yydefgoto[] =
 {
-      -1,     1,     2,    25,    26,    99,    27,    28,    29,    30,
-      64,   100,   101,   145,   173,    31,    32,   115,    33,    66,
-     111,    67,    34,   119,    35,    68,    36,    37,   127,    38,
-      70,    39,    40,    41,   102,   103,    69,   104,   140,   141,
-      42,    73,   154,    59,    60
+      -1,     1,     2,    25,    26,   101,    27,    28,    29,    30,
+      65,   102,   103,   147,   175,    31,    32,   117,    33,    67,
+     113,    68,    34,   121,    35,    69,    36,    37,   129,    38,
+      71,    39,    40,    41,   104,   105,    70,   106,   142,   143,
+      42,    74,   156,    60,    61,    51
 };
 
 /* YYPACT[STATE-NUM] -- Index in YYTABLE of the portion describing
    STATE-NUM.  */
-#define YYPACT_NINF -78
+#define YYPACT_NINF -80
 static const yytype_int16 yypact[] =
 {
-     -78,    33,   130,   -78,   -28,    73,    73,     7,    73,    36,
-      41,    73,    26,    52,    -4,    58,   -78,   -78,   -78,   -78,
-     -78,   -78,   -78,    90,   -78,    94,   -78,   -78,   -78,   -78,
-     -78,   -78,   -78,   -78,   -78,   -78,   -78,   -78,   -78,   -78,
-     -78,   -78,   -78,   -78,   -78,   -78,    74,    85,   -78,    96,
-     -78,   -78,   131,   134,   147,   -78,   -78,    -4,    -4,   193,
-     -10,   -78,   162,   164,    38,   102,    64,   148,     5,   192,
-       5,   165,   -78,   174,   -78,   -78,   -78,   -78,   -78,    65,
-     -78,    -4,    -4,   174,   103,   103,   -78,   -78,   175,   185,
-     197,    73,    73,    -4,   194,   103,   -78,   231,   -78,   -78,
-     -78,   -78,   220,   -78,   -78,   204,    73,    73,   210,   -78,
-     -78,   -78,   -78,   -78,   -78,   -78,   -78,   -78,   -78,   -78,
-     -78,   -78,   -78,   -78,   -78,   -78,   -78,   -78,   -78,   -78,
-     -78,   -78,   205,   -78,   -78,   -78,   -78,   -78,    -4,   222,
-     208,   222,   195,   222,   103,     2,   209,   -78,   -78,   222,
-     211,   222,   199,    -4,   212,   -78,   -78,   213,   214,   222,
-     207,   -78,   -78,   215,   -78,   216,   -78,   111,   -78,   -78,
-     -78,   217,    73,   -78,   -78,   -78,   -78,   -78
+     -80,     2,   132,   -80,   -13,    -1,    -1,    -2,    -1,     9,
+      33,    -1,    27,    40,    -3,    38,   -80,   -80,   -80,   -80,
+     -80,   -80,   -80,    71,   -80,    77,   -80,   -80,   -80,   -80,
+     -80,   -80,   -80,   -80,   -80,   -80,   -80,   -80,   -80,   -80,
+     -80,   -80,   -80,   -80,   -80,   -80,    57,    61,   -80,    63,
+     -80,    76,   -80,    87,   101,   133,   -80,   -80,    -3,    -3,
+     195,    -6,   -80,   136,   149,    39,   104,    65,   150,     5,
+     194,     5,   167,   -80,   176,   -80,   -80,   -80,   -80,   -80,
+     -80,    68,   -80,    -3,    -3,   176,    72,    72,   -80,   -80,
+     177,   187,    78,    -1,    -1,    -3,   196,    72,   -80,   222,
+     -80,   -80,   -80,   -80,   221,   -80,   -80,   205,    -1,    -1,
+     211,   -80,   -80,   -80,   -80,   -80,   -80,   -80,   -80,   -80,
+     -80,   -80,   -80,   -80,   -80,   -80,   -80,   -80,   -80,   -80,
+     -80,   -80,   -80,   -80,   206,   -80,   -80,   -80,   -80,   -80,
+      -3,   223,   209,   223,   197,   223,    72,     7,   210,   -80,
+     -80,   223,   212,   223,   201,    -3,   213,   -80,   -80,   214,
+     215,   223,   208,   -80,   -80,   216,   -80,   217,   -80,   113,
+     -80,   -80,   -80,   218,    -1,   -80,   -80,   -80,   -80,   -80
 };
 
 /* YYPGOTO[NTERM-NUM].  */
 static const yytype_int16 yypgoto[] =
 {
-     -78,   -78,   -78,   -78,   121,   -35,   -78,   -78,   -78,   -78,
-     219,   -78,   -78,   -78,   -78,   -78,   -78,   -78,   -44,   -78,
-     -78,   -78,   -78,   -78,   -78,   -78,   -78,   -78,   -78,    -6,
-     -78,   -78,   -78,   -78,   -78,   183,   218,    21,   143,    -5,
-     146,   196,    69,   -53,   -77
+     -80,   -80,   -80,   -80,   122,   -34,   -80,   -80,   -80,   -80,
+     220,   -80,   -80,   -80,   -80,   -80,   -80,   -80,    59,   -80,
+     -80,   -80,   -80,   -80,   -80,   -80,   -80,   -80,   -80,   125,
+     -80,   -80,   -80,   -80,   -80,   183,   219,    22,   142,    -5,
+     147,   192,    69,   -54,   -79,   -80
 };
 
 /* YYTABLE[YYPACT[STATE-NUM]].  What to do in state STATE-NUM.  If
@@ -715,62 +721,62 @@ static const yytype_int16 yypgoto[] =
 #define YYTABLE_NINF -82
 static const yytype_int16 yytable[] =
 {
-      46,    47,    43,    49,    79,    80,    52,   134,   135,     6,
-       7,     8,     9,    10,    11,    12,    13,    84,   144,    14,
-      15,    55,    56,    85,   118,    57,   126,   160,   132,   133,
-      58,   110,   161,     3,   123,    24,   123,    48,   -28,    88,
-     142,   -28,   -28,   -28,   -28,   -28,   -28,   -28,   -28,   -28,
-      89,    53,   -28,   -28,    90,   -28,    91,    92,    93,    94,
-      95,    96,   120,    97,   128,    88,    50,   159,    98,   -49,
-     -49,    51,   -49,   -49,   -49,   -49,    89,    54,   -49,   -49,
-      90,   105,   106,   107,   108,   152,   139,   113,    61,    97,
-     124,    62,   124,   131,   109,    63,    81,    82,    44,    45,
-     167,   149,   -30,    88,    72,   -30,   -30,   -30,   -30,   -30,
-     -30,   -30,   -30,   -30,    89,    74,   -30,   -30,    90,   -30,
-      91,    92,    93,    94,    95,    96,    75,    97,    55,    56,
-      -2,     4,    98,     5,     6,     7,     8,     9,    10,    11,
-      12,    13,    81,    82,    14,    15,    16,    17,    18,    19,
-      20,    21,    22,     7,     8,    23,    10,    11,    12,    13,
-      24,    76,    14,    15,    77,   -81,    88,   177,   -81,   -81,
-     -81,   -81,   -81,   -81,   -81,   -81,   -81,    78,    24,   -81,
-     -81,    90,   -81,   -81,   -81,   -81,   -81,   -81,   114,   117,
-      97,   125,    86,    88,    87,   122,   -72,   -72,   -72,   -72,
-     -72,   -72,   -72,   -72,   130,   136,   -72,   -72,    90,   153,
-     156,   157,   158,   116,   121,   137,   129,    97,   163,   143,
-     165,   138,   122,    72,    81,    82,    81,    82,   171,   166,
-      81,    82,   146,   147,   148,   151,   153,    82,   155,   162,
-     172,   164,   168,   169,   170,   174,   175,   176,    65,   112,
-     150,     0,     0,     0,     0,    83,     0,     0,    71
+      46,    47,     3,    49,    81,    82,    53,   136,   137,     6,
+       7,     8,     9,    10,    11,    12,    13,    43,   146,    14,
+      15,    86,    56,    57,    44,    45,    58,    87,    48,   134,
+     135,    59,   162,   112,    50,    24,   125,   163,   125,   -28,
+      90,   144,   -28,   -28,   -28,   -28,   -28,   -28,   -28,   -28,
+     -28,    91,    54,   -28,   -28,    92,   -28,    93,    94,    95,
+      96,    97,    98,    52,    99,    55,    90,   161,    62,   100,
+     -49,   -49,    63,   -49,   -49,   -49,   -49,    91,    64,   -49,
+     -49,    92,   107,   108,   109,   110,   154,    73,   141,   115,
+      99,    75,   126,    76,   126,   111,   133,    56,    57,    83,
+      84,   169,   140,   151,   -30,    90,    77,   -30,   -30,   -30,
+     -30,   -30,   -30,   -30,   -30,   -30,    91,    78,   -30,   -30,
+      92,   -30,    93,    94,    95,    96,    97,    98,   120,    99,
+     128,    79,    -2,     4,   100,     5,     6,     7,     8,     9,
+      10,    11,    12,    13,    83,    84,    14,    15,    16,    17,
+      18,    19,    20,    21,    22,     7,     8,    23,    10,    11,
+      12,    13,    24,    80,    14,    15,    88,   -81,    90,   179,
+     -81,   -81,   -81,   -81,   -81,   -81,   -81,   -81,   -81,    89,
+      24,   -81,   -81,    92,   -81,   -81,   -81,   -81,   -81,   -81,
+     116,   119,    99,   127,   122,    90,   130,   124,   -72,   -72,
+     -72,   -72,   -72,   -72,   -72,   -72,   132,   138,   -72,   -72,
+      92,   155,   158,   159,   160,   118,   123,   139,   131,    99,
+     165,   145,   167,   148,   124,    73,    83,    84,    83,    84,
+     173,   168,    83,    84,   149,   150,   153,   155,    84,   157,
+     164,   174,   166,   170,   171,   172,   176,   177,   178,    66,
+     114,   152,    85,     0,     0,     0,     0,     0,     0,    72
 };
 
 static const yytype_int16 yycheck[] =
 {
-       5,     6,    30,     8,    57,    58,    11,    84,    85,     4,
-       5,     6,     7,     8,     9,    10,    11,    27,    95,    14,
-      15,    25,    26,    33,    68,    29,    70,    25,    81,    82,
-      34,    66,    30,     0,    69,    30,    71,    30,     0,     1,
-      93,     3,     4,     5,     6,     7,     8,     9,    10,    11,
-      12,    25,    14,    15,    16,    17,    18,    19,    20,    21,
-      22,    23,    68,    25,    70,     1,    30,   144,    30,     5,
-       6,    30,     8,     9,    10,    11,    12,    25,    14,    15,
-      16,    17,    18,    19,    20,   138,    91,    66,    30,    25,
-      69,     1,    71,    28,    30,     1,    31,    32,    25,    26,
-     153,   106,     0,     1,    30,     3,     4,     5,     6,     7,
-       8,     9,    10,    11,    12,    30,    14,    15,    16,    17,
-      18,    19,    20,    21,    22,    23,    30,    25,    25,    26,
-       0,     1,    30,     3,     4,     5,     6,     7,     8,     9,
-      10,    11,    31,    32,    14,    15,    16,    17,    18,    19,
-      20,    21,    22,     5,     6,    25,     8,     9,    10,    11,
-      30,    30,    14,    15,    30,     0,     1,   172,     3,     4,
-       5,     6,     7,     8,     9,    10,    11,    30,    30,    14,
-      15,    16,    17,    18,    19,    20,    21,    22,    67,    68,
-      25,    70,    30,     1,    30,    30,     4,     5,     6,     7,
-       8,     9,    10,    11,    30,    30,    14,    15,    16,    14,
-     141,   142,   143,    67,    68,    30,    70,    25,   149,    25,
-     151,    24,    30,    30,    31,    32,    31,    32,   159,    30,
-      31,    32,     1,    13,    30,    25,    14,    32,    30,    30,
-      33,    30,    30,    30,    30,    30,    30,    30,    29,    66,
-     107,    -1,    -1,    -1,    -1,    59,    -1,    -1,    40
+       5,     6,     0,     8,    58,    59,    11,    86,    87,     4,
+       5,     6,     7,     8,     9,    10,    11,    30,    97,    14,
+      15,    27,    25,    26,    25,    26,    29,    33,    30,    83,
+      84,    34,    25,    67,    25,    30,    70,    30,    72,     0,
+       1,    95,     3,     4,     5,     6,     7,     8,     9,    10,
+      11,    12,    25,    14,    15,    16,    17,    18,    19,    20,
+      21,    22,    23,    30,    25,    25,     1,   146,    30,    30,
+       5,     6,     1,     8,     9,    10,    11,    12,     1,    14,
+      15,    16,    17,    18,    19,    20,   140,    30,    93,    67,
+      25,    30,    70,    30,    72,    30,    28,    25,    26,    31,
+      32,   155,    24,   108,     0,     1,    30,     3,     4,     5,
+       6,     7,     8,     9,    10,    11,    12,    30,    14,    15,
+      16,    17,    18,    19,    20,    21,    22,    23,    69,    25,
+      71,    30,     0,     1,    30,     3,     4,     5,     6,     7,
+       8,     9,    10,    11,    31,    32,    14,    15,    16,    17,
+      18,    19,    20,    21,    22,     5,     6,    25,     8,     9,
+      10,    11,    30,    30,    14,    15,    30,     0,     1,   174,
+       3,     4,     5,     6,     7,     8,     9,    10,    11,    30,
+      30,    14,    15,    16,    17,    18,    19,    20,    21,    22,
+      68,    69,    25,    71,    69,     1,    71,    30,     4,     5,
+       6,     7,     8,     9,    10,    11,    30,    30,    14,    15,
+      16,    14,   143,   144,   145,    68,    69,    30,    71,    25,
+     151,    25,   153,     1,    30,    30,    31,    32,    31,    32,
+     161,    30,    31,    32,    13,    30,    25,    14,    32,    30,
+      30,    33,    30,    30,    30,    30,    30,    30,    30,    29,
+      67,   109,    60,    -1,    -1,    -1,    -1,    -1,    -1,    40
 };
 
 /* YYSTOS[STATE-NUM] -- The (internal number of the) accessing
@@ -782,19 +788,19 @@ static const yytype_uint8 yystos[] =
       20,    21,    22,    25,    30,    38,    39,    41,    42,    43,
       44,    50,    51,    53,    57,    59,    61,    62,    64,    66,
       67,    68,    75,    30,    25,    26,    74,    74,    30,    74,
-      30,    30,    74,    25,    25,    25,    26,    29,    34,    78,
-      79,    30,     1,     1,    45,    45,    54,    56,    60,    71,
-      65,    71,    30,    76,    30,    30,    30,    30,    30,    78,
-      78,    31,    32,    76,    27,    33,    30,    30,     1,    12,
-      16,    18,    19,    20,    21,    22,    23,    25,    30,    40,
-      46,    47,    69,    70,    72,    17,    18,    19,    20,    30,
-      40,    55,    70,    72,    39,    52,    75,    39,    53,    58,
-      64,    75,    30,    40,    72,    39,    53,    63,    64,    75,
-      30,    28,    78,    78,    79,    79,    30,    30,    24,    74,
-      73,    74,    78,    25,    79,    48,     1,    13,    30,    74,
-      73,    25,    78,    14,    77,    30,    77,    77,    77,    79,
-      25,    30,    30,    77,    30,    77,    30,    78,    30,    30,
-      30,    77,    33,    49,    30,    30,    30,    74
+      25,    80,    30,    74,    25,    25,    25,    26,    29,    34,
+      78,    79,    30,     1,     1,    45,    45,    54,    56,    60,
+      71,    65,    71,    30,    76,    30,    30,    30,    30,    30,
+      30,    78,    78,    31,    32,    76,    27,    33,    30,    30,
+       1,    12,    16,    18,    19,    20,    21,    22,    23,    25,
+      30,    40,    46,    47,    69,    70,    72,    17,    18,    19,
+      20,    30,    40,    55,    70,    72,    39,    52,    75,    39,
+      53,    58,    64,    75,    30,    40,    72,    39,    53,    63,
+      64,    75,    30,    28,    78,    78,    79,    79,    30,    30,
+      24,    74,    73,    74,    78,    25,    79,    48,     1,    13,
+      30,    74,    73,    25,    78,    14,    77,    30,    77,    77,
+      77,    79,    25,    30,    30,    77,    30,    77,    30,    78,
+      30,    30,    30,    77,    33,    49,    30,    30,    30,    74
 };
 
 #define yyerrok		(yyerrstatus = 0)
@@ -1781,8 +1787,8 @@ yyreduce:
   case 48:
 
     {
-	struct symbol *sym = sym_lookup(NULL, 0);
-	sym->flags |= SYMBOL_CHOICE;
+	struct symbol *sym = sym_lookup((yyvsp[(2) - (3)].string), 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());
@@ -2014,7 +2020,12 @@ yyreduce:
 
   case 108:
 
-    { (yyval.symbol) = sym_lookup((yyvsp[(1) - (1)].string), 1); free((yyvsp[(1) - (1)].string)); ;}
+    { (yyval.symbol) = sym_lookup((yyvsp[(1) - (1)].string), SYMBOL_CONST); free((yyvsp[(1) - (1)].string)); ;}
+    break;
+
+  case 109:
+
+    { (yyval.string) = NULL; ;}
     break;
 
 
Index: linux-2.6/scripts/kconfig/zconf.y
===================================================================
--- linux-2.6.orig/scripts/kconfig/zconf.y
+++ 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)

Re: [PATCH] x86: vSMP selection in config

[Sam Ravnborg]

On Sun, Feb 24, 2008 at 10:43:49PM -0800, Yinghai Lu wrote:
> 
> 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

Re: [PATCH] x86: vSMP selection in config

[Yinghai Lu]

On Tuesday 26 February 2008 12:05:12 pm Sam Ravnborg wrote:
> On Sun, Feb 24, 2008 at 10:43:49PM -0800, Yinghai Lu wrote:
> > 
> > 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.

also Kconfig seems ignore
if PARAVIRT_GUEST

endif

and 
to use config PARAVIRT in that if block.

YH

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

[Yinghai Lu]

On Sun, Feb 24, 2008 at 4:29 AM, Andi Kleen <andi@firstfloor.org> wrote:
> On Sat, Feb 23, 2008 at 09:48:42PM -0800, Yinghai Lu wrote:
>  >
>  > 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

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

[Ravikiran Thirumalai]

On Sun, Feb 24, 2008 at 01:29:03PM +0100, Andi Kleen wrote:
>On Sat, Feb 23, 2008 at 09:48:42PM -0800, Yinghai Lu wrote:
>> 
>> 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 ...
>
>> 
>> 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

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

[Yinghai Lu]

On Mon, Feb 25, 2008 at 11:08 AM, Ravikiran Thirumalai
<kiran@scalemp.com> wrote:
> On Sun, Feb 24, 2008 at 01:29:03PM +0100, Andi Kleen wrote:
>  >On Sat, Feb 23, 2008 at 09:48:42PM -0800, Yinghai Lu wrote:
>  >>
>  >> 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 ...
>  >
>  >>
>  >> 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 :(.

please check if you happy with

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

Thanks

Yinghai Lu

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

[Ravikiran Thirumalai]

On Mon, Feb 25, 2008 at 02:05:45PM -0800, Yinghai Lu wrote:
>On Mon, Feb 25, 2008 at 11:08 AM, Ravikiran Thirumalai
><kiran@scalemp.com> wrote:
>> ...
>>  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

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

[Andi Kleen]

> 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

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

[Ravikiran Thirumalai]

On Tue, Feb 26, 2008 at 04:46:25AM +0100, Andi Kleen wrote:
>> 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!

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

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

[Yinghai Lu]

On Mon, Feb 25, 2008 at 8:05 PM, Ravikiran Thirumalai <kiran@scalemp.com> wrote:
> On Tue, Feb 26, 2008 at 04:46:25AM +0100, Andi Kleen wrote:
>  >> 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!
>
>
>  >
>  >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

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

[Ravikiran Thirumalai]

On Mon, Feb 25, 2008 at 09:27:42PM -0800, Yinghai Lu wrote:
>On Mon, Feb 25, 2008 at 8:05 PM, Ravikiran Thirumalai <kiran@scalemp.com> wrote:
>> On Tue, Feb 26, 2008 at 04:46:25AM +0100, Andi Kleen wrote:
>>
>>  >
>>  >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

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

[Yinghai Lu]

On Tue, Feb 26, 2008 at 10:42 AM, Ravikiran Thirumalai
<kiran@scalemp.com> wrote:
> On Mon, Feb 25, 2008 at 09:27:42PM -0800, Yinghai Lu wrote:
>  >On Mon, Feb 25, 2008 at 8:05 PM, Ravikiran Thirumalai <kiran@scalemp.com> wrote:
>  >> On Tue, Feb 26, 2008 at 04:46:25AM +0100, Andi Kleen wrote:
>  >>
>  >>  >
>
> >>  >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)

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

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

[Ravikiran Thirumalai]

On Tue, Feb 26, 2008 at 11:00:58AM -0800, Yinghai Lu wrote:
>
>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.

>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

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

[Yinghai Lu]

On Tue, Feb 26, 2008 at 12:32 PM, Ravikiran Thirumalai
<kiran@scalemp.com> wrote:
> On Tue, Feb 26, 2008 at 11:00:58AM -0800, Yinghai Lu wrote:
>  >
>  >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.
>
>
>  >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.
Yes

YH

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

[Yinghai Lu]

On Tue, Feb 26, 2008 at 12:32 PM, Ravikiran Thirumalai
<kiran@scalemp.com> wrote:
> On Tue, Feb 26, 2008 at 11:00:58AM -0800, Yinghai Lu wrote:
>  >
>  >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

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

[Ravikiran Thirumalai]

On Tue, Feb 26, 2008 at 01:10:57PM -0800, Yinghai Lu wrote:
>On Tue, Feb 26, 2008 at 12:32 PM, Ravikiran Thirumalai
><kiran@scalemp.com> wrote:
>> On Tue, Feb 26, 2008 at 11:00:58AM -0800, Yinghai Lu wrote:
>>  >
>>  >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.

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

[Yinghai Lu]

On Tue, Feb 26, 2008 at 1:24 PM, Ravikiran Thirumalai <kiran@scalemp.com> wrote:
>
> On Tue, Feb 26, 2008 at 01:10:57PM -0800, Yinghai Lu wrote:
>  >On Tue, Feb 26, 2008 at 12:32 PM, Ravikiran Thirumalai
>  ><kiran@scalemp.com> wrote:
>  >> On Tue, Feb 26, 2008 at 11:00:58AM -0800, Yinghai Lu wrote:
>  >>  >
>  >>  >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.

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

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

[Ravikiran Thirumalai]

On Tue, Feb 26, 2008 at 03:16:16PM -0800, Yinghai Lu wrote:
>On Tue, Feb 26, 2008 at 1:24 PM, Ravikiran Thirumalai <kiran@scalemp.com> wrote:
>>
>> On Tue, Feb 26, 2008 at 01:10:57PM -0800, Yinghai Lu wrote:
>>  >On Tue, Feb 26, 2008 at 12:32 PM, Ravikiran Thirumalai
>>  ><kiran@scalemp.com> wrote:
>>  >> On Tue, Feb 26, 2008 at 11:00:58AM -0800, Yinghai Lu wrote:
>>  >>  >
>>  >>  >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.
>
>srat only have apicid to node mapping, and ram to node mapping.

Which is very important for performance.

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

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

[Yinghai Lu]

On Tue, Feb 26, 2008 at 3:31 PM, Ravikiran Thirumalai <kiran@scalemp.com> wrote:
> On Tue, Feb 26, 2008 at 03:16:16PM -0800, Yinghai Lu wrote:
>  >On Tue, Feb 26, 2008 at 1:24 PM, Ravikiran Thirumalai <kiran@scalemp.com> wrote:
>  >>
>  >> On Tue, Feb 26, 2008 at 01:10:57PM -0800, Yinghai Lu wrote:
>  >>  >On Tue, Feb 26, 2008 at 12:32 PM, Ravikiran Thirumalai
>  >>  ><kiran@scalemp.com> wrote:
>  >>  >> On Tue, Feb 26, 2008 at 11:00:58AM -0800, Yinghai Lu wrote:
>  >>  >>  >
>  >>  >>  >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.
>  >
>  >srat only have apicid to node mapping, and ram to node mapping.
>
>  Which is very important for performance.
>
>
>  >
>  >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

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

[Yinghai Lu]

On Friday 22 February 2008 11:02:30 am Yinghai Lu wrote:
> On Friday 22 February 2008 04:25:18 am Andi Kleen wrote:
> > Yinghai Lu <Yinghai.Lu@Sun.COM> writes:
> > 
> > > 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?
> > 
> > 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.
> 
> 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

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

[Andi Kleen]

> or move
> CPUs have X86_FEATURE_CONSTANT_TSC check before apic_clustered check?

No that would be not correct on Intel boxes.

-Andi