Re: [PATCH v4 2/6] x86/microcode/intel: Establish staging control logic

["Chang S. Bae" <chang.seok.bae@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2327 bytes --]

On 8/13/2025 1:55 PM, Dave Hansen wrote:
> 
> But that's not the problem. The issue is that this line of code:
> 
> #define cpu_primary_thread_mask cpu_none_mask

With CONFIG_SMP=n, on the core side (include/linux/cpu_smt.h), the code 
clarifies there is no SMT:

# define cpu_smt_control               (CPU_SMT_NOT_IMPLEMENTED)

This leads kernel/cpu.c to return an empty mask:

static inline const struct cpumask *cpuhp_get_primary_thread_mask(void)
{
	return cpu_none_mask;
}

On the x86 side, the definition is explicit that “primary threads” are 
SMT threads (arch/x86/kernel/smpboot.c):

/* CPUs which are the primary SMT threads */
struct cpumask __cpu_primary_thread_mask __read_mostly;

And via ifdeffery, this mask is only available to SMP kernels.

So it seems I had been subscribing this model -- no primary threads 
without SMP.

> reads as 100% bogus to me. Even on !SMP kernels,
> 'cpu_primary_thread_mask' should have one CPU in it. Right? The _one_
> thread that's present is a primary thread. If this were a mask for
> secondary threads, 'cpu_none_mask' would make sense. But it's not.

Your confidence made me take another look.

Digging into the history, I found that x86 used to have this in the !SMP 
case:

static inline bool topology_is_primary_thread(unsigned int cpu)
{
	return true;
}

That stayed until the recent commit 4b455f59945aa ("cpu/SMT: Provide a 
default topology_is_primary_thread()"), which now defines it in 
include/linux/topology.h with this telling comment:

/*
  * When disabling SMT, the primary thread of the SMT will remain
  * enabled/active. Architectures that have a special primary thread
  * (e.g. x86) need to override this function. ...
  */

This comment basically supports your point.

> So could we please make use 'cpu_primary_thread_mask' is getting defined
> correctly whether it's really getting compiled into the end image or not?

Given that, I’m thinking of simplifying the x86 side a bit -- by making 
the primary thread mask configured and available regardless of 
CONFIG_SMP, matching the behavior of other cpumasks. And its relevant 
helpers are also available, like in the attached diff.

I think the change still aligns x86 with the core code -- especially 
with the note in topology_is_primary_thread(). With that, the user may 
be introduced here.

[-- Attachment #2: tmp.diff --]
[-- Type: text/plain, Size: 2947 bytes --]

diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index 6c79ee7c0957..281252af6e9d 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -218,6 +218,12 @@ static inline unsigned int topology_amd_nodes_per_pkg(void)
 	return __amd_nodes_per_pkg;
 }
 
+#else /* CONFIG_SMP */
+static inline int topology_phys_to_logical_pkg(unsigned int pkg) { return 0; }
+static inline int topology_max_smt_threads(void) { return 1; }
+static inline unsigned int topology_amd_nodes_per_pkg(void) { return 1; }
+#endif /* !CONFIG_SMP */
+
 extern struct cpumask __cpu_primary_thread_mask;
 #define cpu_primary_thread_mask ((const struct cpumask *)&__cpu_primary_thread_mask)
 
@@ -231,12 +237,6 @@ static inline bool topology_is_primary_thread(unsigned int cpu)
 }
 #define topology_is_primary_thread topology_is_primary_thread
 
-#else /* CONFIG_SMP */
-static inline int topology_phys_to_logical_pkg(unsigned int pkg) { return 0; }
-static inline int topology_max_smt_threads(void) { return 1; }
-static inline unsigned int topology_amd_nodes_per_pkg(void) { return 1; }
-#endif /* !CONFIG_SMP */
-
 static inline void arch_fix_phys_package_id(int num, u32 slot)
 {
 }
diff --git a/arch/x86/kernel/cpu/topology.c b/arch/x86/kernel/cpu/topology.c
index e35ccdc84910..946004d7dd1d 100644
--- a/arch/x86/kernel/cpu/topology.c
+++ b/arch/x86/kernel/cpu/topology.c
@@ -52,6 +52,9 @@ u32 cpuid_to_apicid[] __ro_after_init = { [0 ... NR_CPUS - 1] = BAD_APICID, };
 /* Bitmaps to mark registered APICs at each topology domain */
 static struct { DECLARE_BITMAP(map, MAX_LOCAL_APIC); } apic_maps[TOPO_MAX_DOMAIN] __ro_after_init;
 
+/* CPUs which are the primary SMT threads */
+struct cpumask __cpu_primary_thread_mask __read_mostly;
+
 /*
  * Keep track of assigned, disabled and rejected CPUs. Present assigned
  * with 1 as CPU #0 is reserved for the boot CPU.
@@ -75,15 +78,11 @@ bool arch_match_cpu_phys_id(int cpu, u64 phys_id)
 	return phys_id == (u64)cpuid_to_apicid[cpu];
 }
 
-#ifdef CONFIG_SMP
 static void cpu_mark_primary_thread(unsigned int cpu, unsigned int apicid)
 {
 	if (!(apicid & (__max_threads_per_core - 1)))
 		cpumask_set_cpu(cpu, &__cpu_primary_thread_mask);
 }
-#else
-static inline void cpu_mark_primary_thread(unsigned int cpu, unsigned int apicid) { }
-#endif
 
 /*
  * Convert the APIC ID to a domain level ID by masking out the low bits
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 33e166f6ab12..7804175d2d87 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -103,9 +103,6 @@ EXPORT_PER_CPU_SYMBOL(cpu_core_map);
 DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_die_map);
 EXPORT_PER_CPU_SYMBOL(cpu_die_map);
 
-/* CPUs which are the primary SMT threads */
-struct cpumask __cpu_primary_thread_mask __read_mostly;
-
 /* Representing CPUs for which sibling maps can be computed */
 static cpumask_var_t cpu_sibling_setup_mask;