[PATCH 3/6] KVM: x86: selftests: Set up AMD VM in pmu_counters_test

[PATCH 3/6] KVM: x86: selftests: Set up AMD VM in pmu_counters_test

[Colton Lewis]

Branch in main() depending on if the CPU is Intel or AMD. They are
subject to vastly different requirements because the AMD PMU lacks
many properties defined by the Intel PMU including the entire CPUID
0xa function where Intel stores all the PMU properties. AMD lacks this
as well as any consistent notion of PMU versions as Intel does. Every
feature is a separate flag and they aren't the same features as Intel.

Set up a VM for testing core AMD counters and ensure proper CPUID
features are set.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 .../selftests/kvm/x86_64/pmu_counters_test.c  | 80 ++++++++++++++++---
 1 file changed, 68 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
index 0e305e43a93b..a11df073331a 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
@@ -33,7 +33,7 @@
 static uint8_t kvm_pmu_version;
 static bool kvm_has_perf_caps;
 
-static struct kvm_vm *pmu_vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
+static struct kvm_vm *intel_pmu_vm_create(struct kvm_vcpu **vcpu,
 						  void *guest_code,
 						  uint8_t pmu_version,
 						  uint64_t perf_capabilities)
@@ -303,7 +303,7 @@ static void test_arch_events(uint8_t pmu_version, uint64_t perf_capabilities,
 	if (!pmu_version)
 		return;
 
-	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_arch_events,
+	vm = intel_pmu_vm_create(&vcpu, guest_test_arch_events,
 					 pmu_version, perf_capabilities);
 
 	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH,
@@ -463,7 +463,7 @@ static void test_gp_counters(uint8_t pmu_version, uint64_t perf_capabilities,
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
 
-	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_gp_counters,
+	vm = intel_pmu_vm_create(&vcpu, guest_test_gp_counters,
 					 pmu_version, perf_capabilities);
 
 	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_NR_GP_COUNTERS,
@@ -530,7 +530,7 @@ static void test_fixed_counters(uint8_t pmu_version, uint64_t perf_capabilities,
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
 
-	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_fixed_counters,
+	vm = intel_pmu_vm_create(&vcpu, guest_test_fixed_counters,
 					 pmu_version, perf_capabilities);
 
 	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_FIXED_COUNTERS_BITMASK,
@@ -627,18 +627,74 @@ static void test_intel_counters(void)
 	}
 }
 
-int main(int argc, char *argv[])
+static uint8_t nr_core_counters(void)
 {
-	TEST_REQUIRE(kvm_is_pmu_enabled());
+	const uint8_t nr_counters = kvm_cpu_property(X86_PROPERTY_NUM_PERF_CTR_CORE);
+	const bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
+	/* The default numbers promised if the property is 0 */
+	const uint8_t amd_nr_core_ext_counters = 6;
+	const uint8_t amd_nr_core_counters = 4;
+
+	if (nr_counters != 0)
+		return nr_counters;
+
+	if (core_ext)
+		return amd_nr_core_ext_counters;
+
+	return amd_nr_core_counters;
+}
+
+static void guest_test_core_counters(void)
+{
+	GUEST_DONE();
+}
 
-	TEST_REQUIRE(host_cpu_is_intel);
-	TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
-	TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
+static void test_core_counters(void)
+{
+	uint8_t nr_counters = nr_core_counters();
+	bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
+	bool perf_mon_v2 = kvm_cpu_has(X86_FEATURE_PERF_MON_V2);
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
 
-	kvm_pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
-	kvm_has_perf_caps = kvm_cpu_has(X86_FEATURE_PDCM);
+	vm = vm_create_with_one_vcpu(&vcpu, guest_test_core_counters);
 
-	test_intel_counters();
+	/* This property may not be there in older underlying CPUs,
+	 * but it simplifies the test code for it to be set
+	 * unconditionally.
+	 */
+	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_NUM_PERF_CTR_CORE, nr_counters);
+	if (core_ext)
+		vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_CTR_EXT_CORE);
+	if (perf_mon_v2)
+		vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_MON_V2);
+
+	pr_info("Testing core counters: CoreExt = %u, PerfMonV2 = %u, NumCounters = %u\n",
+		core_ext, perf_mon_v2, nr_counters);
+	run_vcpu(vcpu);
+
+	kvm_vm_free(vm);
+}
+
+static void test_amd_counters(void)
+{
+	test_core_counters();
+}
+
+int main(int argc, char *argv[])
+{
+	TEST_REQUIRE(kvm_is_pmu_enabled());
+
+	if (host_cpu_is_intel) {
+		TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
+		TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
+		kvm_pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
+		kvm_has_perf_caps = kvm_cpu_has(X86_FEATURE_PDCM);
+		test_intel_counters();
+	} else if (host_cpu_is_amd) {
+		/* AMD CPUs don't have the same properties to look at. */
+		test_amd_counters();
+	}
 
 	return 0;
 }
-- 
2.46.0.rc2.264.g509ed76dc8-goog

[PATCH 0/6] Extend pmu_counters_test to AMD CPUs

[Colton Lewis]

(I was positive I had sent this already, but I couldn't find it on the
mailing list to reply to and ask for reviews.)

Extend pmu_counters_test to AMD CPUs.

As the AMD PMU is quite different from Intel with different events and
feature sets, this series introduces a new code path to test it,
specifically focusing on the core counters including the
PerfCtrExtCore and PerfMonV2 features. Northbridge counters and cache
counters exist, but are not as important and can be deferred to a
later series.

The first patch is a bug fix that could be submitted separately.

The series has been tested on both Intel and AMD machines, but I have
not found an AMD machine old enough to lack PerfCtrExtCore. I have
made efforts that no part of the code has any dependency on its
presence.

I am aware of similar work in this direction done by Jinrong Liang
[1]. He told me he is not working on it currently and I am not
intruding by making my own submission.

[1] https://lore.kernel.org/kvm/20231121115457.76269-1-cloudliang@tencent.com/

Colton Lewis (6):
  KVM: x86: selftests: Fix typos in macro variable use
  KVM: x86: selftests: Define AMD PMU CPUID leaves
  KVM: x86: selftests: Set up AMD VM in pmu_counters_test
  KVM: x86: selftests: Test read/write core counters
  KVM: x86: selftests: Test core events
  KVM: x86: selftests: Test PerfMonV2

 .../selftests/kvm/include/x86_64/processor.h  |   7 +
 .../selftests/kvm/x86_64/pmu_counters_test.c  | 267 ++++++++++++++++--
 2 files changed, 249 insertions(+), 25 deletions(-)

--
2.46.0.76.ge559c4bf1a-goog

[PATCH 1/6] KVM: x86: selftests: Fix typos in macro variable use

[Colton Lewis]

