[v2] x86, fpu/kvm: fix crash with AMX

[PATCH 2/4] selftests: kvm: replace numbered sync points with actions

Posted by Paolo Bonzini 1 month, 1 week ago

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).

Using named actions not only makes it easier to extend the test without
having to shift all sync point numbers, it makes the code easier to read.

[Commit message by Sean Christopherson]

Cc: stable@vger.kernel.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	I wrote this before seeing your patch... It's obviously
	similar but different enough that I kept my version. :)
	Thanks anyway for including it, your commit message was
	better so I used it.

 tools/testing/selftests/kvm/x86/amx_test.c | 88 +++++++++++-----------
 1 file changed, 43 insertions(+), 45 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/amx_test.c b/tools/testing/selftests/kvm/x86/amx_test.c
index f4ce5a185a7d..4ac41c1a7255 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 {
+	/* Check TMM0 against tiledata */
+	TEST_COMPARE_TILEDATA = 1,
+
+	/* Full VM save/restore */
+	TEST_SAVE_RESTORE = 2,
+};
+
 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(TEST_SAVE_RESTORE);
 
 	/* xfd=0, enable amx */
 	wrmsr(MSR_IA32_XFD, 0);
-	GUEST_SYNC(2);
+	GUEST_SYNC(TEST_SAVE_RESTORE);
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == 0);
 	set_tilecfg(amx_cfg);
 	__ldtilecfg(amx_cfg);
-	GUEST_SYNC(3);
+	GUEST_SYNC(TEST_SAVE_RESTORE);
 	/* Check save/restore when trap to userspace */
 	__tileloadd(tiledata);
-	GUEST_SYNC(4);
+	GUEST_SYNC(TEST_COMPARE_TILEDATA | TEST_SAVE_RESTORE);
 	__tilerelease();
-	GUEST_SYNC(5);
+	GUEST_SYNC(TEST_SAVE_RESTORE);
 	/*
 	 * After XSAVEC, XTILEDATA is cleared in the xstate_bv but is set in
 	 * the xcomp_bv.
@@ -154,6 +162,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 +176,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(TEST_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(TEST_COMPARE_TILEDATA | TEST_SAVE_RESTORE);
 
 	GUEST_DONE();
 }
@@ -180,18 +190,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(TEST_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(TEST_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(TEST_SAVE_RESTORE);
 }
 
 int main(int argc, char *argv[])
@@ -244,6 +254,7 @@ int main(int argc, char *argv[])
 	memset(addr_gva2hva(vm, xstate), 0, PAGE_SIZE * DIV_ROUND_UP(XSAVE_SIZE, PAGE_SIZE));
 	vcpu_args_set(vcpu, 3, amx_cfg, tiledata, xstate);
 
+	int iter = 0;
 	for (;;) {
 		vcpu_run(vcpu);
 		TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
@@ -253,20 +264,9 @@ 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]);
+			++iter;
+			if (uc.args[1] & TEST_COMPARE_TILEDATA) {
+				fprintf(stderr, "GUEST_SYNC #%d, check TMM0 contents\n", iter);
 
 				/* Compacted mode, get amx offset by xsave area
 				 * size subtract 8K amx size.
@@ -279,11 +279,25 @@ int main(int argc, char *argv[])
 				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;
+			}
+			if (uc.args[1] & TEST_SAVE_RESTORE) {
+				fprintf(stderr, "GUEST_SYNC #%d, save/restore VM state\n", iter);
+				state = vcpu_save_state(vcpu);
+				memset(&regs1, 0, sizeof(regs1));
+				vcpu_regs_get(vcpu, &regs1);
+
+				kvm_vm_release(vm);
+
+				/* Restore state in a new VM.  */
+				vcpu = vm_recreate_with_one_vcpu(vm);
+				vcpu_load_state(vcpu, state);
+				kvm_x86_state_cleanup(state);
+
+				memset(&regs2, 0, sizeof(regs2));
+				vcpu_regs_get(vcpu, &regs2);
+				TEST_ASSERT(!memcmp(&regs1, &regs2, sizeof(regs2)),
+					    "Unexpected register values after vcpu_load_state; rdi: %lx rsi: %lx",
+					    (ulong) regs2.rdi, (ulong) regs2.rsi);
 			}
 			break;
 		case UCALL_DONE:
