[PATCH v2 0/7] Overhauling amx_test

[PATCH v2 0/7] Overhauling amx_test

[Mingwei Zhang]

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

[PATCH v2 1/7] KVM: selftests: x86: Add a working xstate data structure

[Mingwei Zhang]

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);
-- 
2.39.1.581.gbfd45094c4-goog

Re: [PATCH v2 1/7] KVM: selftests: x86: Add a working xstate data structure

[Chao Gao]

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?

Re: [PATCH v2 1/7] KVM: selftests: x86: Add a working xstate data structure

[Mingwei Zhang]

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.

[PATCH v2 2/7] KVM: selftests: x86: Fix an error in comment of amx_test

[Mingwei Zhang]

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));
-- 
2.39.1.581.gbfd45094c4-goog

Re: [PATCH v2 2/7] KVM: selftests: x86: Fix an error in comment of amx_test

[Chao Gao]

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.
>+	 */
> 	xstate->header.xstate_bv = XFEATURE_MASK_XTILEDATA;
> 	__xsavec(xstate, XFEATURE_MASK_XTILEDATA);
> 	GUEST_ASSERT(!(xstate->header.xstate_bv & XFEATURE_MASK_XTILEDATA));

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
>

Re: [PATCH v2 2/7] KVM: selftests: x86: Fix an error in comment of amx_test

[Mingwei Zhang]

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.
> >+	 */
> > 	xstate->header.xstate_bv = XFEATURE_MASK_XTILEDATA;
> > 	__xsavec(xstate, XFEATURE_MASK_XTILEDATA);
> > 	GUEST_ASSERT(!(xstate->header.xstate_bv & XFEATURE_MASK_XTILEDATA));
> 
> 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
> >

[PATCH v2 3/7] KVM: selftests: x86: Add check of CR0.TS in the #NM handler in amx_test

[Mingwei Zhang]

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);
-- 
2.39.1.581.gbfd45094c4-goog

Re: [PATCH v2 3/7] KVM: selftests: x86: Add check of CR0.TS in the #NM handler in amx_test

[Aaron Lewis]

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
>

Re: [PATCH v2 3/7] KVM: selftests: x86: Add check of CR0.TS in the #NM handler in amx_test

[Mingwei Zhang]

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
> >

[PATCH v2 4/7] KVM: selftests: Add the XFD check to IA32_XFD in #NM handler

[Mingwei Zhang]

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 */
-- 
2.39.1.581.gbfd45094c4-goog

[PATCH v2 5/7] KVM: selftests: Fix the checks to XFD_ERR using and operation

[Mingwei Zhang]

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);
-- 
2.39.1.581.gbfd45094c4-goog

[PATCH v2 6/7] KVM: selftests: x86: Enable checking on xcomp_bv in amx_test

[Mingwei Zhang]

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);
-- 
2.39.1.581.gbfd45094c4-goog

[PATCH v2 7/7] KVM: selftests: x86: Repeat the checking of xheader when IA32_XFD[XTILEDATA] is set in amx_test

[Mingwei Zhang]

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);
-- 
2.39.1.581.gbfd45094c4-goog

Re: [PATCH v2 0/7] Overhauling amx_test

[Aaron Lewis]

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/

Re: [PATCH v2 0/7] Overhauling amx_test

[Mingwei Zhang]

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.