[PATCH 0/5] x86, fpu/kvm: fix crash with AMX

[PATCH 0/5] x86, fpu/kvm: fix crash with AMX

[Paolo Bonzini]

Fix a possible host panic, due to an unexpected #NM, when a KVM guest
is using AMX features.

The guest's XFD value, which is stored in fpstate->xfd, is used for both
guest execution and host XSAVE operations.  However, the guest-configured
XFD setting can disable features that the host needs enabled to successfully
XRSTOR the guest FPU state.

The first patch replaces inline code in vcpu_enter_guest() with a new
function exported by kernel/fpu.  The new function is similar to
fpregs_lock_and_load() but operates with preemption disabled and
also restores the extra state (currently xfd_err) in struct guest_fpu.

The second patch then introduces a new xfd field in struct guest_fpu,
so that the guest's XFD setting can be swapped while leaving the host
value untouched in fpstate->xfd.

Patches 3 and 4 introduce a test.

Patch 5 makes KVM use fpregs_lock_and_load(), exporting it in place of
two lower-level functions whose other uses are now gone.

Reviews and acks are welcome (this could go in through either
the x86 or KVM trees).

Paolo


Paolo Bonzini (5):
  x86, fpu: introduce fpu_load_guest_fpstate()
  x86, fpu: separate fpstate->xfd and guest XFD
  selftests: kvm: renumber some sync points in amx_test
  selftests, kvm: try getting XFD and XSAVE state out of sync
  KVM: x86: kvm_fpu_get() is fpregs_lock_and_load()

 arch/x86/include/asm/fpu/api.h             |  7 ++--
 arch/x86/include/asm/fpu/types.h           |  7 ++++
 arch/x86/kernel/fpu/core.c                 | 38 ++++++++++-------
 arch/x86/kernel/fpu/xstate.h               | 18 ++++----
 arch/x86/kvm/fpu.h                         |  6 +--
 arch/x86/kvm/x86.c                         | 14 ++-----
 tools/testing/selftests/kvm/x86/amx_test.c | 49 +++++++++++++++-------
 7 files changed, 82 insertions(+), 57 deletions(-)

-- 
2.52.0

[PATCH 1/5] x86, fpu: introduce fpu_load_guest_fpstate()

[Paolo Bonzini]

Create a variant of fpregs_lock_and_load() that KVM can use in its
vCPU entry code after preemption has been disabled.  While basing
it on the existing logic in vcpu_enter_guest(), ensure that
fpregs_assert_state_consistent() always runs and sprinkle a few
more assertions.

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 |  1 +
 arch/x86/kernel/fpu/core.c     | 17 +++++++++++++++++
 arch/x86/kvm/x86.c             |  8 +-------
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
index cd6f194a912b..0820b2621416 100644
--- a/arch/x86/include/asm/fpu/api.h
+++ b/arch/x86/include/asm/fpu/api.h
@@ -147,6 +147,7 @@ extern void *get_xsave_addr(struct xregs_state *xsave, int xfeature_nr);
 /* KVM specific functions */
 extern bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu);
 extern void fpu_free_guest_fpstate(struct fpu_guest *gfpu);
+extern void fpu_load_guest_fpstate(struct fpu_guest *gfpu);
 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);
 
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 3ab27fb86618..a480fa8c65d5 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -878,6 +878,23 @@ void fpregs_lock_and_load(void)
 	fpregs_assert_state_consistent();
 }
 
+void fpu_load_guest_fpstate(struct fpu_guest *gfpu)
+{
+#ifdef CONFIG_X86_DEBUG_FPU
+	struct fpu *fpu = x86_task_fpu(current);
+	WARN_ON_ONCE(gfpu->fpstate != fpu->fpstate);
+#endif
+
+	lockdep_assert_preemption_disabled();
+	if (test_thread_flag(TIF_NEED_FPU_LOAD))
+		fpregs_restore_userregs();
+
+	fpregs_assert_state_consistent();
+	if (gfpu->xfd_err)
+		wrmsrq(MSR_IA32_XFD_ERR, gfpu->xfd_err);
+}
+EXPORT_SYMBOL_FOR_KVM(fpu_load_guest_fpstate);
+
 #ifdef CONFIG_X86_DEBUG_FPU
 /*
  * If current FPU state according to its tracking (loaded FPU context on this
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ff8812f3a129..01d95192dfc5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11300,13 +11300,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 		kvm_make_request(KVM_REQ_EVENT, vcpu);
 	}
 
-	fpregs_assert_state_consistent();
-	if (test_thread_flag(TIF_NEED_FPU_LOAD))
-		switch_fpu_return();
-
-	if (vcpu->arch.guest_fpu.xfd_err)
-		wrmsrq(MSR_IA32_XFD_ERR, vcpu->arch.guest_fpu.xfd_err);
-
+	fpu_load_guest_fpstate(&vcpu->arch.guest_fpu);
 	kvm_load_xfeatures(vcpu, true);
 
 	if (unlikely(vcpu->arch.switch_db_regs &&
-- 
2.52.0

[PATCH 2/5] x86, fpu: separate fpstate->xfd and guest XFD

[Paolo Bonzini]

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.

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;
 		break;
 	case MSR_IA32_XFD_ERR:
 		if (!msr_info->host_initiated &&
@@ -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;
 		break;
 	case MSR_IA32_XFD_ERR:
 		if (!msr_info->host_initiated &&
@@ -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

[PATCH 3/5] selftests: kvm: renumber some sync points in amx_test

[Paolo Bonzini]

Make room for the next test; separated for ease of review.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tools/testing/selftests/kvm/x86/amx_test.c | 26 ++++++++++++----------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/amx_test.c b/tools/testing/selftests/kvm/x86/amx_test.c
index f4ce5a185a7d..dd980cdac5df 100644
--- a/tools/testing/selftests/kvm/x86/amx_test.c
+++ b/tools/testing/selftests/kvm/x86/amx_test.c
@@ -144,7 +144,7 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 	__tileloadd(tiledata);
 	GUEST_SYNC(4);
 	__tilerelease();
-	GUEST_SYNC(5);
+	GUEST_SYNC(10);
 	/*
 	 * After XSAVEC, XTILEDATA is cleared in the xstate_bv but is set in
 	 * the xcomp_bv.
@@ -154,6 +154,8 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 	GUEST_ASSERT(!(xstate->header.xstate_bv & XFEATURE_MASK_XTILE_DATA));
 	GUEST_ASSERT(xstate->header.xcomp_bv & XFEATURE_MASK_XTILE_DATA);
 
+	/* #NM test */
+
 	/* xfd=0x40000, disable amx tiledata */
 	wrmsr(MSR_IA32_XFD, XFEATURE_MASK_XTILE_DATA);
 
@@ -166,13 +168,13 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 	GUEST_ASSERT(!(xstate->header.xstate_bv & XFEATURE_MASK_XTILE_DATA));
 	GUEST_ASSERT((xstate->header.xcomp_bv & XFEATURE_MASK_XTILE_DATA));
 
-	GUEST_SYNC(6);
+	GUEST_SYNC(11);
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == XFEATURE_MASK_XTILE_DATA);
 	set_tilecfg(amx_cfg);
 	__ldtilecfg(amx_cfg);
 	/* Trigger #NM exception */
 	__tileloadd(tiledata);
-	GUEST_SYNC(10);
+	GUEST_SYNC(15);
 
 	GUEST_DONE();
 }
@@ -180,18 +182,18 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 void guest_nm_handler(struct ex_regs *regs)
 {
 	/* Check if #NM is triggered by XFEATURE_MASK_XTILE_DATA */
-	GUEST_SYNC(7);
+	GUEST_SYNC(12);
 	GUEST_ASSERT(!(get_cr0() & X86_CR0_TS));
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILE_DATA);
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == XFEATURE_MASK_XTILE_DATA);
-	GUEST_SYNC(8);
+	GUEST_SYNC(13);
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILE_DATA);
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == XFEATURE_MASK_XTILE_DATA);
 	/* Clear xfd_err */
 	wrmsr(MSR_IA32_XFD_ERR, 0);
 	/* xfd=0, enable amx */
 	wrmsr(MSR_IA32_XFD, 0);
-	GUEST_SYNC(9);
+	GUEST_SYNC(14);
 }
 
 int main(int argc, char *argv[])
