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

["Nikunj A. Dadhania" <nikunj@amd.com>]

On 3/10/2026 12:57 AM, Dave Hansen wrote:
> On 3/9/26 11:40, Tom Lendacky wrote:
>> The SNP guest is dying in __x2apic_enable() when trying to read
>> MSR_IA32_APICBASE, which will trigger a #VC.
>>
>> If I set CR4[16] in cr4_init() then the SNP guest boots fine.
> 
> That sounds pretty definitive.

Thanks Dave and Tom for the help to uncover CR4.FSGSBASE implicit dependency.

> How does this work on the boot CPU? How does it manage to get FSGSBASE
> set up before __x2apic_enable()? 

The boot CPU doesn't have FSGSBASE enabled before __x2apic_enable() either.

> Or is it on the early exception code, which might not use FSGSBASE instructions?

Correct. The key difference is timing relative to ALTERNATIVE patching. I added instrumentation to verify this behavior:

  Boot CPU sequence (without CR pinning disable patch):
     traps: trap_init() ENTRY: CR4.FSGSBASE=0, cpu=0
     traps: trap_init() AFTER cpu_init: CR4.FSGSBASE=0, cpu=0
     arch_cpu_finalize_init() ENTRY: CR4.FSGSBASE=0, cpu=0
     identify_boot_cpu() ENTRY: CR4.FSGSBASE=0, cpu=0
     identify_boot_cpu() EXIT: CR4.FSGSBASE=1, cpu=0
     SMP alternatives: BEFORE apply_alternatives: CR4.FSGSBASE=1, boot_cpu_has(FSGSBASE)=1, cpu=0
     SMP alternatives: Starting ALTERNATIVE patching

The boot CPU enables CR4.FSGSBASE in identify_boot_cpu() *before* ALTERNATIVE
patching occurs in alternative_instructions(). This means 

  cpu_init()
  -> x2apic_setup()
     -> __x2apic_enable()

executes unpatched paranoid_entry() code that uses RDMSR/SWAPGS instead of
RDGSBASE/WRGSBASE. Due to this, boot CPU does not have this problem.

  Secondary CPU sequence (without CR pinning disable patch):
     smpboot: start_secondary() BEFORE cr4_init: CR4.FSGSBASE=0, cpu=1
     cr4_init() ENTRY: CR4=0x10f0, FSGSBASE=0, cpu=1
     cr4_init() EXIT: CR4=0x10f0->0x3318f0, FSGSBASE=0->1, cpu=1, pinning=1

Secondary CPUs boot after alternatives have been applied globally. They
execute already-patched paranoid_entry() code that uses RDGSBASE/WRGSBASE
instructions, which require CR4.FSGSBASE=1.

Currently, secondary CPUs get CR4.FSGSBASE set implicitly through CR pinning.
The CR pinning disable patch removes this implicit setting, exposing the
hidden dependency. Without CR4.FSGSBASE enabled, RDGSBASE/WRGSBASE should
trigger #UD.

Note: I also verified the root cause by disabling the FSGSBASE ALTERNATIVE
patching, which forced the code to always use RDMSR/SWAPGS. With this change,
SNP guests boot successfully even without CR4.FSGSBASE set early, confirming
the issue is the timing between ALTERNATIVE patching (global) and CR4.FSGSBASE
enablement(per-CPU)
 
> Either way, I do think this needs to get fixed up. It was not acceptable
> for cr4_init() implicitly to set pinned features and then have the CPU
> boot code come along and do:
> 
> 	cr4_set_bits(X86_CR4_FSGSBASE);
> 
> It all basically worked by accident before.

Agreed. The attached pre-patch makes the dependency explicit by directly
enabling CR4.FSGSBASE in cr4_init() when the feature is available. This
ensures secondary CPUs have FSGSBASE enabled before any exceptions can
occur, regardless of CR pinning state:

  Secondary CPU sequence (with this patch):
    smpboot: start_secondary() BEFORE cr4_init: CR4.FSGSBASE=0, cpu=1
    cr4_init() ENTRY: CR4=0x10f0, FSGSBASE=0, cpu=1
    cr4_init() EXIT: CR4=0x10f0->0x310f0, FSGSBASE=0->1, cpu=1, pinning=0