Without the leading underscore, these variables are referencing a
variable in the calling scope. It only worked before by accident
because all calling scopes had a variable with the right name.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 tools/testing/selftests/kvm/x86_64/pmu_counters_test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
index 698cb36989db..0e305e43a93b 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
@@ -174,7 +174,7 @@ do {										\
 
 #define GUEST_TEST_EVENT(_idx, _event, _pmc, _pmc_msr, _ctrl_msr, _value, FEP)	\
 do {										\
-	wrmsr(pmc_msr, 0);							\
+	wrmsr(_pmc_msr, 0);							\
 										\
 	if (this_cpu_has(X86_FEATURE_CLFLUSHOPT))				\
 		GUEST_MEASURE_EVENT(_ctrl_msr, _value, "clflushopt .", FEP);	\
@@ -331,9 +331,9 @@ __GUEST_ASSERT(expect_gp ? vector == GP_VECTOR : !vector,			\
 	       expect_gp ? "#GP" : "no fault", msr, vector)			\
 
 #define GUEST_ASSERT_PMC_VALUE(insn, msr, val, expected)			\
-	__GUEST_ASSERT(val == expected_val,					\
+	__GUEST_ASSERT(val == expected,					\
 		       "Expected " #insn "(0x%x) to yield 0x%lx, got 0x%lx",	\
-		       msr, expected_val, val);
+		       msr, expected, val);
 
 static void guest_test_rdpmc(uint32_t rdpmc_idx, bool expect_success,
 			     uint64_t expected_val)
-- 
2.46.0.76.ge559c4bf1a-goog

[PATCH 2/6] KVM: x86: selftests: Define AMD PMU CPUID leaves

[Colton Lewis]

This defined the CPUID calls to determine what extensions and
properties are available. AMD reference manual names listed below.

* PerfCtrExtCore (six core counters instead of four)
* PerfCtrExtNB (four counters for northbridge events)
* PerfCtrExtL2I (four counters for L2 cache events)
* PerfMonV2 (support for registers to control multiple
  counters with a single register write)
* LbrAndPmcFreeze (support for freezing last branch recorded stack on
  performance counter overflow)
* NumPerfCtrCore (number of core counters)
* NumPerfCtrNB (number of northbridge counters)

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 tools/testing/selftests/kvm/include/x86_64/processor.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index a0c1440017bb..9d87b5f8974f 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -183,6 +183,9 @@ 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_PERF_CTR_EXT_CORE	KVM_X86_CPU_FEATURE(0x80000001, 0, ECX, 23)
+#define	X86_FEATURE_PERF_CTR_EXT_NB	KVM_X86_CPU_FEATURE(0x80000001, 0, ECX, 24)
+#define	X86_FEATURE_PERF_CTR_EXT_L2I	KVM_X86_CPU_FEATURE(0x80000001, 0, ECX, 28)
 #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)
@@ -195,6 +198,8 @@ struct kvm_x86_cpu_feature {
 #define	X86_FEATURE_VGIF		KVM_X86_CPU_FEATURE(0x8000000A, 0, EDX, 16)
 #define X86_FEATURE_SEV			KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 1)
 #define X86_FEATURE_SEV_ES		KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 3)
+#define	X86_FEATURE_PERF_MON_V2		KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 0)
+#define	X86_FEATURE_PERF_LBR_PMC_FREEZE	KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 2)
 
 /*
  * KVM defined paravirt features.
@@ -281,6 +286,8 @@ struct kvm_x86_cpu_property {
 #define X86_PROPERTY_GUEST_MAX_PHY_ADDR		KVM_X86_CPU_PROPERTY(0x80000008, 0, EAX, 16, 23)
 #define X86_PROPERTY_SEV_C_BIT			KVM_X86_CPU_PROPERTY(0x8000001F, 0, EBX, 0, 5)
 #define X86_PROPERTY_PHYS_ADDR_REDUCTION	KVM_X86_CPU_PROPERTY(0x8000001F, 0, EBX, 6, 11)
+#define X86_PROPERTY_NUM_PERF_CTR_CORE		KVM_X86_CPU_PROPERTY(0x80000022, 0, EBX, 0, 3)
+#define X86_PROPERTY_NUM_PERF_CTR_NB		KVM_X86_CPU_PROPERTY(0x80000022, 0, EBX, 10, 15)
 
 #define X86_PROPERTY_MAX_CENTAUR_LEAF		KVM_X86_CPU_PROPERTY(0xC0000000, 0, EAX, 0, 31)
 
-- 
2.46.0.76.ge559c4bf1a-goog

[PATCH 3/6] KVM: x86: selftests: Set up AMD VM in pmu_counters_test

[Colton Lewis]

Branch in main() depending on if the CPU is Intel or AMD. They are
subject to vastly different requirements because the AMD PMU lacks
many properties defined by the Intel PMU including the entire CPUID
0xa function where Intel stores all the PMU properties. AMD lacks this
as well as any consistent notion of PMU versions as Intel does. Every
feature is a separate flag and they aren't the same features as Intel.

Set up a VM for testing core AMD counters and ensure proper CPUID
features are set.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 .../selftests/kvm/x86_64/pmu_counters_test.c  | 80 ++++++++++++++++---
 1 file changed, 68 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
index 0e305e43a93b..a11df073331a 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
@@ -33,7 +33,7 @@
 static uint8_t kvm_pmu_version;
 static bool kvm_has_perf_caps;
 
-static struct kvm_vm *pmu_vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
+static struct kvm_vm *intel_pmu_vm_create(struct kvm_vcpu **vcpu,
 						  void *guest_code,
 						  uint8_t pmu_version,
 						  uint64_t perf_capabilities)
@@ -303,7 +303,7 @@ static void test_arch_events(uint8_t pmu_version, uint64_t perf_capabilities,
 	if (!pmu_version)
 		return;
 
-	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_arch_events,
+	vm = intel_pmu_vm_create(&vcpu, guest_test_arch_events,
 					 pmu_version, perf_capabilities);
 
 	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH,
@@ -463,7 +463,7 @@ static void test_gp_counters(uint8_t pmu_version, uint64_t perf_capabilities,
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
 
-	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_gp_counters,
+	vm = intel_pmu_vm_create(&vcpu, guest_test_gp_counters,
 					 pmu_version, perf_capabilities);
 
 	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_NR_GP_COUNTERS,
@@ -530,7 +530,7 @@ static void test_fixed_counters(uint8_t pmu_version, uint64_t perf_capabilities,
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
 
-	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_fixed_counters,
+	vm = intel_pmu_vm_create(&vcpu, guest_test_fixed_counters,
 					 pmu_version, perf_capabilities);
 
 	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_FIXED_COUNTERS_BITMASK,
@@ -627,18 +627,74 @@ static void test_intel_counters(void)
 	}
 }
 
-int main(int argc, char *argv[])
+static uint8_t nr_core_counters(void)
 {
-	TEST_REQUIRE(kvm_is_pmu_enabled());
+	const uint8_t nr_counters = kvm_cpu_property(X86_PROPERTY_NUM_PERF_CTR_CORE);
+	const bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
+	/* The default numbers promised if the property is 0 */
+	const uint8_t amd_nr_core_ext_counters = 6;
+	const uint8_t amd_nr_core_counters = 4;
+
+	if (nr_counters != 0)
+		return nr_counters;
+
+	if (core_ext)
+		return amd_nr_core_ext_counters;
+
+	return amd_nr_core_counters;
+}
+
+static void guest_test_core_counters(void)
+{
+	GUEST_DONE();
+}
 
-	TEST_REQUIRE(host_cpu_is_intel);
-	TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
-	TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
+static void test_core_counters(void)
+{
+	uint8_t nr_counters = nr_core_counters();
+	bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
+	bool perf_mon_v2 = kvm_cpu_has(X86_FEATURE_PERF_MON_V2);
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
 
-	kvm_pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
-	kvm_has_perf_caps = kvm_cpu_has(X86_FEATURE_PDCM);
+	vm = vm_create_with_one_vcpu(&vcpu, guest_test_core_counters);
 
-	test_intel_counters();
+	/* This property may not be there in older underlying CPUs,
+	 * but it simplifies the test code for it to be set
+	 * unconditionally.
+	 */
+	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_NUM_PERF_CTR_CORE, nr_counters);
+	if (core_ext)
+		vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_CTR_EXT_CORE);
+	if (perf_mon_v2)
+		vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_MON_V2);
+
+	pr_info("Testing core counters: CoreExt = %u, PerfMonV2 = %u, NumCounters = %u\n",
+		core_ext, perf_mon_v2, nr_counters);
+	run_vcpu(vcpu);
+
+	kvm_vm_free(vm);
+}
+
+static void test_amd_counters(void)
+{
+	test_core_counters();
+}
+
+int main(int argc, char *argv[])
+{
+	TEST_REQUIRE(kvm_is_pmu_enabled());
+
+	if (host_cpu_is_intel) {
+		TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
+		TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
+		kvm_pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
+		kvm_has_perf_caps = kvm_cpu_has(X86_FEATURE_PDCM);
+		test_intel_counters();
+	} else if (host_cpu_is_amd) {
+		/* AMD CPUs don't have the same properties to look at. */
+		test_amd_counters();
+	}
 
 	return 0;
 }
-- 
2.46.0.76.ge559c4bf1a-goog

[PATCH 4/6] KVM: x86: selftests: Test read/write core counters

[Colton Lewis]

