[PATCH v3 00/11] KVM: selftests: Test the consistency of the PMU's CPUID and its features

[PATCH v3 00/11] KVM: selftests: Test the consistency of the PMU's CPUID and its features

[Jinrong Liang]

Hi,

The KVM selftests show advantages over KUT in terms of finding defects through
flexible and varied guest settings from the KVM user space.

This patchset tests whether the Intel vPMU works properly with different Intel
CPUID.0xA configurations. It also provides test scaffolding and a sufficient
number of PMU test cases to subsequently offer adequate code coverage of AMD
vPMU or Intel complex features, such as LBR or PEBS, in selftests.

All patches have been tested on both Intel and AMD machines, with one exception
patch 11 "KVM: selftests: Test AMD Guest PerfMonV2" has not been tested on my
AMD machine, as does not support PerfMonV2.

Any feedback or suggestions are greatly appreciated.

Sincerely,
Jinrong Liang

Please note that following patch should be applied before this patch series:

https://lore.kernel.org/kvm/20230810090945.16053-2-cloudliang@tencent.com/

Changelog:

v3:
- Rebased to 74c2185c5b74(tag: kvm-x86-next-2023.08.02)  
- Add a new patch to test AMD PMU legacy four performance counters.
- Add a new patch to test AMD Guest PerfMonV2.
- Refactor code to simplify logic and improve readability.
- Use TEST_ASSERT_EQ() instead of ASSERT_EQ() when checking return values.
- Add vcpu_set_cpuid_property() helper for setting properties. (Sean)
- Add arch_event_is_supported() helper to check if an event is supported. (Sean)
- Add fixed_counter_is_supported() helper to check if a fixed counter is supported. (Sean)
- Drop macros that hides important details. (Sean)
- Use enumerations to avoid performance events magic numbers. (Sean)
- TEST_FAIL() instead of TEST_ASSERT() in run_vcpu() wrapper. (Sean)
- Update variable names for better readability and consistency. (Sean)
- Rename functions to better reflect their purpose. (Sean)
- Improve comments for better clarity and understanding of the code. (Sean, Jim)

v2:
https://lore.kernel.org/kvm/20230530134248.23998-1-cloudliang@tencent.com/T/

Jinrong Liang (11):
  KVM: selftests: Add vcpu_set_cpuid_property() to set properties
  KVM: selftests: Add pmu.h for PMU events and common masks
  KVM: selftests: Test Intel PMU architectural events on gp counters
  KVM: selftests: Test Intel PMU architectural events on fixed counters
  KVM: selftests: Test consistency of CPUID with num of gp counters
  KVM: selftests: Test consistency of CPUID with num of fixed counters
  KVM: selftests: Test Intel supported fixed counters bit mask
  KVM: selftests: Test consistency of PMU MSRs with Intel PMU version
  KVM: selftests: Add x86 feature and properties for AMD PMU in
    processor.h
  KVM: selftests: Test AMD PMU events on legacy four performance
    counters
  KVM: selftests: Test AMD Guest PerfMonV2

 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/include/x86_64/pmu.h        | 124 +++++
 .../selftests/kvm/include/x86_64/processor.h  |  11 +
 .../selftests/kvm/lib/x86_64/processor.c      |  14 +
 .../kvm/x86_64/pmu_basic_functionality_test.c | 505 ++++++++++++++++++
 5 files changed, 655 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/include/x86_64/pmu.h
 create mode 100644 tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c


base-commit: 74c2185c5b74fd0ae91133ad5afe8684f6a02b91
prerequisite-patch-id: 8718ffb8c05e453db9aae9896787cb6650d3cd52
-- 
2.39.3

[PATCH v3 01/11] KVM: selftests: Add vcpu_set_cpuid_property() to set properties

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

Add vcpu_set_cpuid_property() helper function for setting properties,
which simplifies the process of setting CPUID properties for vCPUs.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 .../selftests/kvm/include/x86_64/processor.h       |  4 ++++
 tools/testing/selftests/kvm/lib/x86_64/processor.c | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index 4fd042112526..6b146e1c6736 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -973,6 +973,10 @@ static inline void vcpu_set_cpuid(struct kvm_vcpu *vcpu)
 
 void vcpu_set_cpuid_maxphyaddr(struct kvm_vcpu *vcpu, uint8_t maxphyaddr);
 
+void vcpu_set_cpuid_property(struct kvm_vcpu *vcpu,
+			     struct kvm_x86_cpu_property property,
+			     uint32_t value);
+
 void vcpu_clear_cpuid_entry(struct kvm_vcpu *vcpu, uint32_t function);
 void vcpu_set_or_clear_cpuid_feature(struct kvm_vcpu *vcpu,
 				     struct kvm_x86_cpu_feature feature,
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index d8288374078e..0e029be66783 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -760,6 +760,20 @@ void vcpu_set_cpuid_maxphyaddr(struct kvm_vcpu *vcpu, uint8_t maxphyaddr)
 	vcpu_set_cpuid(vcpu);
 }
 
