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

Sean Christopherson
2025-09-17T12:47:54Z

On Tue, Sep 16, 2025, Binbin Wu wrote:
> > +/*
> > + * Lock and/or reload guest FPU and access xstate MSRs. For accesses initiated
>
>
> Lock is unconditional and reload is conditional.
> "and/or" seems not accurate?

Agreed. This?

/*
* Lock andr (re)load guest FPU and access xstate MSRs. For accesses initiated
* by host, guest FPU is loaded in __msr_io(). For accesses initiated by guest,
* guest FPU should have been loaded already.
*/

>
> > + * by host, guest FPU is loaded in __msr_io(). For accesses initiated by guest,
> > + * guest FPU should have been loaded already.
> > + */
> > +static __always_inline void kvm_access_xstate_msr(struct kvm_vcpu *vcpu,
> > + struct msr_data *msr_info,
> > + int access)
> > +{
> > + BUILD_BUG_ON(access != MSR_TYPE_R && access != MSR_TYPE_W);
> > +
> > + KVM_BUG_ON(!is_xstate_managed_msr(vcpu, msr_info->index), vcpu->kvm);
> > + KVM_BUG_ON(!vcpu->arch.guest_fpu.fpstate->in_use, vcpu->kvm);
> > +
> > + kvm_fpu_get();
> > + if (access == MSR_TYPE_R)
> > + rdmsrq(msr_info->index, msr_info->data);
> > + else
> > + wrmsrq(msr_info->index, msr_info->data);
> > + kvm_fpu_put();
> > +}
> > +
> > +static __maybe_unused void kvm_set_xstate_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > +{
> > + kvm_access_xstate_msr(vcpu, msr_info, MSR_TYPE_W);
> > +}
> > +
> > +static __maybe_unused void kvm_get_xstate_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > +{
> > + kvm_access_xstate_msr(vcpu, msr_info, MSR_TYPE_R);
> > +}
> > +
> > int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > {
> > u32 msr = msr_info->index;
> > @@ -4551,11 +4614,25 @@ static int __msr_io(struct kvm_vcpu *vcpu, struct kvm_msrs *msrs,
> > int (*do_msr)(struct kvm_vcpu *vcpu,
> > unsigned index, u64 *data))
> > {
> > + bool fpu_loaded = false;
> > int i;
> > - for (i = 0; i < msrs->nmsrs; ++i)
> > + for (i = 0; i < msrs->nmsrs; ++i) {
> > + /*
> > + * If userspace is accessing one or more XSTATE-managed MSRs,
> > + * temporarily load the guest's FPU state so that the guest's
> > + * MSR value(s) is resident in hardware, i.e. so that KVM can
>
> Using "i.e." and "so that" together feels repetitive.[...]

/*
* If userspace is accessing one or more XSTATE-managed MSRs,
* temporarily load the guest's FPU state so that the guest's
* MSR value(s) is resident in hardware and thus can be accessed
* via RDMSR/WRMSR.
*/