On Fri, Feb 24, 2023, Aaron Lewis wrote:
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index ebe83cfe521c..380daa82b023 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -667,6 +667,15 @@ static inline bool this_pmu_has(struct kvm_x86_pmu_feature feature)
> !this_cpu_has(feature.anti_feature);
> }
>
> +static __always_inline uint64_t this_cpu_supported_xcr0(void)
> +{
> + uint32_t a, b, c, d;
> +
> + cpuid(0xd, &a, &b, &c, &d);
> +
> + return a | ((uint64_t)d << 32);
This can be done via X86_PROPERTY. It's not that much prettier, but it at least
avoids open coding things.
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index fa49d51753e5..2bb0ec8dddf3 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -228,8 +228,11 @@ struct kvm_x86_cpu_property {
#define X86_PROPERTY_PMU_NR_GP_COUNTERS KVM_X86_CPU_PROPERTY(0xa, 0, EAX, 8, 15)
#define X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH KVM_X86_CPU_PROPERTY(0xa, 0, EAX, 24, 31)
+#define X86_PROPERTY_SUPPORTED_XCR0_LO KVM_X86_CPU_PROPERTY(0xd, 0, EAX, 0, 31)
#define X86_PROPERTY_XSTATE_MAX_SIZE_XCR0 KVM_X86_CPU_PROPERTY(0xd, 0, EBX, 0, 31)
#define X86_PROPERTY_XSTATE_MAX_SIZE KVM_X86_CPU_PROPERTY(0xd, 0, ECX, 0, 31)
+#define X86_PROPERTY_SUPPORTED_XCR0_HI KVM_X86_CPU_PROPERTY(0xd, 0, EDX, 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_TOTAL_TILE_BYTES KVM_X86_CPU_PROPERTY(0x1d, 1, EAX, 0, 15)
@@ -669,11 +672,11 @@ static inline bool this_pmu_has(struct kvm_x86_pmu_feature feature)
static __always_inline uint64_t this_cpu_supported_xcr0(void)
{
- uint32_t a, b, c, d;
+ if (!this_cpu_has_p(X86_PROPERTY_SUPPORTED_XCR0_LO))
+ return 0;
- cpuid(0xd, &a, &b, &c, &d);
-
- return a | ((uint64_t)d << 32);
+ return this_cpu_property(X86_PROPERTY_SUPPORTED_XCR0_LO) |
+ ((uint64_t)this_cpu_property(X86_PROPERTY_SUPPORTED_XCR0_HI) << 32);
}
typedef u32 __attribute__((vector_size(16))) sse128_t;
> +/* Architectural check. */
If you're going to bother with a comment, might as well actually explain the
architectural rule.
/*
* Assert that architectural dependency rules are satisfied, e.g. that AVX is
* supported if and only if SSE is supported.
*/
> +#define ASSERT_XFEATURE_DEPENDENCIES(supported_xcr0, xfeatures, dependencies) \
> +do { \
> + uint64_t __supported = (supported_xcr0) & ((xfeatures) | (dependencies)); \
> + \
> + GUEST_ASSERT_3((__supported & (xfeatures)) != (xfeatures) || \
> + __supported == ((xfeatures) | (dependencies)), \
> + __supported, (xfeatures), (dependencies)); \
> +} while (0)
> +
> +/*
> + * KVM's own software-defined rules. While not architectural it really
> + * ought to be true.
This should call out what KVM's "rule" is. But it's not really a rule, it's more
of a contract with userspace.
/*
* Assert that KVM reports a sane, usable as-is XCR0. Architecturally, a CPU
* isn't strictly required to _support_ all XFeatures related to a feature, but
* at the same time XSETBV will #GP if bundled XFeatures aren't enabled and
* disabled coherently. E.g. a CPU can technically enumerate supported for
* XTILE_CFG but not XTILE_DATA, but attempt to enable XTILE_CFG without also
* enabling XTILE_DATA will #GP.
*/
> + /* Tell stdout not to buffer its content */
> + setbuf(stdout, NULL);
This copy pasta is no longer necessary, see kvm_selftest_init().