Run a basic test to ensure we can write an arbitrary value to the core
counters and read it back.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 .../selftests/kvm/x86_64/pmu_counters_test.c  | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
index a11df073331a..9620fc33d26e 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
@@ -324,6 +324,7 @@ static void test_arch_events(uint8_t pmu_version, uint64_t perf_capabilities,
  */
 #define MAX_NR_GP_COUNTERS	8
 #define MAX_NR_FIXED_COUNTERS	3
+#define MAX_NR_CORE_COUNTERS	6
 
 #define GUEST_ASSERT_PMC_MSR_ACCESS(insn, msr, expect_gp, vector)		\
 __GUEST_ASSERT(expect_gp ? vector == GP_VECTOR : !vector,			\
@@ -644,8 +645,48 @@ static uint8_t nr_core_counters(void)
 	return amd_nr_core_counters;
 }
 
+static void guest_test_rdwr_core_counters(void)
+{
+	bool core_ext = this_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
+	uint8_t nr_counters = this_cpu_property(X86_PROPERTY_NUM_PERF_CTR_CORE);
+	uint8_t i;
+	uint32_t esel_msr_base = core_ext ? MSR_F15H_PERF_CTL : MSR_K7_EVNTSEL0;
+	uint32_t cnt_msr_base = core_ext ? MSR_F15H_PERF_CTR : MSR_K7_PERFCTR0;
+	uint32_t msr_step = core_ext ? 2 : 1;
+
+	for (i = 0; i < MAX_NR_CORE_COUNTERS; i++) {
+		uint64_t test_val = 0xffff;
+		uint32_t esel_msr = esel_msr_base + msr_step * i;
+		uint32_t cnt_msr = cnt_msr_base + msr_step * i;
+		bool expect_gp = !(i < nr_counters);
+		uint8_t vector;
+		uint64_t val;
+
+		/* Test event selection register. */
+		vector = wrmsr_safe(esel_msr, test_val);
+		GUEST_ASSERT_PMC_MSR_ACCESS(WRMSR, esel_msr, expect_gp, vector);
+
+		vector = rdmsr_safe(esel_msr, &val);
+		GUEST_ASSERT_PMC_MSR_ACCESS(RDMSR, esel_msr, expect_gp, vector);
+
+		if (!expect_gp)
+			GUEST_ASSERT_PMC_VALUE(RDMSR, esel_msr, val, test_val);
+
+		/* Test counter register. */
+		vector = wrmsr_safe(cnt_msr, test_val);
+		GUEST_ASSERT_PMC_MSR_ACCESS(WRMSR, cnt_msr, expect_gp, vector);
+
+		vector = rdmsr_safe(cnt_msr, &val);
+		GUEST_ASSERT_PMC_MSR_ACCESS(RDMSR, cnt_msr, expect_gp, vector);
+
+		if (!expect_gp)
+			GUEST_ASSERT_PMC_VALUE(RDMSR, cnt_msr, val, test_val);
+	}
+}
+
 static void guest_test_core_counters(void)
 {
+	guest_test_rdwr_core_counters();
 	GUEST_DONE();
 }
 
-- 
2.46.0.76.ge559c4bf1a-goog

[PATCH 5/6] KVM: x86: selftests: Test core events

[Colton Lewis]

Test events on core counters by iterating through every combination of
events in amd_pmu_zen_events with every core counter.

For each combination, calculate the appropriate register addresses for
the event selection/control register and the counter register. The
base addresses and layout schemes change depending on whether we have
the CoreExt feature.

To do the testing, reuse GUEST_TEST_EVENT to run a standard known
workload. Decouple it from guest_assert_event_count (now
guest_assert_intel_event_count) to generalize to AMD.

Then assert the most specific detail that can be reasonably known
about the counter result. Exact count is defined and known for some
events and for other events merely asserted to be nonzero.

Note on exact counts: AMD counts one more branch than Intel for the
same workload. Though I can't confirm a reason, the only thing it
could be is the boundary of the loop instruction being counted
differently. Presumably, when the counter reaches 0 and execution
continues to the next instruction, AMD counts this as a branch and
Intel doesn't.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 .../selftests/kvm/x86_64/pmu_counters_test.c  | 87 ++++++++++++++++---
 1 file changed, 77 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
index 9620fc33d26e..fae078b444b3 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
@@ -29,6 +29,9 @@
 /* Total number of instructions retired within the measured section. */
 #define NUM_INSNS_RETIRED		(NUM_LOOPS * NUM_INSNS_PER_LOOP + NUM_EXTRA_INSNS)
 
+/* AMD counting one extra branch. Probably at loop boundary condition. */
+#define NUM_BRANCH_INSNS_RETIRED_AMD	(NUM_LOOPS+1)
+#define NUM_INSNS_RETIRED_AMD		(NUM_INSNS_RETIRED+1)
 
 static uint8_t kvm_pmu_version;
 static bool kvm_has_perf_caps;
@@ -98,7 +101,7 @@ static uint8_t guest_get_pmu_version(void)
  * Sanity check that in all cases, the event doesn't count when it's disabled,
  * and that KVM correctly emulates the write of an arbitrary value.
  */
-static void guest_assert_event_count(uint8_t idx,
+static void guest_assert_intel_event_count(uint8_t idx,
 				     struct kvm_x86_pmu_feature event,
 				     uint32_t pmc, uint32_t pmc_msr)
 {
@@ -140,6 +143,33 @@ static void guest_assert_event_count(uint8_t idx,
 	GUEST_ASSERT_EQ(_rdpmc(pmc), 0xdead);
 }
 
+static void guest_assert_amd_event_count(uint8_t evt_idx, uint8_t cnt_idx, uint32_t pmc_msr)
+{
+	uint64_t count;
+	uint64_t count_pmc;
+
+	count = rdmsr(pmc_msr);
+	count_pmc = _rdpmc(cnt_idx);
+	GUEST_ASSERT_EQ(count, count_pmc);
+
+	switch (evt_idx) {
+	case AMD_ZEN_CORE_CYCLES_INDEX:
+		GUEST_ASSERT_NE(count, 0);
+		break;
+	case AMD_ZEN_INSTRUCTIONS_INDEX:
+		GUEST_ASSERT_EQ(count, NUM_INSNS_RETIRED_AMD);
+		break;
+	case AMD_ZEN_BRANCHES_INDEX:
+		GUEST_ASSERT_EQ(count, NUM_BRANCH_INSNS_RETIRED_AMD);
+		break;
+	case AMD_ZEN_BRANCH_MISSES_INDEX:
+		GUEST_ASSERT_NE(count, 0);
+		break;
+	default:
+		break;
+	}
+
+}
 /*
  * Enable and disable the PMC in a monolithic asm blob to ensure that the
  * compiler can't insert _any_ code into the measured sequence.  Note, ECX
@@ -172,28 +202,29 @@ do {										\
 	);									\
 } while (0)
 
-#define GUEST_TEST_EVENT(_idx, _event, _pmc, _pmc_msr, _ctrl_msr, _value, FEP)	\
+#define GUEST_TEST_EVENT(_pmc_msr, _ctrl_msr, _ctrl_value, FEP)			\
 do {										\
 	wrmsr(_pmc_msr, 0);							\
 										\
 	if (this_cpu_has(X86_FEATURE_CLFLUSHOPT))				\
-		GUEST_MEASURE_EVENT(_ctrl_msr, _value, "clflushopt .", FEP);	\
+		GUEST_MEASURE_EVENT(_ctrl_msr, _ctrl_value, "clflushopt .", FEP);	\
 	else if (this_cpu_has(X86_FEATURE_CLFLUSH))				\
-		GUEST_MEASURE_EVENT(_ctrl_msr, _value, "clflush .", FEP);	\
+		GUEST_MEASURE_EVENT(_ctrl_msr, _ctrl_value, "clflush .", FEP);	\
 	else									\
-		GUEST_MEASURE_EVENT(_ctrl_msr, _value, "nop", FEP);		\
-										\
-	guest_assert_event_count(_idx, _event, _pmc, _pmc_msr);			\
+		GUEST_MEASURE_EVENT(_ctrl_msr, _ctrl_value, "nop", FEP);		\
 } while (0)
 
 static void __guest_test_arch_event(uint8_t idx, struct kvm_x86_pmu_feature event,
 				    uint32_t pmc, uint32_t pmc_msr,
 				    uint32_t ctrl_msr, uint64_t ctrl_msr_value)
 {
-	GUEST_TEST_EVENT(idx, event, pmc, pmc_msr, ctrl_msr, ctrl_msr_value, "");
+	GUEST_TEST_EVENT(pmc_msr, ctrl_msr, ctrl_msr_value, "");
+	guest_assert_intel_event_count(idx, event, pmc, pmc_msr);
 
-	if (is_forced_emulation_enabled)
-		GUEST_TEST_EVENT(idx, event, pmc, pmc_msr, ctrl_msr, ctrl_msr_value, KVM_FEP);
+	if (is_forced_emulation_enabled) {
+		GUEST_TEST_EVENT(pmc_msr, ctrl_msr, ctrl_msr_value, KVM_FEP);
+		guest_assert_intel_event_count(idx, event, pmc, pmc_msr);
+	}
 }
 
 #define X86_PMU_FEATURE_NULL						\
@@ -684,9 +715,45 @@ static void guest_test_rdwr_core_counters(void)
 	}
 }
 
+static void __guest_test_core_event(uint8_t event_idx, uint8_t counter_idx)
+{
+	/* One fortunate area of actual compatibility! This register
+	 * layout is the same for both AMD and Intel.
+	 */
+	uint64_t eventsel = ARCH_PERFMON_EVENTSEL_OS |
+		ARCH_PERFMON_EVENTSEL_ENABLE |
+		amd_pmu_zen_events[event_idx];
+	bool core_ext = this_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
+	uint64_t esel_msr_base = core_ext ? MSR_F15H_PERF_CTL : MSR_K7_EVNTSEL0;
+	uint64_t cnt_msr_base = core_ext ? MSR_F15H_PERF_CTR : MSR_K7_PERFCTR0;
+	uint64_t msr_step = core_ext ? 2 : 1;
+	uint64_t esel_msr = esel_msr_base + msr_step * counter_idx;
+	uint64_t cnt_msr = cnt_msr_base + msr_step * counter_idx;
+
+	GUEST_TEST_EVENT(cnt_msr, esel_msr, eventsel, "");
+	guest_assert_amd_event_count(event_idx, counter_idx, cnt_msr);
+
+	if (is_forced_emulation_enabled) {
+		GUEST_TEST_EVENT(cnt_msr, esel_msr, eventsel, KVM_FEP);
+		guest_assert_amd_event_count(event_idx, counter_idx, cnt_msr);
+	}
+
+}
+
+static void guest_test_core_events(void)
+{
+	uint8_t nr_counters = this_cpu_property(X86_PROPERTY_NUM_PERF_CTR_CORE);
+
+	for (uint8_t i = 0; i < NR_AMD_ZEN_EVENTS; i++) {
+		for (uint8_t j = 0; j < nr_counters; j++)
+			__guest_test_core_event(i, j);
+	}
+}
+
 static void guest_test_core_counters(void)
 {
 	guest_test_rdwr_core_counters();
+	guest_test_core_events();
 	GUEST_DONE();
 }
 
-- 
2.46.0.76.ge559c4bf1a-goog

[PATCH 6/6] KVM: x86: selftests: Test PerfMonV2

[Colton Lewis]

Test PerfMonV2, which defines global registers to enable multiple
performance counters with a single MSR write, in its own function.

If the feature is available, ensure the global control register has
the ability to start and stop the performance counters and the global
status register correctly flags an overflow by the associated counter.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 .../selftests/kvm/x86_64/pmu_counters_test.c  | 53 +++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
index fae078b444b3..a6aa37ee460a 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
@@ -750,10 +750,63 @@ static void guest_test_core_events(void)
 	}
 }
 