+void vcpu_set_cpuid_property(struct kvm_vcpu *vcpu,
+			     struct kvm_x86_cpu_property property,
+			     uint32_t value)
+{
+	struct kvm_cpuid_entry2 *entry;
+
+	entry = __vcpu_get_cpuid_entry(vcpu, property.function, property.index);
+
+	(&entry->eax)[property.reg] &= ~GENMASK(property.hi_bit, property.lo_bit);
+	(&entry->eax)[property.reg] |= value << (property.lo_bit);
+
+	vcpu_set_cpuid(vcpu);
+}
+
 void vcpu_clear_cpuid_entry(struct kvm_vcpu *vcpu, uint32_t function)
 {
 	struct kvm_cpuid_entry2 *entry = vcpu_get_cpuid_entry(vcpu, function);
-- 
2.39.3

[PATCH v3 02/11] KVM: selftests: Add pmu.h for PMU events and common masks

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

By defining the PMU performance events and masks relevant for x86 in
the new pmu.h header, it becomes easier to reference them, minimizing
potential errors in code that handles these values.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 .../selftests/kvm/include/x86_64/pmu.h        | 124 ++++++++++++++++++
 1 file changed, 124 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/include/x86_64/pmu.h

diff --git a/tools/testing/selftests/kvm/include/x86_64/pmu.h b/tools/testing/selftests/kvm/include/x86_64/pmu.h
new file mode 100644
index 000000000000..eb60b2065fac
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/x86_64/pmu.h
@@ -0,0 +1,124 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * tools/testing/selftests/kvm/include/x86_64/pmu.h
+ *
+ * Copyright (C) 2023, Tencent, Inc.
+ */
+#ifndef SELFTEST_KVM_PMU_H
+#define SELFTEST_KVM_PMU_H
+
+#include "processor.h"
+
+#define GP_COUNTER_NR_OFS_BIT			8
+#define EVENT_LENGTH_OFS_BIT			24
+#define INTEL_PMC_IDX_FIXED			32
+
+#define AMD64_NR_COUNTERS			4
+#define AMD64_NR_COUNTERS_CORE			6
+
+#define PMU_CAP_FW_WRITES			BIT_ULL(13)
+#define RDPMC_FIXED_BASE			BIT_ULL(30)
+
+#define ARCH_PERFMON_EVENTSEL_EVENT		GENMASK_ULL(7, 0)
+#define ARCH_PERFMON_EVENTSEL_UMASK		GENMASK_ULL(15, 8)
+#define ARCH_PERFMON_EVENTSEL_USR		BIT_ULL(16)
+#define ARCH_PERFMON_EVENTSEL_OS		BIT_ULL(17)
+
+#define ARCH_PERFMON_EVENTSEL_EDGE		BIT_ULL(18)
+#define ARCH_PERFMON_EVENTSEL_PIN_CONTROL	BIT_ULL(19)
+#define ARCH_PERFMON_EVENTSEL_INT		BIT_ULL(20)
+#define ARCH_PERFMON_EVENTSEL_ANY		BIT_ULL(21)
+#define ARCH_PERFMON_EVENTSEL_ENABLE		BIT_ULL(22)
+#define ARCH_PERFMON_EVENTSEL_INV		BIT_ULL(23)
+#define ARCH_PERFMON_EVENTSEL_CMASK		GENMASK_ULL(31, 24)
+
+#define PMU_VERSION_MASK			GENMASK_ULL(7, 0)
+#define EVENT_LENGTH_MASK			GENMASK_ULL(31, EVENT_LENGTH_OFS_BIT)
+#define GP_COUNTER_NR_MASK			GENMASK_ULL(15, GP_COUNTER_NR_OFS_BIT)
+#define FIXED_COUNTER_NR_MASK			GENMASK_ULL(4, 0)
+
+/* Definitions for Architectural Performance Events */
+#define ARCH_EVENT(select, umask) (((select) & 0xff) | ((umask) & 0xff) << 8)
+
+enum intel_pmu_architectural_events {
+	/*
+	 * The order of the architectural events matters as support for each
+	 * event is enumerated via CPUID using the index of the event.
+	 */
+	INTEL_ARCH_CPU_CYCLES,
+	INTEL_ARCH_INSTRUCTIONS_RETIRED,
+	INTEL_ARCH_REFERENCE_CYCLES,
+	INTEL_ARCH_LLC_REFERENCES,
+	INTEL_ARCH_LLC_MISSES,
+	INTEL_ARCH_BRANCHES_RETIRED,
+	INTEL_ARCH_BRANCHES_MISPREDICTED,
+
+	NR_REAL_INTEL_ARCH_EVENTS,
+
+	/*
+	 * Pseudo-architectural event used to implement IA32_FIXED_CTR2, a.k.a.
+	 * TSC reference cycles. The architectural reference cycles event may
+	 * or may not actually use the TSC as the reference, e.g. might use the
+	 * core crystal clock or the bus clock (yeah, "architectural").
+	 */
+	PSEUDO_ARCH_REFERENCE_CYCLES = NR_REAL_INTEL_ARCH_EVENTS,
+	NR_INTEL_ARCH_EVENTS,
+};
+
+static const uint64_t intel_arch_events[] = {
+	[INTEL_ARCH_CPU_CYCLES]			= ARCH_EVENT(0x3c, 0x0),
+	[INTEL_ARCH_INSTRUCTIONS_RETIRED]	= ARCH_EVENT(0xc0, 0x0),
+	[INTEL_ARCH_REFERENCE_CYCLES]		= ARCH_EVENT(0x3c, 0x1),
+	[INTEL_ARCH_LLC_REFERENCES]		= ARCH_EVENT(0x2e, 0x4f),
+	[INTEL_ARCH_LLC_MISSES]			= ARCH_EVENT(0x2e, 0x41),
+	[INTEL_ARCH_BRANCHES_RETIRED]		= ARCH_EVENT(0xc4, 0x0),
+	[INTEL_ARCH_BRANCHES_MISPREDICTED]	= ARCH_EVENT(0xc5, 0x0),
+	[PSEUDO_ARCH_REFERENCE_CYCLES]		= ARCH_EVENT(0xa4, 0x1),
+};
+
+/* mapping between fixed pmc index and intel_arch_events array */
+static const int fixed_pmc_events[] = {
+	[0] = INTEL_ARCH_INSTRUCTIONS_RETIRED,
+	[1] = INTEL_ARCH_CPU_CYCLES,
+	[2] = PSEUDO_ARCH_REFERENCE_CYCLES,
+};
+
+enum amd_pmu_k7_events {
+	AMD_ZEN_CORE_CYCLES,
+	AMD_ZEN_INSTRUCTIONS,
+	AMD_ZEN_BRANCHES,
+	AMD_ZEN_BRANCH_MISSES,
+};
+
+static const uint64_t amd_arch_events[] = {
+	[AMD_ZEN_CORE_CYCLES]			= ARCH_EVENT(0x76, 0x00),
+	[AMD_ZEN_INSTRUCTIONS]			= ARCH_EVENT(0xc0, 0x00),
+	[AMD_ZEN_BRANCHES]			= ARCH_EVENT(0xc2, 0x00),
+	[AMD_ZEN_BRANCH_MISSES]			= ARCH_EVENT(0xc3, 0x00),
+};
+
+static inline bool arch_event_is_supported(struct kvm_vcpu *vcpu,
+					   uint8_t arch_event)
+{
+	struct kvm_cpuid_entry2 *entry;
+
+	entry = vcpu_get_cpuid_entry(vcpu, 0xa);
+
+	return !(entry->ebx & BIT_ULL(arch_event)) &&
+		(kvm_cpuid_property(vcpu->cpuid,
+		 X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH) > arch_event);
+}
+
+static inline bool fixed_counter_is_supported(struct kvm_vcpu *vcpu,
+					      uint8_t fixed_counter_idx)
+{
+	struct kvm_cpuid_entry2 *entry;
+
+	entry = vcpu_get_cpuid_entry(vcpu, 0xa);
+
+	return (entry->ecx & BIT_ULL(fixed_counter_idx) ||
+		(kvm_cpuid_property(vcpu->cpuid, X86_PROPERTY_PMU_NR_FIXED_COUNTERS) >
+		 fixed_counter_idx));
+}
+
+#endif /* SELFTEST_KVM_PMU_H */
-- 
2.39.3

[PATCH v3 03/11] KVM: selftests: Test Intel PMU architectural events on gp counters

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

Add test cases to check if different Architectural events are available
after it's marked as unavailable via CPUID. It covers vPMU event filtering
logic based on Intel CPUID, which is a complement to pmu_event_filter.

According to Intel SDM, the number of architectural events is reported
through CPUID.0AH:EAX[31:24] and the architectural event x is supported
if EBX[x]=0 && EAX[31:24]>x.

Co-developed-by: Like Xu <likexu@tencent.com>
Signed-off-by: Like Xu <likexu@tencent.com>
Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../kvm/x86_64/pmu_basic_functionality_test.c | 158 ++++++++++++++++++
 2 files changed, 159 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 77026907968f..965a36562ef8 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -80,6 +80,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/mmio_warning_test
 TEST_GEN_PROGS_x86_64 += x86_64/monitor_mwait_test
 TEST_GEN_PROGS_x86_64 += x86_64/nested_exceptions_test
 TEST_GEN_PROGS_x86_64 += x86_64/platform_info_test
+TEST_GEN_PROGS_x86_64 += x86_64/pmu_basic_functionality_test
 TEST_GEN_PROGS_x86_64 += x86_64/pmu_event_filter_test
 TEST_GEN_PROGS_x86_64 += x86_64/set_boot_cpu_id
 TEST_GEN_PROGS_x86_64 += x86_64/set_sregs_test
diff --git a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
new file mode 100644
index 000000000000..c04eb0bdf69f
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test the consistency of the PMU's CPUID and its features
+ *
+ * Copyright (C) 2023, Tencent, Inc.
+ *
+ * Check that the VM's PMU behaviour is consistent with the
+ * VM CPUID definition.
+ */
+
+#define _GNU_SOURCE /* for program_invocation_short_name */
+#include <x86intrin.h>
+
+#include "pmu.h"
+
+/* Guest payload for any performance counter counting */
+#define NUM_BRANCHES			10
+
+static struct kvm_vm *pmu_vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
+						  void *guest_code)
+{
+	struct kvm_vm *vm;
+
+	vm = vm_create_with_one_vcpu(vcpu, guest_code);
+	vm_init_descriptor_tables(vm);
+	vcpu_init_descriptor_tables(*vcpu);
+
+	return vm;
+}
+
+static uint64_t run_vcpu(struct kvm_vcpu *vcpu, uint64_t *ucall_arg)
+{
+	struct ucall uc;
+
+	vcpu_run(vcpu);
+	switch (get_ucall(vcpu, &uc)) {
+	case UCALL_SYNC:
+		*ucall_arg = uc.args[1];
+		break;
+	case UCALL_DONE:
+		break;
+	default:
+		TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
+	}
+	return uc.cmd;
+}
+
+static void guest_measure_loop(uint64_t event_code)
+{
+	uint32_t nr_gp_counters = this_cpu_property(X86_PROPERTY_PMU_NR_GP_COUNTERS);
+	uint32_t pmu_version = this_cpu_property(X86_PROPERTY_PMU_VERSION);
+	uint32_t counter_msr;
+	unsigned int i;
+
+	if (rdmsr(MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES)
+		counter_msr = MSR_IA32_PMC0;
+	else
+		counter_msr = MSR_IA32_PERFCTR0;
+
+	for (i = 0; i < nr_gp_counters; i++) {
+		wrmsr(counter_msr + i, 0);
+		wrmsr(MSR_P6_EVNTSEL0 + i, ARCH_PERFMON_EVENTSEL_OS |
+		      ARCH_PERFMON_EVENTSEL_ENABLE | event_code);
+
+		if (pmu_version > 1) {
+			wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, BIT_ULL(i));
+			__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
+			wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
+			GUEST_SYNC(_rdpmc(i));
+		} else {
+			__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
+			GUEST_SYNC(_rdpmc(i));
+		}
+	}
+
+	GUEST_DONE();
+}
+
+static void test_arch_events_cpuid(struct kvm_vcpu *vcpu,
+				   uint8_t arch_events_bitmap_size,
+				   uint8_t arch_events_unavailable_mask,
+				   uint8_t idx)
+{
+	uint64_t counter_val = 0;
+	bool is_supported;
+
+	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH,
+				arch_events_bitmap_size);
+	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_EVENTS_MASK,
+				arch_events_unavailable_mask);
+
+	is_supported = arch_event_is_supported(vcpu, idx);
+	vcpu_args_set(vcpu, 1, intel_arch_events[idx]);
+
+	while (run_vcpu(vcpu, &counter_val) != UCALL_DONE)
+		TEST_ASSERT_EQ(is_supported, !!counter_val);
+}
+
+static void intel_check_arch_event_is_unavl(uint8_t idx)
+{
+	uint8_t eax_evt_vec, ebx_unavl_mask, i, j;
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+
+	/*
+	 * A brute force iteration of all combinations of values is likely to
+	 * exhaust the limit of the single-threaded thread fd nums, so it's
+	 * tested here by iterating through all valid values on a single bit.
+	 */
+	for (i = 0; i < ARRAY_SIZE(intel_arch_events); i++) {
+		eax_evt_vec = BIT_ULL(i);
+		for (j = 0; j < ARRAY_SIZE(intel_arch_events); j++) {
+			ebx_unavl_mask = BIT_ULL(j);
+			vm = pmu_vm_create_with_one_vcpu(&vcpu,
+							 guest_measure_loop);
+			test_arch_events_cpuid(vcpu, eax_evt_vec,
+					       ebx_unavl_mask, idx);
+
+			kvm_vm_free(vm);
+		}
+	}
+}
+
+static void intel_test_arch_events(void)
+{
+	uint8_t idx;
+
+	for (idx = 0; idx < ARRAY_SIZE(intel_arch_events); idx++) {
+		/*
+		 * Given the stability of performance event recurrence,
+		 * only these arch events are currently being tested:
+		 *
+		 * - Core cycle event (idx = 0)
+		 * - Instruction retired event (idx = 1)
+		 * - Reference cycles event (idx = 2)
+		 * - Branch instruction retired event (idx = 5)
+		 */
+		if (idx > INTEL_ARCH_INSTRUCTIONS_RETIRED &&
+		    idx != INTEL_ARCH_BRANCHES_RETIRED)
+			continue;
+
+		intel_check_arch_event_is_unavl(idx);
+	}
+}
+
+int main(int argc, char *argv[])
+{
+	TEST_REQUIRE(get_kvm_param_bool("enable_pmu"));
+
+	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);
+	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_PDCM));
+
+	intel_test_arch_events();
+
+	return 0;
+}
-- 
2.39.3

[PATCH v3 04/11] KVM: selftests: Test Intel PMU architectural events on fixed counters

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

Update test to cover Intel PMU architectural events on fixed counters.
Per Intel SDM, PMU users can also count architecture performance events
on fixed counters (specifically, FIXED_CTR0 for the retired instructions
and FIXED_CTR1 for cpu core cycles event). Therefore, if guest's CPUID
indicates that an architecture event is not available, the corresponding
fixed counter will also not count that event.

Co-developed-by: Like Xu <likexu@tencent.com>
Signed-off-by: Like Xu <likexu@tencent.com>
Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 .../kvm/x86_64/pmu_basic_functionality_test.c | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
index c04eb0bdf69f..daa45aa285bb 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
@@ -47,6 +47,7 @@ static uint64_t run_vcpu(struct kvm_vcpu *vcpu, uint64_t *ucall_arg)
 
 static void guest_measure_loop(uint64_t event_code)
 {
+	uint32_t nr_fixed_counter = this_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS);
 	uint32_t nr_gp_counters = this_cpu_property(X86_PROPERTY_PMU_NR_GP_COUNTERS);
 	uint32_t pmu_version = this_cpu_property(X86_PROPERTY_PMU_VERSION);
 	uint32_t counter_msr;
@@ -73,6 +74,26 @@ static void guest_measure_loop(uint64_t event_code)
 		}
 	}
 
+	if (pmu_version < 2 || nr_fixed_counter < 1)
+		goto done;
+
+	if (event_code == intel_arch_events[INTEL_ARCH_INSTRUCTIONS_RETIRED])
+		i = 0;
+	else if (event_code == intel_arch_events[INTEL_ARCH_CPU_CYCLES])
+		i = 1;
+	else
+		goto done;
+
+	wrmsr(MSR_CORE_PERF_FIXED_CTR0 + i, 0);
+	wrmsr(MSR_CORE_PERF_FIXED_CTR_CTRL, BIT_ULL(4 * i));
+	wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, BIT_ULL(INTEL_PMC_IDX_FIXED + i));
+
+	__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
+
+	wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
+	GUEST_SYNC(_rdpmc(RDPMC_FIXED_BASE | i));
+
+done:
 	GUEST_DONE();
 }
 
-- 
2.39.3

[PATCH v3 05/11] KVM: selftests: Test consistency of CPUID with num of gp counters

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

Add test to check if non-existent counters can be accessed in guest after
determining the number of Intel generic performance counters by CPUID.
When the num of counters is less than 3, KVM does not emulate #GP if
a counter isn't present due to compatibility MSR_P6_PERFCTRx handling.
Nor will the KVM emulate more counters than it can support.

Co-developed-by: Like Xu <likexu@tencent.com>
Signed-off-by: Like Xu <likexu@tencent.com>
Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 .../kvm/x86_64/pmu_basic_functionality_test.c | 78 +++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
index daa45aa285bb..b86033e51d5c 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
@@ -16,6 +16,11 @@
 /* Guest payload for any performance counter counting */
 #define NUM_BRANCHES			10
 
+static const uint64_t perf_caps[] = {
+	0,
+	PMU_CAP_FW_WRITES,
+};
+
 static struct kvm_vm *pmu_vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
 						  void *guest_code)
 {
@@ -164,6 +169,78 @@ static void intel_test_arch_events(void)
 	}
 }
 
