[PATCH] x86: add /proc/cpuinfo/physical id quirks

[PATCH] x86: add /proc/cpuinfo/physical id quirks

[Alex Chiang]

As systems become larger and more complex, it is not always possible
to assume that an APIC ID maps directly to a given physical slot.

>From a UI point-of-view, it's nice if the 'physical id' field in
/proc/cpuinfo matches the silk-screening or labelling on the system
chassis.

Add a quirk that allows oddball platforms to ensure that what the kernel
displays in /proc/cpuinfo matches the physical reality.

Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Alex Chiang <achiang@hp.com>
---
 Makefile        |    2 +-
 common.c        |    5 +++++
 cpu.h           |    8 ++++++++
 physid_quirks.c |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 68 insertions(+), 1 deletion(-)

---
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 3efcb2b..95d54cf 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -9,7 +9,7 @@ endif
 
 obj-y			:= intel_cacheinfo.o addon_cpuid_features.o
 obj-y			+= proc.o capflags.o powerflags.o common.o
-obj-y			+= vmware.o hypervisor.o
+obj-y			+= vmware.o hypervisor.o physid_quirks.o
 
 obj-$(CONFIG_X86_32)	+= bugs.o cmpxchg.o
 obj-$(CONFIG_X86_64)	+= bugs_64.o
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 54a3ead..714c56f 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -815,6 +815,11 @@ static void __cpuinit identify_cpu(struct cpuinfo_x86 *c)
 	detect_ht(c);
 #endif
 
+	physid_fixup_table = NULL;
+	dmi_check_system(physid_need_fixups_table);
+	if (physid_fixup_table)
+		c->phys_proc_id = physid_fixup_table[c->phys_proc_id];
+
 	init_hypervisor(c);
 
 	/*
diff --git a/arch/x86/kernel/cpu/cpu.h b/arch/x86/kernel/cpu/cpu.h
index 6de9a90..5ae221e 100644
--- a/arch/x86/kernel/cpu/cpu.h
+++ b/arch/x86/kernel/cpu/cpu.h
@@ -8,6 +8,8 @@ struct cpu_model_info {
 	const char	*model_names[16];
 };
 
+struct cpuinfo_x86;
+
 /* attempt to consolidate cpu attributes */
 struct cpu_dev {
 	const char	*c_vendor;
@@ -34,4 +36,10 @@ extern const struct cpu_dev *const __x86_cpu_dev_start[],
 
 extern void display_cacheinfo(struct cpuinfo_x86 *c);
 
+/* defined in physid_quirks.c */
+#include <linux/dmi.h>
+extern int (*physid_fixup_table);
+extern int hp_check_fixup_physids(const struct dmi_system_id *);
+extern struct dmi_system_id physid_need_fixups_table[];
+
 #endif
diff --git a/arch/x86/kernel/cpu/physid_quirks.c b/arch/x86/kernel/cpu/physid_quirks.c
new file mode 100644
index 0000000..40cf429
--- /dev/null
+++ b/arch/x86/kernel/cpu/physid_quirks.c
@@ -0,0 +1,54 @@
+/*
+ *  physid_quirks.c - quirks for the 'physical id' field in /proc/cpuinfo
+ *
+ *  Copyright (C) 2009 Hewlett-Packard Development Company, L.P.
+ *	Alex Chiang <achiang@hp.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under the terms and conditions of the GNU General Public License,
+ *  version 2, as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "cpu.h"
+
+int (*physid_fixup_table);
+
+struct dmi_system_id __initdata physid_need_fixups_table[] = {
+	{
+		.callback = hp_check_fixup_physids,
+		.ident = "HP ProLiant DL785 G5/G6",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "ProLiant DL785"),
+		},
+	},
+	{}
+};
+
+/*
+ * The mapping from node to physical socket id is described
+ * in the (HP internal) DL785 system specification.
+ */
+static int hp_dl785_physids_4p[8] = { 8, 2, 7, 1, 0, 0, 0, 0 };
+static int hp_dl785_physids_8p[8] = { 8, 6, 7, 4, 3, 5, 2, 1 };
+int __cpuinit hp_check_fixup_physids(const struct dmi_system_id *d)
+{
+	int present = num_present_cpus();
+
+	/* DL785 only supports 4-socket and 8-socket configs */
+	if (present == 16 || present == 24)
+		physid_fixup_table = hp_dl785_physids_4p;
+	else
+		physid_fixup_table = hp_dl785_physids_8p;
+
+	return 1;
+}

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Suresh Siddha]

