public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] KVM: x86: Provide a capability to disable APERF/MPERF read intercepts
@ 2025-03-21 22:14 Jim Mattson
  2025-03-21 22:14 ` [PATCH v3 1/2] " Jim Mattson
  2025-03-21 22:14 ` [PATCH v3 2/2] KVM: selftests: Test behavior of KVM_X86_DISABLE_EXITS_APERFMPERF Jim Mattson
  0 siblings, 2 replies; 10+ messages in thread
From: Jim Mattson @ 2025-03-21 22:14 UTC (permalink / raw)
  To: linux-kernel, kvm, Sean Christopherson, Paolo Bonzini; +Cc: Jim Mattson

Allow guest reads of IA32_APERF and IA32_MPERF, so that it can
determine the effective frequency multiplier for the physical LPU.

Commit b51700632e0e ("KVM: X86: Provide a capability to disable cstate
msr read intercepts") allowed the userspace VMM to grant a guest read
access to four core C-state residency MSRs. Do the same for IA32_APERF
and IA32_MPERF.

While this isn't sufficient to claim support for
CPUID.6:ECX.APERFMPERF[bit 0], it may suffice in a sufficiently
restricted environment (i.e. vCPUs pinned to LPUs, no TSC multiplier,
and no suspend/resume).

v1 -> v2: Add {IA32_APERF,IA32_MPERF} to vmx_possible_passthrough_msrs[]
v2 -> v3: Add a selftest

Jim Mattson (2):
  KVM: x86: Provide a capability to disable APERF/MPERF read intercepts
  KVM: selftests: Test behavior of KVM_X86_DISABLE_EXITS_APERFMPERF

 Documentation/virt/kvm/api.rst                |   1 +
 arch/x86/include/asm/kvm_host.h               |   1 +
 arch/x86/kvm/svm/svm.c                        |   7 +
 arch/x86/kvm/svm/svm.h                        |   2 +-
 arch/x86/kvm/vmx/vmx.c                        |   6 +
 arch/x86/kvm/vmx/vmx.h                        |   2 +-
 arch/x86/kvm/x86.c                            |   8 +-
 arch/x86/kvm/x86.h                            |   5 +
 include/uapi/linux/kvm.h                      |   1 +
 tools/include/uapi/linux/kvm.h                |   4 +-
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../selftests/kvm/x86/aperfmperf_test.c       | 162 ++++++++++++++++++
 12 files changed, 196 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86/aperfmperf_test.c

-- 
2.49.0.395.g12beb8f557-goog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/2] KVM: x86: Provide a capability to disable APERF/MPERF read intercepts
  2025-03-21 22:14 [PATCH v3 0/2] KVM: x86: Provide a capability to disable APERF/MPERF read intercepts Jim Mattson
@ 2025-03-21 22:14 ` Jim Mattson
  2025-04-28 22:58   ` Sean Christopherson
  2025-03-21 22:14 ` [PATCH v3 2/2] KVM: selftests: Test behavior of KVM_X86_DISABLE_EXITS_APERFMPERF Jim Mattson
  1 sibling, 1 reply; 10+ messages in thread
From: Jim Mattson @ 2025-03-21 22:14 UTC (permalink / raw)
  To: linux-kernel, kvm, Sean Christopherson, Paolo Bonzini; +Cc: Jim Mattson

Allow a guest to read the physical IA32_APERF and IA32_MPERF MSRs
without interception.

The IA32_APERF and IA32_MPERF MSRs are not virtualized. Writes are not
handled at all. The MSR values are not zeroed on vCPU creation, saved
on suspend, or restored on resume. No accommodation is made for
processor migration or for sharing a logical processor with other
tasks. No adjustments are made for non-unit TSC multipliers. The MSRs
do not account for time the same way as the comparable PMU events,
whether the PMU is virtualized by the traditional emulation method or
the new mediated pass-through approach.

Nonetheless, in a properly constrained environment, this capability
can be combined with a guest CPUID table that advertises support for
CPUID.6:ECX.APERFMPERF[bit 0] to induce a Linux guest to report the
effective physical CPU frequency in /proc/cpuinfo. Moreover, there is
no performance cost for this capability.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 Documentation/virt/kvm/api.rst  | 1 +
 arch/x86/include/asm/kvm_host.h | 1 +
 arch/x86/kvm/svm/svm.c          | 7 +++++++
 arch/x86/kvm/svm/svm.h          | 2 +-
 arch/x86/kvm/vmx/vmx.c          | 6 ++++++
 arch/x86/kvm/vmx/vmx.h          | 2 +-
 arch/x86/kvm/x86.c              | 8 +++++++-
 arch/x86/kvm/x86.h              | 5 +++++
 include/uapi/linux/kvm.h        | 1 +
 tools/include/uapi/linux/kvm.h  | 4 +++-
 10 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 2b52eb77e29c..6431cd33f06a 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -7684,6 +7684,7 @@ Valid bits in args[0] are::
   #define KVM_X86_DISABLE_EXITS_HLT              (1 << 1)
   #define KVM_X86_DISABLE_EXITS_PAUSE            (1 << 2)
   #define KVM_X86_DISABLE_EXITS_CSTATE           (1 << 3)
+  #define KVM_X86_DISABLE_EXITS_APERFMPERF       (1 << 4)
 
 Enabling this capability on a VM provides userspace with a way to no
 longer intercept some instructions for improved latency in some
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 32ae3aa50c7e..1287f365eff7 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1381,6 +1381,7 @@ struct kvm_arch {
 	bool hlt_in_guest;
 	bool pause_in_guest;
 	bool cstate_in_guest;
+	bool aperfmperf_in_guest;
 
 	unsigned long irq_sources_bitmap;
 	s64 kvmclock_offset;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e67de787fc71..439e5f41f29e 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -111,6 +111,8 @@ static const struct svm_direct_access_msrs {
 	{ .index = MSR_IA32_CR_PAT,			.always = false },
 	{ .index = MSR_AMD64_SEV_ES_GHCB,		.always = true  },
 	{ .index = MSR_TSC_AUX,				.always = false },
+	{ .index = MSR_IA32_APERF,			.always = false },
+	{ .index = MSR_IA32_MPERF,			.always = false },
 	{ .index = X2APIC_MSR(APIC_ID),			.always = false },
 	{ .index = X2APIC_MSR(APIC_LVR),		.always = false },
 	{ .index = X2APIC_MSR(APIC_TASKPRI),		.always = false },
@@ -1359,6 +1361,11 @@ static void init_vmcb(struct kvm_vcpu *vcpu)
 	if (boot_cpu_has(X86_FEATURE_V_SPEC_CTRL))
 		set_msr_interception(vcpu, svm->msrpm, MSR_IA32_SPEC_CTRL, 1, 1);
 
+	if (kvm_aperfmperf_in_guest(vcpu->kvm)) {
+		set_msr_interception(vcpu, svm->msrpm, MSR_IA32_APERF, 1, 0);
+		set_msr_interception(vcpu, svm->msrpm, MSR_IA32_MPERF, 1, 0);
+	}
+
 	if (kvm_vcpu_apicv_active(vcpu))
 		avic_init_vmcb(svm, vmcb);
 
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index ea44c1da5a7c..5b38d5c00788 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -44,7 +44,7 @@ static inline struct page *__sme_pa_to_page(unsigned long pa)
 #define	IOPM_SIZE PAGE_SIZE * 3
 #define	MSRPM_SIZE PAGE_SIZE * 2
 
-#define MAX_DIRECT_ACCESS_MSRS	48
+#define MAX_DIRECT_ACCESS_MSRS	50
 #define MSRPM_OFFSETS	32
 extern u32 msrpm_offsets[MSRPM_OFFSETS] __read_mostly;
 extern bool npt_enabled;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 3b92f893b239..6d07313e4472 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -186,6 +186,8 @@ static u32 vmx_possible_passthrough_msrs[MAX_POSSIBLE_PASSTHROUGH_MSRS] = {
 	MSR_CORE_C3_RESIDENCY,
 	MSR_CORE_C6_RESIDENCY,
 	MSR_CORE_C7_RESIDENCY,
+	MSR_IA32_APERF,
+	MSR_IA32_MPERF,
 };
 
 /*
@@ -7593,6 +7595,10 @@ int vmx_vcpu_create(struct kvm_vcpu *vcpu)
 		vmx_disable_intercept_for_msr(vcpu, MSR_CORE_C6_RESIDENCY, MSR_TYPE_R);
 		vmx_disable_intercept_for_msr(vcpu, MSR_CORE_C7_RESIDENCY, MSR_TYPE_R);
 	}
+	if (kvm_aperfmperf_in_guest(vcpu->kvm)) {
+		vmx_disable_intercept_for_msr(vcpu, MSR_IA32_APERF, MSR_TYPE_R);
+		vmx_disable_intercept_for_msr(vcpu, MSR_IA32_MPERF, MSR_TYPE_R);
+	}
 
 	vmx->loaded_vmcs = &vmx->vmcs01;
 
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 951e44dc9d0e..0d3c5bdd556e 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -356,7 +356,7 @@ struct vcpu_vmx {
 	struct lbr_desc lbr_desc;
 
 	/* Save desired MSR intercept (read: pass-through) state */
-#define MAX_POSSIBLE_PASSTHROUGH_MSRS	16
+#define MAX_POSSIBLE_PASSTHROUGH_MSRS	18
 	struct {
 		DECLARE_BITMAP(read, MAX_POSSIBLE_PASSTHROUGH_MSRS);
 		DECLARE_BITMAP(write, MAX_POSSIBLE_PASSTHROUGH_MSRS);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 4b64ab350bcd..1b3cdca806b4 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4535,6 +4535,9 @@ static u64 kvm_get_allowed_disable_exits(void)
 {
 	u64 r = KVM_X86_DISABLE_EXITS_PAUSE;
 
+	if (boot_cpu_has(X86_FEATURE_APERFMPERF))
+		r |= KVM_X86_DISABLE_EXITS_APERFMPERF;
+
 	if (!mitigate_smt_rsb) {
 		r |= KVM_X86_DISABLE_EXITS_HLT |
 			KVM_X86_DISABLE_EXITS_CSTATE;
@@ -6543,7 +6546,8 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 
 		if (!mitigate_smt_rsb && boot_cpu_has_bug(X86_BUG_SMT_RSB) &&
 		    cpu_smt_possible() &&
-		    (cap->args[0] & ~KVM_X86_DISABLE_EXITS_PAUSE))
+		    (cap->args[0] & ~(KVM_X86_DISABLE_EXITS_PAUSE |
+				      KVM_X86_DISABLE_EXITS_APERFMPERF)))
 			pr_warn_once(SMT_RSB_MSG);
 
 		if (cap->args[0] & KVM_X86_DISABLE_EXITS_PAUSE)
@@ -6554,6 +6558,8 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 			kvm->arch.hlt_in_guest = true;
 		if (cap->args[0] & KVM_X86_DISABLE_EXITS_CSTATE)
 			kvm->arch.cstate_in_guest = true;
+		if (cap->args[0] & KVM_X86_DISABLE_EXITS_APERFMPERF)
+			kvm->arch.aperfmperf_in_guest = true;
 		r = 0;
 disable_exits_unlock:
 		mutex_unlock(&kvm->lock);
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 91e50a513100..0c3ac99454e5 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -488,6 +488,11 @@ static inline bool kvm_cstate_in_guest(struct kvm *kvm)
 	return kvm->arch.cstate_in_guest;
 }
 
+static inline bool kvm_aperfmperf_in_guest(struct kvm *kvm)
+{
+	return kvm->arch.aperfmperf_in_guest;
+}
+
 static inline bool kvm_notify_vmexit_enabled(struct kvm *kvm)
 {
 	return kvm->arch.notify_vmexit_flags & KVM_X86_NOTIFY_VMEXIT_ENABLED;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 45e6d8fca9b9..b4a4eb52f6df 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -617,6 +617,7 @@ struct kvm_ioeventfd {
 #define KVM_X86_DISABLE_EXITS_HLT            (1 << 1)
 #define KVM_X86_DISABLE_EXITS_PAUSE          (1 << 2)
 #define KVM_X86_DISABLE_EXITS_CSTATE         (1 << 3)
+#define KVM_X86_DISABLE_EXITS_APERFMPERF     (1 << 4)
 
 /* for KVM_ENABLE_CAP */
 struct kvm_enable_cap {
diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
index 502ea63b5d2e..9b60f0509cdc 100644
--- a/tools/include/uapi/linux/kvm.h
+++ b/tools/include/uapi/linux/kvm.h
@@ -617,10 +617,12 @@ struct kvm_ioeventfd {
 #define KVM_X86_DISABLE_EXITS_HLT            (1 << 1)
 #define KVM_X86_DISABLE_EXITS_PAUSE          (1 << 2)
 #define KVM_X86_DISABLE_EXITS_CSTATE         (1 << 3)
+#define KVM_X86_DISABLE_EXITS_APERFMPERF     (1 << 4)
 #define KVM_X86_DISABLE_VALID_EXITS          (KVM_X86_DISABLE_EXITS_MWAIT | \
                                               KVM_X86_DISABLE_EXITS_HLT | \
                                               KVM_X86_DISABLE_EXITS_PAUSE | \
-                                              KVM_X86_DISABLE_EXITS_CSTATE)
+					      KVM_X86_DISABLE_EXITS_CSTATE | \
+					      KVM_X86_DISABLE_EXITS_APERFMPERF)
 
 /* for KVM_ENABLE_CAP */
 struct kvm_enable_cap {
-- 
2.49.0.395.g12beb8f557-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/2] KVM: selftests: Test behavior of KVM_X86_DISABLE_EXITS_APERFMPERF
  2025-03-21 22:14 [PATCH v3 0/2] KVM: x86: Provide a capability to disable APERF/MPERF read intercepts Jim Mattson
  2025-03-21 22:14 ` [PATCH v3 1/2] " Jim Mattson