+static void guest_wr_and_rd_msrs(uint32_t base, uint8_t begin, uint8_t offset)
+{
+	uint8_t wr_vector, rd_vector;
+	uint64_t msr_val;
+	unsigned int i;
+
+	for (i = begin; i < begin + offset; i++) {
+		wr_vector = wrmsr_safe(base + i, 0xffff);
+		rd_vector = rdmsr_safe(base + i, &msr_val);
+		if (wr_vector == GP_VECTOR || rd_vector == GP_VECTOR)
+			GUEST_SYNC(GP_VECTOR);
+		else
+			GUEST_SYNC(msr_val);
+	}
+
+	GUEST_DONE();
+}
+
+/* Access the first out-of-range counter register to trigger #GP */
+static void test_oob_gp_counter(uint8_t eax_gp_num, uint8_t offset,
+				uint64_t perf_cap, uint64_t expected)
+{
+	uint32_t ctr_msr = MSR_IA32_PERFCTR0;
+	struct kvm_vcpu *vcpu;
+	uint64_t msr_val = 0;
+	struct kvm_vm *vm;
+
+	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_wr_and_rd_msrs);
+
+	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_NR_GP_COUNTERS,
+				eax_gp_num);
+
+	if (perf_cap & PMU_CAP_FW_WRITES)
+		ctr_msr = MSR_IA32_PMC0;
+
+	vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, perf_cap);
+	vcpu_args_set(vcpu, 3, ctr_msr, eax_gp_num, offset);
+	while (run_vcpu(vcpu, &msr_val) != UCALL_DONE)
+		TEST_ASSERT_EQ(expected, msr_val);
+
+	kvm_vm_free(vm);
+}
+
+static void intel_test_counters_num(void)
+{
+	uint8_t nr_gp_counters = kvm_cpu_property(X86_PROPERTY_PMU_NR_GP_COUNTERS);
+	unsigned int i;
+
+	TEST_REQUIRE(nr_gp_counters > 2);
+
+	for (i = 0; i < ARRAY_SIZE(perf_caps); i++) {
+		/*
+		 * For compatibility reasons, KVM does not emulate #GP
+		 * when MSR_P6_PERFCTR[0|1] is not present, but it doesn't
+		 * affect checking the presence of MSR_IA32_PMCx with #GP.
+		 */
+		if (perf_caps[i] & PMU_CAP_FW_WRITES)
+			test_oob_gp_counter(0, 1, perf_caps[i], GP_VECTOR);
+
+		test_oob_gp_counter(2, 1, perf_caps[i], GP_VECTOR);
+		test_oob_gp_counter(nr_gp_counters, 1, perf_caps[i], GP_VECTOR);
+
+		/* KVM doesn't emulate more counters than it can support. */
+		test_oob_gp_counter(nr_gp_counters + 1, 1, perf_caps[i],
+				    GP_VECTOR);
+
+		/* Test that KVM drops writes to MSR_P6_PERFCTR[0|1]. */
+		if (!perf_caps[i])
+			test_oob_gp_counter(0, 2, perf_caps[i], 0);
+	}
+}
+
 int main(int argc, char *argv[])
 {
 	TEST_REQUIRE(get_kvm_param_bool("enable_pmu"));
@@ -174,6 +251,7 @@ int main(int argc, char *argv[])
 	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_PDCM));
 
 	intel_test_arch_events();
+	intel_test_counters_num();
 
 	return 0;
 }
-- 
2.39.3

[PATCH v3 06/11] KVM: selftests: Test consistency of CPUID with num of fixed counters

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

Add test to check if non-existent counters can be accessed in guest after
determining the number of Intel generic performance counters by CPUID.
Per SDM, fixed-function performance counter 'i' is supported if ECX[i] ||
(EDX[4:0] > i). KVM doesn't emulate more counters than it can support.

Co-developed-by: Like Xu <likexu@tencent.com>
Signed-off-by: Like Xu <likexu@tencent.com>
Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 .../kvm/x86_64/pmu_basic_functionality_test.c | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
index b86033e51d5c..db1c1230700a 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
@@ -212,10 +212,43 @@ static void test_oob_gp_counter(uint8_t eax_gp_num, uint8_t offset,
 	kvm_vm_free(vm);
 }
 
+static void intel_test_oob_fixed_ctr(uint8_t edx_fixed_num,
+				     uint32_t fixed_bitmask, uint64_t expected)
+{
+	uint8_t idx = edx_fixed_num;
+	struct kvm_vcpu *vcpu;
+	uint64_t msr_val = 0;
+	struct kvm_vm *vm;
+	bool visible;
+
+	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_wr_and_rd_msrs);
+
+	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_FIXED_COUNTERS_BITMASK,
+				fixed_bitmask);
+	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_NR_FIXED_COUNTERS,
+				edx_fixed_num);
+
+	visible = fixed_counter_is_supported(vcpu, idx);
+
+	/* KVM doesn't emulate more fixed counters than it can support. */
+	if (idx >= kvm_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS))
+		visible = false;
+
+	vcpu_args_set(vcpu, 3, MSR_CORE_PERF_FIXED_CTR0, idx, 1);
+	if (visible) {
+		while (run_vcpu(vcpu, &msr_val) != UCALL_DONE)
+			TEST_ASSERT_EQ(expected, msr_val);
+	}
+
+	kvm_vm_free(vm);
+}
+
 static void intel_test_counters_num(void)
 {
+	uint8_t nr_fixed_counters = kvm_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS);
 	uint8_t nr_gp_counters = kvm_cpu_property(X86_PROPERTY_PMU_NR_GP_COUNTERS);
 	unsigned int i;
+	uint32_t ecx;
 
 	TEST_REQUIRE(nr_gp_counters > 2);
 
@@ -239,6 +272,14 @@ static void intel_test_counters_num(void)
 		if (!perf_caps[i])
 			test_oob_gp_counter(0, 2, perf_caps[i], 0);
 	}
+
+	for (ecx = 0;
+	     ecx <= kvm_cpu_property(X86_PROPERTY_PMU_FIXED_COUNTERS_BITMASK) + 1;
+	     ecx++) {
+		intel_test_oob_fixed_ctr(0, ecx, GP_VECTOR);
+		intel_test_oob_fixed_ctr(nr_fixed_counters, ecx, GP_VECTOR);
+		intel_test_oob_fixed_ctr(nr_fixed_counters + 1, ecx, GP_VECTOR);
+	}
 }
 
 int main(int argc, char *argv[])
-- 
2.39.3

[PATCH v3 07/11] KVM: selftests: Test Intel supported fixed counters bit mask

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

Add a test to check that fixed counters enabled via guest
CPUID.0xA.ECX (instead of EDX[04:00]) work as normal as usual.

Co-developed-by: Like Xu <likexu@tencent.com>
Signed-off-by: Like Xu <likexu@tencent.com>
Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 .../kvm/x86_64/pmu_basic_functionality_test.c | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
index db1c1230700a..3bbf3bd2846b 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
@@ -282,6 +282,65 @@ static void intel_test_counters_num(void)
 	}
 }
 
+static void intel_guest_run_fixed_counters(void)
+{
+	uint64_t supported_bitmask = this_cpu_property(X86_PROPERTY_PMU_FIXED_COUNTERS_BITMASK);
+	uint32_t nr_fixed_counter = this_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS);
+	uint64_t msr_val;
+	unsigned int i;
+	bool expected;
+
+	for (i = 0; i < nr_fixed_counter; i++) {
+		expected = supported_bitmask & BIT_ULL(i) || i < nr_fixed_counter;
+
+		wrmsr_safe(MSR_CORE_PERF_FIXED_CTR0 + i, 0);
+		wrmsr_safe(MSR_CORE_PERF_FIXED_CTR_CTRL, BIT_ULL(4 * i));
+		wrmsr_safe(MSR_CORE_PERF_GLOBAL_CTRL, BIT_ULL(INTEL_PMC_IDX_FIXED + i));
+		__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
+		wrmsr_safe(MSR_CORE_PERF_GLOBAL_CTRL, 0);
+		rdmsr_safe(MSR_CORE_PERF_FIXED_CTR0 + i, &msr_val);
+
+		GUEST_ASSERT(expected == !!msr_val);
+	}
+
+	GUEST_DONE();
+}
+
+static void test_fixed_counters_setup(struct kvm_vcpu *vcpu,
+				      uint32_t fixed_bitmask,
+				      uint8_t edx_fixed_num)
+{
+	int ret;
+
+	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_FIXED_COUNTERS_BITMASK,
+				fixed_bitmask);
+	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_NR_FIXED_COUNTERS,
+				edx_fixed_num);
+
+	do {
+		ret = run_vcpu(vcpu, NULL);
+	} while (ret != UCALL_DONE);
+}
+
+static void intel_test_fixed_counters(void)
+{
+	uint8_t nr_fixed_counters = kvm_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS);
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	uint32_t ecx;
+	uint8_t edx;
+
+	for (edx = 0; edx <= nr_fixed_counters; edx++) {
+		/* KVM doesn't emulate more fixed counters than it can support. */
+		for (ecx = 0; ecx <= (BIT_ULL(nr_fixed_counters) - 1); ecx++) {
+			vm = pmu_vm_create_with_one_vcpu(&vcpu,
+							 intel_guest_run_fixed_counters);
+			test_fixed_counters_setup(vcpu, ecx, edx);
+			kvm_vm_free(vm);
+		}
+	}
+}
+
 int main(int argc, char *argv[])
 {
 	TEST_REQUIRE(get_kvm_param_bool("enable_pmu"));
@@ -293,6 +352,7 @@ int main(int argc, char *argv[])
 
 	intel_test_arch_events();
 	intel_test_counters_num();
+	intel_test_fixed_counters();
 
 	return 0;
 }
-- 
2.39.3

[PATCH v3 08/11] KVM: selftests: Test consistency of PMU MSRs with Intel PMU version

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

KVM user sapce may control the Intel guest PMU version number via
CPUID.0AH:EAX[07:00]. A test is added to check if a typical PMU register
that is not available at the current version number is leaking.

Co-developed-by: Like Xu <likexu@tencent.com>
Signed-off-by: Like Xu <likexu@tencent.com>
Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 .../kvm/x86_64/pmu_basic_functionality_test.c | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
index 3bbf3bd2846b..70adfad45010 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
@@ -16,6 +16,12 @@
 /* Guest payload for any performance counter counting */
 #define NUM_BRANCHES			10
 
+/*
+ * KVM implements the first two non-existent counters (MSR_P6_PERFCTRx)
+ * via kvm_pr_unimpl_wrmsr() instead of #GP.
+ */
+#define MSR_INTEL_ARCH_PMU_GPCTR (MSR_IA32_PERFCTR0 + 2)
+
 static const uint64_t perf_caps[] = {
 	0,
 	PMU_CAP_FW_WRITES,
@@ -341,6 +347,66 @@ static void intel_test_fixed_counters(void)
 	}
 }
 