On Fri, 2009-08-14 at 09:36 -0700, Alex Chiang wrote:
> As systems become larger and more complex, it is not always possible
> to assume that an APIC ID maps directly to a given physical slot.
> 
> From a UI point-of-view, it's nice if the 'physical id' field in
> /proc/cpuinfo matches the silk-screening or labelling on the system
> chassis.
> 
> Add a quirk that allows oddball platforms to ensure that what the kernel
> displays in /proc/cpuinfo matches the physical reality.

Alex, Does it makes sense to add a new entry in /proc/cpuinfo rather
than overloading the 'physical id' by modifying phys_proc_id.

That way, even if there is a mis-match between the bios and the OS fixup
tables, we won't screw up other topology setup etc in the kernel that
are dependent on the phys_proc_id.

thanks,
suresh

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Alex Chiang]

* Suresh Siddha <suresh.b.siddha@intel.com>:
> On Fri, 2009-08-14 at 09:36 -0700, Alex Chiang wrote:
> > As systems become larger and more complex, it is not always possible
> > to assume that an APIC ID maps directly to a given physical slot.
> > 
> > From a UI point-of-view, it's nice if the 'physical id' field in
> > /proc/cpuinfo matches the silk-screening or labelling on the system
> > chassis.
> > 
> > Add a quirk that allows oddball platforms to ensure that what the kernel
> > displays in /proc/cpuinfo matches the physical reality.
> 
> Alex, Does it makes sense to add a new entry in /proc/cpuinfo rather
> than overloading the 'physical id' by modifying phys_proc_id.

Hm, I'm not entirely sure about that, for two reasons.

First (and this is the weaker reason), I'd prefer not to keep
adding new fields to /proc/cpuinfo if we can help it, as it just
makes for a continually more complicated ABI/API for userspace.

Second, I guess I'm not sure what else 'physical id' /should/
represent. I'm willing to be corrected on this point, so if I'm
wrong, just call it simple ignorance. :)

> That way, even if there is a mis-match between the bios and the
> OS fixup tables, we won't screw up other topology setup etc in
> the kernel that are dependent on the phys_proc_id.

My quick grep earlier led me to believe that as long as the
phys_proc_ids were /consistent/ then it didn't seem to matter
what their /values/ were.

In other words, my patch simply says, "all cores that had
phys_proc_id X now have phys_proc_id Y". All the cores on a
physical package have identical phys_proc_ids, and cores on a
different physical package do /not/ collide.

But again, that just might be my ignorance again. If we do indeed
care about the values of phys_proc_ids, please let me know and
I'd be happy to rework the patch.

Thanks.

/ac

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Suresh Siddha]

On Fri, 2009-08-14 at 12:27 -0700, Alex Chiang wrote:
> Hm, I'm not entirely sure about that, for two reasons.
> 
> First (and this is the weaker reason), I'd prefer not to keep
> adding new fields to /proc/cpuinfo if we can help it, as it just
> makes for a continually more complicated ABI/API for userspace.

Overriding same field with different values based on the system will be
confusing. On some systems it will be derived from cpuid, some will be
based on physical slots. This is too confusing.

> Second, I guess I'm not sure what else 'physical id' /should/
> represent. I'm willing to be corrected on this point, so if I'm
> wrong, just call it simple ignorance. :)

As far as possible we kept these fields closer to what the cpuid
instruction says, so that firmware won't mess up the topology detection
etc by having wrong values in the bios tables.

> My quick grep earlier led me to believe that as long as the
> phys_proc_ids were /consistent/ then it didn't seem to matter
> what their /values/ were.

This is correct. But there are several assumptions in the patch that
might break in the future and where ever possible we simply don't want
to depend on what bios says and rather depend on what the cpu/hardware
says.

For example, we shouldn't assume that original phys proc id's calculated
from cpuid etc need not be contiguous and start from 0 etc. This is
platform dependent and may vary from one version to another version of
processor etc.

Also, you are selecting the fixup table based on number of present cpu
etc. I am just nervous that how this all workout across cpu generations
having different cores, sparsely populated sockets in the platform etc.
We just can't afford to go and fix all the old kernels if we have any
problem in the future config setups.

Easiest route will be to add a new entry in /proc/cpuinfo

thanks,
suresh

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Alex Chiang]

Hi Suresh,

Thanks for having the patience to educate me. I appreciate the
effort. :)

A few more questions...