@@ -293,22 +307,6 @@ int main(int argc, char *argv[])
 			TEST_FAIL("Unknown ucall %lu", uc.cmd);
 		}
 
-		state = vcpu_save_state(vcpu);
-		memset(&regs1, 0, sizeof(regs1));
-		vcpu_regs_get(vcpu, &regs1);
-
-		kvm_vm_release(vm);
-
-		/* Restore state in a new VM.  */
-		vcpu = vm_recreate_with_one_vcpu(vm);
-		vcpu_load_state(vcpu, state);
-		kvm_x86_state_cleanup(state);
-
-		memset(&regs2, 0, sizeof(regs2));
-		vcpu_regs_get(vcpu, &regs2);
-		TEST_ASSERT(!memcmp(&regs1, &regs2, sizeof(regs2)),
-			    "Unexpected register values after vcpu_load_state; rdi: %lx rsi: %lx",
-			    (ulong) regs2.rdi, (ulong) regs2.rsi);
 	}
 done:
 	kvm_vm_free(vm);
-- 
2.52.0

Re: [PATCH 2/4] selftests: kvm: replace numbered sync points with actions

Posted by Sean Christopherson 1 month ago

On Thu, Jan 01, 2026, Paolo Bonzini wrote:
> 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).
> 
> Using named actions not only makes it easier to extend the test without
> having to shift all sync point numbers, it makes the code easier to read.
> 
> [Commit message by Sean Christopherson]
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 	I wrote this before seeing your patch... It's obviously
> 	similar but different enough that I kept my version. :)

Heh, no worries.

> @@ -244,6 +254,7 @@ int main(int argc, char *argv[])
>  	memset(addr_gva2hva(vm, xstate), 0, PAGE_SIZE * DIV_ROUND_UP(XSAVE_SIZE, PAGE_SIZE));
>  	vcpu_args_set(vcpu, 3, amx_cfg, tiledata, xstate);
>  
> +	int iter = 0;

If we want to retain "tracing" of guest syncs, I vote to provide the information
from the guest, otherwise I'll end up counting GUEST_SYNC() calls on my fingers
(and run out of fingers) :-D.

E.g. if we wrap all GUEST_SYNC() calls in a macro, we can print the line number
without having to hardcode sync point numbers.

# ./x86/amx_test 
Random seed: 0x6b8b4567
GUEST_SYNC line 164, save/restore VM state
GUEST_SYNC line 168, save/restore VM state
GUEST_SYNC line 172, save/restore VM state
GUEST_SYNC line 175, save tiledata
GUEST_SYNC line 175, check TMM0 contents
GUEST_SYNC line 175, save/restore VM state
GUEST_SYNC line 181, before KVM_SET_XSAVE
GUEST_SYNC line 181, after KVM_SET_XSAVE
GUEST_SYNC line 182, save/restore VM state
GUEST_SYNC line 186, save/restore VM state
GUEST_SYNC line 210, save/restore VM state
GUEST_SYNC line 224, save/restore VM state
GUEST_SYNC line 231, save/restore VM state
GUEST_SYNC line 234, check TMM0 contents
GUEST_SYNC line 234, save/restore VM state
UCALL_DONE

---
 tools/testing/selftests/kvm/x86/amx_test.c | 55 +++++++++++++---------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/amx_test.c b/tools/testing/selftests/kvm/x86/amx_test.c