@@ -257,14 +259,14 @@ int main(int argc, char *argv[])
 			case 1:
 			case 2:
 			case 3:
-			case 5:
-			case 6:
-			case 7:
-			case 8:
+			case 10:
+			case 11:
+			case 12:
+			case 13:
 				fprintf(stderr, "GUEST_SYNC(%ld)\n", uc.args[1]);
 				break;
 			case 4:
-			case 10:
+			case 15:
 				fprintf(stderr,
 				"GUEST_SYNC(%ld), check save/restore status\n", uc.args[1]);
 
@@ -280,7 +282,7 @@ int main(int argc, char *argv[])
 				TEST_ASSERT(ret == 0, "memcmp failed, ret=%d", ret);
 				kvm_x86_state_cleanup(state);
 				break;
-			case 9:
+			case 14:
 				fprintf(stderr,
 				"GUEST_SYNC(%ld), #NM exception and enable amx\n", uc.args[1]);
 				break;
-- 
2.52.0

[PATCH 4/5] selftests, kvm: try getting XFD and XSAVE state out of sync

[Paolo Bonzini]

The host is allowed to set FPU state that includes a disabled
xstate component.  Check that this does not cause bad effects.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tools/testing/selftests/kvm/x86/amx_test.c | 25 +++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/amx_test.c b/tools/testing/selftests/kvm/x86/amx_test.c
index dd980cdac5df..5222ec6f71d3 100644
--- a/tools/testing/selftests/kvm/x86/amx_test.c
+++ b/tools/testing/selftests/kvm/x86/amx_test.c
@@ -142,7 +142,16 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 	GUEST_SYNC(3);
 	/* Check save/restore when trap to userspace */
 	__tileloadd(tiledata);
+
 	GUEST_SYNC(4);
+	/* xfd=0x40000, disable amx tiledata */
+	wrmsr(MSR_IA32_XFD, XFEATURE_MASK_XTILE_DATA);
+
+	GUEST_SYNC(5);
+	/* host tries setting tiledata while guest XFD is set */
+	GUEST_SYNC(6);
+
+	wrmsr(MSR_IA32_XFD, 0);
 	__tilerelease();
 	GUEST_SYNC(10);
 	/*
@@ -202,6 +211,7 @@ int main(int argc, char *argv[])
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
 	struct kvm_x86_state *state;
+	struct kvm_x86_state *tile_state = NULL;
 	int xsave_restore_size;
 	vm_vaddr_t amx_cfg, tiledata, xstate;
 	struct ucall uc;
@@ -259,6 +269,7 @@ int main(int argc, char *argv[])
 			case 1:
 			case 2:
 			case 3:
+			case 6:
 			case 10:
 			case 11:
 			case 12:
@@ -267,8 +278,7 @@ int main(int argc, char *argv[])
 				break;
 			case 4:
 			case 15:
-				fprintf(stderr,
-				"GUEST_SYNC(%ld), check save/restore status\n", uc.args[1]);
+				fprintf(stderr, "GUEST_SYNC(%ld), check save/restore status\n", uc.args[1]);
 
 				/* Compacted mode, get amx offset by xsave area
 				 * size subtract 8K amx size.
@@ -280,8 +290,17 @@ int main(int argc, char *argv[])
 				/* Only check TMM0 register, 1 tile */
 				ret = memcmp(amx_start, tiles_data, TILE_SIZE);
 				TEST_ASSERT(ret == 0, "memcmp failed, ret=%d", ret);
-				kvm_x86_state_cleanup(state);
+				if (uc.args[1] == 4)
+					tile_state = state;
+				else
+					kvm_x86_state_cleanup(state);
 				break;
+			case 5:
+				fprintf(stderr, "GUEST_SYNC(%ld), before KVM_SET_XSAVE\n", uc.args[1]);
+				vcpu_xsave_set(vcpu, tile_state->xsave);
+				fprintf(stderr, "GUEST_SYNC(%ld), after KVM_SET_XSAVE\n", uc.args[1]);
+				/* do not restore full state */
+				continue;
 			case 14:
 				fprintf(stderr,
 				"GUEST_SYNC(%ld), #NM exception and enable amx\n", uc.args[1]);
-- 
2.52.0

[PATCH 5/5] KVM: x86: kvm_fpu_get() is fpregs_lock_and_load()

[Paolo Bonzini]

The only difference is the usage of switch_fpu_return() vs.
fpregs_restore_userregs().  In turn, these are only different
if there is no FPU at all, but KVM requires one.  Therefore use the
pre-made export---the code is simpler and there is no functional change.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kernel/fpu/core.c | 2 +--
 arch/x86/kvm/fpu.h         | 6 +-----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index ff17c96d290a..6571952c6ef1 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -846,7 +846,6 @@ void switch_fpu_return(void)
 
 	fpregs_restore_userregs();
 }
-EXPORT_SYMBOL_FOR_KVM(switch_fpu_return);
 
 void fpregs_lock_and_load(void)
 {
@@ -865,6 +864,7 @@ void fpregs_lock_and_load(void)
 
 	fpregs_assert_state_consistent();
 }
+EXPORT_SYMBOL_FOR_KVM(fpregs_lock_and_load);
 
 void fpu_load_guest_fpstate(struct fpu_guest *gfpu)
 {
@@ -899,7 +899,6 @@ void fpregs_assert_state_consistent(void)
 
 	WARN_ON_FPU(!fpregs_state_valid(fpu, smp_processor_id()));
 }
-EXPORT_SYMBOL_FOR_KVM(fpregs_assert_state_consistent);
 #endif
 
 void fpregs_mark_activate(void)
diff --git a/arch/x86/kvm/fpu.h b/arch/x86/kvm/fpu.h
index f898781b6a06..b6a03d8fa8af 100644
--- a/arch/x86/kvm/fpu.h
+++ b/arch/x86/kvm/fpu.h
@@ -149,11 +149,7 @@ static inline void _kvm_write_mmx_reg(int reg, const u64 *data)
 
 static inline void kvm_fpu_get(void)
 {
-	fpregs_lock();
-
-	fpregs_assert_state_consistent();
-	if (test_thread_flag(TIF_NEED_FPU_LOAD))
-		switch_fpu_return();
+	fpregs_lock_and_load();
 }
 
 static inline void kvm_fpu_put(void)
-- 
2.52.0

Re: [PATCH 2/5] x86, fpu: separate fpstate->xfd and guest XFD

