| Message ID | 1557159106-32381-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/boot: Print the CPU model string alongside the Family/Model/Stepping info | expand |
>>> On 06.05.19 at 18:11, <andrew.cooper3@citrix.com> wrote: > @@ -303,13 +303,18 @@ void __init early_cpu_init(void) > c->x86_capability[cpufeat_word(X86_FEATURE_FPU)] = edx; > c->x86_capability[cpufeat_word(X86_FEATURE_SSE3)] = ecx; > > + c->extended_cpuid_level = cpuid_eax(0x80000000); > + if ((c->extended_cpuid_level >> 16) != 0x8000) > + c->extended_cpuid_level = 0; > + > printk(XENLOG_INFO > "CPU Vendor: %s, Family %u (%#x), Model %u (%#x), Stepping %u (raw %08x)\n", > x86_cpuid_vendor_to_str(c->x86_vendor), c->x86, c->x86, > c->x86_model, c->x86_model, c->x86_mask, eax); > + if (get_model_name(c)) > + printk(XENLOG_INFO "CPU Model: %.48s\n", c->x86_model_id); Afaics the function doesn't return false when the three leaves are all zero. In this case the line should imo not be logged. Furthermore this extra line is redundant with print_cpu_info() as well as against the idea of the "cpuinfo" command line option (intended to be used to log non-essential details). I'd certainly prefer the extra line to be qualified by an opt_cpu_info check, but I won't insist. In any event I'd like to ask though that the redundancy be addressed. Jan
diff --git a/xen/arch/x86/cpu/centaur.c b/xen/arch/x86/cpu/centaur.c index 34a5bfc..0e634a1 100644 --- a/xen/arch/x86/cpu/centaur.c +++ b/xen/arch/x86/cpu/centaur.c @@ -44,7 +44,6 @@ static void init_c3(struct cpuinfo_x86 *c) __set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability); } - get_model_name(c); display_cacheinfo(c); } diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c index fa8548e..12172aa 100644 --- a/xen/arch/x86/cpu/common.c +++ b/xen/arch/x86/cpu/common.c @@ -188,13 +188,13 @@ void ctxt_switch_levelling(const struct vcpu *next) bool_t opt_cpu_info; boolean_param("cpuinfo", opt_cpu_info); -int get_model_name(struct cpuinfo_x86 *c) +static bool get_model_name(struct cpuinfo_x86 *c) { unsigned int *v; char *p, *q; if (c->extended_cpuid_level < 0x80000004) - return 0; + return false; v = (unsigned int *) c->x86_model_id; cpuid(0x80000002, &v[0], &v[1], &v[2], &v[3]); @@ -214,7 +214,7 @@ int get_model_name(struct cpuinfo_x86 *c) *q++ = '\0'; /* Zero-pad the rest */ } - return 1; + return true; } @@ -303,13 +303,18 @@ void __init early_cpu_init(void) c->x86_capability[cpufeat_word(X86_FEATURE_FPU)] = edx; c->x86_capability[cpufeat_word(X86_FEATURE_SSE3)] = ecx; + c->extended_cpuid_level = cpuid_eax(0x80000000); + if ((c->extended_cpuid_level >> 16) != 0x8000) + c->extended_cpuid_level = 0; + printk(XENLOG_INFO "CPU Vendor: %s, Family %u (%#x), Model %u (%#x), Stepping %u (raw %08x)\n", x86_cpuid_vendor_to_str(c->x86_vendor), c->x86, c->x86, c->x86_model, c->x86_model, c->x86_mask, eax); + if (get_model_name(c)) + printk(XENLOG_INFO "CPU Model: %.48s\n", c->x86_model_id); - eax = cpuid_eax(0x80000000); - if ((eax >> 16) == 0x8000 && eax >= 0x80000008) { + if (c->extended_cpuid_level >= 0x80000008) { eax = cpuid_eax(0x80000008); paddr_bits = eax & 0xff; if (paddr_bits > PADDR_BITS) diff --git a/xen/arch/x86/cpu/cpu.h b/xen/arch/x86/cpu/cpu.h index 54bd0d3..d30f42b 100644 --- a/xen/arch/x86/cpu/cpu.h +++ b/xen/arch/x86/cpu/cpu.h @@ -12,5 +12,4 @@ extern unsigned int opt_cpuid_mask_ecx, opt_cpuid_mask_edx; extern unsigned int opt_cpuid_mask_xsave_eax; extern unsigned int opt_cpuid_mask_ext_ecx, opt_cpuid_mask_ext_edx; -extern int get_model_name(struct cpuinfo_x86 *c); extern void display_cacheinfo(struct cpuinfo_x86 *c);
This is also useful information when looking at boot logs. To do this, reuse get_model_name() which requires c->extended_cpuid_level to be calculated. early_cpu_init() already opencodes the calculation, so set c->extended_cpuid_level directly. While playing in this area, clean up get_model_name(). Its use in generic_identify() makes the sole external call from the Centaur code redundant. Make it local and switch to using a boolean return value. As sample boot now looks like: (XEN) CPU Vendor: Intel, Family 6 (0x6), Model 60 (0x3c), Stepping 3 (raw 000306c3) (XEN) CPU Model: Intel(R) Xeon(R) CPU E3-1240 v3 @ 3.40GHz Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wei.liu2@citrix.com> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/cpu/centaur.c | 1 - xen/arch/x86/cpu/common.c | 15 ++++++++++----- xen/arch/x86/cpu/cpu.h | 1 - 3 files changed, 10 insertions(+), 7 deletions(-)