-------------------------------------------------------------------------

From: Nikunj A Dadhania <nikunj@amd.com>
Subject: [PATCH] x86/cpu: Enable FSGSBASE early in cr4_init()

== Background ==

Exception entry code (paranoid_entry()) uses ALTERNATIVE patching based on
X86_FEATURE_FSGSBASE to decide whether to use RDGSBASE/WRGSBASE
instructions or the slower RDMSR/SWAPGS sequence for saving/restoring
GSBASE.

For boot cpu, ALTERNATIVE patching happens after enabling FSGSBASE in CR4.
When the feature is available, the code is permanently patched to use
RDGSBASE/WRGSBASE, which require CR4.FSGSBASE=1 to execute without
triggering #UD.

== Boot Sequence ==

Boot CPU (with CR pinning enabled):
  trap_init()
    cpu_init()                   <- Uses unpatched code (RDMSR/SWAPGS)
      x2apic_setup()
  ...
  arch_cpu_finalize_init()
    identify_boot_cpu()
      identify_cpu()
        cr4_set_bits(X86_CR4_FSGSBASE)  # Enables the feature
                                        # This becomes part of cr4_pinned_bits
    ...
    alternative_instructions()   <- Patches code to use RDGSBASE/WRGSBASE

Secondary CPUs (with CR pinning enabled):
  start_secondary()
    cr4_init()                   <- Code already patched, CR4.FSGSBASE=1
                                    set implicitly via cr4_pinned_bits

    cpu_init()                   <- exceptions work because FSGSBASE is
                                    already enabled

Secondary CPU (with CR pinning disabled):
  start_secondary()
    cr4_init()                   <- Code already patched, CR4.FSGSBASE=0
    cpu_init()
      x2apic_setup()
        rdmsrq(MSR_IA32_APICBASE)  <- Triggers #VC in SNP guests
          exc_vmm_communication()
            paranoid_entry()       <- Uses RDGSBASE with CR4.FSGSBASE=0
                                      (patched code)
    ...
    ap_starting()
      identify_secondary_cpu()
        identify_cpu()
	  cr4_set_bits(X86_CR4_FSGSBASE)  <- Enables the feature, which is
                                             too late

== CR Pinning ==

Currently, CR4.FSGSBASE is set implicitly through CR pinning: the boot CPU
sets it during identify_cpu(), it becomes part of cr4_pinned_bits, and
cr4_init() applies those pinned bits to secondary CPUs. This works but
creates an undocumented dependency between cr4_init() and the pinning
mechanism.

== Problem ==

Secondary CPUs boot after alternatives have been applied globally. They
execute already-patched paranoid_entry() code that uses RDGSBASE/WRGSBASE
instructions, which require CR4.FSGSBASE=1. Upcoming changes to CR pinning
behavior will break the implicit dependency, causing secondary CPUs to
generate #UD.

This issue manifests on AMD SEV-SNP guests, where the rdmsrq() in
x2apic_setup() triggers a #VC exception early during cpu_init(). The #VC
handler (exc_vmm_communication()) executes the patched paranoid_entry()
path. Without CR4.FSGSBASE enabled, RDGSBASE instructions trigger #UD.

== Fix ==

Make the dependency explicit by directly enabling CR4.FSGSBASE in
cr4_init() when the feature is available. This ensures secondary CPUs have
FSGSBASE enabled before any exceptions can occur, matching the boot CPU's
final state.

Fixes: c82965f9e530 ("x86/entry/64: Handle FSGSBASE enabled paranoid entry/exit")
Cc: stable@vger.kernel.org
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Sohil Mehta <sohil.mehta@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Reported-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
 arch/x86/kernel/cpu/common.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 1c3261cae40c..f5f9b242a983 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -502,6 +502,16 @@ void cr4_init(void)
 
 	if (boot_cpu_has(X86_FEATURE_PCID))
 		cr4 |= X86_CR4_PCIDE;
+
+	/*
+	 * 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;
+
 	if (static_branch_likely(&cr_pinning))
 		cr4 = (cr4 & ~cr4_pinned_mask) | cr4_pinned_bits;
 
-- 
2.48.1