[Yao Yuan]

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;
>  		break;
>  	case MSR_IA32_XFD_ERR:
>  		if (!msr_info->host_initiated &&
> @@ -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;
>  		break;
>  	case MSR_IA32_XFD_ERR:
>  		if (!msr_info->host_initiated &&
> @@ -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
>
>

Re: [PATCH 1/5] x86, fpu: introduce fpu_load_guest_fpstate()

[Yao Yuan]

On Wed, Dec 24, 2025 at 01:12:45AM +0800, Paolo Bonzini wrote:
> Create a variant of fpregs_lock_and_load() that KVM can use in its
> vCPU entry code after preemption has been disabled.  While basing
> it on the existing logic in vcpu_enter_guest(), ensure that
> fpregs_assert_state_consistent() always runs and sprinkle a few
> more assertions.
>
> 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 |  1 +
>  arch/x86/kernel/fpu/core.c     | 17 +++++++++++++++++
>  arch/x86/kvm/x86.c             |  8 +-------
>  3 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
> index cd6f194a912b..0820b2621416 100644
> --- a/arch/x86/include/asm/fpu/api.h
> +++ b/arch/x86/include/asm/fpu/api.h
> @@ -147,6 +147,7 @@ extern void *get_xsave_addr(struct xregs_state *xsave, int xfeature_nr);
>  /* KVM specific functions */
>  extern bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu);
>  extern void fpu_free_guest_fpstate(struct fpu_guest *gfpu);
> +extern void fpu_load_guest_fpstate(struct fpu_guest *gfpu);
>  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);
>
> diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
> index 3ab27fb86618..a480fa8c65d5 100644
> --- a/arch/x86/kernel/fpu/core.c
> +++ b/arch/x86/kernel/fpu/core.c
> @@ -878,6 +878,23 @@ void fpregs_lock_and_load(void)
>  	fpregs_assert_state_consistent();
>  }
>
> +void fpu_load_guest_fpstate(struct fpu_guest *gfpu)
> +{
> +#ifdef CONFIG_X86_DEBUG_FPU
> +	struct fpu *fpu = x86_task_fpu(current);
> +	WARN_ON_ONCE(gfpu->fpstate != fpu->fpstate);
> +#endif
> +
> +	lockdep_assert_preemption_disabled();

Hi Paolo,

Do we need make sure the irq is disabled w/ lockdep ?

The irq_fpu_usable() returns true for:

!in_nmi () && in_hardirq() and !softirq_count()

It's possible that the TIF_NEED_FPU_LOAD is set again
w/ interrupt is enabled.

> +	if (test_thread_flag(TIF_NEED_FPU_LOAD))
> +		fpregs_restore_userregs();
> +
> +	fpregs_assert_state_consistent();
> +	if (gfpu->xfd_err)
> +		wrmsrq(MSR_IA32_XFD_ERR, gfpu->xfd_err);
> +}
> +EXPORT_SYMBOL_FOR_KVM(fpu_load_guest_fpstate);
> +
>  #ifdef CONFIG_X86_DEBUG_FPU
>  /*
>   * If current FPU state according to its tracking (loaded FPU context on this
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ff8812f3a129..01d95192dfc5 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -11300,13 +11300,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>  		kvm_make_request(KVM_REQ_EVENT, vcpu);
>  	}
>
> -	fpregs_assert_state_consistent();
> -	if (test_thread_flag(TIF_NEED_FPU_LOAD))
> -		switch_fpu_return();
> -
> -	if (vcpu->arch.guest_fpu.xfd_err)
> -		wrmsrq(MSR_IA32_XFD_ERR, vcpu->arch.guest_fpu.xfd_err);
> -
> +	fpu_load_guest_fpstate(&vcpu->arch.guest_fpu);
>  	kvm_load_xfeatures(vcpu, true);
>
>  	if (unlikely(vcpu->arch.switch_db_regs &&
> --
> 2.52.0
>

Re: [PATCH 1/5] x86, fpu: introduce fpu_load_guest_fpstate()

[Sean Christopherson]

On Fri, Dec 26, 2025, Yao Yuan wrote:
> On Wed, Dec 24, 2025 at 01:12:45AM +0800, Paolo Bonzini wrote:
> > Create a variant of fpregs_lock_and_load() that KVM can use in its
> > vCPU entry code after preemption has been disabled.  While basing
> > it on the existing logic in vcpu_enter_guest(), ensure that
> > fpregs_assert_state_consistent() always runs and sprinkle a few
> > more assertions.
> >
> > 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 |  1 +
> >  arch/x86/kernel/fpu/core.c     | 17 +++++++++++++++++
> >  arch/x86/kvm/x86.c             |  8 +-------
> >  3 files changed, 19 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
> > index cd6f194a912b..0820b2621416 100644
> > --- a/arch/x86/include/asm/fpu/api.h
> > +++ b/arch/x86/include/asm/fpu/api.h
> > @@ -147,6 +147,7 @@ extern void *get_xsave_addr(struct xregs_state *xsave, int xfeature_nr);
> >  /* KVM specific functions */
> >  extern bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu);
> >  extern void fpu_free_guest_fpstate(struct fpu_guest *gfpu);
> > +extern void fpu_load_guest_fpstate(struct fpu_guest *gfpu);
> >  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);
> >
> > diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
> > index 3ab27fb86618..a480fa8c65d5 100644
> > --- a/arch/x86/kernel/fpu/core.c
> > +++ b/arch/x86/kernel/fpu/core.c
> > @@ -878,6 +878,23 @@ void fpregs_lock_and_load(void)
> >  	fpregs_assert_state_consistent();
> >  }
> >
> > +void fpu_load_guest_fpstate(struct fpu_guest *gfpu)
> > +{
> > +#ifdef CONFIG_X86_DEBUG_FPU
> > +	struct fpu *fpu = x86_task_fpu(current);
> > +	WARN_ON_ONCE(gfpu->fpstate != fpu->fpstate);
> > +#endif
> > +
> > +	lockdep_assert_preemption_disabled();
> 
> Hi Paolo,
> 
> Do we need make sure the irq is disabled w/ lockdep ?

Yes please, e.g. see commit 2620fe268e80 ("KVM: x86: Revert "KVM: X86: Fix fpu
state crash in kvm guest"").

> The irq_fpu_usable() returns true for:
> 
> !in_nmi () && in_hardirq() and !softirq_count()
> 
> It's possible that the TIF_NEED_FPU_LOAD is set again
> w/ interrupt is enabled.

Re: [PATCH 2/5] x86, fpu: separate fpstate->xfd and guest XFD

[Sean Christopherson]

On Wed, Dec 24, 2025, 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.
>
> 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>
> ---

...

>  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;

After staring at this and playing with the selftest for pretty much the entire
day, I'm strongly against this fix.

The fix works only because the userspace XFD[18] must be '0' since the kernel
never re-disables XFD features after they are enabled.  Which is probably fine
in practice since re-disabling a component for a guest task would need to force
the guest FPU back into an init state as well, but I don't love the complexity.

This also creates a nasty, subtle asymmetry in KVM's ABI.  Notably, the comment
above is wrong.  XSAVE does NOT run with fpstate->xfd, it runs with whatever
happens to be in hardware.  For non-guest tasks, fpstate->xfd is guaranteed to
be resident in hardware when save_fpregs_to_fpstate() runs, but for guest tasks,
it will usually be the _guest's_ value.  So in the common case, KVM_GET_XSAVE2
would not return the same data set by KVM_SET_XSAVE.

In theory we could ensure KVM saved exactly what is resident in hardware, but
that's quite tricky (and costly!) as it would require doing xfd_update_state()
before _every_ save_fpregs_to_fpstate(), e.g. not just in fpu_swap_kvm_fpstate().
E.g. if the host kernel used the FPU from IRQ context (spoiler alert!), then KVM
wouldn't have a chance to swap in the maximal XFD[18]=0 value (i.e. the userspace
task's XFD).

As evidenced by how long it took for someone to run into this bug, saving non-init
XTILE data with XFD[18]=1 requires hitting a rare edge case, so for all intents
and purposes KVM's ABI is that XSTATE compontents that are disabled via XFD are
effectively dropped across save/load.  Which is a bit gross, but architecturally
acceptable because the SDM explicitly says software must not rely on state being
retained when XFD[i]=1, i.e. KVM is allowed to clobber the state.

Lastly, the fix is effectively papering over another bug, which I'm pretty sure
is the underlying issue that was originally encountered.  Assuming QEMU doesn't
intercept MSR_IA32_XFD for its own purposes, the only sequence I've come up with
that would result in KVM trying to load XTILE data with XFD[18]=1, without a
colluding userspace VMM (Paolo's selftest) is:

  1. vCPU loads non-init XTILE data without ever setting XFD to a non-zero value
     (KVM only disables XFD interception on writes with a non-zero value).
  2. Guest executes WRMSR(MSR_IA32_XFD) to set XFD[18] = 1
  3. VM-Exit due to the WRMSR
  4. Host IRQ arrives and triggers kernel_fpu_begin()
  5. save_fpregs_to_fpstate() saves guest FPU with XFD[18]=0
  6. fpu_update_guest_xfd() stuffs guest_fpu->fpstate->xfd = XFD[18]=1
  7. vcpu_enter_guest() attempts to load XTILE data with XFD[18]=1

Note!  There's no KVM_SET_XSAVE2 in the above, i.e. this doesn't require userspace
to trigger save/restore for live migration or whatever, the only timing condition
is the arrival of an IRQ that uses kernel FPU during the XFD 0=>1 VM-Exit.

E.g. with a simulated IRQ in the KVM:

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ff8812f3a129..9fc7c0aadfd7 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4261,6 +4261,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
                if (data & ~kvm_guest_supported_xfd(vcpu))
                        return 1;
 
+               /* Simulate an IRQ arriving at just the wrong time. */
+               kernel_fpu_begin();
+               kernel_fpu_end();
+
                fpu_update_guest_xfd(&vcpu->arch.guest_fpu, data);
                break;
        case MSR_IA32_XFD_ERR:

and a lightly modified KVM AMX selftest to write XFD *before* TILERELEASE:

diff --git a/tools/testing/selftests/kvm/x86/amx_test.c b/tools/testing/selftests/kvm/x86/amx_test.c
index f4ce5a185a7d..13d4a6befd5e 100644
--- a/tools/testing/selftests/kvm/x86/amx_test.c
+++ b/tools/testing/selftests/kvm/x86/amx_test.c
@@ -143,6 +143,8 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
        /* Check save/restore when trap to userspace */
        __tileloadd(tiledata);
        GUEST_SYNC(4);
+       /* xfd=0x40000, disable amx tiledata */
+       wrmsr(MSR_IA32_XFD, XFEATURE_MASK_XTILE_DATA);
        __tilerelease();
        GUEST_SYNC(5);
        /*

we get a nice explosion:

  ------------[ cut here ]------------
  WARNING: arch/x86/kernel/fpu/core.c:289 at fpu_free_guest_fpstate+0x2f/0x40, CPU#30: kvm-nx-lpage-re/853
  Modules linked in: kvm_intel kvm irqbypass
  CPU: 30 UID: 1000 PID: 853 Comm: kvm-nx-lpage-re Tainted: G      D W           6.19.0-rc2-ffa07f7fd437-x86_amx_nm_xfd_non_init-vm #171 NONE 
  Tainted: [D]=DIE, [W]=WARN
  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
  RIP: 0010:fpu_free_guest_fpstate+0x2f/0x40
  Call Trace:
   <TASK>
   kvm_arch_vcpu_destroy+0xa1/0x120 [kvm]
   kvm_destroy_vcpus+0xc6/0x150 [kvm]
   kvm_arch_destroy_vm+0x2d/0xe0 [kvm]
   kvm_destroy_vm+0x15e/0x260 [kvm]
   kvm_vcpu_release+0x35/0x50 [kvm]
   __fput+0xd9/0x290
   task_work_run+0x5b/0x80
   do_exit+0x290/0x9d0
   vhost_task_fn+0x9d/0xe0
   ret_from_fork+0x1a3/0x200
   ret_from_fork_asm+0x11/0x20
   </TASK>

So, given that KVM's effective ABI is to record XSTATE_BV[i]=0 if XFD[i]==1, I
vote to fix this by emulating that behavior when stuffing XFD in
fpu_update_guest_xfd(), and then manually closing the hole Paolo found in
fpu_copy_uabi_to_guest_fpstate().

--
From: Sean Christopherson <seanjc@google.com>
Date: Mon, 29 Dec 2025 12:14:09 -0800
Subject: [PATCH] x86/fpu: Clear XSTATE_BV[i] in save state whenever XFD[i]=1

When loading guest XSAVE state via KVM_SET_XSAVE, and when updating XFD in
response to a guest WRMSR, clear XFD-disabled features in the saved (or to
be restored) XSTATE_BV to ensure KVM doesn't attempt to load state for
features that are disabled via the guest's XFD.  Because the kernel
executes XRSTOR with the guest's XFD, saving XSTATE_BV[i]=1 with XFD[i]=1
will cause XRSTOR to #NM and panic the kernel.

E.g. if fpu_update_guest_xfd() sets XFD without clearing XSTATE_BV:

  ------------[ cut here ]------------
  WARNING: arch/x86/kernel/traps.c:1524 at exc_device_not_available+0x101/0x110, CPU#29: amx_test/848
  Modules linked in: kvm_intel kvm irqbypass
  CPU: 29 UID: 1000 PID: 848 Comm: amx_test Not tainted 6.19.0-rc2-ffa07f7fd437-x86_amx_nm_xfd_non_init-vm #171 NONE
  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
  RIP: 0010:exc_device_not_available+0x101/0x110
  Call Trace:
   <TASK>
   asm_exc_device_not_available+0x1a/0x20
  RIP: 0010:restore_fpregs_from_fpstate+0x36/0x90
   switch_fpu_return+0x4a/0xb0
   kvm_arch_vcpu_ioctl_run+0x1245/0x1e40 [kvm]
   kvm_vcpu_ioctl+0x2c3/0x8f0 [kvm]
   __x64_sys_ioctl+0x8f/0xd0
   do_syscall_64+0x62/0x940
   entry_SYSCALL_64_after_hwframe+0x4b/0x53
   </TASK>
  ---[ end trace 0000000000000000 ]---

and if userspace stuffs XSTATE_BV[i]=1 via KVM_SET_XSAVE:

  ------------[ cut here ]------------
  WARNING: arch/x86/kernel/traps.c:1524 at exc_device_not_available+0x101/0x110, CPU#14: amx_test/867
  Modules linked in: kvm_intel kvm irqbypass
  CPU: 14 UID: 1000 PID: 867 Comm: amx_test Not tainted 6.19.0-rc2-2dace9faccd6-x86_amx_nm_xfd_non_init-vm #168 NONE
  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
  RIP: 0010:exc_device_not_available+0x101/0x110
  Call Trace:
   <TASK>
   asm_exc_device_not_available+0x1a/0x20
  RIP: 0010:restore_fpregs_from_fpstate+0x36/0x90
   fpu_swap_kvm_fpstate+0x6b/0x120
   kvm_load_guest_fpu+0x30/0x80 [kvm]
   kvm_arch_vcpu_ioctl_run+0x85/0x1e40 [kvm]
   kvm_vcpu_ioctl+0x2c3/0x8f0 [kvm]
   __x64_sys_ioctl+0x8f/0xd0
   do_syscall_64+0x62/0x940
   entry_SYSCALL_64_after_hwframe+0x4b/0x53
   </TASK>
  ---[ end trace 0000000000000000 ]---

Note, the new behavior is consistent with KVM's de facto ABI and with the
AMX architecture.  Per Intel's SDM, XSAVE saves XSTATE_BV as '0' for
components that are disabled via XFD (and non-compacted XSAVE saves the
initial configuration of the state component):

  If XSAVE, XSAVEC, XSAVEOPT, or XSAVES is saving the state component i,
  the instruction does not generate #NM when XCR0[i] = IA32_XFD[i] = 1;
  instead, it operates as if XINUSE[i] = 0 (and the state component was
  in its initial state): it saves bit i of XSTATE_BV field of the XSAVE
  header as 0; in addition, XSAVE saves the initial configuration of the
  state component (the other instructions do not save state component i).

Because fpu_swap_kvm_fpstate() => save_fpregs_to_fpstate() saves the
outgoing FPU state with the current XFD, it is extremely unlikely for
userspace to save non-initialized data and/or XSTATE_BV[i]=1 for
XFD-disabled features.  Specifically, the only known sequence where KVM
can save XTILE data and also see XFD[18]=1 _without_ a colluding VMM is by
hitting the bug in fpu_update_guest_xfd():

  1. vCPU loads non-init XTILE data without ever setting XFD to a non-zero
     value (KVM only disables XFD interception on the first non-zero write).
  2. Guest executes WRMSR(MSR_IA32_XFD) to set XFD[18] = 1
  3. VM-Exit due to the WRMSR
  4. Host IRQ arrives and triggers kernel_fpu_begin()
  5. save_fpregs_to_fpstate() saves guest FPU with XFD[18]=0
  6. fpu_update_guest_xfd() stuffs guest_fpu->fpstate->xfd = XFD[18]=1
  7. vcpu_enter_guest() attempts to load XTILE data with XFD[18]=1

Alternatively, KVM could always do XRSTOR with XFD=0, e.g. by using the
VMM task's XFD, but that approach really only papers over the bug, and it
would create a subtle asymmetry in KVM's ABI as KVM_GET_XSAVE2 would NOT
return the XSTATE_BV set by KVM_SET_XSAVE.  And if hardening KVM against
XFD-related bugs is desirable, a more robust solution would be to eat #NM
faults on XRSTOR and signal to KVM that XRSTOR failed, e.g. so that KVM
can terminate the VM instead of panicking the host.

Reported-by: Paolo Bonzini <pbonzini@redhat.com>
Closes: https://lore.kernel.org/all/20251224001249.1041934-1-pbonzini@redhat.com
Cc: stable@vger.kernel.org
Fixes: 820a6ee944e7 ("kvm: x86: Add emulation for IA32_XFD", 2022-01-14)
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/fpu/api.h |  2 +-
 arch/x86/kernel/fpu/core.c     | 35 ++++++++++++++++++++++++++++------
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
index cd6f194a912b..0b218f5eaafd 100644
--- a/arch/x86/include/asm/fpu/api.h
+++ b/arch/x86/include/asm/fpu/api.h
@@ -160,7 +160,7 @@ static inline void fpu_sync_guest_vmexit_xfd_state(void) { }
 
 extern void fpu_copy_guest_fpstate_to_uabi(struct fpu_guest *gfpu, void *buf,
 					   unsigned int size, u64 xfeatures, u32 pkru);
-extern int fpu_copy_uabi_to_guest_fpstate(struct fpu_guest *gfpu, const void *buf, u64 xcr0, u32 *vpkru);
+extern int fpu_copy_uabi_to_guest_fpstate(struct fpu_guest *gfpu, void *buf, u64 xcr0, u32 *vpkru);
 
 static inline void fpstate_set_confidential(struct fpu_guest *gfpu)
 {
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index da233f20ae6f..af756875c701 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -319,10 +319,25 @@ 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)
 {
+	struct fpstate *fpstate = guest_fpu->fpstate;
+
 	fpregs_lock();
-	guest_fpu->fpstate->xfd = xfd;
-	if (guest_fpu->fpstate->in_use)
-		xfd_update_state(guest_fpu->fpstate);
+	fpstate->xfd = xfd;
+	if (fpstate->in_use)
+		xfd_update_state(fpstate);
+
+	/*
+	 * If the guest's FPU state is NOT resident in hardware, clear disabled
+	 * components in XSTATE_BV as attempting to load disabled components
+	 * will generate #NM _in the host_, and KVM's ABI is that saving guest
+	 * XSAVE state should see XSTATE_BV[i]=0 if XFD[i]=1.
+	 *
+	 * If the guest's FPU state is in hardware, simply do nothing as XSAVE
+	 * itself saves XSTATE_BV[i] as 0 if XFD[i]=1.
+	 */
+	if (xfd && test_thread_flag(TIF_NEED_FPU_LOAD))
+		fpstate->regs.xsave.header.xfeatures &= ~xfd;
+
 	fpregs_unlock();
 }
 EXPORT_SYMBOL_FOR_KVM(fpu_update_guest_xfd);
@@ -412,11 +427,11 @@ void fpu_copy_guest_fpstate_to_uabi(struct fpu_guest *gfpu, void *buf,
 }
 EXPORT_SYMBOL_FOR_KVM(fpu_copy_guest_fpstate_to_uabi);
 
-int fpu_copy_uabi_to_guest_fpstate(struct fpu_guest *gfpu, const void *buf,
-				   u64 xcr0, u32 *vpkru)
+int fpu_copy_uabi_to_guest_fpstate(struct fpu_guest *gfpu, void *buf, u64 xcr0,
+				   u32 *vpkru)
 {
 	struct fpstate *kstate = gfpu->fpstate;
-	const union fpregs_state *ustate = buf;
+	union fpregs_state *ustate = buf;
 
 	if (!cpu_feature_enabled(X86_FEATURE_XSAVE)) {
 		if (ustate->xsave.header.xfeatures & ~XFEATURE_MASK_FPSSE)
@@ -430,6 +445,14 @@ int fpu_copy_uabi_to_guest_fpstate(struct fpu_guest *gfpu, const void *buf,
 	if (ustate->xsave.header.xfeatures & ~xcr0)
 		return -EINVAL;
 
+	/*
+	 * Initialize features that are disabled via XFD instead of restoring
+	 * state provided by userspace.  KVM's ABI is that XFD-disabled state
+	 * is undefined on "save" and initialized on "load" (KVM doesn't reject
+	 * non-initialized XFD state for backwards compatibility).
+	 */
+	ustate->xsave.header.xfeatures &= ~kstate->xfd;
+
 	/*
 	 * Nullify @vpkru to preserve its current value if PKRU's bit isn't set
 	 * in the header.  KVM's odd ABI is to leave PKRU untouched in this

base-commit: 1ea29dfaec1a81bab4ac00c442293fda2cc56942
--

Re: [PATCH 1/5] x86, fpu: introduce fpu_load_guest_fpstate()

[Paolo Bonzini]

On Mon, Dec 29, 2025 at 4:58 PM Sean Christopherson <seanjc@google.com> wrote:
> > Do we need make sure the irq is disabled w/ lockdep ?
>
> Yes please

Sure, no objection.

Paolo

Re: [PATCH 2/5] x86, fpu: separate fpstate->xfd and guest XFD

[Paolo Bonzini]

On Mon, Dec 29, 2025 at 11:45 PM Sean Christopherson <seanjc@google.com> wrote:
> The fix works only because the userspace XFD[18] must be '0' since the kernel
> never re-disables XFD features after they are enabled.

Yes, this is why I considered XFD[18]=1 to be a bug.

> Which is probably fine
> in practice since re-disabling a component for a guest task would need to force
> the guest FPU back into an init state as well, but I don't love the complexity.
>
> This also creates a nasty, subtle asymmetry in KVM's ABI.

I find this second argument more convincing; I preferred the
complexity of an extra field to track guest xfd, to having to patch
xstate_bv.

(Initially I was worried also about mismatches between xstate_bv and
xcomp_bv but those are find; When the compacted format is in use
xcomp_bv[62:0] is simply EDX:EAX & XCR0).

> Lastly, the fix is effectively papering over another bug, which I'm pretty sure
> is the underlying issue that was originally encountered.

Yes, I agree this is the most likely scenario. Whether it's papering
over it or otherwise, it depends on what you consider the invariants
to be.

> So, given that KVM's effective ABI is to record XSTATE_BV[i]=0 if XFD[i]==1, I
> vote to fix this by emulating that behavior when stuffing XFD in
> fpu_update_guest_xfd(), and then manually closing the hole Paolo found in
> fpu_copy_uabi_to_guest_fpstate().

I disagree with changing the argument from const void* to void*.
Let's instead treat it as a KVM backwards-compatibility quirk:

    union fpregs_state *xstate =
        (union fpregs_state *)guest_xsave->region;
    xstate->xsave.header.xfeatures &=
        ~vcpu->arch.guest_fpu.fpstate->xfd;

It keeps the kernel/ API const as expected and if anything I'd
consider adding a WARN to fpu_copy_uabi_to_guest_fpstate(), basically
asserting that there would be no #NM on the subsequent restore.

> @@ -319,10 +319,25 @@ 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)
>  {
> +       struct fpstate *fpstate = guest_fpu->fpstate;
> +
>         fpregs_lock();
> -       guest_fpu->fpstate->xfd = xfd;
> -       if (guest_fpu->fpstate->in_use)
> -               xfd_update_state(guest_fpu->fpstate);
> +       fpstate->xfd = xfd;
> +       if (fpstate->in_use)
> +               xfd_update_state(fpstate);
> +
> +       /*
> +        * If the guest's FPU state is NOT resident in hardware, clear disabled
> +        * components in XSTATE_BV as attempting to load disabled components
> +        * will generate #NM _in the host_, and KVM's ABI is that saving guest
> +        * XSAVE state should see XSTATE_BV[i]=0 if XFD[i]=1.
> +        *
> +        * If the guest's FPU state is in hardware, simply do nothing as XSAVE
> +        * itself saves XSTATE_BV[i] as 0 if XFD[i]=1.

s/saves/(from fpu_swap_kvm_fpstate) will save/

> +        */
> +       if (xfd && test_thread_flag(TIF_NEED_FPU_LOAD))
> +               fpstate->regs.xsave.header.xfeatures &= ~xfd;

No objections to this part.  I'll play with this to adjust the
selftests either tomorrow or, more likely, on January 2nd, and send a
v2 that also includes the change from preemption_disabled to
irqs_disabled.

I take it that you don't have any qualms with the new
fpu_load_guest_fpstate function, but let me know if you prefer to have
it in a separate submission destined to 6.20 only.

Paolo

Re: [PATCH 3/5] selftests: kvm: renumber some sync points in amx_test

[Sean Christopherson]

[-- Attachment #1: Type: text/plain, Size: 625 bytes --]

On Wed, Dec 24, 2025, Paolo Bonzini wrote:
> Make room for the next test; separated for ease of review.

Heh, but after review, the discontiguous sync numbers are super confusing.  Rather
than use arbitrary, incrementing numbers, what if we specify the action the host
should take?  Then there's very little "magic" or implicit synchronization between
the guest and host.  The only downside is that the "stage" prints are useless/lost,
but IMO that's largely a non-issue.

Tangentially related, the test doesn't ever verify that a #NM actually occurs, now
would be a good time to address that.

Full set of patches attached.

[-- Attachment #2: 0001-KVM-selftests-Use-named-sync-actions-in-AMX-test-ins.patch --]
[-- Type: text/x-diff, Size: 6602 bytes --]

From 4031198ac76584a0e906e36796a3b41f6e428d1b Mon Sep 17 00:00:00 2001
From: Sean Christopherson <seanjc@google.com>
Date: Wed, 24 Dec 2025 01:12:47 +0100
Subject: [PATCH 1/4] KVM: selftests: Use named sync actions in AMX test
 instead of arbitrary numbers

Rework the guest=>host syncs in the AMX test to use named actions instead
of arbitrary, incrementing numbers.  The "stage" of the test has no real
meaning, what matters is what action the test wants the host to perform.
The incrementing numbers are somewhat helpful for triaging failures, but
fully debugging failures almost always requires a much deeper dive into
the test (and KVM).

Opportunistically delete all prints to stderr as they aren't helpful
without the arbitrary numbers.

Using named actions will make it easier to extend the test to validate
more obscure/specific scenarios without creating a maintenance nightmare.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/x86/amx_test.c | 88 +++++++++++-----------
 1 file changed, 42 insertions(+), 46 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/amx_test.c b/tools/testing/selftests/kvm/x86/amx_test.c
index f4ce5a185a7d..3bcb10f26a70 100644
--- a/tools/testing/selftests/kvm/x86/amx_test.c
+++ b/tools/testing/selftests/kvm/x86/amx_test.c
@@ -124,6 +124,14 @@ static void set_tilecfg(struct tile_config *cfg)
 	}
 }
 
+enum amx_test_syncs {
+	AMX_SYNC_SAVE 		= BIT(0),
+	AMX_SYNC_RESTORE	= BIT(1),
+	AMX_SYNC_CHECK_TILEDATA	= BIT(2),
+
+	AMX_SYNC_SAVE_RESTORE = AMX_SYNC_SAVE | AMX_SYNC_RESTORE,
+};
+
 static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 						    struct tile_data *tiledata,
 						    struct xstate *xstate)
@@ -131,20 +139,20 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 	GUEST_ASSERT(this_cpu_has(X86_FEATURE_XSAVE) &&
 		     this_cpu_has(X86_FEATURE_OSXSAVE));
 	check_xtile_info();
-	GUEST_SYNC(1);
+	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE);
 
 	/* xfd=0, enable amx */
 	wrmsr(MSR_IA32_XFD, 0);
-	GUEST_SYNC(2);
+	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE);
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == 0);
 	set_tilecfg(amx_cfg);
 	__ldtilecfg(amx_cfg);