@ 2025-03-21 22:14 ` Jim Mattson
  2025-04-29  1:26   ` Sean Christopherson
  1 sibling, 1 reply; 10+ messages in thread
From: Jim Mattson @ 2025-03-21 22:14 UTC (permalink / raw)
  To: linux-kernel, kvm, Sean Christopherson, Paolo Bonzini; +Cc: Jim Mattson

For a VCPU thread pinned to a single LPU, verify that interleaved host
and guest reads of IA32_[AM]PERF return strictly increasing values when
APERFMPERF exiting is disabled.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../selftests/kvm/x86/aperfmperf_test.c       | 162 ++++++++++++++++++
 2 files changed, 163 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86/aperfmperf_test.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 4277b983cace..bfee69b33310 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -116,6 +116,7 @@ TEST_GEN_PROGS_x86 += x86/amx_test
 TEST_GEN_PROGS_x86 += x86/max_vcpuid_cap_test
 TEST_GEN_PROGS_x86 += x86/triple_fault_event_test
 TEST_GEN_PROGS_x86 += x86/recalc_apic_map_test
+TEST_GEN_PROGS_x86 += x86/aperfmperf_test
 TEST_GEN_PROGS_x86 += access_tracking_perf_test
 TEST_GEN_PROGS_x86 += coalesced_io_test
 TEST_GEN_PROGS_x86 += demand_paging_test