* Suresh Siddha <suresh.b.siddha@intel.com>:
> On Fri, 2009-08-14 at 12:27 -0700, Alex Chiang wrote:
> > Hm, I'm not entirely sure about that, for two reasons.
> > 
> > First (and this is the weaker reason), I'd prefer not to keep
> > adding new fields to /proc/cpuinfo if we can help it, as it just
> > makes for a continually more complicated ABI/API for userspace.
> 
> Overriding same field with different values based on the system will be
> confusing. On some systems it will be derived from cpuid, some will be
> based on physical slots. This is too confusing.
> 
> > Second, I guess I'm not sure what else 'physical id' /should/
> > represent. I'm willing to be corrected on this point, so if I'm
> > wrong, just call it simple ignorance. :)
> 
> As far as possible we kept these fields closer to what the cpuid
> instruction says, so that firmware won't mess up the topology detection
> etc by having wrong values in the bios tables.

I understand your point about keeping the output of /proc/cpuinfo
close to what the cpuid instruction says.

But in regard to the particular field that we're talking about
here -- 'physical id' -- that doesn't seem to be represented from
cpuid anyway. We're stuffing an APIC ID into that field, even
when we already have other APIC ID output.

So my point was that we're already kinda doing something funny to
'physical id' and I'd like to fix it for my platform so that it
matches the actual chassis.

> > My quick grep earlier led me to believe that as long as the
> > phys_proc_ids were /consistent/ then it didn't seem to matter
> > what their /values/ were.
> 
> This is correct. But there are several assumptions in the patch that
> might break in the future and where ever possible we simply don't want
> to depend on what bios says and rather depend on what the cpu/hardware
> says.
> 
> For example, we shouldn't assume that original phys proc id's calculated
> from cpuid etc need not be contiguous and start from 0 etc. This is
> platform dependent and may vary from one version to another version of
> processor etc.

Sorry, I'm having a hard time parsing this sentence. Do you mean
to say:

	we shouldn't assume that phys_proc_id calculated from
	cpuid are contiguous and start from 0

?

I agree with you (although I thought that they should be 0-based)
but this quirk addresses a specific platform, where I can assume
certain things about the BIOS, etc.

> Also, you are selecting the fixup table based on number of present cpu
> etc. I am just nervous that how this all workout across cpu generations
> having different cores, sparsely populated sockets in the platform etc.
> We just can't afford to go and fix all the old kernels if we have any
> problem in the future config setups.
 
I agree with you in general, but again, this is a specific
platform quirk where I have a good idea of what is a supported
configuration.

I take your point though, and can rework the patch so that if we
don't find a configuration that we're expecting (either 4p or 8p)
then just revert to the default behaviour and don't break
anything.

> Easiest route will be to add a new entry in /proc/cpuinfo

Well, if you remain unconvinced that fixing up 'physical id' is
the proper thing to do, here are some alternate proposals:

	/proc/cpuinfo/chassis id
	/sys/devices/system/cpu/$cpu/chassis id
	/sys/devices/system/cpu/$cpu/topology/chassis id

Thoughts?

Thanks.

/ac

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Suresh Siddha]

On Wed, 2009-08-19 at 14:02 -0700, Alex Chiang wrote:
> I understand your point about keeping the output of /proc/cpuinfo
> close to what the cpuid instruction says.
> 
> But in regard to the particular field that we're talking about
> here -- 'physical id' -- that doesn't seem to be represented from
> cpuid anyway. We're stuffing an APIC ID into that field, even
> when we already have other APIC ID output.

For the current generation platforms, APIC ID's are typically from
cpuid, except for large SGI UV platforms etc.

> > For example, we shouldn't assume that original phys proc id's calculated
> > from cpuid etc need not be contiguous and start from 0 etc. This is
> > platform dependent and may vary from one version to another version of
> > processor etc.
> 
> Sorry, I'm having a hard time parsing this sentence. Do you mean
> to say:
> 
> 	we shouldn't assume that phys_proc_id calculated from
> 	cpuid are contiguous and start from 0
> 
> ?

yes.

> 
> I agree with you (although I thought that they should be 0-based)
> but this quirk addresses a specific platform, where I can assume
> certain things about the BIOS, etc.

What happens if for some reason, newer bios/newer cpu generations on
this platform start having holes in the physical id space? We can't rule
out these kind of changes and we don't want to go behind distros
requesting fixes.

> I agree with you in general, but again, this is a specific
> platform quirk where I have a good idea of what is a supported
> configuration.