-	GUEST_SYNC(3);
+	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE);
 	/* Check save/restore when trap to userspace */
 	__tileloadd(tiledata);
-	GUEST_SYNC(4);
+	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE | AMX_SYNC_CHECK_TILEDATA);
 	__tilerelease();
-	GUEST_SYNC(5);
+	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE);
 	/*
 	 * After XSAVEC, XTILEDATA is cleared in the xstate_bv but is set in
 	 * the xcomp_bv.
@@ -166,13 +174,13 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 	GUEST_ASSERT(!(xstate->header.xstate_bv & XFEATURE_MASK_XTILE_DATA));
 	GUEST_ASSERT((xstate->header.xcomp_bv & XFEATURE_MASK_XTILE_DATA));
 
-	GUEST_SYNC(6);
+	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE);
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == XFEATURE_MASK_XTILE_DATA);
 	set_tilecfg(amx_cfg);
 	__ldtilecfg(amx_cfg);
 	/* Trigger #NM exception */
 	__tileloadd(tiledata);
-	GUEST_SYNC(10);
+	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE | AMX_SYNC_CHECK_TILEDATA);
 
 	GUEST_DONE();
 }
@@ -180,18 +188,18 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 void guest_nm_handler(struct ex_regs *regs)
 {
 	/* Check if #NM is triggered by XFEATURE_MASK_XTILE_DATA */
-	GUEST_SYNC(7);
+	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE);
 	GUEST_ASSERT(!(get_cr0() & X86_CR0_TS));
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILE_DATA);
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == XFEATURE_MASK_XTILE_DATA);
-	GUEST_SYNC(8);
+	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE);
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILE_DATA);
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == XFEATURE_MASK_XTILE_DATA);
 	/* Clear xfd_err */
 	wrmsr(MSR_IA32_XFD_ERR, 0);
 	/* xfd=0, enable amx */
 	wrmsr(MSR_IA32_XFD, 0);
