| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aVxRAv888jsmQJ8-@google.com>
Date: Mon, 5 Jan 2026 16:02:10 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Paolo Bonzini <pbonzini@...hat.com>
Cc: linux-kernel@...r.kernel.org, kvm@...r.kernel.org, x86@...nel.org,
stable@...r.kernel.org
Subject: Re: [PATCH 2/4] selftests: kvm: replace numbered sync points with actions
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@...r.kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.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(®s1, 0, sizeof(regs1));
vcpu_regs_get(vcpu, ®s1);
base-commit: bc6eb58bab2fda28ef473ff06f4229c814c29380
--
Powered by blists - more mailing lists