I am just nervous about future bios changes etc.

> > Easiest route will be to add a new entry in /proc/cpuinfo
> 
> Well, if you remain unconvinced that fixing up 'physical id' is
> the proper thing to do, here are some alternate proposals:
> 
> 	/proc/cpuinfo/chassis id
> 	/sys/devices/system/cpu/$cpu/chassis id
> 	/sys/devices/system/cpu/$cpu/topology/chassis id
> 

I really like this alternate proposal. This is simple and straight
forward to everyone.

thanks,
suresh

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Alex Chiang]

* Suresh Siddha <suresh.b.siddha@intel.com>:
> On Wed, 2009-08-19 at 14:02 -0700, Alex Chiang wrote:
> > I agree with you (although I thought that they should be
> > 0-based) but this quirk addresses a specific platform, where
> > I can assume certain things about the BIOS, etc.
> 
> What happens if for some reason, newer bios/newer cpu
> generations on this platform start having holes in the physical
> id space? We can't rule out these kind of changes and we don't
> want to go behind distros requesting fixes.
> 
> > I agree with you in general, but again, this is a specific
> > platform quirk where I have a good idea of what is a
> > supported configuration.
> 
> I am just nervous about future bios changes etc.

Ok, let's separate the two conversations happening here.

To me, the BIOS concerns are moot. I work closely with the BIOS
engineers for this platform; I have knowledge of future plans for
this BIOS and platform; and I know that they will not make any
changes that break the assumptions in my patch.

If they do, we will catch it during platform testing, file a bug,
and not let them release their BIOS until it's fixed. Does that
satisfy you? :)

So the algorithm for mapping an APIC ID to a physical/chassis ID
for this platform will not ever change.

Now on the other hand, the /interface/ that we present to the
user is the interesting conversation to have.

> > > Easiest route will be to add a new entry in /proc/cpuinfo
> > 
> > Well, if you remain unconvinced that fixing up 'physical id' is
> > the proper thing to do, here are some alternate proposals:
> > 
> > 	/proc/cpuinfo/chassis id
> > 	/sys/devices/system/cpu/$cpu/chassis id
> > 	/sys/devices/system/cpu/$cpu/topology/chassis id
> > 
> 
> I really like this alternate proposal. This is simple and straight
> forward to everyone.

I am leaning towards sysfs, and prefer:

 	/sys/devices/system/cpu/$cpu/chassis_id

How does that sound?

Thanks.

/ac

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Andi Kleen]

> I am leaning towards sysfs, and prefer:
> 
>  	/sys/devices/system/cpu/$cpu/chassis_id
> 
> How does that sound?

I would prefer to simply use the existing physical id for this like
in your original patch.  We already have a bewildering zoo of different
CPU IDs, no need to increase the confusion even more.

Incidentially mcelog already knows how to use physical ID for this,
would need to be changed for a new sysfs interface

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Suresh Siddha]

On Thu, 2009-08-20 at 13:54 -0700, Alex Chiang wrote:
> To me, the BIOS concerns are moot. I work closely with the BIOS
> engineers for this platform; I have knowledge of future plans for
> this BIOS and platform; and I know that they will not make any
> changes that break the assumptions in my patch.
> 
> If they do, we will catch it during platform testing, file a bug,
> and not let them release their BIOS until it's fixed. Does that
> satisfy you? :)
> 
> So the algorithm for mapping an APIC ID to a physical/chassis ID
> for this platform will not ever change.

What happens if there is some genuine reason to do that because of some
other platform workaround etc?


> I am leaning towards sysfs, and prefer:
> 
>  	/sys/devices/system/cpu/$cpu/chassis_id
> 

Looks fine to me.

thanks,
suresh

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Alex Chiang]

* Andi Kleen <andi@firstfloor.org>:
> > I am leaning towards sysfs, and prefer:
> > 
> >  	/sys/devices/system/cpu/$cpu/chassis_id
> > 
> > How does that sound?
> 
> I would prefer to simply use the existing physical id for this
> like in your original patch.  We already have a bewildering zoo
> of different CPU IDs, no need to increase the confusion even
> more.

This was my initial instinct as well, and was why I was trying to
find out the original intent of "physical id". It seemed
unambiguous to me, but I was trying to figure out what history
(if any) there was around its semantics.

Since "physical id" is basically a made-up field (i.e., isn't
represented directly in cpuid instruction), it does seem like the
appropriate place for any quirky platforms to hook into if
necessary.