-	GUEST_SYNC(9);
+	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE);
 }
 
 int main(int argc, char *argv[])
@@ -199,11 +207,10 @@ int main(int argc, char *argv[])
 	struct kvm_regs regs1, regs2;
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
-	struct kvm_x86_state *state;
+	struct kvm_x86_state *state = NULL;
 	int xsave_restore_size;
 	vm_vaddr_t amx_cfg, tiledata, xstate;
 	struct ucall uc;
-	u32 amx_offset;
 	int ret;
 
 	/*
@@ -253,47 +260,35 @@ int main(int argc, char *argv[])
 			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
-			switch (uc.args[1]) {
-			case 1:
-			case 2:
-			case 3:
-			case 5:
-			case 6:
-			case 7:
-			case 8:
-				fprintf(stderr, "GUEST_SYNC(%ld)\n", uc.args[1]);
-				break;
-			case 4:
-			case 10:
-				fprintf(stderr,
-				"GUEST_SYNC(%ld), check save/restore status\n", uc.args[1]);
-
-				/* Compacted mode, get amx offset by xsave area
-				 * size subtract 8K amx size.
-				 */
-				amx_offset = xsave_restore_size - NUM_TILES*TILE_SIZE;
-				state = vcpu_save_state(vcpu);
-				void *amx_start = (void *)state->xsave + amx_offset;
-				void *tiles_data = (void *)addr_gva2hva(vm, tiledata);
-				/* Only check TMM0 register, 1 tile */
-				ret = memcmp(amx_start, tiles_data, TILE_SIZE);
-				TEST_ASSERT(ret == 0, "memcmp failed, ret=%d", ret);
-				kvm_x86_state_cleanup(state);
-				break;
-			case 9:
-				fprintf(stderr,
-				"GUEST_SYNC(%ld), #NM exception and enable amx\n", uc.args[1]);
-				break;
-			}
 			break;
 		case UCALL_DONE:
