Re: [PATCH v15 09/41] KVM: x86: Load guest FPU state when access XSAVE-managed MSRs

[Binbin Wu]

On 9/16/2025 4:28 PM, Binbin Wu wrote:
> On 9/13/2025 7:22 AM, Sean Christopherson wrote:
> > Load the guest's FPU state if userspace is accessing MSRs whose values
> > are managed by XSAVES. Introduce two helpers, kvm_{get,set}_xstate_msr(),
> > to facilitate access to such kind of MSRs.
> >
> > If MSRs supported in kvm_caps.supported_xss are passed through to guest,
> > the guest MSRs are swapped with host's before vCPU exits to userspace and
> > after it reenters kernel before next VM-entry.
> >
> > Because the modified code is also used for the KVM_GET_MSRS device ioctl(),
> > explicitly check @vcpu is non-null before attempting to load guest state.
> > The XSAVE-managed MSRs cannot be retrieved via the device ioctl() without
> > loading guest FPU state (which doesn't exist).
> >
> > Note that guest_cpuid_has() is not queried as host userspace is allowed to
> > access MSRs that have not been exposed to the guest, e.g. it might do
> > KVM_SET_MSRS prior to KVM_SET_CPUID2.
> >
> > The two helpers are put here in order to manifest accessing xsave-managed
> > MSRs requires special check and handling to guarantee the correctness of
> > read/write to the MSRs.
> >
> > Co-developed-by: Yang Weijiang <weijiang.yang@xxxxxxxxx>
> > Signed-off-by: Yang Weijiang <weijiang.yang@xxxxxxxxx>
> > Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
> > Tested-by: Mathias Krause <minipli@xxxxxxxxxxxxxx>
> > Tested-by: John Allen <john.allen@xxxxxxx>
> > Tested-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
> > Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> > [sean: drop S_CET, add big comment, move accessors to x86.c]
> > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
>
> Reviewed-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
>
> Two nits below.
>
> > ---
> >   arch/x86/kvm/x86.c | 86 +++++++++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 85 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index c5e38d6943fe..a95ca2fbd3a9 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -136,6 +136,9 @@ static int __set_sregs2(struct kvm_vcpu *vcpu, struct kvm_sregs2 *sregs2);
> >   static void __get_sregs2(struct kvm_vcpu *vcpu, struct kvm_sregs2 *sregs2);
> >     static DEFINE_MUTEX(vendor_module_lock);
> > +static void kvm_load_guest_fpu(struct kvm_vcpu *vcpu);
> > +static void kvm_put_guest_fpu(struct kvm_vcpu *vcpu);
> > +
> >   struct kvm_x86_ops kvm_x86_ops __read_mostly;
> >     #define KVM_X86_OP(func)                         \
> > @@ -3801,6 +3804,66 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
> >       mark_page_dirty_in_slot(vcpu->kvm, ghc->memslot, gpa_to_gfn(ghc->gpa));
> >   }
> >   +/*
> > + * Returns true if the MSR in question is managed via XSTATE, i.e. is context
> > + * switched with the rest of guest FPU state.  Note!  S_CET is _not_ context
> > + * switched via XSTATE even though it _is_ saved/restored via XSAVES/XRSTORS.
> > + * Because S_CET is loaded on VM-Enter and VM-Exit via dedicated VMCS fields,
> > + * the value saved/restored via XSTATE is always the host's value.  That detail
> > + * is _extremely_ important, as the guest's S_CET must _never_ be resident in
> > + * hardware while executing in the host.  Loading guest values for U_CET and
> > + * PL[0-3]_SSP while executing in the kernel is safe, as U_CET is specific to
> > + * userspace, and PL[0-3]_SSP are only consumed when transitioning to lower
> > + * privilegel levels, i.e. are effectively only consumed by userspace as well.
> > + */

privilegel -> privilege