diff --git a/tools/testing/selftests/kvm/x86/aperfmperf_test.c b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
new file mode 100644
index 000000000000..7473afb7f6fa
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Test for KVM_X86_DISABLE_EXITS_APERFMPERF
+ *
+ * Copyright (C) 2025, Google LLC.
+ *
+ * Test the ability to disable VM-exits for rdmsr of IA32_APERF and
+ * IA32_MPERF. When these VM-exits are disabled, reads of these MSRs
+ * return the host's values.
+ *
+ * Note: Requires read access to /dev/cpu/<lpu>/msr to read host MSRs.
+ */
+
+#include <fcntl.h>
+#include <limits.h>
+#include <pthread.h>
+#include <sched.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <unistd.h>
+#include <asm/msr-index.h>
+
+#include "kvm_util.h"
+#include "processor.h"
+#include "test_util.h"
+
+#define NUM_ITERATIONS 100
+
+static void pin_thread(int cpu)
+{
+	cpu_set_t cpuset;
+	int rc;
+
+	CPU_ZERO(&cpuset);
+	CPU_SET(cpu, &cpuset);
+
+	rc = pthread_setaffinity_np(pthread_self(), sizeof(cpuset), &cpuset);
+	TEST_ASSERT(rc == 0, "%s: Can't set thread affinity", __func__);
+}
+
+static int open_dev_msr(int cpu)
+{
+	char path[PATH_MAX];
+	int msr_fd;
+
+	snprintf(path, sizeof(path), "/dev/cpu/%d/msr", cpu);
+	msr_fd = open(path, O_RDONLY);
+	__TEST_REQUIRE(msr_fd >= 0, "Can't open %s for read", path);
+
+	return msr_fd;
+}
+
+static uint64_t read_dev_msr(int msr_fd, uint32_t msr)
+{
+	uint64_t data;
+	ssize_t rc;
+
+	rc = pread(msr_fd, &data, sizeof(data), msr);
+	TEST_ASSERT(rc == sizeof(data), "Read of MSR 0x%x failed", msr);
+
+	return data;
+}
+
+static void guest_code(void)
+{
+	int i;
+
+	for (i = 0; i < NUM_ITERATIONS; i++) {
+		uint64_t aperf = rdmsr(MSR_IA32_APERF);
+		uint64_t mperf = rdmsr(MSR_IA32_MPERF);
+
+		GUEST_SYNC2(aperf, mperf);
+	}
+
+	GUEST_DONE();
+}
+
+static bool kvm_can_disable_aperfmperf_exits(struct kvm_vm *vm)
+{
+	int flags = vm_check_cap(vm, KVM_CAP_X86_DISABLE_EXITS);
+
+	return flags & KVM_X86_DISABLE_EXITS_APERFMPERF;
+}
+
+int main(int argc, char *argv[])
+{
+	uint64_t host_aperf_before, host_mperf_before;
+	int cpu = sched_getcpu();
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	int msr_fd;
+	int i;
+
+	pin_thread(cpu);
+
+	msr_fd = open_dev_msr(cpu);
+
+	/*
+	 * This test requires a non-standard VM initialization, because
+	 * KVM_ENABLE_CAP cannot be used on a VM file descriptor after
+	 * a VCPU has been created.
+	 */
+	vm = vm_create(1);
+
+	TEST_REQUIRE(kvm_can_disable_aperfmperf_exits(vm));
+
+	vm_enable_cap(vm, KVM_CAP_X86_DISABLE_EXITS,
+		      KVM_X86_DISABLE_EXITS_APERFMPERF);
+
+	vcpu = vm_vcpu_add(vm, 0, guest_code);
+
+	host_aperf_before = read_dev_msr(msr_fd, MSR_IA32_APERF);
+	host_mperf_before = read_dev_msr(msr_fd, MSR_IA32_MPERF);
+
+	for (i = 0; i < NUM_ITERATIONS; i++) {
+		uint64_t host_aperf_after, host_mperf_after;
+		uint64_t guest_aperf, guest_mperf;
+		struct ucall uc;
+
+		vcpu_run(vcpu);
+		TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
+
+		switch (get_ucall(vcpu, &uc)) {
+		case UCALL_DONE:
+			break;
+		case UCALL_ABORT:
+			REPORT_GUEST_ASSERT(uc);
+		case UCALL_SYNC:
+			guest_aperf = uc.args[0];
+			guest_mperf = uc.args[1];
+
+			host_aperf_after = read_dev_msr(msr_fd, MSR_IA32_APERF);
+			host_mperf_after = read_dev_msr(msr_fd, MSR_IA32_MPERF);
+
+			TEST_ASSERT(host_aperf_before < guest_aperf,
+				    "APERF: host_before (%lu) >= guest (%lu)",
+				    host_aperf_before, guest_aperf);
+			TEST_ASSERT(guest_aperf < host_aperf_after,
+				    "APERF: guest (%lu) >= host_after (%lu)",
+				    guest_aperf, host_aperf_after);
+			TEST_ASSERT(host_mperf_before < guest_mperf,
+				    "MPERF: host_before (%lu) >= guest (%lu)",
+				    host_mperf_before, guest_mperf);
+			TEST_ASSERT(guest_mperf < host_mperf_after,
+				    "MPERF: guest (%lu) >= host_after (%lu)",
+				    guest_mperf, host_mperf_after);
+
+			host_aperf_before = host_aperf_after;
+			host_mperf_before = host_mperf_after;
+
+			break;
+		}
+	}
+
+	TEST_ASSERT_EQ(i, NUM_ITERATIONS);
+
+	kvm_vm_free(vm);
+	close(msr_fd);
+
+	return 0;
+}
-- 
2.49.0.395.g12beb8f557-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] KVM: x86: Provide a capability to disable APERF/MPERF read intercepts
  2025-03-21 22:14 ` [PATCH v3 1/2] " Jim Mattson
@ 2025-04-28 22:58   ` Sean Christopherson
  2025-05-05 15:51     ` Jim Mattson
  0 siblings, 1 reply; 10+ messages in thread
From: Sean Christopherson @ 2025-04-28 22:58 UTC (permalink / raw)
  To: Jim Mattson; +Cc: linux-kernel, kvm, Paolo Bonzini

On Fri, Mar 21, 2025, Jim Mattson wrote:
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 2b52eb77e29c..6431cd33f06a 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -7684,6 +7684,7 @@ Valid bits in args[0] are::
>    #define KVM_X86_DISABLE_EXITS_HLT              (1 << 1)
>    #define KVM_X86_DISABLE_EXITS_PAUSE            (1 << 2)
>    #define KVM_X86_DISABLE_EXITS_CSTATE           (1 << 3)
> +  #define KVM_X86_DISABLE_EXITS_APERFMPERF       (1 << 4)

Might be pre-existing with C-states, but I think the documentation needs to call
out that userspace is responsible for enumerating APERFMPERF in guest CPUID.

And more importantly, KVM either needs to honor APERFMPERF in each vCPU's CPUID,
or the documentation needs to call out that KVM doesn't honor guest CPUID for 
APERF/MPERF MSRs.  I don't have a strong preference either way, but I'm leaning
toward having KVM honor CPUID so that if someone copy+pastes the KVM selftest   
code for the host enabling, it'll do the right thing.  On the other hand, KVM  
doesn't (and shouldn't) fully emulate the MSRs, so I'm a-ok if we ignore CPUID
entirely (but document it).

Ignoring CPUID entirely would also make it easier to document that KVM doesn't
upport loading/saving C-state or APERF/MPERF MSRs via load/store lists on VM-Enter
and VM-Exit.  E.g. we can simply say KVM doesn't emulate the MSRs in any capacity,
and that the capability disable the exit/interception, no more no less.

Heh, I guess maybe I've talked myself into having KVM ignore guest CPUID :-) 

> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index ea44c1da5a7c..5b38d5c00788 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -44,7 +44,7 @@ static inline struct page *__sme_pa_to_page(unsigned long pa)
>  #define	IOPM_SIZE PAGE_SIZE * 3
>  #define	MSRPM_SIZE PAGE_SIZE * 2
>  
> -#define MAX_DIRECT_ACCESS_MSRS	48
> +#define MAX_DIRECT_ACCESS_MSRS	50

Ugh, I really need to get the MSR interception cleanup series posted.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 4b64ab350bcd..1b3cdca806b4 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4535,6 +4535,9 @@ static u64 kvm_get_allowed_disable_exits(void)
>  {
>  	u64 r = KVM_X86_DISABLE_EXITS_PAUSE;
>  
> +	if (boot_cpu_has(X86_FEATURE_APERFMPERF))
> +		r |= KVM_X86_DISABLE_EXITS_APERFMPERF;
> +
>  	if (!mitigate_smt_rsb) {
>  		r |= KVM_X86_DISABLE_EXITS_HLT |
>  			KVM_X86_DISABLE_EXITS_CSTATE;
> @@ -6543,7 +6546,8 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>  
>  		if (!mitigate_smt_rsb && boot_cpu_has_bug(X86_BUG_SMT_RSB) &&
>  		    cpu_smt_possible() &&
> -		    (cap->args[0] & ~KVM_X86_DISABLE_EXITS_PAUSE))
> +		    (cap->args[0] & ~(KVM_X86_DISABLE_EXITS_PAUSE |
> +				      KVM_X86_DISABLE_EXITS_APERFMPERF)))
>  			pr_warn_once(SMT_RSB_MSG);
>  
>  		if (cap->args[0] & KVM_X86_DISABLE_EXITS_PAUSE)
> @@ -6554,6 +6558,8 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>  			kvm->arch.hlt_in_guest = true;
>  		if (cap->args[0] & KVM_X86_DISABLE_EXITS_CSTATE)
>  			kvm->arch.cstate_in_guest = true;
> +		if (cap->args[0] & KVM_X86_DISABLE_EXITS_APERFMPERF)
> +			kvm->arch.aperfmperf_in_guest = true;

Rather that an ever-growing stream of a booleans, what about tracing the flags
as a u64 and providing a builder macro to generate the helper?  The latter is a
bit gratuitous, but this seems like the type of boilerplate that would be
embarassingly easy to screw up without anyone noticing.

Very lightly tested...

--
From: Sean Christopherson <seanjc@google.com>
Date: Mon, 28 Apr 2025 11:35:47 -0700
Subject: [PATCH] KVM: x86: Consolidate DISABLE_EXITS_xxx handling into a
 single kvm_arch field

Replace the individual xxx_in_guest booleans with a single field to track
exits that have been disabled for a VM.  To further cut down on the amount
of boilerplate needed for each disabled exit, add a builder macro to
generate the accessor.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/kvm_host.h |  5 +----
 arch/x86/kvm/svm/svm.c          |  2 +-
 arch/x86/kvm/vmx/vmx.c          |  2 +-
 arch/x86/kvm/x86.c              | 25 ++++++++-----------------
 arch/x86/kvm/x86.h              | 28 +++++++++-------------------
 5 files changed, 20 insertions(+), 42 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 6c06f3d6e081..4b174499b29c 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1389,10 +1389,7 @@ struct kvm_arch {
 
 	gpa_t wall_clock;
 
-	bool mwait_in_guest;
-	bool hlt_in_guest;
-	bool pause_in_guest;
-	bool cstate_in_guest;
+	u64 disabled_exits;
 
 	unsigned long irq_sources_bitmap;
 	s64 kvmclock_offset;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index cc1c721ba067..0f0c06be85d6 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5053,7 +5053,7 @@ static int svm_vm_init(struct kvm *kvm)
 	}
 
 	if (!pause_filter_count || !pause_filter_thresh)
-		kvm->arch.pause_in_guest = true;
+		kvm->arch.disabled_exits |= KVM_X86_DISABLE_EXITS_PAUSE;
 
 	if (enable_apicv) {
 		int ret = avic_vm_init(kvm);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index ef2d7208dd20..109ade8fc47b 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7613,7 +7613,7 @@ int vmx_vcpu_create(struct kvm_vcpu *vcpu)
 int vmx_vm_init(struct kvm *kvm)
 {
 	if (!ple_gap)
-		kvm->arch.pause_in_guest = true;
+		kvm->arch.disabled_exits |= KVM_X86_DISABLE_EXITS_PAUSE;
 
 	if (boot_cpu_has(X86_BUG_L1TF) && enable_ept) {
 		switch (l1tf_mitigation) {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f6ce044b090a..3800d6cfecce 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6591,27 +6591,18 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 			break;
 
 		mutex_lock(&kvm->lock);
-		if (kvm->created_vcpus)
-			goto disable_exits_unlock;
-
+		if (!kvm->created_vcpus) {
 #define SMT_RSB_MSG "This processor is affected by the Cross-Thread Return Predictions vulnerability. " \
 		    "KVM_CAP_X86_DISABLE_EXITS should only be used with SMT disabled or trusted guests."
 
-		if (!mitigate_smt_rsb && boot_cpu_has_bug(X86_BUG_SMT_RSB) &&
-		    cpu_smt_possible() &&
-		    (cap->args[0] & ~KVM_X86_DISABLE_EXITS_PAUSE))
-			pr_warn_once(SMT_RSB_MSG);
+			if (!mitigate_smt_rsb && cpu_smt_possible() &&
+			    boot_cpu_has_bug(X86_BUG_SMT_RSB) &&
+			    (cap->args[0] & ~KVM_X86_DISABLE_EXITS_PAUSE))
+				pr_warn_once(SMT_RSB_MSG);
 
-		if (cap->args[0] & KVM_X86_DISABLE_EXITS_PAUSE)
-			kvm->arch.pause_in_guest = true;
-		if (cap->args[0] & KVM_X86_DISABLE_EXITS_MWAIT)
-			kvm->arch.mwait_in_guest = true;
-		if (cap->args[0] & KVM_X86_DISABLE_EXITS_HLT)
-			kvm->arch.hlt_in_guest = true;
-		if (cap->args[0] & KVM_X86_DISABLE_EXITS_CSTATE)
-			kvm->arch.cstate_in_guest = true;
-		r = 0;
-disable_exits_unlock:
+			kvm->arch.disabled_exits |= cap->args[0];
+			r = 0;
+		}
 		mutex_unlock(&kvm->lock);
 		break;
 	case KVM_CAP_MSR_PLATFORM_INFO:
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 88a9475899c8..1675017eea88 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -481,25 +481,15 @@ static inline u64 nsec_to_cycles(struct kvm_vcpu *vcpu, u64 nsec)
 	    __rem;						\
 	 })
 
-static inline bool kvm_mwait_in_guest(struct kvm *kvm)
-{
-	return kvm->arch.mwait_in_guest;
-}
-
-static inline bool kvm_hlt_in_guest(struct kvm *kvm)
-{
-	return kvm->arch.hlt_in_guest;
-}
-
-static inline bool kvm_pause_in_guest(struct kvm *kvm)
-{
-	return kvm->arch.pause_in_guest;
-}
-
-static inline bool kvm_cstate_in_guest(struct kvm *kvm)
-{
-	return kvm->arch.cstate_in_guest;
-}
+#define BUILD_DISABLED_EXITS_HELPER(lname, uname)				\
+static inline bool kvm_##lname##_in_guest(struct kvm *kvm)			\
+{										\
+	return kvm->arch.disabled_exits & KVM_X86_DISABLE_EXITS_##uname;	\
+}
+BUILD_DISABLED_EXITS_HELPER(hlt, HLT);
+BUILD_DISABLED_EXITS_HELPER(pause, PAUSE);
+BUILD_DISABLED_EXITS_HELPER(mwait, MWAIT);
+BUILD_DISABLED_EXITS_HELPER(cstate, CSTATE);
 
 static inline bool kvm_notify_vmexit_enabled(struct kvm *kvm)
 {

base-commit: 45eb29140e68ffe8e93a5471006858a018480a45
--

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/2] KVM: selftests: Test behavior of KVM_X86_DISABLE_EXITS_APERFMPERF
  2025-03-21 22:14 ` [PATCH v3 2/2] KVM: selftests: Test behavior of KVM_X86_DISABLE_EXITS_APERFMPERF Jim Mattson
