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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3800f472-025b-4d44-ad4a-a5bb1f9841d2@linux.intel.com>
Date: Wed, 17 Sep 2025 10:51:48 +0800
From: Binbin Wu <binbin.wu@...ux.intel.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
 linux-kernel@...r.kernel.org, Tom Lendacky <thomas.lendacky@....com>,
 Mathias Krause <minipli@...ecurity.net>, John Allen <john.allen@....com>,
 Rick Edgecombe <rick.p.edgecombe@...el.com>, Chao Gao <chao.gao@...el.com>,
 Maxim Levitsky <mlevitsk@...hat.com>, Xiaoyao Li <xiaoyao.li@...el.com>,
 Zhang Yi Z <yi.z.zhang@...ux.intel.com>
Subject: Re: [PATCH v15 09/41] KVM: x86: Load guest FPU state when access
 XSAVE-managed MSRs



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