[PATCH] x86: Report Intel platform_id in /proc/cpuinfo
From: Andi KleenWe have a need to distinguish systems based on their platform ID. For example this is useful to distinguish systems with L4 cache versus ones without. There is a 3 bit identifier (also called processor flags) in the IA32_PLATFORM_ID MSR that can give a more fine grained identification of the CPU than just the model number/stepping. IA32_PLATFORM_ID is architectural. The MSR can be also accessed through /dev/cpu/*/msr, but that requires root and is awkward. The patch moves the reading of PLATFORM_INFO from the (late) microcode driver code into the main intel CPU initialization path and then also prints it in /proc/cpuinfo v2: Handle 0 platform_id. Fix commit message. v3: Move some code to cpu/intel.c v4: Update description too. v5: Move msr probe code out of line to workaround potential gcc 6 bug. This fixes booting on all 0day systems. Cc: h...@hmh.eng.br Signed-off-by: Andi Kleen --- arch/x86/include/asm/processor.h | 2 ++ arch/x86/kernel/cpu/intel.c | 15 +++ arch/x86/kernel/cpu/microcode/intel.c | 8 ++-- arch/x86/kernel/cpu/proc.c| 2 ++ 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 63def9537a2d..c1313b3f3e59 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -135,6 +135,8 @@ struct cpuinfo_x86 { /* Index into per_cpu list: */ u16 cpu_index; u32 microcode; + u32 platform_id; + u8 has_platform_id; }; #define X86_VENDOR_INTEL 0 diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index fcd484d2bb03..7da7f008cee0 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -61,6 +61,19 @@ void check_mpx_erratum(struct cpuinfo_x86 *c) } } +/* noinline to work around problem with gcc 6.2 */ +static noinline void probe_platformid(struct cpuinfo_x86 *c) +{ + if ((c->x86_model >= 5) || (c->x86 > 6)) { + unsigned val[2]; + + /* get processor flags from MSR 0x17 */ + rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); + c->platform_id = (val[1] >> 18) & 7; + c->has_platform_id = true; + } +} + static void early_init_intel(struct cpuinfo_x86 *c) { u64 misc_enable; @@ -211,6 +224,8 @@ static void early_init_intel(struct cpuinfo_x86 *c) } check_mpx_erratum(c); + + probe_platformid(c); } #ifdef CONFIG_X86_32 diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index cdc0deab00c9..fab07e49192e 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -855,17 +855,13 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig) { static struct cpu_signature prev; struct cpuinfo_x86 *c = _data(cpu_num); - unsigned int val[2]; memset(csig, 0, sizeof(*csig)); csig->sig = cpuid_eax(0x0001); - if ((c->x86_model >= 5) || (c->x86 > 6)) { - /* get processor flags from MSR 0x17 */ - rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); - csig->pf = 1 << ((val[1] >> 18) & 7); - } + if (c->has_platform_id) + csig->pf = 1 << c->platform_id; csig->rev = c->microcode; diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c index 18ca99f2798b..5345d50ed709 100644 --- a/arch/x86/kernel/cpu/proc.c +++ b/arch/x86/kernel/cpu/proc.c @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) seq_puts(m, "stepping\t: unknown\n"); if (c->microcode) seq_printf(m, "microcode\t: 0x%x\n", c->microcode); + if (c->has_platform_id) + seq_printf(m, "platform_id\t: %d\n", c->platform_id); if (cpu_has(c, X86_FEATURE_TSC)) { unsigned int freq = cpufreq_quick_get(cpu); -- 2.5.5
[PATCH] x86: Report Intel platform_id in /proc/cpuinfo
From: Andi Kleen We have a need to distinguish systems based on their platform ID. For example this is useful to distinguish systems with L4 cache versus ones without. There is a 3 bit identifier (also called processor flags) in the IA32_PLATFORM_ID MSR that can give a more fine grained identification of the CPU than just the model number/stepping. IA32_PLATFORM_ID is architectural. The MSR can be also accessed through /dev/cpu/*/msr, but that requires root and is awkward. The patch moves the reading of PLATFORM_INFO from the (late) microcode driver code into the main intel CPU initialization path and then also prints it in /proc/cpuinfo v2: Handle 0 platform_id. Fix commit message. v3: Move some code to cpu/intel.c v4: Update description too. v5: Move msr probe code out of line to workaround potential gcc 6 bug. This fixes booting on all 0day systems. Cc: h...@hmh.eng.br Signed-off-by: Andi Kleen --- arch/x86/include/asm/processor.h | 2 ++ arch/x86/kernel/cpu/intel.c | 15 +++ arch/x86/kernel/cpu/microcode/intel.c | 8 ++-- arch/x86/kernel/cpu/proc.c| 2 ++ 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 63def9537a2d..c1313b3f3e59 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -135,6 +135,8 @@ struct cpuinfo_x86 { /* Index into per_cpu list: */ u16 cpu_index; u32 microcode; + u32 platform_id; + u8 has_platform_id; }; #define X86_VENDOR_INTEL 0 diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index fcd484d2bb03..7da7f008cee0 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -61,6 +61,19 @@ void check_mpx_erratum(struct cpuinfo_x86 *c) } } +/* noinline to work around problem with gcc 6.2 */ +static noinline void probe_platformid(struct cpuinfo_x86 *c) +{ + if ((c->x86_model >= 5) || (c->x86 > 6)) { + unsigned val[2]; + + /* get processor flags from MSR 0x17 */ + rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); + c->platform_id = (val[1] >> 18) & 7; + c->has_platform_id = true; + } +} + static void early_init_intel(struct cpuinfo_x86 *c) { u64 misc_enable; @@ -211,6 +224,8 @@ static void early_init_intel(struct cpuinfo_x86 *c) } check_mpx_erratum(c); + + probe_platformid(c); } #ifdef CONFIG_X86_32 diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index cdc0deab00c9..fab07e49192e 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -855,17 +855,13 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig) { static struct cpu_signature prev; struct cpuinfo_x86 *c = _data(cpu_num); - unsigned int val[2]; memset(csig, 0, sizeof(*csig)); csig->sig = cpuid_eax(0x0001); - if ((c->x86_model >= 5) || (c->x86 > 6)) { - /* get processor flags from MSR 0x17 */ - rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); - csig->pf = 1 << ((val[1] >> 18) & 7); - } + if (c->has_platform_id) + csig->pf = 1 << c->platform_id; csig->rev = c->microcode; diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c index 18ca99f2798b..5345d50ed709 100644 --- a/arch/x86/kernel/cpu/proc.c +++ b/arch/x86/kernel/cpu/proc.c @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) seq_puts(m, "stepping\t: unknown\n"); if (c->microcode) seq_printf(m, "microcode\t: 0x%x\n", c->microcode); + if (c->has_platform_id) + seq_printf(m, "platform_id\t: %d\n", c->platform_id); if (cpu_has(c, X86_FEATURE_TSC)) { unsigned int freq = cpufreq_quick_get(cpu); -- 2.5.5
[PATCH] x86: Report Intel platform_id in /proc/cpuinfo
From: Andi KleenWe have a need to distinguish systems based on their platform ID. For example this is useful to distinguish systems with L4 cache versus ones without. There is a 3 bit identifier (also called processor flags) in the IA32_PLATFORM_ID MSR that can give a more fine grained identification of the CPU than just the model number/stepping. IA32_PLATFORM_ID is architectural. The MSR can be also accessed through /dev/cpu/*/msr, but that requires root and is awkward. The patch moves the reading of PLATFORM_INFO from the (late) microcode driver code into the main intel CPU initialization path and then also prints it in /proc/cpuinfo v2: Handle 0 platform_id. Fix commit message. v3: Move some code to cpu/intel.c v4: Update description too. Cc: h...@hmh.eng.br Signed-off-by: Andi Kleen --- arch/x86/include/asm/processor.h | 2 ++ arch/x86/kernel/cpu/intel.c | 9 + arch/x86/kernel/cpu/microcode/intel.c | 8 ++-- arch/x86/kernel/cpu/proc.c| 2 ++ 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 63def9537a2d..c1313b3f3e59 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -135,6 +135,8 @@ struct cpuinfo_x86 { /* Index into per_cpu list: */ u16 cpu_index; u32 microcode; + u32 platform_id; + u8 has_platform_id; }; #define X86_VENDOR_INTEL 0 diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index fcd484d2bb03..b9f139716636 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -210,6 +210,15 @@ static void early_init_intel(struct cpuinfo_x86 *c) c->x86_coreid_bits = get_count_order((ebx >> 16) & 0xff); } + if ((c->x86_model >= 5) || (c->x86 > 6)) { + unsigned val[2]; + + /* get processor flags from MSR 0x17 */ + rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); + c->platform_id = (val[1] >> 18) & 7; + c->has_platform_id = true; + } + check_mpx_erratum(c); } diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index cdc0deab00c9..fab07e49192e 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -855,17 +855,13 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig) { static struct cpu_signature prev; struct cpuinfo_x86 *c = _data(cpu_num); - unsigned int val[2]; memset(csig, 0, sizeof(*csig)); csig->sig = cpuid_eax(0x0001); - if ((c->x86_model >= 5) || (c->x86 > 6)) { - /* get processor flags from MSR 0x17 */ - rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); - csig->pf = 1 << ((val[1] >> 18) & 7); - } + if (c->has_platform_id) + csig->pf = 1 << c->platform_id; csig->rev = c->microcode; diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c index 18ca99f2798b..5345d50ed709 100644 --- a/arch/x86/kernel/cpu/proc.c +++ b/arch/x86/kernel/cpu/proc.c @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) seq_puts(m, "stepping\t: unknown\n"); if (c->microcode) seq_printf(m, "microcode\t: 0x%x\n", c->microcode); + if (c->has_platform_id) + seq_printf(m, "platform_id\t: %d\n", c->platform_id); if (cpu_has(c, X86_FEATURE_TSC)) { unsigned int freq = cpufreq_quick_get(cpu); -- 2.5.5
[PATCH] x86: Report Intel platform_id in /proc/cpuinfo
From: Andi Kleen We have a need to distinguish systems based on their platform ID. For example this is useful to distinguish systems with L4 cache versus ones without. There is a 3 bit identifier (also called processor flags) in the IA32_PLATFORM_ID MSR that can give a more fine grained identification of the CPU than just the model number/stepping. IA32_PLATFORM_ID is architectural. The MSR can be also accessed through /dev/cpu/*/msr, but that requires root and is awkward. The patch moves the reading of PLATFORM_INFO from the (late) microcode driver code into the main intel CPU initialization path and then also prints it in /proc/cpuinfo v2: Handle 0 platform_id. Fix commit message. v3: Move some code to cpu/intel.c v4: Update description too. Cc: h...@hmh.eng.br Signed-off-by: Andi Kleen --- arch/x86/include/asm/processor.h | 2 ++ arch/x86/kernel/cpu/intel.c | 9 + arch/x86/kernel/cpu/microcode/intel.c | 8 ++-- arch/x86/kernel/cpu/proc.c| 2 ++ 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 63def9537a2d..c1313b3f3e59 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -135,6 +135,8 @@ struct cpuinfo_x86 { /* Index into per_cpu list: */ u16 cpu_index; u32 microcode; + u32 platform_id; + u8 has_platform_id; }; #define X86_VENDOR_INTEL 0 diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index fcd484d2bb03..b9f139716636 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -210,6 +210,15 @@ static void early_init_intel(struct cpuinfo_x86 *c) c->x86_coreid_bits = get_count_order((ebx >> 16) & 0xff); } + if ((c->x86_model >= 5) || (c->x86 > 6)) { + unsigned val[2]; + + /* get processor flags from MSR 0x17 */ + rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); + c->platform_id = (val[1] >> 18) & 7; + c->has_platform_id = true; + } + check_mpx_erratum(c); } diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index cdc0deab00c9..fab07e49192e 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -855,17 +855,13 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig) { static struct cpu_signature prev; struct cpuinfo_x86 *c = _data(cpu_num); - unsigned int val[2]; memset(csig, 0, sizeof(*csig)); csig->sig = cpuid_eax(0x0001); - if ((c->x86_model >= 5) || (c->x86 > 6)) { - /* get processor flags from MSR 0x17 */ - rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); - csig->pf = 1 << ((val[1] >> 18) & 7); - } + if (c->has_platform_id) + csig->pf = 1 << c->platform_id; csig->rev = c->microcode; diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c index 18ca99f2798b..5345d50ed709 100644 --- a/arch/x86/kernel/cpu/proc.c +++ b/arch/x86/kernel/cpu/proc.c @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) seq_puts(m, "stepping\t: unknown\n"); if (c->microcode) seq_printf(m, "microcode\t: 0x%x\n", c->microcode); + if (c->has_platform_id) + seq_printf(m, "platform_id\t: %d\n", c->platform_id); if (cpu_has(c, X86_FEATURE_TSC)) { unsigned int freq = cpufreq_quick_get(cpu); -- 2.5.5
Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo
On Thu, 22 Sep 2016, Andi Kleen wrote: > The processor flags are already used in the microcode driver. > The MSR can be also accessed through /dev/cpu/*/msr, but that > requires root and is awkward. > > This patch just exports the value retrieved by the microcode > driver in /proc/cpuinfo. If the microcode driver is disabled > it won't be shown, but that seems reasonable. The patch is fine, but these two paragraphs are describing something else. Thanks, tglx
Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo
On Thu, 22 Sep 2016, Andi Kleen wrote: > The processor flags are already used in the microcode driver. > The MSR can be also accessed through /dev/cpu/*/msr, but that > requires root and is awkward. > > This patch just exports the value retrieved by the microcode > driver in /proc/cpuinfo. If the microcode driver is disabled > it won't be shown, but that seems reasonable. The patch is fine, but these two paragraphs are describing something else. Thanks, tglx
[PATCH] x86: Report Intel platform_id in /proc/cpuinfo
From: Andi KleenWe have a need to distinguish systems based on their platform ID. For example this is useful to distinguish systems with L4 cache versus ones without. There is a 3 bit identifier (also called processor flags) in the IA32_PLATFORM_ID MSR that can give a more fine grained identification of the CPU than just the model number/stepping. IA32_PLATFORM_ID is architectural. The processor flags are already used in the microcode driver. The MSR can be also accessed through /dev/cpu/*/msr, but that requires root and is awkward. This patch just exports the value retrieved by the microcode driver in /proc/cpuinfo. If the microcode driver is disabled it won't be shown, but that seems reasonable. v2: Handle 0 platform_id. Fix commit message. v3: Move some code to cpu/intel.c Cc: h...@hmh.eng.br Signed-off-by: Andi Kleen --- arch/x86/include/asm/processor.h | 2 ++ arch/x86/kernel/cpu/intel.c | 9 + arch/x86/kernel/cpu/microcode/intel.c | 8 ++-- arch/x86/kernel/cpu/proc.c| 2 ++ 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 63def9537a2d..c1313b3f3e59 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -135,6 +135,8 @@ struct cpuinfo_x86 { /* Index into per_cpu list: */ u16 cpu_index; u32 microcode; + u32 platform_id; + u8 has_platform_id; }; #define X86_VENDOR_INTEL 0 diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index fcd484d2bb03..b9f139716636 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -210,6 +210,15 @@ static void early_init_intel(struct cpuinfo_x86 *c) c->x86_coreid_bits = get_count_order((ebx >> 16) & 0xff); } + if ((c->x86_model >= 5) || (c->x86 > 6)) { + unsigned val[2]; + + /* get processor flags from MSR 0x17 */ + rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); + c->platform_id = (val[1] >> 18) & 7; + c->has_platform_id = true; + } + check_mpx_erratum(c); } diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index cdc0deab00c9..fab07e49192e 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -855,17 +855,13 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig) { static struct cpu_signature prev; struct cpuinfo_x86 *c = _data(cpu_num); - unsigned int val[2]; memset(csig, 0, sizeof(*csig)); csig->sig = cpuid_eax(0x0001); - if ((c->x86_model >= 5) || (c->x86 > 6)) { - /* get processor flags from MSR 0x17 */ - rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); - csig->pf = 1 << ((val[1] >> 18) & 7); - } + if (c->has_platform_id) + csig->pf = 1 << c->platform_id; csig->rev = c->microcode; diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c index 18ca99f2798b..5345d50ed709 100644 --- a/arch/x86/kernel/cpu/proc.c +++ b/arch/x86/kernel/cpu/proc.c @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) seq_puts(m, "stepping\t: unknown\n"); if (c->microcode) seq_printf(m, "microcode\t: 0x%x\n", c->microcode); + if (c->has_platform_id) + seq_printf(m, "platform_id\t: %d\n", c->platform_id); if (cpu_has(c, X86_FEATURE_TSC)) { unsigned int freq = cpufreq_quick_get(cpu); -- 2.5.5
[PATCH] x86: Report Intel platform_id in /proc/cpuinfo
From: Andi Kleen We have a need to distinguish systems based on their platform ID. For example this is useful to distinguish systems with L4 cache versus ones without. There is a 3 bit identifier (also called processor flags) in the IA32_PLATFORM_ID MSR that can give a more fine grained identification of the CPU than just the model number/stepping. IA32_PLATFORM_ID is architectural. The processor flags are already used in the microcode driver. The MSR can be also accessed through /dev/cpu/*/msr, but that requires root and is awkward. This patch just exports the value retrieved by the microcode driver in /proc/cpuinfo. If the microcode driver is disabled it won't be shown, but that seems reasonable. v2: Handle 0 platform_id. Fix commit message. v3: Move some code to cpu/intel.c Cc: h...@hmh.eng.br Signed-off-by: Andi Kleen --- arch/x86/include/asm/processor.h | 2 ++ arch/x86/kernel/cpu/intel.c | 9 + arch/x86/kernel/cpu/microcode/intel.c | 8 ++-- arch/x86/kernel/cpu/proc.c| 2 ++ 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 63def9537a2d..c1313b3f3e59 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -135,6 +135,8 @@ struct cpuinfo_x86 { /* Index into per_cpu list: */ u16 cpu_index; u32 microcode; + u32 platform_id; + u8 has_platform_id; }; #define X86_VENDOR_INTEL 0 diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index fcd484d2bb03..b9f139716636 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -210,6 +210,15 @@ static void early_init_intel(struct cpuinfo_x86 *c) c->x86_coreid_bits = get_count_order((ebx >> 16) & 0xff); } + if ((c->x86_model >= 5) || (c->x86 > 6)) { + unsigned val[2]; + + /* get processor flags from MSR 0x17 */ + rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); + c->platform_id = (val[1] >> 18) & 7; + c->has_platform_id = true; + } + check_mpx_erratum(c); } diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index cdc0deab00c9..fab07e49192e 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -855,17 +855,13 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig) { static struct cpu_signature prev; struct cpuinfo_x86 *c = _data(cpu_num); - unsigned int val[2]; memset(csig, 0, sizeof(*csig)); csig->sig = cpuid_eax(0x0001); - if ((c->x86_model >= 5) || (c->x86 > 6)) { - /* get processor flags from MSR 0x17 */ - rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); - csig->pf = 1 << ((val[1] >> 18) & 7); - } + if (c->has_platform_id) + csig->pf = 1 << c->platform_id; csig->rev = c->microcode; diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c index 18ca99f2798b..5345d50ed709 100644 --- a/arch/x86/kernel/cpu/proc.c +++ b/arch/x86/kernel/cpu/proc.c @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) seq_puts(m, "stepping\t: unknown\n"); if (c->microcode) seq_printf(m, "microcode\t: 0x%x\n", c->microcode); + if (c->has_platform_id) + seq_printf(m, "platform_id\t: %d\n", c->platform_id); if (cpu_has(c, X86_FEATURE_TSC)) { unsigned int freq = cpufreq_quick_get(cpu); -- 2.5.5
Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo
On Thu, 2 Jun 2016, Andi Kleen wrote: Sorry for answering late. This got stuck in my backlog forever. > diff --git a/arch/x86/kernel/cpu/microcode/intel.c > b/arch/x86/kernel/cpu/microcode/intel.c > index ee81c54..bcd3f59 100644 > --- a/arch/x86/kernel/cpu/microcode/intel.c > +++ b/arch/x86/kernel/cpu/microcode/intel.c > @@ -812,6 +812,8 @@ static int collect_cpu_info(int cpu_num, struct > cpu_signature *csig) > /* get processor flags from MSR 0x17 */ > rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); > csig->pf = 1 << ((val[1] >> 18) & 7); > + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; > + cpu_data(cpu_num).has_platform_id = true; Please move that evaluation to arch/x86/kernel/cpu/intel.c and let the micro code driver use the information from cpu_data. Thanks, tglx
Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo
On Thu, 2 Jun 2016, Andi Kleen wrote: Sorry for answering late. This got stuck in my backlog forever. > diff --git a/arch/x86/kernel/cpu/microcode/intel.c > b/arch/x86/kernel/cpu/microcode/intel.c > index ee81c54..bcd3f59 100644 > --- a/arch/x86/kernel/cpu/microcode/intel.c > +++ b/arch/x86/kernel/cpu/microcode/intel.c > @@ -812,6 +812,8 @@ static int collect_cpu_info(int cpu_num, struct > cpu_signature *csig) > /* get processor flags from MSR 0x17 */ > rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); > csig->pf = 1 << ((val[1] >> 18) & 7); > + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; > + cpu_data(cpu_num).has_platform_id = true; Please move that evaluation to arch/x86/kernel/cpu/intel.c and let the micro code driver use the information from cpu_data. Thanks, tglx
Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo
On Tue, Jun 21, 2016, at 13:05, Andi Kleen wrote: > Andi Kleenwrites: > > Ping! Any comments on this patch? Well, FWIW, it looks good enough to me. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo
On Tue, Jun 21, 2016, at 13:05, Andi Kleen wrote: > Andi Kleen writes: > > Ping! Any comments on this patch? Well, FWIW, it looks good enough to me. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo
Andi Kleenwrites: Ping! Any comments on this patch? > From: Andi Kleen > > We have a need to distinguish systems based on their platform ID. > For example this is useful to distinguish systems with L4 cache > versus ones without. > > There is a 3 bit identifier (also called processor flags) in > the IA32_PLATFORM_ID MSR that can give a more fine grained > identification of the CPU than just the model number/stepping. > > IA32_PLATFORM_ID is architectural. > > The processor flags are already used in the microcode driver. > The MSR can be also accessed through /dev/cpu/*/msr, but that > requires root and is awkward. > > This patch just exports the value retrieved by the microcode > driver in /proc/cpuinfo. If the microcode driver is disabled > it won't be shown, but that seems reasonable. > > v2: Handle 0 platform_id. Fix commit message. > Cc: h...@hmh.eng.br > Signed-off-by: Andi Kleen > --- > arch/x86/include/asm/processor.h | 2 ++ > arch/x86/kernel/cpu/microcode/intel.c | 2 ++ > arch/x86/kernel/cpu/proc.c| 2 ++ > 3 files changed, 6 insertions(+) > > diff --git a/arch/x86/include/asm/processor.h > b/arch/x86/include/asm/processor.h > index 20c11d1..270209c 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -136,6 +136,8 @@ struct cpuinfo_x86 { > /* Index into per_cpu list: */ > u16 cpu_index; > u32 microcode; > + u32 platform_id; > + u8 has_platform_id; > }; > > #define X86_VENDOR_INTEL 0 > diff --git a/arch/x86/kernel/cpu/microcode/intel.c > b/arch/x86/kernel/cpu/microcode/intel.c > index ee81c54..bcd3f59 100644 > --- a/arch/x86/kernel/cpu/microcode/intel.c > +++ b/arch/x86/kernel/cpu/microcode/intel.c > @@ -812,6 +812,8 @@ static int collect_cpu_info(int cpu_num, struct > cpu_signature *csig) > /* get processor flags from MSR 0x17 */ > rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); > csig->pf = 1 << ((val[1] >> 18) & 7); > + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; > + cpu_data(cpu_num).has_platform_id = true; > } > > csig->rev = c->microcode; > diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c > index 18ca99f..5345d50 100644 > --- a/arch/x86/kernel/cpu/proc.c > +++ b/arch/x86/kernel/cpu/proc.c > @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) > seq_puts(m, "stepping\t: unknown\n"); > if (c->microcode) > seq_printf(m, "microcode\t: 0x%x\n", c->microcode); > + if (c->has_platform_id) > + seq_printf(m, "platform_id\t: %d\n", c->platform_id); > > if (cpu_has(c, X86_FEATURE_TSC)) { > unsigned int freq = cpufreq_quick_get(cpu); -- a...@linux.intel.com -- Speaking for myself only
Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo
Andi Kleen writes: Ping! Any comments on this patch? > From: Andi Kleen > > We have a need to distinguish systems based on their platform ID. > For example this is useful to distinguish systems with L4 cache > versus ones without. > > There is a 3 bit identifier (also called processor flags) in > the IA32_PLATFORM_ID MSR that can give a more fine grained > identification of the CPU than just the model number/stepping. > > IA32_PLATFORM_ID is architectural. > > The processor flags are already used in the microcode driver. > The MSR can be also accessed through /dev/cpu/*/msr, but that > requires root and is awkward. > > This patch just exports the value retrieved by the microcode > driver in /proc/cpuinfo. If the microcode driver is disabled > it won't be shown, but that seems reasonable. > > v2: Handle 0 platform_id. Fix commit message. > Cc: h...@hmh.eng.br > Signed-off-by: Andi Kleen > --- > arch/x86/include/asm/processor.h | 2 ++ > arch/x86/kernel/cpu/microcode/intel.c | 2 ++ > arch/x86/kernel/cpu/proc.c| 2 ++ > 3 files changed, 6 insertions(+) > > diff --git a/arch/x86/include/asm/processor.h > b/arch/x86/include/asm/processor.h > index 20c11d1..270209c 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -136,6 +136,8 @@ struct cpuinfo_x86 { > /* Index into per_cpu list: */ > u16 cpu_index; > u32 microcode; > + u32 platform_id; > + u8 has_platform_id; > }; > > #define X86_VENDOR_INTEL 0 > diff --git a/arch/x86/kernel/cpu/microcode/intel.c > b/arch/x86/kernel/cpu/microcode/intel.c > index ee81c54..bcd3f59 100644 > --- a/arch/x86/kernel/cpu/microcode/intel.c > +++ b/arch/x86/kernel/cpu/microcode/intel.c > @@ -812,6 +812,8 @@ static int collect_cpu_info(int cpu_num, struct > cpu_signature *csig) > /* get processor flags from MSR 0x17 */ > rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); > csig->pf = 1 << ((val[1] >> 18) & 7); > + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; > + cpu_data(cpu_num).has_platform_id = true; > } > > csig->rev = c->microcode; > diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c > index 18ca99f..5345d50 100644 > --- a/arch/x86/kernel/cpu/proc.c > +++ b/arch/x86/kernel/cpu/proc.c > @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) > seq_puts(m, "stepping\t: unknown\n"); > if (c->microcode) > seq_printf(m, "microcode\t: 0x%x\n", c->microcode); > + if (c->has_platform_id) > + seq_printf(m, "platform_id\t: %d\n", c->platform_id); > > if (cpu_has(c, X86_FEATURE_TSC)) { > unsigned int freq = cpufreq_quick_get(cpu); -- a...@linux.intel.com -- Speaking for myself only
[PATCH] x86: Report Intel platform_id in /proc/cpuinfo
From: Andi KleenWe have a need to distinguish systems based on their platform ID. For example this is useful to distinguish systems with L4 cache versus ones without. There is a 3 bit identifier (also called processor flags) in the IA32_PLATFORM_ID MSR that can give a more fine grained identification of the CPU than just the model number/stepping. IA32_PLATFORM_ID is architectural. The processor flags are already used in the microcode driver. The MSR can be also accessed through /dev/cpu/*/msr, but that requires root and is awkward. This patch just exports the value retrieved by the microcode driver in /proc/cpuinfo. If the microcode driver is disabled it won't be shown, but that seems reasonable. v2: Handle 0 platform_id. Fix commit message. Cc: h...@hmh.eng.br Signed-off-by: Andi Kleen --- arch/x86/include/asm/processor.h | 2 ++ arch/x86/kernel/cpu/microcode/intel.c | 2 ++ arch/x86/kernel/cpu/proc.c| 2 ++ 3 files changed, 6 insertions(+) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 20c11d1..270209c 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -136,6 +136,8 @@ struct cpuinfo_x86 { /* Index into per_cpu list: */ u16 cpu_index; u32 microcode; + u32 platform_id; + u8 has_platform_id; }; #define X86_VENDOR_INTEL 0 diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index ee81c54..bcd3f59 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -812,6 +812,8 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig) /* get processor flags from MSR 0x17 */ rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); csig->pf = 1 << ((val[1] >> 18) & 7); + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; + cpu_data(cpu_num).has_platform_id = true; } csig->rev = c->microcode; diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c index 18ca99f..5345d50 100644 --- a/arch/x86/kernel/cpu/proc.c +++ b/arch/x86/kernel/cpu/proc.c @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) seq_puts(m, "stepping\t: unknown\n"); if (c->microcode) seq_printf(m, "microcode\t: 0x%x\n", c->microcode); + if (c->has_platform_id) + seq_printf(m, "platform_id\t: %d\n", c->platform_id); if (cpu_has(c, X86_FEATURE_TSC)) { unsigned int freq = cpufreq_quick_get(cpu); -- 2.8.3
[PATCH] x86: Report Intel platform_id in /proc/cpuinfo
From: Andi Kleen We have a need to distinguish systems based on their platform ID. For example this is useful to distinguish systems with L4 cache versus ones without. There is a 3 bit identifier (also called processor flags) in the IA32_PLATFORM_ID MSR that can give a more fine grained identification of the CPU than just the model number/stepping. IA32_PLATFORM_ID is architectural. The processor flags are already used in the microcode driver. The MSR can be also accessed through /dev/cpu/*/msr, but that requires root and is awkward. This patch just exports the value retrieved by the microcode driver in /proc/cpuinfo. If the microcode driver is disabled it won't be shown, but that seems reasonable. v2: Handle 0 platform_id. Fix commit message. Cc: h...@hmh.eng.br Signed-off-by: Andi Kleen --- arch/x86/include/asm/processor.h | 2 ++ arch/x86/kernel/cpu/microcode/intel.c | 2 ++ arch/x86/kernel/cpu/proc.c| 2 ++ 3 files changed, 6 insertions(+) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 20c11d1..270209c 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -136,6 +136,8 @@ struct cpuinfo_x86 { /* Index into per_cpu list: */ u16 cpu_index; u32 microcode; + u32 platform_id; + u8 has_platform_id; }; #define X86_VENDOR_INTEL 0 diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index ee81c54..bcd3f59 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -812,6 +812,8 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig) /* get processor flags from MSR 0x17 */ rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); csig->pf = 1 << ((val[1] >> 18) & 7); + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; + cpu_data(cpu_num).has_platform_id = true; } csig->rev = c->microcode; diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c index 18ca99f..5345d50 100644 --- a/arch/x86/kernel/cpu/proc.c +++ b/arch/x86/kernel/cpu/proc.c @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) seq_puts(m, "stepping\t: unknown\n"); if (c->microcode) seq_printf(m, "microcode\t: 0x%x\n", c->microcode); + if (c->has_platform_id) + seq_printf(m, "platform_id\t: %d\n", c->platform_id); if (cpu_has(c, X86_FEATURE_TSC)) { unsigned int freq = cpufreq_quick_get(cpu); -- 2.8.3
Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo
On Thu, Jun 02, 2016 at 03:27:07PM -0300, Henrique de Moraes Holschuh wrote: > On Tue, 31 May 2016, Andi Kleen wrote: > > We have a need to distinguish systems based on their platform ID. > > For example this is useful to distinguish systems with L4 cache > > versus ones without. > > > > There is a 5 bit identifier (also called processor flags) in > > There is a 3 bit identifier... Thanks. > > diff --git a/arch/x86/kernel/cpu/microcode/intel.c > > b/arch/x86/kernel/cpu/microcode/intel.c > > index ee81c54..6244a88 100644 > > --- a/arch/x86/kernel/cpu/microcode/intel.c > > +++ b/arch/x86/kernel/cpu/microcode/intel.c > > @@ -812,6 +812,7 @@ static int collect_cpu_info(int cpu_num, struct > > cpu_signature *csig) > > /* get processor flags from MSR 0x17 */ > > rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); > > csig->pf = 1 << ((val[1] >> 18) & 7); > > + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; > > See below. It might be better to have "cpu_data(cpu_num).platform_id = > csig->pf" instead. Ok. > > > diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c > > index 18ca99f..1c4e4f5 100644 > > --- a/arch/x86/kernel/cpu/proc.c > > +++ b/arch/x86/kernel/cpu/proc.c > > @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) > > seq_puts(m, "stepping\t: unknown\n"); > > if (c->microcode) > > seq_printf(m, "microcode\t: 0x%x\n", c->microcode); > > + if (c->platform_id) > > + seq_printf(m, "platform_id\t: %d\n", c->platform_id); > > platform_id can meaningfully be zero on Intel, and in fact it often is (just > look for output from the microcode driver that logs pf=0x1). In those > processors, (MSR(17) >> 18 & 7) is zero, and csig->pf is 1. > > May I humbly suggest using the mask "(1 << value)" notation for platform_id > as used by the microcode driver? We already do it for csig->pf, and it has > the advantage that mask notation will never be zero (so, the field is always > non-zero where relevant). That would be confusing because the value is used in some documents. I would prefer to match them. > > Alterntively, the patch could be changed to always print platform_id on > Intel processors, instead of just printing it out when it is non-zero. What I can do is to add an extra flag and print it when the flag is set, even if it is zero. Thanks for the review. -Andi
Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo
On Thu, Jun 02, 2016 at 03:27:07PM -0300, Henrique de Moraes Holschuh wrote: > On Tue, 31 May 2016, Andi Kleen wrote: > > We have a need to distinguish systems based on their platform ID. > > For example this is useful to distinguish systems with L4 cache > > versus ones without. > > > > There is a 5 bit identifier (also called processor flags) in > > There is a 3 bit identifier... Thanks. > > diff --git a/arch/x86/kernel/cpu/microcode/intel.c > > b/arch/x86/kernel/cpu/microcode/intel.c > > index ee81c54..6244a88 100644 > > --- a/arch/x86/kernel/cpu/microcode/intel.c > > +++ b/arch/x86/kernel/cpu/microcode/intel.c > > @@ -812,6 +812,7 @@ static int collect_cpu_info(int cpu_num, struct > > cpu_signature *csig) > > /* get processor flags from MSR 0x17 */ > > rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); > > csig->pf = 1 << ((val[1] >> 18) & 7); > > + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; > > See below. It might be better to have "cpu_data(cpu_num).platform_id = > csig->pf" instead. Ok. > > > diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c > > index 18ca99f..1c4e4f5 100644 > > --- a/arch/x86/kernel/cpu/proc.c > > +++ b/arch/x86/kernel/cpu/proc.c > > @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) > > seq_puts(m, "stepping\t: unknown\n"); > > if (c->microcode) > > seq_printf(m, "microcode\t: 0x%x\n", c->microcode); > > + if (c->platform_id) > > + seq_printf(m, "platform_id\t: %d\n", c->platform_id); > > platform_id can meaningfully be zero on Intel, and in fact it often is (just > look for output from the microcode driver that logs pf=0x1). In those > processors, (MSR(17) >> 18 & 7) is zero, and csig->pf is 1. > > May I humbly suggest using the mask "(1 << value)" notation for platform_id > as used by the microcode driver? We already do it for csig->pf, and it has > the advantage that mask notation will never be zero (so, the field is always > non-zero where relevant). That would be confusing because the value is used in some documents. I would prefer to match them. > > Alterntively, the patch could be changed to always print platform_id on > Intel processors, instead of just printing it out when it is non-zero. What I can do is to add an extra flag and print it when the flag is set, even if it is zero. Thanks for the review. -Andi
Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo
On Tue, 31 May 2016, Andi Kleen wrote: > We have a need to distinguish systems based on their platform ID. > For example this is useful to distinguish systems with L4 cache > versus ones without. > > There is a 5 bit identifier (also called processor flags) in There is a 3 bit identifier... > the IA32_PLATFORM_ID MSR that can give a more fine grained > identification of the CPU than just the model number/stepping. > > IA32_PLATFORM_ID is architectural. > > The processor flags are already used in the microcode driver. > The MSR can be also accessed through /dev/cpu/*/msr, but that > requires root and is awkward. > > This patch just exports the value retrieved by the microcode > driver in /proc/cpuinfo. If the microcode driver is disabled > it won't be shown, but that seems reasonable. ... > diff --git a/arch/x86/kernel/cpu/microcode/intel.c > b/arch/x86/kernel/cpu/microcode/intel.c > index ee81c54..6244a88 100644 > --- a/arch/x86/kernel/cpu/microcode/intel.c > +++ b/arch/x86/kernel/cpu/microcode/intel.c > @@ -812,6 +812,7 @@ static int collect_cpu_info(int cpu_num, struct > cpu_signature *csig) > /* get processor flags from MSR 0x17 */ > rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); > csig->pf = 1 << ((val[1] >> 18) & 7); > + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; See below. It might be better to have "cpu_data(cpu_num).platform_id = csig->pf" instead. > diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c > index 18ca99f..1c4e4f5 100644 > --- a/arch/x86/kernel/cpu/proc.c > +++ b/arch/x86/kernel/cpu/proc.c > @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) > seq_puts(m, "stepping\t: unknown\n"); > if (c->microcode) > seq_printf(m, "microcode\t: 0x%x\n", c->microcode); > + if (c->platform_id) > + seq_printf(m, "platform_id\t: %d\n", c->platform_id); platform_id can meaningfully be zero on Intel, and in fact it often is (just look for output from the microcode driver that logs pf=0x1). In those processors, (MSR(17) >> 18 & 7) is zero, and csig->pf is 1. May I humbly suggest using the mask "(1 << value)" notation for platform_id as used by the microcode driver? We already do it for csig->pf, and it has the advantage that mask notation will never be zero (so, the field is always non-zero where relevant). Alterntively, the patch could be changed to always print platform_id on Intel processors, instead of just printing it out when it is non-zero. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo
On Tue, 31 May 2016, Andi Kleen wrote: > We have a need to distinguish systems based on their platform ID. > For example this is useful to distinguish systems with L4 cache > versus ones without. > > There is a 5 bit identifier (also called processor flags) in There is a 3 bit identifier... > the IA32_PLATFORM_ID MSR that can give a more fine grained > identification of the CPU than just the model number/stepping. > > IA32_PLATFORM_ID is architectural. > > The processor flags are already used in the microcode driver. > The MSR can be also accessed through /dev/cpu/*/msr, but that > requires root and is awkward. > > This patch just exports the value retrieved by the microcode > driver in /proc/cpuinfo. If the microcode driver is disabled > it won't be shown, but that seems reasonable. ... > diff --git a/arch/x86/kernel/cpu/microcode/intel.c > b/arch/x86/kernel/cpu/microcode/intel.c > index ee81c54..6244a88 100644 > --- a/arch/x86/kernel/cpu/microcode/intel.c > +++ b/arch/x86/kernel/cpu/microcode/intel.c > @@ -812,6 +812,7 @@ static int collect_cpu_info(int cpu_num, struct > cpu_signature *csig) > /* get processor flags from MSR 0x17 */ > rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); > csig->pf = 1 << ((val[1] >> 18) & 7); > + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; See below. It might be better to have "cpu_data(cpu_num).platform_id = csig->pf" instead. > diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c > index 18ca99f..1c4e4f5 100644 > --- a/arch/x86/kernel/cpu/proc.c > +++ b/arch/x86/kernel/cpu/proc.c > @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) > seq_puts(m, "stepping\t: unknown\n"); > if (c->microcode) > seq_printf(m, "microcode\t: 0x%x\n", c->microcode); > + if (c->platform_id) > + seq_printf(m, "platform_id\t: %d\n", c->platform_id); platform_id can meaningfully be zero on Intel, and in fact it often is (just look for output from the microcode driver that logs pf=0x1). In those processors, (MSR(17) >> 18 & 7) is zero, and csig->pf is 1. May I humbly suggest using the mask "(1 << value)" notation for platform_id as used by the microcode driver? We already do it for csig->pf, and it has the advantage that mask notation will never be zero (so, the field is always non-zero where relevant). Alterntively, the patch could be changed to always print platform_id on Intel processors, instead of just printing it out when it is non-zero. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
[PATCH] x86: Report Intel platform_id in /proc/cpuinfo
From: Andi KleenWe have a need to distinguish systems based on their platform ID. For example this is useful to distinguish systems with L4 cache versus ones without. There is a 5 bit identifier (also called processor flags) in the IA32_PLATFORM_ID MSR that can give a more fine grained identification of the CPU than just the model number/stepping. IA32_PLATFORM_ID is architectural. The processor flags are already used in the microcode driver. The MSR can be also accessed through /dev/cpu/*/msr, but that requires root and is awkward. This patch just exports the value retrieved by the microcode driver in /proc/cpuinfo. If the microcode driver is disabled it won't be shown, but that seems reasonable. Signed-off-by: Andi Kleen --- arch/x86/include/asm/processor.h | 1 + arch/x86/kernel/cpu/microcode/intel.c | 1 + arch/x86/kernel/cpu/proc.c| 2 ++ 3 files changed, 4 insertions(+) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 20c11d1..bc4b0ef 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -136,6 +136,7 @@ struct cpuinfo_x86 { /* Index into per_cpu list: */ u16 cpu_index; u32 microcode; + u32 platform_id; }; #define X86_VENDOR_INTEL 0 diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index ee81c54..6244a88 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -812,6 +812,7 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig) /* get processor flags from MSR 0x17 */ rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); csig->pf = 1 << ((val[1] >> 18) & 7); + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; } csig->rev = c->microcode; diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c index 18ca99f..1c4e4f5 100644 --- a/arch/x86/kernel/cpu/proc.c +++ b/arch/x86/kernel/cpu/proc.c @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) seq_puts(m, "stepping\t: unknown\n"); if (c->microcode) seq_printf(m, "microcode\t: 0x%x\n", c->microcode); + if (c->platform_id) + seq_printf(m, "platform_id\t: %d\n", c->platform_id); if (cpu_has(c, X86_FEATURE_TSC)) { unsigned int freq = cpufreq_quick_get(cpu); -- 2.8.2
[PATCH] x86: Report Intel platform_id in /proc/cpuinfo
From: Andi Kleen We have a need to distinguish systems based on their platform ID. For example this is useful to distinguish systems with L4 cache versus ones without. There is a 5 bit identifier (also called processor flags) in the IA32_PLATFORM_ID MSR that can give a more fine grained identification of the CPU than just the model number/stepping. IA32_PLATFORM_ID is architectural. The processor flags are already used in the microcode driver. The MSR can be also accessed through /dev/cpu/*/msr, but that requires root and is awkward. This patch just exports the value retrieved by the microcode driver in /proc/cpuinfo. If the microcode driver is disabled it won't be shown, but that seems reasonable. Signed-off-by: Andi Kleen --- arch/x86/include/asm/processor.h | 1 + arch/x86/kernel/cpu/microcode/intel.c | 1 + arch/x86/kernel/cpu/proc.c| 2 ++ 3 files changed, 4 insertions(+) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 20c11d1..bc4b0ef 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -136,6 +136,7 @@ struct cpuinfo_x86 { /* Index into per_cpu list: */ u16 cpu_index; u32 microcode; + u32 platform_id; }; #define X86_VENDOR_INTEL 0 diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index ee81c54..6244a88 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -812,6 +812,7 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig) /* get processor flags from MSR 0x17 */ rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); csig->pf = 1 << ((val[1] >> 18) & 7); + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; } csig->rev = c->microcode; diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c index 18ca99f..1c4e4f5 100644 --- a/arch/x86/kernel/cpu/proc.c +++ b/arch/x86/kernel/cpu/proc.c @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) seq_puts(m, "stepping\t: unknown\n"); if (c->microcode) seq_printf(m, "microcode\t: 0x%x\n", c->microcode); + if (c->platform_id) + seq_printf(m, "platform_id\t: %d\n", c->platform_id); if (cpu_has(c, X86_FEATURE_TSC)) { unsigned int freq = cpufreq_quick_get(cpu); -- 2.8.2
Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo
On Thu, May 5, 2016, at 07:12, Andi Kleen wrote: > We have a need to distinguish systems based on their platform ID. > For example this is useful to distinguish systems with L4 cache > versus ones without. Thank you for doing this, it will be useful. > There is a 5 bit identifier (also called processor flags) in > the IA32_PLATFORM_ID MSR that can give a more fine grained > identification of the CPU than just the model number/stepping. There's a relevant typo, there. Suggestion: "There is a 3-bit identifier (bits 52:50, also called processor flags) in..." > IA32_PLATFORM_ID is architectural. > > The processor flags are already used in the microcode driver. > The MSR can be also accessed through /dev/cpu/*/msr, but that > requires root and is awkward. The existence of /dev/cpu/*/msr is actively dangerous, except maybe inside a VM when the hypervisor does whitelisted-only filtering of MSR access. That thing should either die ASAP, or to grow a processor vendor-family-model-aware whitelist. That said, the microcode-related platform ID bits (in microcode processor flags mask format) are available in sysfs when the microcode driver is loaded, at: /sys/devices/system/cpu/cpu*/microcode/processor_flags However, there is real value in exporting these values in /proc/cpuinfo (it will show up in the typical debugging output, and it will all be in one single place), so I *do* agree that we should add platform_id to /proc/cpuinfo on Intel. Do note we are still missing the real microcode signature, which *cannot* be fully derived from /proc/cpuinfo contents right now, even when you have the platform_id bits from MSR 17h, because it is missing some bits from cpuid(1).EAX. The last time one of these cpuid bits was not zero was, AFAIK, for the "Pentium Overdrive Processors"... but we now have Intel x86 SoCs, and I have seen the two "type" bits from cpuid(1).EAX refered as "SoC type" in at least one recent Intel document (sorry, I can't remember which), which leads me to believe they might see some reuse sooner or later. So, might also want to add a "processor signature" field, which is the full contents of EAX for cpuid(1). This would also shown on AMD processors for completeness. Alternatively, we could add a "microcode id" or "microcode signature" field for Intel instead of the proposed "platform_id" field. The microcode id field would have both the contents of cpuid(1).eax *and* "1 << MSR 17h [52:50]", e.g. "microcode_id: 0x106a5, 0x02". > This patch just exports the value retrieved by the microcode > driver in /proc/cpuinfo. If the microcode driver is disabled > it won't be shown, but that seems reasonable. Agreed. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo
On Thu, May 5, 2016, at 07:12, Andi Kleen wrote: > We have a need to distinguish systems based on their platform ID. > For example this is useful to distinguish systems with L4 cache > versus ones without. Thank you for doing this, it will be useful. > There is a 5 bit identifier (also called processor flags) in > the IA32_PLATFORM_ID MSR that can give a more fine grained > identification of the CPU than just the model number/stepping. There's a relevant typo, there. Suggestion: "There is a 3-bit identifier (bits 52:50, also called processor flags) in..." > IA32_PLATFORM_ID is architectural. > > The processor flags are already used in the microcode driver. > The MSR can be also accessed through /dev/cpu/*/msr, but that > requires root and is awkward. The existence of /dev/cpu/*/msr is actively dangerous, except maybe inside a VM when the hypervisor does whitelisted-only filtering of MSR access. That thing should either die ASAP, or to grow a processor vendor-family-model-aware whitelist. That said, the microcode-related platform ID bits (in microcode processor flags mask format) are available in sysfs when the microcode driver is loaded, at: /sys/devices/system/cpu/cpu*/microcode/processor_flags However, there is real value in exporting these values in /proc/cpuinfo (it will show up in the typical debugging output, and it will all be in one single place), so I *do* agree that we should add platform_id to /proc/cpuinfo on Intel. Do note we are still missing the real microcode signature, which *cannot* be fully derived from /proc/cpuinfo contents right now, even when you have the platform_id bits from MSR 17h, because it is missing some bits from cpuid(1).EAX. The last time one of these cpuid bits was not zero was, AFAIK, for the "Pentium Overdrive Processors"... but we now have Intel x86 SoCs, and I have seen the two "type" bits from cpuid(1).EAX refered as "SoC type" in at least one recent Intel document (sorry, I can't remember which), which leads me to believe they might see some reuse sooner or later. So, might also want to add a "processor signature" field, which is the full contents of EAX for cpuid(1). This would also shown on AMD processors for completeness. Alternatively, we could add a "microcode id" or "microcode signature" field for Intel instead of the proposed "platform_id" field. The microcode id field would have both the contents of cpuid(1).eax *and* "1 << MSR 17h [52:50]", e.g. "microcode_id: 0x106a5, 0x02". > This patch just exports the value retrieved by the microcode > driver in /proc/cpuinfo. If the microcode driver is disabled > it won't be shown, but that seems reasonable. Agreed. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
[PATCH] x86: Report Intel platform_id in /proc/cpuinfo
From: Andi KleenWe have a need to distinguish systems based on their platform ID. For example this is useful to distinguish systems with L4 cache versus ones without. There is a 5 bit identifier (also called processor flags) in the IA32_PLATFORM_ID MSR that can give a more fine grained identification of the CPU than just the model number/stepping. IA32_PLATFORM_ID is architectural. The processor flags are already used in the microcode driver. The MSR can be also accessed through /dev/cpu/*/msr, but that requires root and is awkward. This patch just exports the value retrieved by the microcode driver in /proc/cpuinfo. If the microcode driver is disabled it won't be shown, but that seems reasonable. Signed-off-by: Andi Kleen --- arch/x86/include/asm/processor.h | 1 + arch/x86/kernel/cpu/microcode/intel.c | 1 + arch/x86/kernel/cpu/proc.c| 2 ++ 3 files changed, 4 insertions(+) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 20c11d1..bc4b0ef 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -136,6 +136,7 @@ struct cpuinfo_x86 { /* Index into per_cpu list: */ u16 cpu_index; u32 microcode; + u32 platform_id; }; #define X86_VENDOR_INTEL 0 diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index ee81c54..6244a88 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -812,6 +812,7 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig) /* get processor flags from MSR 0x17 */ rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); csig->pf = 1 << ((val[1] >> 18) & 7); + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; } csig->rev = c->microcode; diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c index 18ca99f..1c4e4f5 100644 --- a/arch/x86/kernel/cpu/proc.c +++ b/arch/x86/kernel/cpu/proc.c @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) seq_puts(m, "stepping\t: unknown\n"); if (c->microcode) seq_printf(m, "microcode\t: 0x%x\n", c->microcode); + if (c->platform_id) + seq_printf(m, "platform_id\t: %d\n", c->platform_id); if (cpu_has(c, X86_FEATURE_TSC)) { unsigned int freq = cpufreq_quick_get(cpu); -- 2.8.1
[PATCH] x86: Report Intel platform_id in /proc/cpuinfo
From: Andi Kleen We have a need to distinguish systems based on their platform ID. For example this is useful to distinguish systems with L4 cache versus ones without. There is a 5 bit identifier (also called processor flags) in the IA32_PLATFORM_ID MSR that can give a more fine grained identification of the CPU than just the model number/stepping. IA32_PLATFORM_ID is architectural. The processor flags are already used in the microcode driver. The MSR can be also accessed through /dev/cpu/*/msr, but that requires root and is awkward. This patch just exports the value retrieved by the microcode driver in /proc/cpuinfo. If the microcode driver is disabled it won't be shown, but that seems reasonable. Signed-off-by: Andi Kleen --- arch/x86/include/asm/processor.h | 1 + arch/x86/kernel/cpu/microcode/intel.c | 1 + arch/x86/kernel/cpu/proc.c| 2 ++ 3 files changed, 4 insertions(+) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 20c11d1..bc4b0ef 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -136,6 +136,7 @@ struct cpuinfo_x86 { /* Index into per_cpu list: */ u16 cpu_index; u32 microcode; + u32 platform_id; }; #define X86_VENDOR_INTEL 0 diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index ee81c54..6244a88 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -812,6 +812,7 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig) /* get processor flags from MSR 0x17 */ rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); csig->pf = 1 << ((val[1] >> 18) & 7); + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; } csig->rev = c->microcode; diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c index 18ca99f..1c4e4f5 100644 --- a/arch/x86/kernel/cpu/proc.c +++ b/arch/x86/kernel/cpu/proc.c @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) seq_puts(m, "stepping\t: unknown\n"); if (c->microcode) seq_printf(m, "microcode\t: 0x%x\n", c->microcode); + if (c->platform_id) + seq_printf(m, "platform_id\t: %d\n", c->platform_id); if (cpu_has(c, X86_FEATURE_TSC)) { unsigned int freq = cpufreq_quick_get(cpu); -- 2.8.1