@ 2025-04-29  1:26   ` Sean Christopherson
  2025-05-07 18:19     ` Jim Mattson
  0 siblings, 1 reply; 10+ messages in thread
From: Sean Christopherson @ 2025-04-29  1:26 UTC (permalink / raw)
  To: Jim Mattson; +Cc: linux-kernel, kvm, Paolo Bonzini

On Fri, Mar 21, 2025, Jim Mattson wrote:
> +#include <fcntl.h>
> +#include <limits.h>
> +#include <pthread.h>
> +#include <sched.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <asm/msr-index.h>
> +
> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "test_util.h"
> +
> +#define NUM_ITERATIONS 100
> +
> +static void pin_thread(int cpu)
> +{
> +	cpu_set_t cpuset;
> +	int rc;
> +
> +	CPU_ZERO(&cpuset);
> +	CPU_SET(cpu, &cpuset);
> +
> +	rc = pthread_setaffinity_np(pthread_self(), sizeof(cpuset), &cpuset);
> +	TEST_ASSERT(rc == 0, "%s: Can't set thread affinity", __func__);

Heh, you copy-pasted this from hardware_disable_test.c, didn't you?  :-)

Would it make sense to turn this into a generic API that takes care of the entire
sched_getcpu() => pthread_setaffinity_np()?  E.g. kvm_pin_task_to_current_cpu().
I suspect there are other (potential) tests that don't care about what CPU they
run on, so long as the test is pinned.

> +}
> +
> +static int open_dev_msr(int cpu)
> +{
> +	char path[PATH_MAX];
> +	int msr_fd;
> +
> +	snprintf(path, sizeof(path), "/dev/cpu/%d/msr", cpu);
> +	msr_fd = open(path, O_RDONLY);
> +	__TEST_REQUIRE(msr_fd >= 0, "Can't open %s for read", path);

Please use open_path_or_exit().

Hmm, and I'm planning on posting a small series to add a variant that takes an
ENOENT message, and spits out a (hopefully) helpful message for the EACCES case.
It would be nice to have this one spit out something like "Is msk.ko loaded?",
but I would say don't worry about trying to coordinate anything.  Worst case
scenario we can add a help message when the dust settles.

> +	return msr_fd;
> +}
> +
> +static uint64_t read_dev_msr(int msr_fd, uint32_t msr)
> +{
> +	uint64_t data;
> +	ssize_t rc;
> +
> +	rc = pread(msr_fd, &data, sizeof(data), msr);
> +	TEST_ASSERT(rc == sizeof(data), "Read of MSR 0x%x failed", msr);
> +
> +	return data;
> +}
> +
> +static void guest_code(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < NUM_ITERATIONS; i++) {
> +		uint64_t aperf = rdmsr(MSR_IA32_APERF);
> +		uint64_t mperf = rdmsr(MSR_IA32_MPERF);
> +
> +		GUEST_SYNC2(aperf, mperf);

Does the test generate multiple RDMSR per MSR if you do:

		GUEST_SYNC2(rdmsr(MSR_IA32_APERF), rdmsr(MSR_IA32_MPERF));

If the code generation comes out


> +	}
> +
> +	GUEST_DONE();
> +}
> +
> +static bool kvm_can_disable_aperfmperf_exits(struct kvm_vm *vm)
> +{
> +	int flags = vm_check_cap(vm, KVM_CAP_X86_DISABLE_EXITS);
> +
> +	return flags & KVM_X86_DISABLE_EXITS_APERFMPERF;
> +}

