| 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: <CABgObfZSchPMdqSvvVPgy9s5-TkHHZpLPHNYSsK-YHRye0SAaw@mail.gmail.com>
Date: Wed, 7 Jan 2026 23:28:07 +0100
From: Paolo Bonzini <pbonzini@...hat.com>
To: Sean Christopherson <seanjc@...gle.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 Tue, Jan 6, 2026 at 1:02 AM Sean Christopherson <seanjc@...gle.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(®s1, 0, sizeof(regs1));
> vcpu_regs_get(vcpu, ®s1);
>
> base-commit: bc6eb58bab2fda28ef473ff06f4229c814c29380
> --
>
Powered by blists - more mailing lists