+static void intel_guest_check_pmu_version(uint8_t version)
+{
+	switch (version) {
+	case 0:
+		GUEST_SYNC(wrmsr_safe(MSR_INTEL_ARCH_PMU_GPCTR, 0xffffull));
+	case 1:
+		GUEST_SYNC(wrmsr_safe(MSR_CORE_PERF_GLOBAL_CTRL, 0x1ull));
+	case 2:
+		/*
+		 * AnyThread Bit is only supported in version 3
+		 *
+		 * The strange thing is that when version=0, writing ANY-Any
+		 * Thread bit (bit 21) in MSR_P6_EVNTSEL0 and MSR_P6_EVNTSEL1
+		 * will not generate #GP. While writing ANY-Any Thread bit
+		 * (bit 21) in MSR_P6_EVNTSEL0+x (MAX_GP_CTR_NUM > x > 2) to
+		 * ANY-Any Thread bit (bit 21) will generate #GP.
+		 */
+		if (version == 0)
+			break;
+
+		GUEST_SYNC(wrmsr_safe(MSR_P6_EVNTSEL0,
+				      ARCH_PERFMON_EVENTSEL_ANY));
+		break;
+	default:
+		/* KVM currently supports up to pmu version 2 */
+		GUEST_SYNC(GP_VECTOR);
+	}
+
+	GUEST_DONE();
+}
+
+static void test_pmu_version_setup(struct kvm_vcpu *vcpu, uint8_t version,
+				   uint64_t expected)
+{
+	uint64_t msr_val = 0;
+
+	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_VERSION, version);
+
+	vcpu_args_set(vcpu, 1, version);
+	while (run_vcpu(vcpu, &msr_val) != UCALL_DONE)
+		TEST_ASSERT_EQ(expected, msr_val);
+}
+
+static void intel_test_pmu_version(void)
+{
+	uint8_t unsupported_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION) + 1;
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	uint8_t version;
+
+	TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS) > 2);
+
+	for (version = 0; version <= unsupported_version; version++) {
+		vm = pmu_vm_create_with_one_vcpu(&vcpu,
+						 intel_guest_check_pmu_version);
+		test_pmu_version_setup(vcpu, version, GP_VECTOR);
+		kvm_vm_free(vm);
+	}
+}
+
 int main(int argc, char *argv[])
 {
 	TEST_REQUIRE(get_kvm_param_bool("enable_pmu"));
@@ -353,6 +419,7 @@ int main(int argc, char *argv[])
 	intel_test_arch_events();
 	intel_test_counters_num();
 	intel_test_fixed_counters();
+	intel_test_pmu_version();
 
 	return 0;
 }
-- 
2.39.3

[PATCH v3 09/11] KVM: selftests: Add x86 feature and properties for AMD PMU in processor.h

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

Add x86 feature and properties for AMD PMU so that tests don't have
to manually retrieve the correct CPUID leaf+register, and so that the
resulting code is self-documenting.

Signed-off-by: Jinrong Liang <cloudliang@tencent.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 6b146e1c6736..07b980b8bec2 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -167,6 +167,7 @@ struct kvm_x86_cpu_feature {
  */
 #define	X86_FEATURE_SVM			KVM_X86_CPU_FEATURE(0x80000001, 0, ECX, 2)
 #define	X86_FEATURE_NX			KVM_X86_CPU_FEATURE(0x80000001, 0, EDX, 20)
+#define	X86_FEATURE_AMD_PMU_EXT_CORE	KVM_X86_CPU_FEATURE(0x80000001, 0, ECX, 23)
 #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)
@@ -182,6 +183,9 @@ 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_AMD_PERFMON_V2	KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 0)
+#define X86_FEATURE_AMD_LBR_STACK	KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 1)
+#define X86_FEATURE_AMD_LBR_PMC_FREEZE	KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 2)
 
 /*
  * KVM defined paravirt features.
@@ -267,6 +271,9 @@ struct kvm_x86_cpu_property {
 #define X86_PROPERTY_MAX_VIRT_ADDR		KVM_X86_CPU_PROPERTY(0x80000008, 0, EAX, 8, 15)
 #define X86_PROPERTY_PHYS_ADDR_REDUCTION	KVM_X86_CPU_PROPERTY(0x8000001F, 0, EBX, 6, 11)
 
+#define X86_PROPERTY_AMD_PMU_NR_CORE_COUNTERS	KVM_X86_CPU_PROPERTY(0x80000022, 0, EBX, 0, 3)
+#define X86_PROPERTY_AMD_PMU_LBR_STACK_SIZE	KVM_X86_CPU_PROPERTY(0x80000022, 0, EBX, 4, 9)
+
 #define X86_PROPERTY_MAX_CENTAUR_LEAF		KVM_X86_CPU_PROPERTY(0xC0000000, 0, EAX, 0, 31)
 
 /*
-- 
2.39.3

[PATCH v3 10/11] KVM: selftests: Test AMD PMU events on legacy four performance counters

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

Add tests to check AMD PMU legacy four performance counters.

Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 .../kvm/x86_64/pmu_basic_functionality_test.c | 72 ++++++++++++++-----
 1 file changed, 54 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
index 70adfad45010..cb2a7ad5c504 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
@@ -58,20 +58,29 @@ static uint64_t run_vcpu(struct kvm_vcpu *vcpu, uint64_t *ucall_arg)
 
 static void guest_measure_loop(uint64_t event_code)
 {
-	uint32_t nr_fixed_counter = this_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS);
-	uint32_t nr_gp_counters = this_cpu_property(X86_PROPERTY_PMU_NR_GP_COUNTERS);
-	uint32_t pmu_version = this_cpu_property(X86_PROPERTY_PMU_VERSION);
+	uint8_t nr_gp_counters, pmu_version = 1;
+	uint64_t event_sel_msr;
 	uint32_t counter_msr;
 	unsigned int i;
 
-	if (rdmsr(MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES)
-		counter_msr = MSR_IA32_PMC0;
-	else
-		counter_msr = MSR_IA32_PERFCTR0;
+	if (host_cpu_is_intel) {
+		nr_gp_counters = this_cpu_property(X86_PROPERTY_PMU_NR_GP_COUNTERS);
+		pmu_version = this_cpu_property(X86_PROPERTY_PMU_VERSION);
+		event_sel_msr = MSR_P6_EVNTSEL0;
+
+		if (rdmsr(MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES)
+			counter_msr = MSR_IA32_PMC0;
+		else
+			counter_msr = MSR_IA32_PERFCTR0;
+	} else {
+		nr_gp_counters = AMD64_NR_COUNTERS;
+		event_sel_msr = MSR_K7_EVNTSEL0;
+		counter_msr = MSR_K7_PERFCTR0;
+	}
 
 	for (i = 0; i < nr_gp_counters; i++) {
 		wrmsr(counter_msr + i, 0);
-		wrmsr(MSR_P6_EVNTSEL0 + i, ARCH_PERFMON_EVENTSEL_OS |
+		wrmsr(event_sel_msr + i, ARCH_PERFMON_EVENTSEL_OS |
 		      ARCH_PERFMON_EVENTSEL_ENABLE | event_code);
 
 		if (pmu_version > 1) {
@@ -85,7 +94,12 @@ static void guest_measure_loop(uint64_t event_code)
 		}
 	}
 
-	if (pmu_version < 2 || nr_fixed_counter < 1)
+	if (host_cpu_is_amd || pmu_version < 2)
+		goto done;
+
+	uint32_t nr_fixed_counter = this_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS);
+
+	if (nr_fixed_counter < 1)
 		goto done;
 
 	if (event_code == intel_arch_events[INTEL_ARCH_INSTRUCTIONS_RETIRED])
@@ -407,19 +421,41 @@ static void intel_test_pmu_version(void)
 	}
 }
 
+static void amd_test_pmu_counters(void)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	unsigned int i;
+	uint64_t msr_val;
+
+	for (i = 0; i < ARRAY_SIZE(amd_arch_events); i++) {
+		vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_measure_loop);
+		vcpu_args_set(vcpu, 1, amd_arch_events[i]);
+		while (run_vcpu(vcpu, &msr_val) != UCALL_DONE)
+			TEST_ASSERT(msr_val, "Unexpected AMD counter values");
+
+		kvm_vm_free(vm);
+	}
+}
+
 int main(int argc, char *argv[])
 {
 	TEST_REQUIRE(get_kvm_param_bool("enable_pmu"));
 
-	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);
-	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_PDCM));
-
-	intel_test_arch_events();
-	intel_test_counters_num();
-	intel_test_fixed_counters();
-	intel_test_pmu_version();
+	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);
+		TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_PDCM));
+
+		intel_test_arch_events();
+		intel_test_counters_num();
+		intel_test_fixed_counters();
+		intel_test_pmu_version();
+	} else if (host_cpu_is_amd) {
+		amd_test_pmu_counters();
+	} else {
+		TEST_FAIL("Unknown CPU vendor");
+	}
 
 	return 0;
 }
-- 
2.39.3

[PATCH v3 11/11] KVM: selftests: Test AMD Guest PerfMonV2

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

Add test case for AMD Guest PerfMonV2. Also test Intel
MSR_CORE_PERF_GLOBAL_STATUS and MSR_CORE_PERF_GLOBAL_OVF_CTRL.

Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 .../kvm/x86_64/pmu_basic_functionality_test.c | 48 ++++++++++++++++++-
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
index cb2a7ad5c504..02bd1fe3900b 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
@@ -58,7 +58,9 @@ static uint64_t run_vcpu(struct kvm_vcpu *vcpu, uint64_t *ucall_arg)
 
 static void guest_measure_loop(uint64_t event_code)
 {
+	uint64_t global_ovf_ctrl_msr, global_status_msr, global_ctrl_msr;
 	uint8_t nr_gp_counters, pmu_version = 1;
+	uint8_t gp_counter_bit_width = 48;
 	uint64_t event_sel_msr;
 	uint32_t counter_msr;
 	unsigned int i;
@@ -68,6 +70,12 @@ static void guest_measure_loop(uint64_t event_code)
 		pmu_version = this_cpu_property(X86_PROPERTY_PMU_VERSION);
 		event_sel_msr = MSR_P6_EVNTSEL0;
 
+		if (pmu_version > 1) {
+			global_ovf_ctrl_msr = MSR_CORE_PERF_GLOBAL_OVF_CTRL;
+			global_status_msr = MSR_CORE_PERF_GLOBAL_STATUS;
+			global_ctrl_msr = MSR_CORE_PERF_GLOBAL_CTRL;
+		}
+
 		if (rdmsr(MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES)
 			counter_msr = MSR_IA32_PMC0;
 		else
@@ -76,6 +84,17 @@ static void guest_measure_loop(uint64_t event_code)
 		nr_gp_counters = AMD64_NR_COUNTERS;
 		event_sel_msr = MSR_K7_EVNTSEL0;
 		counter_msr = MSR_K7_PERFCTR0;
+
+		if (this_cpu_has(X86_FEATURE_AMD_PMU_EXT_CORE) &&
+		    this_cpu_has(X86_FEATURE_AMD_PERFMON_V2)) {
+			nr_gp_counters = this_cpu_property(X86_PROPERTY_AMD_PMU_NR_CORE_COUNTERS);
+			global_ovf_ctrl_msr = MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR;
+			global_status_msr = MSR_AMD64_PERF_CNTR_GLOBAL_STATUS;
+			global_ctrl_msr = MSR_AMD64_PERF_CNTR_GLOBAL_CTL;
+			event_sel_msr = MSR_F15H_PERF_CTL0;
+			counter_msr = MSR_F15H_PERF_CTR0;
+			pmu_version = 2;
+		}
 	}
 
 	for (i = 0; i < nr_gp_counters; i++) {
@@ -84,14 +103,39 @@ static void guest_measure_loop(uint64_t event_code)
 		      ARCH_PERFMON_EVENTSEL_ENABLE | event_code);
 
 		if (pmu_version > 1) {
-			wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, BIT_ULL(i));
+			wrmsr(global_ctrl_msr, BIT_ULL(i));
 			__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
-			wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
+			wrmsr(global_ctrl_msr, 0);
 			GUEST_SYNC(_rdpmc(i));
 		} else {
 			__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
 			GUEST_SYNC(_rdpmc(i));
 		}
+
+		if (pmu_version > 1 && _rdpmc(i)) {
+			wrmsr(global_ctrl_msr, 0);
+			wrmsr(counter_msr + i, 0);
+			__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
+			GUEST_ASSERT(!_rdpmc(i));
+
+			wrmsr(global_ctrl_msr, BIT_ULL(i));
+			__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
+			GUEST_ASSERT(_rdpmc(i));
+
+			if (host_cpu_is_intel)
+				gp_counter_bit_width =
+					this_cpu_property(X86_PROPERTY_PMU_GP_COUNTERS_BIT_WIDTH);
+
+			wrmsr(global_ctrl_msr, 0);
+			wrmsr(counter_msr + i, (1ULL << gp_counter_bit_width) - 2);
+			wrmsr(global_ctrl_msr, BIT_ULL(i));
+			__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
+			GUEST_ASSERT(rdmsr(global_status_msr) & BIT_ULL(i));
+
+			wrmsr(global_ctrl_msr, 0);
+			wrmsr(global_ovf_ctrl_msr, BIT_ULL(i));
+			GUEST_ASSERT(!(rdmsr(global_status_msr) & BIT_ULL(i)));
+		}
 	}
 
 	if (host_cpu_is_amd || pmu_version < 2)
-- 
2.39.3

Re: [PATCH v3 02/11] KVM: selftests: Add pmu.h for PMU events and common masks

[Sean Christopherson]

On Mon, Aug 14, 2023, Jinrong Liang wrote:
> +static const uint64_t intel_arch_events[] = {

Hmm, rather than have a bunch of static arrays, I think it makes sense to go ahead
and add lib/pmu.c.  Hmm, and this should probably be further namespaced, e.g.
intel_pmu_arch_events

> +	[INTEL_ARCH_CPU_CYCLES]			= ARCH_EVENT(0x3c, 0x0),
> +	[INTEL_ARCH_INSTRUCTIONS_RETIRED]	= ARCH_EVENT(0xc0, 0x0),
> +	[INTEL_ARCH_REFERENCE_CYCLES]		= ARCH_EVENT(0x3c, 0x1),
> +	[INTEL_ARCH_LLC_REFERENCES]		= ARCH_EVENT(0x2e, 0x4f),
> +	[INTEL_ARCH_LLC_MISSES]			= ARCH_EVENT(0x2e, 0x41),
> +	[INTEL_ARCH_BRANCHES_RETIRED]		= ARCH_EVENT(0xc4, 0x0),
> +	[INTEL_ARCH_BRANCHES_MISPREDICTED]	= ARCH_EVENT(0xc5, 0x0),
> +	[PSEUDO_ARCH_REFERENCE_CYCLES]		= ARCH_EVENT(0xa4, 0x1),
> +};
> +
> +/* mapping between fixed pmc index and intel_arch_events array */
> +static const int fixed_pmc_events[] = {

Please be consistent with names (even if KVM itself may be anything but).  KVM
gets away with sloppiness because the arrays are only visible to pmu_intel.c,
but that's not the case here.

intel_pmu_fixed_pmc_events?

> +	[0] = INTEL_ARCH_INSTRUCTIONS_RETIRED,
> +	[1] = INTEL_ARCH_CPU_CYCLES,
> +	[2] = PSEUDO_ARCH_REFERENCE_CYCLES,
> +};
> +
> +enum amd_pmu_k7_events {
> +	AMD_ZEN_CORE_CYCLES,
> +	AMD_ZEN_INSTRUCTIONS,
> +	AMD_ZEN_BRANCHES,
> +	AMD_ZEN_BRANCH_MISSES,
> +};
> +
> +static const uint64_t amd_arch_events[] = {

And then amd_pmu_arch_events.

> +	[AMD_ZEN_CORE_CYCLES]			= ARCH_EVENT(0x76, 0x00),
> +	[AMD_ZEN_INSTRUCTIONS]			= ARCH_EVENT(0xc0, 0x00),
> +	[AMD_ZEN_BRANCHES]			= ARCH_EVENT(0xc2, 0x00),
> +	[AMD_ZEN_BRANCH_MISSES]			= ARCH_EVENT(0xc3, 0x00),
> +};
> +
> +static inline bool arch_event_is_supported(struct kvm_vcpu *vcpu,
> +					   uint8_t arch_event)

