x86/fred: Fix early boot failures on SEV-ES/SNP guests
Subsystems (5)
PATCH 1/1 x86/fred: Fix early boot failures on SEV-ES/SNP guests
FRED enabled SEV-ES and SNP guests fail to boot due to the following issues in the early boot sequence:
* FRED does not have a #VC exception handler in the dispatch logic
* For secondary CPUs, FRED is enabled before setting up the FRED MSRs, and console output triggers a #VC which cannot be handled
* Early FRED #VC exceptions should use boot_ghcb until per-CPU GHCBs are initialized
Fix these issues to ensure SEV-ES/SNP guests can handle #VC exceptions correctly during early boot when FRED is enabled.
---
Reason to add stable tag:
With FRED support for SVM here
https://lore.kernel.org/kvm/[email protected],
SVM and SEV guests running 6.9 and later kernels will support FRED. However, *SEV-ES and SNP guests cannot support FRED* and will fail to boot with the following error:
[ 0.005144] Using GB pages for direct mapping
[ 0.008402] Initialize FRED on CPU0
qemu-system-x86_64: cpus are not resettable, terminatingThree problems were identified as detailed in the commit message above and is fixed with this patch.
I would like the patch to be backported to the LTS kernels (6.12 and 6.18) to ensure SEV-ES and SNP guests running these stable kernel versions can boot with FRED enabled on FRED-enabled hypervisors.
---
arch/x86/coco/sev/noinstr.c | 6 ++++++ arch/x86/entry/entry_fred.c | 5 +++++ arch/x86/kernel/fred.c | 14 +++++++++++--- 3 files changed, 22 insertions(+), 3 deletions(-)
arch/x86/coco/sev/noinstr.c +6 -0
@@ -121,6 +121,9 @@ noinstr struct ghcb *__sev_get_ghcb(struct ghcb_state *state)WARN_ON (!irqs_disabled ()); +if (!sev_cfg.ghcbs_initialized) +return boot_ghcb; + data =this_cpu_read (runtime_data); ghcb = &data->ghcb_page;
@@ -164,6 +167,9 @@ noinstr void __sev_put_ghcb(struct ghcb_state *state)WARN_ON (!irqs_disabled ()); +if (!sev_cfg.ghcbs_initialized) +return ; + data =this_cpu_read (runtime_data); ghcb = &data->ghcb_page;
arch/x86/entry/entry_fred.c +5 -0
@@ -208,6 +208,11 @@ static noinstr void fred_hwexc(struct pt_regs *regs, unsigned long error_code)#ifdef CONFIG_X86_CET case X86_TRAP_CP :return exc_control_protection (regs, error_code);#endif +case X86_TRAP_VC : +if (user_mode (regs)) +return user_exc_vmm_communication (regs, error_code); +else +return kernel_exc_vmm_communication (regs, error_code);default :return fred_bad_type (regs, error_code); }
arch/x86/kernel/fred.c +11 -3
@@ -27,9 +27,6 @@ EXPORT_PER_CPU_SYMBOL(fred_rsp0);void cpu_init_fred_exceptions (void ) { -/* When FRED is enabled by default, remove this log message */ -pr_info ("Initialize FRED on CPU%d\n" ,smp_processor_id ()); -/* * If a kernel event is delivered before a CPU goes to user level for * the first time, its SS is NULL thus NULL is pushed into the SS field
@@ -70,6 +67,17 @@ void cpu_init_fred_exceptions(void)/* Use int $0x80 for 32-bit system calls in FRED mode */ setup_clear_cpu_cap (X86_FEATURE_SYSFAST32 );setup_clear_cpu_cap (X86_FEATURE_SYSCALL32 ); + +/* +* For secondary processors, FRED bit in CR4 gets enabled in cr4_init() +* and FRED MSRs are not configured till the end of this function. For +* SEV-ES and SNP guests, any console write before the FRED MSRs are +* setup will cause a #VC and cannot be handled. Move the pr_info to +* the end of this function. +* +* When FRED is enabled by default, remove this log message +*/ +pr_info ("Initialized FRED on CPU%d\n" ,smp_processor_id ()); }/* Must be called after setup_cpu_entry_areas() */
base-commit: 3c2ca964f75460093a8aad6b314a6cd558e80e66
Signature · 1 line
2.48.1
Greg KH
maintainerQuoted 31 lines · Nikunj A Dadhania
FRED enabled SEV-ES and SNP guests fail to boot due to the following issues in the early boot sequence:
* FRED does not have a #VC exception handler in the dispatch logic
* For secondary CPUs, FRED is enabled before setting up the FRED MSRs, and console output triggers a #VC which cannot be handled
* Early FRED #VC exceptions should use
boot_ghcbuntil per-CPU GHCBs are initializedFix these issues to ensure SEV-ES/SNP guests can handle #VC exceptions correctly during early boot when FRED is enabled.
Fixes 14619d912b65 ("x86/fred: FRED entry/exit and dispatch code")Cc [email protected] # 6.9+Signed-off-by Nikunj A Dadhania <[email protected]>---
Reason to add stable tag:
With FRED support for SVM here
https://lore.kernel.org/kvm/[email protected],
SVM and SEV guests running 6.9 and later kernels will support FRED. However, *SEV-ES and SNP guests cannot support FRED* and will fail to boot with the following error:[ 0.005144] Using GB pages for direct mapping [ 0.008402] Initialize FRED on CPU0 qemu-system-x86_64: cpus are not resettable, terminating
Quoted 6 lines
Three problems were identified as detailed in the commit message above and is fixed with this patch.
I would like the patch to be backported to the LTS kernels (6.12 and 6.18) to ensure SEV-ES and SNP guests running these stable kernel versions can boot with FRED enabled on FRED-enabled hypervisors.
That sounds like new hardware support, if you really want that, why not just use newer kernel versions with this fix in it? Obviously no one is running those kernels on that hardware today, so this isn't a regression :)
thanks,
greg k-h
Quoted 35 lines · Greg KH
Quoted 34 lines · Nikunj A Dadhania
FRED enabled SEV-ES and SNP guests fail to boot due to the following issues in the early boot sequence:
* FRED does not have a #VC exception handler in the dispatch logic
* For secondary CPUs, FRED is enabled before setting up the FRED MSRs, and console output triggers a #VC which cannot be handled
* Early FRED #VC exceptions should use
boot_ghcbuntil per-CPU GHCBs are initializedFix these issues to ensure SEV-ES/SNP guests can handle #VC exceptions correctly during early boot when FRED is enabled.
Fixes 14619d912b65 ("x86/fred: FRED entry/exit and dispatch code")Cc [email protected] # 6.9+Signed-off-by Nikunj A Dadhania <[email protected]>---
Reason to add stable tag:
With FRED support for SVM here
https://lore.kernel.org/kvm/[email protected],
SVM and SEV guests running 6.9 and later kernels will support FRED. However, *SEV-ES and SNP guests cannot support FRED* and will fail to boot with the following error:[ 0.005144] Using GB pages for direct mapping [ 0.008402] Initialize FRED on CPU0 qemu-system-x86_64: cpus are not resettable, terminatingThree problems were identified as detailed in the commit message above and is fixed with this patch.
Quoted 7 lines
Quoted 3 lines
I would like the patch to be backported to the LTS kernels (6.12 and 6.18) to ensure SEV-ES and SNP guests running these stable kernel versions can boot with FRED enabled on FRED-enabled hypervisors.
That sounds like new hardware support, if you really want that, why not just use newer kernel versions with this fix in it? Obviously no one is running those kernels on that hardware today, so this isn't a regression :)
Fair point.
However, the situation is a bit nuanced: FRED hardware is available now, and users running current stable kernels as guests will encounter boot failures when the hypervisor is updated to support FRED. While not a traditional regression, it creates a compatibility gap where stable guest kernels cannot run on updated hypervisors.
Other option would be to disable FRED for SEV-ES and SNP guest in stable kernel.
Regards
Nikunj
Greg KH
maintainerQuoted 46 lines · Nikunj A. Dadhania
Quoted 35 lines · Greg KH
Quoted 34 lines · Nikunj A Dadhania
FRED enabled SEV-ES and SNP guests fail to boot due to the following issues in the early boot sequence:
* FRED does not have a #VC exception handler in the dispatch logic
* For secondary CPUs, FRED is enabled before setting up the FRED MSRs, and console output triggers a #VC which cannot be handled
* Early FRED #VC exceptions should use
boot_ghcbuntil per-CPU GHCBs are initializedFix these issues to ensure SEV-ES/SNP guests can handle #VC exceptions correctly during early boot when FRED is enabled.
Fixes 14619d912b65 ("x86/fred: FRED entry/exit and dispatch code")Cc [email protected] # 6.9+Signed-off-by Nikunj A Dadhania <[email protected]>---
Reason to add stable tag:
With FRED support for SVM here
https://lore.kernel.org/kvm/[email protected],
SVM and SEV guests running 6.9 and later kernels will support FRED. However, *SEV-ES and SNP guests cannot support FRED* and will fail to boot with the following error:[ 0.005144] Using GB pages for direct mapping [ 0.008402] Initialize FRED on CPU0 qemu-system-x86_64: cpus are not resettable, terminatingThree problems were identified as detailed in the commit message above and is fixed with this patch.
Quoted 7 lines
Quoted 3 lines
I would like the patch to be backported to the LTS kernels (6.12 and 6.18) to ensure SEV-ES and SNP guests running these stable kernel versions can boot with FRED enabled on FRED-enabled hypervisors.
That sounds like new hardware support, if you really want that, why not just use newer kernel versions with this fix in it? Obviously no one is running those kernels on that hardware today, so this isn't a regression :)
Quoted 7 lines
Fair point.
However, the situation is a bit nuanced: FRED hardware is available now, and users running current stable kernels as guests will encounter boot failures when the hypervisor is updated to support FRED. While not a traditional regression, it creates a compatibility gap where stable guest kernels cannot run on updated hypervisors.
Great, then upgrade those guest kernels as they have never been able to run on those hypervisors :)
Quoted 1 line
Other option would be to disable FRED for SEV-ES and SNP guest in stable kernel.
That's a choice for the hypervisor vendors to choose.
thanks,
greg k-h
Sean Christopherson
reviewerQuoted 39 lines · Greg KH
Quoted 38 lines · Nikunj A. Dadhania
Quoted 35 lines · Greg KH
Quoted 34 lines · Nikunj A Dadhania
FRED enabled SEV-ES and SNP guests fail to boot due to the following issues in the early boot sequence:
* FRED does not have a #VC exception handler in the dispatch logic
* For secondary CPUs, FRED is enabled before setting up the FRED MSRs, and console output triggers a #VC which cannot be handled
* Early FRED #VC exceptions should use
boot_ghcbuntil per-CPU GHCBs are initializedFix these issues to ensure SEV-ES/SNP guests can handle #VC exceptions correctly during early boot when FRED is enabled.
Fixes 14619d912b65 ("x86/fred: FRED entry/exit and dispatch code")Cc [email protected] # 6.9+Signed-off-by Nikunj A Dadhania <[email protected]>---
Reason to add stable tag:
With FRED support for SVM here
https://lore.kernel.org/kvm/[email protected],
SVM and SEV guests running 6.9 and later kernels will support FRED. However, *SEV-ES and SNP guests cannot support FRED* and will fail to boot with the following error:[ 0.005144] Using GB pages for direct mapping [ 0.008402] Initialize FRED on CPU0 qemu-system-x86_64: cpus are not resettable, terminatingThree problems were identified as detailed in the commit message above and is fixed with this patch.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
I would like the patch to be backported to the LTS kernels (6.12 and 6.18) to ensure SEV-ES and SNP guests running these stable kernel versions can boot with FRED enabled on FRED-enabled hypervisors.
That sounds like new hardware support, if you really want that, why not just use newer kernel versions with this fix in it? Obviously no one is running those kernels on that hardware today, so this isn't a regression :)
I disagree, this absolutely is a regression. Kernels without commit 14619d912b65 will boot on this "new" hardware, kernels with the commit will not.
Quoted 10 lines
Quoted 7 lines
Fair point.
However, the situation is a bit nuanced: FRED hardware is available now, and users running current stable kernels as guests will encounter boot failures when the hypervisor is updated to support FRED. While not a traditional regression, it creates a compatibility gap where stable guest kernels cannot run on updated hypervisors.
Great, then upgrade those guest kernels as they have never been able to run on those hypervisors :)
As above, *upgrading* from e.g. 6.6 to 6.12 will suddenly fail to boot.
Quoted 3 lines
Quoted 1 line
Other option would be to disable FRED for SEV-ES and SNP guest in stable kernel.
That's a choice for the hypervisor vendors to choose.
No, because the hypervisor has no clue what kernel version the guest is running.
Quoted 5 lines · Sean Christopherson
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
That sounds like new hardware support, if you really want that, why not just use newer kernel versions with this fix in it? Obviously no one is running those kernels on that hardware today, so this isn't a regression 🙂
I disagree, this absolutely is a regression. Kernels without commit
14619d912b65will boot on this "new" hardware, kernels with the commit will not.
Yeah, it is a regression for sure. It's a weird one, but it is a regression.
We need to either disable FRED on SEV-ES/SNP+FRED systems or fix it. We obviously want to fix it in mainline. I guess stable could do something different here and disable FRED instead if it wanted. That would avoid even a whiff of appearing to add new hardware support.
I'd personally prefer to just keep stable and mainline as close as possible.
P.S. #VC and #VE are a scourge
Greg KH
maintainerQuoted 44 lines · Sean Christopherson
Quoted 43 lines · Greg KH
Quoted 42 lines · Nikunj A. Dadhania
Quoted 39 lines · Greg KH
Quoted 37 lines · Nikunj A Dadhania
FRED enabled SEV-ES and SNP guests fail to boot due to the following issues in the early boot sequence:
* FRED does not have a #VC exception handler in the dispatch logic
* For secondary CPUs, FRED is enabled before setting up the FRED MSRs, and console output triggers a #VC which cannot be handled
* Early FRED #VC exceptions should use
boot_ghcbuntil per-CPU GHCBs are initializedFix these issues to ensure SEV-ES/SNP guests can handle #VC exceptions correctly during early boot when FRED is enabled.
Fixes 14619d912b65 ("x86/fred: FRED entry/exit and dispatch code")Cc [email protected] # 6.9+Signed-off-by Nikunj A Dadhania <[email protected]>---
Reason to add stable tag:
With FRED support for SVM here
https://lore.kernel.org/kvm/[email protected],
SVM and SEV guests running 6.9 and later kernels will support FRED. However, *SEV-ES and SNP guests cannot support FRED* and will fail to boot with the following error:[ 0.005144] Using GB pages for direct mapping [ 0.008402] Initialize FRED on CPU0 qemu-system-x86_64: cpus are not resettable, terminatingThree problems were identified as detailed in the commit message above and is fixed with this patch.
I would like the patch to be backported to the LTS kernels (6.12 and 6.18) to ensure SEV-ES and SNP guests running these stable kernel versions can boot with FRED enabled on FRED-enabled hypervisors.
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
That sounds like new hardware support, if you really want that, why not just use newer kernel versions with this fix in it? Obviously no one is running those kernels on that hardware today, so this isn't a regression :)
I disagree, this absolutely is a regression. Kernels without commit
14619d912b65will boot on this "new" hardware, kernels with the commit will not.
That commit added the new FRED feature, which "broke" when it hits real hardware. Not really a "regression" in my opinion as obviously it never worked at all :)
Anyway, I'll let you x86 maintainers here hash that out, just my thoughts...
thanks,
greg k-h
Greg KH
maintainerQuoted 5 lines · Nikunj A Dadhania
arch/x86/kernel/fred.c +1
-0@@ -70,6 +67,17 @@ void cpu_init_fred_exceptions(void)/* Use int $0x80 for 32-bit system calls in FRED mode */ setup_clear_cpu_cap (X86_FEATURE_SYSFAST32 );setup_clear_cpu_cap (X86_FEATURE_SYSCALL32 ); +
arch/x86/kernel/fred.c void cpu_init_fred_exceptions(void) · line 80
Quoted 10 lines
arch/x86/kernel/fred.c +10
-0+/* +* For secondary processors, FRED bit in CR4 gets enabled in cr4_init() +* and FRED MSRs are not configured till the end of this function. For +* SEV-ES and SNP guests, any console write before the FRED MSRs are +* setup will cause a #VC and cannot be handled. Move the pr_info to +* the end of this function. +* +* When FRED is enabled by default, remove this log message +*/ +pr_info ("Initialized FRED on CPU%d\n" ,smp_processor_id ());
Did you forget to fix this up?
Also, when the kernel is working properly, it is quiet, so why is this log message needed?
thanks,
greg k-h
Quoted 10 lines · Greg KH
Quoted 9 lines · Nikunj A Dadhania
arch/x86/kernel/fred.c +5
-0@@ -70,6 +67,17 @@ void cpu_init_fred_exceptions(void)/* Use int $0x80 for 32-bit system calls in FRED mode */ setup_clear_cpu_cap (X86_FEATURE_SYSFAST32 );setup_clear_cpu_cap (X86_FEATURE_SYSCALL32 ); + +/* +* For secondary processors, FRED bit in CR4 gets enabled in cr4_init() +* and FRED MSRs are not configured till the end of this function. For +* SEV-ES and SNP guests, any console write before the FRED MSRs are
Quoted 8 lines
arch/x86/kernel/fred.c void cpu_init_fred_exceptions(void) · line 80
Quoted 6 lines
arch/x86/kernel/fred.c +6
-0+* setup will cause a #VC and cannot be handled. Move the pr_info to +* the end of this function. +* +* When FRED is enabled by default, remove this log message +*/ +pr_info ("Initialized FRED on CPU%d\n" ,smp_processor_id ());Did you forget to fix this up?
I didn't forget, I have moved the message to the end of cpu_init_fred_exceptions() because the original placement triggered #VC exceptions on SEV-ES/SNP guests before FRED MSRs were configured, causing boot failures.
Quoted 6 lines
Also, when the kernel is working properly, it is quiet, so why is this log message needed?
thanks,
greg k-h
Xin Li
maintainerQuoted 8 lines · Nikunj A Dadhania
FRED enabled SEV-ES and SNP guests fail to boot due to the following issues in the early boot sequence:
* FRED does not have a #VC exception handler in the dispatch logic
This should be a separate patch.
Quoted 3 lines
* 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)?
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.
arch/x86/kernel/smpboot.c +34 -7
@@ -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 ());
Quoted 9 lines · Xin Li
Quoted 5 lines · Nikunj A Dadhania
* 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
Quoted 99 lines
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()andcpu_init_exception_handling(), sincecr4_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 beforestart_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_diris moved ahead ofcr4_init(), which appears to be harmless.Signed-off-by Xin Li (Intel) <[email protected]>arch/x86/kernel/smpboot.c +34
-7@@ -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 ());
Xin Li
maintainerQuoted 12 lines · Nikunj A. Dadhania
Quoted 3 lines
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 myarch/x86/kernel/fred.cmodification (movingpr_info()to avoid the #VC). SEV-ES / SEV-SNP guests boot successfully with FRED enabled.Are you planning to post this for inclusion?
Yes, I will.
kernel test robot
build botkernel test robot noticed the following build warnings:
Hi Nikunj,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 3c2ca964f75460093a8aad6b314a6cd558e80e66]
url: https://github.com/intel-lab-lkp/linux/commits/Nikunj-A-Dadhania/x86-fred-Fix-early-boot-failures-on-SEV-ES-SNP-guests/20260205-131359 base: 3c2ca964f75460093a8aad6b314a6cd558e80e66 patch link: https://lore.kernel.org/r/20260205051030.1225975-1-nikunj%40amd.com patch subject: [PATCH] x86/fred: Fix early boot failures on SEV-ES/SNP guests config: x86_64-kexec (https://download.01.org/0day-ci/archive/20260205/[email protected]/config) compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260205/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
arch/x86/entry/entry_fred.c:213:11: error: call to undeclared function 'user_exc_vmm_communication'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
213 | return user_exc_vmm_communication(regs, error_code);
| ^
>> arch/x86/entry/entry_fred.c:213:4: warning: void function 'fred_hwexc' should not return a value [-Wreturn-mismatch]
213 | return user_exc_vmm_communication(regs, error_code);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/x86/entry/entry_fred.c:215:11: error: call to undeclared function 'kernel_exc_vmm_communication'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
215 | return kernel_exc_vmm_communication(regs, error_code);
| ^
arch/x86/entry/entry_fred.c:215:4: warning: void function 'fred_hwexc' should not return a value [-Wreturn-mismatch]
215 | return kernel_exc_vmm_communication(regs, error_code);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~2 warnings and 2 errors generated.
vim +/fred_hwexc +213 arch/x86/entry/entry_fred.c
180
181 static noinstr void fred_hwexc(struct pt_regs *regs, unsigned long error_code)
182 {
183 /* Optimize for #PF. That's the only exception which matters performance wise */
184 if (likely(regs->fred_ss.vector == X86_TRAP_PF))
185 return exc_page_fault(regs, error_code);
186
187 switch (regs->fred_ss.vector) {
188 case X86_TRAP_DE: return exc_divide_error(regs);
189 case X86_TRAP_DB: return fred_exc_debug(regs);
190 case X86_TRAP_BR: return exc_bounds(regs);
191 case X86_TRAP_UD: return exc_invalid_op(regs);
192 case X86_TRAP_NM: return exc_device_not_available(regs);
193 case X86_TRAP_DF: return exc_double_fault(regs, error_code);
194 case X86_TRAP_TS: return exc_invalid_tss(regs, error_code);
195 case X86_TRAP_NP: return exc_segment_not_present(regs, error_code);
196 case X86_TRAP_SS: return exc_stack_segment(regs, error_code);
197 case X86_TRAP_GP: return exc_general_protection(regs, error_code);
198 case X86_TRAP_MF: return exc_coprocessor_error(regs);
199 case X86_TRAP_AC: return exc_alignment_check(regs, error_code);
200 case X86_TRAP_XF: return exc_simd_coprocessor_error(regs);
201
202 #ifdef CONFIG_X86_MCE
203 case X86_TRAP_MC: return fred_exc_machine_check(regs);
204 #endif
205 #ifdef CONFIG_INTEL_TDX_GUEST
206 case X86_TRAP_VE: return exc_virtualization_exception(regs);
207 #endif
208 #ifdef CONFIG_X86_CET
209 case X86_TRAP_CP: return exc_control_protection(regs, error_code);
210 #endif
211 case X86_TRAP_VC:
212 if (user_mode(regs))
> 213 return user_exc_vmm_communication(regs, error_code);
214 else
215 return kernel_exc_vmm_communication(regs, error_code);
216 default: return fred_bad_type(regs, error_code);
217 }
218
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Quoted 33 lines · kernel test robot
Hi Nikunj,
kernel test robot noticed the following build warnings:
[auto build test WARNING on
3c2ca964f75460093a8aad6b314a6cd558e80e66]url: https://github.com/intel-lab-lkp/linux/commits/Nikunj-A-Dadhania/x86-fred-Fix-early-boot-failures-on-SEV-ES-SNP-guests/20260205-131359 base: 3c2ca964f75460093a8aad6b314a6cd558e80e66 patch link: https://lore.kernel.org/r/20260205051030.1225975-1-nikunj%40amd.com patch subject: [PATCH] x86/fred: Fix early boot failures on SEV-ES/SNP guests config: x86_64-kexec (https://download.01.org/0day-ci/archive/20260205/[email protected]/config) compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260205/[email protected]/reproduce)If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/All warnings (new ones prefixed by >>):
arch/x86/entry/entry_fred.c:213:11: error: call to undeclared function 'user_exc_vmm_communication'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]213 | return user_exc_vmm_communication(regs, error_code); | ^Quoted 1 line
Quoted 1 line
arch/x86/entry/entry_fred.c:213:4: warning: void function 'fred_hwexc' should not return a value [-Wreturn-mismatch]213 | return user_exc_vmm_communication(regs, error_code); | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/x86/entry/entry_fred.c:215:11: error: call to undeclared function 'kernel_exc_vmm_communication'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] 215 | return kernel_exc_vmm_communication(regs, error_code); | ^ arch/x86/entry/entry_fred.c:215:4: warning: void function 'fred_hwexc' should not return a value [-Wreturn-mismatch] 215 | return kernel_exc_vmm_communication(regs, error_code); | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Quoted 3 lines
2 warnings and 2 errors generated.
Thanks for the report, below patch should solve the problem:
arch/x86/entry/entry_fred.c +4 -0
@@ -208,11 +208,15 @@ static noinstr void fred_hwexc(struct pt_regs *regs, unsigned long error_code)#ifdef CONFIG_X86_CET case X86_TRAP_CP :return exc_control_protection (regs, error_code);#endif + +#ifdef CONFIG_AMD_MEM_ENCRYPT case X86_TRAP_VC :if (user_mode (regs))return user_exc_vmm_communication (regs, error_code);else return kernel_exc_vmm_communication (regs, error_code); +#endif +default :return fred_bad_type (regs, error_code); }
Regards
Nikunj
Xin Li
maintainerQuoted 8 lines · Nikunj A. Dadhania
if (
user_mode(regs))
returnuser_exc_vmm_communication(regs, error_code); else
returnkernel_exc_vmm_communication(regs, error_code);
Please rewrite this piece of code, like how X86_TRAP_DB is handled today.
Should kernel #VC be handled at a higher level RSP? You can check FRED #DB settings.
kernel test robot
build botkernel test robot noticed the following build errors:
Hi Nikunj,
kernel test robot noticed the following build errors:
[auto build test ERROR on 3c2ca964f75460093a8aad6b314a6cd558e80e66]
url: https://github.com/intel-lab-lkp/linux/commits/Nikunj-A-Dadhania/x86-fred-Fix-early-boot-failures-on-SEV-ES-SNP-guests/20260205-131359 base: 3c2ca964f75460093a8aad6b314a6cd558e80e66 patch link: https://lore.kernel.org/r/20260205051030.1225975-1-nikunj%40amd.com patch subject: [PATCH] x86/fred: Fix early boot failures on SEV-ES/SNP guests config: x86_64-randconfig-161-20260205 (https://download.01.org/0day-ci/archive/20260205/[email protected]/config) compiler: gcc-14 (Debian 14.2.0-19) 14.2.0 smatch version: v0.5.0-8994-gd50c5a4c reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260205/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
arch/x86/entry/entry_fred.c: In function 'fred_hwexc':
>> arch/x86/entry/entry_fred.c:213:32: error: implicit declaration of function 'user_exc_vmm_communication' [-Wimplicit-function-declaration]
213 | return user_exc_vmm_communication(regs, error_code);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
>> arch/x86/entry/entry_fred.c:213:32: error: 'return' with a value, in function returning void [-Wreturn-mismatch]
213 | return user_exc_vmm_communication(regs, error_code);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/x86/entry/entry_fred.c:181:21: note: declared here
181 | static noinstr void fred_hwexc(struct pt_regs *regs, unsigned long error_code)
| ^~~~~~~~~~
>> arch/x86/entry/entry_fred.c:215:32: error: implicit declaration of function 'kernel_exc_vmm_communication' [-Wimplicit-function-declaration]
215 | return kernel_exc_vmm_communication(regs, error_code);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/x86/entry/entry_fred.c:215:32: error: 'return' with a value, in function returning void [-Wreturn-mismatch]
215 | return kernel_exc_vmm_communication(regs, error_code);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/x86/entry/entry_fred.c:181:21: note: declared here
181 | static noinstr void fred_hwexc(struct pt_regs *regs, unsigned long error_code)
| ^~~~~~~~~~
vim +/user_exc_vmm_communication +213 arch/x86/entry/entry_fred.c
180
181 static noinstr void fred_hwexc(struct pt_regs *regs, unsigned long error_code)
182 {
183 /* Optimize for #PF. That's the only exception which matters performance wise */
184 if (likely(regs->fred_ss.vector == X86_TRAP_PF))
185 return exc_page_fault(regs, error_code);
186
187 switch (regs->fred_ss.vector) {
188 case X86_TRAP_DE: return exc_divide_error(regs);
189 case X86_TRAP_DB: return fred_exc_debug(regs);
190 case X86_TRAP_BR: return exc_bounds(regs);
191 case X86_TRAP_UD: return exc_invalid_op(regs);
192 case X86_TRAP_NM: return exc_device_not_available(regs);
193 case X86_TRAP_DF: return exc_double_fault(regs, error_code);
194 case X86_TRAP_TS: return exc_invalid_tss(regs, error_code);
195 case X86_TRAP_NP: return exc_segment_not_present(regs, error_code);
196 case X86_TRAP_SS: return exc_stack_segment(regs, error_code);
197 case X86_TRAP_GP: return exc_general_protection(regs, error_code);
198 case X86_TRAP_MF: return exc_coprocessor_error(regs);
199 case X86_TRAP_AC: return exc_alignment_check(regs, error_code);
200 case X86_TRAP_XF: return exc_simd_coprocessor_error(regs);
201
202 #ifdef CONFIG_X86_MCE
203 case X86_TRAP_MC: return fred_exc_machine_check(regs);
204 #endif
205 #ifdef CONFIG_INTEL_TDX_GUEST
206 case X86_TRAP_VE: return exc_virtualization_exception(regs);
207 #endif
208 #ifdef CONFIG_X86_CET
209 case X86_TRAP_CP: return exc_control_protection(regs, error_code);
210 #endif
211 case X86_TRAP_VC:
212 if (user_mode(regs))
> 213 return user_exc_vmm_communication(regs, error_code); 214 else > 215 return kernel_exc_vmm_communication(regs, error_code);
216 default: return fred_bad_type(regs, error_code);
217 }
218
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
kernel test robot
build botkernel test robot noticed the following build errors:
Hi Nikunj,
kernel test robot noticed the following build errors:
[auto build test ERROR on 3c2ca964f75460093a8aad6b314a6cd558e80e66]
url: https://github.com/intel-lab-lkp/linux/commits/Nikunj-A-Dadhania/x86-fred-Fix-early-boot-failures-on-SEV-ES-SNP-guests/20260205-131359 base: 3c2ca964f75460093a8aad6b314a6cd558e80e66 patch link: https://lore.kernel.org/r/20260205051030.1225975-1-nikunj%40amd.com patch subject: [PATCH] x86/fred: Fix early boot failures on SEV-ES/SNP guests config: x86_64-kexec (https://download.01.org/0day-ci/archive/20260205/[email protected]/config) compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260205/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> arch/x86/entry/entry_fred.c:213:11: error: call to undeclared function 'user_exc_vmm_communication'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
213 | return user_exc_vmm_communication(regs, error_code);
| ^
arch/x86/entry/entry_fred.c:213:4: warning: void function 'fred_hwexc' should not return a value [-Wreturn-mismatch]
213 | return user_exc_vmm_communication(regs, error_code);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> arch/x86/entry/entry_fred.c:215:11: error: call to undeclared function 'kernel_exc_vmm_communication'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
215 | return kernel_exc_vmm_communication(regs, error_code);
| ^
arch/x86/entry/entry_fred.c:215:4: warning: void function 'fred_hwexc' should not return a value [-Wreturn-mismatch]
215 | return kernel_exc_vmm_communication(regs, error_code);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~2 warnings and 2 errors generated.
vim +/user_exc_vmm_communication +213 arch/x86/entry/entry_fred.c
180
181 static noinstr void fred_hwexc(struct pt_regs *regs, unsigned long error_code)
182 {
183 /* Optimize for #PF. That's the only exception which matters performance wise */
184 if (likely(regs->fred_ss.vector == X86_TRAP_PF))
185 return exc_page_fault(regs, error_code);
186
187 switch (regs->fred_ss.vector) {
188 case X86_TRAP_DE: return exc_divide_error(regs);
189 case X86_TRAP_DB: return fred_exc_debug(regs);
190 case X86_TRAP_BR: return exc_bounds(regs);
191 case X86_TRAP_UD: return exc_invalid_op(regs);
192 case X86_TRAP_NM: return exc_device_not_available(regs);
193 case X86_TRAP_DF: return exc_double_fault(regs, error_code);
194 case X86_TRAP_TS: return exc_invalid_tss(regs, error_code);
195 case X86_TRAP_NP: return exc_segment_not_present(regs, error_code);
196 case X86_TRAP_SS: return exc_stack_segment(regs, error_code);
197 case X86_TRAP_GP: return exc_general_protection(regs, error_code);
198 case X86_TRAP_MF: return exc_coprocessor_error(regs);
199 case X86_TRAP_AC: return exc_alignment_check(regs, error_code);
200 case X86_TRAP_XF: return exc_simd_coprocessor_error(regs);
201
202 #ifdef CONFIG_X86_MCE
203 case X86_TRAP_MC: return fred_exc_machine_check(regs);
204 #endif
205 #ifdef CONFIG_INTEL_TDX_GUEST
206 case X86_TRAP_VE: return exc_virtualization_exception(regs);
207 #endif
208 #ifdef CONFIG_X86_CET
209 case X86_TRAP_CP: return exc_control_protection(regs, error_code);
210 #endif
211 case X86_TRAP_VC:
212 if (user_mode(regs))
> 213 return user_exc_vmm_communication(regs, error_code); 214 else > 215 return kernel_exc_vmm_communication(regs, error_code);
216 default: return fred_bad_type(regs, error_code);
217 }
218
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
On 2/4/26 21:10, Nikunj A Dadhania wrote:
...
arch/x86/entry/entry_fred.c static noinstr void fred_hwexc(struct pt_regs *regs, unsigned long error_code) · line 217
Quoted 13 lines
arch/x86/entry/entry_fred.c +5
-0@@ -208,6 +208,11 @@ static noinstr void fred_hwexc(struct pt_regs *regs, unsigned long error_code)#ifdef CONFIG_X86_CET case X86_TRAP_CP :return exc_control_protection (regs, error_code);#endif +case X86_TRAP_VC : +if (user_mode (regs)) +return user_exc_vmm_communication (regs, error_code); +else +return kernel_exc_vmm_communication (regs, error_code);default :return fred_bad_type (regs, error_code); }
Please look at the code in the ~20 lines above this hunk. It has a nice, consistent form of:
case X86_TRAP_FOO: return exc_foo_action(...);
Could we keep that going, please?
Second, these functions are defined in arch/x86/coco/sev/vc-handle.c. That looks suspiciously like CONFIG_AMD_MEM_ENCRYPT code and not something that will compile everywhere. Also note the other features in the switch() block. See all the #ifdefs on those?
Have you compiled this?
Quoted 20 lines
arch/x86/kernel/fred.c +2
-3@@ -27,9 +27,6 @@ EXPORT_PER_CPU_SYMBOL(fred_rsp0);void cpu_init_fred_exceptions (void ) { -/* When FRED is enabled by default, remove this log message */ -pr_info ("Initialize FRED on CPU%d\n" ,smp_processor_id ()); -/* * If a kernel event is delivered before a CPU goes to user level for * the first time, its SS is NULL thus NULL is pushed into the SS field @@ -70,6 +67,17 @@ void cpu_init_fred_exceptions(void)/* Use int $0x80 for 32-bit system calls in FRED mode */ setup_clear_cpu_cap (X86_FEATURE_SYSFAST32 );setup_clear_cpu_cap (X86_FEATURE_SYSCALL32 ); + +/*
arch/x86/kernel/fred.c void cpu_init_fred_exceptions(void) · line 81
Quoted 10 lines
arch/x86/kernel/fred.c +9
-0+* For secondary processors, FRED bit in CR4 gets enabled in cr4_init() +* and FRED MSRs are not configured till the end of this function. For +* SEV-ES and SNP guests, any console write before the FRED MSRs are +* setup will cause a #VC and cannot be handled. Move the pr_info to +* the end of this function. +* +* When FRED is enabled by default, remove this log message +*/ +pr_info ("Initialized FRED on CPU%d\n" ,smp_processor_id ()); }
This seems really gross. Now there's a window where printk() doesn't work. To fix it, we start moving printk()'s?
Please, no.
Shouldn't we flip the FRED CR4 bit _last_, once all the MSRs are set up? Why is it backwards in the first place? Why can't it be fixed?
Quoted 2 lines · Dave Hansen
Shouldn't we flip the FRED CR4 bit _last_, once all the MSRs are set up? Why is it backwards in the first place? Why can't it be fixed?
Ahhh, it was done by CR4 pinning. It's the first thing in C code for booting secondaries:
static void notrace __noendbr start_secondary(void *unused)
{
cr4_init();Since FRED is set in 'cr4_pinned_mask', cr4_init() sets the FRED bit far before the FRED MSRs are ready. Anyone else doing native_write_cr4() will do the same thing. That's obviously not what was intended from the pinning code or the FRED init code.
Shouldn't we fix this properly rather than moving printk()'s around?
One idea is just to turn off all the CR-pinning logic while bringing CPUs up. That way, nothing before:
set_cpu_online(smp_processor_id(), true);
can get tripped up by CR pinning. I've attached a completely untested patch to do that.
The other thing would be to make pinning actually per-cpu:
'cr4_pinned_bits' could be per-cpu and we'd just keep it empty until the CPU is actually booted and everything is fully set up.
Either way, this is looking like it'll be a bit more than one patch to do properly.
---
b/arch/x86/kernel/cpu/common.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/common.c renamed from arch/x86/kernel/cpu/common.c~no-cr4-pinning-for-offline-cpus +18 -3
@@ -434,6 +434,21 @@ static const unsigned long cr4_pinned_mastatic DEFINE_STATIC_KEY_FALSE_RO (cr_pinning);static unsigned long cr4_pinned_bits__ro_after_init ; +static bool cr4_pinning_enabled (void ) +{ +if (!static_branch_likely (&cr_pinning)) +return false; + +/* +* Do not enforce pinning during CPU bringup. It might +* turn on features that are not set up yet, like FRED. +*/ +if (!cpu_online (smp_processor_id ())) +return false; + +return true; +} +void native_write_cr0 (unsigned long val) {unsigned long bits_missing =0 ;
@@ -441,7 +456,7 @@ void native_write_cr0(unsigned long val) set_register: asmvolatile ("mov %0,%%cr0" :"+r" (val) : :"memory" ); -if (static_branch_likely (&cr_pinning)) { +if (cr4_pinning_enabled ()) {if (unlikely ((val &X86_CR0_WP ) !=X86_CR0_WP )) { bits_missing =X86_CR0_WP ; val |= bits_missing;
@@ -460,7 +475,7 @@ void __no_profile native_write_cr4(unsig set_register: asmvolatile ("mov %0,%%cr4" :"+r" (val) : :"memory" ); -if (static_branch_likely (&cr_pinning)) { +if (cr4_pinning_enabled ()) {if (unlikely ((val & cr4_pinned_mask) != cr4_pinned_bits)) { bits_changed = (val & cr4_pinned_mask) ^ cr4_pinned_bits; val = (val & ~cr4_pinned_mask) | cr4_pinned_bits;
@@ -502,7 +517,7 @@ void cr4_init(void)if (boot_cpu_has (X86_FEATURE_PCID )) cr4 |=X86_CR4_PCIDE ; -if (static_branch_likely (&cr_pinning)) +if (cr4_pinning_enabled ()) cr4 = (cr4 & ~cr4_pinned_mask) | cr4_pinned_bits;__write_cr4 (cr4);
_
Quoted 11 lines · Dave Hansen
Quoted 2 lines · Dave Hansen
Shouldn't we flip the FRED CR4 bit _last_, once all the MSRs are set up? Why is it backwards in the first place? Why can't it be fixed?
Ahhh, it was done by CR4 pinning. It's the first thing in C code for booting secondaries:
static void notrace
__noendbrstart_secondary(void *unused){ cr4_init();
Quoted 6 lines
Since FRED is set in '
cr4_pinned_mask',cr4_init()sets the FRED bit far before the FRED MSRs are ready. Anyone else doingnative_write_cr4()will do the same thing. That's obviously not what was intended from the pinning code or the FRED init code.Shouldn't we fix this properly rather than moving printk()'s around?
I believe that is what this part of the thread decided on:
https://lore.kernel.org/kvm/[email protected]/T/#m3e44c2c53aca3bcd872de4ce1e50a14500e62e4e
Thanks,
Tom
Quoted 15 lines
One idea is just to turn off all the CR-pinning logic while bringing CPUs up. That way, nothing before:
set_cpu_online(smp_processor_id(), true);can get tripped up by CR pinning. I've attached a completely untested patch to do that.
The other thing would be to make pinning actually per-cpu:
'cr4_pinned_bits' could be per-cpu and we'd just keep it empty until the CPU is actually booted and everything is fully set up.Either way, this is looking like it'll be a bit more than one patch to do properly.
Quoted 17 lines · Tom Lendacky
Quoted 16 lines · Dave Hansen
Quoted 2 lines · Dave Hansen
Shouldn't we flip the FRED CR4 bit _last_, once all the MSRs are set up? Why is it backwards in the first place? Why can't it be fixed?
Ahhh, it was done by CR4 pinning. It's the first thing in C code for booting secondaries:
static void notrace
__noendbrstart_secondary(void *unused){ cr4_init();Since FRED is set in '
cr4_pinned_mask',cr4_init()sets the FRED bit far before the FRED MSRs are ready. Anyone else doingnative_write_cr4()will do the same thing. That's obviously not what was intended from the pinning code or the FRED init code.
Quoted 17 lines
Quoted 1 line
Shouldn't we fix this properly rather than moving printk()'s around?
I believe that is what this part of the thread decided on:
https://lore.kernel.org/kvm/[email protected]/T/#m3e44c2c53aca3bcd872de4ce1e50a14500e62e4e
Thanks,
TomQuoted 8 lines
One idea is just to turn off all the CR-pinning logic while bringing CPUs up. That way, nothing before:
set_cpu_online(smp_processor_id(), true);can get tripped up by CR pinning. I've attached a completely untested patch to do that.
Yes, this works as well. And Xin Li's patch also resolves the issue by moving the cr4_init() later after initializing FRED MSRs.
Regards,
Nikunj
Quoted 33 lines · Nikunj A. Dadhania
Quoted 30 lines · Tom Lendacky
Quoted 11 lines · Dave Hansen
Quoted 2 lines · Dave Hansen
Shouldn't we flip the FRED CR4 bit _last_, once all the MSRs are set up? Why is it backwards in the first place? Why can't it be fixed?
Ahhh, it was done by CR4 pinning. It's the first thing in C code for booting secondaries:
static void notrace
__noendbrstart_secondary(void *unused){ cr4_init();Quoted 6 lines
Since FRED is set in '
cr4_pinned_mask',cr4_init()sets the FRED bit far before the FRED MSRs are ready. Anyone else doingnative_write_cr4()will do the same thing. That's obviously not what was intended from the pinning code or the FRED init code.Shouldn't we fix this properly rather than moving printk()'s around?
I believe that is what this part of the thread decided on:
https://lore.kernel.org/kvm/[email protected]/T/#m3e44c2c53aca3bcd872de4ce1e50a14500e62e4e
Thanks,
TomQuoted 4 lines
One idea is just to turn off all the CR-pinning logic while bringing CPUs up. That way, nothing before:
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
set_cpu_online(smp_processor_id(), true);can get tripped up by CR pinning. I've attached a completely untested patch to do that.
Yes, this works as well. And Xin Li's patch also resolves the issue by moving the
cr4_init()later after initializing FRED MSRs.
Hi Dave,
This is what I have in my patch queue, Can I include your authorship and SOB?
Only change I made in your patch is s/cr4_pinning/cr_pinning.
From: Dave Hansen <[email protected]>
Subject: [PATCH] x86/cpu: Disable CR pinning during CPU bringup
CR pinning can prematurely enable features during secondary CPU bringup before their supporting infrastructure is initialized. Specifically, when FRED is enabled, cr4_init() sets CR4.FRED via the pinned mask early in start_secondary(), long before cpu_init_fred_exceptions() configures the required FRED MSRs. This creates a window where exceptions cannot be properly handled.
For SEV-ES/SNP and TDX guests, any console output during this window triggers #VC or #VE exceptions that result in triple faults because the exception handlers rely on FRED MSRs that aren't yet configured.
Disable CR pinning for offline CPUs by checking cpu_online() in the pinning enforcement path. This allows features like FRED to be enabled in CR4 only after their supporting MSRs and handlers are fully configured.
--- arch/x86/kernel/cpu/common.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/common.c +18 -3
@@ -434,6 +434,21 @@ static const unsigned long cr4_pinned_mask = X86_CR4_SMEP | X86_CR4_SMAP | X86_Cstatic DEFINE_STATIC_KEY_FALSE_RO (cr_pinning);static unsigned long cr4_pinned_bits__ro_after_init ; +static bool cr_pinning_enabled (void ) +{ +if (!static_branch_likely (&cr_pinning)) +return false; + +/* +* Do not enforce pinning during CPU bringup. It might +* turn on features that are not set up yet, like FRED. +*/ +if (!cpu_online (smp_processor_id ())) +return false; + +return true; +} +void native_write_cr0 (unsigned long val) {unsigned long bits_missing =0 ;
@@ -441,7 +456,7 @@ void native_write_cr0(unsigned long val) set_register: asmvolatile ("mov %0,%%cr0" :"+r" (val) : :"memory" ); -if (static_branch_likely (&cr_pinning)) { +if (cr_pinning_enabled ()) {if (unlikely ((val &X86_CR0_WP ) !=X86_CR0_WP )) { bits_missing =X86_CR0_WP ; val |= bits_missing;
@@ -460,7 +475,7 @@ void __no_profile native_write_cr4(unsigned long val) set_register: asmvolatile ("mov %0,%%cr4" :"+r" (val) : :"memory" ); -if (static_branch_likely (&cr_pinning)) { +if (cr_pinning_enabled ()) {if (unlikely ((val & cr4_pinned_mask) != cr4_pinned_bits)) { bits_changed = (val & cr4_pinned_mask) ^ cr4_pinned_bits; val = (val & ~cr4_pinned_mask) | cr4_pinned_bits;
@@ -502,7 +517,7 @@ void cr4_init(void)if (boot_cpu_has (X86_FEATURE_PCID )) cr4 |=X86_CR4_PCIDE ; -if (static_branch_likely (&cr_pinning)) +if (cr_pinning_enabled ()) cr4 = (cr4 & ~cr4_pinned_mask) | cr4_pinned_bits;__write_cr4 (cr4);
Signature · 1 line
2.48.1
Quoted 6 lines
CR pinning can prematurely enable features during secondary CPU bringup before their supporting infrastructure is initialized. Specifically, when FRED is enabled,
cr4_init()sets CR4.FRED via the pinned mask early instart_secondary(), long beforecpu_init_fred_exceptions()configures the required FRED MSRs. This creates a window where exceptions cannot be properly handled.
This is a collision of FRED, CR-pinning and SEV. Future me would appreciate having all that background in one place:
== CR Pinning Background ==
Modern CPU hardening features like SMAP/SMEP are enabled by flipping control register (CR) bits. Attackers find these features inconvenient and often try to disable them.
CR-pinning is a kernel hardening feature that detects when security-sensitive control bits are flipped off, complains about it, then turns them back on. The CR-pinning checks are performed in the CR manipulation helpers.
X86_CR4_FRED controls FRED enabling and is pinned. There is a single, system-wide static key that controls CR-pinning behavior. The static key is enabled by the boot CPU after it has established its CR configuration.
The end result is that CR-pinning is not active while initializing the boot CPU but it is active while bringing up secondary CPUs.
== FRED Background ==
FRED is a new hardware entry/exit feature for the kernel. It is not on by default and started out as Intel-only. AMD is just adding support now.
FRED has MSRs for configuration and is enabled by the pinned X86_CR4_FRED bit. It should not be enabled until after MSRs are properly initialized.
== SEV Background ==
Some flavors of AMD SEV have special virtualization exceptions:
#VC. These exceptions happen in "weird" places like when accessing MMIO, running CPUID or even accessing apparently normal kernel memory.
Writes to the console can generate #VC.
== Problem ==
CR-pinning implicitly enables FRED on secondary CPUs at a different point than the boot CPU. This point is *before* the CPU has done an explicit cr4_set_bits(X86_CR4_FRED) and before the MSRs are initialized. This means that there is a window where no exceptions can be handled.
For SEV-ES/SNP and TDX guests, any console output during this window triggers #VC or #VE exceptions that result in triple faults because the exception handlers rely on FRED MSRs that aren't yet configured.
== Fix ==
Defer CR-pinning enforcement during secondary CPU bringup. This avoids any implicit CR changes during CPU bringup, ensuring that FRED is not enabled before it is configured and able to handle a
#VC.
This also aligns boot and secondary CPU bringup.
Note: FRED is not on by default anywhere so this is not likely to be causing many problems. The only reason this was noticed was that AMD started to enable FRED and was turning it on.
With that, you can add:
arch/x86/kernel/cpu/common.c void cr4_init(void) · line 522
Quoted 8 lines · Nikunj A. Dadhania
arch/x86/kernel/cpu/common.c +1
-1@@ -502,7 +517,7 @@ void cr4_init(void)if (boot_cpu_has (X86_FEATURE_PCID )) cr4 |=X86_CR4_PCIDE ; -if (static_branch_likely (&cr_pinning)) +if (cr_pinning_enabled ()) cr4 = (cr4 & ~cr4_pinned_mask) | cr4_pinned_bits;
Maybe I am missing something, but is there a reason to keep this check anymore?
AFAIU, cr_pinning_enabled() will always be false during cr4_init(). cr4_init() always happens during early bringup when the cpu is marked offline.
Quoted 1 line
arch/x86/kernel/cpu/common.c +0
-0__write_cr4 (cr4);
Quoted 12 lines · Sohil Mehta
arch/x86/kernel/cpu/common.c void cr4_init(void) · line 522
Quoted 8 lines · Nikunj A. Dadhania
arch/x86/kernel/cpu/common.c +1
-1@@ -502,7 +517,7 @@ void cr4_init(void)if (boot_cpu_has (X86_FEATURE_PCID )) cr4 |=X86_CR4_PCIDE ; -if (static_branch_likely (&cr_pinning)) +if (cr_pinning_enabled ()) cr4 = (cr4 & ~cr4_pinned_mask) | cr4_pinned_bits;Maybe I am missing something, but is there a reason to keep this check anymore?
Quoted 3 lines
AFAIU,
cr_pinning_enabled()will always be false duringcr4_init().cr4_init()always happens during early bringup when the cpu is marked offline.
Yeah, that should be dead code now.
It wouldn't hurt to say:
WARN_ON_ONCE(cr_pinning_enabled());
to kinda document that this code doesn't work with CR pinning.
Sohil Mehta
Reviewed-byQuoted 13 lines · Dave Hansen
Quoted 12 lines · Sohil Mehta
arch/x86/kernel/cpu/common.c void cr4_init(void) · line 522
Quoted 8 lines · Nikunj A. Dadhania
arch/x86/kernel/cpu/common.c +1
-1@@ -502,7 +517,7 @@ void cr4_init(void)if (boot_cpu_has (X86_FEATURE_PCID )) cr4 |=X86_CR4_PCIDE ; -if (static_branch_likely (&cr_pinning)) +if (cr_pinning_enabled ()) cr4 = (cr4 & ~cr4_pinned_mask) | cr4_pinned_bits;Maybe I am missing something, but is there a reason to keep this check anymore?
Quoted 5 lines
Quoted 3 lines
AFAIU,
cr_pinning_enabled()will always be false duringcr4_init().cr4_init()always happens during early bringup when the cpu is marked offline.Yeah, that should be dead code now.
With that change,
Quoted 5 lines
It wouldn't hurt to say:
WARN_ON_ONCE(cr_pinning_enabled());to kinda document that this code doesn't work with CR pinning.
H. Peter Anvin
maintainerQuoted 26 lines · Dave Hansen
Quoted 2 lines · Dave Hansen
Shouldn't we flip the FRED CR4 bit _last_, once all the MSRs are set up? Why is it backwards in the first place? Why can't it be fixed?
Ahhh, it was done by CR4 pinning. It's the first thing in C code for booting secondaries:
static void notrace
__noendbrstart_secondary(void *unused){ cr4_init();Since FRED is set in '
cr4_pinned_mask',cr4_init()sets the FRED bit far before the FRED MSRs are ready. Anyone else doingnative_write_cr4()will do the same thing. That's obviously not what was intended from the pinning code or the FRED init code.Shouldn't we fix this properly rather than moving printk()'s around?
One idea is just to turn off all the CR-pinning logic while bringing CPUs up. That way, nothing before:
set_cpu_online(smp_processor_id(), true);can get tripped up by CR pinning. I've attached a completely untested patch to do that.
Quoted 6 lines
The other thing would be to make pinning actually per-cpu:
'cr4_pinned_bits' could be per-cpu and we'd just keep it empty until the CPU is actually booted and everything is fully set up.Either way, this is looking like it'll be a bit more than one patch to do properly.
We could initialize the FRED MSRs much earlier, like we do during S3 resume.
Quoted 19 lines · Dave Hansen
On 2/4/26 21:10, Nikunj A Dadhania wrote:
...arch/x86/entry/entry_fred.c static noinstr void fred_hwexc(struct pt_regs *regs, unsigned long error_code) · line 217
Quoted 13 lines
arch/x86/entry/entry_fred.c +5
-0@@ -208,6 +208,11 @@ static noinstr void fred_hwexc(struct pt_regs *regs, unsigned long error_code)#ifdef CONFIG_X86_CET case X86_TRAP_CP :return exc_control_protection (regs, error_code);#endif +case X86_TRAP_VC : +if (user_mode (regs)) +return user_exc_vmm_communication (regs, error_code); +else +return kernel_exc_vmm_communication (regs, error_code);default :return fred_bad_type (regs, error_code); }Please look at the code in the ~20 lines above this hunk. It has a nice, consistent form of:
Quoted 3 lines
case X86_TRAP_FOO: return exc_foo_action(...);Could we keep that going, please?
There are couple of options, I will test and get back.
Quoted 6 lines
Second, these functions are defined in
arch/x86/coco/sev/vc-handle.c. That looks suspiciously likeCONFIG_AMD_MEM_ENCRYPTcode and not something that will compile everywhere. Also note the other features in theswitch()block. See all the #ifdefs on those?Have you compiled this?
Compiled and tested. I missed testing without CONFIG_AMD_MEM_ENCRYPT, will add.
Regards,
Nikunj