-			fprintf(stderr, "UCALL_DONE\n");
 			goto done;
 		default:
 			TEST_FAIL("Unknown ucall %lu", uc.cmd);
 		}
 
-		state = vcpu_save_state(vcpu);
+		if (uc.args[1] & AMX_SYNC_SAVE)
+			state = vcpu_save_state(vcpu);
+
+		if (uc.args[1] & AMX_SYNC_CHECK_TILEDATA) {
+			/* Compacted mode, get amx offset by xsave area
+			 * size subtract 8K amx size.
+			 */
+			u32 amx_offset = xsave_restore_size - NUM_TILES*TILE_SIZE;
+			void *amx_start = (void *)state->xsave + amx_offset;
+			void *tiles_data = (void *)addr_gva2hva(vm, tiledata);
+
+			/* Only check TMM0 register, 1 tile */
+			ret = memcmp(amx_start, tiles_data, TILE_SIZE);
+			TEST_ASSERT(ret == 0, "memcmp failed, ret=%d", ret);
+		}
+
+
+		if (!(uc.args[1] & AMX_SYNC_RESTORE))
+			continue;
+
+		TEST_ASSERT(state, "RESTORE without a SAVE");
+
 		memset(&regs1, 0, sizeof(regs1));
 		vcpu_regs_get(vcpu, &regs1);
 
@@ -303,6 +298,7 @@ int main(int argc, char *argv[])
 		vcpu = vm_recreate_with_one_vcpu(vm);
 		vcpu_load_state(vcpu, state);
 		kvm_x86_state_cleanup(state);
+		state = NULL;
 
 		memset(&regs2, 0, sizeof(regs2));
 		vcpu_regs_get(vcpu, &regs2);

base-commit: d5228761ade7dda4bd54d273374041c15041c29e
-- 
2.52.0.351.gbe84eed79e-goog