> Incidentially mcelog already knows how to use physical ID for this,
> would need to be changed for a new sysfs interface

There is probably a lot of userspace stuff that looks at physical
id that would need to be taught about a new interface.

It turns out I need to rework my patch anyway because I need to
think about the case where a user disables some cores in the
BIOS, in which case my (fragile) table selection scheme falls
apart.

But I would like to get some agreement/guidance about how we
end up presenting the information to the user.

Thanks.

/ac

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Suresh Siddha]

On Thu, 2009-08-20 at 14:03 -0700, Andi Kleen wrote:
> > I am leaning towards sysfs, and prefer:
> > 
> >  	/sys/devices/system/cpu/$cpu/chassis_id
> > 
> > How does that sound?
> 
> I would prefer to simply use the existing physical id for this like
> in your original patch.  We already have a bewildering zoo of different
> CPU IDs, no need to increase the confusion even more.
> 
> Incidentially mcelog already knows how to use physical ID for this,
> would need to be changed for a new sysfs interface

Andi, Based on our past experiences, I am nervous if there are some
software/firmware interactions that can negatively affect the cpu
topology detection.

If we really think overloading the "physical id" helps with existing
software, then I am ok with changing the user visible part of the /proc/
or /sys files but not with the kernel's per cpu info's phys_proc_id,
which is used for topology detection etc.

thanks,
suresh

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Suresh Siddha]

On Thu, 2009-08-20 at 14:20 -0700, Alex Chiang wrote:
> It turns out I need to rework my patch anyway because I need to
> think about the case where a user disables some cores in the
> BIOS, in which case my (fragile) table selection scheme falls
> apart.

These are the sort of reasons why we want topology detection to be
completely based on what cpuid instruction says and nothing else.

thanks,
suresh

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[H. Peter Anvin]

On 08/20/2009 02:26 PM, Suresh Siddha wrote:
> On Thu, 2009-08-20 at 14:20 -0700, Alex Chiang wrote:
>> It turns out I need to rework my patch anyway because I need to
>> think about the case where a user disables some cores in the
>> BIOS, in which case my (fragile) table selection scheme falls
>> apart.
> 
> These are the sort of reasons why we want topology detection to be
> completely based on what cpuid instruction says and nothing else.
> 

I agree... if this ID is used for topology detection, we shouldn't
replace it arbitrarily with information from BIOS just to hope that it
matches the motherboard stencil.  *Furthermore*, there is no reason why
motherboard stencilAs are purely numeric... consider the rather obvious
case of two rows of four CPUs; they may have CPU slots labelled A1, A2,
A3, A4, B1, B2, B3, B4.  It might very well be the right thing to
support arbitrary strings for platforms we recognize.

As such I think we should have a socket name field in both /proc/cpuinfo
and sysfs.

	-hpa

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Alex Chiang]

* H. Peter Anvin <hpa@zytor.com>:
> On 08/20/2009 02:26 PM, Suresh Siddha wrote:
> > On Thu, 2009-08-20 at 14:20 -0700, Alex Chiang wrote:
> >> It turns out I need to rework my patch anyway because I need
> >> to think about the case where a user disables some cores in
> >> the BIOS, in which case my (fragile) table selection scheme
> >> falls apart.
> > 
> > These are the sort of reasons why we want topology detection
> > to be completely based on what cpuid instruction says and
> > nothing else.
> 
> I agree... if this ID is used for topology detection, we
> shouldn't replace it arbitrarily with information from BIOS
> just to hope that it matches the motherboard stencil.

Well, to be fair, in my patch, the phys_proc_ids remained
consistent for a given physical processor package.

That is, if all the cores in a socket initially started with
phys_proc_id X, they all ended up with phys_proc_id Y.
Additionally, no other cores in other sockets ended up with Y.
So in that sense, there was no real change in how we detected CPU
topology. All the core/thread/sibling ids remained correct and
unchanged.

Second, my patch wasn't sticking arbitrary information in from
BIOS; it was merely rearranging the existing phys_proc_ids.

> *Furthermore*, there is no reason why motherboard stencilAs are
> purely numeric... consider the rather obvious case of two rows
> of four CPUs; they may have CPU slots labelled A1, A2, A3, A4,
> B1, B2, B3, B4.  It might very well be the right thing to
> support arbitrary strings for platforms we recognize.

But yes, I do agree that this is a good point, and even here at
HP, we have other platforms with complex physical topologies.