index 37b166260ee3..9593ecd47d28 100644
--- a/tools/testing/selftests/kvm/x86/amx_test.c
+++ b/tools/testing/selftests/kvm/x86/amx_test.c
@@ -131,19 +131,27 @@ static void set_tilecfg(struct tile_config *cfg)
 }
 
 enum {
+	TEST_SYNC_LINE_NUMBER_MASK = GENMASK(15, 0),
+
 	/* Retrieve TMM0 from guest, stash it for TEST_RESTORE_TILEDATA */
-	TEST_SAVE_TILEDATA = 1,
+	TEST_SAVE_TILEDATA = BIT(16),
 
 	/* Check TMM0 against tiledata */
-	TEST_COMPARE_TILEDATA = 2,
+	TEST_COMPARE_TILEDATA = BIT(17),
 
 	/* Restore TMM0 from earlier save */
-	TEST_RESTORE_TILEDATA = 4,
+	TEST_RESTORE_TILEDATA = BIT(18),
 
 	/* Full VM save/restore */
-	TEST_SAVE_RESTORE = 8,
+	TEST_SAVE_RESTORE = BIT(19),
 };
 
+#define AMX_GUEST_SYNC(action)						\
+do {									\
+	kvm_static_assert(!((action) & TEST_SYNC_LINE_NUMBER_MASK));	\
+	GUEST_SYNC((action) | __LINE__);				\
+} while (0)
+
 static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 						    struct tile_data *tiledata,
 						    struct xstate *xstate)
@@ -153,29 +161,29 @@ 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(TEST_SAVE_RESTORE);
+	AMX_GUEST_SYNC(TEST_SAVE_RESTORE);
 
 	/* xfd=0, enable amx */
 	wrmsr(MSR_IA32_XFD, 0);
-	GUEST_SYNC(TEST_SAVE_RESTORE);
+	AMX_GUEST_SYNC(TEST_SAVE_RESTORE);
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == 0);
 	set_tilecfg(amx_cfg);
 	__ldtilecfg(amx_cfg);
-	GUEST_SYNC(TEST_SAVE_RESTORE);
+	AMX_GUEST_SYNC(TEST_SAVE_RESTORE);
 	/* Check save/restore when trap to userspace */
 	__tileloadd(tiledata);
-	GUEST_SYNC(TEST_SAVE_TILEDATA | TEST_COMPARE_TILEDATA | TEST_SAVE_RESTORE);
+	AMX_GUEST_SYNC(TEST_SAVE_TILEDATA | TEST_COMPARE_TILEDATA | TEST_SAVE_RESTORE);
 
 	/* xfd=0x40000, disable amx tiledata */
 	wrmsr(MSR_IA32_XFD, XFEATURE_MASK_XTILE_DATA);
 
 	/* host tries setting tiledata while guest XFD is set */
-	GUEST_SYNC(TEST_RESTORE_TILEDATA);
-	GUEST_SYNC(TEST_SAVE_RESTORE);
+	AMX_GUEST_SYNC(TEST_RESTORE_TILEDATA);
+	AMX_GUEST_SYNC(TEST_SAVE_RESTORE);
 
 	wrmsr(MSR_IA32_XFD, 0);
 	__tilerelease();
