On Tue, Dec 21, 2021 at 06:36:17PM +0100, Paolo Bonzini wrote:
> 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@xxxxxxxxx>
>
> 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)
Paolo, with this GUEST_SYNC(3) between __ldtilecfg and __tileloadd
instructions, the guest code generate (vector:0x6, Invalid Opcode)
exception, which is a strange issue. thanks!
Yang
>
> >+ /* 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.
>
Thanks Paolo, the new code like below:
GUEST_SYNC(5);
/* bit 18 not in the XCOMP_BV after xsavec() */
set_xstatebv(xsave_data, XFEATURE_MASK_XTILEDATA);
__xsavec(xsave_data, XFEATURE_MASK_XTILEDATA);
GUEST_ASSERT((get_xstatebv(xsave_data) & XFEATURE_MASK_XTILEDATA) == 0)
Yang
> >+ /* 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 need add regs->rip +=3 to skip __tileloadd() instruction(generate #NM)
when VM resume from GUEST_SYNC(9).
Thanks!
Yang
> 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.
>
Yes, the new code will add save_state/load_state as in the state_test.c file.
Thanks!
Yang
> 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);
> >+}
> >
