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

[Dave Hansen <dave.hansen@intel.com>]

On 2/4/26 21:10, Nikunj A Dadhania wrote:
...
> --- a/arch/x86/entry/entry_fred.c
> +++ b/arch/x86/entry/entry_fred.c
> @@ -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?

> diff --git a/arch/x86/kernel/fred.c b/arch/x86/kernel/fred.c
> index e736b19e18de..8cf4da546a8e 100644
> --- a/arch/x86/kernel/fred.c
> +++ b/arch/x86/kernel/fred.c
> @@ -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());
>  }

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?