AMX architecture involves several entities such as xstate, XCR0, IA32_XFD. This series add several missing checks on top of the existing amx_test.
v1 -> v2: - Add a working xstate data structure suggested by seanjc. - Split the checking of CR0.TS from the checking of XFD. - Fix all the issues pointed by in review.
v1: https://lore.kernel.org/all/20230110185823.1856951-1-mizhang@google.com/
Mingwei Zhang (7): KVM: selftests: x86: Fix an error in comment of amx_test KVM: selftests: x86: Add a working xstate data structure KVM: selftests: x86: Add check of CR0.TS in the #NM handler in amx_test KVM: selftests: Add the XFD check to IA32_XFD in #NM handler KVM: selftests: Fix the checks to XFD_ERR using and operation KVM: selftests: x86: Enable checking on xcomp_bv in amx_test KVM: selftests: x86: Repeat the checking of xheader when IA32_XFD[XTILEDATA] is set in amx_test
.../selftests/kvm/include/x86_64/processor.h | 12 ++++ tools/testing/selftests/kvm/x86_64/amx_test.c | 59 ++++++++++--------- 2 files changed, 43 insertions(+), 28 deletions(-)
Add a working xstate data structure for the usage of AMX and potential future usage on other xstate components. AMX selftest requires checking both the xstate_bv and xcomp_bv. Existing code relies on pointer arithmetics to fetch xstate_bv and does not support xcomp_bv.
So, add a working xstate data structure into processor.h for x86.
Suggested-by: Sean Christopherson seanjc@google.com Signed-off-by: Mingwei Zhang mizhang@google.com --- .../selftests/kvm/include/x86_64/processor.h | 12 +++++++ tools/testing/selftests/kvm/x86_64/amx_test.c | 36 ++++++------------- 2 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h index 2a5f47d51388..8110fdfd0d01 100644 --- a/tools/testing/selftests/kvm/include/x86_64/processor.h +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h @@ -45,6 +45,18 @@ #define X86_CR4_SMAP (1ul << 21) #define X86_CR4_PKE (1ul << 22)
+struct xstate_header { + u64 xstate_bv; + u64 xcomp_bv; + u64 reserved[6]; +} __attribute__((packed)); + +struct xstate { + u8 i387[512]; + struct xstate_header header; + u8 extended_state_area[0]; +} __attribute__ ((packed, aligned (64))); + /* Note, these are ordered alphabetically to match kvm_cpuid_entry2. Eww. */ enum cpuid_output_regs { KVM_CPUID_EAX, diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index bd72c6eb3b67..d506821a5a26 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -41,10 +41,6 @@
#define XSAVE_HDR_OFFSET 512
-struct xsave_data { - u8 area[XSAVE_SIZE]; -} __aligned(64); - struct tile_config { u8 palette_id; u8 start_row; @@ -103,13 +99,13 @@ static inline void __tilerelease(void) asm volatile(".byte 0xc4, 0xe2, 0x78, 0x49, 0xc0" ::); }
-static inline void __xsavec(struct xsave_data *data, uint64_t rfbm) +static inline void __xsavec(struct xstate *xstate, uint64_t rfbm) { uint32_t rfbm_lo = rfbm; uint32_t rfbm_hi = rfbm >> 32;
asm volatile("xsavec (%%rdi)" - : : "D" (data), "a" (rfbm_lo), "d" (rfbm_hi) + : : "D" (xstate), "a" (rfbm_lo), "d" (rfbm_hi) : "memory"); }
@@ -158,16 +154,6 @@ static void set_tilecfg(struct tile_config *cfg) } }
-static void set_xstatebv(void *data, uint64_t bv) -{ - *(uint64_t *)(data + XSAVE_HDR_OFFSET) = bv; -} - -static u64 get_xstatebv(void *data) -{ - return *(u64 *)(data + XSAVE_HDR_OFFSET); -} - static void init_regs(void) { uint64_t cr4, xcr0; @@ -184,7 +170,7 @@ static void init_regs(void)
static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg, struct tile_data *tiledata, - struct xsave_data *xsave_data) + struct xstate *xstate) { init_regs(); check_cpuid_xsave(); @@ -205,9 +191,9 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg, __tilerelease(); 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); + xstate->header.xstate_bv = XFEATURE_MASK_XTILEDATA; + __xsavec(xstate, XFEATURE_MASK_XTILEDATA); + GUEST_ASSERT(!(xstate->header.xstate_bv & XFEATURE_MASK_XTILEDATA));
/* xfd=0x40000, disable amx tiledata */ wrmsr(MSR_IA32_XFD, XFEATURE_MASK_XTILEDATA); @@ -244,7 +230,7 @@ int main(int argc, char *argv[]) struct kvm_run *run; struct kvm_x86_state *state; int xsave_restore_size; - vm_vaddr_t amx_cfg, tiledata, xsavedata; + vm_vaddr_t amx_cfg, tiledata, xstate; struct ucall uc; u32 amx_offset; int stage, ret; @@ -284,10 +270,10 @@ int main(int argc, char *argv[]) tiledata = vm_vaddr_alloc_pages(vm, 2); memset(addr_gva2hva(vm, tiledata), rand() | 1, 2 * getpagesize());
- /* xsave data for guest_code */ - xsavedata = vm_vaddr_alloc_pages(vm, 3); - memset(addr_gva2hva(vm, xsavedata), 0, 3 * getpagesize()); - vcpu_args_set(vcpu, 3, amx_cfg, tiledata, xsavedata); + /* XSAVE state for guest_code */ + xstate = vm_vaddr_alloc_pages(vm, DIV_ROUND_UP(XSAVE_SIZE, PAGE_SIZE)); + memset(addr_gva2hva(vm, xstate), 0, DIV_ROUND_UP(XSAVE_SIZE, PAGE_SIZE)); + vcpu_args_set(vcpu, 3, amx_cfg, tiledata, xstate);
for (stage = 1; ; stage++) { vcpu_run(vcpu);
On Tue, Feb 14, 2023 at 06:46:00PM +0000, Mingwei Zhang wrote:
- /* xsave data for guest_code */
- xsavedata = vm_vaddr_alloc_pages(vm, 3);
- memset(addr_gva2hva(vm, xsavedata), 0, 3 * getpagesize());
- vcpu_args_set(vcpu, 3, amx_cfg, tiledata, xsavedata);
- /* XSAVE state for guest_code */
- xstate = vm_vaddr_alloc_pages(vm, DIV_ROUND_UP(XSAVE_SIZE, PAGE_SIZE));
- memset(addr_gva2hva(vm, xstate), 0, DIV_ROUND_UP(XSAVE_SIZE, PAGE_SIZE));
^ this should be the size in bytes instead of in pages. Right?
On Sun, Feb 19, 2023 at 12:33 AM Chao Gao chao.gao@intel.com wrote:
On Tue, Feb 14, 2023 at 06:46:00PM +0000, Mingwei Zhang wrote:
/* xsave data for guest_code */xsavedata = vm_vaddr_alloc_pages(vm, 3);memset(addr_gva2hva(vm, xsavedata), 0, 3 * getpagesize());vcpu_args_set(vcpu, 3, amx_cfg, tiledata, xsavedata);
/* XSAVE state for guest_code */xstate = vm_vaddr_alloc_pages(vm, DIV_ROUND_UP(XSAVE_SIZE, PAGE_SIZE));memset(addr_gva2hva(vm, xstate), 0, DIV_ROUND_UP(XSAVE_SIZE, PAGE_SIZE));^ this should be the size in bytes instead of in pages. Right?
Right, thanks for catching that. I will fix it in the next version.
After the execution of __tilerelease(), AMX component will be in INIT state. Therefore, execution of XSAVEC saving the AMX state into memory will cause the xstate_bv[18] cleared in xheader. However, the xcomp_bv[18] will remain set. Fix the error in comment. Also, update xsavec() to XSAVEC because xcomp_bv[18] is set due to the instruction, not the function. Finally, use XTILEDATA instead 'bit 18' in comments.
Cc: Jim Mattson jmattson@google.com Cc: Venkatesh Srinivas venkateshs@google.com Cc: Aaron Lewis aaronlewis@google.com Signed-off-by: Mingwei Zhang mizhang@google.com --- tools/testing/selftests/kvm/x86_64/amx_test.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index d506821a5a26..aac727ff7cf8 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -190,7 +190,10 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg, GUEST_SYNC(4); __tilerelease(); GUEST_SYNC(5); - /* bit 18 not in the XCOMP_BV after xsavec() */ + /* + * After XSAVEC, XTILEDATA is cleared in the xstate_bv but is set in + * the xcomp_bv. + */ xstate->header.xstate_bv = XFEATURE_MASK_XTILEDATA; __xsavec(xstate, XFEATURE_MASK_XTILEDATA); GUEST_ASSERT(!(xstate->header.xstate_bv & XFEATURE_MASK_XTILEDATA));
On Tue, Feb 14, 2023 at 06:46:01PM +0000, Mingwei Zhang wrote:
After the execution of __tilerelease(), AMX component will be in INIT state. Therefore, execution of XSAVEC saving the AMX state into memory will cause the xstate_bv[18] cleared in xheader. However, the xcomp_bv[18] will remain set. Fix the error in comment. Also, update xsavec() to XSAVEC because xcomp_bv[18] is set due to the instruction, not the function. Finally, use XTILEDATA instead 'bit 18' in comments.
Cc: Jim Mattson jmattson@google.com Cc: Venkatesh Srinivas venkateshs@google.com Cc: Aaron Lewis aaronlewis@google.com Signed-off-by: Mingwei Zhang mizhang@google.com
tools/testing/selftests/kvm/x86_64/amx_test.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index d506821a5a26..aac727ff7cf8 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -190,7 +190,10 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg, GUEST_SYNC(4); __tilerelease(); GUEST_SYNC(5);
- /* bit 18 not in the XCOMP_BV after xsavec() */
- /*
* After XSAVEC, XTILEDATA is cleared in the xstate_bv but is set in* the xcomp_bv.*/
maybe it would be better to add another GUEST_ASSERT() to enforce that XTILEDATA is set in the xcomp_bv.
-- 2.39.1.581.gbfd45094c4-goog
On Sun, Feb 19, 2023, Chao Gao wrote:
On Tue, Feb 14, 2023 at 06:46:01PM +0000, Mingwei Zhang wrote:
After the execution of __tilerelease(), AMX component will be in INIT state. Therefore, execution of XSAVEC saving the AMX state into memory will cause the xstate_bv[18] cleared in xheader. However, the xcomp_bv[18] will remain set. Fix the error in comment. Also, update xsavec() to XSAVEC because xcomp_bv[18] is set due to the instruction, not the function. Finally, use XTILEDATA instead 'bit 18' in comments.
Cc: Jim Mattson jmattson@google.com Cc: Venkatesh Srinivas venkateshs@google.com Cc: Aaron Lewis aaronlewis@google.com Signed-off-by: Mingwei Zhang mizhang@google.com
tools/testing/selftests/kvm/x86_64/amx_test.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index d506821a5a26..aac727ff7cf8 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -190,7 +190,10 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg, GUEST_SYNC(4); __tilerelease(); GUEST_SYNC(5);
- /* bit 18 not in the XCOMP_BV after xsavec() */
- /*
* After XSAVEC, XTILEDATA is cleared in the xstate_bv but is set in* the xcomp_bv.*/maybe it would be better to add another GUEST_ASSERT() to enforce that XTILEDATA is set in the xcomp_bv.
yeah. The check has been added in the 6th patch of the series, but I think it is hard to see. I will reorder the change next to this one in the next version.
-- 2.39.1.581.gbfd45094c4-goog
Add check of CR0.TS[bit 3] before the check of IA32_XFD_ERR in the #NM handler in amx_test. This is because XFD may not be the only reason of the IA32_XFD MSR and the bitmap corresponding to the state components required by the faulting instruction." (Intel SDM vol 1. Section 13.14)
Add the missing check of CR0.TS.
Signed-off-by: Mingwei Zhang mizhang@google.com --- tools/testing/selftests/kvm/x86_64/amx_test.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index aac727ff7cf8..847752998660 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -215,6 +215,7 @@ void guest_nm_handler(struct ex_regs *regs) { /* Check if #NM is triggered by XFEATURE_MASK_XTILEDATA */ GUEST_SYNC(7); + GUEST_ASSERT((get_cr0() & X86_CR0_TS) == 0); GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILEDATA); GUEST_SYNC(8); GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILEDATA);
On Tue, Feb 14, 2023 at 6:46 PM Mingwei Zhang mizhang@google.com wrote:
Add check of CR0.TS[bit 3] before the check of IA32_XFD_ERR in the #NM handler in amx_test. This is because XFD may not be the only reason of the IA32_XFD MSR and the bitmap corresponding to the state components required by the faulting instruction." (Intel SDM vol 1. Section 13.14)
Add the missing check of CR0.TS.
Signed-off-by: Mingwei Zhang mizhang@google.com
tools/testing/selftests/kvm/x86_64/amx_test.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index aac727ff7cf8..847752998660 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -215,6 +215,7 @@ void guest_nm_handler(struct ex_regs *regs) { /* Check if #NM is triggered by XFEATURE_MASK_XTILEDATA */ GUEST_SYNC(7);
GUEST_ASSERT((get_cr0() & X86_CR0_TS) == 0);
Can't we infer that the #NM is the result of an XFD error due to the fact that IA32_XFD_ERR is set? Is this check needed? SDM vol 1, 13.14, EXTENDED FEATURE DISABLE (XFD) - Device-not-available exceptions that are not due to XFD - those resulting from setting CR0.TS to 1 - do not modify the IA32_XFD_ERR MSR.
GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILEDATA); GUEST_SYNC(8); GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILEDATA);-- 2.39.1.581.gbfd45094c4-goog
On Fri, Feb 17, 2023, Aaron Lewis wrote:
On Tue, Feb 14, 2023 at 6:46 PM Mingwei Zhang mizhang@google.com wrote:
Add check of CR0.TS[bit 3] before the check of IA32_XFD_ERR in the #NM handler in amx_test. This is because XFD may not be the only reason of the IA32_XFD MSR and the bitmap corresponding to the state components required by the faulting instruction." (Intel SDM vol 1. Section 13.14)
Add the missing check of CR0.TS.
Signed-off-by: Mingwei Zhang mizhang@google.com
tools/testing/selftests/kvm/x86_64/amx_test.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index aac727ff7cf8..847752998660 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -215,6 +215,7 @@ void guest_nm_handler(struct ex_regs *regs) { /* Check if #NM is triggered by XFEATURE_MASK_XTILEDATA */ GUEST_SYNC(7);
GUEST_ASSERT((get_cr0() & X86_CR0_TS) == 0);Can't we infer that the #NM is the result of an XFD error due to the fact that IA32_XFD_ERR is set? Is this check needed? SDM vol 1, 13.14, EXTENDED FEATURE DISABLE (XFD)
- Device-not-available exceptions that are not due to XFD - those resulting from setting CR0.TS to 1 - do not modify the IA32_XFD_ERR MSR.
We don't infer from the reasons of #NM and that is the purpose of this selftest. Yes, this looks a little bit pedantic. But still, it is worth adding the check since violation of that indicates either 1) the selftest mistakenly did not clear XFD_ERR prior to #NM or 2) hardware is broken.
GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILEDATA); GUEST_SYNC(8); GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILEDATA);-- 2.39.1.581.gbfd45094c4-goog
Add an extra check to IA32_XFD to ensure the behavior is consistent with the AMX archtecture. In addition, repeat the checks across context switch to ensure the values of IA32_XFD and IA32_XFD_ERR are well preserved.
Signed-off-by: Mingwei Zhang mizhang@google.com --- tools/testing/selftests/kvm/x86_64/amx_test.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index 847752998660..44c907215343 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -217,8 +217,10 @@ void guest_nm_handler(struct ex_regs *regs) GUEST_SYNC(7); GUEST_ASSERT((get_cr0() & X86_CR0_TS) == 0); GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILEDATA); + GUEST_ASSERT(rdmsr(MSR_IA32_XFD) & XFEATURE_MASK_XTILEDATA); GUEST_SYNC(8); GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILEDATA); + GUEST_ASSERT(rdmsr(MSR_IA32_XFD) & XFEATURE_MASK_XTILEDATA); /* Clear xfd_err */ wrmsr(MSR_IA32_XFD_ERR, 0); /* xfd=0, enable amx */
Fix the checks to XFD_ERR using logical AND operation because XFD_ERR might contain more information in the future. According Intel SDM Vol 1. 13.14:
"Specifically, the MSR is loaded with the logical AND of the IA32_XFD MSR and the bitmap corresponding to the state component(s) required by the faulting instruction."
So fix the check by using AND instead of '=='.
Signed-off-by: Mingwei Zhang mizhang@google.com --- tools/testing/selftests/kvm/x86_64/amx_test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index 44c907215343..bd8bd9936f8e 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -216,10 +216,10 @@ void guest_nm_handler(struct ex_regs *regs) /* Check if #NM is triggered by XFEATURE_MASK_XTILEDATA */ GUEST_SYNC(7); GUEST_ASSERT((get_cr0() & X86_CR0_TS) == 0); - GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILEDATA); + GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) & XFEATURE_MASK_XTILEDATA); GUEST_ASSERT(rdmsr(MSR_IA32_XFD) & XFEATURE_MASK_XTILEDATA); GUEST_SYNC(8); - GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) == XFEATURE_MASK_XTILEDATA); + GUEST_ASSERT(rdmsr(MSR_IA32_XFD_ERR) & XFEATURE_MASK_XTILEDATA); GUEST_ASSERT(rdmsr(MSR_IA32_XFD) & XFEATURE_MASK_XTILEDATA); /* Clear xfd_err */ wrmsr(MSR_IA32_XFD_ERR, 0);
After tilerelease instruction, AMX tiles are in INIT state. According to Intel SDM vol 1. 13.10: "If RFBM[i] = 1, XSTATE_BV[i] is set to the value of XINUSE[i].", XSTATE_BV[18] should be cleared after xsavec.
On the other hand, according to Intel SDM vol 1. 13.4.3: "If XCOMP_BV[i] = 1, state component i is located at a byte offset locationI from the base address of the XSAVE area". Since at the time of xsavec, XCR0[18] is set indicating AMX tile data component is still enabled, xcomp_bv[18] should be set.
Complete the checks by adding the assert to xcomp_bv[18] after xsavec.
Signed-off-by: Mingwei Zhang mizhang@google.com --- tools/testing/selftests/kvm/x86_64/amx_test.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index bd8bd9936f8e..0e4c65f9e2f2 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -197,6 +197,7 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg, xstate->header.xstate_bv = XFEATURE_MASK_XTILEDATA; __xsavec(xstate, XFEATURE_MASK_XTILEDATA); GUEST_ASSERT(!(xstate->header.xstate_bv & XFEATURE_MASK_XTILEDATA)); + GUEST_ASSERT((xstate->header.xcomp_bv & XFEATURE_MASK_XTILEDATA));
/* xfd=0x40000, disable amx tiledata */ wrmsr(MSR_IA32_XFD, XFEATURE_MASK_XTILEDATA);
Repeat the checking of AMX component in xheader after XSAVEC when IA32_XFD[XTILEDATA] is set. This check calibrates the functionality scope of IA32_XFD: it does not intercept the XSAVE state management. Regardless of the values in IA32_XFD, AMX component state will still be managed by XSAVE* and XRSTOR* as long as the corresponding bits are set XCR0.
Signed-off-by: Mingwei Zhang mizhang@google.com --- tools/testing/selftests/kvm/x86_64/amx_test.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index 0e4c65f9e2f2..d991a473dc11 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -201,6 +201,16 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
/* xfd=0x40000, disable amx tiledata */ wrmsr(MSR_IA32_XFD, XFEATURE_MASK_XTILEDATA); + + /* + * XTILEDATA is cleared in xstate_bv but set in xcomp_bv, this property + * remains the same even when amx tiledata is disabled by IA32_XFD. + */ + xstate->header.xstate_bv = XFEATURE_MASK_XTILEDATA; + __xsavec(xstate, XFEATURE_MASK_XTILEDATA); + GUEST_ASSERT(!(xstate->header.xstate_bv & XFEATURE_MASK_XTILEDATA)); + GUEST_ASSERT((xstate->header.xcomp_bv & XFEATURE_MASK_XTILEDATA)); + GUEST_SYNC(6); GUEST_ASSERT(rdmsr(MSR_IA32_XFD) == XFEATURE_MASK_XTILEDATA); set_tilecfg(amx_cfg);
On Tue, Feb 14, 2023 at 6:46 PM Mingwei Zhang mizhang@google.com wrote:
AMX architecture involves several entities such as xstate, XCR0, IA32_XFD. This series add several missing checks on top of the existing amx_test.
v1 -> v2:
- Add a working xstate data structure suggested by seanjc.
- Split the checking of CR0.TS from the checking of XFD.
- Fix all the issues pointed by in review.
v1: https://lore.kernel.org/all/20230110185823.1856951-1-mizhang@google.com/
Mingwei Zhang (7): KVM: selftests: x86: Fix an error in comment of amx_test KVM: selftests: x86: Add a working xstate data structure KVM: selftests: x86: Add check of CR0.TS in the #NM handler in amx_test KVM: selftests: Add the XFD check to IA32_XFD in #NM handler KVM: selftests: Fix the checks to XFD_ERR using and operation KVM: selftests: x86: Enable checking on xcomp_bv in amx_test KVM: selftests: x86: Repeat the checking of xheader when IA32_XFD[XTILEDATA] is set in amx_test
.../selftests/kvm/include/x86_64/processor.h | 12 ++++ tools/testing/selftests/kvm/x86_64/amx_test.c | 59 ++++++++++--------- 2 files changed, 43 insertions(+), 28 deletions(-)
-- 2.39.1.581.gbfd45094c4-goog
Would you be open to adding my series to the end of this one? That way we have one series that's overhauling amx_test.
https://lore.kernel.org/kvm/20230217215959.1569092-1-aaronlewis@google.com/
On Fri, Feb 17, 2023, Aaron Lewis wrote:
On Tue, Feb 14, 2023 at 6:46 PM Mingwei Zhang mizhang@google.com wrote:
AMX architecture involves several entities such as xstate, XCR0, IA32_XFD. This series add several missing checks on top of the existing amx_test.
v1 -> v2:
- Add a working xstate data structure suggested by seanjc.
- Split the checking of CR0.TS from the checking of XFD.
- Fix all the issues pointed by in review.
v1: https://lore.kernel.org/all/20230110185823.1856951-1-mizhang@google.com/
Mingwei Zhang (7): KVM: selftests: x86: Fix an error in comment of amx_test KVM: selftests: x86: Add a working xstate data structure KVM: selftests: x86: Add check of CR0.TS in the #NM handler in amx_test KVM: selftests: Add the XFD check to IA32_XFD in #NM handler KVM: selftests: Fix the checks to XFD_ERR using and operation KVM: selftests: x86: Enable checking on xcomp_bv in amx_test KVM: selftests: x86: Repeat the checking of xheader when IA32_XFD[XTILEDATA] is set in amx_test
.../selftests/kvm/include/x86_64/processor.h | 12 ++++ tools/testing/selftests/kvm/x86_64/amx_test.c | 59 ++++++++++--------- 2 files changed, 43 insertions(+), 28 deletions(-)
-- 2.39.1.581.gbfd45094c4-goog
Would you be open to adding my series to the end of this one? That way we have one series that's overhauling amx_test.
https://lore.kernel.org/kvm/20230217215959.1569092-1-aaronlewis@google.com/
Sure, I will integrate your changes into this series.
linux-kselftest-mirror@lists.linaro.org
-
Aaron Lewis -
Chao Gao -
Mingwei Zhang