-	GUEST_SYNC(TEST_SAVE_RESTORE);
+	AMX_GUEST_SYNC(TEST_SAVE_RESTORE);
 	/*
 	 * After XSAVEC, XTILEDATA is cleared in the xstate_bv but is set in
 	 * the xcomp_bv.
@@ -199,7 +207,7 @@ 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(TEST_SAVE_RESTORE);
+	AMX_GUEST_SYNC(TEST_SAVE_RESTORE);
 	GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == XFEATURE_MASK_XTILE_DATA);
 	set_tilecfg(amx_cfg);
 	__ldtilecfg(amx_cfg);
@@ -213,17 +221,17 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
 	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(TEST_SAVE_RESTORE);
+	AMX_GUEST_SYNC(TEST_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(TEST_SAVE_RESTORE);
+	AMX_GUEST_SYNC(TEST_SAVE_RESTORE);
 
 	__tileloadd(tiledata);
-	GUEST_SYNC(TEST_COMPARE_TILEDATA | TEST_SAVE_RESTORE);
+	AMX_GUEST_SYNC(TEST_COMPARE_TILEDATA | TEST_SAVE_RESTORE);
 
 	GUEST_DONE();
 }
@@ -275,7 +283,6 @@ int main(int argc, char *argv[])
 	memset(addr_gva2hva(vm, xstate), 0, PAGE_SIZE * DIV_ROUND_UP(XSAVE_SIZE, PAGE_SIZE));
 	vcpu_args_set(vcpu, 3, amx_cfg, tiledata, xstate);
 
-	int iter = 0;
 	for (;;) {
 		vcpu_run(vcpu);
 		TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
@@ -285,13 +292,14 @@ int main(int argc, char *argv[])
 			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
-			++iter;
 			if (uc.args[1] & TEST_SAVE_TILEDATA) {
-				fprintf(stderr, "GUEST_SYNC #%d, save tiledata\n", iter);
+				fprintf(stderr, "GUEST_SYNC line %d, save tiledata\n",
+					(u16)(uc.args[1] & TEST_SYNC_LINE_NUMBER_MASK));
 				tile_state = vcpu_save_state(vcpu);
 			}
 			if (uc.args[1] & TEST_COMPARE_TILEDATA) {
-				fprintf(stderr, "GUEST_SYNC #%d, check TMM0 contents\n", iter);
+				fprintf(stderr, "GUEST_SYNC line %d, check TMM0 contents\n",
+					(u16)(uc.args[1] & TEST_SYNC_LINE_NUMBER_MASK));
 
 				/* Compacted mode, get amx offset by xsave area
 				 * size subtract 8K amx size.
@@ -304,12 +312,15 @@ int main(int argc, char *argv[])
 				TEST_ASSERT(ret == 0, "memcmp failed, ret=%d", ret);
 			}
 			if (uc.args[1] & TEST_RESTORE_TILEDATA) {
-				fprintf(stderr, "GUEST_SYNC #%d, before KVM_SET_XSAVE\n", iter);
+				fprintf(stderr, "GUEST_SYNC line %d, before KVM_SET_XSAVE\n",
+					(u16)(uc.args[1] & TEST_SYNC_LINE_NUMBER_MASK));
 				vcpu_xsave_set(vcpu, tile_state->xsave);
-				fprintf(stderr, "GUEST_SYNC #%d, after KVM_SET_XSAVE\n", iter);
+				fprintf(stderr, "GUEST_SYNC line %d, after KVM_SET_XSAVE\n",
+					(u16)(uc.args[1] & TEST_SYNC_LINE_NUMBER_MASK));
 			}
 			if (uc.args[1] & TEST_SAVE_RESTORE) {
-				fprintf(stderr, "GUEST_SYNC #%d, save/restore VM state\n", iter);
+				fprintf(stderr, "GUEST_SYNC line %d, save/restore VM state\n",
+					(u16)(uc.args[1] & TEST_SYNC_LINE_NUMBER_MASK));
 				state = vcpu_save_state(vcpu);
 				memset(&regs1, 0, sizeof(regs1));
 				vcpu_regs_get(vcpu, &regs1);

base-commit: bc6eb58bab2fda28ef473ff06f4229c814c29380
--

Re: [PATCH 2/4] selftests: kvm: replace numbered sync points with actions

Posted by Paolo Bonzini 1 month ago

On Tue, Jan 6, 2026 at 1:02 AM Sean Christopherson <seanjc@google.com> wrote:
> > @@ -244,6 +254,7 @@ int main(int argc, char *argv[])
> >       memset(addr_gva2hva(vm, xstate), 0, PAGE_SIZE * DIV_ROUND_UP(XSAVE_SIZE, PAGE_SIZE));
> >       vcpu_args_set(vcpu, 3, amx_cfg, tiledata, xstate);
> >
> > +     int iter = 0;
>
> If we want to retain "tracing" of guest syncs, I vote to provide the information
> from the guest, otherwise I'll end up counting GUEST_SYNC() calls on my fingers
> (and run out of fingers) :-D.

I had a similar idea, but I was too lazy to implement it because for a
very linear test such as this one, "12n" in vi does wonders...

> E.g. if we wrap all GUEST_SYNC() calls in a macro, we can print the line number
> without having to hardcode sync point numbers.

... but there are actually better reasons than laziness and linearity
to keep the simple "iter++".

First, while using line numbers has the advantage of zero maintenance,
the disadvantage is that they change all the time as you're debugging.
So you are left slightly puzzled if the number changed because the
test passed or because of the extra debugging code you added.

Second, the iteration number is probably more useful to identify the
places at which the VM was reentered (which are where the iteration
number changes), than to identify the specific GUEST_SYNC that failed;
from that perspective there's not much difference between line
numbers, manually-numbered sync points, or incrementing a counter in
main().

Paolo

> # ./x86/amx_test
> Random seed: 0x6b8b4567
> GUEST_SYNC line 164, save/restore VM state
> GUEST_SYNC line 168, save/restore VM state
> GUEST_SYNC line 172, save/restore VM state
> GUEST_SYNC line 175, save tiledata
> GUEST_SYNC line 175, check TMM0 contents
> GUEST_SYNC line 175, save/restore VM state
> GUEST_SYNC line 181, before KVM_SET_XSAVE
> GUEST_SYNC line 181, after KVM_SET_XSAVE
> GUEST_SYNC line 182, save/restore VM state
> GUEST_SYNC line 186, save/restore VM state
> GUEST_SYNC line 210, save/restore VM state
> GUEST_SYNC line 224, save/restore VM state
> GUEST_SYNC line 231, save/restore VM state
> GUEST_SYNC line 234, check TMM0 contents
> GUEST_SYNC line 234, save/restore VM state
> UCALL_DONE
>
> ---
>  tools/testing/selftests/kvm/x86/amx_test.c | 55 +++++++++++++---------
>  1 file changed, 33 insertions(+), 22 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86/amx_test.c b/tools/testing/selftests/kvm/x86/amx_test.c
> index 37b166260ee3..9593ecd47d28 100644
> --- a/tools/testing/selftests/kvm/x86/amx_test.c
> +++ b/tools/testing/selftests/kvm/x86/amx_test.c
> @@ -131,19 +131,27 @@ static void set_tilecfg(struct tile_config *cfg)
>  }
>
>  enum {
> +       TEST_SYNC_LINE_NUMBER_MASK = GENMASK(15, 0),
> +
>         /* Retrieve TMM0 from guest, stash it for TEST_RESTORE_TILEDATA */
> -       TEST_SAVE_TILEDATA = 1,
> +       TEST_SAVE_TILEDATA = BIT(16),
>
>         /* Check TMM0 against tiledata */
> -       TEST_COMPARE_TILEDATA = 2,
> +       TEST_COMPARE_TILEDATA = BIT(17),
>
>         /* Restore TMM0 from earlier save */
> -       TEST_RESTORE_TILEDATA = 4,
> +       TEST_RESTORE_TILEDATA = BIT(18),
>
>         /* Full VM save/restore */
> -       TEST_SAVE_RESTORE = 8,
> +       TEST_SAVE_RESTORE = BIT(19),
>  };
>
> +#define AMX_GUEST_SYNC(action)                                         \
> +do {                                                                   \
> +       kvm_static_assert(!((action) & TEST_SYNC_LINE_NUMBER_MASK));    \
> +       GUEST_SYNC((action) | __LINE__);                                \
> +} while (0)
> +
>  static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
>                                                     struct tile_data *tiledata,
>                                                     struct xstate *xstate)
> @@ -153,29 +161,29 @@ 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(TEST_SAVE_RESTORE);
> +       AMX_GUEST_SYNC(TEST_SAVE_RESTORE);
>
>         /* xfd=0, enable amx */
>         wrmsr(MSR_IA32_XFD, 0);
> -       GUEST_SYNC(TEST_SAVE_RESTORE);
> +       AMX_GUEST_SYNC(TEST_SAVE_RESTORE);
>         GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == 0);
>         set_tilecfg(amx_cfg);
>         __ldtilecfg(amx_cfg);
> -       GUEST_SYNC(TEST_SAVE_RESTORE);
> +       AMX_GUEST_SYNC(TEST_SAVE_RESTORE);
>         /* Check save/restore when trap to userspace */
>         __tileloadd(tiledata);
> -       GUEST_SYNC(TEST_SAVE_TILEDATA | TEST_COMPARE_TILEDATA | TEST_SAVE_RESTORE);
> +       AMX_GUEST_SYNC(TEST_SAVE_TILEDATA | TEST_COMPARE_TILEDATA | TEST_SAVE_RESTORE);
>
>         /* xfd=0x40000, disable amx tiledata */
>         wrmsr(MSR_IA32_XFD, XFEATURE_MASK_XTILE_DATA);
>
>         /* host tries setting tiledata while guest XFD is set */
> -       GUEST_SYNC(TEST_RESTORE_TILEDATA);
> -       GUEST_SYNC(TEST_SAVE_RESTORE);
> +       AMX_GUEST_SYNC(TEST_RESTORE_TILEDATA);
> +       AMX_GUEST_SYNC(TEST_SAVE_RESTORE);
>
>         wrmsr(MSR_IA32_XFD, 0);
>         __tilerelease();
> -       GUEST_SYNC(TEST_SAVE_RESTORE);
> +       AMX_GUEST_SYNC(TEST_SAVE_RESTORE);
>         /*
>          * After XSAVEC, XTILEDATA is cleared in the xstate_bv but is set in
>          * the xcomp_bv.
> @@ -199,7 +207,7 @@ 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(TEST_SAVE_RESTORE);
> +       AMX_GUEST_SYNC(TEST_SAVE_RESTORE);
>         GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == XFEATURE_MASK_XTILE_DATA);
>         set_tilecfg(amx_cfg);
>         __ldtilecfg(amx_cfg);
> @@ -213,17 +221,17 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
>         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(TEST_SAVE_RESTORE);
> +       AMX_GUEST_SYNC(TEST_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(TEST_SAVE_RESTORE);
> +       AMX_GUEST_SYNC(TEST_SAVE_RESTORE);
>
>         __tileloadd(tiledata);
> -       GUEST_SYNC(TEST_COMPARE_TILEDATA | TEST_SAVE_RESTORE);
> +       AMX_GUEST_SYNC(TEST_COMPARE_TILEDATA | TEST_SAVE_RESTORE);
>
>         GUEST_DONE();
>  }
> @@ -275,7 +283,6 @@ int main(int argc, char *argv[])
>         memset(addr_gva2hva(vm, xstate), 0, PAGE_SIZE * DIV_ROUND_UP(XSAVE_SIZE, PAGE_SIZE));
>         vcpu_args_set(vcpu, 3, amx_cfg, tiledata, xstate);
>
> -       int iter = 0;
>         for (;;) {
>                 vcpu_run(vcpu);
>                 TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
> @@ -285,13 +292,14 @@ int main(int argc, char *argv[])
>                         REPORT_GUEST_ASSERT(uc);
>                         /* NOT REACHED */
>                 case UCALL_SYNC:
> -                       ++iter;
>                         if (uc.args[1] & TEST_SAVE_TILEDATA) {
> -                               fprintf(stderr, "GUEST_SYNC #%d, save tiledata\n", iter);
> +                               fprintf(stderr, "GUEST_SYNC line %d, save tiledata\n",
> +                                       (u16)(uc.args[1] & TEST_SYNC_LINE_NUMBER_MASK));
>                                 tile_state = vcpu_save_state(vcpu);
>                         }
>                         if (uc.args[1] & TEST_COMPARE_TILEDATA) {
> -                               fprintf(stderr, "GUEST_SYNC #%d, check TMM0 contents\n", iter);
> +                               fprintf(stderr, "GUEST_SYNC line %d, check TMM0 contents\n",
> +                                       (u16)(uc.args[1] & TEST_SYNC_LINE_NUMBER_MASK));
>
>                                 /* Compacted mode, get amx offset by xsave area
>                                  * size subtract 8K amx size.
> @@ -304,12 +312,15 @@ int main(int argc, char *argv[])
>                                 TEST_ASSERT(ret == 0, "memcmp failed, ret=%d", ret);
>                         }
>                         if (uc.args[1] & TEST_RESTORE_TILEDATA) {
> -                               fprintf(stderr, "GUEST_SYNC #%d, before KVM_SET_XSAVE\n", iter);
> +                               fprintf(stderr, "GUEST_SYNC line %d, before KVM_SET_XSAVE\n",
> +                                       (u16)(uc.args[1] & TEST_SYNC_LINE_NUMBER_MASK));
>                                 vcpu_xsave_set(vcpu, tile_state->xsave);
> -                               fprintf(stderr, "GUEST_SYNC #%d, after KVM_SET_XSAVE\n", iter);
> +                               fprintf(stderr, "GUEST_SYNC line %d, after KVM_SET_XSAVE\n",
> +                                       (u16)(uc.args[1] & TEST_SYNC_LINE_NUMBER_MASK));
>                         }
>                         if (uc.args[1] & TEST_SAVE_RESTORE) {
> -                               fprintf(stderr, "GUEST_SYNC #%d, save/restore VM state\n", iter);
> +                               fprintf(stderr, "GUEST_SYNC line %d, save/restore VM state\n",
> +                                       (u16)(uc.args[1] & TEST_SYNC_LINE_NUMBER_MASK));
>                                 state = vcpu_save_state(vcpu);
>                                 memset(&regs1, 0, sizeof(regs1));
>                                 vcpu_regs_get(vcpu, &regs1);
>
> base-commit: bc6eb58bab2fda28ef473ff06f4229c814c29380
> --
>

Re: [PATCH 2/4] selftests: kvm: replace numbered sync points with actions

Posted by Sean Christopherson 1 month ago

On Wed, Jan 07, 2026, Paolo Bonzini wrote:
> On Tue, Jan 6, 2026 at 1:02 AM Sean Christopherson <seanjc@google.com> wrote:
> > > @@ -244,6 +254,7 @@ int main(int argc, char *argv[])
> > >       memset(addr_gva2hva(vm, xstate), 0, PAGE_SIZE * DIV_ROUND_UP(XSAVE_SIZE, PAGE_SIZE));
> > >       vcpu_args_set(vcpu, 3, amx_cfg, tiledata, xstate);
> > >
> > > +     int iter = 0;
> >
> > If we want to retain "tracing" of guest syncs, I vote to provide the information
> > from the guest, otherwise I'll end up counting GUEST_SYNC() calls on my fingers
> > (and run out of fingers) :-D.
> 
> I had a similar idea, but I was too lazy to implement it because for a
> very linear test such as this one, "12n" in vi does wonders...
> 
> > E.g. if we wrap all GUEST_SYNC() calls in a macro, we can print the line number
> > without having to hardcode sync point numbers.
> 
> ... but there are actually better reasons than laziness and linearity
> to keep the simple "iter++".
> 
> First, while using line numbers has the advantage of zero maintenance,
> the disadvantage is that they change all the time as you're debugging.
> So you are left slightly puzzled if the number changed because the
> test passed or because of the extra debugging code you added.

True.  I'm good with the current patch.

> Second, the iteration number is probably more useful to identify the
> places at which the VM was reentered (which are where the iteration
> number changes), than to identify the specific GUEST_SYNC that failed;
> from that perspective there's not much difference between line
> numbers, manually-numbered sync points, or incrementing a counter in
> main().

[PATCH 1/4] x86/fpu: Clear XSTATE_BV[i] in save state whenever XFD[i]=1
[PATCH 2/4] selftests: kvm: replace numbered sync points with actions
[PATCH 3/4] selftests: kvm: try getting XFD and XSAVE state out of sync
[PATCH 4/4] selftests: kvm: Verify TILELOADD actually #NM faults when XFD[18]=1