Same namespace problem.  And I'd put the "is" earlier so that it's clearly a
question and not a command, e.g. "is this arch event supported?" versus "this
arch event is supported".

So pmu_is_arch_event_supported()?  Actually, no, you're reinventing the wheel.
You can query all of the Intel arch events from within the guest via this_pmu_has(),
e.g. see X86_PMU_FEATURE_BRANCH_INSNS_RETIRED.  You just need a helper (or array)
to get from an arbitrary index to its associated feature.

And now that GUEST_ASSERT_EQ() prints values, you can just do

			counter = _rdpmc(i);
			GUEST_ASSERT_EQ(this_pmu_has(...), !!counter);

in guest_measure_loop() instead of funneling the counter into ucall and back to
the host.

> +{
> +	struct kvm_cpuid_entry2 *entry;
> +
> +	entry = vcpu_get_cpuid_entry(vcpu, 0xa);
> +
> +	return !(entry->ebx & BIT_ULL(arch_event)) &&
> +		(kvm_cpuid_property(vcpu->cpuid,
> +		 X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH) > arch_event);
> +}
> +
> +static inline bool fixed_counter_is_supported(struct kvm_vcpu *vcpu,

More namespace problems.  I don't love pmu_is_fixed_counter_supported(), because
that glosses over this operating on the vCPU, e.g. not KVM and not guest CPUID
(from within the guest).

And with a bit of massaging to the "anti-feature" framework, this_pmu_has() and
kvm_pmu_has() can be extended (unless I'm missing something).

diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index 07b980b8bec2..21f0c45c2ac6 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -287,12 +287,12 @@ struct kvm_x86_cpu_property {
  * architectural event is supported.
  */
 struct kvm_x86_pmu_feature {
-       struct kvm_x86_cpu_feature anti_feature;
+       struct kvm_x86_cpu_feature pmu_feature;
 };
 #define        KVM_X86_PMU_FEATURE(name, __bit)                                        \
 ({                                                                             \
        struct kvm_x86_pmu_feature feature = {                                  \
-               .anti_feature = KVM_X86_CPU_FEATURE(0xa, 0, EBX, __bit),        \
+               .pmu_feature = KVM_X86_CPU_FEATURE(0xa, 0, EBX, __bit), \
        };                                                                      \
                                                                                \
        feature;                                                                \
@@ -690,10 +690,19 @@ static __always_inline bool this_cpu_has_p(struct kvm_x86_cpu_property property)
 
 static inline bool this_pmu_has(struct kvm_x86_pmu_feature feature)
 {
-       uint32_t nr_bits = this_cpu_property(X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH);
+       uint32_t nr_bits;
 
-       return nr_bits > feature.anti_feature.bit &&
-              !this_cpu_has(feature.anti_feature);
+       if (feature.pmu_feature.reg == KVM_CPUID_EBX) {
+               nr_bits = this_cpu_property(X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH);
+               return nr_bits > feature.pmu_feature.bit &&
+                      !this_cpu_has(feature.pmu_feature);
+       } else if (feature.pmu_feature.reg == KVM_CPUID_ECX) {
+               nr_bits = this_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS);
+               return nr_bits > feature.pmu_feature.bit ||
+                      this_cpu_has(feature.pmu_feature);
+       } else {
+               TEST_FAIL(...);
+       }
 }
 
 static __always_inline uint64_t this_cpu_supported_xcr0(void)


That doesn't give you a direct path to replacing fixed_counter_is_supported(),
but the usage in intel_test_oob_fixed_ctr() is bizarre and looks wrong, e.g. if
it's not supported, the test does nothing.

Re: [PATCH v3 03/11] KVM: selftests: Test Intel PMU architectural events on gp counters

[Sean Christopherson]

On Mon, Aug 14, 2023, Jinrong Liang wrote:
> +static void test_arch_events_cpuid(struct kvm_vcpu *vcpu,
> +				   uint8_t arch_events_bitmap_size,
> +				   uint8_t arch_events_unavailable_mask,
> +				   uint8_t idx)
> +{
> +	uint64_t counter_val = 0;
> +	bool is_supported;
> +
> +	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH,
> +				arch_events_bitmap_size);
> +	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_EVENTS_MASK,
> +				arch_events_unavailable_mask);
> +
> +	is_supported = arch_event_is_supported(vcpu, idx);
> +	vcpu_args_set(vcpu, 1, intel_arch_events[idx]);
> +
> +	while (run_vcpu(vcpu, &counter_val) != UCALL_DONE)
> +		TEST_ASSERT_EQ(is_supported, !!counter_val);
> +}
> +
> +static void intel_check_arch_event_is_unavl(uint8_t idx)
> +{
> +	uint8_t eax_evt_vec, ebx_unavl_mask, i, j;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +
> +	/*
> +	 * A brute force iteration of all combinations of values is likely to
> +	 * exhaust the limit of the single-threaded thread fd nums, so it's
> +	 * tested here by iterating through all valid values on a single bit.
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(intel_arch_events); i++) {
> +		eax_evt_vec = BIT_ULL(i);
> +		for (j = 0; j < ARRAY_SIZE(intel_arch_events); j++) {
> +			ebx_unavl_mask = BIT_ULL(j);
> +			vm = pmu_vm_create_with_one_vcpu(&vcpu,
> +							 guest_measure_loop);
> +			test_arch_events_cpuid(vcpu, eax_evt_vec,
> +					       ebx_unavl_mask, idx);
> +
> +			kvm_vm_free(vm);

This is messy.  If you're going to use a helper, then use the helper.  If not,
then open code everything.  Half and half just makes everything unnecessarily
hard to follow.  E.g. if you reorganize things, and move even more checks into
the guest, I think you can end up with:


static void test_arch_events_cpuid(uint8_t i, uint8_t j, uint8_t idx)
{
	uint8_t eax_evt_vec = BIT_ULL(i);
	uint8_t ebx_unavl_mask = BIT_ULL(j);
	struct kvm_vcpu *vcpu;
	struct kvm_vm *vm;

	vm = pmu_vm_create_with_one_vcpu(&vcpu, guest_measure_loop);

	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH,
				arch_events_bitmap_size);
	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_EVENTS_MASK,
				arch_events_unavailable_mask);

	vcpu_args_set(vcpu, 1, idx);

	run_vcpu(vcpu, &counter_val)

	kvm_vm_free(vm);
}

static void intel_check_arch_event_is_unavl(uint8_t idx)
{
	/*
	 * A brute force iteration of all combinations of values is likely to
	 * exhaust the limit of the single-threaded thread fd nums, so it's
	 * tested here by iterating through all valid values on a single bit.
	 */
	for (i = 0; i < ARRAY_SIZE(intel_arch_events); i++) {
		eax_evt_vec = BIT_ULL(i);
		for (j = 0; j < ARRAY_SIZE(intel_arch_events); j++)
			test_arch_events_cpuid(i, j, idx);
	}
}

Re: [PATCH v3 03/11] KVM: selftests: Test Intel PMU architectural events on gp counters

[Sean Christopherson]

On Mon, Aug 14, 2023, Jinrong Liang wrote:
> Add test case for AMD Guest PerfMonV2. Also test Intel
> MSR_CORE_PERF_GLOBAL_STATUS and MSR_CORE_PERF_GLOBAL_OVF_CTRL.
> 
> Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
> ---
>  .../kvm/x86_64/pmu_basic_functionality_test.c | 48 ++++++++++++++++++-
>  1 file changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> index cb2a7ad5c504..02bd1fe3900b 100644
> --- a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> @@ -58,7 +58,9 @@ static uint64_t run_vcpu(struct kvm_vcpu *vcpu, uint64_t *ucall_arg)
>  
>  static void guest_measure_loop(uint64_t event_code)
>  {
> +	uint64_t global_ovf_ctrl_msr, global_status_msr, global_ctrl_msr;
>  	uint8_t nr_gp_counters, pmu_version = 1;
> +	uint8_t gp_counter_bit_width = 48;
>  	uint64_t event_sel_msr;
>  	uint32_t counter_msr;
>  	unsigned int i;
> @@ -68,6 +70,12 @@ static void guest_measure_loop(uint64_t event_code)
>  		pmu_version = this_cpu_property(X86_PROPERTY_PMU_VERSION);
>  		event_sel_msr = MSR_P6_EVNTSEL0;
>  
> +		if (pmu_version > 1) {
> +			global_ovf_ctrl_msr = MSR_CORE_PERF_GLOBAL_OVF_CTRL;
> +			global_status_msr = MSR_CORE_PERF_GLOBAL_STATUS;
> +			global_ctrl_msr = MSR_CORE_PERF_GLOBAL_CTRL;
> +		}
> +
>  		if (rdmsr(MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES)
>  			counter_msr = MSR_IA32_PMC0;
>  		else
> @@ -76,6 +84,17 @@ static void guest_measure_loop(uint64_t event_code)
>  		nr_gp_counters = AMD64_NR_COUNTERS;
>  		event_sel_msr = MSR_K7_EVNTSEL0;
>  		counter_msr = MSR_K7_PERFCTR0;
> +
> +		if (this_cpu_has(X86_FEATURE_AMD_PMU_EXT_CORE) &&
> +		    this_cpu_has(X86_FEATURE_AMD_PERFMON_V2)) {
> +			nr_gp_counters = this_cpu_property(X86_PROPERTY_AMD_PMU_NR_CORE_COUNTERS);
> +			global_ovf_ctrl_msr = MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR;
> +			global_status_msr = MSR_AMD64_PERF_CNTR_GLOBAL_STATUS;
> +			global_ctrl_msr = MSR_AMD64_PERF_CNTR_GLOBAL_CTL;
> +			event_sel_msr = MSR_F15H_PERF_CTL0;
> +			counter_msr = MSR_F15H_PERF_CTR0;
> +			pmu_version = 2;
> +		}

Please use an if-else when the two things are completely exclusive, i.e. don't
set "defaults" and then override them.

>  	}
>  
>  	for (i = 0; i < nr_gp_counters; i++) {
> @@ -84,14 +103,39 @@ static void guest_measure_loop(uint64_t event_code)
>  		      ARCH_PERFMON_EVENTSEL_ENABLE | event_code);
>  
>  		if (pmu_version > 1) {
> -			wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, BIT_ULL(i));
> +			wrmsr(global_ctrl_msr, BIT_ULL(i));
>  			__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
> -			wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
> +			wrmsr(global_ctrl_msr, 0);
>  			GUEST_SYNC(_rdpmc(i));
>  		} else {
>  			__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
>  			GUEST_SYNC(_rdpmc(i));
>  		}

This is extremely difficult to follow.  I think the same thing to do is to split
this up into helpers, e.g. send pmu_version > 1 into one path, and pmu_version <= 1
into an entirely different path.

E.g. something like this?

static void guest_measure_loop(uint64_t event_code)
{
	uint64_t global_ovf_ctrl_msr, global_status_msr, global_ctrl_msr;
	uint8_t nr_gp_counters, pmu_version;
	uint8_t gp_counter_bit_width;
	uint64_t event_sel_msr;
	uint32_t counter_msr;
	unsigned int i;

	if (host_cpu_is_intel)
		pmu_version = this_cpu_property(X86_PROPERTY_PMU_VERSION);
	else if (this_cpu_has(X86_FEATURE_PERFCTR_CORE) &&
		 this_cpu_has(X86_FEATURE_PERFMON_V2)) {
		pmu_version = 2;
	} else {
		pmu_version = 1;
	}

	if (pmu_version <= 1) {
		guest_measure_pmu_legacy(...);
		return;
	}

	if (host_cpu_is_intel) {
		nr_gp_counters = this_cpu_property(X86_PROPERTY_PMU_NR_GP_COUNTERS);
		global_ovf_ctrl_msr = MSR_CORE_PERF_GLOBAL_OVF_CTRL;
		global_status_msr = MSR_CORE_PERF_GLOBAL_STATUS;
		global_ctrl_msr = MSR_CORE_PERF_GLOBAL_CTRL;
		gp_counter_bit_width = this_cpu_property(X86_PROPERTY_PMU_GP_COUNTERS_BIT_WIDTH);

		if (rdmsr(MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES)
			counter_msr = MSR_IA32_PMC0;
		else
			counter_msr = MSR_IA32_PERFCTR0;
	} else {
		nr_gp_counters = this_cpu_property(X86_PROPERTY_AMD_PMU_NR_CORE_COUNTERS);
		global_ovf_ctrl_msr = MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR;
		global_status_msr = MSR_AMD64_PERF_CNTR_GLOBAL_STATUS;
		global_ctrl_msr = MSR_AMD64_PERF_CNTR_GLOBAL_CTL;
		event_sel_msr = MSR_F15H_PERF_CTL0;
		counter_msr = MSR_F15H_PERF_CTR0;
		gp_counter_bit_width = 48;
	}

	for (i = 0; i < nr_gp_counters; i++) {
		wrmsr(counter_msr + i, 0);
		wrmsr(event_sel_msr + i, ARCH_PERFMON_EVENTSEL_OS |
		      ARCH_PERFMON_EVENTSEL_ENABLE | event_code);

		wrmsr(global_ctrl_msr, BIT_ULL(i));
		__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
		wrmsr(global_ctrl_msr, 0);
		counter = _rdpmc(i);
		GUEST_ASSERT_EQ(this_pmu_has(...), !!counter);

		if ( _rdpmc(i)) {
			wrmsr(global_ctrl_msr, 0);
			wrmsr(counter_msr + i, 0);
			__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
			GUEST_ASSERT(!_rdpmc(i));

			wrmsr(global_ctrl_msr, BIT_ULL(i));
			__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
			GUEST_ASSERT(_rdpmc(i));

			wrmsr(global_ctrl_msr, 0);
			wrmsr(counter_msr + i, (1ULL << gp_counter_bit_width) - 2);
			wrmsr(global_ctrl_msr, BIT_ULL(i));
			__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
			GUEST_ASSERT(rdmsr(global_status_msr) & BIT_ULL(i));

			wrmsr(global_ctrl_msr, 0);
			wrmsr(global_ovf_ctrl_msr, BIT_ULL(i));
			GUEST_ASSERT(!(rdmsr(global_status_msr) & BIT_ULL(i)));
		}
	}

Re: [PATCH v3 04/11] KVM: selftests: Test Intel PMU architectural events on fixed counters

[Sean Christopherson]

On Mon, Aug 14, 2023, Jinrong Liang wrote:
> From: Jinrong Liang <cloudliang@tencent.com>
> 
> Update test to cover Intel PMU architectural events on fixed counters.
> Per Intel SDM, PMU users can also count architecture performance events
> on fixed counters (specifically, FIXED_CTR0 for the retired instructions
> and FIXED_CTR1 for cpu core cycles event). Therefore, if guest's CPUID
> indicates that an architecture event is not available, the corresponding
> fixed counter will also not count that event.
> 
> Co-developed-by: Like Xu <likexu@tencent.com>
> Signed-off-by: Like Xu <likexu@tencent.com>
> Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
> ---
>  .../kvm/x86_64/pmu_basic_functionality_test.c | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> index c04eb0bdf69f..daa45aa285bb 100644
> --- a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> @@ -47,6 +47,7 @@ static uint64_t run_vcpu(struct kvm_vcpu *vcpu, uint64_t *ucall_arg)
>  
>  static void guest_measure_loop(uint64_t event_code)
>  {
> +	uint32_t nr_fixed_counter = this_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS);

There's zero reason to cache this.

>  	uint32_t nr_gp_counters = this_cpu_property(X86_PROPERTY_PMU_NR_GP_COUNTERS);
>  	uint32_t pmu_version = this_cpu_property(X86_PROPERTY_PMU_VERSION);
>  	uint32_t counter_msr;
> @@ -73,6 +74,26 @@ static void guest_measure_loop(uint64_t event_code)
>  		}
>  	}
>  
> +	if (pmu_version < 2 || nr_fixed_counter < 1)

Because you can simply do:

	if (pmu_version < 2 ||
	    this_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS) < 1)
		goto done;
	
> +		goto done;
> +
> +	if (event_code == intel_arch_events[INTEL_ARCH_INSTRUCTIONS_RETIRED])
> +		i = 0;
> +	else if (event_code == intel_arch_events[INTEL_ARCH_CPU_CYCLES])
> +		i = 1;
> +	else
> +		goto done;
> +
> +	wrmsr(MSR_CORE_PERF_FIXED_CTR0 + i, 0);
> +	wrmsr(MSR_CORE_PERF_FIXED_CTR_CTRL, BIT_ULL(4 * i));
> +	wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, BIT_ULL(INTEL_PMC_IDX_FIXED + i));
> +
> +	__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
> +
> +	wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
> +	GUEST_SYNC(_rdpmc(RDPMC_FIXED_BASE | i));
> +
> +done:
>  	GUEST_DONE();
>  }
>  
> -- 
> 2.39.3
> 

