[1/2] x86/CPU: re-work populating of cpu_data[]

Commit Message

Jan Beulich Feb. 12, 2024, 12:53 p.m. UTC

Forever since its introduction x86_mc_get_cpu_info() has been checking a
structure field against BAD_APICID which would never have been set to
that particular value. Arrange for this to actually be true ahead of
CPU detection having run, which then also includes pruning after CPU
removal.

For this,
- introduce a new helper function splitting reset logic out of
  identify_cpu(), and going slightly farther than the original code to
  achieve consistent results,
- introduce a new helper macro for both initializing and resetting
  various fields.

For boot_cpu_data this in particular means that cpuid_level will no
longer be set to the bogus value of -1. Even before removal of 32-bit
support we already assumed basic CPUID leaves to be available.

Furthermore make sure initialize_cpu_data() doesn't populate fields with
BSP's values which clearly aren't going to be applicable to APs.

Fixes: e16fc5ba591c ("x86: mce: Provide extended physical CPU info")
Signed-off-by: Jan Beulich <[e-mail]>

Patch

--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -413,6 +413,19 @@  void __init early_cpu_init(bool verbose)
 	initialize_cpu_data(0);
 }
 
+void reset_cpuinfo(struct cpuinfo_x86 *c, bool keep_basic)
+{
+	if (!keep_basic) {
+		c->x86 = c->x86_vendor = 0;	/* So far unknown... */
+		c->x86_model = c->x86_mask = 0;	/* So far unknown... */
+		memset(&c->x86_capability, 0, sizeof(c->x86_capability));
+		memset(&c->x86_vendor_id, 0, sizeof(c->x86_vendor_id));
+		memset(&c->x86_model_id, 0, sizeof(c->x86_model_id));
+	}
+
+	CPU_DATA_INIT((*c));
+}
+
 static void generic_identify(struct cpuinfo_x86 *c)
 {
 	u32 eax, ebx, ecx, edx, tmp;
@@ -518,16 +531,7 @@  void identify_cpu(struct cpuinfo_x86 *c)
 {
 	int i;
 
-	c->x86_cache_size = -1;
-	c->x86_model = c->x86_mask = 0;	/* So far unknown... */
-	c->x86_model_id[0] = '\0';  /* Unset */
-	c->x86_max_cores = 1;
-	c->x86_num_siblings = 1;
-	c->x86_clflush_size = 0;
-	c->cpu_core_id = XEN_INVALID_CORE_ID;
-	c->compute_unit_id = INVALID_CUID;
-	memset(&c->x86_capability, 0, sizeof c->x86_capability);
-
+	reset_cpuinfo(c, false);
 	generic_identify(c);
 
 #ifdef NOISY_CAPS
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -40,6 +40,24 @@  struct cpuinfo_x86 {
     unsigned short x86_clflush_size;
 } __cacheline_aligned;
 
+#define CPU_DATA_INIT(what...)                     \
+        what.cpuid_level = 1,                      \
+        what.extended_cpuid_level = 0,             \
+        what.x86_cache_size = -1,                  \
+        what.x86_max_cores = 1,                    \
+        what.x86_num_siblings = 1,                 \
+        what.apicid = BAD_APICID,                  \
+        what.phys_proc_id = XEN_INVALID_SOCKET_ID, \
+        what.cpu_core_id = XEN_INVALID_CORE_ID,    \
+        what.compute_unit_id = INVALID_CUID
+
+/*
+ * @keep_basic set to true retains data firmly assumed to be symmetric
+ * across all CPUs.  With it set to false only CPU_DATA_INIT() will be
+ * invoked on the passed structure.
+ */
+void reset_cpuinfo(struct cpuinfo_x86 *c, bool keep_basic);
+
 extern struct cpuinfo_x86 boot_cpu_data;
 
 static inline bool cpu_has(const struct cpuinfo_x86 *info, unsigned int feat)
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -157,7 +157,7 @@  void *stack_start = cpu0_stack + STACK_S
 /* Used by the boot asm to stash the relocated multiboot info pointer. */
 unsigned int asmlinkage __initdata multiboot_ptr;
 
-struct cpuinfo_x86 __read_mostly boot_cpu_data = { 0, 0, 0, 0, -1 };
+struct cpuinfo_x86 __read_mostly boot_cpu_data = { CPU_DATA_INIT() };
 
 unsigned long __read_mostly mmu_cr4_features = XEN_MINIMAL_CR4;
 
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -61,7 +61,8 @@  unsigned int __read_mostly nr_sockets;
 cpumask_t **__read_mostly socket_cpumask;
 static cpumask_t *secondary_socket_cpumask;
 
-struct cpuinfo_x86 cpu_data[NR_CPUS];
+struct cpuinfo_x86 __read_mostly cpu_data[NR_CPUS] =
+    { [0 ... NR_CPUS - 1] = { CPU_DATA_INIT() } };
 
 u32 x86_cpu_to_apicid[NR_CPUS] __read_mostly =
 	{ [0 ... NR_CPUS-1] = BAD_APICID };
@@ -81,7 +82,12 @@  void *stack_base[NR_CPUS];
 
 void initialize_cpu_data(unsigned int cpu)
 {
-    cpu_data[cpu] = boot_cpu_data;
+    struct cpuinfo_x86 c = boot_cpu_data;
+
+    /* Must not partially clear the BSP's collected data. */
+    if ( cpu || system_state > SYS_STATE_smp_boot )
+        reset_cpuinfo(&c, true);
+    cpu_data[cpu] = c;
 }
 
 static bool smp_store_cpu_info(unsigned int id)
@@ -954,7 +960,6 @@  static void cleanup_cpu_root_pgt(unsigne
 static void cpu_smpboot_free(unsigned int cpu, bool remove)
 {
     unsigned int socket = cpu_to_socket(cpu);
-    struct cpuinfo_x86 *c = cpu_data;
 
     if ( cpumask_empty(socket_cpumask[socket]) )
     {
@@ -966,9 +971,7 @@  static void cpu_smpboot_free(unsigned in
 
     if ( remove )
     {
-        c[cpu].phys_proc_id = XEN_INVALID_SOCKET_ID;
-        c[cpu].cpu_core_id = XEN_INVALID_CORE_ID;
-        c[cpu].compute_unit_id = INVALID_CUID;
+        reset_cpuinfo(&cpu_data[cpu], false);
 
         FREE_CPUMASK_VAR(per_cpu(cpu_sibling_mask, cpu));
         FREE_CPUMASK_VAR(per_cpu(cpu_core_mask, cpu));