[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <41a68344-e2e1-42f3-82a9-1b88cd4ba4d7@amd.com>
Date: Thu, 5 Feb 2026 14:24:24 +0530
From: "Nikunj A. Dadhania" <nikunj@....com>
To: Xin Li <xin@...or.com>
CC: <linux-kernel@...r.kernel.org>, <kvm@...r.kernel.org>, <bp@...en8.de>,
<thomas.lendacky@....com>, <tglx@...nel.org>, <mingo@...hat.com>,
<dave.hansen@...ux.intel.com>, <hpa@...or.com>, <seanjc@...gle.com>,
<pbonzini@...hat.com>, <x86@...nel.org>, <jon.grimm@....com>,
<stable@...r.kernel.org>
Subject: Re: [PATCH] x86/fred: Fix early boot failures on SEV-ES/SNP guests
On 2/5/2026 12:41 PM, Xin Li wrote:
>> On Feb 4, 2026, at 9:10 PM, Nikunj A Dadhania <nikunj@....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@...or.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());
>
Powered by blists - more mailing lists