Re: [PATCH v3 05/11] KVM: selftests: Test consistency of CPUID with num of gp counters

[Sean Christopherson]

On Mon, Aug 14, 2023, Jinrong Liang wrote:
> From: Jinrong Liang <cloudliang@tencent.com>
> 
> Add test to check if non-existent counters can be accessed in guest after
> determining the number of Intel generic performance counters by CPUID.
> When the num of counters is less than 3, KVM does not emulate #GP if
> a counter isn't present due to compatibility MSR_P6_PERFCTRx handling.
> Nor will the KVM emulate more counters than it can support.
> 
> Co-developed-by: Like Xu <likexu@tencent.com>
> Signed-off-by: Like Xu <likexu@tencent.com>
> Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
> ---
>  .../kvm/x86_64/pmu_basic_functionality_test.c | 78 +++++++++++++++++++
>  1 file changed, 78 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> index daa45aa285bb..b86033e51d5c 100644
> --- a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> @@ -16,6 +16,11 @@
>  /* Guest payload for any performance counter counting */
>  #define NUM_BRANCHES			10
>  
> +static const uint64_t perf_caps[] = {
> +	0,
> +	PMU_CAP_FW_WRITES,
> +};
> +
>  static struct kvm_vm *pmu_vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
>  						  void *guest_code)
>  {
> @@ -164,6 +169,78 @@ static void intel_test_arch_events(void)
>  	}
>  }
>  
> +static void guest_wr_and_rd_msrs(uint32_t base, uint8_t begin, uint8_t offset)
> +{
> +	uint8_t wr_vector, rd_vector;
> +	uint64_t msr_val;
> +	unsigned int i;
> +
> +	for (i = begin; i < begin + offset; i++) {
> +		wr_vector = wrmsr_safe(base + i, 0xffff);
> +		rd_vector = rdmsr_safe(base + i, &msr_val);
> +		if (wr_vector == GP_VECTOR || rd_vector == GP_VECTOR)
> +			GUEST_SYNC(GP_VECTOR);

Rather than pass around the "expected" vector, and shuffle #GP vs. the msr_val
up (which can get false negatives if msr_val == 13), just read
MSR_IA32_PERF_CAPABILITIES from within the guest and GUEST_ASSERT accordingly.

Re: [PATCH v3 06/11] KVM: selftests: Test consistency of CPUID with num of fixed counters

[Sean Christopherson]

On Mon, Aug 14, 2023, Jinrong Liang wrote:
> @@ -239,6 +272,14 @@ static void intel_test_counters_num(void)
>  		if (!perf_caps[i])
>  			test_oob_gp_counter(0, 2, perf_caps[i], 0);
>  	}
> +
> +	for (ecx = 0;
> +	     ecx <= kvm_cpu_property(X86_PROPERTY_PMU_FIXED_COUNTERS_BITMASK) + 1;

This is the perfect time to use a local variable:

	for (ecx = 0; ecx <= fixed_counters_bitmask; ecx++)

> +	     ecx++) {
> +		intel_test_oob_fixed_ctr(0, ecx, GP_VECTOR);
> +		intel_test_oob_fixed_ctr(nr_fixed_counters, ecx, GP_VECTOR);
> +		intel_test_oob_fixed_ctr(nr_fixed_counters + 1, ecx, GP_VECTOR);
> +	}
>  }
>  
>  int main(int argc, char *argv[])
> -- 
> 2.39.3
> 