Once you go down this path of thinking though, ACPI starts
looming, and that has other issues associated with it (in my
mind, mostly around the cleanliness of implementation as ACPI
starts poking its fingers everywhere).

> As such I think we should have a socket name field in both
> /proc/cpuinfo and sysfs.

Well, now we have 2 yeas (hpa and Suresh), 1 nay (Andi), and 1
"tell me what you'd like" (me ;)

/ac

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[H. Peter Anvin]

On 08/20/2009 02:59 PM, Alex Chiang wrote:
> 
> Once you go down this path of thinking though, ACPI starts
> looming, and that has other issues associated with it (in my
> mind, mostly around the cleanliness of implementation as ACPI
> starts poking its fingers everywhere).
> 

Seems more like DMI than ACPI to me, but this kind of stuff is already
creeping into everything no matter what.

*However* the whole point of this is to not muck with data structures
that the kernel actually cares about in any way, and have a separate
field which is only for human consumption.

	-hpa

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Andi Kleen]

> I agree... if this ID is used for topology detection, we shouldn't
> replace it arbitrarily with information from BIOS just to hope that it
> matches the motherboard stencil.  *Furthermore*, there is no reason why
> motherboard stencilAs are purely numeric... consider the rather obvious
> case of two rows of four CPUs; they may have CPU slots labelled A1, A2,
> A3, A4, B1, B2, B3, B4.  It might very well be the right thing to
> support arbitrary strings for platforms we recognize.

Maintaining a manual mapping to strings in the kernel to such strings 
would be just crazy. You would need a new entry for basically
every system.

The reason to correct SOCKETID is that it it is output on errors.
If it is numerical and you know it's wrong you can correct it,
and then you can identify the right CPU. Otherwise you lose.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Andi Kleen]

> If we really think overloading the "physical id" helps with existing
> software, then I am ok with changing the user visible part of the /proc/
> or /sys files but not with the kernel's per cpu info's phys_proc_id,
> which is used for topology detection etc.

It's not just the cpuinfo, but also the socketid reported for
machine check. But yes converting it only for those two 
would be a possibility.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[H. Peter Anvin]

On 08/20/2009 05:32 PM, Andi Kleen wrote:
>> I agree... if this ID is used for topology detection, we shouldn't
>> replace it arbitrarily with information from BIOS just to hope that it
>> matches the motherboard stencil.  *Furthermore*, there is no reason why
>> motherboard stencilAs are purely numeric... consider the rather obvious
>> case of two rows of four CPUs; they may have CPU slots labelled A1, A2,
>> A3, A4, B1, B2, B3, B4.  It might very well be the right thing to
>> support arbitrary strings for platforms we recognize.
> 
> Maintaining a manual mapping to strings in the kernel to such strings 
> would be just crazy. You would need a new entry for basically
> every system.
> 
> The reason to correct SOCKETID is that it it is output on errors.
> If it is numerical and you know it's wrong you can correct it,
> and then you can identify the right CPU. Otherwise you lose.
> 

You're not making any sense.  You seem to imply that restricting it to a
numerical ID makes it somehow easier, but it's *the same problem*.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86: add /proc/cpuinfo/physical id quirks

[Alex Chiang]

* Andi Kleen <andi@firstfloor.org>:
> > I agree... if this ID is used for topology detection, we
> > shouldn't replace it arbitrarily with information from BIOS
> > just to hope that it matches the motherboard stencil.
> > *Furthermore*, there is no reason why motherboard stencilAs
> > are purely numeric... consider the rather obvious case of two
> > rows of four CPUs; they may have CPU slots labelled A1, A2,
> > A3, A4, B1, B2, B3, B4.  It might very well be the right
> > thing to support arbitrary strings for platforms we
> > recognize.
> 
> Maintaining a manual mapping to strings in the kernel to such
> strings would be just crazy. You would need a new entry for
> basically every system.
 
Well, ideally we could read it from a standard location, say
evaluating the ACPI _SUN method for the cpu object and then plug
that answer into a standard kernel data structure.

Hardware vendors can stick whatever they want into that method;
the kernel just passes it through to userspace.

It's what we do for PCI slots today, and I wrote a patch for ia64
that does that type of fixup: fe086a7.

> The reason to correct SOCKETID is that it it is output on errors.
> If it is numerical and you know it's wrong you can correct it,
> and then you can identify the right CPU. Otherwise you lose.

I think numerical vs. ascii is the wrong way to think about it,
since ACPI could provide either.

/ac