[Xen-devel] [PATCH] x86/boot: Print the CPU model string alongside the Family/Model/Stepping info

Andrew Cooper posted 1 patch 10 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/1557159106-32381-1-git-send-email-andrew.cooper3@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(-)

[Xen-devel] [PATCH] x86/boot: Print the CPU model string alongside the Family/Model/Stepping info

Posted by Andrew Cooper 10 weeks ago
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(-)

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);
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86/boot: Print the CPU model string alongside the Family/Model/Stepping info

Posted by Jan Beulich 10 weeks ago
>>> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel