2021-04-22 01:31:50

by Ricardo Koller

[permalink] [raw]
Subject: [PATCH 0/5] KVM: x86: Use kernel x86 cpuid utilities in KVM selftests

The kernel has a set of utilities and definitions to deal with x86 cpu
features. The x86 KVM selftests don't use them, and instead have
evolved to use differing and ad-hoc methods for checking features. The
advantage of the kernel feature definitions is that they use a format
that embeds the info needed to extract them from cpuid (function, index,
and register to use).

The first 3 patches massage the related cpuid header files in the kernel
side, then copy them into tools/ so they can be included by selftests.
The last 2 patches replace the tests checking for cpu features to use
the definitions and utilities introduced from the kernel.

Thanks,
Ricardo

Ricardo Koller (5):
KVM: x86: Move reverse CPUID helpers to separate header file
x86/cpu: Expose CPUID regs, leaf and index definitions to tools
tools headers x86: Copy cpuid helpers from the kernel
KVM: selftests: Introduce utilities for checking x86 features
KVM: selftests: Use kernel x86 cpuid features format

arch/x86/events/intel/pt.c | 1 +
arch/x86/include/asm/cpufeature.h | 23 +-
arch/x86/include/asm/processor.h | 11 -
arch/x86/kernel/cpu/scattered.c | 2 +-
arch/x86/kernel/cpuid.c | 2 +-
arch/x86/kvm/cpuid.h | 177 +-----------
arch/x86/kvm/reverse_cpuid.h | 185 +++++++++++++
tools/arch/x86/include/asm/cpufeature.h | 257 ++++++++++++++++++
tools/arch/x86/include/asm/cpufeatures.h | 3 +
.../selftests/kvm/include/x86_64/cpuid.h | 61 +++++
.../selftests/kvm/include/x86_64/processor.h | 16 --
.../kvm/include/x86_64/reverse_cpuid.h | 185 +++++++++++++
.../selftests/kvm/include/x86_64/svm_util.h | 11 +-
tools/testing/selftests/kvm/lib/x86_64/svm.c | 6 +-
tools/testing/selftests/kvm/lib/x86_64/vmx.c | 5 +-
tools/testing/selftests/kvm/steal_time.c | 5 +-
.../kvm/x86_64/cr4_cpuid_sync_test.c | 23 +-
.../selftests/kvm/x86_64/set_sregs_test.c | 25 +-
.../selftests/kvm/x86_64/vmx_pmu_msrs_test.c | 8 +-
.../kvm/x86_64/vmx_set_nested_state_test.c | 5 +-
.../selftests/kvm/x86_64/xss_msr_test.c | 10 +-
21 files changed, 749 insertions(+), 272 deletions(-)
create mode 100644 arch/x86/kvm/reverse_cpuid.h
create mode 100644 tools/arch/x86/include/asm/cpufeature.h
create mode 100644 tools/testing/selftests/kvm/include/x86_64/cpuid.h
create mode 100644 tools/testing/selftests/kvm/include/x86_64/reverse_cpuid.h

--
2.31.1.368.gbe11c130af-goog


2021-04-22 01:31:51

by Ricardo Koller

[permalink] [raw]
Subject: [PATCH 1/5] KVM: x86: Move reverse CPUID helpers to separate header file

Split out the reverse CPUID machinery to a dedicated header file
so that KVM selftests can reuse the reverse CPUID definitions without
introducing any '#ifdef __KERNEL__' pollution.

Co-developed-by: Sean Christopherson <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
Signed-off-by: Ricardo Koller <[email protected]>
---
arch/x86/kvm/cpuid.h | 177 +--------------------------------
arch/x86/kvm/reverse_cpuid.h | 185 +++++++++++++++++++++++++++++++++++
2 files changed, 186 insertions(+), 176 deletions(-)
create mode 100644 arch/x86/kvm/reverse_cpuid.h

diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index 888e88b42e8d..6132ed3c6ebf 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -3,28 +3,11 @@
#define ARCH_X86_KVM_CPUID_H

#include "x86.h"
+#include "reverse_cpuid.h"
#include <asm/cpu.h>
#include <asm/processor.h>
#include <uapi/asm/kvm_para.h>

-/*
- * Hardware-defined CPUID leafs that are scattered in the kernel, but need to
- * be directly used by KVM. Note, these word values conflict with the kernel's
- * "bug" caps, but KVM doesn't use those.
- */
-enum kvm_only_cpuid_leafs {
- CPUID_12_EAX = NCAPINTS,
- NR_KVM_CPU_CAPS,
-
- NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
-};
-
-#define KVM_X86_FEATURE(w, f) ((w)*32 + (f))
-
-/* Intel-defined SGX sub-features, CPUID level 0x12 (EAX). */
-#define KVM_X86_FEATURE_SGX1 KVM_X86_FEATURE(CPUID_12_EAX, 0)
-#define KVM_X86_FEATURE_SGX2 KVM_X86_FEATURE(CPUID_12_EAX, 1)
-
extern u32 kvm_cpu_caps[NR_KVM_CPU_CAPS] __read_mostly;
void kvm_set_cpu_caps(void);

@@ -76,164 +59,6 @@ static inline bool page_address_valid(struct kvm_vcpu *vcpu, gpa_t gpa)
return kvm_vcpu_is_legal_aligned_gpa(vcpu, gpa, PAGE_SIZE);
}