Please don't add one-off helpers like this, especially when they're the condition
for TEST_REQUIRE().  I *want* the gory details if the test is skipped, so that I
don't have to go look at the source code to figure out what's missing.

And it's literally more code.

> +
> +int main(int argc, char *argv[])
> +{
> +	uint64_t host_aperf_before, host_mperf_before;
> +	int cpu = sched_getcpu();
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +	int msr_fd;
> +	int i;
> +
> +	pin_thread(cpu);
> +
> +	msr_fd = open_dev_msr(cpu);
> +
> +	/*
> +	 * This test requires a non-standard VM initialization, because
> +	 * KVM_ENABLE_CAP cannot be used on a VM file descriptor after
> +	 * a VCPU has been created.

Hrm, we should really sort this out.  Every test that needs to enable a capability
is having to copy+paste this pattern.  I don't love the idea of expanding
__vm_create_with_one_vcpu(), but there's gotta be a solution that isn't horrible,
and anything is better than endly copy paste.

> +	 */
> +	vm = vm_create(1);
> +
> +	TEST_REQUIRE(kvm_can_disable_aperfmperf_exits(vm));

	TEST_REQUIRE(vm_check_cap(vm, KVM_CAP_X86_DISABLE_EXITS) &
		     KVM_X86_DISABLE_EXITS_APERFMPERF);
> +
> +	vm_enable_cap(vm, KVM_CAP_X86_DISABLE_EXITS,
> +		      KVM_X86_DISABLE_EXITS_APERFMPERF);
> +
> +	vcpu = vm_vcpu_add(vm, 0, guest_code);
> +
> +	host_aperf_before = read_dev_msr(msr_fd, MSR_IA32_APERF);
> +	host_mperf_before = read_dev_msr(msr_fd, MSR_IA32_MPERF);
> +
> +	for (i = 0; i < NUM_ITERATIONS; i++) {
> +		uint64_t host_aperf_after, host_mperf_after;
> +		uint64_t guest_aperf, guest_mperf;
> +		struct ucall uc;
> +
> +		vcpu_run(vcpu);
> +		TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
> +
> +		switch (get_ucall(vcpu, &uc)) {
> +		case UCALL_DONE:
> +			break;
> +		case UCALL_ABORT:
> +			REPORT_GUEST_ASSERT(uc);
> +		case UCALL_SYNC:
> +			guest_aperf = uc.args[0];
> +			guest_mperf = uc.args[1];
> +
> +			host_aperf_after = read_dev_msr(msr_fd, MSR_IA32_APERF);
> +			host_mperf_after = read_dev_msr(msr_fd, MSR_IA32_MPERF);
> +
> +			TEST_ASSERT(host_aperf_before < guest_aperf,
> +				    "APERF: host_before (%lu) >= guest (%lu)",
> +				    host_aperf_before, guest_aperf);

Honest question, is decimal really better than hex for these?

> +			TEST_ASSERT(guest_aperf < host_aperf_after,
> +				    "APERF: guest (%lu) >= host_after (%lu)",
> +				    guest_aperf, host_aperf_after);
> +			TEST_ASSERT(host_mperf_before < guest_mperf,
> +				    "MPERF: host_before (%lu) >= guest (%lu)",
> +				    host_mperf_before, guest_mperf);
> +			TEST_ASSERT(guest_mperf < host_mperf_after,
> +				    "MPERF: guest (%lu) >= host_after (%lu)",
> +				    guest_mperf, host_mperf_after);
> +
> +			host_aperf_before = host_aperf_after;
> +			host_mperf_before = host_mperf_after;
> +
> +			break;
> +		}
> +	}
> +
> +	TEST_ASSERT_EQ(i, NUM_ITERATIONS);

Why?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] KVM: x86: Provide a capability to disable APERF/MPERF read intercepts
  2025-04-28 22:58   ` Sean Christopherson
@ 2025-05-05 15:51     ` Jim Mattson
  2025-05-05 16:34       ` Sean Christopherson
  0 siblings, 1 reply; 10+ messages in thread
From: Jim Mattson @ 2025-05-05 15:51 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: linux-kernel, kvm, Paolo Bonzini

On Mon, Apr 28, 2025 at 3:58 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Fri, Mar 21, 2025, Jim Mattson wrote:
> > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> > index 2b52eb77e29c..6431cd33f06a 100644
> > --- a/Documentation/virt/kvm/api.rst
> > +++ b/Documentation/virt/kvm/api.rst
> > @@ -7684,6 +7684,7 @@ Valid bits in args[0] are::
> >    #define KVM_X86_DISABLE_EXITS_HLT              (1 << 1)
> >    #define KVM_X86_DISABLE_EXITS_PAUSE            (1 << 2)
> >    #define KVM_X86_DISABLE_EXITS_CSTATE           (1 << 3)
> > +  #define KVM_X86_DISABLE_EXITS_APERFMPERF       (1 << 4)
>
> Might be pre-existing with C-states, but I think the documentation needs to call
> out that userspace is responsible for enumerating APERFMPERF in guest CPUID.
>
> And more importantly, KVM either needs to honor APERFMPERF in each vCPU's CPUID,
> or the documentation needs to call out that KVM doesn't honor guest CPUID for
> APERF/MPERF MSRs.  I don't have a strong preference either way, but I'm leaning
> toward having KVM honor CPUID so that if someone copy+pastes the KVM selftest
> code for the host enabling, it'll do the right thing.  On the other hand, KVM
> doesn't (and shouldn't) fully emulate the MSRs, so I'm a-ok if we ignore CPUID
> entirely (but document it).
>
> Ignoring CPUID entirely would also make it easier to document that KVM doesn't
> upport loading/saving C-state or APERF/MPERF MSRs via load/store lists on VM-Enter
> and VM-Exit.  E.g. we can simply say KVM doesn't emulate the MSRs in any capacity,
> and that the capability disable the exit/interception, no more no less.
>
> Heh, I guess maybe I've talked myself into having KVM ignore guest CPUID :-)

I concur. I will add a note to that effect.

> > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > index ea44c1da5a7c..5b38d5c00788 100644
> > --- a/arch/x86/kvm/svm/svm.h
> > +++ b/arch/x86/kvm/svm/svm.h
> > @@ -44,7 +44,7 @@ static inline struct page *__sme_pa_to_page(unsigned long pa)
> >  #define      IOPM_SIZE PAGE_SIZE * 3
> >  #define      MSRPM_SIZE PAGE_SIZE * 2
> >
> > -#define MAX_DIRECT_ACCESS_MSRS       48
> > +#define MAX_DIRECT_ACCESS_MSRS       50
>
> Ugh, I really need to get the MSR interception cleanup series posted.
>
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 4b64ab350bcd..1b3cdca806b4 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -4535,6 +4535,9 @@ static u64 kvm_get_allowed_disable_exits(void)
> >  {
> >       u64 r = KVM_X86_DISABLE_EXITS_PAUSE;
> >
> > +     if (boot_cpu_has(X86_FEATURE_APERFMPERF))
> > +             r |= KVM_X86_DISABLE_EXITS_APERFMPERF;
> > +
> >       if (!mitigate_smt_rsb) {
> >               r |= KVM_X86_DISABLE_EXITS_HLT |
> >                       KVM_X86_DISABLE_EXITS_CSTATE;
> > @@ -6543,7 +6546,8 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> >
> >               if (!mitigate_smt_rsb && boot_cpu_has_bug(X86_BUG_SMT_RSB) &&
> >                   cpu_smt_possible() &&
> > -                 (cap->args[0] & ~KVM_X86_DISABLE_EXITS_PAUSE))
> > +                 (cap->args[0] & ~(KVM_X86_DISABLE_EXITS_PAUSE |
> > +                                   KVM_X86_DISABLE_EXITS_APERFMPERF)))
> >                       pr_warn_once(SMT_RSB_MSG);
> >
> >               if (cap->args[0] & KVM_X86_DISABLE_EXITS_PAUSE)
> > @@ -6554,6 +6558,8 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> >                       kvm->arch.hlt_in_guest = true;
> >               if (cap->args[0] & KVM_X86_DISABLE_EXITS_CSTATE)
> >                       kvm->arch.cstate_in_guest = true;
> > +             if (cap->args[0] & KVM_X86_DISABLE_EXITS_APERFMPERF)
> > +                     kvm->arch.aperfmperf_in_guest = true;
>
> Rather that an ever-growing stream of a booleans, what about tracing the flags
> as a u64 and providing a builder macro to generate the helper?  The latter is a
> bit gratuitous, but this seems like the type of boilerplate that would be
> embarassingly easy to screw up without anyone noticing.
>
> Very lightly tested...
>
> --
> From: Sean Christopherson <seanjc@google.com>
> Date: Mon, 28 Apr 2025 11:35:47 -0700
> Subject: [PATCH] KVM: x86: Consolidate DISABLE_EXITS_xxx handling into a
>  single kvm_arch field
>
> Replace the individual xxx_in_guest booleans with a single field to track
> exits that have been disabled for a VM.  To further cut down on the amount
> of boilerplate needed for each disabled exit, add a builder macro to
> generate the accessor.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  5 +----
>  arch/x86/kvm/svm/svm.c          |  2 +-
>  arch/x86/kvm/vmx/vmx.c          |  2 +-
>  arch/x86/kvm/x86.c              | 25 ++++++++-----------------
>  arch/x86/kvm/x86.h              | 28 +++++++++-------------------
>  5 files changed, 20 insertions(+), 42 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 6c06f3d6e081..4b174499b29c 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1389,10 +1389,7 @@ struct kvm_arch {
>
>         gpa_t wall_clock;
>
> -       bool mwait_in_guest;
> -       bool hlt_in_guest;
> -       bool pause_in_guest;
> -       bool cstate_in_guest;
> +       u64 disabled_exits;
>
>         unsigned long irq_sources_bitmap;
>         s64 kvmclock_offset;
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index cc1c721ba067..0f0c06be85d6 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -5053,7 +5053,7 @@ static int svm_vm_init(struct kvm *kvm)
>         }
>
>         if (!pause_filter_count || !pause_filter_thresh)
> -               kvm->arch.pause_in_guest = true;
> +               kvm->arch.disabled_exits |= KVM_X86_DISABLE_EXITS_PAUSE;
>
>         if (enable_apicv) {
>                 int ret = avic_vm_init(kvm);
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index ef2d7208dd20..109ade8fc47b 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7613,7 +7613,7 @@ int vmx_vcpu_create(struct kvm_vcpu *vcpu)
>  int vmx_vm_init(struct kvm *kvm)
>  {
>         if (!ple_gap)
> -               kvm->arch.pause_in_guest = true;
> +               kvm->arch.disabled_exits |= KVM_X86_DISABLE_EXITS_PAUSE;
>
>         if (boot_cpu_has(X86_BUG_L1TF) && enable_ept) {
>                 switch (l1tf_mitigation) {
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index f6ce044b090a..3800d6cfecce 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6591,27 +6591,18 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>                         break;
>
>                 mutex_lock(&kvm->lock);
> -               if (kvm->created_vcpus)
> -                       goto disable_exits_unlock;
> -
> +               if (!kvm->created_vcpus) {
>  #define SMT_RSB_MSG "This processor is affected by the Cross-Thread Return Predictions vulnerability. " \
>                     "KVM_CAP_X86_DISABLE_EXITS should only be used with SMT disabled or trusted guests."
>
> -               if (!mitigate_smt_rsb && boot_cpu_has_bug(X86_BUG_SMT_RSB) &&
> -                   cpu_smt_possible() &&
> -                   (cap->args[0] & ~KVM_X86_DISABLE_EXITS_PAUSE))
> -                       pr_warn_once(SMT_RSB_MSG);
> +                       if (!mitigate_smt_rsb && cpu_smt_possible() &&
> +                           boot_cpu_has_bug(X86_BUG_SMT_RSB) &&
> +                           (cap->args[0] & ~KVM_X86_DISABLE_EXITS_PAUSE))
> +                               pr_warn_once(SMT_RSB_MSG);
>
> -               if (cap->args[0] & KVM_X86_DISABLE_EXITS_PAUSE)
> -                       kvm->arch.pause_in_guest = true;
> -               if (cap->args[0] & KVM_X86_DISABLE_EXITS_MWAIT)
> -                       kvm->arch.mwait_in_guest = true;
> -               if (cap->args[0] & KVM_X86_DISABLE_EXITS_HLT)
> -                       kvm->arch.hlt_in_guest = true;
> -               if (cap->args[0] & KVM_X86_DISABLE_EXITS_CSTATE)
> -                       kvm->arch.cstate_in_guest = true;
> -               r = 0;
> -disable_exits_unlock:
> +                       kvm->arch.disabled_exits |= cap->args[0];
> +                       r = 0;
> +               }
>                 mutex_unlock(&kvm->lock);
>                 break;
>         case KVM_CAP_MSR_PLATFORM_INFO:
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index 88a9475899c8..1675017eea88 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -481,25 +481,15 @@ static inline u64 nsec_to_cycles(struct kvm_vcpu *vcpu, u64 nsec)
>             __rem;                                              \
>          })
>
> -static inline bool kvm_mwait_in_guest(struct kvm *kvm)
> -{
> -       return kvm->arch.mwait_in_guest;
> -}
> -
> -static inline bool kvm_hlt_in_guest(struct kvm *kvm)
> -{
> -       return kvm->arch.hlt_in_guest;
> -}
> -
> -static inline bool kvm_pause_in_guest(struct kvm *kvm)
> -{
> -       return kvm->arch.pause_in_guest;
> -}
> -
> -static inline bool kvm_cstate_in_guest(struct kvm *kvm)
> -{
> -       return kvm->arch.cstate_in_guest;
> -}
> +#define BUILD_DISABLED_EXITS_HELPER(lname, uname)                              \
> +static inline bool kvm_##lname##_in_guest(struct kvm *kvm)                     \
> +{                                                                              \
> +       return kvm->arch.disabled_exits & KVM_X86_DISABLE_EXITS_##uname;        \
> +}
> +BUILD_DISABLED_EXITS_HELPER(hlt, HLT);
> +BUILD_DISABLED_EXITS_HELPER(pause, PAUSE);
> +BUILD_DISABLED_EXITS_HELPER(mwait, MWAIT);
> +BUILD_DISABLED_EXITS_HELPER(cstate, CSTATE);

The boilerplate is bad, but that's abhorrent.

>  static inline bool kvm_notify_vmexit_enabled(struct kvm *kvm)
>  {
>
> base-commit: 45eb29140e68ffe8e93a5471006858a018480a45
> --

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] KVM: x86: Provide a capability to disable APERF/MPERF read intercepts
  2025-05-05 15:51     ` Jim Mattson
