Re: [PATCH v2 1/2] x86/cpu: Disable CR pinning during CPU bringup

[Nikunj A. Dadhania]

On 3/11/2026 7:37 PM, Dave Hansen wrote:
> Nikunj, thanks for tracking this down and filling in the last piece of
> the puzzle about ALTERNATIVEs patching.
> 
> On 3/11/26 03:41, Nikunj A. Dadhania wrote:
>> +	/*
>> +	 * Enable FSGSBASE if available. Exception entry code (paranoid_entry)
>> +	 * is patched to use RDGSBASE/WRGSBASE when this feature is present,
>> +	 * and those instructions require CR4.FSGSBASE=1. Secondary CPUs must
>> +	 * enable this before any exceptions occur.
>> +	 */
>> +	if (boot_cpu_has(X86_FEATURE_FSGSBASE))
>> +		cr4 |= X86_CR4_FSGSBASE;
> 
> But this still double-enables X86_CR4_FSGSBASE. Could we initialize
> X86_CR4_FSGSBASE in *one* place, please?

Makes sense. Moving X86_CR4_FSGSBASE enablement to cr4_init() and calling cr4_init()
early in the boot CPU path eliminates the double-enable. I've tested this lightly - 
let me know if this is the right approach.

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index a24c7805acdb..98efbd13a5a6 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -595,7 +595,7 @@ extern void load_fixmap_gdt(int);
 extern void cpu_init(void);
 extern void cpu_init_exception_handling(bool boot_cpu);
 extern void cpu_init_replace_early_idt(void);
-extern void cr4_init(void);
+extern void cr4_init(bool boot_cpu);
 
 extern void set_task_blockstep(struct task_struct *task, bool on);
 
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 1c3261cae40c..aa07e2eef228 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -496,14 +496,26 @@ unsigned long cr4_read_shadow(void)
 }
 EXPORT_SYMBOL_FOR_KVM(cr4_read_shadow);
 
-void cr4_init(void)
+void cr4_init(bool boot_cpu)
 {
 	unsigned long cr4 = __read_cr4();
 
-	if (boot_cpu_has(X86_FEATURE_PCID))
-		cr4 |= X86_CR4_PCIDE;
-	if (static_branch_likely(&cr_pinning))
-		cr4 = (cr4 & ~cr4_pinned_mask) | cr4_pinned_bits;
+	/*
+	 * CPUs that support FSGSBASE may use RDGSBASE/WRGSBASE in
+	 * paranoid_entry(). Enable the feature before any exceptions
+	 * occur.
+	 */
+	if (boot_cpu_has(X86_FEATURE_FSGSBASE)) {
+		cr4 |= X86_CR4_FSGSBASE;
+		elf_hwcap2 |= HWCAP2_FSGSBASE;
+	}
+
+	if (!boot_cpu) {
+		if (boot_cpu_has(X86_FEATURE_PCID))
+			cr4 |= X86_CR4_PCIDE;
+		if (static_branch_likely(&cr_pinning))
+			cr4 = (cr4 & ~cr4_pinned_mask) | cr4_pinned_bits;
+	}
 
 	__write_cr4(cr4);
 
@@ -2047,12 +2059,6 @@ static void identify_cpu(struct cpuinfo_x86 *c)
 	setup_umip(c);
 	setup_lass(c);
 
-	/* Enable FSGSBASE instructions if available. */
-	if (cpu_has(c, X86_FEATURE_FSGSBASE)) {
-		cr4_set_bits(X86_CR4_FSGSBASE);
-		elf_hwcap2 |= HWCAP2_FSGSBASE;
-	}
-
 	/*
 	 * The vendor-specific functions might have changed features.
 	 * Now we do "generic changes."
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 5cd6950ab672..52642908c2bf 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -233,7 +233,7 @@ static void notrace __noendbr start_secondary(void *unused)
 	 * before cpu_init(), SMP booting is too fragile that we want to
 	 * limit the things done here to the most necessary things.
 	 */
-	cr4_init();
+	cr4_init(false);
 
 	/*
 	 * 32-bit specific. 64-bit reaches this code with the correct page
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 4dbff8ef9b1c..0f7400257ce5 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -1685,6 +1685,13 @@ void __init trap_init(void)
 	/* Init GHCB memory pages when running as an SEV-ES guest */
 	sev_es_init_vc_handling();
 
+	/*
+	 * Initialize CR4 early, before cpu_init(). This ensures features like
+	 * FSGSBASE are enabled before exception handlers run, avoiding double
+	 * initialization later.
+	 */
+	cr4_init(true);
+
 	/* Initialize TSS before setting up traps so ISTs work */
 	cpu_init_exception_handling(true);
 
diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c
index db9b8e222b38..2ab0e20288e9 100644
--- a/arch/x86/xen/smp_pv.c
+++ b/arch/x86/xen/smp_pv.c
@@ -58,7 +58,7 @@ static void cpu_bringup(void)
 {
 	int cpu;
 
-	cr4_init();
+	cr4_init(false);
 	cpuhp_ap_sync_alive();
 	cpu_init();
 	fpu__init_cpu();


> 
> Also, please avoid passive voice in stuff like this. It's just more
> efficient to say:
> 
> 	CPUs that support FSGSBASE may use RDGSBASE/WRGSBASE in
> 	paranoid_entry(). Enable the feature before any exceptions
> 	occur.

Ack

Regards,
Nikunj