-struct cpuid_reg {
- u32 function;
- u32 index;
- int reg;
-};
-
-static const struct cpuid_reg reverse_cpuid[] = {
- [CPUID_1_EDX] = { 1, 0, CPUID_EDX},
- [CPUID_8000_0001_EDX] = {0x80000001, 0, CPUID_EDX},
- [CPUID_8086_0001_EDX] = {0x80860001, 0, CPUID_EDX},
- [CPUID_1_ECX] = { 1, 0, CPUID_ECX},
- [CPUID_C000_0001_EDX] = {0xc0000001, 0, CPUID_EDX},
- [CPUID_8000_0001_ECX] = {0x80000001, 0, CPUID_ECX},
- [CPUID_7_0_EBX] = { 7, 0, CPUID_EBX},
- [CPUID_D_1_EAX] = { 0xd, 1, CPUID_EAX},
- [CPUID_8000_0008_EBX] = {0x80000008, 0, CPUID_EBX},
- [CPUID_6_EAX] = { 6, 0, CPUID_EAX},
- [CPUID_8000_000A_EDX] = {0x8000000a, 0, CPUID_EDX},
- [CPUID_7_ECX] = { 7, 0, CPUID_ECX},
- [CPUID_8000_0007_EBX] = {0x80000007, 0, CPUID_EBX},
- [CPUID_7_EDX] = { 7, 0, CPUID_EDX},
- [CPUID_7_1_EAX] = { 7, 1, CPUID_EAX},
- [CPUID_12_EAX] = {0x00000012, 0, CPUID_EAX},
-};
-
-/*
- * Reverse CPUID and its derivatives can only be used for hardware-defined
- * feature words, i.e. words whose bits directly correspond to a CPUID leaf.
- * Retrieving a feature bit or masking guest CPUID from a Linux-defined word
- * is nonsensical as the bit number/mask is an arbitrary software-defined value
- * and can't be used by KVM to query/control guest capabilities. And obviously
- * the leaf being queried must have an entry in the lookup table.
- */
-static __always_inline void reverse_cpuid_check(unsigned int x86_leaf)
-{
- BUILD_BUG_ON(x86_leaf == CPUID_LNX_1);
- BUILD_BUG_ON(x86_leaf == CPUID_LNX_2);
- BUILD_BUG_ON(x86_leaf == CPUID_LNX_3);
- BUILD_BUG_ON(x86_leaf == CPUID_LNX_4);
- BUILD_BUG_ON(x86_leaf >= ARRAY_SIZE(reverse_cpuid));
- BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0);
-}
-
-/*
- * Translate feature bits that are scattered in the kernel's cpufeatures word
- * into KVM feature words that align with hardware's definitions.
- */
-static __always_inline u32 __feature_translate(int x86_feature)
-{
- if (x86_feature == X86_FEATURE_SGX1)
- return KVM_X86_FEATURE_SGX1;
- else if (x86_feature == X86_FEATURE_SGX2)
- return KVM_X86_FEATURE_SGX2;
-
- return x86_feature;
-}
-
-static __always_inline u32 __feature_leaf(int x86_feature)
-{
- return __feature_translate(x86_feature) / 32;
-}
-
-/*
- * Retrieve the bit mask from an X86_FEATURE_* definition. Features contain
- * the hardware defined bit number (stored in bits 4:0) and a software defined
- * "word" (stored in bits 31:5). The word is used to index into arrays of
- * bit masks that hold the per-cpu feature capabilities, e.g. this_cpu_has().
- */
-static __always_inline u32 __feature_bit(int x86_feature)
-{
- x86_feature = __feature_translate(x86_feature);
-
- reverse_cpuid_check(x86_feature / 32);
- return 1 << (x86_feature & 31);
-}
-
-#define feature_bit(name) __feature_bit(X86_FEATURE_##name)
-
-static __always_inline struct cpuid_reg x86_feature_cpuid(unsigned int x86_feature)
-{
- unsigned int x86_leaf = __feature_leaf(x86_feature);
-
- reverse_cpuid_check(x86_leaf);
- return reverse_cpuid[x86_leaf];
-}
-
-static __always_inline u32 *__cpuid_entry_get_reg(struct kvm_cpuid_entry2 *entry,
- u32 reg)
-{
- switch (reg) {
- case CPUID_EAX:
- return &entry->eax;
- case CPUID_EBX:
- return &entry->ebx;
- case CPUID_ECX:
- return &entry->ecx;
- case CPUID_EDX:
- return &entry->edx;
- default:
- BUILD_BUG();
- return NULL;
- }
-}
-
-static __always_inline u32 *cpuid_entry_get_reg(struct kvm_cpuid_entry2 *entry,
- unsigned int x86_feature)
-{
- const struct cpuid_reg cpuid = x86_feature_cpuid(x86_feature);
-
- return __cpuid_entry_get_reg(entry, cpuid.reg);
-}
-
-static __always_inline u32 cpuid_entry_get(struct kvm_cpuid_entry2 *entry,
- unsigned int x86_feature)
-{
- u32 *reg = cpuid_entry_get_reg(entry, x86_feature);
-
- return *reg & __feature_bit(x86_feature);
-}
-
-static __always_inline bool cpuid_entry_has(struct kvm_cpuid_entry2 *entry,
- unsigned int x86_feature)
-{
- return cpuid_entry_get(entry, x86_feature);
-}
-
-static __always_inline void cpuid_entry_clear(struct kvm_cpuid_entry2 *entry,
- unsigned int x86_feature)
-{
- u32 *reg = cpuid_entry_get_reg(entry, x86_feature);
-
- *reg &= ~__feature_bit(x86_feature);
-}
-
-static __always_inline void cpuid_entry_set(struct kvm_cpuid_entry2 *entry,
- unsigned int x86_feature)
-{
- u32 *reg = cpuid_entry_get_reg(entry, x86_feature);
-
- *reg |= __feature_bit(x86_feature);
-}
-
-static __always_inline void cpuid_entry_change(struct kvm_cpuid_entry2 *entry,
- unsigned int x86_feature,
- bool set)
-{
- u32 *reg = cpuid_entry_get_reg(entry, x86_feature);
-
- /*
- * Open coded instead of using cpuid_entry_{clear,set}() to coerce the
- * compiler into using CMOV instead of Jcc when possible.
- */
- if (set)
- *reg |= __feature_bit(x86_feature);
- else
- *reg &= ~__feature_bit(x86_feature);
-}
-
static __always_inline void cpuid_entry_override(struct kvm_cpuid_entry2 *entry,
enum cpuid_leafs leaf)
{
diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
new file mode 100644
index 000000000000..8e0756ddab1a
--- /dev/null
+++ b/arch/x86/kvm/reverse_cpuid.h
@@ -0,0 +1,185 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef ARCH_X86_KVM_REVERSE_CPUID_H
+#define ARCH_X86_KVM_REVERSE_CPUID_H
+
+#include <uapi/asm/kvm.h>
+#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
+
+/*
+ * Hardware-defined CPUID leafs that are scattered in the kernel, but need to
+ * be directly used by KVM. Note, these word values conflict with the kernel's
+ * "bug" caps, but KVM doesn't use those.
+ */
+enum kvm_only_cpuid_leafs {
+ CPUID_12_EAX = NCAPINTS,
+ NR_KVM_CPU_CAPS,
+
+ NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
+};
+
+#define KVM_X86_FEATURE(w, f) ((w)*32 + (f))
+
+/* Intel-defined SGX sub-features, CPUID level 0x12 (EAX). */
+#define KVM_X86_FEATURE_SGX1 KVM_X86_FEATURE(CPUID_12_EAX, 0)
+#define KVM_X86_FEATURE_SGX2 KVM_X86_FEATURE(CPUID_12_EAX, 1)
+
+struct cpuid_reg {
+ u32 function;
+ u32 index;
+ int reg;
+};
+
+static const struct cpuid_reg reverse_cpuid[] = {
+ [CPUID_1_EDX] = { 1, 0, CPUID_EDX},
+ [CPUID_8000_0001_EDX] = {0x80000001, 0, CPUID_EDX},
+ [CPUID_8086_0001_EDX] = {0x80860001, 0, CPUID_EDX},
+ [CPUID_1_ECX] = { 1, 0, CPUID_ECX},
+ [CPUID_C000_0001_EDX] = {0xc0000001, 0, CPUID_EDX},
+ [CPUID_8000_0001_ECX] = {0x80000001, 0, CPUID_ECX},
+ [CPUID_7_0_EBX] = { 7, 0, CPUID_EBX},
+ [CPUID_D_1_EAX] = { 0xd, 1, CPUID_EAX},
+ [CPUID_8000_0008_EBX] = {0x80000008, 0, CPUID_EBX},
+ [CPUID_6_EAX] = { 6, 0, CPUID_EAX},
+ [CPUID_8000_000A_EDX] = {0x8000000a, 0, CPUID_EDX},
+ [CPUID_7_ECX] = { 7, 0, CPUID_ECX},
+ [CPUID_8000_0007_EBX] = {0x80000007, 0, CPUID_EBX},
+ [CPUID_7_EDX] = { 7, 0, CPUID_EDX},
+ [CPUID_7_1_EAX] = { 7, 1, CPUID_EAX},
+ [CPUID_12_EAX] = {0x00000012, 0, CPUID_EAX},
+};
+
+/*
+ * Reverse CPUID and its derivatives can only be used for hardware-defined
+ * feature words, i.e. words whose bits directly correspond to a CPUID leaf.
+ * Retrieving a feature bit or masking guest CPUID from a Linux-defined word
+ * is nonsensical as the bit number/mask is an arbitrary software-defined value
+ * and can't be used by KVM to query/control guest capabilities. And obviously
+ * the leaf being queried must have an entry in the lookup table.
+ */
+static __always_inline void reverse_cpuid_check(unsigned int x86_leaf)
+{
+ BUILD_BUG_ON(x86_leaf == CPUID_LNX_1);
+ BUILD_BUG_ON(x86_leaf == CPUID_LNX_2);
+ BUILD_BUG_ON(x86_leaf == CPUID_LNX_3);
+ BUILD_BUG_ON(x86_leaf == CPUID_LNX_4);
+ BUILD_BUG_ON(x86_leaf >= ARRAY_SIZE(reverse_cpuid));
+ BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0);
+}
+
+/*
+ * Translate feature bits that are scattered in the kernel's cpufeatures word
+ * into KVM feature words that align with hardware's definitions.
+ */
+static __always_inline u32 __feature_translate(int x86_feature)
+{
+ if (x86_feature == X86_FEATURE_SGX1)
+ return KVM_X86_FEATURE_SGX1;
+ else if (x86_feature == X86_FEATURE_SGX2)
+ return KVM_X86_FEATURE_SGX2;
+
+ return x86_feature;
+}
+
+static __always_inline u32 __feature_leaf(int x86_feature)
+{
+ return __feature_translate(x86_feature) / 32;
+}
+
+/*
+ * Retrieve the bit mask from an X86_FEATURE_* definition. Features contain
+ * the hardware defined bit number (stored in bits 4:0) and a software defined
+ * "word" (stored in bits 31:5). The word is used to index into arrays of
+ * bit masks that hold the per-cpu feature capabilities, e.g. this_cpu_has().
+ */
+static __always_inline u32 __feature_bit(int x86_feature)
+{
+ x86_feature = __feature_translate(x86_feature);
+
+ reverse_cpuid_check(x86_feature / 32);
+ return 1 << (x86_feature & 31);
+}
+
+#define feature_bit(name) __feature_bit(X86_FEATURE_##name)
+
+static __always_inline struct cpuid_reg x86_feature_cpuid(unsigned int x86_feature)
+{
+ unsigned int x86_leaf = __feature_leaf(x86_feature);
+
+ reverse_cpuid_check(x86_leaf);
+ return reverse_cpuid[x86_leaf];
+}
+
+static __always_inline u32 *__cpuid_entry_get_reg(struct kvm_cpuid_entry2 *entry,
+ u32 reg)
+{
+ switch (reg) {
+ case CPUID_EAX:
+ return &entry->eax;
+ case CPUID_EBX:
+ return &entry->ebx;
+ case CPUID_ECX:
+ return &entry->ecx;
+ case CPUID_EDX:
+ return &entry->edx;
+ default:
+ BUILD_BUG();
+ return NULL;
+ }
+}
+
+static __always_inline u32 *cpuid_entry_get_reg(struct kvm_cpuid_entry2 *entry,
+ unsigned int x86_feature)
+{
+ const struct cpuid_reg cpuid = x86_feature_cpuid(x86_feature);
+
+ return __cpuid_entry_get_reg(entry, cpuid.reg);
+}
+
+static __always_inline u32 cpuid_entry_get(struct kvm_cpuid_entry2 *entry,
+ unsigned int x86_feature)
+{
+ u32 *reg = cpuid_entry_get_reg(entry, x86_feature);
+
+ return *reg & __feature_bit(x86_feature);
+}
+
+static __always_inline bool cpuid_entry_has(struct kvm_cpuid_entry2 *entry,
+ unsigned int x86_feature)
+{
+ return cpuid_entry_get(entry, x86_feature);
+}
+
+static __always_inline void cpuid_entry_clear(struct kvm_cpuid_entry2 *entry,
+ unsigned int x86_feature)
+{
+ u32 *reg = cpuid_entry_get_reg(entry, x86_feature);
+
+ *reg &= ~__feature_bit(x86_feature);
+}
+
+static __always_inline void cpuid_entry_set(struct kvm_cpuid_entry2 *entry,
+ unsigned int x86_feature)
+{
+ u32 *reg = cpuid_entry_get_reg(entry, x86_feature);
+
+ *reg |= __feature_bit(x86_feature);
+}
+
+static __always_inline void cpuid_entry_change(struct kvm_cpuid_entry2 *entry,
+ unsigned int x86_feature,
+ bool set)
+{
+ u32 *reg = cpuid_entry_get_reg(entry, x86_feature);
+
+ /*
+ * Open coded instead of using cpuid_entry_{clear,set}() to coerce the
+ * compiler into using CMOV instead of Jcc when possible.
+ */
+ if (set)
+ *reg |= __feature_bit(x86_feature);
+ else
+ *reg &= ~__feature_bit(x86_feature);
+}
+
+#endif /* ARCH_X86_KVM_REVERSE_CPUID_H */
--
2.31.1.368.gbe11c130af-goog

2021-04-22 01:31:51

by Ricardo Koller

[permalink] [raw]
Subject: [PATCH 2/5] x86/cpu: Expose CPUID regs, leaf and index definitions to tools

Move cpuid_regs, cpuid_regs_idx, and cpuid_leafs out of their
'#ifdef __KERNEL__' guards so that KVM selftests can reuse the
definitions in future patches. Move cpuid_regs and cpuid_regs_idx from
processor.h to cpufeature.h to avoid blasting processor.h with several
'#ifdefs'.

Suggested-by: Sean Christopherson <[email protected]>
Signed-off-by: Ricardo Koller <[email protected]>
---
arch/x86/events/intel/pt.c | 1 +
arch/x86/include/asm/cpufeature.h | 23 ++++++++++++++++++-----
arch/x86/include/asm/processor.h | 11 -----------
arch/x86/kernel/cpu/scattered.c | 2 +-
arch/x86/kernel/cpuid.c | 2 +-
5 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index e94af4a54d0d..882b1478556e 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -21,6 +21,7 @@
#include <asm/io.h>
#include <asm/intel_pt.h>
#include <asm/intel-family.h>
+#include <asm/cpufeature.h>

#include "../perf_event.h"
#include "pt.h"
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 1728d4ce5730..22458ab5aac4 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -2,12 +2,19 @@
#ifndef _ASM_X86_CPUFEATURE_H
#define _ASM_X86_CPUFEATURE_H

-#include <asm/processor.h>
+#include <linux/types.h>

-#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
+#ifndef __ASSEMBLY__
+struct cpuid_regs {
+ u32 eax, ebx, ecx, edx;
+};

-#include <asm/asm.h>
-#include <linux/bitops.h>
+enum cpuid_regs_idx {
+ CPUID_EAX = 0,
+ CPUID_EBX,
+ CPUID_ECX,
+ CPUID_EDX,
+};

enum cpuid_leafs
{
@@ -32,6 +39,11 @@ enum cpuid_leafs
CPUID_7_EDX,
CPUID_8000_001F_EAX,
};
+#ifdef __KERNEL__
+
+#include <asm/processor.h>
+#include <asm/asm.h>
+#include <linux/bitops.h>

#ifdef CONFIG_X86_FEATURE_NAMES
extern const char * const x86_cap_flags[NCAPINTS*32];
@@ -240,5 +252,6 @@ static __always_inline bool _static_cpu_has(u16 bit)
#define CPU_FEATURE_TYPEVAL boot_cpu_data.x86_vendor, boot_cpu_data.x86, \
boot_cpu_data.x86_model

-#endif /* defined(__KERNEL__) && !defined(__ASSEMBLY__) */
+#endif /* defined(__KERNEL__) */
+#endif /* !defined(__ASSEMBLY__) */
#endif /* _ASM_X86_CPUFEATURE_H */
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index dc6d149bf851..bc7fa3de7ccc 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -142,17 +142,6 @@ struct cpuinfo_x86 {
unsigned initialized : 1;
} __randomize_layout;

-struct cpuid_regs {
- u32 eax, ebx, ecx, edx;
-};
-
-enum cpuid_regs_idx {
- CPUID_EAX = 0,
- CPUID_EBX,
- CPUID_ECX,
- CPUID_EDX,
-};
-
#define X86_VENDOR_INTEL 0
#define X86_VENDOR_CYRIX 1
#define X86_VENDOR_AMD 2
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 21d1f062895a..bcbcda1e329b 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -6,7 +6,7 @@

#include <asm/memtype.h>
#include <asm/apic.h>
-#include <asm/processor.h>
+#include <asm/cpufeature.h>

#include "cpu.h"

diff --git a/arch/x86/kernel/cpuid.c b/arch/x86/kernel/cpuid.c
index 6f7b8cc1bc9f..23e67220445b 100644
--- a/arch/x86/kernel/cpuid.c
+++ b/arch/x86/kernel/cpuid.c
@@ -37,7 +37,7 @@
#include <linux/gfp.h>
#include <linux/completion.h>

-#include <asm/processor.h>
+#include <asm/cpufeature.h>
#include <asm/msr.h>

static struct class *cpuid_class;
--
2.31.1.368.gbe11c130af-goog

2021-04-22 01:31:54

by Ricardo Koller

[permalink] [raw]
Subject: [PATCH 3/5] tools headers x86: Copy cpuid helpers from the kernel

Copy arch/x86/include/asm/acpufeature.h and arch/x86/kvm/reverse_cpuid.h
from the kernel so that KVM selftests can use them in the next commits.
Also update the tools copy of arch/x86/include/asm/acpufeatures.h.

cpufeature.h is copied into tools/arch/x86/include like most other
headers. reverse_cpuid.h is a special case as it's copied into the KVM
selftests include location: tools/testing/selftests/kvm/include/x86_64/.
These should be kept in sync, ideally with the help of some script like
check-headers.sh used by tools/perf/.

Signed-off-by: Ricardo Koller <[email protected]>
---
tools/arch/x86/include/asm/cpufeature.h | 257 ++++++++++++++++++
tools/arch/x86/include/asm/cpufeatures.h | 3 +
.../kvm/include/x86_64/reverse_cpuid.h | 185 +++++++++++++
3 files changed, 445 insertions(+)
create mode 100644 tools/arch/x86/include/asm/cpufeature.h
create mode 100644 tools/testing/selftests/kvm/include/x86_64/reverse_cpuid.h

diff --git a/tools/arch/x86/include/asm/cpufeature.h b/tools/arch/x86/include/asm/cpufeature.h
new file mode 100644
index 000000000000..22458ab5aac4
--- /dev/null
+++ b/tools/arch/x86/include/asm/cpufeature.h
@@ -0,0 +1,257 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_CPUFEATURE_H
+#define _ASM_X86_CPUFEATURE_H
+
+#include <linux/types.h>
+
+#ifndef __ASSEMBLY__
+struct cpuid_regs {
+ u32 eax, ebx, ecx, edx;
+};
+
+enum cpuid_regs_idx {
+ CPUID_EAX = 0,
+ CPUID_EBX,
+ CPUID_ECX,
+ CPUID_EDX,
+};
+
+enum cpuid_leafs
+{
+ CPUID_1_EDX = 0,
+ CPUID_8000_0001_EDX,
+ CPUID_8086_0001_EDX,
+ CPUID_LNX_1,
+ CPUID_1_ECX,
+ CPUID_C000_0001_EDX,
+ CPUID_8000_0001_ECX,
+ CPUID_LNX_2,
+ CPUID_LNX_3,
+ CPUID_7_0_EBX,
+ CPUID_D_1_EAX,
+ CPUID_LNX_4,
+ CPUID_7_1_EAX,
+ CPUID_8000_0008_EBX,
+ CPUID_6_EAX,
+ CPUID_8000_000A_EDX,
+ CPUID_7_ECX,
+ CPUID_8000_0007_EBX,
+ CPUID_7_EDX,
+ CPUID_8000_001F_EAX,
+};
+#ifdef __KERNEL__
+
+#include <asm/processor.h>
+#include <asm/asm.h>
+#include <linux/bitops.h>
+
+#ifdef CONFIG_X86_FEATURE_NAMES
+extern const char * const x86_cap_flags[NCAPINTS*32];
+extern const char * const x86_power_flags[32];
+#define X86_CAP_FMT "%s"
+#define x86_cap_flag(flag) x86_cap_flags[flag]
+#else
+#define X86_CAP_FMT "%d:%d"
+#define x86_cap_flag(flag) ((flag) >> 5), ((flag) & 31)
+#endif
+
+/*
+ * In order to save room, we index into this array by doing
+ * X86_BUG_<name> - NCAPINTS*32.
+ */
+extern const char * const x86_bug_flags[NBUGINTS*32];
+
+#define test_cpu_cap(c, bit) \
+ test_bit(bit, (unsigned long *)((c)->x86_capability))
+
+/*
+ * There are 32 bits/features in each mask word. The high bits
+ * (selected with (bit>>5) give us the word number and the low 5
+ * bits give us the bit/feature number inside the word.
+ * (1UL<<((bit)&31) gives us a mask for the feature_bit so we can
+ * see if it is set in the mask word.
+ */
+#define CHECK_BIT_IN_MASK_WORD(maskname, word, bit) \
+ (((bit)>>5)==(word) && (1UL<<((bit)&31) & maskname##word ))
+
+/*
+ * {REQUIRED,DISABLED}_MASK_CHECK below may seem duplicated with the
+ * following BUILD_BUG_ON_ZERO() check but when NCAPINTS gets changed, all
+ * header macros which use NCAPINTS need to be changed. The duplicated macro
+ * use causes the compiler to issue errors for all headers so that all usage
+ * sites can be corrected.
+ */
+#define REQUIRED_MASK_BIT_SET(feature_bit) \
+ ( CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 0, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 1, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 2, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 3, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 4, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 5, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 6, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 7, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 8, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 9, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 10, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 11, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 12, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 13, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 14, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 15, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 16, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 17, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 18, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 19, feature_bit) || \
+ REQUIRED_MASK_CHECK || \
+ BUILD_BUG_ON_ZERO(NCAPINTS != 20))
+
+#define DISABLED_MASK_BIT_SET(feature_bit) \
+ ( CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 0, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 1, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 2, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 3, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 4, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 5, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 6, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 7, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 8, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 9, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 10, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 11, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 12, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 13, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 14, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 15, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 16, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 17, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 18, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 19, feature_bit) || \
+ DISABLED_MASK_CHECK || \
+ BUILD_BUG_ON_ZERO(NCAPINTS != 20))
+
+#define cpu_has(c, bit) \
+ (__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \
+ test_cpu_cap(c, bit))
+
+#define this_cpu_has(bit) \
+ (__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \
+ x86_this_cpu_test_bit(bit, \
+ (unsigned long __percpu *)&cpu_info.x86_capability))
+
+/*
+ * This macro is for detection of features which need kernel
+ * infrastructure to be used. It may *not* directly test the CPU
+ * itself. Use the cpu_has() family if you want true runtime
+ * testing of CPU features, like in hypervisor code where you are
+ * supporting a possible guest feature where host support for it
+ * is not relevant.
+ */
+#define cpu_feature_enabled(bit) \
+ (__builtin_constant_p(bit) && DISABLED_MASK_BIT_SET(bit) ? 0 : static_cpu_has(bit))
+
+#define boot_cpu_has(bit) cpu_has(&boot_cpu_data, bit)
+
+#define set_cpu_cap(c, bit) set_bit(bit, (unsigned long *)((c)->x86_capability))
+
+extern void setup_clear_cpu_cap(unsigned int bit);
+extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);
+
+#define setup_force_cpu_cap(bit) do { \
+ set_cpu_cap(&boot_cpu_data, bit); \
+ set_bit(bit, (unsigned long *)cpu_caps_set); \
+} while (0)
+
+#define setup_force_cpu_bug(bit) setup_force_cpu_cap(bit)
+
+#if defined(__clang__) && !defined(CONFIG_CC_HAS_ASM_GOTO)
+
+/*
+ * Workaround for the sake of BPF compilation which utilizes kernel
+ * headers, but clang does not support ASM GOTO and fails the build.
+ */
+#ifndef __BPF_TRACING__
+#warning "Compiler lacks ASM_GOTO support. Add -D __BPF_TRACING__ to your compiler arguments"
+#endif
+
+#define static_cpu_has(bit) boot_cpu_has(bit)
+
+#else
+
+/*
+ * Static testing of CPU features. Used the same as boot_cpu_has(). It
+ * statically patches the target code for additional performance. Use
+ * static_cpu_has() only in fast paths, where every cycle counts. Which
+ * means that the boot_cpu_has() variant is already fast enough for the
+ * majority of cases and you should stick to using it as it is generally
+ * only two instructions: a RIP-relative MOV and a TEST.
+ */
+static __always_inline bool _static_cpu_has(u16 bit)
+{
+ asm_volatile_goto("1: jmp 6f\n"
+ "2:\n"
+ ".skip -(((5f-4f) - (2b-1b)) > 0) * "
+ "((5f-4f) - (2b-1b)),0x90\n"
+ "3:\n"
+ ".section .altinstructions,\"a\"\n"
+ " .long 1b - .\n" /* src offset */
+ " .long 4f - .\n" /* repl offset */
+ " .word %P[always]\n" /* always replace */
+ " .byte 3b - 1b\n" /* src len */
+ " .byte 5f - 4f\n" /* repl len */
+ " .byte 3b - 2b\n" /* pad len */
+ ".previous\n"
+ ".section .altinstr_replacement,\"ax\"\n"
+ "4: jmp %l[t_no]\n"
+ "5:\n"
+ ".previous\n"
+ ".section .altinstructions,\"a\"\n"
+ " .long 1b - .\n" /* src offset */
+ " .long 0\n" /* no replacement */
+ " .word %P[feature]\n" /* feature bit */
+ " .byte 3b - 1b\n" /* src len */
+ " .byte 0\n" /* repl len */
+ " .byte 0\n" /* pad len */
+ ".previous\n"
+ ".section .altinstr_aux,\"ax\"\n"
+ "6:\n"
+ " testb %[bitnum],%[cap_byte]\n"
+ " jnz %l[t_yes]\n"
+ " jmp %l[t_no]\n"
+ ".previous\n"
+ : : [feature] "i" (bit),
+ [always] "i" (X86_FEATURE_ALWAYS),
+ [bitnum] "i" (1 << (bit & 7)),
+ [cap_byte] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])
+ : : t_yes, t_no);
+t_yes:
+ return true;
+t_no:
+ return false;
+}
+
+#define static_cpu_has(bit) \
+( \
+ __builtin_constant_p(boot_cpu_has(bit)) ? \
+ boot_cpu_has(bit) : \
+ _static_cpu_has(bit) \
+)
+#endif
+
+#define cpu_has_bug(c, bit) cpu_has(c, (bit))
+#define set_cpu_bug(c, bit) set_cpu_cap(c, (bit))
+#define clear_cpu_bug(c, bit) clear_cpu_cap(c, (bit))
+
+#define static_cpu_has_bug(bit) static_cpu_has((bit))
+#define boot_cpu_has_bug(bit) cpu_has_bug(&boot_cpu_data, (bit))
+#define boot_cpu_set_bug(bit) set_cpu_cap(&boot_cpu_data, (bit))
+
+#define MAX_CPU_FEATURES (NCAPINTS * 32)
+#define cpu_have_feature boot_cpu_has
+
+#define CPU_FEATURE_TYPEFMT "x86,ven%04Xfam%04Xmod%04X"
+#define CPU_FEATURE_TYPEVAL boot_cpu_data.x86_vendor, boot_cpu_data.x86, \
+ boot_cpu_data.x86_model
+
+#endif /* defined(__KERNEL__) */
+#endif /* !defined(__ASSEMBLY__) */
+#endif /* _ASM_X86_CPUFEATURE_H */
diff --git a/tools/arch/x86/include/asm/cpufeatures.h b/tools/arch/x86/include/asm/cpufeatures.h
index cc96e26d69f7..dddc746b5455 100644
--- a/tools/arch/x86/include/asm/cpufeatures.h
+++ b/tools/arch/x86/include/asm/cpufeatures.h
@@ -290,6 +290,8 @@
#define X86_FEATURE_FENCE_SWAPGS_KERNEL (11*32+ 5) /* "" LFENCE in kernel entry SWAPGS path */
#define X86_FEATURE_SPLIT_LOCK_DETECT (11*32+ 6) /* #AC for split lock */
#define X86_FEATURE_PER_THREAD_MBA (11*32+ 7) /* "" Per-thread Memory Bandwidth Allocation */
+#define X86_FEATURE_SGX1 (11*32+ 8) /* "" Basic SGX */
+#define X86_FEATURE_SGX2 (11*32+ 9) /* "" SGX Enclave Dynamic Memory Management (EDMM) */

/* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
#define X86_FEATURE_AVX_VNNI (12*32+ 4) /* AVX VNNI instructions */
@@ -336,6 +338,7 @@
#define X86_FEATURE_AVIC (15*32+13) /* Virtual Interrupt Controller */
#define X86_FEATURE_V_VMSAVE_VMLOAD (15*32+15) /* Virtual VMSAVE VMLOAD */
#define X86_FEATURE_VGIF (15*32+16) /* Virtual GIF */
+#define X86_FEATURE_V_SPEC_CTRL (15*32+20) /* Virtual SPEC_CTRL */
#define X86_FEATURE_SVME_ADDR_CHK (15*32+28) /* "" SVME addr check */