@ 2025-05-05 16:34       ` Sean Christopherson
  2025-05-07 18:09         ` Jim Mattson
  0 siblings, 1 reply; 10+ messages in thread
From: Sean Christopherson @ 2025-05-05 16:34 UTC (permalink / raw)
  To: Jim Mattson; +Cc: linux-kernel, kvm, Paolo Bonzini

On Mon, May 05, 2025, Jim Mattson wrote:
> On Mon, Apr 28, 2025 at 3:58 PM Sean Christopherson <seanjc@google.com> wrote:
> > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> > index 88a9475899c8..1675017eea88 100644
> > --- a/arch/x86/kvm/x86.h
> > +++ b/arch/x86/kvm/x86.h
> > @@ -481,25 +481,15 @@ static inline u64 nsec_to_cycles(struct kvm_vcpu *vcpu, u64 nsec)
> >             __rem;                                              \
> >          })
> >
> > -static inline bool kvm_mwait_in_guest(struct kvm *kvm)
> > -{
> > -       return kvm->arch.mwait_in_guest;
> > -}
> > -
> > -static inline bool kvm_hlt_in_guest(struct kvm *kvm)
> > -{
> > -       return kvm->arch.hlt_in_guest;
> > -}
> > -
> > -static inline bool kvm_pause_in_guest(struct kvm *kvm)
> > -{
> > -       return kvm->arch.pause_in_guest;
> > -}
> > -
> > -static inline bool kvm_cstate_in_guest(struct kvm *kvm)
> > -{
> > -       return kvm->arch.cstate_in_guest;
> > -}
> > +#define BUILD_DISABLED_EXITS_HELPER(lname, uname)                              \
> > +static inline bool kvm_##lname##_in_guest(struct kvm *kvm)                     \
> > +{                                                                              \
> > +       return kvm->arch.disabled_exits & KVM_X86_DISABLE_EXITS_##uname;        \
> > +}
> > +BUILD_DISABLED_EXITS_HELPER(hlt, HLT);
> > +BUILD_DISABLED_EXITS_HELPER(pause, PAUSE);
> > +BUILD_DISABLED_EXITS_HELPER(mwait, MWAIT);
> > +BUILD_DISABLED_EXITS_HELPER(cstate, CSTATE);
> 
> The boilerplate is bad, but that's abhorrent.

