x86: add /proc/cpuinfo/physical id quirks
PATCH 1/1 x86: add /proc/cpuinfo/physical id quirks
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.
Quoted 1 line
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.
--- Makefile | 2 +- common.c | 5 +++++ cpu.h | 8 ++++++++ physid_quirks.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 1 deletion(-)
---
arch/x86/kernel/cpu/Makefile +1 -1
@@ -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
arch/x86/kernel/cpu/common.c +5 -0
@@ -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);/*
arch/x86/kernel/cpu/cpu.h +8 -0
@@ -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
arch/x86/kernel/cpu/physid_quirks.c new file +54 -0
54 lines not on this page · raw message
Quoted 9 lines · 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/cpuinfomatches 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/cpuinfomatches 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
* Suresh Siddha <[email protected]>:
Quoted 8 lines · Alex Chiang
Quoted 7 lines
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/cpuinfomatches the silk-screening or labelling on the system chassis.
Quoted 5 lines
Quoted 2 lines
Add a quirk that allows oddball platforms to ensure that what the kernel displays in
/proc/cpuinfomatches the physical reality.Alex, Does it makes sense to add a new entry in
/proc/cpuinforather than overloading the 'physical id' by modifyingphys_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. :)
Quoted 3 lines
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
Quoted 5 lines · Alex Chiang
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/cpuinfoif 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.
Quoted 3 lines
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.
Quoted 3 lines
My quick grep earlier led me to believe that as long as the
phys_proc_idswere /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
Hi Suresh,
Thanks for having the patience to educate me. I appreciate the effort. :)
A few more questions...
* Suresh Siddha <[email protected]>:
Quoted 11 lines · Alex Chiang
Quoted 5 lines
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/cpuinfoif 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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 9 lines
Quoted 3 lines
My quick grep earlier led me to believe that as long as the
phys_proc_idswere /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.
Quoted 4 lines
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.
Quoted 5 lines
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.
Quoted 1 line
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
Quoted 7 lines · Alex Chiang
I understand your point about keeping the output of
/proc/cpuinfoclose 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.
Quoted 5 lines
Quoted 4 lines
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.
Quoted 7 lines
Sorry, I'm having a hard time parsing this sentence. Do you mean to say:
we shouldn't assume that
phys_proc_idcalculated from cpuid are contiguous and start from 0?
yes.
Quoted 4 lines
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.
Quoted 3 lines
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.
Quoted 9 lines
Quoted 1 line
Easiest route will be to add a new entry in
/proc/cpuinfoWell, if you remain unconvinced that fixing up 'physical id' is the proper thing to do, here are some alternate proposals:
/proc/cpuinfo/chassisid/sys/devices/system/cpu/$cpu/chassisid/sys/devices/system/cpu/$cpu/topology/chassisid
I really like this alternate proposal. This is simple and straight forward to everyone.
thanks,
suresh
* Suresh Siddha <[email protected]>:
Quoted 10 lines · Alex Chiang
Quoted 3 lines
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.
Quoted 5 lines
Quoted 3 lines
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.
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
Easiest route will be to add a new entry in
/proc/cpuinfoWell, if you remain unconvinced that fixing up 'physical id' is the proper thing to do, here are some alternate proposals:
Quoted 7 lines
Quoted 4 lines
/proc/cpuinfo/chassisid/sys/devices/system/cpu/$cpu/chassisid/sys/devices/system/cpu/$cpu/topology/chassisidI 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
Quoted 5 lines
I am leaning towards sysfs, and prefer:
/sys/devices/system/cpu/$cpu/chassis_idHow 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
Signature · 1 line
[email protected] -- Speaking for myself only.
* Andi Kleen <[email protected]>:
Quoted 10 lines
Quoted 5 lines
I am leaning towards sysfs, and prefer:
/sys/devices/system/cpu/$cpu/chassis_idHow 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.
Quoted 2 lines
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
Quoted 4 lines · Alex Chiang
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
H. Peter Anvin
reviewerQuoted 9 lines · Suresh Siddha
Quoted 4 lines · Alex Chiang
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
* H. Peter Anvin <[email protected]>:
Quoted 7 lines · Suresh Siddha
Quoted 6 lines · Alex Chiang
Quoted 4 lines
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 5 lines
*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).
Quoted 2 lines
As such I think we should have a socket name field in both
/proc/cpuinfoand sysfs.
Well, now we have 2 yeas (hpa and Suresh), 1 nay (Andi), and 1 "tell me what you'd like" (me ;)
/ac
H. Peter Anvin
reviewerQuoted 6 lines · Alex Chiang
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
Quoted 7 lines
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
Signature · 1 line
[email protected] -- Speaking for myself only.
H. Peter Anvin
reviewerQuoted 8 lines · Andi Kleen
Quoted 7 lines
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.
Quoted 8 lines
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
Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf.
* Andi Kleen <[email protected]>:
Quoted 10 lines
Quoted 9 lines
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.
Quoted 3 lines
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.
Quoted 3 lines
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
Quoted 4 lines · Andi Kleen
Quoted 4 lines
I am leaning towards sysfs, and prefer:
/sys/devices/system/cpu/$cpu/chassis_id
Quoted 8 lines
Quoted 1 line
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
Quoted 4 lines
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'sphys_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
Signature · 1 line
[email protected] -- Speaking for myself only.
Quoted 11 lines · Alex Chiang
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?
Quoted 4 lines
I am leaning towards sysfs, and prefer:
/sys/devices/system/cpu/$cpu/chassis_id
Looks fine to me.
thanks,
suresh