Re: [PATCH 3/3] selftest: Support amx selftest

Paolo Bonzini
2021-12-21T17:36:40Z

On 12/22/21 00:15, Yang Zhong wrote:
> This selftest do two test cases, one is to trigger #NM
> exception and check MSR XFD_ERR value. Another case is
> guest load tile data into tmm0 registers and trap to host
> side to check memory data after save/restore.
>
> Signed-off-by: Yang Zhong <yang.zhong@intel.com>

This is a great start, mainly I'd add a lot more GUEST_SYNCs.

Basically any instruction after the initial GUEST_ASSERTs are a
potential point for GUEST_SYNC, except right after the call to set_tilecfg:

GUEST_SYNC(1)

> + /* xfd=0, enable amx */
> + wrmsr(MSR_IA32_XFD, 0);

GUEST_SYNC(2)

> + GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == 0);
> + set_tilecfg(amx_cfg);
> + __ldtilecfg(amx_cfg);

GUEST_SYNC(3)

> + /* Check save/restore when trap to userspace */
> + __tileloadd(tiledata);
> + GUEST_SYNC(1);

This would become 4; here add tilerelease+GUEST_SYNC(5)+XSAVEC, and
check that state 18 is not included in XCOMP_BV.

> + /* xfd=0x40000, disable amx tiledata */
> + wrmsr(MSR_IA32_XFD, XFEATURE_MASK_XTILEDATA);

GUEST_SYNC(6)

> + GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == XFEATURE_MASK_XTILEDATA);
> + set_tilecfg(amx_cfg);
> + __ldtilecfg(amx_cfg);
> + /* Trigger #NM exception */
> + __tileloadd(tiledata);

GUEST_SYNC(10); this final GUEST_SYNC should also check TMM0 in the host.

> + GUEST_DONE();
> +}
> +
> +void guest_nm_handler(struct ex_regs *regs)
> +{
> + /* Check if #NM is triggered by XFEATURE_MASK_XTILEDATA */

GUEST_SYNC(7)

> + GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILEDATA);
> + /* Clear xfd_err */

Same here, I'd do a GUEST_SYNC(8) and re-read MSR_IA32_XFD_ERR.

> + wrmsr(MSR_IA32_XFD_ERR, 0);
> + GUEST_SYNC(2);

This becomes GUEST_SYNC(9).

> +}

> + case UCALL_SYNC:
> + switch (uc.args[1]) {
> + case 1:
> + fprintf(stderr,
> + "Exit VM by GUEST_SYNC(1), check save/restore.\n");
> +
> + /* 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(vm, VCPU_ID);
> + 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\n", ret);
> + kvm_x86_state_cleanup(state);
> + break;

All GUEST_SYNCs should do save_state/load_state like state_test.c. Then
of course you can *also* check TMM0 after __tileloadd, which would be
cases 4 and 10.

Thanks,

Paolo

> + case 2:
> + fprintf(stderr,
> + "Exit VM by GUEST_SYNC(2), generate #NM exception.\n");
> + goto done;
> + }
> + break;
> + case UCALL_DONE:
> + goto done;
> + default:
> + TEST_FAIL("Unknown ucall %lu", uc.cmd);
> + }
> + }
> +done:
> + kvm_vm_free(vm);
> +}
>