[-- Attachment #3: 0002-KVM-selftests-Extend-AMX-test-to-set-XFD-with-active.patch --]
[-- Type: text/x-diff, Size: 1731 bytes --]

From e4dac37229beede8658fccffd6600e7d20c55a04 Mon Sep 17 00:00:00 2001
From: Sean Christopherson <seanjc@google.com>
Date: Mon, 29 Dec 2025 15:05:46 -0800
Subject: [PATCH 2/4] KVM: selftests: Extend AMX test to set XFD with active
 XTILE data

Re-load tile data in the AMX test before setting XFD, e.g. to verify that
KVM doesn't try to state for a disabled component.

Explicitly release tile data before performing the #NM test to minimize
the chances of a false pass, and to verify that TILERELEASE can execute
even if XFD[18]=1.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/x86/amx_test.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86/amx_test.c b/tools/testing/selftests/kvm/x86/amx_test.c
index 3bcb10f26a70..ab6d8748d7b8 100644
--- a/tools/testing/selftests/kvm/x86/amx_test.c
+++ b/tools/testing/selftests/kvm/x86/amx_test.c
@@ -162,6 +162,11 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 	GUEST_ASSERT(!(xstate->header.xstate_bv & XFEATURE_MASK_XTILE_DATA));
 	GUEST_ASSERT(xstate->header.xcomp_bv & XFEATURE_MASK_XTILE_DATA);
 
+	set_tilecfg(amx_cfg);
+	__ldtilecfg(amx_cfg);
+	__tileloadd(tiledata);
+	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE | AMX_SYNC_CHECK_TILEDATA);
+
 	/* xfd=0x40000, disable amx tiledata */
 	wrmsr(MSR_IA32_XFD, XFEATURE_MASK_XTILE_DATA);
 
@@ -176,6 +181,7 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 
 	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE);
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == XFEATURE_MASK_XTILE_DATA);
+	__tilerelease();
 	set_tilecfg(amx_cfg);
 	__ldtilecfg(amx_cfg);
 	/* Trigger #NM exception */
-- 
2.52.0.351.gbe84eed79e-goog


[-- Attachment #4: 0003-KVM-selftests-Add-test-to-verify-KVM-allows-loading-.patch --]
[-- Type: text/x-diff, Size: 3640 bytes --]

From 9f69a3c7912b3ab855567960f9a574b98355cfba Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Mon, 29 Dec 2025 10:45:43 -0800
Subject: [PATCH 3/4] KVM: selftests: Add test to verify KVM allows loading
 XTILE data with XFD=1

Extend the AMX test to verify that loading guest state via KVM_SET_XSAVE
for a disabled component, i.e. with XSTATE_BV[i]=1 and XFD[i]=1, doesn't
cause KVM to explode.  To load the "bad" state, load XSAVE state from a
snapshot taken before setting XFD.  Take care to restore *only* XSAVE
state, as restoring GPRs will corrupt the guest and restoring MSRs will
make the test useless (because the test would revert to XFD=0).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/x86/amx_test.c | 44 ++++++++++++++++++----
 1 file changed, 37 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/amx_test.c b/tools/testing/selftests/kvm/x86/amx_test.c
index ab6d8748d7b8..1d5fffddc625 100644
--- a/tools/testing/selftests/kvm/x86/amx_test.c
+++ b/tools/testing/selftests/kvm/x86/amx_test.c
@@ -128,6 +128,7 @@ enum amx_test_syncs {
 	AMX_SYNC_SAVE 		= BIT(0),
 	AMX_SYNC_RESTORE	= BIT(1),
 	AMX_SYNC_CHECK_TILEDATA	= BIT(2),
+	AMX_SYNC_RESTORE_XSTATE	= BIT(3),
 
 	AMX_SYNC_SAVE_RESTORE = AMX_SYNC_SAVE | AMX_SYNC_RESTORE,
 };
@@ -165,11 +166,19 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 	set_tilecfg(amx_cfg);
 	__ldtilecfg(amx_cfg);
 	__tileloadd(tiledata);
-	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE | AMX_SYNC_CHECK_TILEDATA);
+
+	/* Save state and check tile data, but don't restore just yet. */
+	GUEST_SYNC(AMX_SYNC_SAVE | AMX_SYNC_CHECK_TILEDATA);
 
 	/* xfd=0x40000, disable amx tiledata */
 	wrmsr(MSR_IA32_XFD, XFEATURE_MASK_XTILE_DATA);
 
+	/*
+	 * Restore the previously saved XSTATE so that the host tries setting
+	 * tiledata while guest XFD is set.
+	 */
+	GUEST_SYNC(AMX_SYNC_RESTORE_XSTATE);
+
 	/*
 	 * XTILEDATA is cleared in xstate_bv but set in xcomp_bv, this property
 	 * remains the same even when amx tiledata is disabled by IA32_XFD.
@@ -289,20 +298,41 @@ int main(int argc, char *argv[])
 			TEST_ASSERT(ret == 0, "memcmp failed, ret=%d", ret);
 		}
 
-
-		if (!(uc.args[1] & AMX_SYNC_RESTORE))
+		if (!(uc.args[1] & (AMX_SYNC_RESTORE | AMX_SYNC_RESTORE_XSTATE)))
 			continue;
 
 		TEST_ASSERT(state, "RESTORE without a SAVE");
 
+		TEST_ASSERT(!(uc.args[1] & AMX_SYNC_RESTORE) ||
+			    !(uc.args[1] & AMX_SYNC_RESTORE_XSTATE),
+			    "Only one type of restore is supported per sync");
+
 		memset(&regs1, 0, sizeof(regs1));
 		vcpu_regs_get(vcpu, &regs1);
 
-		kvm_vm_release(vm);
+		/*
+		 * Restore XSTATE from a previous snapshot, e.g. to verify that
+		 * KVM allows loading XTILE data when it's disabled via XFD.
+		 */
+		if (uc.args[1] & AMX_SYNC_RESTORE_XSTATE)
+			vcpu_xsave_set(vcpu, state->xsave);
+
+		if (uc.args[1] & AMX_SYNC_RESTORE) {
+			/*
+			 * Restoring *all* state from a previous snapshot will
+			 * corrupt the guest, e.g. GPRs and RIP, and send it
+			 * into the weeds.
+			 */
+			TEST_ASSERT(uc.args[1] & AMX_SYNC_SAVE,
+				    "Restoring an old snapshot will corrupt the guest");
+
+			kvm_vm_release(vm);
+
+			/* Restore state in a new VM.  */
+			vcpu = vm_recreate_with_one_vcpu(vm);
+			vcpu_load_state(vcpu, state);
+		}
 
-		/* Restore state in a new VM.  */
-		vcpu = vm_recreate_with_one_vcpu(vm);
-		vcpu_load_state(vcpu, state);
 		kvm_x86_state_cleanup(state);
 		state = NULL;
 
-- 
2.52.0.351.gbe84eed79e-goog


[-- Attachment #5: 0004-KVM-selftests-Verify-TILELOADD-actually-NM-faults-wh.patch --]
[-- Type: text/x-diff, Size: 2910 bytes --]

From 9fc3309dbdf202e2bfaf7c79cbb608232d34e480 Mon Sep 17 00:00:00 2001
From: Sean Christopherson <seanjc@google.com>
Date: Mon, 29 Dec 2025 12:23:30 -0800
Subject: [PATCH 4/4] KVM: selftests: Verify TILELOADD actually #NM faults when
 XFD[18]=1

Rework the AMX test's #NM handling to use kvm_asm_safe() to verify an #NM
actually occurs.  As is, a completely missing #NM could go unnoticed.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/x86/amx_test.c | 29 ++++++++++++++--------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/amx_test.c b/tools/testing/selftests/kvm/x86/amx_test.c
index 1d5fffddc625..09c0e3440f0e 100644
--- a/tools/testing/selftests/kvm/x86/amx_test.c
+++ b/tools/testing/selftests/kvm/x86/amx_test.c
@@ -69,6 +69,12 @@ static inline void __tileloadd(void *tile)
 		     : : "a"(tile), "d"(0));
 }
 