Assuming it's the macros you hate, keep the "u64 disabled_exits" change but
manually code all of the getters?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] KVM: x86: Provide a capability to disable APERF/MPERF read intercepts
  2025-05-05 16:34       ` Sean Christopherson
@ 2025-05-07 18:09         ` Jim Mattson
  0 siblings, 0 replies; 10+ messages in thread
From: Jim Mattson @ 2025-05-07 18:09 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: linux-kernel, kvm, Paolo Bonzini

On Mon, May 5, 2025 at 9:34 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, May 05, 2025, Jim Mattson wrote:
> > On Mon, Apr 28, 2025 at 3:58 PM Sean Christopherson <seanjc@google.com> wrote:
> > > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> > > index 88a9475899c8..1675017eea88 100644
> > > --- a/arch/x86/kvm/x86.h
> > > +++ b/arch/x86/kvm/x86.h
> > > @@ -481,25 +481,15 @@ static inline u64 nsec_to_cycles(struct kvm_vcpu *vcpu, u64 nsec)
> > >             __rem;                                              \
> > >          })
> > >
> > > -static inline bool kvm_mwait_in_guest(struct kvm *kvm)
> > > -{
> > > -       return kvm->arch.mwait_in_guest;
> > > -}
> > > -
> > > -static inline bool kvm_hlt_in_guest(struct kvm *kvm)
> > > -{
> > > -       return kvm->arch.hlt_in_guest;
> > > -}
> > > -
> > > -static inline bool kvm_pause_in_guest(struct kvm *kvm)
> > > -{
> > > -       return kvm->arch.pause_in_guest;
> > > -}
> > > -
> > > -static inline bool kvm_cstate_in_guest(struct kvm *kvm)
> > > -{
> > > -       return kvm->arch.cstate_in_guest;
> > > -}
> > > +#define BUILD_DISABLED_EXITS_HELPER(lname, uname)                              \
> > > +static inline bool kvm_##lname##_in_guest(struct kvm *kvm)                     \
> > > +{                                                                              \
> > > +       return kvm->arch.disabled_exits & KVM_X86_DISABLE_EXITS_##uname;        \
> > > +}
> > > +BUILD_DISABLED_EXITS_HELPER(hlt, HLT);
> > > +BUILD_DISABLED_EXITS_HELPER(pause, PAUSE);
> > > +BUILD_DISABLED_EXITS_HELPER(mwait, MWAIT);
> > > +BUILD_DISABLED_EXITS_HELPER(cstate, CSTATE);
> >
> > The boilerplate is bad, but that's abhorrent.
>
> Assuming it's the macros you hate, keep the "u64 disabled_exits" change but
> manually code all of the getters?

Sounds good. I'll try to get a V4 out this week.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/2] KVM: selftests: Test behavior of KVM_X86_DISABLE_EXITS_APERFMPERF
  2025-04-29  1:26   ` Sean Christopherson
@ 2025-05-07 18:19     ` Jim Mattson
  2025-05-07 19:54       ` Sean Christopherson
  0 siblings, 1 reply; 10+ messages in thread
From: Jim Mattson @ 2025-05-07 18:19 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: linux-kernel, kvm, Paolo Bonzini

On Mon, Apr 28, 2025 at 6:26 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Fri, Mar 21, 2025, Jim Mattson wrote:
> > +#include <fcntl.h>
> > +#include <limits.h>
> > +#include <pthread.h>
> > +#include <sched.h>
> > +#include <stdbool.h>
> > +#include <stdio.h>
> > +#include <stdint.h>
> > +#include <unistd.h>
> > +#include <asm/msr-index.h>
> > +
> > +#include "kvm_util.h"
> > +#include "processor.h"
> > +#include "test_util.h"
> > +
> > +#define NUM_ITERATIONS 100
> > +
> > +static void pin_thread(int cpu)
> > +{
> > +     cpu_set_t cpuset;
> > +     int rc;
> > +
> > +     CPU_ZERO(&cpuset);
> > +     CPU_SET(cpu, &cpuset);
> > +
> > +     rc = pthread_setaffinity_np(pthread_self(), sizeof(cpuset), &cpuset);
> > +     TEST_ASSERT(rc == 0, "%s: Can't set thread affinity", __func__);
>
> Heh, you copy-pasted this from hardware_disable_test.c, didn't you?  :-)

Probably.

> Would it make sense to turn this into a generic API that takes care of the entire
> sched_getcpu() => pthread_setaffinity_np()?  E.g. kvm_pin_task_to_current_cpu().
> I suspect there are other (potential) tests that don't care about what CPU they
> run on, so long as the test is pinned.

Sure.

> > +}
> > +
> > +static int open_dev_msr(int cpu)
> > +{
> > +     char path[PATH_MAX];
> > +     int msr_fd;
> > +
> > +     snprintf(path, sizeof(path), "/dev/cpu/%d/msr", cpu);
> > +     msr_fd = open(path, O_RDONLY);
> > +     __TEST_REQUIRE(msr_fd >= 0, "Can't open %s for read", path);
>
> Please use open_path_or_exit().

TIL.

> Hmm, and I'm planning on posting a small series to add a variant that takes an
> ENOENT message, and spits out a (hopefully) helpful message for the EACCES case.
> It would be nice to have this one spit out something like "Is msk.ko loaded?",
> but I would say don't worry about trying to coordinate anything.  Worst case
> scenario we can add a help message when the dust settles.
>
> > +     return msr_fd;
> > +}
> > +
> > +static uint64_t read_dev_msr(int msr_fd, uint32_t msr)
> > +{
> > +     uint64_t data;
> > +     ssize_t rc;
> > +
> > +     rc = pread(msr_fd, &data, sizeof(data), msr);
> > +     TEST_ASSERT(rc == sizeof(data), "Read of MSR 0x%x failed", msr);
> > +
> > +     return data;
> > +}
> > +
> > +static void guest_code(void)
> > +{
> > +     int i;
> > +
> > +     for (i = 0; i < NUM_ITERATIONS; i++) {
> > +             uint64_t aperf = rdmsr(MSR_IA32_APERF);
> > +             uint64_t mperf = rdmsr(MSR_IA32_MPERF);
> > +
> > +             GUEST_SYNC2(aperf, mperf);
>
> Does the test generate multiple RDMSR per MSR if you do:
>
>                 GUEST_SYNC2(rdmsr(MSR_IA32_APERF), rdmsr(MSR_IA32_MPERF));
>
> If the code generation comes out

I'll have to check.

>
> > +     }
> > +
> > +     GUEST_DONE();
> > +}
> > +
> > +static bool kvm_can_disable_aperfmperf_exits(struct kvm_vm *vm)
> > +{
> > +     int flags = vm_check_cap(vm, KVM_CAP_X86_DISABLE_EXITS);
> > +
> > +     return flags & KVM_X86_DISABLE_EXITS_APERFMPERF;
> > +}
>
> Please don't add one-off helpers like this, especially when they're the condition
> for TEST_REQUIRE().  I *want* the gory details if the test is skipped, so that I
> don't have to go look at the source code to figure out what's missing.
>
> And it's literally more code.

Okay.

> > +
> > +int main(int argc, char *argv[])
> > +{
> > +     uint64_t host_aperf_before, host_mperf_before;
> > +     int cpu = sched_getcpu();
> > +     struct kvm_vcpu *vcpu;
> > +     struct kvm_vm *vm;
> > +     int msr_fd;
> > +     int i;
> > +
> > +     pin_thread(cpu);
> > +
> > +     msr_fd = open_dev_msr(cpu);
> > +
> > +     /*
> > +      * This test requires a non-standard VM initialization, because
> > +      * KVM_ENABLE_CAP cannot be used on a VM file descriptor after
> > +      * a VCPU has been created.
>
> Hrm, we should really sort this out.  Every test that needs to enable a capability
> is having to copy+paste this pattern.  I don't love the idea of expanding
> __vm_create_with_one_vcpu(), but there's gotta be a solution that isn't horrible,
> and anything is better than endly copy paste.

This is all your fault, I believe. But, I'll see what I can do.

> > +      */
> > +     vm = vm_create(1);
> > +
> > +     TEST_REQUIRE(kvm_can_disable_aperfmperf_exits(vm));
>
>         TEST_REQUIRE(vm_check_cap(vm, KVM_CAP_X86_DISABLE_EXITS) &
>                      KVM_X86_DISABLE_EXITS_APERFMPERF);
> > +
> > +     vm_enable_cap(vm, KVM_CAP_X86_DISABLE_EXITS,
> > +                   KVM_X86_DISABLE_EXITS_APERFMPERF);
> > +
> > +     vcpu = vm_vcpu_add(vm, 0, guest_code);
> > +
> > +     host_aperf_before = read_dev_msr(msr_fd, MSR_IA32_APERF);
> > +     host_mperf_before = read_dev_msr(msr_fd, MSR_IA32_MPERF);
> > +
> > +     for (i = 0; i < NUM_ITERATIONS; i++) {
> > +             uint64_t host_aperf_after, host_mperf_after;
> > +             uint64_t guest_aperf, guest_mperf;
> > +             struct ucall uc;
> > +
> > +             vcpu_run(vcpu);
> > +             TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
> > +
> > +             switch (get_ucall(vcpu, &uc)) {
> > +             case UCALL_DONE:
> > +                     break;
> > +             case UCALL_ABORT:
> > +                     REPORT_GUEST_ASSERT(uc);
> > +             case UCALL_SYNC:
> > +                     guest_aperf = uc.args[0];
> > +                     guest_mperf = uc.args[1];
> > +
> > +                     host_aperf_after = read_dev_msr(msr_fd, MSR_IA32_APERF);
> > +                     host_mperf_after = read_dev_msr(msr_fd, MSR_IA32_MPERF);
> > +
> > +                     TEST_ASSERT(host_aperf_before < guest_aperf,
> > +                                 "APERF: host_before (%lu) >= guest (%lu)",
> > +                                 host_aperf_before, guest_aperf);
>
> Honest question, is decimal really better than hex for these?

They are just numbers, so any base should be fine. I guess it depends
on which base you're most comfortable with. I could add a command-line
parameter.

> > +                     TEST_ASSERT(guest_aperf < host_aperf_after,
> > +                                 "APERF: guest (%lu) >= host_after (%lu)",
> > +                                 guest_aperf, host_aperf_after);
> > +                     TEST_ASSERT(host_mperf_before < guest_mperf,
> > +                                 "MPERF: host_before (%lu) >= guest (%lu)",
> > +                                 host_mperf_before, guest_mperf);
> > +                     TEST_ASSERT(guest_mperf < host_mperf_after,
> > +                                 "MPERF: guest (%lu) >= host_after (%lu)",
> > +                                 guest_mperf, host_mperf_after);
> > +
> > +                     host_aperf_before = host_aperf_after;
> > +                     host_mperf_before = host_mperf_after;
> > +
> > +                     break;
> > +             }
> > +     }
> > +
> > +     TEST_ASSERT_EQ(i, NUM_ITERATIONS);
>
> Why?

I think this was leftover from a version where it was possible to
break out of the loop early. I'll get rid of it.

V4 this week, I hope.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/2] KVM: selftests: Test behavior of KVM_X86_DISABLE_EXITS_APERFMPERF
  2025-05-07 18:19     ` Jim Mattson
