Re: [PATCH] x86/fred: Fix early boot failures on SEV-ES/SNP guests

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

On 2/5/2026 12:41 PM, Xin Li wrote:
>> On Feb 4, 2026, at 9:10 PM, Nikunj A Dadhania <nikunj@amd.com> wrote:

>>
>> * For secondary CPUs, FRED is enabled before setting up the FRED MSRs, and
>>  console output triggers a #VC which cannot be handled
> 
> Yes, this is a problem.  I ever looked into it for TDX, and had the following patch:
> 
> Can you please check if it works for you (#VC handler is set in the bringup IDT on AMD)?

Yes, this works as well. With your change that moves cr4_init(), I no longer
need my arch/x86/kernel/fred.c modification (moving pr_info() to avoid the #VC).
SEV-ES / SEV-SNP guests boot successfully with FRED enabled.

Are you planning to post this for inclusion?

Regards
Nikunj
 I 
> 
> 
>     x86/smp: Set up exception handling before cr4_init()
>     
>     The current AP boot sequence initializes CR4 before setting up
>     exception handling.  With FRED enabled, however, CR4.FRED is set
>     prior to initializing the FRED configuration MSRs, introducing a
>     brief window where a triple fault could occur.  This isn't
>     considered a problem, as the early boot code is carefully designed
>     to avoid triggering exceptions.  Moreover, if an exception does
>     occur at this stage, it's preferable for the CPU to triple fault
>     rather than risk a potential exploit.
>     
>     However, under TDX, printk() triggers a #VE, so any logging during
>     this small window results in a triple fault.
>     
>     Swap the order of cr4_init() and cpu_init_exception_handling(),
>     since cr4_init() only involves reading from and writing to CR4,
>     and setting up exception handling does not depend on any specific
>     CR4 bits being set (Arguably CR4.PAE, CR4.PSE and CR4.PGE are
>     related but they are already set before start_secondary() anyway).
>     
>     Notably, this triple fault can still occur before FRED is enabled,
>     while the bringup IDT is in use, since it lacks a #VE handler.
>     
>     BTW, on 32-bit systems, loading CR3 with swapper_pg_dir is moved
>     ahead of cr4_init(), which appears to be harmless.
>     
>     Signed-off-by: Xin Li (Intel) <xin@zytor.com>
> 
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index eb289abece23..24497258c16b 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -231,13 +231,6 @@ static void ap_calibrate_delay(void)
>   */
>  static void notrace __noendbr start_secondary(void *unused)
>  {
> -	/*
> -	 * Don't put *anything* except direct CPU state initialization
> -	 * before cpu_init(), SMP booting is too fragile that we want to
> -	 * limit the things done here to the most necessary things.
> -	 */
> -	cr4_init();
> -
>  	/*
>  	 * 32-bit specific. 64-bit reaches this code with the correct page
>  	 * table established. Yet another historical divergence.
> @@ -248,8 +241,37 @@ static void notrace __noendbr start_secondary(void *unused)
>  		__flush_tlb_all();
>  	}
>  
> +	/*
> +	 * AP startup assembly code has setup the following before calling
> +	 * start_secondary() on 64-bit:
> +	 *
> +	 * 1) CS set to __KERNEL_CS.
> +	 * 2) CR3 switched to the init_top_pgt.
> +	 * 3) CR4.PAE, CR4.PSE and CR4.PGE are set.
> +	 * 4) GDT set to per-CPU gdt_page.
> +	 * 5) ALL data segments set to the NULL descriptor.
> +	 * 6) MSR_GS_BASE set to per-CPU offset.
> +	 * 7) IDT set to bringup IDT.
> +	 * 8) CR0 set to CR0_STATE.
> +	 *
> +	 * So it's ready to setup exception handling.
> +	 */
>  	cpu_init_exception_handling(false);
>  
> +	/*
> +	 * Ensure bits set in cr4_pinned_bits are set in CR4.
> +	 *
> +	 * cr4_pinned_bits is a subset of cr4_pinned_mask, which includes
> +	 * the following bits:
> +	 *         X86_CR4_SMEP
> +	 *         X86_CR4_SMAP
> +	 *         X86_CR4_UMIP
> +	 *         X86_CR4_FSGSBASE
> +	 *         X86_CR4_CET
> +	 *         X86_CR4_FRED
> +	 */
> +	cr4_init();
> +
>  	/*
>  	 * Load the microcode before reaching the AP alive synchronization
>  	 * point below so it is not part of the full per CPU serialized
> @@ -275,6 +297,11 @@ static void notrace __noendbr start_secondary(void *unused)
>  	 */
>  	cpuhp_ap_sync_alive();
>  
> +	/*
> +	 * Don't put *anything* except direct CPU state initialization
> +	 * before cpu_init(), SMP booting is too fragile that we want to
> +	 * limit the things done here to the most necessary things.
> +	 */
>  	cpu_init();
>  	fpu__init_cpu();
>  	rcutree_report_cpu_starting(raw_smp_processor_id());
>