Re: [PATCH v3 05/11] KVM: selftests: Test consistency of CPUID with num of gp counters

[Sean Christopherson]

On Thu, Aug 17, 2023, Sean Christopherson wrote:
> On Mon, Aug 14, 2023, Jinrong Liang wrote:
> > From: Jinrong Liang <cloudliang@tencent.com>
> > 
> > Add test to check if non-existent counters can be accessed in guest after
> > determining the number of Intel generic performance counters by CPUID.
> > When the num of counters is less than 3, KVM does not emulate #GP if
> > a counter isn't present due to compatibility MSR_P6_PERFCTRx handling.
> > Nor will the KVM emulate more counters than it can support.
> > 
> > Co-developed-by: Like Xu <likexu@tencent.com>
> > Signed-off-by: Like Xu <likexu@tencent.com>
> > Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
> > ---
> >  .../kvm/x86_64/pmu_basic_functionality_test.c | 78 +++++++++++++++++++
> >  1 file changed, 78 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> > index daa45aa285bb..b86033e51d5c 100644
> > --- a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> > +++ b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> > @@ -16,6 +16,11 @@
> >  /* Guest payload for any performance counter counting */
> >  #define NUM_BRANCHES			10
> >  
> > +static const uint64_t perf_caps[] = {
> > +	0,
> > +	PMU_CAP_FW_WRITES,
> > +};
> > +
> >  static struct kvm_vm *pmu_vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
> >  						  void *guest_code)
> >  {
> > @@ -164,6 +169,78 @@ static void intel_test_arch_events(void)
> >  	}
> >  }
> >  
> > +static void guest_wr_and_rd_msrs(uint32_t base, uint8_t begin, uint8_t offset)
> > +{
> > +	uint8_t wr_vector, rd_vector;
> > +	uint64_t msr_val;
> > +	unsigned int i;
> > +
> > +	for (i = begin; i < begin + offset; i++) {
> > +		wr_vector = wrmsr_safe(base + i, 0xffff);
> > +		rd_vector = rdmsr_safe(base + i, &msr_val);

Unless I'm missing something, there is zero reason to pass "base" and "being"
separately, just do the math in the host.  A "base" that isn't actually the base
when viewed without the full context is super confusing.

> > +		if (wr_vector == GP_VECTOR || rd_vector == GP_VECTOR)
> > +			GUEST_SYNC(GP_VECTOR);
> 
> Rather than pass around the "expected" vector, and shuffle #GP vs. the msr_val
> up (which can get false negatives if msr_val == 13), just read
> MSR_IA32_PERF_CAPABILITIES from within the guest and GUEST_ASSERT accordingly.

Ah, you did that so that the fixed counter test can reuse the guest code.  Just
use separate trampolines in the guest, e.g.

static void __guest_wrmsr_rdmsr(uint32_t base, uint8_t nr_msrs, bool expect_gp)
{
	uint64_t msr_val;
	uint8_t vector;
	uint32_t i;

	for (i = base; i < base + nr_msrs; i++) {
		vector = wrmsr_safe(i, 0xffff);
		GUEST_ASSERT(expect_gp ? vector == GP_VECTOR : !vector,
			     "...");

		vector = rdmsr_safe(i, &msr_val);
		GUEST_ASSERT(expect_gp ? vector == GP_VECTOR : !vector,
			     "...");
		if (!expect_gp)
			GUEST_ASSERT_EQ(msr_val, 0);
	}

	GUEST_DONE();
}

static void guest_rd_wr_fixed_counter(uint32_t base, uint8_t nr_msrs)
{
	__guest_wrmsr_rdmsr(base, nr_msrs, true);
}

static void guest_rd_wr_gp_counter(uint32_t base, uint8_t nr_msrs)
{
	uint64_t perf_capabilities = rdmsr();

	__guest_wrmsr_rdmsr(base, nr_msrs, !!perf_capabilities);
}

Re: [PATCH v3 07/11] KVM: selftests: Test Intel supported fixed counters bit mask

[Sean Christopherson]

On Mon, Aug 14, 2023, Jinrong Liang wrote:
> +static void intel_test_fixed_counters(void)
> +{
> +	uint8_t nr_fixed_counters = kvm_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS);
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +	uint32_t ecx;
> +	uint8_t edx;
> +
> +	for (edx = 0; edx <= nr_fixed_counters; edx++) {
> +		/* KVM doesn't emulate more fixed counters than it can support. */
> +		for (ecx = 0; ecx <= (BIT_ULL(nr_fixed_counters) - 1); ecx++) {
> +			vm = pmu_vm_create_with_one_vcpu(&vcpu,
> +							 intel_guest_run_fixed_counters);
> +			test_fixed_counters_setup(vcpu, ecx, edx);
> +			kvm_vm_free(vm);

Same comments as a previous patch, either use a helper or open code, don't mix
both.

> +		}
> +	}
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  	TEST_REQUIRE(get_kvm_param_bool("enable_pmu"));
> @@ -293,6 +352,7 @@ int main(int argc, char *argv[])
>  
>  	intel_test_arch_events();
>  	intel_test_counters_num();
> +	intel_test_fixed_counters();
>  
>  	return 0;
>  }
> -- 
> 2.39.3
> 

Re: [PATCH v3 08/11] KVM: selftests: Test consistency of PMU MSRs with Intel PMU version

[Sean Christopherson]

On Mon, Aug 14, 2023, Jinrong Liang wrote:
> @@ -341,6 +347,66 @@ static void intel_test_fixed_counters(void)
>  	}
>  }
>  
> +static void intel_guest_check_pmu_version(uint8_t version)
> +{
> +	switch (version) {
> +	case 0:
> +		GUEST_SYNC(wrmsr_safe(MSR_INTEL_ARCH_PMU_GPCTR, 0xffffull));
> +	case 1:
> +		GUEST_SYNC(wrmsr_safe(MSR_CORE_PERF_GLOBAL_CTRL, 0x1ull));
> +	case 2:
> +		/*
> +		 * AnyThread Bit is only supported in version 3
> +		 *
> +		 * The strange thing is that when version=0, writing ANY-Any
> +		 * Thread bit (bit 21) in MSR_P6_EVNTSEL0 and MSR_P6_EVNTSEL1
> +		 * will not generate #GP. While writing ANY-Any Thread bit
> +		 * (bit 21) in MSR_P6_EVNTSEL0+x (MAX_GP_CTR_NUM > x > 2) to
> +		 * ANY-Any Thread bit (bit 21) will generate #GP.
> +		 */
> +		if (version == 0)
> +			break;
> +
> +		GUEST_SYNC(wrmsr_safe(MSR_P6_EVNTSEL0,
> +				      ARCH_PERFMON_EVENTSEL_ANY));
> +		break;
> +	default:
> +		/* KVM currently supports up to pmu version 2 */
> +		GUEST_SYNC(GP_VECTOR);

This seems largely pointless, but I suppose it doesn't hurt anything.

> +	}
> +
> +	GUEST_DONE();
> +}
> +
> +static void test_pmu_version_setup(struct kvm_vcpu *vcpu, uint8_t version,
> +				   uint64_t expected)
> +{
> +	uint64_t msr_val = 0;
> +
> +	vcpu_set_cpuid_property(vcpu, X86_PROPERTY_PMU_VERSION, version);
> +
> +	vcpu_args_set(vcpu, 1, version);
> +	while (run_vcpu(vcpu, &msr_val) != UCALL_DONE)
> +		TEST_ASSERT_EQ(expected, msr_val);
> +}
> +
> +static void intel_test_pmu_version(void)
> +{
> +	uint8_t unsupported_version = kvm_cpu_property(X86_PROPERTY_PMU_VERSION) + 1;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +	uint8_t version;
> +
> +	TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS) > 2);
> +
> +	for (version = 0; version <= unsupported_version; version++) {
> +		vm = pmu_vm_create_with_one_vcpu(&vcpu,
> +						 intel_guest_check_pmu_version);
> +		test_pmu_version_setup(vcpu, version, GP_VECTOR);

Why pass GP_VECTOR?  It's the _only_ expected result, just have the guest assert
that it got a #GP...

> +		kvm_vm_free(vm);

Again, stop making half-baked helpers.

> +	}
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  	TEST_REQUIRE(get_kvm_param_bool("enable_pmu"));
> @@ -353,6 +419,7 @@ int main(int argc, char *argv[])
>  	intel_test_arch_events();
>  	intel_test_counters_num();
>  	intel_test_fixed_counters();
> +	intel_test_pmu_version();
>  
>  	return 0;
>  }
> -- 
> 2.39.3
> 

Re: [PATCH v3 09/11] KVM: selftests: Add x86 feature and properties for AMD PMU in processor.h

[Sean Christopherson]

On Mon, Aug 14, 2023, Jinrong Liang wrote:
> From: Jinrong Liang <cloudliang@tencent.com>
> 
> Add x86 feature and properties for AMD PMU so that tests don't have
> to manually retrieve the correct CPUID leaf+register, and so that the
> resulting code is self-documenting.
> 
> Signed-off-by: Jinrong Liang <cloudliang@tencent.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 6b146e1c6736..07b980b8bec2 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -167,6 +167,7 @@ struct kvm_x86_cpu_feature {
>   */
>  #define	X86_FEATURE_SVM			KVM_X86_CPU_FEATURE(0x80000001, 0, ECX, 2)
>  #define	X86_FEATURE_NX			KVM_X86_CPU_FEATURE(0x80000001, 0, EDX, 20)
> +#define	X86_FEATURE_AMD_PMU_EXT_CORE	KVM_X86_CPU_FEATURE(0x80000001, 0, ECX, 23)

I think it makes sense to follow the kernel, even though I find the kernel name
a bit funky in this case.  I.e. X86_FEATURE_PERFCTR_CORE

>  #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)
> @@ -182,6 +183,9 @@ 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_AMD_PERFMON_V2	KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 0)

X86_FEATURE_PERFMON_V2

> +#define X86_FEATURE_AMD_LBR_STACK	KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 1)

Eh, I'd drop the "AMD".

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

Same here.

