* [PATCH v2 1/4] KVM: selftests: Move XFD CPUID checking out of __vm_xsave_require_permission()
2022-11-28 22:57 [PATCH v2 0/4] KVM: selftests: AMX test fix and cleanups Sean Christopherson
@ 2022-11-28 22:57 ` Sean Christopherson
2022-11-28 22:57 ` [PATCH v2 2/4] KVM: selftests: Move __vm_xsave_require_permission() below CPUID helpers Sean Christopherson
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2022-11-28 22:57 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, linux-kernel, Lei Wang, Sean Christopherson
From: Lei Wang <lei4.wang@intel.com>
Move the kvm_cpu_has() check on X86_FEATURE_XFD out of the helper to
enable off-by-default XSAVE-managed features and into the one test that
currenty requires XFD (XFeature Disable) support. kvm_cpu_has() uses
kvm_get_supported_cpuid() and thus caches KVM_GET_SUPPORTED_CPUID, and so
using kvm_cpu_has() before ARCH_REQ_XCOMP_GUEST_PERM effectively results
in the test caching stale values, e.g. subsequent checks on AMX_TILE will
get false negatives.
Although off-by-default features are nonsensical without XFD, checking
for XFD virtualization prior to enabling such features isn't strictly
required.
Signed-off-by: Lei Wang <lei4.wang@intel.com>
Fixes: 7fbb653e01fd ("KVM: selftests: Check KVM's supported CPUID, not host CPUID, for XFD")
Link: https://lore.kernel.org/r/20221125023839.315207-1-lei4.wang@intel.com
[sean: add Fixes, reword changelog]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/lib/x86_64/processor.c | 2 --
tools/testing/selftests/kvm/x86_64/amx_test.c | 1 +
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index d532c20c74fd..aac7b32a794b 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -563,8 +563,6 @@ void __vm_xsave_require_permission(int bit, const char *name)
.addr = (unsigned long) &bitmask
};
- TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_XFD));
-
kvm_fd = open_kvm_dev_path_or_exit();
rc = __kvm_ioctl(kvm_fd, KVM_GET_DEVICE_ATTR, &attr);
close(kvm_fd);
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c
index 21de6ae42086..1256c7faadd3 100644
--- a/tools/testing/selftests/kvm/x86_64/amx_test.c
+++ b/tools/testing/selftests/kvm/x86_64/amx_test.c
@@ -254,6 +254,7 @@ int main(int argc, char *argv[])
/* Create VM */
vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+ TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_XFD));
TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_XSAVE));
TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_AMX_TILE));
TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_XTILECFG));
--
2.38.1.584.g0f3c55d4c2-goog
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 2/4] KVM: selftests: Move __vm_xsave_require_permission() below CPUID helpers
2022-11-28 22:57 [PATCH v2 0/4] KVM: selftests: AMX test fix and cleanups Sean Christopherson
2022-11-28 22:57 ` [PATCH v2 1/4] KVM: selftests: Move XFD CPUID checking out of __vm_xsave_require_permission() Sean Christopherson
@ 2022-11-28 22:57 ` Sean Christopherson
2022-11-28 22:57 ` [PATCH v2 3/4] KVM: selftests: Disallow "get supported CPUID" before REQ_XCOMP_GUEST_PERM Sean Christopherson
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2022-11-28 22:57 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, linux-kernel, Lei Wang, Sean Christopherson
Move __vm_xsave_require_permission() below the CPUID helpers so that a
future change can reference the cached result of KVM_GET_SUPPORTED_CPUID
while keeping the definition of the variable close to its intended user,
kvm_get_supported_cpuid().
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
.../selftests/kvm/lib/x86_64/processor.c | 64 +++++++++----------
1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index aac7b32a794b..23067465c035 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -552,38 +552,6 @@ static void vcpu_setup(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
vcpu_sregs_set(vcpu, &sregs);
}
-void __vm_xsave_require_permission(int bit, const char *name)
-{
- int kvm_fd;
- u64 bitmask;
- long rc;
- struct kvm_device_attr attr = {
- .group = 0,
- .attr = KVM_X86_XCOMP_GUEST_SUPP,
- .addr = (unsigned long) &bitmask
- };
-
- kvm_fd = open_kvm_dev_path_or_exit();
- rc = __kvm_ioctl(kvm_fd, KVM_GET_DEVICE_ATTR, &attr);
- close(kvm_fd);
-
- if (rc == -1 && (errno == ENXIO || errno == EINVAL))
- __TEST_REQUIRE(0, "KVM_X86_XCOMP_GUEST_SUPP not supported");
-
- TEST_ASSERT(rc == 0, "KVM_GET_DEVICE_ATTR(0, KVM_X86_XCOMP_GUEST_SUPP) error: %ld", rc);
-
- __TEST_REQUIRE(bitmask & (1ULL << bit),
- "Required XSAVE feature '%s' not supported", name);
-
- TEST_REQUIRE(!syscall(SYS_arch_prctl, ARCH_REQ_XCOMP_GUEST_PERM, bit));
-
- rc = syscall(SYS_arch_prctl, ARCH_GET_XCOMP_GUEST_PERM, &bitmask);
- TEST_ASSERT(rc == 0, "prctl(ARCH_GET_XCOMP_GUEST_PERM) error: %ld", rc);
- TEST_ASSERT(bitmask & (1ULL << bit),
- "prctl(ARCH_REQ_XCOMP_GUEST_PERM) failure bitmask=0x%lx",
- bitmask);
-}
-
void kvm_arch_vm_post_create(struct kvm_vm *vm)
{
vm_create_irqchip(vm);
@@ -705,6 +673,38 @@ uint64_t kvm_get_feature_msr(uint64_t msr_index)
return buffer.entry.data;
}
+void __vm_xsave_require_permission(int bit, const char *name)
+{
+ int kvm_fd;
+ u64 bitmask;
+ long rc;
+ struct kvm_device_attr attr = {
+ .group = 0,
+ .attr = KVM_X86_XCOMP_GUEST_SUPP,
+ .addr = (unsigned long) &bitmask
+ };
+
+ kvm_fd = open_kvm_dev_path_or_exit();
+ rc = __kvm_ioctl(kvm_fd, KVM_GET_DEVICE_ATTR, &attr);
+ close(kvm_fd);
+
+ if (rc == -1 && (errno == ENXIO || errno == EINVAL))
+ __TEST_REQUIRE(0, "KVM_X86_XCOMP_GUEST_SUPP not supported");
+
+ TEST_ASSERT(rc == 0, "KVM_GET_DEVICE_ATTR(0, KVM_X86_XCOMP_GUEST_SUPP) error: %ld", rc);
+
+ __TEST_REQUIRE(bitmask & (1ULL << bit),
+ "Required XSAVE feature '%s' not supported", name);
+
+ TEST_REQUIRE(!syscall(SYS_arch_prctl, ARCH_REQ_XCOMP_GUEST_PERM, bit));
+
+ rc = syscall(SYS_arch_prctl, ARCH_GET_XCOMP_GUEST_PERM, &bitmask);
+ TEST_ASSERT(rc == 0, "prctl(ARCH_GET_XCOMP_GUEST_PERM) error: %ld", rc);
+ TEST_ASSERT(bitmask & (1ULL << bit),
+ "prctl(ARCH_REQ_XCOMP_GUEST_PERM) failure bitmask=0x%lx",
+ bitmask);
+}
+
void vcpu_init_cpuid(struct kvm_vcpu *vcpu, const struct kvm_cpuid2 *cpuid)
{
TEST_ASSERT(cpuid != vcpu->cpuid, "@cpuid can't be the vCPU's CPUID");
--
2.38.1.584.g0f3c55d4c2-goog
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 3/4] KVM: selftests: Disallow "get supported CPUID" before REQ_XCOMP_GUEST_PERM
2022-11-28 22:57 [PATCH v2 0/4] KVM: selftests: AMX test fix and cleanups Sean Christopherson
2022-11-28 22:57 ` [PATCH v2 1/4] KVM: selftests: Move XFD CPUID checking out of __vm_xsave_require_permission() Sean Christopherson
2022-11-28 22:57 ` [PATCH v2 2/4] KVM: selftests: Move __vm_xsave_require_permission() below CPUID helpers Sean Christopherson
@ 2022-11-28 22:57 ` Sean Christopherson
2022-11-28 22:57 ` [PATCH v2 4/4] KVM: selftests: Do kvm_cpu_has() checks before creating VM+vCPU Sean Christopherson
2022-11-29 1:58 ` [PATCH v2 0/4] KVM: selftests: AMX test fix and cleanups Wang, Lei
4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2022-11-28 22:57 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, linux-kernel, Lei Wang, Sean Christopherson
Disallow using kvm_get_supported_cpuid() and thus caching KVM's supported
CPUID info before enabling XSAVE-managed features that are off-by-default
and must be enabled by ARCH_REQ_XCOMP_GUEST_PERM. Caching the supported
CPUID before all XSAVE features are enabled can result in false negatives
due to testing features that were cached before they were enabled.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
.../selftests/kvm/lib/x86_64/processor.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index 23067465c035..1d3829e652e6 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -601,21 +601,24 @@ void vcpu_arch_free(struct kvm_vcpu *vcpu)
free(vcpu->cpuid);
}
+/* Do not use kvm_supported_cpuid directly except for validity checks. */
+static void *kvm_supported_cpuid;
+
const struct kvm_cpuid2 *kvm_get_supported_cpuid(void)
{
- static struct kvm_cpuid2 *cpuid;
int kvm_fd;
- if (cpuid)
- return cpuid;
+ if (kvm_supported_cpuid)
+ return kvm_supported_cpuid;
- cpuid = allocate_kvm_cpuid2(MAX_NR_CPUID_ENTRIES);
+ kvm_supported_cpuid = allocate_kvm_cpuid2(MAX_NR_CPUID_ENTRIES);
kvm_fd = open_kvm_dev_path_or_exit();
- kvm_ioctl(kvm_fd, KVM_GET_SUPPORTED_CPUID, cpuid);
+ kvm_ioctl(kvm_fd, KVM_GET_SUPPORTED_CPUID,
+ (struct kvm_cpuid2 *)kvm_supported_cpuid);
close(kvm_fd);
- return cpuid;
+ return kvm_supported_cpuid;
}
static uint32_t __kvm_cpu_has(const struct kvm_cpuid2 *cpuid,
@@ -684,6 +687,9 @@ void __vm_xsave_require_permission(int bit, const char *name)
.addr = (unsigned long) &bitmask
};
+ TEST_ASSERT(!kvm_supported_cpuid,
+ "kvm_get_supported_cpuid() cannot be used before ARCH_REQ_XCOMP_GUEST_PERM");
+
kvm_fd = open_kvm_dev_path_or_exit();
rc = __kvm_ioctl(kvm_fd, KVM_GET_DEVICE_ATTR, &attr);
close(kvm_fd);
--
2.38.1.584.g0f3c55d4c2-goog
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 4/4] KVM: selftests: Do kvm_cpu_has() checks before creating VM+vCPU
2022-11-28 22:57 [PATCH v2 0/4] KVM: selftests: AMX test fix and cleanups Sean Christopherson
` (2 preceding siblings ...)
2022-11-28 22:57 ` [PATCH v2 3/4] KVM: selftests: Disallow "get supported CPUID" before REQ_XCOMP_GUEST_PERM Sean Christopherson
@ 2022-11-28 22:57 ` Sean Christopherson
2022-11-29 1:58 ` [PATCH v2 0/4] KVM: selftests: AMX test fix and cleanups Wang, Lei
4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2022-11-28 22:57 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, linux-kernel, Lei Wang, Sean Christopherson
Move the AMX test's kvm_cpu_has() checks before creating the VM+vCPU,
there are no dependencies between the two operations. Opportunistically
add a comment to call out that enabling off-by-default XSAVE-managed
features must be done before KVM_GET_SUPPORTED_CPUID is cached.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/x86_64/amx_test.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c
index 1256c7faadd3..bd72c6eb3b67 100644
--- a/tools/testing/selftests/kvm/x86_64/amx_test.c
+++ b/tools/testing/selftests/kvm/x86_64/amx_test.c
@@ -249,17 +249,21 @@ int main(int argc, char *argv[])
u32 amx_offset;
int stage, ret;
+ /*
+ * Note, all off-by-default features must be enabled before anything
+ * caches KVM_GET_SUPPORTED_CPUID, e.g. before using kvm_cpu_has().
+ */
vm_xsave_require_permission(XSTATE_XTILE_DATA_BIT);
- /* Create VM */
- vm = vm_create_with_one_vcpu(&vcpu, guest_code);
-
TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_XFD));
TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_XSAVE));
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));
+ /* Create VM */
+ vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+
TEST_ASSERT(kvm_cpu_has_p(X86_PROPERTY_XSTATE_MAX_SIZE),
"KVM should enumerate max XSAVE size when XSAVE is supported");
xsave_restore_size = kvm_cpu_property(X86_PROPERTY_XSTATE_MAX_SIZE);
--
2.38.1.584.g0f3c55d4c2-goog
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 0/4] KVM: selftests: AMX test fix and cleanups
2022-11-28 22:57 [PATCH v2 0/4] KVM: selftests: AMX test fix and cleanups Sean Christopherson
` (3 preceding siblings ...)
2022-11-28 22:57 ` [PATCH v2 4/4] KVM: selftests: Do kvm_cpu_has() checks before creating VM+vCPU Sean Christopherson
@ 2022-11-29 1:58 ` Wang, Lei
4 siblings, 0 replies; 6+ messages in thread
From: Wang, Lei @ 2022-11-29 1:58 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel
On 11/29/2022 6:57 AM, Sean Christopherson wrote:
> Fix a bug in the AMX test introduced by moving to kvm_cpu_has() for
> X86_FEATURE_XFD, and clean up the related code. The fix and cleanups
> all revolve around ARCH_REQ_XCOMP_GUEST_PERM impacting the output of
> KVM_GET_SUPPORTED_CPUID, and thus causing problems for selftest's
> caching of KVM's supported CPUID.
>
> Lei Wang, I adjusted your Author and SOB tags to drop the comma, which I
> think? violates that "use your real name" rule, and use formatting from
> patches you've sent in the past[*]. Let me know if that's ok (or not).
That's OK, thanks!
> [*] https://lore.kernel.org/all/20220424101557.134102-9-lei4.wang@intel.com
>
> Lei Wang (1):
> KVM: selftests: Move XFD CPUID checking out of
> __vm_xsave_require_permission()
>
> Sean Christopherson (3):
> KVM: selftests: Move __vm_xsave_require_permission() below CPUID
> helpers
> KVM: selftests: Disallow "get supported CPUID" before
> REQ_XCOMP_GUEST_PERM
> KVM: selftests: Do kvm_cpu_has() checks before creating VM+vCPU
>
> .../selftests/kvm/lib/x86_64/processor.c | 84 ++++++++++---------
> tools/testing/selftests/kvm/x86_64/amx_test.c | 11 ++-
> 2 files changed, 52 insertions(+), 43 deletions(-)
>
>
> base-commit: 519c2002cd92bdc37c8412ca22cb9c7e7bc48c48
^ permalink raw reply [flat|nested] 6+ messages in thread