In this version, I have integrated Aaron's changes to the amx_test. In addition, we also integrated one fix patch for a kernel warning due to xsave address issue.
Patch 1: Fix a host FPU kernel warning due to missing XTILEDATA in xinit.
Patch 2-8: Overhaul amx_test. These patches were basically from v2.
Patch 9-13: Overhaul amx_test from Aaron. I modified the changelog a little bit.
v2 -> v3: - integrate Aaron's 5 commits with minor changes on commit message. - Add one fix patch for a kernel warning.
v2: https://lore.kernel.org/all/20230214184606.510551-1-mizhang@google.com/
Aaron Lewis (5): KVM: selftests: x86: Assert that XTILE is XSAVE-enabled KVM: selftests: x86: Assert that both XTILE{CFG,DATA} are XSAVE-enabled KVM: selftests: x86: Remove redundant check that XSAVE is supported KVM: selftests: x86: Check that the palette table exists before using it KVM: selftests: x86: Check that XTILEDATA supports XFD
Mingwei Zhang (8): x86/fpu/xstate: Avoid getting xstate address of init_fpstate if fpstate contains the component KVM: selftests: x86: Add a working xstate data structure KVM: selftests: x86: Fix an error in comment of amx_test KVM: selftests: x86: Add check of CR0.TS in the #NM handler in amx_test KVM: selftests: x86: Add the XFD check to IA32_XFD in #NM handler KVM: selftests: x86: 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
arch/x86/kernel/fpu/xstate.c | 10 ++- .../selftests/kvm/include/x86_64/processor.h | 14 ++++ tools/testing/selftests/kvm/x86_64/amx_test.c | 80 +++++++++---------- 3 files changed, 59 insertions(+), 45 deletions(-)
Avoid getting xstate address of init_fpstate if fpstate contains the xstate component. Since XTILEDATA (bit 18) was turned off in xinit, when KVM calls __raw_xsave_addr(xinit, 18), it triggers a warning as follows.
__raw_xsave_addr() is an internal function that assume caller does the checking, ie., all function arguments should be checked before calling. So, instead of removing the WARNING, add checks in __copy_xstate_to_uabi_buf().
[ 168.814082] ------------[ cut here ]------------ [ 168.814083] WARNING: CPU: 35 PID: 15304 at arch/x86/kernel/fpu/xstate.c:934 __raw_xsave_addr+0xc8/0xe0 [ 168.814088] Modules linked in: kvm_intel dummy bridge stp llc cdc_ncm cdc_eem cdc_ether usbnet mii ehci_pci ehci_hcd vfat fat cdc_acm xhci_pci xhci_hcd idpf(O) [ 168.814100] CPU: 35 PID: 15304 Comm: amx_test Tainted: G S O 6.2.0-smp-DEV #6 [ 168.814103] RIP: 0010:__raw_xsave_addr+0xc8/0xe0 [ 168.814105] Code: 83 f9 40 72 b0 eb 10 48 63 ca 44 8b 04 8d 60 13 1e 82 eb 03 41 89 f8 44 89 c1 48 01 c8 48 83 c4 08 5d c3 cc 0f 0b 31 c0 eb f3 <0f> 0b 48 c7 c7 c7 28 11 82 e8 da 30 b0 00 31 c0 eb e1 66 0f 1f 44 [ 168.814106] RSP: 0018:ff110020ef79bc90 EFLAGS: 00010246 [ 168.814108] RAX: ffffffff821e0340 RBX: 0000000000000012 RCX: 0000000000000012 [ 168.814109] RDX: 0000000000000012 RSI: 80000000000206e7 RDI: 0000000000040000 [ 168.814110] RBP: ff110020ef79bc98 R08: 0000000000000a00 R09: 0000000000000012 [ 168.814112] R10: 0000000000000012 R11: 0000000000000004 R12: ffa00000089f2a40 [ 168.814113] R13: 0000001200000000 R14: 0000000000000012 R15: ff110020ef288b00 [ 168.814114] FS: 00007f1812761300(0000) GS:ff11003fff4c0000(0000) knlGS:0000000000000000 [ 168.814116] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 168.814117] CR2: 00007f1812555008 CR3: 0000002093a80002 CR4: 0000000000373ee0 [ 168.814118] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 168.814119] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400 [ 168.814120] Call Trace: [ 168.814121] <TASK> [ 168.814122] __copy_xstate_to_uabi_buf+0x3cb/0x520 [ 168.814125] fpu_copy_guest_fpstate_to_uabi+0x29/0x50 [ 168.814127] kvm_arch_vcpu_ioctl+0x9f7/0xee0 [ 168.814130] ? __kmem_cache_free+0x16b/0x220 [ 168.814133] kvm_vcpu_ioctl+0x47c/0x5a0 [ 168.814136] __se_sys_ioctl+0x77/0xc0 [ 168.814138] __x64_sys_ioctl+0x1d/0x20 [ 168.814139] do_syscall_64+0x3d/0x80 [ 168.814142] entry_SYSCALL_64_after_hwframe+0x63/0xcd [ 168.814146] RIP: 0033:0x7f1812892c87 [ 168.814148] Code: 5d c3 cc 48 8b 05 39 1d 07 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 cc cc cc cc cc cc cc cc cc cc b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 09 1d 07 00 f7 d8 64 89 01 48 [ 168.814149] RSP: 002b:00007ffc4cebf538 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 [ 168.814151] RAX: ffffffffffffffda RBX: 00007f1812761280 RCX: 00007f1812892c87 [ 168.814152] RDX: 00000000004dcda0 RSI: 000000009000aecf RDI: 0000000000000007 [ 168.814153] RBP: 0000000000002b00 R08: 00000000004d5010 R09: 0000000000002710 [ 168.814154] R10: 00007f1812906980 R11: 0000000000000246 R12: 00000000004d8110 [ 168.814155] R13: 0000000000000004 R14: 00000000004d78b0 R15: 0000000000000004 [ 168.814156] </TASK> [ 168.814157] ---[ end trace 0000000000000000 ]---
Fixes: e84ba47e313d ("x86/fpu: Hook up PKRU into ptrace()") Signed-off-by: Mingwei Zhang mizhang@google.com --- arch/x86/kernel/fpu/xstate.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 714166cc25f2..5cc1426c3800 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -1063,6 +1063,7 @@ void __copy_xstate_to_uabi_buf(struct membuf to, struct fpstate *fpstate, struct xregs_state *xsave = &fpstate->regs.xsave; struct xstate_header header; unsigned int zerofrom; + void *xsave_addr; u64 mask; int i;
@@ -1151,10 +1152,11 @@ void __copy_xstate_to_uabi_buf(struct membuf to, struct fpstate *fpstate, pkru.pkru = pkru_val; membuf_write(&to, &pkru, sizeof(pkru)); } else { - copy_feature(header.xfeatures & BIT_ULL(i), &to, - __raw_xsave_addr(xsave, i), - __raw_xsave_addr(xinit, i), - xstate_sizes[i]); + xsave_addr = (header.xfeatures & BIT_ULL(i)) ? + __raw_xsave_addr(xsave, i) : + __raw_xsave_addr(xinit, i); + + membuf_write(&to, xsave_addr, xstate_sizes[i]); } /* * Keep track of the last copied state in the non-compacted
Mingwei!
On Tue, Feb 21 2023 at 16:36, Mingwei Zhang wrote:
This does not parse. Aside of that it gets the ordering of the changelog wrong. It explain first what the patch is doing by repeating the way too long subject line and then tries to give some explanation about the problem.
KVM does not call __raw_xsave_addr() and the problem is completely independent of KVM.
I don't see checks added. The patch open codes copy_feature() and makes the __raw_xsave_addr() invocations conditional.
So something like this:
Subject: x86/fpu/xstate: Prevent false-positive warning in __copy_xstate_to_uabi_buf()
__copy_xstate_to_uabi_buf() copies either from the tasks XSAVE buffer or from init_fpstate into the ptrace buffer. Dynamic features, like XTILEDATA, have an all zeroes init state and are not saved in init_fpstate, which means the corresponding bit is not set in the xfeatures bitmap of the init_fpstate header.
But __copy_xstate_to_uabi_buf() retrieves addresses for both the tasks xstate and init_fpstate unconditionally via __raw_xsave_addr().
So if the tasks XSAVE buffer has a dynamic feature set, then the address retrieval for init_fpstate triggers the warning in __raw_xsave_addr() which checks the feature bit in the init_fpstate header.
Prevent this by open coding copy_feature() and making the address retrieval conditional on the tasks XSAVE header bit.
So the order here is (in paragraphs):
1) Explain the context 2) Explain whats wrong 3) Explain the consequence 4) Explain the solution briefly
It does not even need a backtrace, because the backtrace does not give any relevant information which is not in the text above already.
The actual callchain is completely irrelevant for desribing this issue. If you really want to add a backtrace, then please trim it by removing the irrelevant information. See https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backt... for further information.
So for this case this would be:
WARNING: CPU: 35 PID: 15304 at arch/x86/kernel/fpu/xstate.c:934 __raw_xsave_addr+0xc8/0xe0 Call Trace: <TASK> __copy_xstate_to_uabi_buf+0x3cb/0x520 fpu_copy_guest_fpstate_to_uabi+0x29/0x50
But even fpu_copy_guest_fpstate_to_uabi() is already useless because __copy_xstate_to_uabi_buf() has multiple callers which all have the very same problem and they are very simple to find.
Backtraces are useful to illustrate a hard to understand callchain, but having ~40 lines of backtrace which only contains two relevant lines is just a distraction.
See?
Fixes: e84ba47e313d ("x86/fpu: Hook up PKRU into ptrace()")
The problem did not exist at this point in time because dynamic xfeatures did not exist, neither in hardware nor in kernel support.
Even if dynamic features would have existed then the commit would not be the one which introduced the problem, All the commit does is to move back then valid code into a conditional code path.
It fixes:
471f0aa7fa64 ("x86/fpu: Fix copy_xstate_to_uabi() to copy init states correctly")
which attempted to fix exactly the problem you are seeing, but only addressed half of it. The underlying problem was introduced with 2308ee57d93d ("x86/fpu/amx: Enable the AMX feature in 64-bit mode")
Random fixes tags are not really helpful.
Please add a comment here to explain why this is open coded and does not use copy_feature(). Something like:
/* * Open code copy_feature() to prevent retrieval * of a dynamic features address from xinit, which * is invalid because xinit does not contain the * all zeros init states of dynamic features and * emits a warning. */
Other than that. Nice detective work!
Thanks,
tglx
On 2/21/2023 8:36 AM, Mingwei Zhang wrote:
<snip>
So this hunk is under for_each_extended_xfeature(i, mask) -- it skips the copy routine if mask[i] == 0; instead, it fills zeros.
We have this [1]:
if (fpu_state_size_dynamic()) mask &= (header.xfeatures | xinit->header.xcomp_bv);
If header.xfeatures[18] = 0 then mask[18] = 0 because xinit->header.xcomp_bv[18] = 0. Then, it won't hit that code. So, I'm confused about the problem that you described here.
Can you elaborate on your test case a bit? Let me try to reproduce the issue on my end.
Thanks, Chang
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch...
On Tue, Feb 21 2023 at 19:05, Chang S. Bae wrote:
Read the suggested changelog I wrote in my reply to Mingwei.
TLDR:
xsave.header.xfeatures[18] = 1 xinit.header.xfeatures[18] = 0 -> mask[18] = 1 -> __raw_xsave_addr(xsave, 18) <- Success -> __raw_xsave_addr(xinit, 18) <- WARN
Thanks,
tglx
Hi Thomas,
Thanks for the review and I will provide the next version separately from the series, since this one is independent from the rest.
Chang: to reproduce this issue, you can simply run the amx_test in the kvm selftest directory.
Thanks. -Mingwei
On 2/22/2023 10:40 AM, Mingwei Zhang wrote:
Oh, sigh.. This should be caught last time.
Hmm, then since we store init state for legacy ones [1], unless it is too aggressive, perhaps the loop can be simplified like this:
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 714166cc25f2..2dac6f5f3ade 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -1118,21 +1118,13 @@ void __copy_xstate_to_uabi_buf(struct membuf to, struct fpstate *fpstate, zerofrom = offsetof(struct xregs_state, extended_state_area);
/* - * The ptrace buffer is in non-compacted XSAVE format. In - * non-compacted format disabled features still occupy state space, - * but there is no state to copy from in the compacted - * init_fpstate. The gap tracking will zero these states. + * Indicate which states to copy from fpstate. When not present in + * fpstate, those extended states are either initialized or + * disabled. They are also known to have an all zeros init state. + * Thus, remove them from 'mask' to zero those features in the user + * buffer instead of retrieving them from init_fpstate. */ - mask = fpstate->user_xfeatures; - - /* - * Dynamic features are not present in init_fpstate. When they are - * in an all zeros init state, remove those from 'mask' to zero - * those features in the user buffer instead of retrieving them - * from init_fpstate. - */ - if (fpu_state_size_dynamic()) - mask &= (header.xfeatures | xinit->header.xcomp_bv); + mask = header.xfeatures;
for_each_extended_xfeature(i, mask) { /* @@ -1151,9 +1143,8 @@ void __copy_xstate_to_uabi_buf(struct membuf to, struct fpstate *fpstate, pkru.pkru = pkru_val; membuf_write(&to, &pkru, sizeof(pkru)); } else { - copy_feature(header.xfeatures & BIT_ULL(i), &to, + membuf_write(&to, __raw_xsave_addr(xsave, i), - __raw_xsave_addr(xinit, i), xstate_sizes[i]); } /*
Chang: to reproduce this issue, you can simply run the amx_test in the kvm selftest directory.
Yeah, I was able to reproduce it with this ptrace test:
diff --git a/tools/testing/selftests/x86/amx.c b/tools/testing/selftests/x86/amx.c index 625e42901237..ae02bc81846d 100644 --- a/tools/testing/selftests/x86/amx.c +++ b/tools/testing/selftests/x86/amx.c @@ -14,8 +14,10 @@ #include <sys/auxv.h> #include <sys/mman.h> #include <sys/shm.h> +#include <sys/ptrace.h> #include <sys/syscall.h> #include <sys/wait.h> +#include <sys/uio.h>
#include "../kselftest.h" /* For __cpuid_count() */
@@ -826,6 +828,76 @@ static void test_context_switch(void) free(finfo); }
+/* Ptrace test */ + +static bool inject_tiledata(pid_t target) +{ + struct xsave_buffer *xbuf; + struct iovec iov; + + xbuf = alloc_xbuf(); + if (!xbuf) + fatal_error("unable to allocate XSAVE buffer"); + + load_rand_tiledata(xbuf); + + memcpy(&stashed_xsave->bytes[xtiledata.xbuf_offset], + &xbuf->bytes[xtiledata.xbuf_offset], + xtiledata.size); + + iov.iov_base = xbuf; + iov.iov_len = xbuf_size; + + if (ptrace(PTRACE_SETREGSET, target, (uint32_t)NT_X86_XSTATE, &iov)) + fatal_error("PTRACE_SETREGSET"); + + if (ptrace(PTRACE_GETREGSET, target, (uint32_t)NT_X86_XSTATE, &iov)) + err(1, "PTRACE_GETREGSET"); + + if (!memcmp(&stashed_xsave->bytes[xtiledata.xbuf_offset], + &xbuf->bytes[xtiledata.xbuf_offset], + xtiledata.size)) + return true; + else + return false; +} + +static void test_ptrace(void) +{ + pid_t child; + int status; + + child = fork(); + if (child < 0) { + err(1, "fork"); + } else if (!child) { + if (ptrace(PTRACE_TRACEME, 0, NULL, NULL)) + err(1, "PTRACE_TRACEME"); + + /* Use the state to expand the kernel buffer */ + load_rand_tiledata(stashed_xsave); + + raise(SIGTRAP); + _exit(0); + } + + do { + wait(&status); + } while (WSTOPSIG(status) != SIGTRAP); + + printf("\tInject tile data via ptrace()\n"); + + if (inject_tiledata(child)) + printf("[OK]\tTile data was written on ptracee.\n"); + else + printf("[FAIL]\tTile data was not written on ptracee.\n"); + + ptrace(PTRACE_DETACH, child, NULL, NULL); + wait(&status); + if (!WIFEXITED(status) || WEXITSTATUS(status)) + err(1, "ptrace test"); +} + int main(void) { /* Check hardware availability at first */ @@ -846,6 +918,8 @@ int main(void) ctxtswtest_config.num_threads = 5; test_context_switch();
+ test_ptrace(); + clearhandler(SIGILL); free_stashed_xsave();
Thanks, Chang
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch...
On Wed, Feb 22, 2023, Chang S. Bae wrote:
Do we need to change this line and the comments? I don't see any of these was relevant to this issue. The original code semantic is to traverse all user_xfeatures, if it is available in fpstate, copy it from there; otherwise, copy it from init_fpstate. We do not assume the component in init_fpstate (but not in fpstate) are all zeros, do we? If it is safe to assume that, then it might be ok. But at least in this patch, I want to keep the original semantics as is without the assumption.
Same here. Let's not adding this optimization in this patch.
Nice one. Yeah both ptrace and KVM are calling this function so the above code would also be enough to trigger the bug.
Thanks. -Mingwei
On 2/24/2023 3:56 PM, Mingwei Zhang wrote:
Here it has [1]:
* * XSAVE could be used, but that would require to reshuffle the * data when XSAVEC/S is available because XSAVEC/S uses xstate * compaction. But doing so is a pointless exercise because most * components have an all zeros init state except for the legacy * ones (FP and SSE). Those can be saved with FXSAVE into the * legacy area. Adding new features requires to ensure that init * state is all zeroes or if not to add the necessary handling * here. */ fxsave(&init_fpstate.regs.fxsave);
Thus, init_fpstate has zeros for those extended states. Then, copying from init_fpstate is the same as membuf_zero() by the gap tracking. But, we have two ways to do the same thing here.
So I think it works that simply copying the state from fpstate only for those present there, then letting the gap tracking zero out for the rest of the userspace buffer for features that are either disabled or initialized.
Then, we can remove accessing init_fpstate in the copy loop and which is the source of the problem. So I think this line change is relevant and also makes the code simple.
I guess I'm fine if you don't want to do this. Then, let me follow up with something like this at first. Something like yours could be a fallback option for other good reasons, otherwise.
Thanks, Chang
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch...
On Fri, Feb 24, 2023, Chang S. Bae wrote:
ah, I see.
hmm. I see. But this is still because of the software implementation. What if there is a new hardware component that requires a non-zero init state.
For instance, in the past, we had PKRU component, whose init value is 0x555...54. Of course, that is a bad example because now we kick it out of the XSAVE/XRSTOR and special handling that, but there is no guarantee that in the future we will never need a non-zero init state.
So, I will send out my fix and let you, Thomas and potentially other folks to decide what is the best option. Overall, I get your point.
Thanks -Mingwei
On 2/24/2023 5:09 PM, Mingwei Zhang wrote:
Yeah, then that feature enabling has to update [1] to record the special init value there. So one who writes that code has to responsibly check what has to be adjusted like for other feature enabling in general.
Now we have established the dynamic states which are also assumed to have an all-zeros init state. Anything new state located after that should be a dynamic state because of the sigaltstack.
Of course, with that though, I don't know whether we will see anything with a non-zero init state in the future or not.
So, I will send out my fix and let you, Thomas and potentially other folks to decide what is the best option. Overall, I get your point.
Let's coordinate this. We shouldn't throw multiple versions at the same time. (Let me follow up with you.)
Thanks, Chang
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 b1a31de7108a..5cfd7ef40d78 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..bb9dc0008f43 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, PAGE_SIZE * DIV_ROUND_UP(XSAVE_SIZE, PAGE_SIZE)); + vcpu_args_set(vcpu, 3, amx_cfg, tiledata, xstate);
for (stage = 1; ; stage++) { vcpu_run(vcpu);
Please omit the "x86:" from the shortlog. I'm not necessarily against capturing the arch somewhere in the shortlog for KVM selftests changes, but I want the community as a whole to make a concious decision on if and how to do it. I.e. I don't want a bunch of ad hoc versions popping up because we'll end up with a mess. E.g. I personally think three levels of scope is too much, and would rather do something like "KVM: x86/selftests:" or "KVM: selftests/x86:".
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 bb9dc0008f43..16c857c1052e 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));
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 16c857c1052e..ba8c0afdbac8 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);
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 ba8c0afdbac8..ac49b14460b6 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -216,6 +216,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 21, 2023, Mingwei Zhang wrote:
The check is not missing. CR0.TS is never set in selftests, i.e. this is pure paranoia. I've no objection to adding the paranoid check, but it should not presented as a "flaw" in the existing code.
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 ac49b14460b6..296c954dfd6d 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -218,8 +218,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 296c954dfd6d..62fff3363b3b 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -217,10 +217,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);
On Tue, Feb 21, 2023, Mingwei Zhang wrote:
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:
If that happens, then the future change is responsible for updating the check. The test very clearly does a straight write of MSR_IA32_XFD. If there are extra bits set then something is broken.
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 62fff3363b3b..724e991ba814 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);
From: Aaron Lewis aaronlewis@google.com
Assert that XTILE is XSAVE-enabled. check_xsave_supports_xtile() doesn't actually check anything since its return value is not used. Add the intended assert.
Opportunistically, move the assert to a more appropriate location: immediately after XSETBV and remove check_xsave_supports_xtile().
Fixes: 5dc19f1c7dd3 ("KVM: selftests: Convert AMX test to use X86_PROPRETY_XXX") Signed-off-by: Aaron Lewis aaronlewis@google.com Signed-off-by: Mingwei Zhang mizhang@google.com --- tools/testing/selftests/kvm/x86_64/amx_test.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index 724e991ba814..d1fbf8936192 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -115,11 +115,6 @@ static inline void check_cpuid_xsave(void) GUEST_ASSERT(this_cpu_has(X86_FEATURE_OSXSAVE)); }
-static bool check_xsave_supports_xtile(void) -{ - return __xgetbv(0) & XFEATURE_MASK_XTILE; -} - static void check_xtile_info(void) { GUEST_ASSERT(this_cpu_has_p(X86_PROPERTY_XSTATE_MAX_SIZE_XCR0)); @@ -166,6 +161,7 @@ static void init_regs(void) xcr0 = __xgetbv(0); xcr0 |= XFEATURE_MASK_XTILE; __xsetbv(0x0, xcr0); + GUEST_ASSERT(__xgetbv(0) & XFEATURE_MASK_XTILE); }
static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg, @@ -174,7 +170,6 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg, { init_regs(); check_cpuid_xsave(); - check_xsave_supports_xtile(); check_xtile_info(); GUEST_SYNC(1);
From: Aaron Lewis aaronlewis@google.com
Assert that both XTILE{CFG,DATA} are XSAVE-enabled. The original check in amx_test only ensures at least one of the XTILE bits are set, XTILECFG or XTILEDATA, when it really should be checking that both are set.
Assert that both XTILECFG and XTILEDATA a set.
Fixes: bf70636d9443 ("selftest: kvm: Add amx selftest") Signed-off-by: Aaron Lewis aaronlewis@google.com Signed-off-by: Mingwei Zhang mizhang@google.com --- tools/testing/selftests/kvm/x86_64/amx_test.c | 2 +- 1 file changed, 1 insertion(+), 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 d1fbf8936192..1a1565126255 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -161,7 +161,7 @@ static void init_regs(void) xcr0 = __xgetbv(0); xcr0 |= XFEATURE_MASK_XTILE; __xsetbv(0x0, xcr0); - GUEST_ASSERT(__xgetbv(0) & XFEATURE_MASK_XTILE); + GUEST_ASSERT((__xgetbv(0) & XFEATURE_MASK_XTILE) == XFEATURE_MASK_XTILE); }
static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg,
From: Aaron Lewis aaronlewis@google.com
In amx_test, userspace requires that XSAVE is supported before running the test, then the guest checks that it is supported after enabling AMX. Remove the redundant check in the guest that XSAVE is supported.
Opportunistically, move the check that OSXSAVE is set to immediately after the guest sets it, rather than in a separate helper.
Signed-off-by: Aaron Lewis aaronlewis@google.com Signed-off-by: Mingwei Zhang mizhang@google.com --- tools/testing/selftests/kvm/x86_64/amx_test.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index 1a1565126255..deacd21cf744 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -109,12 +109,6 @@ static inline void __xsavec(struct xstate *xstate, uint64_t rfbm) : "memory"); }
-static inline void check_cpuid_xsave(void) -{ - GUEST_ASSERT(this_cpu_has(X86_FEATURE_XSAVE)); - GUEST_ASSERT(this_cpu_has(X86_FEATURE_OSXSAVE)); -} - static void check_xtile_info(void) { GUEST_ASSERT(this_cpu_has_p(X86_PROPERTY_XSTATE_MAX_SIZE_XCR0)); @@ -157,6 +151,7 @@ static void init_regs(void) cr4 = get_cr4(); cr4 |= X86_CR4_OSXSAVE; set_cr4(cr4); + GUEST_ASSERT(this_cpu_has(X86_FEATURE_OSXSAVE));
xcr0 = __xgetbv(0); xcr0 |= XFEATURE_MASK_XTILE; @@ -169,7 +164,6 @@ static void __attribute__((__flatten__)) guest_code(struct tile_config *amx_cfg, struct xstate *xstate) { init_regs(); - check_cpuid_xsave(); check_xtile_info(); GUEST_SYNC(1);
On Tue, Feb 21, 2023, Mingwei Zhang wrote:
It's a bit paranoid, but I actually don't mind the extra check. It's not redundant per se, just useless in its current location. If the check is moved _before_ CR4 is set, then it actually provides value, e.g. if something does go sideways, will fire an assert instead of getting a #GP on set_cr4().
From: Aaron Lewis aaronlewis@google.com
Check that the palette table exists before using it. The maximum number of AMX palette tables is enumerated by CPUID.1DH:EAX. Assert that the palette used in amx_test, CPUID.1DH.1H, does not exceed that maximum.
Signed-off-by: Aaron Lewis aaronlewis@google.com Signed-off-by: Mingwei Zhang mizhang@google.com --- tools/testing/selftests/kvm/include/x86_64/processor.h | 1 + tools/testing/selftests/kvm/x86_64/amx_test.c | 5 +++++ 2 files changed, 6 insertions(+)
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h index 5cfd7ef40d78..a6a86c41ed75 100644 --- a/tools/testing/selftests/kvm/include/x86_64/processor.h +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h @@ -223,6 +223,7 @@ struct kvm_x86_cpu_property { #define X86_PROPERTY_XSTATE_MAX_SIZE KVM_X86_CPU_PROPERTY(0xd, 0, ECX, 0, 31) #define X86_PROPERTY_XSTATE_TILE_SIZE KVM_X86_CPU_PROPERTY(0xd, 18, EAX, 0, 31) #define X86_PROPERTY_XSTATE_TILE_OFFSET KVM_X86_CPU_PROPERTY(0xd, 18, EBX, 0, 31) +#define X86_PROPERTY_AMX_MAX_PALETTE_TABLES KVM_X86_CPU_PROPERTY(0x1d, 0, EAX, 0, 31) #define X86_PROPERTY_AMX_TOTAL_TILE_BYTES KVM_X86_CPU_PROPERTY(0x1d, 1, EAX, 0, 15) #define X86_PROPERTY_AMX_BYTES_PER_TILE KVM_X86_CPU_PROPERTY(0x1d, 1, EAX, 16, 31) #define X86_PROPERTY_AMX_BYTES_PER_ROW KVM_X86_CPU_PROPERTY(0x1d, 1, EBX, 0, 15) diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index deacd21cf744..2fd6a8a928d9 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -30,6 +30,7 @@ #define XSAVE_SIZE ((NUM_TILES * TILE_SIZE) + PAGE_SIZE)
/* Tile configuration associated: */ +#define PALETTE_TABLE_INDEX 1 #define MAX_TILES 16 #define RESERVED_BYTES 14
@@ -120,6 +121,10 @@ static void check_xtile_info(void) GUEST_ASSERT(xtile.xsave_size == 8192); GUEST_ASSERT(sizeof(struct tile_data) >= xtile.xsave_size);
+ GUEST_ASSERT(this_cpu_has_p(X86_PROPERTY_AMX_MAX_PALETTE_TABLES)); + GUEST_ASSERT(this_cpu_property(X86_PROPERTY_AMX_MAX_PALETTE_TABLES) >= + PALETTE_TABLE_INDEX); + GUEST_ASSERT(this_cpu_has_p(X86_PROPERTY_AMX_NR_TILE_REGS)); xtile.max_names = this_cpu_property(X86_PROPERTY_AMX_NR_TILE_REGS); GUEST_ASSERT(xtile.max_names == 8);
From: Aaron Lewis aaronlewis@google.com
Check that XTILEDATA supports XFD. In amx_test, add the requirement that the guest allows the xfeature, XTILEDATA, to be set in XFD. Otherwise, the test may fail.
Signed-off-by: Aaron Lewis aaronlewis@google.com Signed-off-by: Mingwei Zhang mizhang@google.com --- tools/testing/selftests/kvm/include/x86_64/processor.h | 1 + tools/testing/selftests/kvm/x86_64/amx_test.c | 1 + 2 files changed, 2 insertions(+)
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h index a6a86c41ed75..4f6d2d31ff34 100644 --- a/tools/testing/selftests/kvm/include/x86_64/processor.h +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h @@ -140,6 +140,7 @@ struct kvm_x86_cpu_feature { #define X86_FEATURE_XTILEDATA KVM_X86_CPU_FEATURE(0xD, 0, EAX, 18) #define X86_FEATURE_XSAVES KVM_X86_CPU_FEATURE(0xD, 1, EAX, 3) #define X86_FEATURE_XFD KVM_X86_CPU_FEATURE(0xD, 1, EAX, 4) +#define X86_FEATURE_XTILEDATA_XFD KVM_X86_CPU_FEATURE(0xD, 18, ECX, 2)
/* * Extended Leafs, a.k.a. AMD defined diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c index 2fd6a8a928d9..2eb265297898 100644 --- a/tools/testing/selftests/kvm/x86_64/amx_test.c +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c @@ -257,6 +257,7 @@ int main(int argc, char *argv[]) TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_AMX_TILE)); TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_XTILECFG)); TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_XTILEDATA)); + TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_XTILEDATA_XFD));
/* Create VM */ vm = vm_create_with_one_vcpu(&vcpu, guest_code);
On Tue, 21 Feb 2023 16:36:42 +0000, Mingwei Zhang wrote:
Applied everything except patch 7 to kvm-x86 selftests. Please holler if I missed something subtle about patch 7 (using & vs. ==). This is at the head of kvm-x86/selftests, i.e. I can fix it up if necessary.
[01/13] x86/fpu/xstate: Avoid getting xstate address of init_fpstate if fpstate contains the component (no commit info) [02/13] KVM: selftests: x86: Add a working xstate data structure https://github.com/kvm-x86/linux/commit/03e8ddff78ac [03/13] KVM: selftests: x86: Fix an error in comment of amx_test https://github.com/kvm-x86/linux/commit/c23d3b210baf [04/13] KVM: selftests: x86: Enable checking on xcomp_bv in amx_test https://github.com/kvm-x86/linux/commit/1e2de0651567 [05/13] KVM: selftests: x86: Add check of CR0.TS in the #NM handler in amx_test https://github.com/kvm-x86/linux/commit/04f5d4539105 [06/13] KVM: selftests: x86: Add the XFD check to IA32_XFD in #NM handler https://github.com/kvm-x86/linux/commit/eeae141ddb54
[08/13] KVM: selftests: x86: Repeat the checking of xheader when IA32_XFD[XTILEDATA] is set in amx_test https://github.com/kvm-x86/linux/commit/cabed3f958e9 [09/13] KVM: selftests: x86: Assert that XTILE is XSAVE-enabled https://github.com/kvm-x86/linux/commit/44217830267d [10/13] KVM: selftests: x86: Assert that both XTILE{CFG,DATA} are XSAVE-enabled https://github.com/kvm-x86/linux/commit/7b328195c462 [11/13] KVM: selftests: x86: Remove redundant check that XSAVE is supported https://github.com/kvm-x86/linux/commit/08f63d826980 [12/13] KVM: selftests: x86: Check that the palette table exists before using it https://github.com/kvm-x86/linux/commit/7042ef575496 [13/13] KVM: selftests: x86: Check that XTILEDATA supports XFD https://github.com/kvm-x86/linux/commit/564435d346ff
-- https://github.com/kvm-x86/linux/tree/next https://github.com/kvm-x86/linux/tree/fixes
On Fri, Mar 24, 2023, Sean Christopherson wrote:
Continuing my circus of goofs, I already force pushed selftests due to an unrelated mixup. New hashes below (the comment above still stands in case another overwrite is necessary).
[1/11] KVM: selftests: Add a fully functional "struct xstate" for x86 https://github.com/kvm-x86/linux/commit/5de4a3765b7e [2/11] KVM: selftests: Fix an error in comment of amx_test https://github.com/kvm-x86/linux/commit/bec357a4af55 [3/11] KVM: selftests: Enable checking on xcomp_bv in amx_test https://github.com/kvm-x86/linux/commit/48ad4222c43c [4/11] KVM: selftests: Add check of CR0.TS in the #NM handler in amx_test https://github.com/kvm-x86/linux/commit/0aeb9729486a [5/11] KVM: selftests: Assert that XTILE_DATA is set in IA32_XFD on #NM https://github.com/kvm-x86/linux/commit/9cbd9aaa670f [6/11] KVM: selftests: Verify XTILE_DATA in XSTATE isn't affected by IA32_XFD https://github.com/kvm-x86/linux/commit/bfc5afc37c9d [7/11] KVM: selftests: Assert that XTILE is XSAVE-enabled https://github.com/kvm-x86/linux/commit/7e1075f05078 [8/11] KVM: selftests: Assert that both XTILE{CFG,DATA} are XSAVE-enabled https://github.com/kvm-x86/linux/commit/2ab3991b0b9b [9/11] KVM: selftests: Move XSAVE and OSXSAVE CPUID checks into AMX's init_regs() https://github.com/kvm-x86/linux/commit/d01d4a4f7bd2 [10/11] KVM: selftests: Check that the palette table exists before using it https://github.com/kvm-x86/linux/commit/d32fb0714293 [11/11] KVM: selftests: Check that XTILEDATA supports XFD https://github.com/kvm-x86/linux/commit/d563164eaeb1
linux-kselftest-mirror@lists.linaro.org
-
Chang S. Bae -
Mingwei Zhang -
Sean Christopherson -
Thomas Gleixner