+static void guest_test_perf_mon_v2(void)
+{
+	uint64_t i;
+	uint64_t eventsel = ARCH_PERFMON_EVENTSEL_OS |
+		ARCH_PERFMON_EVENTSEL_ENABLE |
+		AMD_ZEN_CORE_CYCLES;
+	bool core_ext = this_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
+	uint64_t sel_msr_base = core_ext ? MSR_F15H_PERF_CTL : MSR_K7_EVNTSEL0;
+	uint64_t cnt_msr_base = core_ext ? MSR_F15H_PERF_CTR : MSR_K7_PERFCTR0;
+	uint64_t msr_step = core_ext ? 2 : 1;
+	uint8_t nr_counters = this_cpu_property(X86_PROPERTY_NUM_PERF_CTR_CORE);
+	bool perf_mon_v2 = this_cpu_has(X86_FEATURE_PERF_MON_V2);
+	uint64_t sel_msr;
+	uint64_t cnt_msr;
+
+	if (!perf_mon_v2)
+		return;
+
+	for (i = 0; i < nr_counters; i++) {
+		sel_msr = sel_msr_base + msr_step * i;
+		cnt_msr = cnt_msr_base + msr_step * i;
+
+		/* Ensure count stays 0 when global register disables counter. */
+		wrmsr(MSR_AMD64_PERF_CNTR_GLOBAL_CTL, 0);
+		wrmsr(sel_msr, eventsel);
+		wrmsr(cnt_msr, 0);
+		__asm__ __volatile__("loop ." : "+c"((int){NUM_LOOPS}));
+		GUEST_ASSERT(!_rdpmc(i));
+
+		/* Ensure counter is >0 when global register enables counter. */
+		wrmsr(MSR_AMD64_PERF_CNTR_GLOBAL_CTL, BIT_ULL(i));
+		__asm__ __volatile__("loop ." : "+c"((int){NUM_LOOPS}));
+		wrmsr(MSR_AMD64_PERF_CNTR_GLOBAL_CTL, 0);
+		GUEST_ASSERT(_rdpmc(i));
+
+		/* Ensure global status register flags a counter overflow. */
+		wrmsr(cnt_msr, -1);
+		wrmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR, 0xff);
+		wrmsr(MSR_AMD64_PERF_CNTR_GLOBAL_CTL, BIT_ULL(i));
+		__asm__ __volatile__("loop ." : "+c"((int){NUM_LOOPS}));
+		wrmsr(MSR_AMD64_PERF_CNTR_GLOBAL_CTL, 0);
+		GUEST_ASSERT(rdmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS) &
+			     BIT_ULL(i));
+
+		/* Ensure global status register flag is cleared correctly. */
+		wrmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR, BIT_ULL(i));
+		GUEST_ASSERT(!(rdmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS) &
+			     BIT_ULL(i)));
+	}
+}
+
+
 static void guest_test_core_counters(void)
 {
 	guest_test_rdwr_core_counters();
 	guest_test_core_events();
+	guest_test_perf_mon_v2();
 	GUEST_DONE();
 }
 
-- 
2.46.0.76.ge559c4bf1a-goog

Re: [PATCH 0/6] Extend pmu_counters_test to AMD CPUs

[Sean Christopherson]

On Tue, Aug 13, 2024, Colton Lewis wrote:
> (I was positive I had sent this already, but I couldn't find it on the
> mailing list to reply to and ask for reviews.)

You did[*], it's sitting in my todo folder.  Two things.

1. Err on the side of caution when potentially resending, and tag everything
RESEND.  Someone seeing a RESEND version without having seen the original version
is no big deal.  But someone seeing two copies of the same patches/emails can get
quite confusing.

2. Something is funky in your send flow.  The original posing says 0/7 in the
cover letter, but there are only 6 patches.

[*] https://lore.kernel.org/all/20240802182240.1916675-1-coltonlewis@google.com

Re: [PATCH 0/6] Extend pmu_counters_test to AMD CPUs

[Colton Lewis]

Sean Christopherson <seanjc@google.com> writes:

> On Tue, Aug 13, 2024, Colton Lewis wrote:
>> (I was positive I had sent this already, but I couldn't find it on the
>> mailing list to reply to and ask for reviews.)

> You did[*], it's sitting in my todo folder.  Two things.

> 1. Err on the side of caution when potentially resending, and tag  
> everything
> RESEND.  Someone seeing a RESEND version without having seen the original  
> version
> is no big deal.  But someone seeing two copies of the same patches/emails  
> can get
> quite confusing.

Sorry for jumping the gun. I couldn't find the original patches in my
email or on the (wrong) list and panicked. I will also tag RESENDs
appropriately in the future.

> 2. Something is funky in your send flow.  The original posing says 0/7 in  
> the
> cover letter, but there are only 6 patches.

Copy/paste error on my end. Before sending the first time I decided to
drop one of my patches because it wasn't useful. I copied from the cover
letter I had already written but forgot to change 7 to 6.

> [*]  
> https://lore.kernel.org/all/20240802182240.1916675-1-coltonlewis@google.com

Re: [PATCH 0/6] Extend pmu_counters_test to AMD CPUs

[Sean Christopherson]

On Tue, Aug 13, 2024, Colton Lewis wrote:
> Sean Christopherson <seanjc@google.com> writes:
> 
> > On Tue, Aug 13, 2024, Colton Lewis wrote:
> > > (I was positive I had sent this already, but I couldn't find it on the
> > > mailing list to reply to and ask for reviews.)
> 
> > You did[*], it's sitting in my todo folder.  Two things.
> 
> > 1. Err on the side of caution when potentially resending, and tag
> > everything
> > RESEND.  Someone seeing a RESEND version without having seen the
> > original version
> > is no big deal.  But someone seeing two copies of the same
> > patches/emails can get
> > quite confusing.
> 
> Sorry for jumping the gun. I couldn't find the original patches in my
> email or on the (wrong) list and panicked.

Ha, no worries.  FWIW, I highly recommend using lore if you can't (quickly) find
something in your own mailbox.  If it hit a tracked list, lore will have it.  And
if you use our corporate mail, the retention policy is 18 months unless you go
out of your way to tag mails to be kept, i.e. lore is more trustworthy in the
long run.

https://lore.kernel.org/all/?q=f:coltonlewis@google.com

Re: [PATCH 0/6] Extend pmu_counters_test to AMD CPUs

[Colton Lewis]

Sean Christopherson <seanjc@google.com> writes:

> On Tue, Aug 13, 2024, Colton Lewis wrote:
>> Sean Christopherson <seanjc@google.com> writes:

>> > On Tue, Aug 13, 2024, Colton Lewis wrote:
>> > > (I was positive I had sent this already, but I couldn't find it on  
>> the
>> > > mailing list to reply to and ask for reviews.)

>> > You did[*], it's sitting in my todo folder.  Two things.

>> > 1. Err on the side of caution when potentially resending, and tag
>> > everything
>> > RESEND.  Someone seeing a RESEND version without having seen the
>> > original version
>> > is no big deal.  But someone seeing two copies of the same
>> > patches/emails can get
>> > quite confusing.

>> Sorry for jumping the gun. I couldn't find the original patches in my
>> email or on the (wrong) list and panicked.

> Ha, no worries.  FWIW, I highly recommend using lore if you can't  
> (quickly) find
> something in your own mailbox.  If it hit a tracked list, lore will have  
> it.  And
> if you use our corporate mail, the retention policy is 18 months unless  
> you go
> out of your way to tag mails to be kept, i.e. lore is more trustworthy in  
> the
> long run.

> https://lore.kernel.org/all/?q=f:coltonlewis@google.com

I did. I have a bookmark for lore, but I now realize that bookmark was
only searching the kvmarm list and this series isn't going to be there
for obvious reasons. I've fixed that.

Re: [PATCH 1/6] KVM: x86: selftests: Fix typos in macro variable use

[Mingwei Zhang]

On Tue, Aug 13, 2024, Colton Lewis wrote:
> Without the leading underscore, these variables are referencing a
> variable in the calling scope. It only worked before by accident
> because all calling scopes had a variable with the right name.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>

This might need a fixes tag, right?
Fixes: cd34fd8c758e ("KVM: selftests: Test PMC virtualization with forced emulation")

no need to cc stable tree though, since this is very minor.

