Re: [PATCH v9 07/22] KVM: VMX: Initialize VMCS FRED fields

[Sean Christopherson]

On Wed, Jan 21, 2026, Binbin Wu wrote:
> On 10/27/2025 4:18 AM, Xin Li (Intel) wrote:
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index fcfa99160018..c8b5359123bf 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -1459,6 +1459,15 @@ void vmx_vcpu_load_vmcs(struct kvm_vcpu *vcpu, int cpu)
> >  				    (unsigned long)(cpu_entry_stack(cpu) + 1));
> >  		}
> >  
> > +		/* Per-CPU FRED MSRs */

Meh, this is pretty self-explanatory code.

> > +		if (kvm_cpu_cap_has(X86_FEATURE_FRED)) {
> > +#ifdef CONFIG_X86_64
> 
> Nit:
> 
> Is this needed?
> 
> FRED is initialized by X86_64_F(), if CONFIG_X86_64 is not enabled, this
> path is not reachable.
> There should be no compilation issue without #ifdef CONFIG_X86_64 / #endif.
> 
> There are several similar patterns in this patch, using  #ifdef CONFIG_X86_64 / 
> #endif or not seems not consistent. E.g. __vmx_vcpu_reset() and init_vmcs()
> doesn't check the config, but here does.
> 
> > +			vmcs_write64(HOST_IA32_FRED_RSP1, __this_cpu_ist_top_va(ESTACK_DB));
> > +			vmcs_write64(HOST_IA32_FRED_RSP2, __this_cpu_ist_top_va(ESTACK_NMI));
> > +			vmcs_write64(HOST_IA32_FRED_RSP3, __this_cpu_ist_top_va(ESTACK_DF));

IMO, this is flawed for other reasons.  KVM shouldn't be relying on kernel
implementation details with respect to what FRED stack handles what event.

The simplest approach would be to read the actual MSR.  _If_ using a per-CPU read
provides meaningful performance benefits over RDMSR (or RDMSR w/ immediate?  I
don't see an API for that...), then have the kernel provide a dedicated accessor.

Then the accessor can be a non-inlined functions, and this code can be e.g.:

	if (IS_ENABLED(CONFIG_X86_64) && kvm_cpu_cap_has(X86_FEATURE_FRED)) {
		vmcs_write64(HOST_IA32_FRED_RSP1, fred_rsp(MSR_IA32_FRED_RSP1));
		vmcs_write64(HOST_IA32_FRED_RSP2, fred_rsp(MSR_IA32_FRED_RSP2));
		vmcs_write64(HOST_IA32_FRED_RSP3, fred_rsp(MSR_IA32_FRED_RSP2));
	}

where fred_rsp() is _declared_ unconditionally, but implemented only for 64-bit.
That way the compiler will be happy, and the actual usage will be dropped before
linking via dead-code elimination.

Actually, we can probably do one better?

	if (cpu_feature_enabled(X86_FEATURE_FRED) && kvm_cpu_cap_has(X86_FEATURE_FRED)) {