/* Intel-defined CPU features, CPUID level 0x00000007:0 (ECX), word 16 */
diff --git a/tools/testing/selftests/kvm/include/x86_64/reverse_cpuid.h b/tools/testing/selftests/kvm/include/x86_64/reverse_cpuid.h
new file mode 100644
index 000000000000..8e0756ddab1a
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/x86_64/reverse_cpuid.h
@@ -0,0 +1,185 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef ARCH_X86_KVM_REVERSE_CPUID_H
+#define ARCH_X86_KVM_REVERSE_CPUID_H
+
+#include <uapi/asm/kvm.h>
+#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
+
+/*
+ * Hardware-defined CPUID leafs that are scattered in the kernel, but need to
+ * be directly used by KVM. Note, these word values conflict with the kernel's
+ * "bug" caps, but KVM doesn't use those.
+ */
+enum kvm_only_cpuid_leafs {
+ CPUID_12_EAX = NCAPINTS,
+ NR_KVM_CPU_CAPS,
+
+ NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
+};
+
+#define KVM_X86_FEATURE(w, f) ((w)*32 + (f))
+
+/* Intel-defined SGX sub-features, CPUID level 0x12 (EAX). */
+#define KVM_X86_FEATURE_SGX1 KVM_X86_FEATURE(CPUID_12_EAX, 0)
+#define KVM_X86_FEATURE_SGX2 KVM_X86_FEATURE(CPUID_12_EAX, 1)
+
+struct cpuid_reg {
+ u32 function;
+ u32 index;
+ int reg;
+};
+
+static const struct cpuid_reg reverse_cpuid[] = {
+ [CPUID_1_EDX] = { 1, 0, CPUID_EDX},
+ [CPUID_8000_0001_EDX] = {0x80000001, 0, CPUID_EDX},
+ [CPUID_8086_0001_EDX] = {0x80860001, 0, CPUID_EDX},
+ [CPUID_1_ECX] = { 1, 0, CPUID_ECX},
+ [CPUID_C000_0001_EDX] = {0xc0000001, 0, CPUID_EDX},
+ [CPUID_8000_0001_ECX] = {0x80000001, 0, CPUID_ECX},
+ [CPUID_7_0_EBX] = { 7, 0, CPUID_EBX},
+ [CPUID_D_1_EAX] = { 0xd, 1, CPUID_EAX},
+ [CPUID_8000_0008_EBX] = {0x80000008, 0, CPUID_EBX},
+ [CPUID_6_EAX] = { 6, 0, CPUID_EAX},
+ [CPUID_8000_000A_EDX] = {0x8000000a, 0, CPUID_EDX},
+ [CPUID_7_ECX] = { 7, 0, CPUID_ECX},
+ [CPUID_8000_0007_EBX] = {0x80000007, 0, CPUID_EBX},
+ [CPUID_7_EDX] = { 7, 0, CPUID_EDX},
+ [CPUID_7_1_EAX] = { 7, 1, CPUID_EAX},
+ [CPUID_12_EAX] = {0x00000012, 0, CPUID_EAX},
+};
+
+/*
+ * Reverse CPUID and its derivatives can only be used for hardware-defined
+ * feature words, i.e. words whose bits directly correspond to a CPUID leaf.
+ * Retrieving a feature bit or masking guest CPUID from a Linux-defined word
+ * is nonsensical as the bit number/mask is an arbitrary software-defined value
+ * and can't be used by KVM to query/control guest capabilities. And obviously
+ * the leaf being queried must have an entry in the lookup table.
+ */
+static __always_inline void reverse_cpuid_check(unsigned int x86_leaf)
+{
+ BUILD_BUG_ON(x86_leaf == CPUID_LNX_1);
+ BUILD_BUG_ON(x86_leaf == CPUID_LNX_2);
+ BUILD_BUG_ON(x86_leaf == CPUID_LNX_3);
+ BUILD_BUG_ON(x86_leaf == CPUID_LNX_4);
+ BUILD_BUG_ON(x86_leaf >= ARRAY_SIZE(reverse_cpuid));
+ BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0);
+}
+
+/*
+ * Translate feature bits that are scattered in the kernel's cpufeatures word
+ * into KVM feature words that align with hardware's definitions.
+ */
+static __always_inline u32 __feature_translate(int x86_feature)
+{
+ if (x86_feature == X86_FEATURE_SGX1)
+ return KVM_X86_FEATURE_SGX1;
+ else if (x86_feature == X86_FEATURE_SGX2)
+ return KVM_X86_FEATURE_SGX2;
+
+ return x86_feature;
+}
+
+static __always_inline u32 __feature_leaf(int x86_feature)
+{
+ return __feature_translate(x86_feature) / 32;
+}
+
+/*
+ * Retrieve the bit mask from an X86_FEATURE_* definition. Features contain
+ * the hardware defined bit number (stored in bits 4:0) and a software defined
+ * "word" (stored in bits 31:5). The word is used to index into arrays of
+ * bit masks that hold the per-cpu feature capabilities, e.g. this_cpu_has().
+ */
+static __always_inline u32 __feature_bit(int x86_feature)
+{
+ x86_feature = __feature_translate(x86_feature);
+
+ reverse_cpuid_check(x86_feature / 32);
+ return 1 << (x86_feature & 31);
+}
+
+#define feature_bit(name) __feature_bit(X86_FEATURE_##name)
+
+static __always_inline struct cpuid_reg x86_feature_cpuid(unsigned int x86_feature)
+{
+ unsigned int x86_leaf = __feature_leaf(x86_feature);
+
+ reverse_cpuid_check(x86_leaf);
+ return reverse_cpuid[x86_leaf];
+}
+
+static __always_inline u32 *__cpuid_entry_get_reg(struct kvm_cpuid_entry2 *entry,
+ u32 reg)
+{
+ switch (reg) {
+ case CPUID_EAX:
+ return &entry->eax;
+ case CPUID_EBX:
+ return &entry->ebx;
+ case CPUID_ECX:
+ return &entry->ecx;
+ case CPUID_EDX:
+ return &entry->edx;
+ default:
+ BUILD_BUG();
+ return NULL;
+ }
+}
+
+static __always_inline u32 *cpuid_entry_get_reg(struct kvm_cpuid_entry2 *entry,
+ unsigned int x86_feature)
+{
+ const struct cpuid_reg cpuid = x86_feature_cpuid(x86_feature);
+
+ return __cpuid_entry_get_reg(entry, cpuid.reg);
+}
+
+static __always_inline u32 cpuid_entry_get(struct kvm_cpuid_entry2 *entry,
+ unsigned int x86_feature)
+{
+ u32 *reg = cpuid_entry_get_reg(entry, x86_feature);
+
+ return *reg & __feature_bit(x86_feature);
+}
+
+static __always_inline bool cpuid_entry_has(struct kvm_cpuid_entry2 *entry,
+ unsigned int x86_feature)
+{
+ return cpuid_entry_get(entry, x86_feature);
+}
+
+static __always_inline void cpuid_entry_clear(struct kvm_cpuid_entry2 *entry,
+ unsigned int x86_feature)
+{
+ u32 *reg = cpuid_entry_get_reg(entry, x86_feature);
+
+ *reg &= ~__feature_bit(x86_feature);
+}
+
+static __always_inline void cpuid_entry_set(struct kvm_cpuid_entry2 *entry,
+ unsigned int x86_feature)
+{
+ u32 *reg = cpuid_entry_get_reg(entry, x86_feature);
+
+ *reg |= __feature_bit(x86_feature);
+}
+
+static __always_inline void cpuid_entry_change(struct kvm_cpuid_entry2 *entry,
+ unsigned int x86_feature,
+ bool set)
+{
+ u32 *reg = cpuid_entry_get_reg(entry, x86_feature);
+
+ /*
+ * Open coded instead of using cpuid_entry_{clear,set}() to coerce the
+ * compiler into using CMOV instead of Jcc when possible.
+ */
+ if (set)
+ *reg |= __feature_bit(x86_feature);
+ else
+ *reg &= ~__feature_bit(x86_feature);
+}
+
+#endif /* ARCH_X86_KVM_REVERSE_CPUID_H */
--
2.31.1.368.gbe11c130af-goog

2021-04-22 01:32:00

by Ricardo Koller

[permalink] [raw]
Subject: [PATCH 5/5] KVM: selftests: Use kernel x86 cpuid features format

Change all tests checking for x86 cpuid features to use the same cpuid
feature definitions as the kernel (from cpufeatures.h). Also change the
tests to use the utilities introduced in cpuid.h.

Signed-off-by: Ricardo Koller <[email protected]>
---
.../selftests/kvm/include/x86_64/processor.h | 16 ------------
.../selftests/kvm/include/x86_64/svm_util.h | 11 ++------
tools/testing/selftests/kvm/lib/x86_64/svm.c | 6 ++---
tools/testing/selftests/kvm/lib/x86_64/vmx.c | 5 ++--
tools/testing/selftests/kvm/steal_time.c | 5 ++--
.../kvm/x86_64/cr4_cpuid_sync_test.c | 23 +++++------------
.../selftests/kvm/x86_64/set_sregs_test.c | 25 ++++++++-----------
.../selftests/kvm/x86_64/vmx_pmu_msrs_test.c | 8 +++---
.../kvm/x86_64/vmx_set_nested_state_test.c | 5 ++--
.../selftests/kvm/x86_64/xss_msr_test.c | 10 +++-----
10 files changed, 36 insertions(+), 78 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index 0b30b4e15c38..022e00b04fff 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -37,22 +37,6 @@
#define X86_CR4_SMAP (1ul << 21)
#define X86_CR4_PKE (1ul << 22)

-/* CPUID.1.ECX */
-#define CPUID_VMX (1ul << 5)
-#define CPUID_SMX (1ul << 6)
-#define CPUID_PCID (1ul << 17)
-#define CPUID_XSAVE (1ul << 26)
-
-/* CPUID.7.EBX */
-#define CPUID_FSGSBASE (1ul << 0)
-#define CPUID_SMEP (1ul << 7)
-#define CPUID_SMAP (1ul << 20)
-
-/* CPUID.7.ECX */
-#define CPUID_UMIP (1ul << 2)
-#define CPUID_PKU (1ul << 3)
-#define CPUID_LA57 (1ul << 16)
-
#define UNEXPECTED_VECTOR_PORT 0xfff0u

/* General Registers in 64-Bit Mode */
diff --git a/tools/testing/selftests/kvm/include/x86_64/svm_util.h b/tools/testing/selftests/kvm/include/x86_64/svm_util.h
index b7531c83b8ae..adba82ff4c9b 100644
--- a/tools/testing/selftests/kvm/include/x86_64/svm_util.h
+++ b/tools/testing/selftests/kvm/include/x86_64/svm_util.h
@@ -12,9 +12,7 @@
#include <stdint.h>
#include "svm.h"
#include "processor.h"
-
-#define CPUID_SVM_BIT 2
-#define CPUID_SVM BIT_ULL(CPUID_SVM_BIT)
+#include "cpuid.h"

#define SVM_EXIT_VMMCALL 0x081

@@ -38,12 +36,7 @@ void nested_svm_check_supported(void);

static inline bool cpu_has_svm(void)
{
- u32 eax = 0x80000001, ecx;
-
- asm("cpuid" :
- "=a" (eax), "=c" (ecx) : "0" (eax) : "ebx", "edx");
-
- return ecx & CPUID_SVM;
+ return this_cpu_has(X86_FEATURE_SVM);
}

#endif /* SELFTEST_KVM_SVM_UTILS_H */
diff --git a/tools/testing/selftests/kvm/lib/x86_64/svm.c b/tools/testing/selftests/kvm/lib/x86_64/svm.c
index 827fe6028dd4..c68245233cf9 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/svm.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/svm.c
@@ -12,6 +12,7 @@
#include "../kvm_util_internal.h"
#include "processor.h"
#include "svm_util.h"
+#include "cpuid.h"

struct gpr64_regs guest_regs;
u64 rflags;
@@ -150,10 +151,7 @@ void run_guest(struct vmcb *vmcb, uint64_t vmcb_gpa)

bool nested_svm_supported(void)
{
- struct kvm_cpuid_entry2 *entry =
- kvm_get_supported_cpuid_entry(0x80000001);
-
- return entry->ecx & CPUID_SVM;
+ return kvm_cpuid_has(X86_FEATURE_SVM);
}

void nested_svm_check_supported(void)
diff --git a/tools/testing/selftests/kvm/lib/x86_64/vmx.c b/tools/testing/selftests/kvm/lib/x86_64/vmx.c
index 2448b30e8efa..be26dcd260a4 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/vmx.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/vmx.c
@@ -10,6 +10,7 @@
#include "../kvm_util_internal.h"
#include "processor.h"
#include "vmx.h"
+#include "cpuid.h"

#define PAGE_SHIFT_4K 12

@@ -381,9 +382,7 @@ void prepare_vmcs(struct vmx_pages *vmx, void *guest_rip, void *guest_rsp)

bool nested_vmx_supported(void)
{
- struct kvm_cpuid_entry2 *entry = kvm_get_supported_cpuid_entry(1);
-
- return entry->ecx & CPUID_VMX;
+ return kvm_cpuid_has(X86_FEATURE_VMX);
}

void nested_vmx_check_supported(void)
diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
index fcc840088c91..04d86601de92 100644
--- a/tools/testing/selftests/kvm/steal_time.c
+++ b/tools/testing/selftests/kvm/steal_time.c
@@ -27,6 +27,8 @@ static uint64_t guest_stolen_time[NR_VCPUS];

#if defined(__x86_64__)

+#include "cpuid.h"
+
/* steal_time must have 64-byte alignment */
#define STEAL_TIME_SIZE ((sizeof(struct kvm_steal_time) + 63) & ~63)

@@ -64,8 +66,7 @@ static void steal_time_init(struct kvm_vm *vm)
{
int i;

- if (!(kvm_get_supported_cpuid_entry(KVM_CPUID_FEATURES)->eax &
- KVM_FEATURE_STEAL_TIME)) {
+ if (!kvm_pv_has(KVM_FEATURE_STEAL_TIME)) {
print_skip("steal-time not supported");
exit(KSFT_SKIP);
}
diff --git a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
index f40fd097cb35..97e97b258983 100644
--- a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
+++ b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
@@ -18,26 +18,17 @@

#include "kvm_util.h"
#include "processor.h"
+#include "cpuid.h"

-#define X86_FEATURE_XSAVE (1<<26)
-#define X86_FEATURE_OSXSAVE (1<<27)
#define VCPU_ID 1

static inline bool cr4_cpuid_is_sync(void)
{
- int func, subfunc;
- uint32_t eax, ebx, ecx, edx;
- uint64_t cr4;
-
- func = 0x1;
- subfunc = 0x0;
- __asm__ __volatile__("cpuid"
- : "=a"(eax), "=b"(ebx), "=c"(ecx), "=d"(edx)
- : "a"(func), "c"(subfunc));
-
- cr4 = get_cr4();
+ uint64_t cr4 = get_cr4();
+ bool cpuid_has_osxsave = this_cpu_has(X86_FEATURE_OSXSAVE);
+ bool cr4_has_osxsave = cr4 & X86_CR4_OSXSAVE;

- return (!!(ecx & X86_FEATURE_OSXSAVE)) == (!!(cr4 & X86_CR4_OSXSAVE));
+ return cpuid_has_osxsave == cr4_has_osxsave;
}

static void guest_code(void)
@@ -66,12 +57,10 @@ int main(int argc, char *argv[])
struct kvm_run *run;
struct kvm_vm *vm;
struct kvm_sregs sregs;
- struct kvm_cpuid_entry2 *entry;
struct ucall uc;
int rc;

- entry = kvm_get_supported_cpuid_entry(1);
- if (!(entry->ecx & X86_FEATURE_XSAVE)) {
+ if (!kvm_cpuid_has(X86_FEATURE_XSAVE)) {
print_skip("XSAVE feature not supported");
return 0;
}
diff --git a/tools/testing/selftests/kvm/x86_64/set_sregs_test.c b/tools/testing/selftests/kvm/x86_64/set_sregs_test.c
index 318be0bf77ab..e3247f33d765 100644
--- a/tools/testing/selftests/kvm/x86_64/set_sregs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/set_sregs_test.c
@@ -21,6 +21,7 @@

#include "kvm_util.h"
#include "processor.h"
+#include "cpuid.h"

#define VCPU_ID 5

@@ -47,34 +48,30 @@ static void test_cr4_feature_bit(struct kvm_vm *vm, struct kvm_sregs *orig,

static uint64_t calc_cr4_feature_bits(struct kvm_vm *vm)
{
- struct kvm_cpuid_entry2 *cpuid_1, *cpuid_7;
uint64_t cr4;

- cpuid_1 = kvm_get_supported_cpuid_entry(1);
- cpuid_7 = kvm_get_supported_cpuid_entry(7);
-
cr4 = X86_CR4_VME | X86_CR4_PVI | X86_CR4_TSD | X86_CR4_DE |
X86_CR4_PSE | X86_CR4_PAE | X86_CR4_MCE | X86_CR4_PGE |
X86_CR4_PCE | X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT;
- if (cpuid_7->ecx & CPUID_UMIP)
+ if (kvm_cpuid_has(X86_FEATURE_UMIP))
cr4 |= X86_CR4_UMIP;
- if (cpuid_7->ecx & CPUID_LA57)
+ if (kvm_cpuid_has(X86_FEATURE_LA57))
cr4 |= X86_CR4_LA57;
- if (cpuid_1->ecx & CPUID_VMX)
+ if (kvm_cpuid_has(X86_FEATURE_VMX))
cr4 |= X86_CR4_VMXE;
- if (cpuid_1->ecx & CPUID_SMX)
+ if (kvm_cpuid_has(X86_FEATURE_SMX))
cr4 |= X86_CR4_SMXE;
- if (cpuid_7->ebx & CPUID_FSGSBASE)
+ if (kvm_cpuid_has(X86_FEATURE_FSGSBASE))
cr4 |= X86_CR4_FSGSBASE;
- if (cpuid_1->ecx & CPUID_PCID)
+ if (kvm_cpuid_has(X86_FEATURE_PCID))
cr4 |= X86_CR4_PCIDE;
- if (cpuid_1->ecx & CPUID_XSAVE)
+ if (kvm_cpuid_has(X86_FEATURE_XSAVE))
cr4 |= X86_CR4_OSXSAVE;
- if (cpuid_7->ebx & CPUID_SMEP)
+ if (kvm_cpuid_has(X86_FEATURE_SMEP))
cr4 |= X86_CR4_SMEP;
- if (cpuid_7->ebx & CPUID_SMAP)
+ if (kvm_cpuid_has(X86_FEATURE_SMAP))
cr4 |= X86_CR4_SMAP;
- if (cpuid_7->ecx & CPUID_PKU)
+ if (kvm_cpuid_has(X86_FEATURE_PKU))
cr4 |= X86_CR4_PKE;

return cr4;
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_pmu_msrs_test.c b/tools/testing/selftests/kvm/x86_64/vmx_pmu_msrs_test.c
index 23051d84b907..3755451f4877 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_pmu_msrs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_pmu_msrs_test.c
@@ -17,10 +17,10 @@

#include "kvm_util.h"
#include "vmx.h"
+#include "cpuid.h"

#define VCPU_ID 0

-#define X86_FEATURE_PDCM (1<<15)
#define PMU_CAP_FW_WRITES (1ULL << 13)
#define PMU_CAP_LBR_FMT 0x3f

@@ -76,7 +76,7 @@ int main(int argc, char *argv[])
if (kvm_get_cpuid_max_basic() >= 0xa) {
entry_1_0 = kvm_get_supported_cpuid_index(1, 0);
entry_a_0 = kvm_get_supported_cpuid_index(0xa, 0);
- pdcm_supported = entry_1_0 && !!(entry_1_0->ecx & X86_FEATURE_PDCM);
+ pdcm_supported = kvm_cpuid_has(X86_FEATURE_PDCM);
eax.full = entry_a_0->eax;
}
if (!pdcm_supported) {
@@ -111,13 +111,13 @@ int main(int argc, char *argv[])
TEST_ASSERT(ret == 0, "Bad PERF_CAPABILITIES didn't fail.");

/* testcase 4, set capabilities when we don't have PDCM bit */
- entry_1_0->ecx &= ~X86_FEATURE_PDCM;
+ entry_1_0->ecx &= ~feature_bit(PDCM);
vcpu_set_cpuid(vm, VCPU_ID, cpuid);
ret = _vcpu_set_msr(vm, 0, MSR_IA32_PERF_CAPABILITIES, host_cap.capabilities);
TEST_ASSERT(ret == 0, "Bad PERF_CAPABILITIES didn't fail.");

/* testcase 5, set capabilities when we don't have PMU version bits */
- entry_1_0->ecx |= X86_FEATURE_PDCM;
+ entry_1_0->ecx |= feature_bit(PDCM);
eax.split.version_id = 0;
entry_1_0->ecx = eax.full;
vcpu_set_cpuid(vm, VCPU_ID, cpuid);
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
index 5827b9bae468..bea74c9ef0f7 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
@@ -11,6 +11,7 @@
#include "kvm_util.h"
#include "processor.h"
#include "vmx.h"
+#include "cpuid.h"

#include <errno.h>
#include <linux/kvm.h>
@@ -255,9 +256,9 @@ void disable_vmx(struct kvm_vm *vm)
break;
TEST_ASSERT(i != cpuid->nent, "CPUID function 1 not found");

- cpuid->entries[i].ecx &= ~CPUID_VMX;
+ cpuid->entries[i].ecx &= ~feature_bit(VMX);
vcpu_set_cpuid(vm, VCPU_ID, cpuid);
- cpuid->entries[i].ecx |= CPUID_VMX;
+ cpuid->entries[i].ecx |= feature_bit(VMX);
}

int main(int argc, char *argv[])
diff --git a/tools/testing/selftests/kvm/x86_64/xss_msr_test.c b/tools/testing/selftests/kvm/x86_64/xss_msr_test.c
index 3529376747c2..962dbb63cffe 100644
--- a/tools/testing/selftests/kvm/x86_64/xss_msr_test.c
+++ b/tools/testing/selftests/kvm/x86_64/xss_msr_test.c
@@ -11,12 +11,11 @@
#include "test_util.h"
#include "kvm_util.h"
#include "vmx.h"
+#include "cpuid.h"

#define VCPU_ID 1
#define MSR_BITS 64

-#define X86_FEATURE_XSAVES (1<<3)
-
bool is_supported_msr(u32 msr_index)
{
struct kvm_msr_list *list;
@@ -37,7 +36,6 @@ bool is_supported_msr(u32 msr_index)

int main(int argc, char *argv[])
{
- struct kvm_cpuid_entry2 *entry;
bool xss_supported = false;
struct kvm_vm *vm;
uint64_t xss_val;
@@ -46,10 +44,8 @@ int main(int argc, char *argv[])
/* Create VM */
vm = vm_create_default(VCPU_ID, 0, 0);

- if (kvm_get_cpuid_max_basic() >= 0xd) {
- entry = kvm_get_supported_cpuid_index(0xd, 1);
- xss_supported = entry && !!(entry->eax & X86_FEATURE_XSAVES);
- }
+ if (kvm_get_cpuid_max_basic() >= 0xd)
+ xss_supported = kvm_cpuid_has(X86_FEATURE_XSAVES);
if (!xss_supported) {
print_skip("IA32_XSS is not supported by the vCPU");
exit(KSFT_SKIP);
--
2.31.1.368.gbe11c130af-goog

2021-04-22 01:33:05

by Ricardo Koller

[permalink] [raw]
Subject: [PATCH 4/5] KVM: selftests: Introduce utilities for checking x86 features

Add utilities for checking CPU features using the same x86 features
format used in the kernel (defined in cpufeatures.h). This format embeds
the function, index, and register to use. By using this format and these
utilities, tests will not have to define their own feature macros and
will be able to use kvm_cpuid_has(FEATURE_XYZ) or
this_cpu_has(FEATURE_XYZ) without having to worry about what register or
index to use.

Signed-off-by: Ricardo Koller <[email protected]>
---
.../selftests/kvm/include/x86_64/cpuid.h | 61 +++++++++++++++++++
1 file changed, 61 insertions(+)
create mode 100644 tools/testing/selftests/kvm/include/x86_64/cpuid.h

diff --git a/tools/testing/selftests/kvm/include/x86_64/cpuid.h b/tools/testing/selftests/kvm/include/x86_64/cpuid.h
new file mode 100644
index 000000000000..4d8c67d528f4
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/x86_64/cpuid.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Adapted from /arch/x86/kvm/cpuid.h
+ */
+
+#ifndef SELFTEST_KVM_CPUID_FEATURE_H
+#define SELFTEST_KVM_CPUID_FEATURE_H
+
+#include <stdint.h>
+#include <asm/cpufeatures.h>
+#include <asm/kvm_para.h>
+#include "reverse_cpuid.h"
+
+static __always_inline u32 *kvm_cpuid_get_register(unsigned int x86_feature)
+{
+ struct kvm_cpuid_entry2 *entry;
+ const struct cpuid_reg cpuid = x86_feature_cpuid(x86_feature);
+
+ entry = kvm_get_supported_cpuid_index(cpuid.function, cpuid.index);
+ if (!entry)
+ return NULL;
+
+ return __cpuid_entry_get_reg(entry, cpuid.reg);
+}
+
+static __always_inline bool kvm_cpuid_has(unsigned int x86_feature)
+{
+ u32 *reg;
+
+ reg = kvm_cpuid_get_register(x86_feature);
+ if (!reg)
+ return false;
+
+ return *reg & __feature_bit(x86_feature);
+}
+
+static __always_inline bool kvm_pv_has(unsigned int kvm_feature)
+{
+ u32 reg;
+
+ reg = kvm_get_supported_cpuid_entry(KVM_CPUID_FEATURES)->eax;
+ return reg & __feature_bit(kvm_feature);
+}
+
+static __always_inline bool this_cpu_has(unsigned int x86_feature)
+{
+ struct kvm_cpuid_entry2 entry;
+ const struct cpuid_reg cpuid = x86_feature_cpuid(x86_feature);
+ u32 *reg;
+
+ entry.eax = cpuid.function;
+ entry.ecx = cpuid.index;
+ __asm__ __volatile__("cpuid"
+ : "+a"(entry.eax), "=b"(entry.ebx),
+ "+c"(entry.ecx), "=d"(entry.edx));
+
+ reg = __cpuid_entry_get_reg(&entry, cpuid.reg);
+ return *reg & __feature_bit(x86_feature);
+}
+
+#endif /* SELFTEST_KVM_CPUID_FEATURE_H */
--
2.31.1.368.gbe11c130af-goog

2021-04-22 07:01:45

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 3/5] tools headers x86: Copy cpuid helpers from the kernel

On 22/04/21 02:56, Ricardo Koller wrote:
> Copy arch/x86/include/asm/acpufeature.h and arch/x86/kvm/reverse_cpuid.h
> from the kernel so that KVM selftests can use them in the next commits.
> Also update the tools copy of arch/x86/include/asm/acpufeatures.h.

Typo.

> These should be kept in sync, ideally with the help of some script like
> check-headers.sh used by tools/perf/.

Please provide such a script.

Also, without an automated way to keep them in sync I think it's better
to copy all of them to tools/testing/selftests/kvm, so that we can be
sure that a maintainer (me) runs the script and keeps them up to date.
I am fairly sure that the x86 maintainers don't want to have anything to
do with all of this business!

Paolo

2021-04-22 07:03:32

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 0/5] KVM: x86: Use kernel x86 cpuid utilities in KVM selftests

On 22/04/21 02:56, Ricardo Koller wrote:
> The kernel has a set of utilities and definitions to deal with x86 cpu
> features. The x86 KVM selftests don't use them, and instead have
> evolved to use differing and ad-hoc methods for checking features. The
> advantage of the kernel feature definitions is that they use a format
> that embeds the info needed to extract them from cpuid (function, index,
> and register to use).
>
> The first 3 patches massage the related cpuid header files in the kernel
> side, then copy them into tools/ so they can be included by selftests.
> The last 2 patches replace the tests checking for cpu features to use
> the definitions and utilities introduced from the kernel.

I queued the first, but I am not sure about the rest.

An alternative is to copy over the code from kvm-unit-tests which
encodes the leaf/subleaf/register/bit values into the X86_FEATURE_*
value. Sharing code with kvm-unit-tests is probably simpler than adding
#ifdef __KERNEL__ and keeping the headers in sync.

Paolo

> Thanks,
> Ricardo
>
> Ricardo Koller (5):
> KVM: x86: Move reverse CPUID helpers to separate header file
> x86/cpu: Expose CPUID regs, leaf and index definitions to tools
> tools headers x86: Copy cpuid helpers from the kernel
> KVM: selftests: Introduce utilities for checking x86 features
> KVM: selftests: Use kernel x86 cpuid features format
>
> arch/x86/events/intel/pt.c | 1 +
> arch/x86/include/asm/cpufeature.h | 23 +-
> arch/x86/include/asm/processor.h | 11 -
> arch/x86/kernel/cpu/scattered.c | 2 +-
> arch/x86/kernel/cpuid.c | 2 +-
> arch/x86/kvm/cpuid.h | 177 +-----------
> arch/x86/kvm/reverse_cpuid.h | 185 +++++++++++++
> tools/arch/x86/include/asm/cpufeature.h | 257 ++++++++++++++++++
> tools/arch/x86/include/asm/cpufeatures.h | 3 +
> .../selftests/kvm/include/x86_64/cpuid.h | 61 +++++
> .../selftests/kvm/include/x86_64/processor.h | 16 --
> .../kvm/include/x86_64/reverse_cpuid.h | 185 +++++++++++++
> .../selftests/kvm/include/x86_64/svm_util.h | 11 +-
> tools/testing/selftests/kvm/lib/x86_64/svm.c | 6 +-
> tools/testing/selftests/kvm/lib/x86_64/vmx.c | 5 +-
> tools/testing/selftests/kvm/steal_time.c | 5 +-
> .../kvm/x86_64/cr4_cpuid_sync_test.c | 23 +-
> .../selftests/kvm/x86_64/set_sregs_test.c | 25 +-
> .../selftests/kvm/x86_64/vmx_pmu_msrs_test.c | 8 +-
> .../kvm/x86_64/vmx_set_nested_state_test.c | 5 +-
> .../selftests/kvm/x86_64/xss_msr_test.c | 10 +-
> 21 files changed, 749 insertions(+), 272 deletions(-)
> create mode 100644 arch/x86/kvm/reverse_cpuid.h
> create mode 100644 tools/arch/x86/include/asm/cpufeature.h
> create mode 100644 tools/testing/selftests/kvm/include/x86_64/cpuid.h
> create mode 100644 tools/testing/selftests/kvm/include/x86_64/reverse_cpuid.h
>

2021-04-23 00:12:42

by Ricardo Koller

[permalink] [raw]
Subject: Re: [PATCH 3/5] tools headers x86: Copy cpuid helpers from the kernel

On Thu, Apr 22, 2021 at 08:59:50AM +0200, Paolo Bonzini wrote:
> On 22/04/21 02:56, Ricardo Koller wrote:
> > Copy arch/x86/include/asm/acpufeature.h and arch/x86/kvm/reverse_cpuid.h
> > from the kernel so that KVM selftests can use them in the next commits.
> > Also update the tools copy of arch/x86/include/asm/acpufeatures.h.
>
> Typo.
>
> > These should be kept in sync, ideally with the help of some script like
> > check-headers.sh used by tools/perf/.
>
> Please provide such a script.
>
> Also, without an automated way to keep them in sync I think it's better to
> copy all of them to tools/testing/selftests/kvm

Will move them to the kvm subdir. The only issue is cpufeatures.h as
that would create a third copy of it: there is one already at
tools/arch/x86/include/asm/cpufeatures.h. Note that we can't move
cpufeatures.h from tools/arch/x86/include/asm to
tools/testing/selftests/kvm as it's already included by others like
tools/perf.

> so that we can be sure that
> a maintainer (me) runs the script and keeps them up to date. I am fairly
> sure that the x86 maintainers don't want to have anything to do with all of
> this business!
>
> Paolo
>

Thanks for the review!

I'll try this approach for the next version: copy the new headers to
tools/testing/selftests/kvm (except cpufeatures.h), and add the script.

2021-04-28 20:56:00

by Ricardo Koller

[permalink] [raw]
Subject: Re: [PATCH 0/5] KVM: x86: Use kernel x86 cpuid utilities in KVM selftests

On Thu, Apr 22, 2021 at 09:02:09AM +0200, Paolo Bonzini wrote:
> On 22/04/21 02:56, Ricardo Koller wrote:
> > The kernel has a set of utilities and definitions to deal with x86 cpu
> > features. The x86 KVM selftests don't use them, and instead have
> > evolved to use differing and ad-hoc methods for checking features. The
> > advantage of the kernel feature definitions is that they use a format
> > that embeds the info needed to extract them from cpuid (function, index,
> > and register to use).
> >
> > The first 3 patches massage the related cpuid header files in the kernel
> > side, then copy them into tools/ so they can be included by selftests.
> > The last 2 patches replace the tests checking for cpu features to use
> > the definitions and utilities introduced from the kernel.
>
> I queued the first, but I am not sure about the rest.
>
> An alternative is to copy over the code from kvm-unit-tests which encodes
> the leaf/subleaf/register/bit values into the X86_FEATURE_* value. Sharing
> code with kvm-unit-tests is probably simpler than adding #ifdef __KERNEL__
> and keeping the headers in sync.
>
> Paolo
>

Thanks. I was thinking about kvm-unit-tests, but the issue is that it
would also be a copy. And just like with kernel headers, it would be
ideal to keep them in-sync. The advantage of the kernel headers is that
it's much easier to check and fix diffs with them. On the other hand, as
you say, there would not be any #ifdef stuff with kvm=unit-tests. Please
let me know what you think.

Thanks,
Ricardo

> > Thanks,
> > Ricardo
> >
> > Ricardo Koller (5):
> > KVM: x86: Move reverse CPUID helpers to separate header file
> > x86/cpu: Expose CPUID regs, leaf and index definitions to tools
> > tools headers x86: Copy cpuid helpers from the kernel
> > KVM: selftests: Introduce utilities for checking x86 features
> > KVM: selftests: Use kernel x86 cpuid features format
> >
> > arch/x86/events/intel/pt.c | 1 +
> > arch/x86/include/asm/cpufeature.h | 23 +-
> > arch/x86/include/asm/processor.h | 11 -
> > arch/x86/kernel/cpu/scattered.c | 2 +-
> > arch/x86/kernel/cpuid.c | 2 +-
> > arch/x86/kvm/cpuid.h | 177 +-----------
> > arch/x86/kvm/reverse_cpuid.h | 185 +++++++++++++
> > tools/arch/x86/include/asm/cpufeature.h | 257 ++++++++++++++++++
> > tools/arch/x86/include/asm/cpufeatures.h | 3 +
> > .../selftests/kvm/include/x86_64/cpuid.h | 61 +++++
> > .../selftests/kvm/include/x86_64/processor.h | 16 --
> > .../kvm/include/x86_64/reverse_cpuid.h | 185 +++++++++++++
> > .../selftests/kvm/include/x86_64/svm_util.h | 11 +-
> > tools/testing/selftests/kvm/lib/x86_64/svm.c | 6 +-
> > tools/testing/selftests/kvm/lib/x86_64/vmx.c | 5 +-
> > tools/testing/selftests/kvm/steal_time.c | 5 +-
> > .../kvm/x86_64/cr4_cpuid_sync_test.c | 23 +-
> > .../selftests/kvm/x86_64/set_sregs_test.c | 25 +-
> > .../selftests/kvm/x86_64/vmx_pmu_msrs_test.c | 8 +-
> > .../kvm/x86_64/vmx_set_nested_state_test.c | 5 +-
> > .../selftests/kvm/x86_64/xss_msr_test.c | 10 +-
> > 21 files changed, 749 insertions(+), 272 deletions(-)
> > create mode 100644 arch/x86/kvm/reverse_cpuid.h
> > create mode 100644 tools/arch/x86/include/asm/cpufeature.h
> > create mode 100644 tools/testing/selftests/kvm/include/x86_64/cpuid.h
> > create mode 100644 tools/testing/selftests/kvm/include/x86_64/reverse_cpuid.h
> >
>

2021-06-29 18:56:09

by Jim Mattson

[permalink] [raw]
Subject: Re: [PATCH 0/5] KVM: x86: Use kernel x86 cpuid utilities in KVM selftests

On Wed, Apr 28, 2021 at 12:46 PM Ricardo Koller <[email protected]> wrote:
>
> On Thu, Apr 22, 2021 at 09:02:09AM +0200, Paolo Bonzini wrote:
> > On 22/04/21 02:56, Ricardo Koller wrote:
> > > The kernel has a set of utilities and definitions to deal with x86 cpu
> > > features. The x86 KVM selftests don't use them, and instead have
> > > evolved to use differing and ad-hoc methods for checking features. The
> > > advantage of the kernel feature definitions is that they use a format
> > > that embeds the info needed to extract them from cpuid (function, index,
> > > and register to use).
> > >
> > > The first 3 patches massage the related cpuid header files in the kernel
> > > side, then copy them into tools/ so they can be included by selftests.
> > > The last 2 patches replace the tests checking for cpu features to use
> > > the definitions and utilities introduced from the kernel.
> >
> > I queued the first, but I am not sure about the rest.
> >
> > An alternative is to copy over the code from kvm-unit-tests which encodes
> > the leaf/subleaf/register/bit values into the X86_FEATURE_* value. Sharing
> > code with kvm-unit-tests is probably simpler than adding #ifdef __KERNEL__
> > and keeping the headers in sync.
> >
> > Paolo
> >
>
> Thanks. I was thinking about kvm-unit-tests, but the issue is that it
> would also be a copy. And just like with kernel headers, it would be
> ideal to keep them in-sync. The advantage of the kernel headers is that
> it's much easier to check and fix diffs with them. On the other hand, as
> you say, there would not be any #ifdef stuff with kvm=unit-tests. Please
> let me know what you think.

I think the kvm-unit-tests implementation is superior to the kernel
implementation, but that's probably because I suggested it. Still, I
think there's an argument to be made that selftests, unlike
kvm-unit-tests, are part of the kernel distribution and should be
consistent with the kernel where possible.

Paolo?

2021-07-08 16:52:08

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 0/5] KVM: x86: Use kernel x86 cpuid utilities in KVM selftests

On 29/06/21 19:28, Jim Mattson wrote:
>> Thanks. I was thinking about kvm-unit-tests, but the issue is that it
>> would also be a copy. And just like with kernel headers, it would be
>> ideal to keep them in-sync. The advantage of the kernel headers is that
>> it's much easier to check and fix diffs with them. On the other hand, as
>> you say, there would not be any #ifdef stuff with kvm=unit-tests. Please
>> let me know what you think.
>
> I think the kvm-unit-tests implementation is superior to the kernel
> implementation, but that's probably because I suggested it. Still, I
> think there's an argument to be made that selftests, unlike
> kvm-unit-tests, are part of the kernel distribution and should be
> consistent with the kernel where possible.
>
> Paolo?

I also prefer the kvm-unit-tests implementation, for what it's worth...
Let's see what the code looks like?

Paolo

2021-07-08 17:22:34

by Ricardo Koller

[permalink] [raw]
Subject: Re: [PATCH 0/5] KVM: x86: Use kernel x86 cpuid utilities in KVM selftests

On Thu, Jul 08, 2021 at 06:50:41PM +0200, Paolo Bonzini wrote:
> On 29/06/21 19:28, Jim Mattson wrote:
> > > Thanks. I was thinking about kvm-unit-tests, but the issue is that it
> > > would also be a copy. And just like with kernel headers, it would be
> > > ideal to keep them in-sync. The advantage of the kernel headers is that
> > > it's much easier to check and fix diffs with them. On the other hand, as
> > > you say, there would not be any #ifdef stuff with kvm=unit-tests. Please
> > > let me know what you think.
> >
> > I think the kvm-unit-tests implementation is superior to the kernel
> > implementation, but that's probably because I suggested it. Still, I
> > think there's an argument to be made that selftests, unlike
> > kvm-unit-tests, are part of the kernel distribution and should be
> > consistent with the kernel where possible.
> >
> > Paolo?
>
> I also prefer the kvm-unit-tests implementation, for what it's worth...
> Let's see what the code looks like?

I'm not sure I understand the question. You mean: let's see how this
looks using kvm-unit-tests headers? If that's the case I can work on a
v3 using kvm-unit-tests.

Thanks,
Ricardo

>
> Paolo
>

2021-07-08 17:58:43

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 0/5] KVM: x86: Use kernel x86 cpuid utilities in KVM selftests

On 08/07/21 19:21, Ricardo Koller wrote:
>> I also prefer the kvm-unit-tests implementation, for what it's worth...
>> Let's see what the code looks like?
> I'm not sure I understand the question. You mean: let's see how this
> looks using kvm-unit-tests headers? If that's the case I can work on a
> v3 using kvm-unit-tests.

Yes, exactly. Thanks!

Paolo

2021-07-08 19:47:08

by Ricardo Koller

[permalink] [raw]
Subject: Re: [PATCH 0/5] KVM: x86: Use kernel x86 cpuid utilities in KVM selftests

On Thu, Jul 08, 2021 at 07:57:24PM +0200, Paolo Bonzini wrote:
> On 08/07/21 19:21, Ricardo Koller wrote:
> > > I also prefer the kvm-unit-tests implementation, for what it's worth...
> > > Let's see what the code looks like?
> > I'm not sure I understand the question. You mean: let's see how this
> > looks using kvm-unit-tests headers? If that's the case I can work on a
> > v3 using kvm-unit-tests.
>
> Yes, exactly. Thanks!

Cool, will give it a try and send a v3.

Thanks,
Ricardo

>
> Paolo
>