Reviewed-by: Mingwei Zhang <mizhang@google.com>
> ---
>  tools/testing/selftests/kvm/x86_64/pmu_counters_test.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
> index 698cb36989db..0e305e43a93b 100644
> --- a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
> @@ -174,7 +174,7 @@ do {										\
>  
>  #define GUEST_TEST_EVENT(_idx, _event, _pmc, _pmc_msr, _ctrl_msr, _value, FEP)	\
>  do {										\
> -	wrmsr(pmc_msr, 0);							\
> +	wrmsr(_pmc_msr, 0);							\
>  										\
>  	if (this_cpu_has(X86_FEATURE_CLFLUSHOPT))				\
>  		GUEST_MEASURE_EVENT(_ctrl_msr, _value, "clflushopt .", FEP);	\
> @@ -331,9 +331,9 @@ __GUEST_ASSERT(expect_gp ? vector == GP_VECTOR : !vector,			\
>  	       expect_gp ? "#GP" : "no fault", msr, vector)			\
>  
>  #define GUEST_ASSERT_PMC_VALUE(insn, msr, val, expected)			\
> -	__GUEST_ASSERT(val == expected_val,					\
> +	__GUEST_ASSERT(val == expected,					\
>  		       "Expected " #insn "(0x%x) to yield 0x%lx, got 0x%lx",	\
> -		       msr, expected_val, val);
> +		       msr, expected, val);
>  
>  static void guest_test_rdpmc(uint32_t rdpmc_idx, bool expect_success,
>  			     uint64_t expected_val)
> -- 
> 2.46.0.76.ge559c4bf1a-goog
> 

Re: [PATCH 2/6] KVM: x86: selftests: Define AMD PMU CPUID leaves

[Mingwei Zhang]

On Tue, Aug 13, 2024, Colton Lewis wrote:
> This defined the CPUID calls to determine what extensions and
> properties are available. AMD reference manual names listed below.
> 
> * PerfCtrExtCore (six core counters instead of four)
> * PerfCtrExtNB (four counters for northbridge events)
> * PerfCtrExtL2I (four counters for L2 cache events)
> * PerfMonV2 (support for registers to control multiple
>   counters with a single register write)
> * LbrAndPmcFreeze (support for freezing last branch recorded stack on
>   performance counter overflow)
> * NumPerfCtrCore (number of core counters)
> * NumPerfCtrNB (number of northbridge counters)
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/include/x86_64/processor.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index a0c1440017bb..9d87b5f8974f 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -183,6 +183,9 @@ 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_PERF_CTR_EXT_CORE	KVM_X86_CPU_FEATURE(0x80000001, 0, ECX, 23)
> +#define	X86_FEATURE_PERF_CTR_EXT_NB	KVM_X86_CPU_FEATURE(0x80000001, 0, ECX, 24)
> +#define	X86_FEATURE_PERF_CTR_EXT_L2I	KVM_X86_CPU_FEATURE(0x80000001, 0, ECX, 28)

You won't be testing Northbridge counters and L2I counters, so these two
could be optional to the patch.
>  #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)
> @@ -195,6 +198,8 @@ struct kvm_x86_cpu_feature {
>  #define	X86_FEATURE_VGIF		KVM_X86_CPU_FEATURE(0x8000000A, 0, EDX, 16)
>  #define X86_FEATURE_SEV			KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 1)
>  #define X86_FEATURE_SEV_ES		KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 3)
> +#define	X86_FEATURE_PERF_MON_V2		KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 0)

Let's use X86_FEATURE_PERFMON_V2 instead.

> +#define	X86_FEATURE_PERF_LBR_PMC_FREEZE	KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 2)

You don't use this feature, do you? If not, this can be optional for the
patch.
>  
>  /*
>   * KVM defined paravirt features.
> @@ -281,6 +286,8 @@ struct kvm_x86_cpu_property {
>  #define X86_PROPERTY_GUEST_MAX_PHY_ADDR		KVM_X86_CPU_PROPERTY(0x80000008, 0, EAX, 16, 23)
>  #define X86_PROPERTY_SEV_C_BIT			KVM_X86_CPU_PROPERTY(0x8000001F, 0, EBX, 0, 5)
>  #define X86_PROPERTY_PHYS_ADDR_REDUCTION	KVM_X86_CPU_PROPERTY(0x8000001F, 0, EBX, 6, 11)
> +#define X86_PROPERTY_NUM_PERF_CTR_CORE		KVM_X86_CPU_PROPERTY(0x80000022, 0, EBX, 0, 3)
> +#define X86_PROPERTY_NUM_PERF_CTR_NB		KVM_X86_CPU_PROPERTY(0x80000022, 0, EBX, 10, 15)
>  

ditto.
>  #define X86_PROPERTY_MAX_CENTAUR_LEAF		KVM_X86_CPU_PROPERTY(0xC0000000, 0, EAX, 0, 31)
>  
> -- 
> 2.46.0.76.ge559c4bf1a-goog
> 

Re: [PATCH 3/6] KVM: x86: selftests: Set up AMD VM in pmu_counters_test

[Mingwei Zhang]

On Tue, Aug 13, 2024, Colton Lewis wrote:
> Branch in main() depending on if the CPU is Intel or AMD. They are
> subject to vastly different requirements because the AMD PMU lacks
> many properties defined by the Intel PMU including the entire CPUID
> 0xa function where Intel stores all the PMU properties. AMD lacks this
> as well as any consistent notion of PMU versions as Intel does. Every
> feature is a separate flag and they aren't the same features as Intel.
> 
> Set up a VM for testing core AMD counters and ensure proper CPUID
> features are set.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  .../selftests/kvm/x86_64/pmu_counters_test.c  | 80 ++++++++++++++++---
>  1 file changed, 68 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
> index 0e305e43a93b..a11df073331a 100644
> --- a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
> @@ -33,7 +33,7 @@
>  static uint8_t kvm_pmu_version;
>  static bool kvm_has_perf_caps;
>  
> -static struct kvm_vm *pmu_vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
> +static struct kvm_vm *intel_pmu_vm_create(struct kvm_vcpu **vcpu,
>  						  void *guest_code,
>  						  uint8_t pmu_version,
>  						  uint64_t perf_capabilities)
> @@ -303,7 +303,7 @@ static void test_arch_events(uint8_t pmu_version, uint64_t perf_capabilities,
>  	if (!pmu_version)
>  		return;
>  
> -	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_arch_events,
> +	vm = intel_pmu_vm_create(&vcpu, guest_test_arch_events,
>  					 pmu_version, perf_capabilities);
>  
>  	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH,
> @@ -463,7 +463,7 @@ static void test_gp_counters(uint8_t pmu_version, uint64_t perf_capabilities,
>  	struct kvm_vcpu *vcpu;
>  	struct kvm_vm *vm;
>  
> -	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_gp_counters,
> +	vm = intel_pmu_vm_create(&vcpu, guest_test_gp_counters,
>  					 pmu_version, perf_capabilities);
>  
>  	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_NR_GP_COUNTERS,
> @@ -530,7 +530,7 @@ static void test_fixed_counters(uint8_t pmu_version, uint64_t perf_capabilities,
>  	struct kvm_vcpu *vcpu;
>  	struct kvm_vm *vm;
>  
> -	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_fixed_counters,
> +	vm = intel_pmu_vm_create(&vcpu, guest_test_fixed_counters,
>  					 pmu_version, perf_capabilities);
>  
>  	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_FIXED_COUNTERS_BITMASK,
> @@ -627,18 +627,74 @@ static void test_intel_counters(void)
>  	}
>  }
>  
> -int main(int argc, char *argv[])
> +static uint8_t nr_core_counters(void)
>  {
> -	TEST_REQUIRE(kvm_is_pmu_enabled());
> +	const uint8_t nr_counters = kvm_cpu_property(X86_PROPERTY_NUM_PERF_CTR_CORE);
> +	const bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
> +	/* The default numbers promised if the property is 0 */
> +	const uint8_t amd_nr_core_ext_counters = 6;
> +	const uint8_t amd_nr_core_counters = 4;
> +
> +	if (nr_counters != 0)
> +		return nr_counters;
> +
> +	if (core_ext)
> +		return amd_nr_core_ext_counters;
> +
> +	return amd_nr_core_counters;
> +}
> +
> +static void guest_test_core_counters(void)
> +{
> +	GUEST_DONE();
> +}
>  
> -	TEST_REQUIRE(host_cpu_is_intel);
> -	TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
> -	TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
> +static void test_core_counters(void)
> +{
> +	uint8_t nr_counters = nr_core_counters();
> +	bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
> +	bool perf_mon_v2 = kvm_cpu_has(X86_FEATURE_PERF_MON_V2);
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
>  
> -	kvm_pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
> -	kvm_has_perf_caps = kvm_cpu_has(X86_FEATURE_PDCM);
> +	vm = vm_create_with_one_vcpu(&vcpu, guest_test_core_counters);
>  
> -	test_intel_counters();
> +	/* This property may not be there in older underlying CPUs,
> +	 * but it simplifies the test code for it to be set
> +	 * unconditionally.
> +	 */
> +	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_NUM_PERF_CTR_CORE, nr_counters);
> +	if (core_ext)
> +		vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_CTR_EXT_CORE);
> +	if (perf_mon_v2)
> +		vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_MON_V2);

hmm, I think this might not be enough. So, when the baremetal machine
supports Perfmon v2, this code is just testing v2. But we should be able
to test anything below v2, ie., v1, v1 without core_ext. So, three
cases need to be tested here: v1 with 4 counters; v1 with core_ext (6
counters); v2.

If, the machine running this selftest does not support v2 but it does
support core extension, then we fall back to test v1 with 4 counters and
v1 with 6 counters.
> +
> +	pr_info("Testing core counters: CoreExt = %u, PerfMonV2 = %u, NumCounters = %u\n",
> +		core_ext, perf_mon_v2, nr_counters);
> +	run_vcpu(vcpu);
> +
> +	kvm_vm_free(vm);
> +}
> +
> +static void test_amd_counters(void)
> +{
> +	test_core_counters();
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	TEST_REQUIRE(kvm_is_pmu_enabled());
> +
> +	if (host_cpu_is_intel) {
> +		TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
> +		TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
> +		kvm_pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
> +		kvm_has_perf_caps = kvm_cpu_has(X86_FEATURE_PDCM);
> +		test_intel_counters();
> +	} else if (host_cpu_is_amd) {
> +		/* AMD CPUs don't have the same properties to look at. */
> +		test_amd_counters();
> +	}
>  
>  	return 0;
>  }
> -- 
> 2.46.0.76.ge559c4bf1a-goog
> 

Re: [PATCH 1/6] KVM: x86: selftests: Fix typos in macro variable use

[Colton Lewis]

Mingwei Zhang <mizhang@google.com> writes:

> On Tue, Aug 13, 2024, Colton Lewis wrote:
>> Without the leading underscore, these variables are referencing a
>> variable in the calling scope. It only worked before by accident
>> because all calling scopes had a variable with the right name.

>> Signed-off-by: Colton Lewis <coltonlewis@google.com>

> This might need a fixes tag, right?
> Fixes: cd34fd8c758e ("KVM: selftests: Test PMC virtualization with forced  
> emulation")

> no need to cc stable tree though, since this is very minor.

> Reviewed-by: Mingwei Zhang <mizhang@google.com>

Yes it does. Thanks for the catch.

>> ---
>>   tools/testing/selftests/kvm/x86_64/pmu_counters_test.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)

>> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c  
>> b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
>> index 698cb36989db..0e305e43a93b 100644
>> --- a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
>> +++ b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
>> @@ -174,7 +174,7 @@ do {										\

>>   #define GUEST_TEST_EVENT(_idx, _event, _pmc, _pmc_msr, _ctrl_msr,  
>> _value, FEP)	\
>>   do {										\
>> -	wrmsr(pmc_msr, 0);							\
>> +	wrmsr(_pmc_msr, 0);							\
>>   										\
>>   	if (this_cpu_has(X86_FEATURE_CLFLUSHOPT))				\
>>   		GUEST_MEASURE_EVENT(_ctrl_msr, _value, "clflushopt .", FEP);	\
>> @@ -331,9 +331,9 @@ __GUEST_ASSERT(expect_gp ? vector ==  
>> GP_VECTOR : !vector,			\
>>   	       expect_gp ? "#GP" : "no fault", msr, vector)			\

>>   #define GUEST_ASSERT_PMC_VALUE(insn, msr, val, expected)			\
>> -	__GUEST_ASSERT(val == expected_val,					\
>> +	__GUEST_ASSERT(val == expected,					\
>>   		       "Expected " #insn "(0x%x) to yield 0x%lx, got 0x%lx",	\
>> -		       msr, expected_val, val);
>> +		       msr, expected, val);

>>   static void guest_test_rdpmc(uint32_t rdpmc_idx, bool expect_success,
>>   			     uint64_t expected_val)
>> --
>> 2.46.0.76.ge559c4bf1a-goog

Re: [PATCH 2/6] KVM: x86: selftests: Define AMD PMU CPUID leaves

[Colton Lewis]

Hi Mingwei, thanks for reviewing!

Mingwei Zhang <mizhang@google.com> writes:

> On Tue, Aug 13, 2024, Colton Lewis wrote:
>> This defined the CPUID calls to determine what extensions and
>> properties are available. AMD reference manual names listed below.

>> * PerfCtrExtCore (six core counters instead of four)
>> * PerfCtrExtNB (four counters for northbridge events)
>> * PerfCtrExtL2I (four counters for L2 cache events)
>> * PerfMonV2 (support for registers to control multiple
>>    counters with a single register write)
>> * LbrAndPmcFreeze (support for freezing last branch recorded stack on
>>    performance counter overflow)
>> * NumPerfCtrCore (number of core counters)
>> * NumPerfCtrNB (number of northbridge counters)

>> Signed-off-by: Colton Lewis <coltonlewis@google.com>
>> ---
>>   tools/testing/selftests/kvm/include/x86_64/processor.h | 7 +++++++
>>   1 file changed, 7 insertions(+)

>> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h  
>> b/tools/testing/selftests/kvm/include/x86_64/processor.h
>> index a0c1440017bb..9d87b5f8974f 100644
>> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
>> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
>> @@ -183,6 +183,9 @@ 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_PERF_CTR_EXT_CORE	KVM_X86_CPU_FEATURE(0x80000001,  
>> 0, ECX, 23)
>> +#define	X86_FEATURE_PERF_CTR_EXT_NB	KVM_X86_CPU_FEATURE(0x80000001, 0,  
>> ECX, 24)
>> +#define	X86_FEATURE_PERF_CTR_EXT_L2I	KVM_X86_CPU_FEATURE(0x80000001, 0,  
>> ECX, 28)

> You won't be testing Northbridge counters and L2I counters, so these two
> could be optional to the patch.

That's correct. Since it was a small thing to include I thought it best
to include and save someone in the future from digging through the
reference manual again.

When you say "optional" is that an endorsement of deleting?

>>   #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)
>> @@ -195,6 +198,8 @@ struct kvm_x86_cpu_feature {
>>   #define	X86_FEATURE_VGIF		KVM_X86_CPU_FEATURE(0x8000000A, 0, EDX, 16)
>>   #define X86_FEATURE_SEV			KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 1)
>>   #define X86_FEATURE_SEV_ES		KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 3)
>> +#define	X86_FEATURE_PERF_MON_V2		KVM_X86_CPU_FEATURE(0x80000022, 0,  
>> EAX, 0)

> Let's use X86_FEATURE_PERFMON_V2 instead.

Done

>> +#define	X86_FEATURE_PERF_LBR_PMC_FREEZE	KVM_X86_CPU_FEATURE(0x80000022,  
>> 0, EAX, 2)

> You don't use this feature, do you? If not, this can be optional for the
> patch.

Correct again, included for the same reasoning above.


>>   /*
>>    * KVM defined paravirt features.
>> @@ -281,6 +286,8 @@ struct kvm_x86_cpu_property {
>>   #define X86_PROPERTY_GUEST_MAX_PHY_ADDR		 
>> KVM_X86_CPU_PROPERTY(0x80000008, 0, EAX, 16, 23)
>>   #define X86_PROPERTY_SEV_C_BIT			KVM_X86_CPU_PROPERTY(0x8000001F, 0,  
>> EBX, 0, 5)
>>   #define X86_PROPERTY_PHYS_ADDR_REDUCTION	 
>> KVM_X86_CPU_PROPERTY(0x8000001F, 0, EBX, 6, 11)
>> +#define X86_PROPERTY_NUM_PERF_CTR_CORE		 
>> KVM_X86_CPU_PROPERTY(0x80000022, 0, EBX, 0, 3)
>> +#define X86_PROPERTY_NUM_PERF_CTR_NB		KVM_X86_CPU_PROPERTY(0x80000022,  
>> 0, EBX, 10, 15)


> ditto.
>>   #define X86_PROPERTY_MAX_CENTAUR_LEAF		KVM_X86_CPU_PROPERTY(0xC0000000,  
>> 0, EAX, 0, 31)

>> --
>> 2.46.0.76.ge559c4bf1a-goog

Re: [PATCH 3/6] KVM: x86: selftests: Set up AMD VM in pmu_counters_test

[Colton Lewis]

Hi Mingwei

Mingwei Zhang <mizhang@google.com> writes:

> On Tue, Aug 13, 2024, Colton Lewis wrote:
>> Branch in main() depending on if the CPU is Intel or AMD. They are
>> subject to vastly different requirements because the AMD PMU lacks
>> many properties defined by the Intel PMU including the entire CPUID
>> 0xa function where Intel stores all the PMU properties. AMD lacks this
>> as well as any consistent notion of PMU versions as Intel does. Every
>> feature is a separate flag and they aren't the same features as Intel.

>> Set up a VM for testing core AMD counters and ensure proper CPUID
>> features are set.

>> Signed-off-by: Colton Lewis <coltonlewis@google.com>
>> ---
>>   .../selftests/kvm/x86_64/pmu_counters_test.c  | 80 ++++++++++++++++---
>>   1 file changed, 68 insertions(+), 12 deletions(-)

>> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c  
>> b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
>> index 0e305e43a93b..a11df073331a 100644
>> --- a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
>> +++ b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
>> @@ -33,7 +33,7 @@
>>   static uint8_t kvm_pmu_version;
>>   static bool kvm_has_perf_caps;

>> -static struct kvm_vm *pmu_vm_create_with_one_vcpu(struct kvm_vcpu  
>> **vcpu,
>> +static struct kvm_vm *intel_pmu_vm_create(struct kvm_vcpu **vcpu,
>>   						  void *guest_code,
>>   						  uint8_t pmu_version,
>>   						  uint64_t perf_capabilities)
>> @@ -303,7 +303,7 @@ static void test_arch_events(uint8_t pmu_version,  
>> uint64_t perf_capabilities,
>>   	if (!pmu_version)
>>   		return;

>> -	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_arch_events,
>> +	vm = intel_pmu_vm_create(&vcpu, guest_test_arch_events,
>>   					 pmu_version, perf_capabilities);

>>   	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH,
>> @@ -463,7 +463,7 @@ static void test_gp_counters(uint8_t pmu_version,  
>> uint64_t perf_capabilities,
>>   	struct kvm_vcpu *vcpu;
>>   	struct kvm_vm *vm;

>> -	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_gp_counters,
>> +	vm = intel_pmu_vm_create(&vcpu, guest_test_gp_counters,
>>   					 pmu_version, perf_capabilities);

>>   	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_NR_GP_COUNTERS,
>> @@ -530,7 +530,7 @@ static void test_fixed_counters(uint8_t pmu_version,  
>> uint64_t perf_capabilities,
>>   	struct kvm_vcpu *vcpu;
>>   	struct kvm_vm *vm;

>> -	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_fixed_counters,
>> +	vm = intel_pmu_vm_create(&vcpu, guest_test_fixed_counters,
>>   					 pmu_version, perf_capabilities);

>>   	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_FIXED_COUNTERS_BITMASK,
>> @@ -627,18 +627,74 @@ static void test_intel_counters(void)
>>   	}
>>   }

>> -int main(int argc, char *argv[])
>> +static uint8_t nr_core_counters(void)
>>   {
>> -	TEST_REQUIRE(kvm_is_pmu_enabled());
>> +	const uint8_t nr_counters =  
>> kvm_cpu_property(X86_PROPERTY_NUM_PERF_CTR_CORE);
>> +	const bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
>> +	/* The default numbers promised if the property is 0 */
>> +	const uint8_t amd_nr_core_ext_counters = 6;
>> +	const uint8_t amd_nr_core_counters = 4;
>> +
>> +	if (nr_counters != 0)
>> +		return nr_counters;
>> +
>> +	if (core_ext)
>> +		return amd_nr_core_ext_counters;
>> +
>> +	return amd_nr_core_counters;
>> +}
>> +
>> +static void guest_test_core_counters(void)
>> +{
>> +	GUEST_DONE();
>> +}

>> -	TEST_REQUIRE(host_cpu_is_intel);
>> -	TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
>> -	TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
>> +static void test_core_counters(void)
>> +{
>> +	uint8_t nr_counters = nr_core_counters();
>> +	bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
>> +	bool perf_mon_v2 = kvm_cpu_has(X86_FEATURE_PERF_MON_V2);
>> +	struct kvm_vcpu *vcpu;
>> +	struct kvm_vm *vm;

>> -	kvm_pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
>> -	kvm_has_perf_caps = kvm_cpu_has(X86_FEATURE_PDCM);
>> +	vm = vm_create_with_one_vcpu(&vcpu, guest_test_core_counters);

>> -	test_intel_counters();
>> +	/* This property may not be there in older underlying CPUs,
>> +	 * but it simplifies the test code for it to be set
>> +	 * unconditionally.
>> +	 */
>> +	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_NUM_PERF_CTR_CORE,  
>> nr_counters);
>> +	if (core_ext)
>> +		vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_CTR_EXT_CORE);
>> +	if (perf_mon_v2)
>> +		vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_MON_V2);