@ 2025-05-07 19:54       ` Sean Christopherson
  0 siblings, 0 replies; 10+ messages in thread
From: Sean Christopherson @ 2025-05-07 19:54 UTC (permalink / raw)
  To: Jim Mattson; +Cc: linux-kernel, kvm, Paolo Bonzini

On Wed, May 07, 2025, Jim Mattson wrote:
> On Mon, Apr 28, 2025 at 6:26 PM Sean Christopherson <seanjc@google.com> wrote:
> > > +     /*
> > > +      * This test requires a non-standard VM initialization, because
> > > +      * KVM_ENABLE_CAP cannot be used on a VM file descriptor after
> > > +      * a VCPU has been created.
> >
> > Hrm, we should really sort this out.  Every test that needs to enable a capability
> > is having to copy+paste this pattern.  I don't love the idea of expanding
> > __vm_create_with_one_vcpu(), but there's gotta be a solution that isn't horrible,
> > and anything is better than endly copy paste.
> 
> This is all your fault, I believe. But, I'll see what I can do.

Ha, that it is, both on the KVM and the selftests side.

Unless you already have something clever in hand, just keep what you have.  I poked
at this a bit today, and came to the conclusion that trying to save two lives of
"manual" effort isn't worth the explosion in APIs and complexity.  I was thinking
that the only additional input would be the capability to enable, but most usage
also needs to specify a payload, and this pattern is used in a few places where
a selftest does more than toggle a capability.

What I really want is the ability to provide a closure to all of the "create with
vCPUs" APIs, e.g.


	vm = vm_create_with_one_vcpu(&vcpu, guest_code, magic() {
		vm_enable_cap(vm, KVM_CAP_X86_DISABLE_EXITS,
			      KVM_X86_DISABLE_EXITS_APERFMPERF);
	});

But even if we managed to make something work, I'm not sure it'd be worth the
plumbing.

One thing that would make me less annoyed would be to eliminate the @vcpu_id
param, e.g.

  static inline struct kvm_vcpu *vm_vcpu_add(struct kvm_vm *vm, void *guest_code)
  {
	return __vm_vcpu_add(vm, vm->nr_vcpus++, guest_code);
  }

so that at least this pattern doesn't have '0' hardcoded everywhere.  But that's
an annoying cleanup due to __vm_vcpu_add() not being a strict superset of
vm_vcpu_add(), i.e. would require a lot of churn.

So for this series, just keep the copy+pasted pattern.

> > > +      */
> > > +     vm = vm_create(1);
> > > +
> > > +     TEST_REQUIRE(kvm_can_disable_aperfmperf_exits(vm));
> >
> >         TEST_REQUIRE(vm_check_cap(vm, KVM_CAP_X86_DISABLE_EXITS) &
> >                      KVM_X86_DISABLE_EXITS_APERFMPERF);
> > > +
> > > +     vm_enable_cap(vm, KVM_CAP_X86_DISABLE_EXITS,
> > > +                   KVM_X86_DISABLE_EXITS_APERFMPERF);
> > > +
> > > +     vcpu = vm_vcpu_add(vm, 0, guest_code);
> > > +
> > > +     host_aperf_before = read_dev_msr(msr_fd, MSR_IA32_APERF);
> > > +     host_mperf_before = read_dev_msr(msr_fd, MSR_IA32_MPERF);
> > > +
> > > +     for (i = 0; i < NUM_ITERATIONS; i++) {
> > > +             uint64_t host_aperf_after, host_mperf_after;
> > > +             uint64_t guest_aperf, guest_mperf;
> > > +             struct ucall uc;
> > > +
> > > +             vcpu_run(vcpu);
> > > +             TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
> > > +
> > > +             switch (get_ucall(vcpu, &uc)) {
> > > +             case UCALL_DONE:
> > > +                     break;
> > > +             case UCALL_ABORT:
> > > +                     REPORT_GUEST_ASSERT(uc);
> > > +             case UCALL_SYNC:
> > > +                     guest_aperf = uc.args[0];
> > > +                     guest_mperf = uc.args[1];
> > > +
> > > +                     host_aperf_after = read_dev_msr(msr_fd, MSR_IA32_APERF);
> > > +                     host_mperf_after = read_dev_msr(msr_fd, MSR_IA32_MPERF);
> > > +
> > > +                     TEST_ASSERT(host_aperf_before < guest_aperf,
> > > +                                 "APERF: host_before (%lu) >= guest (%lu)",
> > > +                                 host_aperf_before, guest_aperf);
> >
> > Honest question, is decimal really better than hex for these?
> 
> They are just numbers, so any base should be fine. I guess it depends
> on which base you're most comfortable with. I could add a command-line
> parameter.

Nah, don't bother, pick whatever you like.  I was genuinely curious if one format
or another made it easier to understand the output.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-05-07 19:54 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-21 22:14 [PATCH v3 0/2] KVM: x86: Provide a capability to disable APERF/MPERF read intercepts Jim Mattson
2025-03-21 22:14 ` [PATCH v3 1/2] " Jim Mattson
2025-04-28 22:58   ` Sean Christopherson
2025-05-05 15:51     ` Jim Mattson
2025-05-05 16:34       ` Sean Christopherson
2025-05-07 18:09         ` Jim Mattson
2025-03-21 22:14 ` [PATCH v3 2/2] KVM: selftests: Test behavior of KVM_X86_DISABLE_EXITS_APERFMPERF Jim Mattson
2025-04-29  1:26   ` Sean Christopherson
2025-05-07 18:19     ` Jim Mattson
2025-05-07 19:54       ` Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox