Changes since v3:
- Use cpuid_entry_override() for the newly introduces CPUID_8000_0007_EDX
leaf in __do_cpuid_func(). [Sean]
- Add comments and reshuffle check in kvm_hv_invtsc_suppressed() [Sean]
Original description:
Normally, genuine Hyper-V doesn't expose architectural invariant TSC
(CPUID.80000007H:EDX[8]) to its guests by default. A special PV MSR
(HV_X64_MSR_TSC_INVARIANT_CONTROL, 0x40000118) and corresponding CPUID
feature bit (CPUID.0x40000003.EAX[15]) were introduced. When bit 0 of the
PV MSR is set, invariant TSC bit starts to show up in CPUID. When the
feature is exposed to Hyper-V guests, reenlightenment becomes unneeded.
Note: strictly speaking, KVM doesn't have to have the feature as exposing
raw invariant TSC bit (CPUID.80000007H:EDX[8]) also seems to work for
modern Windows versions. The feature is, however, tiny and straitforward
and gives additional flexibility so why not.
Vitaly Kuznetsov (6):
x86/hyperv: Add HV_INVARIANT_TSC_EXPOSED define
KVM: x86: Introduce CPUID_8000_0007_EDX 'scattered' leaf
KVM: x86: Hyper-V invariant TSC control
KVM: selftests: Rename 'msr->availble' to 'msr->fault_exepected' in
hyperv_features test
KVM: selftests: Convert hyperv_features test to using
KVM_X86_CPU_FEATURE()
KVM: selftests: Test Hyper-V invariant TSC control
arch/x86/include/asm/hyperv-tlfs.h | 3 +
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kernel/cpu/mshyperv.c | 2 +-
arch/x86/kvm/cpuid.c | 11 +-
arch/x86/kvm/hyperv.c | 19 ++
arch/x86/kvm/hyperv.h | 27 +++
arch/x86/kvm/reverse_cpuid.h | 9 +-
arch/x86/kvm/x86.c | 4 +-
.../selftests/kvm/include/x86_64/hyperv.h | 144 ++++++++----
.../selftests/kvm/include/x86_64/processor.h | 1 +
.../selftests/kvm/x86_64/hyperv_features.c | 212 +++++++++++-------
11 files changed, 295 insertions(+), 138 deletions(-)
--
2.37.3
Add a test for the newly introduced Hyper-V invariant TSC control feature:
- HV_X64_MSR_TSC_INVARIANT_CONTROL is not available without
HV_ACCESS_TSC_INVARIANT CPUID bit set and available with it.
- BIT(0) of HV_X64_MSR_TSC_INVARIANT_CONTROL controls the filtering of
architectural invariant TSC (CPUID.80000007H:EDX[8]) bit.
Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
.../selftests/kvm/include/x86_64/hyperv.h | 3 +
.../selftests/kvm/include/x86_64/processor.h | 1 +
.../selftests/kvm/x86_64/hyperv_features.c | 58 +++++++++++++++++--
3 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/x86_64/hyperv.h b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
index 843748dde1ff..8368d65afbe4 100644
--- a/tools/testing/selftests/kvm/include/x86_64/hyperv.h
+++ b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
@@ -232,4 +232,7 @@
/* hypercall options */
#define HV_HYPERCALL_FAST_BIT BIT(16)
+/* HV_X64_MSR_TSC_INVARIANT_CONTROL bits */
+#define HV_INVARIANT_TSC_EXPOSED BIT_ULL(0)
+
#endif /* !SELFTEST_KVM_HYPERV_H */
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index 0cbc71b7af50..8d106380b0af 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -128,6 +128,7 @@ struct kvm_x86_cpu_feature {
#define X86_FEATURE_GBPAGES KVM_X86_CPU_FEATURE(0x80000001, 0, EDX, 26)
#define X86_FEATURE_RDTSCP KVM_X86_CPU_FEATURE(0x80000001, 0, EDX, 27)
#define X86_FEATURE_LM KVM_X86_CPU_FEATURE(0x80000001, 0, EDX, 29)
+#define X86_FEATURE_INVTSC KVM_X86_CPU_FEATURE(0x80000007, 0, EDX, 8)
#define X86_FEATURE_RDPRU KVM_X86_CPU_FEATURE(0x80000008, 0, EBX, 4)
#define X86_FEATURE_AMD_IBPB KVM_X86_CPU_FEATURE(0x80000008, 0, EBX, 12)
#define X86_FEATURE_NPT KVM_X86_CPU_FEATURE(0x8000000A, 0, EDX, 0)
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
index d4bd18bc580d..18b44450dfb8 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
@@ -46,20 +46,33 @@ struct hcall_data {
static void guest_msr(struct msr_data *msr)
{
- uint64_t ignored;
+ uint64_t msr_val = 0;
uint8_t vector;
GUEST_ASSERT(msr->idx);
- if (!msr->write)
- vector = rdmsr_safe(msr->idx, &ignored);
- else
+ if (!msr->write) {
+ vector = rdmsr_safe(msr->idx, &msr_val);
+ } else {
vector = wrmsr_safe(msr->idx, msr->write_val);
+ if (!vector)
+ msr_val = msr->write_val;
+ }
if (msr->fault_expected)
GUEST_ASSERT_2(vector == GP_VECTOR, msr->idx, vector);
else
GUEST_ASSERT_2(!vector, msr->idx, vector);
+
+ /* Invariant TSC bit appears when TSC invariant control MSR is written to */
+ if (msr->idx == HV_X64_MSR_TSC_INVARIANT_CONTROL) {
+ if (!this_cpu_has(HV_ACCESS_TSC_INVARIANT))
+ GUEST_ASSERT(this_cpu_has(X86_FEATURE_INVTSC));
+ else
+ GUEST_ASSERT(this_cpu_has(X86_FEATURE_INVTSC) ==
+ !!(msr_val & HV_INVARIANT_TSC_EXPOSED));
+ }
+
GUEST_DONE();
}
@@ -114,6 +127,7 @@ static void guest_test_msrs_access(void)
int stage = 0;
vm_vaddr_t msr_gva;
struct msr_data *msr;
+ bool has_invtsc = kvm_cpu_has(X86_FEATURE_INVTSC);
while (true) {
vm = vm_create_with_one_vcpu(&vcpu, guest_msr);
@@ -425,6 +439,42 @@ static void guest_test_msrs_access(void)
break;
case 44:
+ /* MSR is not available when CPUID feature bit is unset */
+ if (!has_invtsc)
+ continue;
+ msr->idx = HV_X64_MSR_TSC_INVARIANT_CONTROL;
+ msr->write = 0;
+ msr->fault_expected = 1;
+ break;
+ case 45:
+ /* MSR is vailable when CPUID feature bit is set */
+ if (!has_invtsc)
+ continue;
+ vcpu_set_cpuid_feature(vcpu, HV_ACCESS_TSC_INVARIANT);
+ msr->idx = HV_X64_MSR_TSC_INVARIANT_CONTROL;
+ msr->write = 0;
+ msr->fault_expected = 0;
+ break;
+ case 46:
+ /* Writing bits other than 0 is forbidden */
+ if (!has_invtsc)
+ continue;
+ msr->idx = HV_X64_MSR_TSC_INVARIANT_CONTROL;
+ msr->write = 1;
+ msr->write_val = 0xdeadbeef;
+ msr->fault_expected = 1;
+ break;
+ case 47:
+ /* Setting bit 0 enables the feature */
+ if (!has_invtsc)
+ continue;
+ msr->idx = HV_X64_MSR_TSC_INVARIANT_CONTROL;
+ msr->write = 1;
+ msr->write_val = 1;
+ msr->fault_expected = 0;
+ break;
+
+ default:
kvm_vm_free(vm);
return;
}
--
2.37.3
It may not be clear what 'msr->availble' means. The test actually
checks that accessing the particular MSR doesn't cause #GP, rename
the varialble accordingly.
Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
.../selftests/kvm/x86_64/hyperv_features.c | 96 +++++++++----------
1 file changed, 48 insertions(+), 48 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
index 79ab0152d281..1383b979e90b 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
@@ -33,7 +33,7 @@ static inline uint8_t hypercall(u64 control, vm_vaddr_t input_address,
struct msr_data {
uint32_t idx;
- bool available;
+ bool fault_expected;
bool write;
u64 write_val;
};
@@ -56,10 +56,10 @@ static void guest_msr(struct msr_data *msr)
else
vector = wrmsr_safe(msr->idx, msr->write_val);
- if (msr->available)
- GUEST_ASSERT_2(!vector, msr->idx, vector);
- else
+ if (msr->fault_expected)
GUEST_ASSERT_2(vector == GP_VECTOR, msr->idx, vector);
+ else
+ GUEST_ASSERT_2(!vector, msr->idx, vector);
GUEST_DONE();
}
@@ -153,12 +153,12 @@ static void guest_test_msrs_access(void)
*/
msr->idx = HV_X64_MSR_GUEST_OS_ID;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 1:
msr->idx = HV_X64_MSR_HYPERCALL;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 2:
feat->eax |= HV_MSR_HYPERCALL_AVAILABLE;
@@ -169,116 +169,116 @@ static void guest_test_msrs_access(void)
msr->idx = HV_X64_MSR_GUEST_OS_ID;
msr->write = 1;
msr->write_val = LINUX_OS_ID;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 3:
msr->idx = HV_X64_MSR_GUEST_OS_ID;
msr->write = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 4:
msr->idx = HV_X64_MSR_HYPERCALL;
msr->write = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 5:
msr->idx = HV_X64_MSR_VP_RUNTIME;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 6:
feat->eax |= HV_MSR_VP_RUNTIME_AVAILABLE;
msr->idx = HV_X64_MSR_VP_RUNTIME;
msr->write = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 7:
/* Read only */
msr->idx = HV_X64_MSR_VP_RUNTIME;
msr->write = 1;
msr->write_val = 1;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 8:
msr->idx = HV_X64_MSR_TIME_REF_COUNT;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 9:
feat->eax |= HV_MSR_TIME_REF_COUNT_AVAILABLE;
msr->idx = HV_X64_MSR_TIME_REF_COUNT;
msr->write = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 10:
/* Read only */
msr->idx = HV_X64_MSR_TIME_REF_COUNT;
msr->write = 1;
msr->write_val = 1;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 11:
msr->idx = HV_X64_MSR_VP_INDEX;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 12:
feat->eax |= HV_MSR_VP_INDEX_AVAILABLE;
msr->idx = HV_X64_MSR_VP_INDEX;
msr->write = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 13:
/* Read only */
msr->idx = HV_X64_MSR_VP_INDEX;
msr->write = 1;
msr->write_val = 1;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 14:
msr->idx = HV_X64_MSR_RESET;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 15:
feat->eax |= HV_MSR_RESET_AVAILABLE;
msr->idx = HV_X64_MSR_RESET;
msr->write = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 16:
msr->idx = HV_X64_MSR_RESET;
msr->write = 1;
msr->write_val = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 17:
msr->idx = HV_X64_MSR_REFERENCE_TSC;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 18:
feat->eax |= HV_MSR_REFERENCE_TSC_AVAILABLE;
msr->idx = HV_X64_MSR_REFERENCE_TSC;
msr->write = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 19:
msr->idx = HV_X64_MSR_REFERENCE_TSC;
msr->write = 1;
msr->write_val = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 20:
msr->idx = HV_X64_MSR_EOM;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 21:
/*
@@ -287,145 +287,145 @@ static void guest_test_msrs_access(void)
*/
msr->idx = HV_X64_MSR_EOM;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 22:
feat->eax |= HV_MSR_SYNIC_AVAILABLE;
msr->idx = HV_X64_MSR_EOM;
msr->write = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 23:
msr->idx = HV_X64_MSR_EOM;
msr->write = 1;
msr->write_val = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 24:
msr->idx = HV_X64_MSR_STIMER0_CONFIG;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 25:
feat->eax |= HV_MSR_SYNTIMER_AVAILABLE;
msr->idx = HV_X64_MSR_STIMER0_CONFIG;
msr->write = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 26:
msr->idx = HV_X64_MSR_STIMER0_CONFIG;
msr->write = 1;
msr->write_val = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 27:
/* Direct mode test */
msr->idx = HV_X64_MSR_STIMER0_CONFIG;
msr->write = 1;
msr->write_val = 1 << 12;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 28:
feat->edx |= HV_STIMER_DIRECT_MODE_AVAILABLE;
msr->idx = HV_X64_MSR_STIMER0_CONFIG;
msr->write = 1;
msr->write_val = 1 << 12;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 29:
msr->idx = HV_X64_MSR_EOI;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 30:
feat->eax |= HV_MSR_APIC_ACCESS_AVAILABLE;
msr->idx = HV_X64_MSR_EOI;
msr->write = 1;
msr->write_val = 1;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 31:
msr->idx = HV_X64_MSR_TSC_FREQUENCY;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 32:
feat->eax |= HV_ACCESS_FREQUENCY_MSRS;
msr->idx = HV_X64_MSR_TSC_FREQUENCY;
msr->write = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 33:
/* Read only */
msr->idx = HV_X64_MSR_TSC_FREQUENCY;
msr->write = 1;
msr->write_val = 1;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 34:
msr->idx = HV_X64_MSR_REENLIGHTENMENT_CONTROL;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 35:
feat->eax |= HV_ACCESS_REENLIGHTENMENT;
msr->idx = HV_X64_MSR_REENLIGHTENMENT_CONTROL;
msr->write = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 36:
msr->idx = HV_X64_MSR_REENLIGHTENMENT_CONTROL;
msr->write = 1;
msr->write_val = 1;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 37:
/* Can only write '0' */
msr->idx = HV_X64_MSR_TSC_EMULATION_STATUS;
msr->write = 1;
msr->write_val = 1;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 38:
msr->idx = HV_X64_MSR_CRASH_P0;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 39:
feat->edx |= HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
msr->idx = HV_X64_MSR_CRASH_P0;
msr->write = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 40:
msr->idx = HV_X64_MSR_CRASH_P0;
msr->write = 1;
msr->write_val = 1;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 41:
msr->idx = HV_X64_MSR_SYNDBG_STATUS;
msr->write = 0;
- msr->available = 0;
+ msr->fault_expected = 1;
break;
case 42:
feat->edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
dbg->eax |= HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING;
msr->idx = HV_X64_MSR_SYNDBG_STATUS;
msr->write = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 43:
msr->idx = HV_X64_MSR_SYNDBG_STATUS;
msr->write = 1;
msr->write_val = 0;
- msr->available = 1;
+ msr->fault_expected = 0;
break;
case 44:
--
2.37.3
CPUID_8000_0007_EDX may come handy when X86_FEATURE_CONSTANT_TSC
needs to be checked.
No functional change intended.
Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
arch/x86/kvm/cpuid.c | 8 ++++++--
arch/x86/kvm/reverse_cpuid.h | 9 ++++++++-
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index ffdc28684cb7..b95a4b7489ec 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -685,6 +685,10 @@ void kvm_set_cpu_caps(void)
if (!tdp_enabled && IS_ENABLED(CONFIG_X86_64))
kvm_cpu_cap_set(X86_FEATURE_GBPAGES);
+ kvm_cpu_cap_init_scattered(CPUID_8000_0007_EDX,
+ SF(CONSTANT_TSC)
+ );
+
kvm_cpu_cap_mask(CPUID_8000_0008_EBX,
F(CLZERO) | F(XSAVEERPTR) |
F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) |
@@ -1137,8 +1141,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
/* L2 cache and TLB: pass through host info. */
break;
case 0x80000007: /* Advanced power management */
- /* invariant TSC is CPUID.80000007H:EDX[8] */
- entry->edx &= (1 << 8);
+ cpuid_entry_override(entry, CPUID_8000_0007_EDX);
+
/* mask against host */
entry->edx &= boot_cpu_data.x86_power;
entry->eax = entry->ebx = entry->ecx = 0;
diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
index a19d473d0184..a5514c89dc29 100644
--- a/arch/x86/kvm/reverse_cpuid.h
+++ b/arch/x86/kvm/reverse_cpuid.h
@@ -12,7 +12,8 @@
* "bug" caps, but KVM doesn't use those.
*/
enum kvm_only_cpuid_leafs {
- CPUID_12_EAX = NCAPINTS,
+ CPUID_12_EAX = NCAPINTS,
+ CPUID_8000_0007_EDX = NCAPINTS + 1,
NR_KVM_CPU_CAPS,
NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
@@ -24,6 +25,9 @@ enum kvm_only_cpuid_leafs {
#define KVM_X86_FEATURE_SGX1 KVM_X86_FEATURE(CPUID_12_EAX, 0)
#define KVM_X86_FEATURE_SGX2 KVM_X86_FEATURE(CPUID_12_EAX, 1)
+/* CPUID level 0x80000007 (EDX). */
+#define KVM_X86_FEATURE_CONSTANT_TSC KVM_X86_FEATURE(CPUID_8000_0007_EDX, 8)
+
struct cpuid_reg {
u32 function;
u32 index;
@@ -48,6 +52,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
[CPUID_7_1_EAX] = { 7, 1, CPUID_EAX},
[CPUID_12_EAX] = {0x00000012, 0, CPUID_EAX},
[CPUID_8000_001F_EAX] = {0x8000001f, 0, CPUID_EAX},
+ [CPUID_8000_0007_EDX] = {0x80000007, 0, CPUID_EDX},
};
/*
@@ -78,6 +83,8 @@ static __always_inline u32 __feature_translate(int x86_feature)
return KVM_X86_FEATURE_SGX1;
else if (x86_feature == X86_FEATURE_SGX2)
return KVM_X86_FEATURE_SGX2;
+ else if (x86_feature == X86_FEATURE_CONSTANT_TSC)
+ return KVM_X86_FEATURE_CONSTANT_TSC;
return x86_feature;
}
--
2.37.3
hyperv_features test needs to set certain CPUID bits in Hyper-V feature
leaves but instead of open coding this, common KVM_X86_CPU_FEATURE()
infrastructure can be used.
Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
.../selftests/kvm/include/x86_64/hyperv.h | 141 ++++++++++++------
.../selftests/kvm/x86_64/hyperv_features.c | 58 +++----
2 files changed, 118 insertions(+), 81 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/x86_64/hyperv.h b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
index b66910702c0a..843748dde1ff 100644
--- a/tools/testing/selftests/kvm/include/x86_64/hyperv.h
+++ b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
@@ -83,61 +83,108 @@
#define HV_X64_MSR_SYNDBG_OPTIONS 0x400000FF
/* HYPERV_CPUID_FEATURES.EAX */
-#define HV_MSR_VP_RUNTIME_AVAILABLE BIT(0)
-#define HV_MSR_TIME_REF_COUNT_AVAILABLE BIT(1)
-#define HV_MSR_SYNIC_AVAILABLE BIT(2)
-#define HV_MSR_SYNTIMER_AVAILABLE BIT(3)
-#define HV_MSR_APIC_ACCESS_AVAILABLE BIT(4)
-#define HV_MSR_HYPERCALL_AVAILABLE BIT(5)
-#define HV_MSR_VP_INDEX_AVAILABLE BIT(6)
-#define HV_MSR_RESET_AVAILABLE BIT(7)
-#define HV_MSR_STAT_PAGES_AVAILABLE BIT(8)
-#define HV_MSR_REFERENCE_TSC_AVAILABLE BIT(9)
-#define HV_MSR_GUEST_IDLE_AVAILABLE BIT(10)
-#define HV_ACCESS_FREQUENCY_MSRS BIT(11)
-#define HV_ACCESS_REENLIGHTENMENT BIT(13)
-#define HV_ACCESS_TSC_INVARIANT BIT(15)
+#define HV_MSR_VP_RUNTIME_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EAX, 0)
+#define HV_MSR_TIME_REF_COUNT_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EAX, 1)
+#define HV_MSR_SYNIC_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EAX, 2)
+#define HV_MSR_SYNTIMER_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EAX, 3)
+#define HV_MSR_APIC_ACCESS_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EAX, 4)
+#define HV_MSR_HYPERCALL_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EAX, 5)
+#define HV_MSR_VP_INDEX_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EAX, 6)
+#define HV_MSR_RESET_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EAX, 7)
+#define HV_MSR_STAT_PAGES_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EAX, 8)
+#define HV_MSR_REFERENCE_TSC_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EAX, 9)
+#define HV_MSR_GUEST_IDLE_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EAX, 10)
+#define HV_ACCESS_FREQUENCY_MSRS \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EAX, 11)
+#define HV_ACCESS_REENLIGHTENMENT \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EAX, 13)
+#define HV_ACCESS_TSC_INVARIANT \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EAX, 15)
/* HYPERV_CPUID_FEATURES.EBX */
-#define HV_CREATE_PARTITIONS BIT(0)
-#define HV_ACCESS_PARTITION_ID BIT(1)
-#define HV_ACCESS_MEMORY_POOL BIT(2)
-#define HV_ADJUST_MESSAGE_BUFFERS BIT(3)
-#define HV_POST_MESSAGES BIT(4)
-#define HV_SIGNAL_EVENTS BIT(5)
-#define HV_CREATE_PORT BIT(6)
-#define HV_CONNECT_PORT BIT(7)
-#define HV_ACCESS_STATS BIT(8)
-#define HV_DEBUGGING BIT(11)
-#define HV_CPU_MANAGEMENT BIT(12)
-#define HV_ISOLATION BIT(22)
+#define HV_CREATE_PARTITIONS \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EBX, 0)
+#define HV_ACCESS_PARTITION_ID \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EBX, 1)
+#define HV_ACCESS_MEMORY_POOL \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EBX, 2)
+#define HV_ADJUST_MESSAGE_BUFFERS \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EBX, 3)
+#define HV_POST_MESSAGES \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EBX, 4)
+#define HV_SIGNAL_EVENTS \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EBX, 5)
+#define HV_CREATE_PORT \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EBX, 6)
+#define HV_CONNECT_PORT \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EBX, 7)
+#define HV_ACCESS_STATS \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EBX, 8)
+#define HV_DEBUGGING \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EBX, 11)
+#define HV_CPU_MANAGEMENT \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EBX, 12)
+#define HV_ISOLATION \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EBX, 22)
/* HYPERV_CPUID_FEATURES.EDX */
-#define HV_X64_MWAIT_AVAILABLE BIT(0)
-#define HV_X64_GUEST_DEBUGGING_AVAILABLE BIT(1)
-#define HV_X64_PERF_MONITOR_AVAILABLE BIT(2)
-#define HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE BIT(3)
-#define HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE BIT(4)
-#define HV_X64_GUEST_IDLE_STATE_AVAILABLE BIT(5)
-#define HV_FEATURE_FREQUENCY_MSRS_AVAILABLE BIT(8)
-#define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE BIT(10)
-#define HV_FEATURE_DEBUG_MSRS_AVAILABLE BIT(11)
-#define HV_STIMER_DIRECT_MODE_AVAILABLE BIT(19)
+#define HV_X64_MWAIT_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EDX, 0)
+#define HV_X64_GUEST_DEBUGGING_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EDX, 1)
+#define HV_X64_PERF_MONITOR_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EDX, 2)
+#define HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EDX, 3)
+#define HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EDX, 4)
+#define HV_X64_GUEST_IDLE_STATE_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EDX, 5)
+#define HV_FEATURE_FREQUENCY_MSRS_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EDX, 8)
+#define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EDX, 10)
+#define HV_FEATURE_DEBUG_MSRS_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EDX, 11)
+#define HV_STIMER_DIRECT_MODE_AVAILABLE \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_FEATURES, 0, EDX, 19)
/* HYPERV_CPUID_ENLIGHTMENT_INFO.EAX */
-#define HV_X64_AS_SWITCH_RECOMMENDED BIT(0)
-#define HV_X64_LOCAL_TLB_FLUSH_RECOMMENDED BIT(1)
-#define HV_X64_REMOTE_TLB_FLUSH_RECOMMENDED BIT(2)
-#define HV_X64_APIC_ACCESS_RECOMMENDED BIT(3)
-#define HV_X64_SYSTEM_RESET_RECOMMENDED BIT(4)
-#define HV_X64_RELAXED_TIMING_RECOMMENDED BIT(5)
-#define HV_DEPRECATING_AEOI_RECOMMENDED BIT(9)
-#define HV_X64_CLUSTER_IPI_RECOMMENDED BIT(10)
-#define HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED BIT(11)
-#define HV_X64_ENLIGHTENED_VMCS_RECOMMENDED BIT(14)
+#define HV_X64_AS_SWITCH_RECOMMENDED \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_ENLIGHTMENT_INFO, 0, EAX, 0)
+#define HV_X64_LOCAL_TLB_FLUSH_RECOMMENDED \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_ENLIGHTMENT_INFO, 0, EAX, 1)
+#define HV_X64_REMOTE_TLB_FLUSH_RECOMMENDED \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_ENLIGHTMENT_INFO, 0, EAX, 2)
+#define HV_X64_APIC_ACCESS_RECOMMENDED \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_ENLIGHTMENT_INFO, 0, EAX, 3)
+#define HV_X64_SYSTEM_RESET_RECOMMENDED \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_ENLIGHTMENT_INFO, 0, EAX, 4)
+#define HV_X64_RELAXED_TIMING_RECOMMENDED \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_ENLIGHTMENT_INFO, 0, EAX, 5)
+#define HV_DEPRECATING_AEOI_RECOMMENDED \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_ENLIGHTMENT_INFO, 0, EAX, 9)
+#define HV_X64_CLUSTER_IPI_RECOMMENDED \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_ENLIGHTMENT_INFO, 0, EAX, 10)
+#define HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_ENLIGHTMENT_INFO, 0, EAX, 11)
+#define HV_X64_ENLIGHTENED_VMCS_RECOMMENDED \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_ENLIGHTMENT_INFO, 0, EAX, 14)
/* HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES.EAX */
-#define HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING BIT(1)
+#define HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING \
+ KVM_X86_CPU_FEATURE(HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES, 0, EAX, 1)
/* Hypercalls */
#define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE 0x0002
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
index 1383b979e90b..d4bd18bc580d 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
@@ -107,7 +107,6 @@ static void vcpu_reset_hv_cpuid(struct kvm_vcpu *vcpu)
static void guest_test_msrs_access(void)
{
struct kvm_cpuid2 *prev_cpuid = NULL;
- struct kvm_cpuid_entry2 *feat, *dbg;
struct kvm_vcpu *vcpu;
struct kvm_run *run;
struct kvm_vm *vm;
@@ -134,9 +133,6 @@ static void guest_test_msrs_access(void)
vcpu_init_cpuid(vcpu, prev_cpuid);
}
- feat = vcpu_get_cpuid_entry(vcpu, HYPERV_CPUID_FEATURES);
- dbg = vcpu_get_cpuid_entry(vcpu, HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES);
-
vm_init_descriptor_tables(vm);
vcpu_init_descriptor_tables(vcpu);
@@ -161,7 +157,7 @@ static void guest_test_msrs_access(void)
msr->fault_expected = 1;
break;
case 2:
- feat->eax |= HV_MSR_HYPERCALL_AVAILABLE;
+ vcpu_set_cpuid_feature(vcpu, HV_MSR_HYPERCALL_AVAILABLE);
/*
* HV_X64_MSR_GUEST_OS_ID has to be written first to make
* HV_X64_MSR_HYPERCALL available.
@@ -188,7 +184,7 @@ static void guest_test_msrs_access(void)
msr->fault_expected = 1;
break;
case 6:
- feat->eax |= HV_MSR_VP_RUNTIME_AVAILABLE;
+ vcpu_set_cpuid_feature(vcpu, HV_MSR_VP_RUNTIME_AVAILABLE);
msr->idx = HV_X64_MSR_VP_RUNTIME;
msr->write = 0;
msr->fault_expected = 0;
@@ -207,7 +203,7 @@ static void guest_test_msrs_access(void)
msr->fault_expected = 1;
break;
case 9:
- feat->eax |= HV_MSR_TIME_REF_COUNT_AVAILABLE;
+ vcpu_set_cpuid_feature(vcpu, HV_MSR_TIME_REF_COUNT_AVAILABLE);
msr->idx = HV_X64_MSR_TIME_REF_COUNT;
msr->write = 0;
msr->fault_expected = 0;
@@ -226,7 +222,7 @@ static void guest_test_msrs_access(void)
msr->fault_expected = 1;
break;
case 12:
- feat->eax |= HV_MSR_VP_INDEX_AVAILABLE;
+ vcpu_set_cpuid_feature(vcpu, HV_MSR_VP_INDEX_AVAILABLE);
msr->idx = HV_X64_MSR_VP_INDEX;
msr->write = 0;
msr->fault_expected = 0;
@@ -245,7 +241,7 @@ static void guest_test_msrs_access(void)
msr->fault_expected = 1;
break;
case 15:
- feat->eax |= HV_MSR_RESET_AVAILABLE;
+ vcpu_set_cpuid_feature(vcpu, HV_MSR_RESET_AVAILABLE);
msr->idx = HV_X64_MSR_RESET;
msr->write = 0;
msr->fault_expected = 0;
@@ -263,7 +259,7 @@ static void guest_test_msrs_access(void)
msr->fault_expected = 1;
break;
case 18:
- feat->eax |= HV_MSR_REFERENCE_TSC_AVAILABLE;
+ vcpu_set_cpuid_feature(vcpu, HV_MSR_REFERENCE_TSC_AVAILABLE);
msr->idx = HV_X64_MSR_REFERENCE_TSC;
msr->write = 0;
msr->fault_expected = 0;
@@ -290,7 +286,7 @@ static void guest_test_msrs_access(void)
msr->fault_expected = 1;
break;
case 22:
- feat->eax |= HV_MSR_SYNIC_AVAILABLE;
+ vcpu_set_cpuid_feature(vcpu, HV_MSR_SYNIC_AVAILABLE);
msr->idx = HV_X64_MSR_EOM;
msr->write = 0;
msr->fault_expected = 0;
@@ -308,7 +304,7 @@ static void guest_test_msrs_access(void)
msr->fault_expected = 1;
break;
case 25:
- feat->eax |= HV_MSR_SYNTIMER_AVAILABLE;
+ vcpu_set_cpuid_feature(vcpu, HV_MSR_SYNTIMER_AVAILABLE);
msr->idx = HV_X64_MSR_STIMER0_CONFIG;
msr->write = 0;
msr->fault_expected = 0;
@@ -327,7 +323,7 @@ static void guest_test_msrs_access(void)
msr->fault_expected = 1;
break;
case 28:
- feat->edx |= HV_STIMER_DIRECT_MODE_AVAILABLE;
+ vcpu_set_cpuid_feature(vcpu, HV_STIMER_DIRECT_MODE_AVAILABLE);
msr->idx = HV_X64_MSR_STIMER0_CONFIG;
msr->write = 1;
msr->write_val = 1 << 12;
@@ -340,7 +336,7 @@ static void guest_test_msrs_access(void)
msr->fault_expected = 1;
break;
case 30:
- feat->eax |= HV_MSR_APIC_ACCESS_AVAILABLE;
+ vcpu_set_cpuid_feature(vcpu, HV_MSR_APIC_ACCESS_AVAILABLE);
msr->idx = HV_X64_MSR_EOI;
msr->write = 1;
msr->write_val = 1;
@@ -353,7 +349,7 @@ static void guest_test_msrs_access(void)
msr->fault_expected = 1;
break;
case 32:
- feat->eax |= HV_ACCESS_FREQUENCY_MSRS;
+ vcpu_set_cpuid_feature(vcpu, HV_ACCESS_FREQUENCY_MSRS);
msr->idx = HV_X64_MSR_TSC_FREQUENCY;
msr->write = 0;
msr->fault_expected = 0;
@@ -372,7 +368,7 @@ static void guest_test_msrs_access(void)
msr->fault_expected = 1;
break;
case 35:
- feat->eax |= HV_ACCESS_REENLIGHTENMENT;
+ vcpu_set_cpuid_feature(vcpu, HV_ACCESS_REENLIGHTENMENT);
msr->idx = HV_X64_MSR_REENLIGHTENMENT_CONTROL;
msr->write = 0;
msr->fault_expected = 0;
@@ -397,7 +393,7 @@ static void guest_test_msrs_access(void)
msr->fault_expected = 1;
break;
case 39:
- feat->edx |= HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
+ vcpu_set_cpuid_feature(vcpu, HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE);
msr->idx = HV_X64_MSR_CRASH_P0;
msr->write = 0;
msr->fault_expected = 0;
@@ -415,8 +411,8 @@ static void guest_test_msrs_access(void)
msr->fault_expected = 1;
break;
case 42:
- feat->edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
- dbg->eax |= HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING;
+ vcpu_set_cpuid_feature(vcpu, HV_FEATURE_DEBUG_MSRS_AVAILABLE);
+ vcpu_set_cpuid_feature(vcpu, HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING);
msr->idx = HV_X64_MSR_SYNDBG_STATUS;
msr->write = 0;
msr->fault_expected = 0;
@@ -463,7 +459,6 @@ static void guest_test_msrs_access(void)
static void guest_test_hcalls_access(void)
{
- struct kvm_cpuid_entry2 *feat, *recomm, *dbg;
struct kvm_cpuid2 *prev_cpuid = NULL;
struct kvm_vcpu *vcpu;
struct kvm_run *run;
@@ -498,15 +493,11 @@ static void guest_test_hcalls_access(void)
vcpu_init_cpuid(vcpu, prev_cpuid);
}
- feat = vcpu_get_cpuid_entry(vcpu, HYPERV_CPUID_FEATURES);
- recomm = vcpu_get_cpuid_entry(vcpu, HYPERV_CPUID_ENLIGHTMENT_INFO);
- dbg = vcpu_get_cpuid_entry(vcpu, HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES);
-
run = vcpu->run;
switch (stage) {
case 0:
- feat->eax |= HV_MSR_HYPERCALL_AVAILABLE;
+ vcpu_set_cpuid_feature(vcpu, HV_MSR_HYPERCALL_AVAILABLE);
hcall->control = 0xdeadbeef;
hcall->expect = HV_STATUS_INVALID_HYPERCALL_CODE;
break;
@@ -516,7 +507,7 @@ static void guest_test_hcalls_access(void)
hcall->expect = HV_STATUS_ACCESS_DENIED;
break;
case 2:
- feat->ebx |= HV_POST_MESSAGES;
+ vcpu_set_cpuid_feature(vcpu, HV_POST_MESSAGES);
hcall->control = HVCALL_POST_MESSAGE;
hcall->expect = HV_STATUS_INVALID_HYPERCALL_INPUT;
break;
@@ -526,7 +517,7 @@ static void guest_test_hcalls_access(void)
hcall->expect = HV_STATUS_ACCESS_DENIED;
break;
case 4:
- feat->ebx |= HV_SIGNAL_EVENTS;
+ vcpu_set_cpuid_feature(vcpu, HV_SIGNAL_EVENTS);
hcall->control = HVCALL_SIGNAL_EVENT;
hcall->expect = HV_STATUS_INVALID_HYPERCALL_INPUT;
break;
@@ -536,12 +527,12 @@ static void guest_test_hcalls_access(void)
hcall->expect = HV_STATUS_INVALID_HYPERCALL_CODE;
break;
case 6:
- dbg->eax |= HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING;
+ vcpu_set_cpuid_feature(vcpu, HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING);
hcall->control = HVCALL_RESET_DEBUG_SESSION;
hcall->expect = HV_STATUS_ACCESS_DENIED;
break;
case 7:
- feat->ebx |= HV_DEBUGGING;
+ vcpu_set_cpuid_feature(vcpu, HV_DEBUGGING);
hcall->control = HVCALL_RESET_DEBUG_SESSION;
hcall->expect = HV_STATUS_OPERATION_DENIED;
break;
@@ -551,7 +542,7 @@ static void guest_test_hcalls_access(void)
hcall->expect = HV_STATUS_ACCESS_DENIED;
break;
case 9:
- recomm->eax |= HV_X64_REMOTE_TLB_FLUSH_RECOMMENDED;
+ vcpu_set_cpuid_feature(vcpu, HV_X64_REMOTE_TLB_FLUSH_RECOMMENDED);
hcall->control = HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE;
hcall->expect = HV_STATUS_SUCCESS;
break;
@@ -560,7 +551,7 @@ static void guest_test_hcalls_access(void)
hcall->expect = HV_STATUS_ACCESS_DENIED;
break;
case 11:
- recomm->eax |= HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED;
+ vcpu_set_cpuid_feature(vcpu, HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED);
hcall->control = HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX;
hcall->expect = HV_STATUS_SUCCESS;
break;
@@ -570,7 +561,7 @@ static void guest_test_hcalls_access(void)
hcall->expect = HV_STATUS_ACCESS_DENIED;
break;
case 13:
- recomm->eax |= HV_X64_CLUSTER_IPI_RECOMMENDED;
+ vcpu_set_cpuid_feature(vcpu, HV_X64_CLUSTER_IPI_RECOMMENDED);
hcall->control = HVCALL_SEND_IPI;
hcall->expect = HV_STATUS_INVALID_HYPERCALL_INPUT;
break;
@@ -585,7 +576,6 @@ static void guest_test_hcalls_access(void)
hcall->expect = HV_STATUS_ACCESS_DENIED;
break;
case 16:
- recomm->ebx = 0xfff;
hcall->control = HVCALL_NOTIFY_LONG_SPIN_WAIT;
hcall->expect = HV_STATUS_SUCCESS;
break;
@@ -595,7 +585,7 @@ static void guest_test_hcalls_access(void)
hcall->ud_expected = true;
break;
case 18:
- feat->edx |= HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE;
+ vcpu_set_cpuid_feature(vcpu, HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE);
hcall->control = HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE | HV_HYPERCALL_FAST_BIT;
hcall->ud_expected = false;
hcall->expect = HV_STATUS_SUCCESS;
--
2.37.3
Normally, genuine Hyper-V doesn't expose architectural invariant TSC
(CPUID.80000007H:EDX[8]) to its guests by default. A special PV MSR
(HV_X64_MSR_TSC_INVARIANT_CONTROL, 0x40000118) and corresponding CPUID
feature bit (CPUID.0x40000003.EAX[15]) were introduced. When bit 0 of the
PV MSR is set, invariant TSC bit starts to show up in CPUID. When the
feature is exposed to Hyper-V guests, reenlightenment becomes unneeded.
Add the feature to KVM. Keep CPUID output intact when the feature
wasn't exposed to L1 and implement the required logic for hiding
invariant TSC when the feature was exposed and invariant TSC control
MSR wasn't written to. Copy genuine Hyper-V behavior and forbid to
disable the feature once it was enabled.
For the reference, for linux guests, support for the feature was added
in commit dce7cd62754b ("x86/hyperv: Allow guests to enable InvariantTSC").
Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/cpuid.c | 3 +++
arch/x86/kvm/hyperv.c | 19 +++++++++++++++++++
arch/x86/kvm/hyperv.h | 27 +++++++++++++++++++++++++++
arch/x86/kvm/x86.c | 4 +++-
5 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index ee940c4c0f64..20fa0fd665db 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1028,6 +1028,7 @@ struct kvm_hv {
u64 hv_reenlightenment_control;
u64 hv_tsc_emulation_control;
u64 hv_tsc_emulation_status;
+ u64 hv_invtsc_control;
/* How many vCPUs have VP index != vCPU index */
atomic_t num_mismatched_vp_indexes;
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index b95a4b7489ec..ea97d78d2522 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -1463,6 +1463,9 @@ bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
if (!__kvm_get_msr(vcpu, MSR_IA32_TSX_CTRL, &data, true) &&
(data & TSX_CTRL_CPUID_CLEAR))
*ebx &= ~(F(RTM) | F(HLE));
+ } else if (function == 0x80000007) {
+ if (kvm_hv_invtsc_suppressed(vcpu))
+ *edx &= ~SF(CONSTANT_TSC);
}
} else {
*eax = *ebx = *ecx = *edx = 0;
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 0adf4a437e85..41b60ec9bc53 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -989,6 +989,7 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
case HV_X64_MSR_REENLIGHTENMENT_CONTROL:
case HV_X64_MSR_TSC_EMULATION_CONTROL:
case HV_X64_MSR_TSC_EMULATION_STATUS:
+ case HV_X64_MSR_TSC_INVARIANT_CONTROL:
case HV_X64_MSR_SYNDBG_OPTIONS:
case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
r = true;
@@ -1273,6 +1274,9 @@ static bool hv_check_msr_access(struct kvm_vcpu_hv *hv_vcpu, u32 msr)
case HV_X64_MSR_TSC_EMULATION_STATUS:
return hv_vcpu->cpuid_cache.features_eax &
HV_ACCESS_REENLIGHTENMENT;
+ case HV_X64_MSR_TSC_INVARIANT_CONTROL:
+ return hv_vcpu->cpuid_cache.features_eax &
+ HV_ACCESS_TSC_INVARIANT;
case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
case HV_X64_MSR_CRASH_CTL:
return hv_vcpu->cpuid_cache.features_edx &
@@ -1400,6 +1404,17 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data,
if (!host)
return 1;
break;
+ case HV_X64_MSR_TSC_INVARIANT_CONTROL:
+ /* Only bit 0 is supported */
+ if (data & ~HV_INVARIANT_TSC_EXPOSED)
+ return 1;
+
+ /* The feature can't be disabled from the guest */
+ if (!host && hv->hv_invtsc_control && !data)
+ return 1;
+
+ hv->hv_invtsc_control = data;
+ break;
case HV_X64_MSR_SYNDBG_OPTIONS:
case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
return syndbg_set_msr(vcpu, msr, data, host);
@@ -1575,6 +1590,9 @@ static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata,
case HV_X64_MSR_TSC_EMULATION_STATUS:
data = hv->hv_tsc_emulation_status;
break;
+ case HV_X64_MSR_TSC_INVARIANT_CONTROL:
+ data = hv->hv_invtsc_control;
+ break;
case HV_X64_MSR_SYNDBG_OPTIONS:
case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
return syndbg_get_msr(vcpu, msr, pdata, host);
@@ -2491,6 +2509,7 @@ int kvm_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
ent->eax |= HV_MSR_REFERENCE_TSC_AVAILABLE;
ent->eax |= HV_ACCESS_FREQUENCY_MSRS;
ent->eax |= HV_ACCESS_REENLIGHTENMENT;
+ ent->eax |= HV_ACCESS_TSC_INVARIANT;
ent->ebx |= HV_POST_MESSAGES;
ent->ebx |= HV_SIGNAL_EVENTS;
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 1030b1b50552..0f7382f2b157 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -136,6 +136,33 @@ static inline bool kvm_hv_has_stimer_pending(struct kvm_vcpu *vcpu)
HV_SYNIC_STIMER_COUNT);
}
+/*
+ * With HV_ACCESS_TSC_INVARIANT feature, invariant TSC (CPUID.80000007H:EDX[8])
+ * is only observed after HV_X64_MSR_TSC_INVARIANT_CONTROL was written to.
+ */
+static inline bool kvm_hv_invtsc_suppressed(struct kvm_vcpu *vcpu)
+{
+ struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
+
+ /*
+ * If Hyper-V's invariant TSC control is not exposed to the guest,
+ * the invariant TSC CPUID flag is not suppressed, Windows guests were
+ * observed to be able to handle it correctly. Going forward, VMMs are
+ * encouraged to enable Hyper-V's invariant TSC control when invariant
+ * TSC CPUID flag is set to make KVM's behavior match genuine Hyper-V.
+ */
+ if (!hv_vcpu ||
+ !(hv_vcpu->cpuid_cache.features_eax & HV_ACCESS_TSC_INVARIANT))
+ return false;
+
+ /*
+ * If Hyper-V's invariant TSC control is exposed to the guest, KVM is
+ * responsible for suppressing the invariant TSC CPUID flag if the
+ * Hyper-V control is not enabled.
+ */
+ return !(to_kvm_hv(vcpu->kvm)->hv_invtsc_control & HV_INVARIANT_TSC_EXPOSED);
+}
+
void kvm_hv_process_stimers(struct kvm_vcpu *vcpu);
void kvm_hv_setup_tsc_page(struct kvm *kvm,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5b8328cb6c14..d9a9335d28bd 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1489,7 +1489,7 @@ static const u32 emulated_msrs_all[] = {
HV_X64_MSR_STIMER0_CONFIG,
HV_X64_MSR_VP_ASSIST_PAGE,
HV_X64_MSR_REENLIGHTENMENT_CONTROL, HV_X64_MSR_TSC_EMULATION_CONTROL,
- HV_X64_MSR_TSC_EMULATION_STATUS,
+ HV_X64_MSR_TSC_EMULATION_STATUS, HV_X64_MSR_TSC_INVARIANT_CONTROL,
HV_X64_MSR_SYNDBG_OPTIONS,
HV_X64_MSR_SYNDBG_CONTROL, HV_X64_MSR_SYNDBG_STATUS,
HV_X64_MSR_SYNDBG_SEND_BUFFER, HV_X64_MSR_SYNDBG_RECV_BUFFER,
@@ -3795,6 +3795,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
case HV_X64_MSR_REENLIGHTENMENT_CONTROL:
case HV_X64_MSR_TSC_EMULATION_CONTROL:
case HV_X64_MSR_TSC_EMULATION_STATUS:
+ case HV_X64_MSR_TSC_INVARIANT_CONTROL:
return kvm_hv_set_msr_common(vcpu, msr, data,
msr_info->host_initiated);
case MSR_IA32_BBL_CR_CTL3:
@@ -4165,6 +4166,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
case HV_X64_MSR_REENLIGHTENMENT_CONTROL:
case HV_X64_MSR_TSC_EMULATION_CONTROL:
case HV_X64_MSR_TSC_EMULATION_STATUS:
+ case HV_X64_MSR_TSC_INVARIANT_CONTROL:
return kvm_hv_get_msr_common(vcpu,
msr_info->index, &msr_info->data,
msr_info->host_initiated);
--
2.37.3
Avoid open coding BIT(0) of HV_X64_MSR_TSC_INVARIANT_CONTROL by adding
a dedicated define. While there's only one user at this moment, the
upcoming KVM implementation of Hyper-V Invariant TSC feature will need
to use it as well.
Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
arch/x86/include/asm/hyperv-tlfs.h | 3 +++
arch/x86/kernel/cpu/mshyperv.c | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 3089ec352743..4849f879948d 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -253,6 +253,9 @@ enum hv_isolation_type {
/* TSC invariant control */
#define HV_X64_MSR_TSC_INVARIANT_CONTROL 0x40000118
+/* HV_X64_MSR_TSC_INVARIANT_CONTROL bits */
+#define HV_INVARIANT_TSC_EXPOSED BIT_ULL(0)
+
/* Register name aliases for temporary compatibility */
#define HV_X64_MSR_STIMER0_COUNT HV_REGISTER_STIMER0_COUNT
#define HV_X64_MSR_STIMER0_CONFIG HV_REGISTER_STIMER0_CONFIG
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 831613959a92..3716c358da98 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -388,7 +388,7 @@ static void __init ms_hyperv_init_platform(void)
* setting of this MSR bit should happen before init_intel()
* is called.
*/
- wrmsrl(HV_X64_MSR_TSC_INVARIANT_CONTROL, 0x1);
+ wrmsrl(HV_X64_MSR_TSC_INVARIANT_CONTROL, HV_INVARIANT_TSC_EXPOSED);
setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
}
--
2.37.3
Why do we need a new 'scattered' leaf? We are only using two bits in
the first one, right? And now we're only going to use one bit in the
second one?
I thought the point of the 'scattered' leaves was to collect a bunch
of feature bits from different CPUID leaves in a single feature word.
Allocating a new feature word for one or two bits seems extravagant.
On Thu, Sep 22, 2022 at 10:53 AM Sean Christopherson <[email protected]> wrote:
>
> On Thu, Sep 22, 2022, Jim Mattson wrote:
> > Why do we need a new 'scattered' leaf? We are only using two bits in
> > the first one, right? And now we're only going to use one bit in the
> > second one?
> >
> > I thought the point of the 'scattered' leaves was to collect a bunch
> > of feature bits from different CPUID leaves in a single feature word.
> >
> > Allocating a new feature word for one or two bits seems extravagant.
>
> Ah, these leafs aren't scattered from KVM's perspective.
>
> The scattered terminology comes from the kernel side, where the KVM-only leafs
> _may_ be used to deal with features that are scattered by the kernel. But there
> is no requirement that KVM-only leafs _must_ be scattered by the kernel, e.g. we
> can and should use this for leafs that KVM wants to expose to the guest, but are
> completely ignored by the kernel. Intel's PSFD feature flag is a good example.
>
> A better shortlog would be:
>
> KVM: x86: Add a KVM-only leaf for CPUID_8000_0007_EDX
Thanks. The 'scattered' terminology seems more confusing than enlightening.
On Thu, Sep 22, 2022, Jim Mattson wrote:
> Why do we need a new 'scattered' leaf? We are only using two bits in
> the first one, right? And now we're only going to use one bit in the
> second one?
>
> I thought the point of the 'scattered' leaves was to collect a bunch
> of feature bits from different CPUID leaves in a single feature word.
>
> Allocating a new feature word for one or two bits seems extravagant.
Ah, these leafs aren't scattered from KVM's perspective.
The scattered terminology comes from the kernel side, where the KVM-only leafs
_may_ be used to deal with features that are scattered by the kernel. But there
is no requirement that KVM-only leafs _must_ be scattered by the kernel, e.g. we
can and should use this for leafs that KVM wants to expose to the guest, but are
completely ignored by the kernel. Intel's PSFD feature flag is a good example.
A better shortlog would be:
KVM: x86: Add a KVM-only leaf for CPUID_8000_0007_EDX
From: Vitaly Kuznetsov <[email protected]> Sent: Thursday, September 22, 2022 7:37 AM
>
> Avoid open coding BIT(0) of HV_X64_MSR_TSC_INVARIANT_CONTROL by adding
> a dedicated define. While there's only one user at this moment, the
> upcoming KVM implementation of Hyper-V Invariant TSC feature will need
> to use it as well.
>
> Signed-off-by: Vitaly Kuznetsov <[email protected]>
> ---
> arch/x86/include/asm/hyperv-tlfs.h | 3 +++
> arch/x86/kernel/cpu/mshyperv.c | 2 +-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index 3089ec352743..4849f879948d 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -253,6 +253,9 @@ enum hv_isolation_type {
> /* TSC invariant control */
> #define HV_X64_MSR_TSC_INVARIANT_CONTROL 0x40000118
>
> +/* HV_X64_MSR_TSC_INVARIANT_CONTROL bits */
> +#define HV_INVARIANT_TSC_EXPOSED BIT_ULL(0)
Nit: This name sounds like a value that is read from some register,
to find out if an Invariant TSC is exposed. But it's actually a value that
is written to make something happen. So my brain wants to name it
something more like HV_EXPOSE_INVARIANT_TSC. Not a big
enough issue to respin for, but if you do respin then maybe change
it.
> +
> /* Register name aliases for temporary compatibility */
> #define HV_X64_MSR_STIMER0_COUNT HV_REGISTER_STIMER0_COUNT
> #define HV_X64_MSR_STIMER0_CONFIG HV_REGISTER_STIMER0_CONFIG
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 831613959a92..3716c358da98 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -388,7 +388,7 @@ static void __init ms_hyperv_init_platform(void)
> * setting of this MSR bit should happen before init_intel()
> * is called.
> */
> - wrmsrl(HV_X64_MSR_TSC_INVARIANT_CONTROL, 0x1);
> + wrmsrl(HV_X64_MSR_TSC_INVARIANT_CONTROL, HV_INVARIANT_TSC_EXPOSED);
> setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> }
>
> --
> 2.37.3
Reviewed-by: Michael Kelley <[email protected]>
Nit, s/availble,/available
On Thu, Sep 22, 2022, Vitaly Kuznetsov wrote:
> It may not be clear what 'msr->availble' means. The test actually
Same typo here.
> checks that accessing the particular MSR doesn't cause #GP, rename
> the varialble accordingly.
s/varialble/variable
At least you're consistent :-)
> Signed-off-by: Vitaly Kuznetsov <[email protected]>
> ---
> .../selftests/kvm/x86_64/hyperv_features.c | 96 +++++++++----------
> 1 file changed, 48 insertions(+), 48 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> index 79ab0152d281..1383b979e90b 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> @@ -33,7 +33,7 @@ static inline uint8_t hypercall(u64 control, vm_vaddr_t input_address,
>
> struct msr_data {
> uint32_t idx;
> - bool available;
> + bool fault_expected;
> bool write;
> u64 write_val;
> };
> @@ -56,10 +56,10 @@ static void guest_msr(struct msr_data *msr)
> else
> vector = wrmsr_safe(msr->idx, msr->write_val);
>
> - if (msr->available)
> - GUEST_ASSERT_2(!vector, msr->idx, vector);
> - else
> + if (msr->fault_expected)
> GUEST_ASSERT_2(vector == GP_VECTOR, msr->idx, vector);
> + else
> + GUEST_ASSERT_2(!vector, msr->idx, vector);
> GUEST_DONE();
> }
>
> @@ -153,12 +153,12 @@ static void guest_test_msrs_access(void)
> */
> msr->idx = HV_X64_MSR_GUEST_OS_ID;
> msr->write = 0;
> - msr->available = 0;
> + msr->fault_expected = 1;
Since all of these are getting inverted, opportunistically use "true" instead of "1"?
On Thu, Sep 22, 2022, Vitaly Kuznetsov wrote:
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> index d4bd18bc580d..18b44450dfb8 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> @@ -46,20 +46,33 @@ struct hcall_data {
>
> static void guest_msr(struct msr_data *msr)
> {
> - uint64_t ignored;
> + uint64_t msr_val = 0;
> uint8_t vector;
>
> GUEST_ASSERT(msr->idx);
>
> - if (!msr->write)
> - vector = rdmsr_safe(msr->idx, &ignored);
> - else
> + if (!msr->write) {
> + vector = rdmsr_safe(msr->idx, &msr_val);
This is subtly going to do weird things if the RDMSR faults. rdmsr_safe()
overwrites @val with whatever happens to be in EDX:EAX if the RDMSR faults, i.e.
this may yield garbage instead of '0'. Arguably rdmsr_safe() is a bad API, but
at the same time the caller really shouldn't consume the result if RDMSR faults
(though aligning with the kernel is also valuable).
Aha! Idea. Assuming none of the MSRs are write-only, what about adding a prep
patch to rework this code so that it verifies RDMSR returns what was written when
a fault didn't occur.
uint8_t vector = 0;
uint64_t msr_val;
GUEST_ASSERT(msr->idx);
if (msr->write)
vector = wrmsr_safe(msr->idx, msr->write_val);
if (!vector)
vector = rdmsr_safe(msr->idx, &msr_val);
if (msr->fault_expected)
GUEST_ASSERT_2(vector == GP_VECTOR, msr->idx, vector);
else
GUEST_ASSERT_2(!vector, msr->idx, vector);
if (vector)
goto done;
GUEST_ASSERT_2(msr_val == msr->write_val, msr_val, msr->write_val);
done:
GUEST_DONE();
and then this patch can just slot in the extra check:
uint8_t vector = 0;
uint64_t msr_val;
GUEST_ASSERT(msr->idx);
if (msr->write)
vector = wrmsr_safe(msr->idx, msr->write_val);
if (!vector)
vector = rdmsr_safe(msr->idx, &msr_val);
if (msr->fault_expected)
GUEST_ASSERT_2(vector == GP_VECTOR, msr->idx, vector);
else
GUEST_ASSERT_2(!vector, msr->idx, vector);
if (vector)
goto done;
GUEST_ASSERT_2(msr_val == msr->write_val, msr_val, msr->write_val);
/* Invariant TSC bit appears when TSC invariant control MSR is written to */
if (msr->idx == HV_X64_MSR_TSC_INVARIANT_CONTROL) {
if (!this_cpu_has(HV_ACCESS_TSC_INVARIANT))
GUEST_ASSERT(this_cpu_has(X86_FEATURE_INVTSC));
else
GUEST_ASSERT(this_cpu_has(X86_FEATURE_INVTSC) ==
!!(msr_val & HV_INVARIANT_TSC_EXPOSED));
}
done:
GUEST_DONE();
On Thu, Sep 22, 2022, Vitaly Kuznetsov wrote:
> diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
> index a19d473d0184..a5514c89dc29 100644
> --- a/arch/x86/kvm/reverse_cpuid.h
> +++ b/arch/x86/kvm/reverse_cpuid.h
> @@ -12,7 +12,8 @@
> * "bug" caps, but KVM doesn't use those.
> */
> enum kvm_only_cpuid_leafs {
> - CPUID_12_EAX = NCAPINTS,
> + CPUID_12_EAX = NCAPINTS,
> + CPUID_8000_0007_EDX = NCAPINTS + 1,
No need to explicitly initialize the new leaf, only the first enum entry needs
explicit initialization to NCAPINTS, i.e. let all other entries automatically
increment. The order doesn't matter, so not caring about the exact value will
avoid bugs due to mismerge and/or bad copy+paste.
Sean Christopherson <[email protected]> writes:
> On Thu, Sep 22, 2022, Vitaly Kuznetsov wrote:
>> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
>> index d4bd18bc580d..18b44450dfb8 100644
>> --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
>> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
>> @@ -46,20 +46,33 @@ struct hcall_data {
>>
>> static void guest_msr(struct msr_data *msr)
>> {
>> - uint64_t ignored;
>> + uint64_t msr_val = 0;
>> uint8_t vector;
>>
>> GUEST_ASSERT(msr->idx);
>>
>> - if (!msr->write)
>> - vector = rdmsr_safe(msr->idx, &ignored);
>> - else
>> + if (!msr->write) {
>> + vector = rdmsr_safe(msr->idx, &msr_val);
>
> This is subtly going to do weird things if the RDMSR faults. rdmsr_safe()
> overwrites @val with whatever happens to be in EDX:EAX if the RDMSR faults, i.e.
> this may yield garbage instead of '0'. Arguably rdmsr_safe() is a bad API, but
> at the same time the caller really shouldn't consume the result if RDMSR faults
> (though aligning with the kernel is also valuable).
>
> Aha! Idea. Assuming none of the MSRs are write-only, what about adding a prep
> patch to rework this code so that it verifies RDMSR returns what was written when
> a fault didn't occur.
>
There is at least one read-only MSR which comes to mind:
HV_X64_MSR_EOI. Also, some of the MSRs don't preserve the written value,
e.g. HV_X64_MSR_RESET which always reads as '0'.
I do, however, like the additional check that RDMSR returns what was
written to the MSR, we will just need an additional flag in 'struct
msr_data' ('check_written_value' maybe?).
--
Vitaly
On Wed, Oct 12, 2022, Vitaly Kuznetsov wrote:
> Sean Christopherson <[email protected]> writes:
>
> > On Thu, Sep 22, 2022, Vitaly Kuznetsov wrote:
> >> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> >> index d4bd18bc580d..18b44450dfb8 100644
> >> --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> >> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> >> @@ -46,20 +46,33 @@ struct hcall_data {
> >>
> >> static void guest_msr(struct msr_data *msr)
> >> {
> >> - uint64_t ignored;
> >> + uint64_t msr_val = 0;
> >> uint8_t vector;
> >>
> >> GUEST_ASSERT(msr->idx);
> >>
> >> - if (!msr->write)
> >> - vector = rdmsr_safe(msr->idx, &ignored);
> >> - else
> >> + if (!msr->write) {
> >> + vector = rdmsr_safe(msr->idx, &msr_val);
> >
> > This is subtly going to do weird things if the RDMSR faults. rdmsr_safe()
> > overwrites @val with whatever happens to be in EDX:EAX if the RDMSR faults, i.e.
> > this may yield garbage instead of '0'. Arguably rdmsr_safe() is a bad API, but
> > at the same time the caller really shouldn't consume the result if RDMSR faults
> > (though aligning with the kernel is also valuable).
> >
> > Aha! Idea. Assuming none of the MSRs are write-only, what about adding a prep
> > patch to rework this code so that it verifies RDMSR returns what was written when
> > a fault didn't occur.
> >
>
> There is at least one read-only MSR which comes to mind:
> HV_X64_MSR_EOI.
I assume s/read-only/write-only since it's EOI?
> Also, some of the MSRs don't preserve the written value,
> e.g. HV_X64_MSR_RESET which always reads as '0'.
Hrm, that's annoying.
> I do, however, like the additional check that RDMSR returns what was
> written to the MSR, we will just need an additional flag in 'struct
> msr_data' ('check_written_value' maybe?).
Rather than force the testcase to specify information that's intrinsic to the MSR,
what about adding helpers to communicate the types? E.g.
if (msr->write)
vector = wrmsr_safe(msr->idx, msr->write_val);
if (!vector && !is_write_only_msr(msr->idx))
vector = rdmsr_safe(msr->idx, &msr_val);
if (msr->fault_expected)
GUEST_ASSERT_2(vector == GP_VECTOR, msr->idx, vector);
else
GUEST_ASSERT_2(!vector, msr->idx, vector);
if (is_read_zero_msr(msr->idx))
GUEST_ASSERT_2(msr_val == 0, msr_val, 0);
else
GUEST_ASSERT_2(msr_val == msr->write_val, msr_val, msr->write_val);
I think that'd make the code a bit less magical and easier to understand for folks
that aren't familiar with Hyper-V. The number of special MSRs is likely very small,
so the helpers should be trivial, e.g.
static bool is_write_only_msr(uint32_t msr)
{
return msr == HV_X64_MSR_EOI;
}
static bool is_read_zero_msr(uint32_t msr)
{
return msr == HV_X64_MSR_RESET || msr == ???;
}
Sean Christopherson <[email protected]> writes:
> On Wed, Oct 12, 2022, Vitaly Kuznetsov wrote:
>> Sean Christopherson <[email protected]> writes:
>>
...
>> > Aha! Idea. Assuming none of the MSRs are write-only, what about adding a prep
>> > patch to rework this code so that it verifies RDMSR returns what was written when
>> > a fault didn't occur.
>> >
>>
>> There is at least one read-only MSR which comes to mind:
>> HV_X64_MSR_EOI.
>
> I assume s/read-only/write-only since it's EOI?
>
Yes, of course)
>> Also, some of the MSRs don't preserve the written value,
>> e.g. HV_X64_MSR_RESET which always reads as '0'.
>
> Hrm, that's annoying.
'Slightly annoying'. In fact, the test never writes anything besides '0'
to the MSR as the code is not ready to handle real vCPU reset. I'll
leave a TODO note about that.
...
> static bool is_write_only_msr(uint32_t msr)
> {
> return msr == HV_X64_MSR_EOI;
> }
This is all we need, basically. I'll go with that.
--
Vitaly