> hmm, I think this might not be enough. So, when the baremetal machine
> supports Perfmon v2, this code is just testing v2. But we should be able
> to test anything below v2, ie., v1, v1 without core_ext. So, three
> cases need to be tested here: v1 with 4 counters; v1 with core_ext (6
> counters); v2.

> If, the machine running this selftest does not support v2 but it does
> support core extension, then we fall back to test v1 with 4 counters and
> v1 with 6 counters.

This should cover all cases the way I wrote it. I detect the number of
counters in nr_core_counters(). That tells me if I am dealing with 4 or
6 and then I set the cpuid property based on that so I can read that
number in later code instead of duplicating the logic.

I could always inline nr_core_counters() to make this more obvious since
it is only called in this function. It was one of the first bits of code
I wrote working on this series and I assumed I would need to call it a
bunch before I decided I could just set the cpuid property after calling
it once.

>> +
>> +	pr_info("Testing core counters: CoreExt = %u, PerfMonV2 = %u,  
>> NumCounters = %u\n",
>> +		core_ext, perf_mon_v2, nr_counters);
>> +	run_vcpu(vcpu);
>> +
>> +	kvm_vm_free(vm);
>> +}
>> +
>> +static void test_amd_counters(void)
>> +{
>> +	test_core_counters();
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +	TEST_REQUIRE(kvm_is_pmu_enabled());
>> +
>> +	if (host_cpu_is_intel) {
>> +		TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
>> +		TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
>> +		kvm_pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
>> +		kvm_has_perf_caps = kvm_cpu_has(X86_FEATURE_PDCM);
>> +		test_intel_counters();
>> +	} else if (host_cpu_is_amd) {
>> +		/* AMD CPUs don't have the same properties to look at. */
>> +		test_amd_counters();
>> +	}

>>   	return 0;
>>   }
>> --
>> 2.46.0.76.ge559c4bf1a-goog

Re: [PATCH 3/6] KVM: x86: selftests: Set up AMD VM in pmu_counters_test

[Mingwei Zhang]

On Wed, Aug 28, 2024 at 3:39 PM Colton Lewis <coltonlewis@google.com> wrote:
>
> Hi Mingwei
>
> Mingwei Zhang <mizhang@google.com> writes:
>
> > On Tue, Aug 13, 2024, Colton Lewis wrote:
> >> Branch in main() depending on if the CPU is Intel or AMD. They are
> >> subject to vastly different requirements because the AMD PMU lacks
> >> many properties defined by the Intel PMU including the entire CPUID
> >> 0xa function where Intel stores all the PMU properties. AMD lacks this
> >> as well as any consistent notion of PMU versions as Intel does. Every
> >> feature is a separate flag and they aren't the same features as Intel.
>
> >> Set up a VM for testing core AMD counters and ensure proper CPUID
> >> features are set.
>
> >> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> >> ---
> >>   .../selftests/kvm/x86_64/pmu_counters_test.c  | 80 ++++++++++++++++---
> >>   1 file changed, 68 insertions(+), 12 deletions(-)
>
> >> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
> >> b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
> >> index 0e305e43a93b..a11df073331a 100644
> >> --- a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
> >> +++ b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
> >> @@ -33,7 +33,7 @@
> >>   static uint8_t kvm_pmu_version;
> >>   static bool kvm_has_perf_caps;
>
> >> -static struct kvm_vm *pmu_vm_create_with_one_vcpu(struct kvm_vcpu
> >> **vcpu,
> >> +static struct kvm_vm *intel_pmu_vm_create(struct kvm_vcpu **vcpu,
> >>                                                void *guest_code,
> >>                                                uint8_t pmu_version,
> >>                                                uint64_t perf_capabilities)
> >> @@ -303,7 +303,7 @@ static void test_arch_events(uint8_t pmu_version,
> >> uint64_t perf_capabilities,
> >>      if (!pmu_version)
> >>              return;
>
> >> -    vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_arch_events,
> >> +    vm = intel_pmu_vm_create(&vcpu, guest_test_arch_events,
> >>                                       pmu_version, perf_capabilities);
>
> >>      vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH,
> >> @@ -463,7 +463,7 @@ static void test_gp_counters(uint8_t pmu_version,
> >> uint64_t perf_capabilities,
> >>      struct kvm_vcpu *vcpu;
> >>      struct kvm_vm *vm;
>
> >> -    vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_gp_counters,
> >> +    vm = intel_pmu_vm_create(&vcpu, guest_test_gp_counters,
> >>                                       pmu_version, perf_capabilities);
>
> >>      vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_NR_GP_COUNTERS,
> >> @@ -530,7 +530,7 @@ static void test_fixed_counters(uint8_t pmu_version,
> >> uint64_t perf_capabilities,
> >>      struct kvm_vcpu *vcpu;
> >>      struct kvm_vm *vm;
>
> >> -    vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_test_fixed_counters,
> >> +    vm = intel_pmu_vm_create(&vcpu, guest_test_fixed_counters,
> >>                                       pmu_version, perf_capabilities);
>
> >>      vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_FIXED_COUNTERS_BITMASK,
> >> @@ -627,18 +627,74 @@ static void test_intel_counters(void)
> >>      }
> >>   }
>
> >> -int main(int argc, char *argv[])
> >> +static uint8_t nr_core_counters(void)
> >>   {
> >> -    TEST_REQUIRE(kvm_is_pmu_enabled());
> >> +    const uint8_t nr_counters =
> >> kvm_cpu_property(X86_PROPERTY_NUM_PERF_CTR_CORE);
> >> +    const bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
> >> +    /* The default numbers promised if the property is 0 */
> >> +    const uint8_t amd_nr_core_ext_counters = 6;
> >> +    const uint8_t amd_nr_core_counters = 4;
> >> +
> >> +    if (nr_counters != 0)
> >> +            return nr_counters;
> >> +
> >> +    if (core_ext)
> >> +            return amd_nr_core_ext_counters;
> >> +
> >> +    return amd_nr_core_counters;
> >> +}
> >> +
> >> +static void guest_test_core_counters(void)
> >> +{
> >> +    GUEST_DONE();
> >> +}
>
> >> -    TEST_REQUIRE(host_cpu_is_intel);
> >> -    TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
> >> -    TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
> >> +static void test_core_counters(void)
> >> +{
> >> +    uint8_t nr_counters = nr_core_counters();
> >> +    bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
> >> +    bool perf_mon_v2 = kvm_cpu_has(X86_FEATURE_PERF_MON_V2);
> >> +    struct kvm_vcpu *vcpu;
> >> +    struct kvm_vm *vm;
>
> >> -    kvm_pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
> >> -    kvm_has_perf_caps = kvm_cpu_has(X86_FEATURE_PDCM);
> >> +    vm = vm_create_with_one_vcpu(&vcpu, guest_test_core_counters);
>
> >> -    test_intel_counters();
> >> +    /* This property may not be there in older underlying CPUs,
> >> +     * but it simplifies the test code for it to be set
> >> +     * unconditionally.
> >> +     */
> >> +    vcpu_set_cpuid_property(vcpu, X86_PROPERTY_NUM_PERF_CTR_CORE,
> >> nr_counters);
> >> +    if (core_ext)
> >> +            vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_CTR_EXT_CORE);
> >> +    if (perf_mon_v2)
> >> +            vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_MON_V2);
>
> > hmm, I think this might not be enough. So, when the baremetal machine
> > supports Perfmon v2, this code is just testing v2. But we should be able
> > to test anything below v2, ie., v1, v1 without core_ext. So, three
> > cases need to be tested here: v1 with 4 counters; v1 with core_ext (6
> > counters); v2.
>
> > If, the machine running this selftest does not support v2 but it does
> > support core extension, then we fall back to test v1 with 4 counters and
> > v1 with 6 counters.
>
> This should cover all cases the way I wrote it. I detect the number of
> counters in nr_core_counters(). That tells me if I am dealing with 4 or
> 6 and then I set the cpuid property based on that so I can read that
> number in later code instead of duplicating the logic.

right. in the current code, you set up the counters properly according
to the hw capability. But the test can do more on a hw with perfmon
v2, right? Because it can test multiple combinations of setup for a
VM: say v1 + 4 counters and v1 + 6 counters etc. I am just following
the style of this selftest on Intel side, in which they do a similar
kind of enumeration of PMU version + PDCM capabilities. In each
configuration, it will invoke a VM and do the test.

>
> I could always inline nr_core_counters() to make this more obvious since
> it is only called in this function. It was one of the first bits of code
> I wrote working on this series and I assumed I would need to call it a
> bunch before I decided I could just set the cpuid property after calling
> it once.
>
> >> +
> >> +    pr_info("Testing core counters: CoreExt = %u, PerfMonV2 = %u,
> >> NumCounters = %u\n",
> >> +            core_ext, perf_mon_v2, nr_counters);
> >> +    run_vcpu(vcpu);
> >> +
> >> +    kvm_vm_free(vm);
> >> +}
> >> +
> >> +static void test_amd_counters(void)
> >> +{
> >> +    test_core_counters();
> >> +}
> >> +
> >> +int main(int argc, char *argv[])
> >> +{
> >> +    TEST_REQUIRE(kvm_is_pmu_enabled());
> >> +
> >> +    if (host_cpu_is_intel) {
> >> +            TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
> >> +            TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
> >> +            kvm_pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
> >> +            kvm_has_perf_caps = kvm_cpu_has(X86_FEATURE_PDCM);
> >> +            test_intel_counters();
> >> +    } else if (host_cpu_is_amd) {
> >> +            /* AMD CPUs don't have the same properties to look at. */
> >> +            test_amd_counters();
> >> +    }
>
> >>      return 0;
> >>   }
> >> --
> >> 2.46.0.76.ge559c4bf1a-goog
>

Re: [PATCH 2/6] KVM: x86: selftests: Define AMD PMU CPUID leaves

[Sean Christopherson]

On Wed, Aug 28, 2024, Colton Lewis wrote:
> Hi Mingwei, thanks for reviewing!
> 
> Mingwei Zhang <mizhang@google.com> writes:
> 
> > On Tue, Aug 13, 2024, Colton Lewis wrote:
> > > This defined the CPUID calls to determine what extensions and
> > > properties are available. AMD reference manual names listed below.
> 
> > > * PerfCtrExtCore (six core counters instead of four)
> > > * PerfCtrExtNB (four counters for northbridge events)
> > > * PerfCtrExtL2I (four counters for L2 cache events)
> > > * PerfMonV2 (support for registers to control multiple
> > >    counters with a single register write)
> > > * LbrAndPmcFreeze (support for freezing last branch recorded stack on
> > >    performance counter overflow)
> > > * NumPerfCtrCore (number of core counters)
> > > * NumPerfCtrNB (number of northbridge counters)
> 
> > > Signed-off-by: Colton Lewis <coltonlewis@google.com>
> > > ---
> > >   tools/testing/selftests/kvm/include/x86_64/processor.h | 7 +++++++
> > >   1 file changed, 7 insertions(+)
> 
> > > diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h
> > > b/tools/testing/selftests/kvm/include/x86_64/processor.h
> > > index a0c1440017bb..9d87b5f8974f 100644
> > > --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> > > +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> > > @@ -183,6 +183,9 @@ 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_PERF_CTR_EXT_CORE	KVM_X86_CPU_FEATURE(0x80000001,
> > > 0, ECX, 23)
> > > +#define	X86_FEATURE_PERF_CTR_EXT_NB	KVM_X86_CPU_FEATURE(0x80000001,
> > > 0, ECX, 24)
> > > +#define	X86_FEATURE_PERF_CTR_EXT_L2I	KVM_X86_CPU_FEATURE(0x80000001,
> > > 0, ECX, 28)
> 
> > You won't be testing Northbridge counters and L2I counters, so these two
> > could be optional to the patch.
> 
> That's correct. Since it was a small thing to include I thought it best
> to include and save someone in the future from digging through the
> reference manual again.

+1.  They're defines, i.e. have no meaningful cost.  And anything that *might*
save me from having to find CPUID entries in the SDM/APM is worth adding.

Re: [PATCH 3/6] KVM: x86: selftests: Set up AMD VM in pmu_counters_test

[Sean Christopherson]

On Wed, Aug 28, 2024, Mingwei Zhang wrote:
> > >> +static void test_core_counters(void)
> > >> +{
> > >> +    uint8_t nr_counters = nr_core_counters();
> > >> +    bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
> > >> +    bool perf_mon_v2 = kvm_cpu_has(X86_FEATURE_PERF_MON_V2);
> > >> +    struct kvm_vcpu *vcpu;
> > >> +    struct kvm_vm *vm;
> >
> > >> -    kvm_pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
> > >> -    kvm_has_perf_caps = kvm_cpu_has(X86_FEATURE_PDCM);
> > >> +    vm = vm_create_with_one_vcpu(&vcpu, guest_test_core_counters);
> >
> > >> -    test_intel_counters();
> > >> +    /* This property may not be there in older underlying CPUs,
> > >> +     * but it simplifies the test code for it to be set
> > >> +     * unconditionally.

But then the test isn't verifying that KVM is honoring the architecture.  I.e.
backdooring information to the guest risks getting false passes because KVM
incorrectly peeks at the same information, which shouldn't exist.

> > >> +     */

	/*
	 * Multi-line function comments should start on the line after the
	 * opening slash-asterisk, like so.
	 */

> > >> +    vcpu_set_cpuid_property(vcpu, X86_PROPERTY_NUM_PERF_CTR_CORE,
> > >> nr_counters);
> > >> +    if (core_ext)
> > >> +            vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_CTR_EXT_CORE);
> > >> +    if (perf_mon_v2)
> > >> +            vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_MON_V2);
> >
> > > hmm, I think this might not be enough. So, when the baremetal machine
> > > supports Perfmon v2, this code is just testing v2. But we should be able
> > > to test anything below v2, ie., v1, v1 without core_ext. So, three
> > > cases need to be tested here: v1 with 4 counters; v1 with core_ext (6
> > > counters); v2.
> >
> > > If, the machine running this selftest does not support v2 but it does
> > > support core extension, then we fall back to test v1 with 4 counters and
> > > v1 with 6 counters.
> >
> > This should cover all cases the way I wrote it. I detect the number of
> > counters in nr_core_counters(). That tells me if I am dealing with 4 or
> > 6 and then I set the cpuid property based on that so I can read that
> > number in later code instead of duplicating the logic.
> 
> right. in the current code, you set up the counters properly according
> to the hw capability. But the test can do more on a hw with perfmon
> v2, right? Because it can test multiple combinations of setup for a
> VM: say v1 + 4 counters and v1 + 6 counters etc. I am just following
> the style of this selftest on Intel side, in which they do a similar
> kind of enumeration of PMU version + PDCM capabilities. In each
> configuration, it will invoke a VM and do the test.

Ya.  This is similar my comments on setting NUM_PER_CTR_CORE when the field
shouldn't exist.  One of the main goals of this test is to verify the KVM honors
the architecture based on userspace's defined virtual CPU model, i.e. guest
CPUID.  That means testing all (or at least, within reason) possible combinations
that can feasibly be supported by KVM given the underlying hardware.

As written, this essentially just tests the maximal configuration that can be
exposed to a guest, which isn't _that_ interesting because KVM tends to get plenty
of coverage for such setups, e.g. by running "real" VMs.

Re: [PATCH 3/6] KVM: x86: selftests: Set up AMD VM in pmu_counters_test

[Colton Lewis]

Sean Christopherson <seanjc@google.com> writes:

> On Wed, Aug 28, 2024, Mingwei Zhang wrote:
>> > >> +static void test_core_counters(void)
>> > >> +{
>> > >> +    uint8_t nr_counters = nr_core_counters();
>> > >> +    bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
>> > >> +    bool perf_mon_v2 = kvm_cpu_has(X86_FEATURE_PERF_MON_V2);
>> > >> +    struct kvm_vcpu *vcpu;
>> > >> +    struct kvm_vm *vm;
>> >
>> > >> -    kvm_pmu_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION);
>> > >> -    kvm_has_perf_caps = kvm_cpu_has(X86_FEATURE_PDCM);
>> > >> +    vm = vm_create_with_one_vcpu(&vcpu, guest_test_core_counters);
>> >
>> > >> -    test_intel_counters();
>> > >> +    /* This property may not be there in older underlying CPUs,
>> > >> +     * but it simplifies the test code for it to be set
>> > >> +     * unconditionally.

> But then the test isn't verifying that KVM is honoring the architecture.   
> I.e.
> backdooring information to the guest risks getting false passes because  
> KVM
> incorrectly peeks at the same information, which shouldn't exist.

>> > >> +     */

> 	/*
> 	 * Multi-line function comments should start on the line after the
> 	 * opening slash-asterisk, like so.
> 	 */

>> > >> +    vcpu_set_cpuid_property(vcpu, X86_PROPERTY_NUM_PERF_CTR_CORE,
>> > >> nr_counters);
>> > >> +    if (core_ext)
>> > >> +            vcpu_set_cpuid_feature(vcpu,  
>> X86_FEATURE_PERF_CTR_EXT_CORE);
>> > >> +    if (perf_mon_v2)
>> > >> +            vcpu_set_cpuid_feature(vcpu, X86_FEATURE_PERF_MON_V2);
>> >
>> > > hmm, I think this might not be enough. So, when the baremetal machine
>> > > supports Perfmon v2, this code is just testing v2. But we should be  
>> able
>> > > to test anything below v2, ie., v1, v1 without core_ext. So, three
>> > > cases need to be tested here: v1 with 4 counters; v1 with core_ext (6
>> > > counters); v2.
>> >
>> > > If, the machine running this selftest does not support v2 but it does
>> > > support core extension, then we fall back to test v1 with 4 counters  
>> and
>> > > v1 with 6 counters.
>> >
>> > This should cover all cases the way I wrote it. I detect the number of
>> > counters in nr_core_counters(). That tells me if I am dealing with 4 or
>> > 6 and then I set the cpuid property based on that so I can read that
>> > number in later code instead of duplicating the logic.

>> right. in the current code, you set up the counters properly according
>> to the hw capability. But the test can do more on a hw with perfmon
>> v2, right? Because it can test multiple combinations of setup for a
>> VM: say v1 + 4 counters and v1 + 6 counters etc. I am just following
>> the style of this selftest on Intel side, in which they do a similar
>> kind of enumeration of PMU version + PDCM capabilities. In each
>> configuration, it will invoke a VM and do the test.

> Ya.  This is similar my comments on setting NUM_PER_CTR_CORE when the  
> field
> shouldn't exist.  One of the main goals of this test is to verify the KVM  
> honors
> the architecture based on userspace's defined virtual CPU model, i.e.  
> guest
> CPUID.  That means testing all (or at least, within reason) possible  
> combinations
> that can feasibly be supported by KVM given the underlying hardware.

> As written, this essentially just tests the maximal configuration that  
> can be
> exposed to a guest, which isn't _that_ interesting because KVM tends to  
> get plenty
> of coverage for such setups, e.g. by running "real" VMs.

Ok. I get what you and Mingwei are saying. I can test those combinations.