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

[Sean Christopherson <seanjc@google.com>]

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