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

Binbin Wu
2025-09-17T02:51:53Z

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@intel.com>
>> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>> Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
>> Tested-by: Mathias Krause <minipli@grsecurity.net>
>> Tested-by: John Allen <john.allen@amd.com>
>> Tested-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> [sean: drop S_CET, add big comment, move accessors to x86.c]
>> Signed-off-by: Sean Christopherson <seanjc@google.com>
>
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
>
> 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