+static inline int tileloadd_safe(void *tile)
+{
+	return kvm_asm_safe(".byte 0xc4,0xe2,0x7b,0x4b,0x04,0x10",
+			    "a"(tile), "d"(0));
+}
+
 static inline void __tilerelease(void)
 {
 	asm volatile(".byte 0xc4, 0xe2, 0x78, 0x49, 0xc0" ::);
@@ -137,6 +143,8 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 						    struct tile_data *tiledata,
 						    struct xstate *xstate)
 {
+	int vector;
+
 	GUEST_ASSERT(this_cpu_has(X86_FEATURE_XSAVE) &&
 		     this_cpu_has(X86_FEATURE_OSXSAVE));
 	check_xtile_info();
@@ -193,16 +201,13 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 	__tilerelease();
 	set_tilecfg(amx_cfg);
 	__ldtilecfg(amx_cfg);
-	/* Trigger #NM exception */
-	__tileloadd(tiledata);
-	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE | AMX_SYNC_CHECK_TILEDATA);
 
-	GUEST_DONE();
-}
+	/* Verify TILELOADD gets #NM when XTILE_DATA is disabled via XFD. */
+	vector = tileloadd_safe(tiledata);
+	__GUEST_ASSERT(vector == NM_VECTOR,
+		       "Wanted #NM on tileloadd with XFD[18]=1, got %s",
+		       ex_str(vector));
 
-void guest_nm_handler(struct ex_regs *regs)
-{
-	/* Check if #NM is triggered by XFEATURE_MASK_XTILE_DATA */
 	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE);
 	GUEST_ASSERT(!(get_cr0() & X86_CR0_TS));
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILE_DATA);
@@ -215,6 +220,11 @@ void guest_nm_handler(struct ex_regs *regs)
 	/* xfd=0, enable amx */
 	wrmsr(MSR_IA32_XFD, 0);
 	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE);
+
+	__tileloadd(tiledata);
+	GUEST_SYNC(AMX_SYNC_SAVE_RESTORE | AMX_SYNC_CHECK_TILEDATA);
+
+	GUEST_DONE();
 }
 
 int main(int argc, char *argv[])
@@ -250,9 +260,6 @@ int main(int argc, char *argv[])
 
 	vcpu_regs_get(vcpu, &regs1);
 
-	/* Register #NM handler */
-	vm_install_exception_handler(vm, NM_VECTOR, guest_nm_handler);
-
 	/* amx cfg for guest_code */
 	amx_cfg = vm_vaddr_alloc_page(vm);
 	memset(addr_gva2hva(vm, amx_cfg), 0x0, getpagesize());
-- 
2.52.0.351.gbe84eed79e-goog

Re: [PATCH 2/5] x86, fpu: separate fpstate->xfd and guest XFD

[Sean Christopherson]

On Tue, Dec 30, 2025, Paolo Bonzini wrote:
> On Mon, Dec 29, 2025 at 11:45 PM Sean Christopherson <seanjc@google.com> wrote:
> > So, given that KVM's effective ABI is to record XSTATE_BV[i]=0 if XFD[i]==1, I
> > vote to fix this by emulating that behavior when stuffing XFD in
> > fpu_update_guest_xfd(), and then manually closing the hole Paolo found in
> > fpu_copy_uabi_to_guest_fpstate().
> 
> I disagree with changing the argument from const void* to void*.
> Let's instead treat it as a KVM backwards-compatibility quirk:
> 
>     union fpregs_state *xstate =
>         (union fpregs_state *)guest_xsave->region;
>     xstate->xsave.header.xfeatures &=
>         ~vcpu->arch.guest_fpu.fpstate->xfd;
> 
> It keeps the kernel/ API const as expected and if anything I'd
> consider adding a WARN to fpu_copy_uabi_to_guest_fpstate(), basically
> asserting that there would be no #NM on the subsequent restore.

Works for me.  

> > @@ -319,10 +319,25 @@ 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)
> >  {
> > +       struct fpstate *fpstate = guest_fpu->fpstate;
> > +
> >         fpregs_lock();
> > -       guest_fpu->fpstate->xfd = xfd;
> > -       if (guest_fpu->fpstate->in_use)
> > -               xfd_update_state(guest_fpu->fpstate);
> > +       fpstate->xfd = xfd;
> > +       if (fpstate->in_use)
> > +               xfd_update_state(fpstate);
> > +
> > +       /*
> > +        * If the guest's FPU state is NOT resident in hardware, clear disabled
> > +        * components in XSTATE_BV as attempting to load disabled components
> > +        * will generate #NM _in the host_, and KVM's ABI is that saving guest
> > +        * XSAVE state should see XSTATE_BV[i]=0 if XFD[i]=1.
> > +        *
> > +        * If the guest's FPU state is in hardware, simply do nothing as XSAVE
> > +        * itself saves XSTATE_BV[i] as 0 if XFD[i]=1.
> 
> s/saves/(from fpu_swap_kvm_fpstate) will save/
> 
> > +        */
> > +       if (xfd && test_thread_flag(TIF_NEED_FPU_LOAD))
> > +               fpstate->regs.xsave.header.xfeatures &= ~xfd;
> 
> No objections to this part.  I'll play with this to adjust the
> selftests either tomorrow or, more likely, on January 2nd, and send a
> v2 that also includes the change from preemption_disabled to
> irqs_disabled.

To hopefully save you some time, I responded to the selftests with cleanups and
adjustments to hit both bugs (see patch 3).

> I take it that you don't have any qualms with the new
> fpu_load_guest_fpstate function,

Hmm, I don't have a strong opinion?  Actually, after looking at patch 5, I agree
that adding fpu_load_guest_fpstate() is useful.  My only hesitation was that
kvm_fpu_{get,put}() would be _very_ similar, but critically different, at which
point NOT using fpu_update_guest_xfd() in kvm_fpu_get() could be confusing.

> but let me know if you prefer to have it in a separate submission destined to
> 6.20 only.

I'd say don't send it to stable@, otherwise I don't have a preference on 6.19
versus 6.20.

Re: [PATCH 5/5] KVM: x86: kvm_fpu_get() is fpregs_lock_and_load()

[Sean Christopherson]

I'd prefer a shortlog that states what change is being made, but otherwise:

Reviewed-by: Sean Christopherson <seanjc@google.com>

On Wed, Dec 24, 2025, Paolo Bonzini wrote:
> The only difference is the usage of switch_fpu_return() vs.
> fpregs_restore_userregs().  In turn, these are only different
> if there is no FPU at all, but KVM requires one.  Therefore use the
> pre-made export---the code is simpler and there is no functional change.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kernel/fpu/core.c | 2 +--
>  arch/x86/kvm/fpu.h         | 6 +-----
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
> index ff17c96d290a..6571952c6ef1 100644
> --- a/arch/x86/kernel/fpu/core.c
> +++ b/arch/x86/kernel/fpu/core.c
> @@ -846,7 +846,6 @@ void switch_fpu_return(void)
>  
>  	fpregs_restore_userregs();
>  }
> -EXPORT_SYMBOL_FOR_KVM(switch_fpu_return);
>  
>  void fpregs_lock_and_load(void)
>  {
> @@ -865,6 +864,7 @@ void fpregs_lock_and_load(void)
>  
>  	fpregs_assert_state_consistent();
>  }
> +EXPORT_SYMBOL_FOR_KVM(fpregs_lock_and_load);
>  
>  void fpu_load_guest_fpstate(struct fpu_guest *gfpu)
>  {
> @@ -899,7 +899,6 @@ void fpregs_assert_state_consistent(void)
>  
>  	WARN_ON_FPU(!fpregs_state_valid(fpu, smp_processor_id()));
>  }
> -EXPORT_SYMBOL_FOR_KVM(fpregs_assert_state_consistent);
>  #endif
>  
>  void fpregs_mark_activate(void)
> diff --git a/arch/x86/kvm/fpu.h b/arch/x86/kvm/fpu.h
> index f898781b6a06..b6a03d8fa8af 100644
> --- a/arch/x86/kvm/fpu.h
> +++ b/arch/x86/kvm/fpu.h
> @@ -149,11 +149,7 @@ static inline void _kvm_write_mmx_reg(int reg, const u64 *data)
>  
>  static inline void kvm_fpu_get(void)
>  {
> -	fpregs_lock();
> -
> -	fpregs_assert_state_consistent();
> -	if (test_thread_flag(TIF_NEED_FPU_LOAD))
> -		switch_fpu_return();
> +	fpregs_lock_and_load();
>  }
>  
>  static inline void kvm_fpu_put(void)
> -- 
> 2.52.0
>