On Wed, Dec 24, 2025 at 01:12:46AM +0100, Paolo Bonzini wrote:
Until now, fpstate->xfd has acted as both the guest value and the value that the host used when executing XSAVES and XRSTORS. This is wrong: the data in the guest's FPU might not be initialized even if a bit is set in XFD and, when that happens, XRSTORing the guest FPU will fail with a #NM exception *on the host*.
Instead, store the value of XFD together with XFD_ERR in struct fpu_guest; it will still be synchronized in fpu_load_guest_fpstate(), but the XRSTOR(S) operation will be able to load any valid state of the FPU independent of the XFD value.
Hi Paolo,
LGTM.
Reviewed-by: Yuan Yao yaoyuan0329os@gmail.com
Cc: stable@vger.kernel.org Fixes: 820a6ee944e7 ("kvm: x86: Add emulation for IA32_XFD", 2022-01-14) Signed-off-by: Paolo Bonzini pbonzini@redhat.com
arch/x86/include/asm/fpu/api.h | 6 ++---- arch/x86/include/asm/fpu/types.h | 7 +++++++ arch/x86/kernel/fpu/core.c | 19 ++++--------------- arch/x86/kernel/fpu/xstate.h | 18 ++++++++++-------- arch/x86/kvm/x86.c | 6 +++--- 5 files changed, 26 insertions(+), 30 deletions(-)
diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h index 0820b2621416..ee9ba06b7dbe 100644 --- a/arch/x86/include/asm/fpu/api.h +++ b/arch/x86/include/asm/fpu/api.h @@ -152,11 +152,9 @@ extern int fpu_swap_kvm_fpstate(struct fpu_guest *gfpu, bool enter_guest); extern int fpu_enable_guest_xfd_features(struct fpu_guest *guest_fpu, u64 xfeatures);
#ifdef CONFIG_X86_64 -extern void fpu_update_guest_xfd(struct fpu_guest *guest_fpu, u64 xfd); -extern void fpu_sync_guest_vmexit_xfd_state(void); +extern void fpu_sync_guest_vmexit_xfd_state(struct fpu_guest *gfpu); #else -static inline void fpu_update_guest_xfd(struct fpu_guest *guest_fpu, u64 xfd) { } -static inline void fpu_sync_guest_vmexit_xfd_state(void) { } +static inline void fpu_sync_guest_vmexit_xfd_state(struct fpu_guest *gfpu) { } #endif
extern void fpu_copy_guest_fpstate_to_uabi(struct fpu_guest *gfpu, void *buf, diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h index 93e99d2583d6..7abe231e2ffe 100644 --- a/arch/x86/include/asm/fpu/types.h +++ b/arch/x86/include/asm/fpu/types.h @@ -545,6 +545,13 @@ struct fpu_guest { */ u64 xfeatures;
- /*
* @xfd: Save the guest value. Note that this is* *not* fpstate->xfd, which is the value* the host uses when doing XSAVE/XRSTOR.*/- u64 xfd;
- /*
*/
- @xfd_err: Save the guest value.
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c index a480fa8c65d5..ff17c96d290a 100644 --- a/arch/x86/kernel/fpu/core.c +++ b/arch/x86/kernel/fpu/core.c @@ -317,16 +317,6 @@ int fpu_enable_guest_xfd_features(struct fpu_guest *guest_fpu, u64 xfeatures) EXPORT_SYMBOL_FOR_KVM(fpu_enable_guest_xfd_features);
#ifdef CONFIG_X86_64 -void fpu_update_guest_xfd(struct fpu_guest *guest_fpu, u64 xfd) -{
- fpregs_lock();
- guest_fpu->fpstate->xfd = xfd;
- if (guest_fpu->fpstate->in_use)
xfd_update_state(guest_fpu->fpstate);- fpregs_unlock();
-} -EXPORT_SYMBOL_FOR_KVM(fpu_update_guest_xfd);
/**
- fpu_sync_guest_vmexit_xfd_state - Synchronize XFD MSR and software state
@@ -339,14 +329,12 @@ EXPORT_SYMBOL_FOR_KVM(fpu_update_guest_xfd);
- Note: It can be invoked unconditionally even when write emulation is
- enabled for the price of a then pointless MSR read.
*/ -void fpu_sync_guest_vmexit_xfd_state(void) +void fpu_sync_guest_vmexit_xfd_state(struct fpu_guest *gfpu) {
- struct fpstate *fpstate = x86_task_fpu(current)->fpstate;
- lockdep_assert_irqs_disabled(); if (fpu_state_size_dynamic()) {
rdmsrq(MSR_IA32_XFD, fpstate->xfd);__this_cpu_write(xfd_state, fpstate->xfd);
rdmsrq(MSR_IA32_XFD, gfpu->xfd);__this_cpu_write(xfd_state, gfpu->xfd);} EXPORT_SYMBOL_FOR_KVM(fpu_sync_guest_vmexit_xfd_state); @@ -890,6 +878,7 @@ void fpu_load_guest_fpstate(struct fpu_guest *gfpu) fpregs_restore_userregs();
fpregs_assert_state_consistent();
- xfd_set_state(gfpu->xfd); if (gfpu->xfd_err) wrmsrq(MSR_IA32_XFD_ERR, gfpu->xfd_err);
} diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h index 52ce19289989..c0ce05bee637 100644 --- a/arch/x86/kernel/fpu/xstate.h +++ b/arch/x86/kernel/fpu/xstate.h @@ -180,26 +180,28 @@ static inline void xfd_validate_state(struct fpstate *fpstate, u64 mask, bool rs #endif
#ifdef CONFIG_X86_64 -static inline void xfd_set_state(u64 xfd) +static inline void __xfd_set_state(u64 xfd) { wrmsrq(MSR_IA32_XFD, xfd); __this_cpu_write(xfd_state, xfd); }
+static inline void xfd_set_state(u64 xfd) +{
- if (__this_cpu_read(xfd_state) != xfd)
__xfd_set_state(xfd);+}
static inline void xfd_update_state(struct fpstate *fpstate) {
- if (fpu_state_size_dynamic()) {
u64 xfd = fpstate->xfd;if (__this_cpu_read(xfd_state) != xfd)xfd_set_state(xfd);- }
- if (fpu_state_size_dynamic())
xfd_set_state(fpstate->xfd);}
extern int __xfd_enable_feature(u64 which, struct fpu_guest *guest_fpu); #else static inline void xfd_set_state(u64 xfd) { }
+static inline void __xfd_set_state(u64 xfd) { } static inline void xfd_update_state(struct fpstate *fpstate) { }
static inline int __xfd_enable_feature(u64 which, struct fpu_guest *guest_fpu) { diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 01d95192dfc5..56fd082859bc 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4261,7 +4261,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) if (data & ~kvm_guest_supported_xfd(vcpu)) return 1;
fpu_update_guest_xfd(&vcpu->arch.guest_fpu, data);
vcpu->arch.guest_fpu.xfd = data;@@ -4617,7 +4617,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) !guest_cpu_cap_has(vcpu, X86_FEATURE_XFD)) return 1;
msr_info->data = vcpu->arch.guest_fpu.fpstate->xfd;
msr_info->data = vcpu->arch.guest_fpu.xfd;@@ -11405,7 +11405,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) * in #NM irqoff handler). */ if (vcpu->arch.xfd_no_write_intercept)
fpu_sync_guest_vmexit_xfd_state();
fpu_sync_guest_vmexit_xfd_state(&vcpu->arch.guest_fpu);kvm_x86_call(handle_exit_irqoff)(vcpu);
-- 2.52.0