>  /*
>   * KVM defined paravirt features.
> @@ -267,6 +271,9 @@ struct kvm_x86_cpu_property {
>  #define X86_PROPERTY_MAX_VIRT_ADDR		KVM_X86_CPU_PROPERTY(0x80000008, 0, EAX, 8, 15)
>  #define X86_PROPERTY_PHYS_ADDR_REDUCTION	KVM_X86_CPU_PROPERTY(0x8000001F, 0, EBX, 6, 11)
>  
> +#define X86_PROPERTY_AMD_PMU_NR_CORE_COUNTERS	KVM_X86_CPU_PROPERTY(0x80000022, 0, EBX, 0, 3)
> +#define X86_PROPERTY_AMD_PMU_LBR_STACK_SIZE	KVM_X86_CPU_PROPERTY(0x80000022, 0, EBX, 4, 9)

And here.  I think we can get away with "GP and fixed counters means Intel, core
counters means AMD", at least as far as X86_FEATURES go.  LBRs might be a different
story, but so long as there's no conflict....

Splattering AMD everywhere but not Intel everywhere bugs me :-)

> +
>  #define X86_PROPERTY_MAX_CENTAUR_LEAF		KVM_X86_CPU_PROPERTY(0xC0000000, 0, EAX, 0, 31)
>  
>  /*
> -- 
> 2.39.3
> 

Re: [PATCH v3 02/11] KVM: selftests: Add pmu.h for PMU events and common masks

[Like Xu]

On 14/8/2023 7:50 pm, Jinrong Liang wrote:
> +#define ARCH_PERFMON_EVENTSEL_EDGE		BIT_ULL(18)
> +#define ARCH_PERFMON_EVENTSEL_PIN_CONTROL	BIT_ULL(19)
> +#define ARCH_PERFMON_EVENTSEL_INT		BIT_ULL(20)
> +#define ARCH_PERFMON_EVENTSEL_ANY		BIT_ULL(21)
> +#define ARCH_PERFMON_EVENTSEL_ENABLE		BIT_ULL(22)
> +#define ARCH_PERFMON_EVENTSEL_INV		BIT_ULL(23)
> +#define ARCH_PERFMON_EVENTSEL_CMASK		GENMASK_ULL(31, 24)

Could you write more test cases to cover all EVENTSEL bits including ENABLE bit ?

Re: [PATCH v3 02/11] KVM: selftests: Add pmu.h for PMU events and common masks

[Jinrong Liang]

Like Xu <like.xu.linux@gmail.com> 于2023年8月21日周一 16:56写道：
>
> On 14/8/2023 7:50 pm, Jinrong Liang wrote:
> > +#define ARCH_PERFMON_EVENTSEL_EDGE           BIT_ULL(18)
> > +#define ARCH_PERFMON_EVENTSEL_PIN_CONTROL    BIT_ULL(19)
> > +#define ARCH_PERFMON_EVENTSEL_INT            BIT_ULL(20)
> > +#define ARCH_PERFMON_EVENTSEL_ANY            BIT_ULL(21)
> > +#define ARCH_PERFMON_EVENTSEL_ENABLE         BIT_ULL(22)
> > +#define ARCH_PERFMON_EVENTSEL_INV            BIT_ULL(23)
> > +#define ARCH_PERFMON_EVENTSEL_CMASK          GENMASK_ULL(31, 24)
>
> Could you write more test cases to cover all EVENTSEL bits including ENABLE bit ?

I am more than willing to write additional test cases to cover all
EVENTSEL bits, including the ENABLE bit.

If you have any specific suggestions or scenarios you'd like me to
cover in the new test cases, please feel free to share them. I am open
to any ideas that can further improve the coverage and quality of our
selftests.

Thanks

Re: [PATCH v3 03/11] KVM: selftests: Test Intel PMU architectural events on gp counters

[Jinrong Liang]

Sean Christopherson <seanjc@google.com> 于2023年8月18日周五 06:54写道：
>
> On Mon, Aug 14, 2023, Jinrong Liang wrote:
> > Add test case for AMD Guest PerfMonV2. Also test Intel
> > MSR_CORE_PERF_GLOBAL_STATUS and MSR_CORE_PERF_GLOBAL_OVF_CTRL.
> >
> > Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
> > ---
> >  .../kvm/x86_64/pmu_basic_functionality_test.c | 48 ++++++++++++++++++-
> >  1 file changed, 46 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> > index cb2a7ad5c504..02bd1fe3900b 100644
> > --- a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> > +++ b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> > @@ -58,7 +58,9 @@ static uint64_t run_vcpu(struct kvm_vcpu *vcpu, uint64_t *ucall_arg)
> >
> >  static void guest_measure_loop(uint64_t event_code)
> >  {
> > +     uint64_t global_ovf_ctrl_msr, global_status_msr, global_ctrl_msr;
> >       uint8_t nr_gp_counters, pmu_version = 1;
> > +     uint8_t gp_counter_bit_width = 48;
> >       uint64_t event_sel_msr;
> >       uint32_t counter_msr;
> >       unsigned int i;
> > @@ -68,6 +70,12 @@ static void guest_measure_loop(uint64_t event_code)
> >               pmu_version = this_cpu_property(X86_PROPERTY_PMU_VERSION);
> >               event_sel_msr = MSR_P6_EVNTSEL0;
> >
> > +             if (pmu_version > 1) {
> > +                     global_ovf_ctrl_msr = MSR_CORE_PERF_GLOBAL_OVF_CTRL;
> > +                     global_status_msr = MSR_CORE_PERF_GLOBAL_STATUS;
> > +                     global_ctrl_msr = MSR_CORE_PERF_GLOBAL_CTRL;
> > +             }
> > +
> >               if (rdmsr(MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES)
> >                       counter_msr = MSR_IA32_PMC0;
> >               else
> > @@ -76,6 +84,17 @@ static void guest_measure_loop(uint64_t event_code)
> >               nr_gp_counters = AMD64_NR_COUNTERS;
> >               event_sel_msr = MSR_K7_EVNTSEL0;
> >               counter_msr = MSR_K7_PERFCTR0;
> > +
> > +             if (this_cpu_has(X86_FEATURE_AMD_PMU_EXT_CORE) &&
> > +                 this_cpu_has(X86_FEATURE_AMD_PERFMON_V2)) {
> > +                     nr_gp_counters = this_cpu_property(X86_PROPERTY_AMD_PMU_NR_CORE_COUNTERS);
> > +                     global_ovf_ctrl_msr = MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR;
> > +                     global_status_msr = MSR_AMD64_PERF_CNTR_GLOBAL_STATUS;
> > +                     global_ctrl_msr = MSR_AMD64_PERF_CNTR_GLOBAL_CTL;
> > +                     event_sel_msr = MSR_F15H_PERF_CTL0;
> > +                     counter_msr = MSR_F15H_PERF_CTR0;
> > +                     pmu_version = 2;
> > +             }
>
> Please use an if-else when the two things are completely exclusive, i.e. don't
> set "defaults" and then override them.
>
> >       }
> >
> >       for (i = 0; i < nr_gp_counters; i++) {
> > @@ -84,14 +103,39 @@ static void guest_measure_loop(uint64_t event_code)
> >                     ARCH_PERFMON_EVENTSEL_ENABLE | event_code);
> >
> >               if (pmu_version > 1) {
> > -                     wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, BIT_ULL(i));
> > +                     wrmsr(global_ctrl_msr, BIT_ULL(i));
> >                       __asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
> > -                     wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
> > +                     wrmsr(global_ctrl_msr, 0);
> >                       GUEST_SYNC(_rdpmc(i));
> >               } else {
> >                       __asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
> >                       GUEST_SYNC(_rdpmc(i));
> >               }
>
> This is extremely difficult to follow.  I think the same thing to do is to split
> this up into helpers, e.g. send pmu_version > 1 into one path, and pmu_version <= 1
> into an entirely different path.
>
> E.g. something like this?

I agree with all the proposed code changes you have provided. Your
comments have been incredibly helpful in making the necessary
improvements to the code. I will diligently follow your suggestions
and modify the code accordingly.

>
> static void guest_measure_loop(uint64_t event_code)
> {
>         uint64_t global_ovf_ctrl_msr, global_status_msr, global_ctrl_msr;
>         uint8_t nr_gp_counters, pmu_version;
>         uint8_t gp_counter_bit_width;
>         uint64_t event_sel_msr;
>         uint32_t counter_msr;
>         unsigned int i;
>
>         if (host_cpu_is_intel)
>                 pmu_version = this_cpu_property(X86_PROPERTY_PMU_VERSION);
>         else if (this_cpu_has(X86_FEATURE_PERFCTR_CORE) &&
>                  this_cpu_has(X86_FEATURE_PERFMON_V2)) {
>                 pmu_version = 2;
>         } else {
>                 pmu_version = 1;
>         }
>
>         if (pmu_version <= 1) {
>                 guest_measure_pmu_legacy(...);
>                 return;
>         }
>
>         if (host_cpu_is_intel) {
>                 nr_gp_counters = this_cpu_property(X86_PROPERTY_PMU_NR_GP_COUNTERS);
>                 global_ovf_ctrl_msr = MSR_CORE_PERF_GLOBAL_OVF_CTRL;
>                 global_status_msr = MSR_CORE_PERF_GLOBAL_STATUS;
>                 global_ctrl_msr = MSR_CORE_PERF_GLOBAL_CTRL;
>                 gp_counter_bit_width = this_cpu_property(X86_PROPERTY_PMU_GP_COUNTERS_BIT_WIDTH);
>
>                 if (rdmsr(MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES)
>                         counter_msr = MSR_IA32_PMC0;
>                 else
>                         counter_msr = MSR_IA32_PERFCTR0;
>         } else {
>                 nr_gp_counters = this_cpu_property(X86_PROPERTY_AMD_PMU_NR_CORE_COUNTERS);
>                 global_ovf_ctrl_msr = MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR;
>                 global_status_msr = MSR_AMD64_PERF_CNTR_GLOBAL_STATUS;
>                 global_ctrl_msr = MSR_AMD64_PERF_CNTR_GLOBAL_CTL;
>                 event_sel_msr = MSR_F15H_PERF_CTL0;
>                 counter_msr = MSR_F15H_PERF_CTR0;
>                 gp_counter_bit_width = 48;
>         }
>
>         for (i = 0; i < nr_gp_counters; i++) {
>                 wrmsr(counter_msr + i, 0);
>                 wrmsr(event_sel_msr + i, ARCH_PERFMON_EVENTSEL_OS |
>                       ARCH_PERFMON_EVENTSEL_ENABLE | event_code);
>
>                 wrmsr(global_ctrl_msr, BIT_ULL(i));
>                 __asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
>                 wrmsr(global_ctrl_msr, 0);
>                 counter = _rdpmc(i);
>                 GUEST_ASSERT_EQ(this_pmu_has(...), !!counter);
>
>                 if ( _rdpmc(i)) {
>                         wrmsr(global_ctrl_msr, 0);
>                         wrmsr(counter_msr + i, 0);
>                         __asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
>                         GUEST_ASSERT(!_rdpmc(i));
>
>                         wrmsr(global_ctrl_msr, BIT_ULL(i));
>                         __asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
>                         GUEST_ASSERT(_rdpmc(i));
>
>                         wrmsr(global_ctrl_msr, 0);
>                         wrmsr(counter_msr + i, (1ULL << gp_counter_bit_width) - 2);
>                         wrmsr(global_ctrl_msr, BIT_ULL(i));
>                         __asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
>                         GUEST_ASSERT(rdmsr(global_status_msr) & BIT_ULL(i));
>
>                         wrmsr(global_ctrl_msr, 0);
>                         wrmsr(global_ovf_ctrl_msr, BIT_ULL(i));
>                         GUEST_ASSERT(!(rdmsr(global_status_msr) & BIT_ULL(i)));
>                 }
>         }
>

I truly appreciate your time and effort in reviewing the code and
providing such valuable feedback. Please feel free to share any
further suggestions or ideas in the future.