* [PATCH 1/7] KVM: selftests: Test Intel PMU architectural events on gp counters
2023-03-23 7:27 [PATCH 0/7] KVM: selftests: Test the consistency of the PMU's CPUID and its features Like Xu
@ 2023-03-23 7:27 ` Like Xu
2023-05-24 22:32 ` Sean Christopherson
2023-05-24 22:59 ` Jim Mattson
2023-03-23 7:27 ` [PATCH 2/7] KVM: selftests: Test Intel PMU architectural events on fixed counters Like Xu
` (6 subsequent siblings)
7 siblings, 2 replies; 16+ messages in thread
From: Like Xu @ 2023-03-23 7:27 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, Jinrong Liang, linux-kernel, kvm
From: Like Xu <likexu@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: Jinrong Liang <cloudliang@tencent.com>
Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
Signed-off-by: Like Xu <likexu@tencent.com>
---
tools/testing/selftests/kvm/Makefile | 1 +
.../selftests/kvm/x86_64/pmu_cpuid_test.c | 202 ++++++++++++++++++
2 files changed, 203 insertions(+)
create mode 100644 tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 84a627c43795..8aa63081b3e6 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -78,6 +78,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_cpuid_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_cpuid_test.c b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
new file mode 100644
index 000000000000..faab0a91e191
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
@@ -0,0 +1,202 @@
+// 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 "processor.h"
+
+/* Guest payload for any performance counter counting */
+#define NUM_BRANCHES 10
+
+#define EVENTSEL_OS BIT_ULL(17)
+#define EVENTSEL_EN BIT_ULL(22)
+#define PMU_CAP_FW_WRITES BIT_ULL(13)
+#define EVENTS_MASK GENMASK_ULL(7, 0)
+#define PMU_VERSION_MASK GENMASK_ULL(7, 0)
+#define GP_CTR_NUM_OFS_BIT 8
+#define GP_CTR_NUM_MASK GENMASK_ULL(15, GP_CTR_NUM_OFS_BIT)
+#define EVT_LEN_OFS_BIT 24
+#define EVT_LEN_MASK GENMASK_ULL(31, EVT_LEN_OFS_BIT)
+
+#define ARCH_EVENT(select, umask) (((select) & 0xff) | ((umask) & 0xff) << 8)
+
+/*
+ * Intel Pre-defined Architectural Performance Events. Note some events
+ * are skipped for testing due to difficulties in stable reproduction.
+ */
+static const uint64_t arch_events[] = {
+ [0] = ARCH_EVENT(0x3c, 0x0),
+ [1] = ARCH_EVENT(0xc0, 0x0),
+ [2] = ARCH_EVENT(0x3c, 0x1),
+ [3] = ARCH_EVENT(0x2e, 0x4f), /* LLC Reference */
+ [4] = ARCH_EVENT(0x2e, 0x41), /* LLC Misses */
+ [5] = ARCH_EVENT(0xc4, 0x0),
+ [6] = ARCH_EVENT(0xc5, 0x0), /* Branch Misses Retired */
+ [7] = ARCH_EVENT(0xa4, 0x1), /* Topdown Slots */
+};
+
+static struct kvm_vcpu *new_vcpu(void *guest_code)
+{
+ struct kvm_vm *vm;
+ struct kvm_vcpu *vcpu;
+
+ vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+ vm_init_descriptor_tables(vm);
+ vcpu_init_descriptor_tables(vcpu);
+
+ return vcpu;
+}
+
+static void free_vcpu(struct kvm_vcpu *vcpu)
+{
+ kvm_vm_free(vcpu->vm);
+}
+
+static void run_vcpu(struct kvm_vcpu *vcpu, const char *msg,
+ bool (*check_ucall)(struct ucall *uc, void *data),
+ void *expect_args)
+{
+ struct ucall uc;
+
+ for (;;) {
+ vcpu_run(vcpu);
+ switch (get_ucall(vcpu, &uc)) {
+ case UCALL_SYNC:
+ TEST_ASSERT(check_ucall(&uc, expect_args), "%s", msg);
+ continue;
+ case UCALL_DONE:
+ break;
+ default:
+ TEST_ASSERT(false, "Unexpected exit: %s",
+ exit_reason_str(vcpu->run->exit_reason));
+ }
+ break;
+ }
+}
+
+static bool first_uc_arg_non_zero(struct ucall *uc, void *data)
+{
+ return uc->args[1];
+}
+
+static void intel_guest_run_arch_event(uint8_t version, uint8_t max_gp_num,
+ bool supported, uint32_t ctr_base_msr,
+ uint64_t evt_code)
+{
+ uint32_t global_msr = MSR_CORE_PERF_GLOBAL_CTRL;
+ unsigned int i;
+
+ for (i = 0; i < max_gp_num; i++) {
+ wrmsr(ctr_base_msr + i, 0);
+ wrmsr(MSR_P6_EVNTSEL0 + i, EVENTSEL_OS | EVENTSEL_EN | evt_code);
+ if (version > 1)
+ wrmsr(global_msr, BIT_ULL(i));
+
+ __asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
+
+ if (version > 1)
+ wrmsr(global_msr, 0);
+
+ GUEST_SYNC(supported == !!_rdpmc(i));
+ }
+
+ GUEST_DONE();
+}
+
+static void test_arch_events_setup(struct kvm_vcpu *vcpu, uint8_t evt_vector,
+ uint8_t unavl_mask, uint8_t idx)
+{
+ struct kvm_cpuid_entry2 *entry;
+ uint32_t ctr_msr = MSR_IA32_PERFCTR0;
+ bool is_supported;
+
+ entry = vcpu_get_cpuid_entry(vcpu, 0xa);
+ entry->eax = (entry->eax & ~EVT_LEN_MASK) |
+ (evt_vector << EVT_LEN_OFS_BIT);
+ entry->ebx = (entry->ebx & ~EVENTS_MASK) | unavl_mask;
+ vcpu_set_cpuid(vcpu);
+
+ if (vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES)
+ ctr_msr = MSR_IA32_PMC0;
+
+ /* Arch event x is supported if EBX[x]=0 && EAX[31:24]>x */
+ is_supported = !(entry->ebx & BIT_ULL(idx)) &&
+ (((entry->eax & EVT_LEN_MASK) >> EVT_LEN_OFS_BIT) > idx);
+
+ vcpu_args_set(vcpu, 5, entry->eax & PMU_VERSION_MASK,
+ (entry->eax & GP_CTR_NUM_MASK) >> GP_CTR_NUM_OFS_BIT,
+ is_supported, ctr_msr, arch_events[idx]);
+}
+
+static void intel_check_arch_event_is_unavl(uint8_t idx)
+{
+ const char *msg = "Unavailable arch event is counting.";
+ uint8_t eax_evt_vec, ebx_unavl_mask, i, j;
+ struct kvm_vcpu *vcpu;
+
+ /*
+ * 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(arch_events); i++) {
+ eax_evt_vec = BIT_ULL(i);
+ for (j = 0; j < ARRAY_SIZE(arch_events); j++) {
+ ebx_unavl_mask = BIT_ULL(j);
+
+ vcpu = new_vcpu(intel_guest_run_arch_event);
+ test_arch_events_setup(vcpu, eax_evt_vec,
+ ebx_unavl_mask, idx);
+ run_vcpu(vcpu, msg, first_uc_arg_non_zero, NULL);
+ free_vcpu(vcpu);
+ }
+ }
+}
+
+static void intel_test_arch_events(void)
+{
+ uint8_t idx;
+
+ for (idx = 0; idx < ARRAY_SIZE(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 > 2 && idx != 5)
+ continue;
+
+ intel_check_arch_event_is_unavl(idx);
+ }
+}
+
+static void intel_test_pmu_cpuid(void)
+{
+ intel_test_arch_events();
+}
+
+int main(int argc, char *argv[])
+{
+ TEST_REQUIRE(get_kvm_param_bool("enable_pmu"));
+
+ 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_pmu_cpuid();
+ }
+
+ return 0;
+}
--
2.40.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 1/7] KVM: selftests: Test Intel PMU architectural events on gp counters
2023-03-23 7:27 ` [PATCH 1/7] KVM: selftests: Test Intel PMU architectural events on gp counters Like Xu
@ 2023-05-24 22:32 ` Sean Christopherson
2023-05-24 22:59 ` Jim Mattson
1 sibling, 0 replies; 16+ messages in thread
From: Sean Christopherson @ 2023-05-24 22:32 UTC (permalink / raw)
To: Like Xu; +Cc: Paolo Bonzini, Jinrong Liang, linux-kernel, kvm
On Thu, Mar 23, 2023, Like Xu wrote:
> From: Like Xu <likexu@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: Jinrong Liang <cloudliang@tencent.com>
> Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
> Signed-off-by: Like Xu <likexu@tencent.com>
> ---
> tools/testing/selftests/kvm/Makefile | 1 +
> .../selftests/kvm/x86_64/pmu_cpuid_test.c | 202 ++++++++++++++++++
"cpuid" is a rather confusing name, e.g. I was half expecting tests where the guest
is executing CPUID. IIUC, this is really just a basic functionality test, for a
somewhat loose definition of "basic".
Maybe something like pmu_basic_functionality_test? Or pmu_functional_test?
> 2 files changed, 203 insertions(+)
> create mode 100644 tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
...
> +#define EVENTSEL_OS BIT_ULL(17)
> +#define EVENTSEL_EN BIT_ULL(22)
> +#define PMU_CAP_FW_WRITES BIT_ULL(13)
> +#define EVENTS_MASK GENMASK_ULL(7, 0)
> +#define PMU_VERSION_MASK GENMASK_ULL(7, 0)
> +#define GP_CTR_NUM_OFS_BIT 8
> +#define GP_CTR_NUM_MASK GENMASK_ULL(15, GP_CTR_NUM_OFS_BIT)
> +#define EVT_LEN_OFS_BIT 24
> +#define EVT_LEN_MASK GENMASK_ULL(31, EVT_LEN_OFS_BIT)
> +
> +#define ARCH_EVENT(select, umask) (((select) & 0xff) | ((umask) & 0xff) << 8)
> +
> +/*
> + * Intel Pre-defined Architectural Performance Events. Note some events
> + * are skipped for testing due to difficulties in stable reproduction.
> + */
> +static const uint64_t arch_events[] = {
> + [0] = ARCH_EVENT(0x3c, 0x0),
> + [1] = ARCH_EVENT(0xc0, 0x0),
> + [2] = ARCH_EVENT(0x3c, 0x1),
> + [3] = ARCH_EVENT(0x2e, 0x4f), /* LLC Reference */
> + [4] = ARCH_EVENT(0x2e, 0x41), /* LLC Misses */
> + [5] = ARCH_EVENT(0xc4, 0x0),
> + [6] = ARCH_EVENT(0xc5, 0x0), /* Branch Misses Retired */
> + [7] = ARCH_EVENT(0xa4, 0x1), /* Topdown Slots */
> +};
All of these definitions belong in library code. Probably something like
tools/testing/selftests/kvm/include/x86_64/pmu.h, or maybe intel_pmu.h?
> +static struct kvm_vcpu *new_vcpu(void *guest_code)
Assuming the next version is sent before the aforementioned cleanup, how about
defining this as
static struct kvm_vm *pmu_vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
void *guest_code)
so that the cleanup can be a more straightforward replacement. That should also
make the free_vcpu() wrapper completely unnecessary.
> +{
> + struct kvm_vm *vm;
> + struct kvm_vcpu *vcpu;
> +
> + vm = vm_create_with_one_vcpu(&vcpu, guest_code);
> + vm_init_descriptor_tables(vm);
> + vcpu_init_descriptor_tables(vcpu);
Ugh, I really need to put together that series to clean up the descriptor table
mess.
https://lore.kernel.org/all/Y8hCBOndYMD9zsDL@google.com
> +
> + return vcpu;
> +}
> +
> +static void free_vcpu(struct kvm_vcpu *vcpu)
> +{
> + kvm_vm_free(vcpu->vm);
Just call kvm_vm_free(). Adding layers of unnecessary helpers just to provide
symmetry is not a good tradeoff. As above, tweak the name/prototyp of new_vcpu()
if you really want something approaching symmetry.
> +static void run_vcpu(struct kvm_vcpu *vcpu, const char *msg,
> + bool (*check_ucall)(struct ucall *uc, void *data),
> + void *expect_args)
> +{
> + struct ucall uc;
> +
> + for (;;) {
> + vcpu_run(vcpu);
> + switch (get_ucall(vcpu, &uc)) {
> + case UCALL_SYNC:
> + TEST_ASSERT(check_ucall(&uc, expect_args), "%s", msg);
A callback is both overkill and debug hostile. If the assert fails, it will report
this common code and not the caller. Since all uses grab the first ucall param,
just "return" that from the run_vcpu() wrapper as out param. Callers will need
to define their own loop, but that's one line of code, and it has the advantage
of making the code much more self-documenting
E.g.
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_ASSERT(false, "Unexpected exit: %s",
exit_reason_str(vcpu->run->exit_reason));
}
return uc.cmd;
}
and in intel_test_fixed_counters() as an example:
while (run_vcpu(vcpu, &val) != UCALL_DONE)
TEST_ASSERT(val, "One or more fixed counters not counting");
> +static void test_arch_events_setup(struct kvm_vcpu *vcpu, uint8_t evt_vector,
> + uint8_t unavl_mask, uint8_t idx)
> +{
> + struct kvm_cpuid_entry2 *entry;
> + uint32_t ctr_msr = MSR_IA32_PERFCTR0;
> + bool is_supported;
> +
> + entry = vcpu_get_cpuid_entry(vcpu, 0xa);
> + entry->eax = (entry->eax & ~EVT_LEN_MASK) |
> + (evt_vector << EVT_LEN_OFS_BIT);
> + entry->ebx = (entry->ebx & ~EVENTS_MASK) | unavl_mask;
> + vcpu_set_cpuid(vcpu);
> +
> + if (vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES)
> + ctr_msr = MSR_IA32_PMC0;
> +
> + /* Arch event x is supported if EBX[x]=0 && EAX[31:24]>x */
> + is_supported = !(entry->ebx & BIT_ULL(idx)) &&
> + (((entry->eax & EVT_LEN_MASK) >> EVT_LEN_OFS_BIT) > idx);
> +
> + vcpu_args_set(vcpu, 5, entry->eax & PMU_VERSION_MASK,
> + (entry->eax & GP_CTR_NUM_MASK) >> GP_CTR_NUM_OFS_BIT,
> + is_supported, ctr_msr, arch_events[idx]);
Why pass all this stuff in as params? Wouldn't it be easier to query CPUID from
the guest side? And define KVM_X86_CPU_PROPERTY macros where possible to avoid
having to open code things in multiple places.
> +static void intel_check_arch_event_is_unavl(uint8_t idx)
> +{
> + const char *msg = "Unavailable arch event is counting.";
> + uint8_t eax_evt_vec, ebx_unavl_mask, i, j;
> + struct kvm_vcpu *vcpu;
> +
> + /*
> + * 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(arch_events); i++) {
> + eax_evt_vec = BIT_ULL(i);
> + for (j = 0; j < ARRAY_SIZE(arch_events); j++) {
> + ebx_unavl_mask = BIT_ULL(j);
> +
> + vcpu = new_vcpu(intel_guest_run_arch_event);
> + test_arch_events_setup(vcpu, eax_evt_vec,
> + ebx_unavl_mask, idx);
> + run_vcpu(vcpu, msg, first_uc_arg_non_zero, NULL);
Oof, this is super confusing. At first, second, and third glances, it looks like
this is asserting that the counter _is_ counting. It took me a while to find this
code in the guest:
GUEST_SYNC(supported == !!_rdpmc(RDPMC_FIXED_BASE | i));
Either have the guest do GUEST_ASSERT() directly and process the output in the
common run_vcpu(), or just have the guest return the counter value and do the
assert here. My vote would be the latter, e.g.
while (run_vcpu(vcpu, &counter_val) != UCALL_DONE)
TEST_ASSERT(is_supported == !!counter_val,
"blah blah blah");
That'd also likely avoid having to plumb the "supported" info into the guest.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/7] KVM: selftests: Test Intel PMU architectural events on gp counters
2023-03-23 7:27 ` [PATCH 1/7] KVM: selftests: Test Intel PMU architectural events on gp counters Like Xu
2023-05-24 22:32 ` Sean Christopherson
@ 2023-05-24 22:59 ` Jim Mattson
1 sibling, 0 replies; 16+ messages in thread
From: Jim Mattson @ 2023-05-24 22:59 UTC (permalink / raw)
To: Like Xu
Cc: Sean Christopherson, Paolo Bonzini, Jinrong Liang, linux-kernel,
kvm
On Thu, Mar 23, 2023 at 12:27 AM Like Xu <like.xu.linux@gmail.com> wrote:
>
> From: Like Xu <likexu@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: Jinrong Liang <cloudliang@tencent.com>
> Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
> Signed-off-by: Like Xu <likexu@tencent.com>
> ---
> tools/testing/selftests/kvm/Makefile | 1 +
> .../selftests/kvm/x86_64/pmu_cpuid_test.c | 202 ++++++++++++++++++
> 2 files changed, 203 insertions(+)
> create mode 100644 tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
>
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 84a627c43795..8aa63081b3e6 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -78,6 +78,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_cpuid_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_cpuid_test.c b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
> new file mode 100644
> index 000000000000..faab0a91e191
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
> @@ -0,0 +1,202 @@
> +// 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 "processor.h"
> +
> +/* Guest payload for any performance counter counting */
> +#define NUM_BRANCHES 10
> +
> +#define EVENTSEL_OS BIT_ULL(17)
> +#define EVENTSEL_EN BIT_ULL(22)
> +#define PMU_CAP_FW_WRITES BIT_ULL(13)
> +#define EVENTS_MASK GENMASK_ULL(7, 0)
> +#define PMU_VERSION_MASK GENMASK_ULL(7, 0)
> +#define GP_CTR_NUM_OFS_BIT 8
> +#define GP_CTR_NUM_MASK GENMASK_ULL(15, GP_CTR_NUM_OFS_BIT)
> +#define EVT_LEN_OFS_BIT 24
> +#define EVT_LEN_MASK GENMASK_ULL(31, EVT_LEN_OFS_BIT)
> +
> +#define ARCH_EVENT(select, umask) (((select) & 0xff) | ((umask) & 0xff) << 8)
> +
> +/*
> + * Intel Pre-defined Architectural Performance Events. Note some events
> + * are skipped for testing due to difficulties in stable reproduction.
> + */
> +static const uint64_t arch_events[] = {
> + [0] = ARCH_EVENT(0x3c, 0x0),
> + [1] = ARCH_EVENT(0xc0, 0x0),
> + [2] = ARCH_EVENT(0x3c, 0x1),
> + [3] = ARCH_EVENT(0x2e, 0x4f), /* LLC Reference */
> + [4] = ARCH_EVENT(0x2e, 0x41), /* LLC Misses */
> + [5] = ARCH_EVENT(0xc4, 0x0),
> + [6] = ARCH_EVENT(0xc5, 0x0), /* Branch Misses Retired */
> + [7] = ARCH_EVENT(0xa4, 0x1), /* Topdown Slots */
> +};
> +
> +static struct kvm_vcpu *new_vcpu(void *guest_code)
> +{
> + struct kvm_vm *vm;
> + struct kvm_vcpu *vcpu;
> +
> + vm = vm_create_with_one_vcpu(&vcpu, guest_code);
> + vm_init_descriptor_tables(vm);
> + vcpu_init_descriptor_tables(vcpu);
> +
> + return vcpu;
> +}
> +
> +static void free_vcpu(struct kvm_vcpu *vcpu)
> +{
> + kvm_vm_free(vcpu->vm);
> +}
> +
> +static void run_vcpu(struct kvm_vcpu *vcpu, const char *msg,
> + bool (*check_ucall)(struct ucall *uc, void *data),
> + void *expect_args)
> +{
> + struct ucall uc;
> +
> + for (;;) {
> + vcpu_run(vcpu);
> + switch (get_ucall(vcpu, &uc)) {
> + case UCALL_SYNC:
> + TEST_ASSERT(check_ucall(&uc, expect_args), "%s", msg);
> + continue;
> + case UCALL_DONE:
> + break;
> + default:
> + TEST_ASSERT(false, "Unexpected exit: %s",
> + exit_reason_str(vcpu->run->exit_reason));
> + }
> + break;
> + }
> +}
> +
> +static bool first_uc_arg_non_zero(struct ucall *uc, void *data)
> +{
> + return uc->args[1];
> +}
> +
> +static void intel_guest_run_arch_event(uint8_t version, uint8_t max_gp_num,
> + bool supported, uint32_t ctr_base_msr,
> + uint64_t evt_code)
> +{
> + uint32_t global_msr = MSR_CORE_PERF_GLOBAL_CTRL;
> + unsigned int i;
> +
> + for (i = 0; i < max_gp_num; i++) {
> + wrmsr(ctr_base_msr + i, 0);
> + wrmsr(MSR_P6_EVNTSEL0 + i, EVENTSEL_OS | EVENTSEL_EN | evt_code);
> + if (version > 1)
> + wrmsr(global_msr, BIT_ULL(i));
> +
> + __asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
> +
> + if (version > 1)
> + wrmsr(global_msr, 0);
> +
> + GUEST_SYNC(supported == !!_rdpmc(i));
Just because the counter is non-zero doesn't mean that the event is
supported. "Supported" would imply that it actually works, doesn't
it?
> + }
> +
> + GUEST_DONE();
> +}
> +
> +static void test_arch_events_setup(struct kvm_vcpu *vcpu, uint8_t evt_vector,
> + uint8_t unavl_mask, uint8_t idx)
> +{
> + struct kvm_cpuid_entry2 *entry;
> + uint32_t ctr_msr = MSR_IA32_PERFCTR0;
> + bool is_supported;
> +
> + entry = vcpu_get_cpuid_entry(vcpu, 0xa);
> + entry->eax = (entry->eax & ~EVT_LEN_MASK) |
> + (evt_vector << EVT_LEN_OFS_BIT);
> + entry->ebx = (entry->ebx & ~EVENTS_MASK) | unavl_mask;
> + vcpu_set_cpuid(vcpu);
> +
> + if (vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES)
> + ctr_msr = MSR_IA32_PMC0;
> +
> + /* Arch event x is supported if EBX[x]=0 && EAX[31:24]>x */
> + is_supported = !(entry->ebx & BIT_ULL(idx)) &&
> + (((entry->eax & EVT_LEN_MASK) >> EVT_LEN_OFS_BIT) > idx);
> +
> + vcpu_args_set(vcpu, 5, entry->eax & PMU_VERSION_MASK,
> + (entry->eax & GP_CTR_NUM_MASK) >> GP_CTR_NUM_OFS_BIT,
> + is_supported, ctr_msr, arch_events[idx]);
> +}
> +
> +static void intel_check_arch_event_is_unavl(uint8_t idx)
> +{
> + const char *msg = "Unavailable arch event is counting.";
> + uint8_t eax_evt_vec, ebx_unavl_mask, i, j;
> + struct kvm_vcpu *vcpu;
> +
> + /*
> + * 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(arch_events); i++) {
> + eax_evt_vec = BIT_ULL(i);
> + for (j = 0; j < ARRAY_SIZE(arch_events); j++) {
> + ebx_unavl_mask = BIT_ULL(j);
> +
> + vcpu = new_vcpu(intel_guest_run_arch_event);
> + test_arch_events_setup(vcpu, eax_evt_vec,
> + ebx_unavl_mask, idx);
> + run_vcpu(vcpu, msg, first_uc_arg_non_zero, NULL);
> + free_vcpu(vcpu);
> + }
> + }
> +}
> +
> +static void intel_test_arch_events(void)
> +{
> + uint8_t idx;
> +
> + for (idx = 0; idx < ARRAY_SIZE(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)
Note that reference cycles is one event that actually cannot be
successfully virtualized.
> + * - Branch instruction retired event (idx = 5)
> + */
> + if (idx > 2 && idx != 5)
> + continue;
> +
> + intel_check_arch_event_is_unavl(idx);
> + }
> +}
> +
> +static void intel_test_pmu_cpuid(void)
> +{
> + intel_test_arch_events();
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + TEST_REQUIRE(get_kvm_param_bool("enable_pmu"));
> +
> + 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_pmu_cpuid();
> + }
> +
> + return 0;
> +}
> --
> 2.40.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/7] KVM: selftests: Test Intel PMU architectural events on fixed counters
2023-03-23 7:27 [PATCH 0/7] KVM: selftests: Test the consistency of the PMU's CPUID and its features Like Xu
2023-03-23 7:27 ` [PATCH 1/7] KVM: selftests: Test Intel PMU architectural events on gp counters Like Xu
@ 2023-03-23 7:27 ` Like Xu
2023-05-24 22:36 ` Sean Christopherson
2023-03-23 7:27 ` [PATCH 3/7] KVM: selftests: Test consistency of CPUID with num of GP counters Like Xu
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Like Xu @ 2023-03-23 7:27 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, Jinrong Liang, linux-kernel, kvm
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>
---
.../selftests/kvm/x86_64/pmu_cpuid_test.c | 37 +++++++++++++++++--
1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
index faab0a91e191..75434aa2a0ec 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
@@ -25,6 +25,9 @@
#define GP_CTR_NUM_MASK GENMASK_ULL(15, GP_CTR_NUM_OFS_BIT)
#define EVT_LEN_OFS_BIT 24
#define EVT_LEN_MASK GENMASK_ULL(31, EVT_LEN_OFS_BIT)
+#define INTEL_PMC_IDX_FIXED 32
+#define RDPMC_FIXED_BASE BIT_ULL(30)
+#define FIXED_CTR_NUM_MASK GENMASK_ULL(4, 0)
#define ARCH_EVENT(select, umask) (((select) & 0xff) | ((umask) & 0xff) << 8)
@@ -43,6 +46,14 @@ static const uint64_t arch_events[] = {
[7] = ARCH_EVENT(0xa4, 0x1), /* Topdown Slots */
};
+/* Association of Fixed Counters with Architectural Performance Events */
+static int fixed_events[] = {1, 0, 7};
+
+static uint64_t evt_code_for_fixed_ctr(uint8_t idx)
+{
+ return arch_events[fixed_events[idx]];
+}
+
static struct kvm_vcpu *new_vcpu(void *guest_code)
{
struct kvm_vm *vm;
@@ -88,8 +99,8 @@ static bool first_uc_arg_non_zero(struct ucall *uc, void *data)
}
static void intel_guest_run_arch_event(uint8_t version, uint8_t max_gp_num,
- bool supported, uint32_t ctr_base_msr,
- uint64_t evt_code)
+ uint8_t max_fixed_num, bool supported,
+ uint32_t ctr_base_msr, uint64_t evt_code)
{
uint32_t global_msr = MSR_CORE_PERF_GLOBAL_CTRL;
unsigned int i;
@@ -108,6 +119,23 @@ static void intel_guest_run_arch_event(uint8_t version, uint8_t max_gp_num,
GUEST_SYNC(supported == !!_rdpmc(i));
}
+ /* No need to test independent arch events on fixed counters. */
+ if (version > 1 && max_fixed_num > 1 &&
+ (evt_code == evt_code_for_fixed_ctr(0) ||
+ evt_code == evt_code_for_fixed_ctr(1))) {
+ i = (evt_code == evt_code_for_fixed_ctr(0)) ? 0 : 1;
+
+ wrmsr(MSR_CORE_PERF_FIXED_CTR0 + i, 0);
+ wrmsr(MSR_CORE_PERF_FIXED_CTR_CTRL, BIT_ULL(4 * i));
+ wrmsr(global_msr, BIT_ULL(INTEL_PMC_IDX_FIXED + i));
+
+ __asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
+
+ wrmsr(global_msr, 0);
+
+ GUEST_SYNC(supported == !!_rdpmc(RDPMC_FIXED_BASE | i));
+ }
+
GUEST_DONE();
}
@@ -131,9 +159,10 @@ static void test_arch_events_setup(struct kvm_vcpu *vcpu, uint8_t evt_vector,
is_supported = !(entry->ebx & BIT_ULL(idx)) &&
(((entry->eax & EVT_LEN_MASK) >> EVT_LEN_OFS_BIT) > idx);
- vcpu_args_set(vcpu, 5, entry->eax & PMU_VERSION_MASK,
+ vcpu_args_set(vcpu, 6, entry->eax & PMU_VERSION_MASK,
(entry->eax & GP_CTR_NUM_MASK) >> GP_CTR_NUM_OFS_BIT,
- is_supported, ctr_msr, arch_events[idx]);
+ (entry->edx & FIXED_CTR_NUM_MASK), is_supported,
+ ctr_msr, arch_events[idx]);
}
static void intel_check_arch_event_is_unavl(uint8_t idx)
--
2.40.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 2/7] KVM: selftests: Test Intel PMU architectural events on fixed counters
2023-03-23 7:27 ` [PATCH 2/7] KVM: selftests: Test Intel PMU architectural events on fixed counters Like Xu
@ 2023-05-24 22:36 ` Sean Christopherson
0 siblings, 0 replies; 16+ messages in thread
From: Sean Christopherson @ 2023-05-24 22:36 UTC (permalink / raw)
To: Like Xu; +Cc: Paolo Bonzini, Jinrong Liang, linux-kernel, kvm
On Thu, Mar 23, 2023, Like Xu 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>
> ---
> .../selftests/kvm/x86_64/pmu_cpuid_test.c | 37 +++++++++++++++++--
> 1 file changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
> index faab0a91e191..75434aa2a0ec 100644
> --- a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
> @@ -25,6 +25,9 @@
> #define GP_CTR_NUM_MASK GENMASK_ULL(15, GP_CTR_NUM_OFS_BIT)
> #define EVT_LEN_OFS_BIT 24
> #define EVT_LEN_MASK GENMASK_ULL(31, EVT_LEN_OFS_BIT)
> +#define INTEL_PMC_IDX_FIXED 32
> +#define RDPMC_FIXED_BASE BIT_ULL(30)
> +#define FIXED_CTR_NUM_MASK GENMASK_ULL(4, 0)
>
> #define ARCH_EVENT(select, umask) (((select) & 0xff) | ((umask) & 0xff) << 8)
>
> @@ -43,6 +46,14 @@ static const uint64_t arch_events[] = {
> [7] = ARCH_EVENT(0xa4, 0x1), /* Topdown Slots */
> };
>
> +/* Association of Fixed Counters with Architectural Performance Events */
> +static int fixed_events[] = {1, 0, 7};
> +
> +static uint64_t evt_code_for_fixed_ctr(uint8_t idx)
> +{
> + return arch_events[fixed_events[idx]];
> +}
This appears to be more fodder for common code.
> static struct kvm_vcpu *new_vcpu(void *guest_code)
> {
> struct kvm_vm *vm;
> @@ -88,8 +99,8 @@ static bool first_uc_arg_non_zero(struct ucall *uc, void *data)
> }
>
> static void intel_guest_run_arch_event(uint8_t version, uint8_t max_gp_num,
> - bool supported, uint32_t ctr_base_msr,
> - uint64_t evt_code)
> + uint8_t max_fixed_num, bool supported,
> + uint32_t ctr_base_msr, uint64_t evt_code)
> {
> uint32_t global_msr = MSR_CORE_PERF_GLOBAL_CTRL;
> unsigned int i;
> @@ -108,6 +119,23 @@ static void intel_guest_run_arch_event(uint8_t version, uint8_t max_gp_num,
> GUEST_SYNC(supported == !!_rdpmc(i));
> }
>
> + /* No need to test independent arch events on fixed counters. */
> + if (version > 1 && max_fixed_num > 1 &&
> + (evt_code == evt_code_for_fixed_ctr(0) ||
> + evt_code == evt_code_for_fixed_ctr(1))) {
> + i = (evt_code == evt_code_for_fixed_ctr(0)) ? 0 : 1;
The ternary operator on top of a duplicate comparison isn't super intuitive.
Maybe use gotos? Definitely just an idea, not a requirement.
if (version <= 1 || max_fixed_num <= 1)
goto done;
if (evt_code == evt_code_for_fixed_ctr(0))
i = 0;
else if (evt_code == evt_code_for_fixed_ctr(1))
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(global_msr, BIT_ULL(INTEL_PMC_IDX_FIXED + i));
> +
> + __asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
> +
> + wrmsr(global_msr, 0);
> +
> + GUEST_SYNC(supported == !!_rdpmc(RDPMC_FIXED_BASE | i));
> + }
> +
> GUEST_DONE();
> }
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/7] KVM: selftests: Test consistency of CPUID with num of GP counters
2023-03-23 7:27 [PATCH 0/7] KVM: selftests: Test the consistency of the PMU's CPUID and its features Like Xu
2023-03-23 7:27 ` [PATCH 1/7] KVM: selftests: Test Intel PMU architectural events on gp counters Like Xu
2023-03-23 7:27 ` [PATCH 2/7] KVM: selftests: Test Intel PMU architectural events on fixed counters Like Xu
@ 2023-03-23 7:27 ` Like Xu
2023-05-24 22:44 ` Sean Christopherson
2023-03-23 7:27 ` [PATCH 4/7] KVM: selftests: Test consistency of CPUID with num of Fixed counters Like Xu
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Like Xu @ 2023-03-23 7:27 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, Jinrong Liang, linux-kernel, kvm
From: Like Xu <likexu@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: Jinrong Liang <cloudliang@tencent.com>
Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
Signed-off-by: Like Xu <likexu@tencent.com>
---
.../selftests/kvm/x86_64/pmu_cpuid_test.c | 102 ++++++++++++++++++
1 file changed, 102 insertions(+)
diff --git a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
index 75434aa2a0ec..50902187d2c9 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
@@ -49,11 +49,31 @@ static const uint64_t arch_events[] = {
/* Association of Fixed Counters with Architectural Performance Events */
static int fixed_events[] = {1, 0, 7};
+static const uint64_t perf_caps[] = {
+ 0,
+ PMU_CAP_FW_WRITES,
+};
+
+/*
+ * KVM implements the first two non-existent counters (MSR_P6_PERFCTRx)
+ * via kvm_pr_unimpl_wrmsr() instead of #GP. It is acceptable here to test
+ * the third counter as there are usually more than 3 available gp counters.
+ */
+#define MSR_INTEL_ARCH_PMU_GPCTR (MSR_IA32_PERFCTR0 + 2)
+
static uint64_t evt_code_for_fixed_ctr(uint8_t idx)
{
return arch_events[fixed_events[idx]];
}
+static uint8_t kvm_gp_ctrs_num(void)
+{
+ const struct kvm_cpuid_entry2 *kvm_entry;
+
+ kvm_entry = get_cpuid_entry(kvm_get_supported_cpuid(), 0xa, 0);
+ return (kvm_entry->eax & GP_CTR_NUM_MASK) >> GP_CTR_NUM_OFS_BIT;
+}
+
static struct kvm_vcpu *new_vcpu(void *guest_code)
{
struct kvm_vm *vm;
@@ -98,6 +118,30 @@ static bool first_uc_arg_non_zero(struct ucall *uc, void *data)
return uc->args[1];
}
+static bool first_uc_arg_equals(struct ucall *uc, void *data)
+{
+ return uc->args[1] == (uint64_t)data;
+}
+
+static void guest_gp_handler(struct ex_regs *regs)
+{
+ GUEST_SYNC(GP_VECTOR);
+ GUEST_DONE();
+}
+
+static void guest_wr_and_rd_msrs(uint32_t base, uint64_t value,
+ uint8_t begin, uint8_t offset)
+{
+ unsigned int i;
+
+ for (i = begin; i < begin + offset; i++) {
+ wrmsr(base + i, value);
+ GUEST_SYNC(rdmsr(base + i));
+ }
+
+ GUEST_DONE();
+}
+
static void intel_guest_run_arch_event(uint8_t version, uint8_t max_gp_num,
uint8_t max_fixed_num, bool supported,
uint32_t ctr_base_msr, uint64_t evt_code)
@@ -165,6 +209,27 @@ static void test_arch_events_setup(struct kvm_vcpu *vcpu, uint8_t evt_vector,
ctr_msr, arch_events[idx]);
}
+static void test_oob_gp_counter_setup(struct kvm_vcpu *vcpu, uint8_t eax_gp_num,
+ uint64_t perf_cap)
+{
+ struct kvm_cpuid_entry2 *entry;
+ uint32_t ctr_msr = MSR_IA32_PERFCTR0;
+
+ entry = vcpu_get_cpuid_entry(vcpu, 0xa);
+ entry->eax = (entry->eax & ~GP_CTR_NUM_MASK) |
+ (eax_gp_num << GP_CTR_NUM_OFS_BIT);
+ vcpu_set_cpuid(vcpu);
+
+ 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, 4, ctr_msr, 0xffff,
+ min(eax_gp_num, kvm_gp_ctrs_num()), 1);
+
+ vm_install_exception_handler(vcpu->vm, GP_VECTOR, guest_gp_handler);
+}
+
static void intel_check_arch_event_is_unavl(uint8_t idx)
{
const char *msg = "Unavailable arch event is counting.";
@@ -190,6 +255,42 @@ static void intel_check_arch_event_is_unavl(uint8_t idx)
}
}
+/* Access the first out-of-range counter register to trigger #GP */
+static void test_oob_gp_counter(uint8_t eax_gp_num, uint64_t perf_cap)
+{
+ const char *msg = "At least one unsupported GP counter is visible.";
+ struct kvm_vcpu *vcpu;
+
+ vcpu = new_vcpu(guest_wr_and_rd_msrs);
+ test_oob_gp_counter_setup(vcpu, eax_gp_num, perf_cap);
+ run_vcpu(vcpu, msg, first_uc_arg_equals, (void *)GP_VECTOR);
+ free_vcpu(vcpu);
+}
+
+static void intel_test_counters_num(void)
+{
+ uint8_t kvm_gp_num = kvm_gp_ctrs_num();
+ unsigned int i;
+
+ TEST_REQUIRE(kvm_gp_num > 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, perf_caps[i]);
+
+ test_oob_gp_counter(2, perf_caps[i]);
+ test_oob_gp_counter(kvm_gp_num, perf_caps[i]);
+
+ /* KVM doesn't emulate more counters than it can support. */
+ test_oob_gp_counter(kvm_gp_num + 1, perf_caps[i]);
+ }
+}
+
static void intel_test_arch_events(void)
{
uint8_t idx;
@@ -213,6 +314,7 @@ static void intel_test_arch_events(void)
static void intel_test_pmu_cpuid(void)
{
intel_test_arch_events();
+ intel_test_counters_num();
}
int main(int argc, char *argv[])
--
2.40.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 3/7] KVM: selftests: Test consistency of CPUID with num of GP counters
2023-03-23 7:27 ` [PATCH 3/7] KVM: selftests: Test consistency of CPUID with num of GP counters Like Xu
@ 2023-05-24 22:44 ` Sean Christopherson
0 siblings, 0 replies; 16+ messages in thread
From: Sean Christopherson @ 2023-05-24 22:44 UTC (permalink / raw)
To: Like Xu; +Cc: Paolo Bonzini, Jinrong Liang, linux-kernel, kvm
On Thu, Mar 23, 2023, Like Xu wrote:
> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
> index 75434aa2a0ec..50902187d2c9 100644
> --- a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
> @@ -49,11 +49,31 @@ static const uint64_t arch_events[] = {
> /* Association of Fixed Counters with Architectural Performance Events */
> static int fixed_events[] = {1, 0, 7};
>
> +static const uint64_t perf_caps[] = {
> + 0,
> + PMU_CAP_FW_WRITES,
> +};
> +
> +/*
> + * KVM implements the first two non-existent counters (MSR_P6_PERFCTRx)
> + * via kvm_pr_unimpl_wrmsr() instead of #GP. It is acceptable here to test
> + * the third counter as there are usually more than 3 available gp counters.
Don't hedge, i.e. don't say things like "usually". And why not test that KVM
drops writes to the first two counters? Unlike KVM-Unit_tests, selftests can
test arbitrary KVM behavior without concern for breaking other use cases.
> +#define MSR_INTEL_ARCH_PMU_GPCTR (MSR_IA32_PERFCTR0 + 2)
> +
> static uint64_t evt_code_for_fixed_ctr(uint8_t idx)
> {
> return arch_events[fixed_events[idx]];
> }
>
> +static uint8_t kvm_gp_ctrs_num(void)
> +{
> + const struct kvm_cpuid_entry2 *kvm_entry;
> +
> + kvm_entry = get_cpuid_entry(kvm_get_supported_cpuid(), 0xa, 0);
> + return (kvm_entry->eax & GP_CTR_NUM_MASK) >> GP_CTR_NUM_OFS_BIT;
This definitely can be defined as a KVM_X86_CPU_PROPERTY(). Ditto for most of
the helpers that are added in future patches.
> static struct kvm_vcpu *new_vcpu(void *guest_code)
> {
> struct kvm_vm *vm;
> @@ -98,6 +118,30 @@ static bool first_uc_arg_non_zero(struct ucall *uc, void *data)
> return uc->args[1];
> }
>
> +static bool first_uc_arg_equals(struct ucall *uc, void *data)
> +{
> + return uc->args[1] == (uint64_t)data;
> +}
> +
> +static void guest_gp_handler(struct ex_regs *regs)
> +{
> + GUEST_SYNC(GP_VECTOR);
> + GUEST_DONE();
> +}
> +
> +static void guest_wr_and_rd_msrs(uint32_t base, uint64_t value,
> + uint8_t begin, uint8_t offset)
> +{
> + unsigned int i;
> +
> + for (i = begin; i < begin + offset; i++) {
> + wrmsr(base + i, value);
> + GUEST_SYNC(rdmsr(base + i));
Unless it won't work for something, use rdmsr_safe() and/oror wrmsr_safe() instead
of installing a dedicated handler. And if I'm reading the code correctly, that will
fix a bug in the test where only the first MSR is tested in the #GP case since the
#GP handler goes straight to GUEST_DONE(), i.e. doesn't skip and continue the rest
of the guest code. Maybe that isn't a bug in practice, e.g. each negative test only
tests a single MSR, but (a) that's not obvious and (b) it's an unnecessary limitation.
> + }
> +
> + GUEST_DONE();
> +}
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/7] KVM: selftests: Test consistency of CPUID with num of Fixed counters
2023-03-23 7:27 [PATCH 0/7] KVM: selftests: Test the consistency of the PMU's CPUID and its features Like Xu
` (2 preceding siblings ...)
2023-03-23 7:27 ` [PATCH 3/7] KVM: selftests: Test consistency of CPUID with num of GP counters Like Xu
@ 2023-03-23 7:27 ` Like Xu
2023-05-24 22:47 ` Sean Christopherson
2023-05-24 23:08 ` Jim Mattson
2023-03-23 7:27 ` [PATCH 5/7] KVM: selftests: Test Intel supported fixed counters bit mask Like Xu
` (3 subsequent siblings)
7 siblings, 2 replies; 16+ messages in thread
From: Like Xu @ 2023-03-23 7:27 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, Jinrong Liang, linux-kernel, kvm
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>
---
.../selftests/kvm/x86_64/pmu_cpuid_test.c | 68 +++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
index 50902187d2c9..c934144be287 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
@@ -74,6 +74,22 @@ static uint8_t kvm_gp_ctrs_num(void)
return (kvm_entry->eax & GP_CTR_NUM_MASK) >> GP_CTR_NUM_OFS_BIT;
}
+static uint8_t kvm_fixed_ctrs_num(void)
+{
+ const struct kvm_cpuid_entry2 *kvm_entry;
+
+ kvm_entry = get_cpuid_entry(kvm_get_supported_cpuid(), 0xa, 0);
+ return kvm_entry->edx & FIXED_CTR_NUM_MASK;
+}
+
+static uint32_t kvm_fixed_ctrs_bitmask(void)
+{
+ const struct kvm_cpuid_entry2 *kvm_entry;
+
+ kvm_entry = get_cpuid_entry(kvm_get_supported_cpuid(), 0xa, 0);
+ return kvm_entry->ecx;
+}
+
static struct kvm_vcpu *new_vcpu(void *guest_code)
{
struct kvm_vm *vm;
@@ -230,6 +246,39 @@ static void test_oob_gp_counter_setup(struct kvm_vcpu *vcpu, uint8_t eax_gp_num,
vm_install_exception_handler(vcpu->vm, GP_VECTOR, guest_gp_handler);
}
+static uint64_t test_oob_fixed_counter_setup(struct kvm_vcpu *vcpu,
+ uint8_t edx_fix_num,
+ uint32_t fixed_bitmask)
+{
+ struct kvm_cpuid_entry2 *entry;
+ uint32_t ctr_msr = MSR_CORE_PERF_FIXED_CTR0;
+ uint8_t idx = edx_fix_num;
+ bool is_supported = true;
+ uint64_t ret = 0xffffULL;
+
+ entry = vcpu_get_cpuid_entry(vcpu, 0xa);
+ entry->ecx = fixed_bitmask;
+ entry->edx = (entry->edx & ~FIXED_CTR_NUM_MASK) | edx_fix_num;
+ vcpu_set_cpuid(vcpu);
+
+ /* Per Intel SDM, FixCtr[i]_is_supported := ECX[i] || (EDX[4:0] > i). */
+ is_supported = (entry->ecx & BIT_ULL(idx) ||
+ ((entry->edx & FIXED_CTR_NUM_MASK) > idx));
+
+ /* KVM doesn't emulate more fixed counters than it can support. */
+ if (idx >= kvm_fixed_ctrs_num())
+ is_supported = false;
+
+ if (!is_supported) {
+ vm_install_exception_handler(vcpu->vm, GP_VECTOR, guest_gp_handler);
+ ret = GP_VECTOR;
+ }
+
+ vcpu_args_set(vcpu, 4, ctr_msr, ret, idx, 1);
+
+ return ret;
+}
+
static void intel_check_arch_event_is_unavl(uint8_t idx)
{
const char *msg = "Unavailable arch event is counting.";
@@ -267,10 +316,23 @@ static void test_oob_gp_counter(uint8_t eax_gp_num, uint64_t perf_cap)
free_vcpu(vcpu);
}
+static void intel_test_oob_fixed_ctr(uint8_t edx_fix_num, uint32_t fixed_bitmask)
+{
+ const char *msg = "At least one unsupported Fixed counter is visible.";
+ struct kvm_vcpu *vcpu;
+ uint64_t ret;
+
+ vcpu = new_vcpu(guest_wr_and_rd_msrs);
+ ret = test_oob_fixed_counter_setup(vcpu, edx_fix_num, fixed_bitmask);
+ run_vcpu(vcpu, msg, first_uc_arg_equals, (void *)ret);
+ free_vcpu(vcpu);
+}
+
static void intel_test_counters_num(void)
{
uint8_t kvm_gp_num = kvm_gp_ctrs_num();
unsigned int i;
+ uint32_t ecx;
TEST_REQUIRE(kvm_gp_num > 2);
@@ -289,6 +351,12 @@ static void intel_test_counters_num(void)
/* KVM doesn't emulate more counters than it can support. */
test_oob_gp_counter(kvm_gp_num + 1, perf_caps[i]);
}
+
+ for (ecx = 0; ecx <= kvm_fixed_ctrs_bitmask() + 1; ecx++) {
+ intel_test_oob_fixed_ctr(0, ecx);
+ intel_test_oob_fixed_ctr(kvm_fixed_ctrs_num(), ecx);
+ intel_test_oob_fixed_ctr(kvm_fixed_ctrs_num() + 1, ecx);
+ }
}
static void intel_test_arch_events(void)
--
2.40.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 4/7] KVM: selftests: Test consistency of CPUID with num of Fixed counters
2023-03-23 7:27 ` [PATCH 4/7] KVM: selftests: Test consistency of CPUID with num of Fixed counters Like Xu
@ 2023-05-24 22:47 ` Sean Christopherson
2023-05-24 23:08 ` Jim Mattson
1 sibling, 0 replies; 16+ messages in thread
From: Sean Christopherson @ 2023-05-24 22:47 UTC (permalink / raw)
To: Like Xu; +Cc: Paolo Bonzini, Jinrong Liang, linux-kernel, kvm
On Thu, Mar 23, 2023, Like Xu 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.
> 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>
> ---
> .../selftests/kvm/x86_64/pmu_cpuid_test.c | 68 +++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
> index 50902187d2c9..c934144be287 100644
> --- a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
> @@ -74,6 +74,22 @@ static uint8_t kvm_gp_ctrs_num(void)
> return (kvm_entry->eax & GP_CTR_NUM_MASK) >> GP_CTR_NUM_OFS_BIT;
> }
>
> +static uint8_t kvm_fixed_ctrs_num(void)
> +{
> + const struct kvm_cpuid_entry2 *kvm_entry;
> +
> + kvm_entry = get_cpuid_entry(kvm_get_supported_cpuid(), 0xa, 0);
> + return kvm_entry->edx & FIXED_CTR_NUM_MASK;
> +}
> +
> +static uint32_t kvm_fixed_ctrs_bitmask(void)
> +{
> + const struct kvm_cpuid_entry2 *kvm_entry;
> +
> + kvm_entry = get_cpuid_entry(kvm_get_supported_cpuid(), 0xa, 0);
> + return kvm_entry->ecx;
> +}
KVM_X86_CPU_PROPERTY
> static struct kvm_vcpu *new_vcpu(void *guest_code)
> {
> struct kvm_vm *vm;
> @@ -230,6 +246,39 @@ static void test_oob_gp_counter_setup(struct kvm_vcpu *vcpu, uint8_t eax_gp_num,
> vm_install_exception_handler(vcpu->vm, GP_VECTOR, guest_gp_handler);
> }
>
> +static uint64_t test_oob_fixed_counter_setup(struct kvm_vcpu *vcpu,
> + uint8_t edx_fix_num,
> + uint32_t fixed_bitmask)
> +{
> + struct kvm_cpuid_entry2 *entry;
> + uint32_t ctr_msr = MSR_CORE_PERF_FIXED_CTR0;
> + uint8_t idx = edx_fix_num;
> + bool is_supported = true;
No need to initialize "true", it's explicitly set below.
> + uint64_t ret = 0xffffULL;
> +
> + entry = vcpu_get_cpuid_entry(vcpu, 0xa);
> + entry->ecx = fixed_bitmask;
> + entry->edx = (entry->edx & ~FIXED_CTR_NUM_MASK) | edx_fix_num;
> + vcpu_set_cpuid(vcpu);
> +
> + /* Per Intel SDM, FixCtr[i]_is_supported := ECX[i] || (EDX[4:0] > i). */
> + is_supported = (entry->ecx & BIT_ULL(idx) ||
> + ((entry->edx & FIXED_CTR_NUM_MASK) > idx));
> +
> + /* KVM doesn't emulate more fixed counters than it can support. */
> + if (idx >= kvm_fixed_ctrs_num())
> + is_supported = false;
Why not this?
is_supported = idx < kvm_fixed_ctrs_num() &&
<CPUID entry stuff>;
> +
> + if (!is_supported) {
> + vm_install_exception_handler(vcpu->vm, GP_VECTOR, guest_gp_handler);
> + ret = GP_VECTOR;
> + }
Same comments as the previous patch(es).
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 4/7] KVM: selftests: Test consistency of CPUID with num of Fixed counters
2023-03-23 7:27 ` [PATCH 4/7] KVM: selftests: Test consistency of CPUID with num of Fixed counters Like Xu
2023-05-24 22:47 ` Sean Christopherson
@ 2023-05-24 23:08 ` Jim Mattson
1 sibling, 0 replies; 16+ messages in thread
From: Jim Mattson @ 2023-05-24 23:08 UTC (permalink / raw)
To: Like Xu
Cc: Sean Christopherson, Paolo Bonzini, Jinrong Liang, linux-kernel,
kvm
On Thu, Mar 23, 2023 at 12:28 AM Like Xu <like.xu.linux@gmail.com> 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.
> 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>
> ---
> .../selftests/kvm/x86_64/pmu_cpuid_test.c | 68 +++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
> index 50902187d2c9..c934144be287 100644
> --- a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
> @@ -74,6 +74,22 @@ static uint8_t kvm_gp_ctrs_num(void)
> return (kvm_entry->eax & GP_CTR_NUM_MASK) >> GP_CTR_NUM_OFS_BIT;
> }
>
> +static uint8_t kvm_fixed_ctrs_num(void)
> +{
> + const struct kvm_cpuid_entry2 *kvm_entry;
> +
> + kvm_entry = get_cpuid_entry(kvm_get_supported_cpuid(), 0xa, 0);
> + return kvm_entry->edx & FIXED_CTR_NUM_MASK;
> +}
> +
> +static uint32_t kvm_fixed_ctrs_bitmask(void)
> +{
> + const struct kvm_cpuid_entry2 *kvm_entry;
> +
> + kvm_entry = get_cpuid_entry(kvm_get_supported_cpuid(), 0xa, 0);
> + return kvm_entry->ecx;
> +}
> +
> static struct kvm_vcpu *new_vcpu(void *guest_code)
> {
> struct kvm_vm *vm;
> @@ -230,6 +246,39 @@ static void test_oob_gp_counter_setup(struct kvm_vcpu *vcpu, uint8_t eax_gp_num,
> vm_install_exception_handler(vcpu->vm, GP_VECTOR, guest_gp_handler);
> }
>
> +static uint64_t test_oob_fixed_counter_setup(struct kvm_vcpu *vcpu,
> + uint8_t edx_fix_num,
> + uint32_t fixed_bitmask)
> +{
> + struct kvm_cpuid_entry2 *entry;
> + uint32_t ctr_msr = MSR_CORE_PERF_FIXED_CTR0;
> + uint8_t idx = edx_fix_num;
> + bool is_supported = true;
> + uint64_t ret = 0xffffULL;
> +
> + entry = vcpu_get_cpuid_entry(vcpu, 0xa);
> + entry->ecx = fixed_bitmask;
> + entry->edx = (entry->edx & ~FIXED_CTR_NUM_MASK) | edx_fix_num;
> + vcpu_set_cpuid(vcpu);
> +
> + /* Per Intel SDM, FixCtr[i]_is_supported := ECX[i] || (EDX[4:0] > i). */
> + is_supported = (entry->ecx & BIT_ULL(idx) ||
> + ((entry->edx & FIXED_CTR_NUM_MASK) > idx));
> +
> + /* KVM doesn't emulate more fixed counters than it can support. */
> + if (idx >= kvm_fixed_ctrs_num())
> + is_supported = false;
> +
> + if (!is_supported) {
> + vm_install_exception_handler(vcpu->vm, GP_VECTOR, guest_gp_handler);
> + ret = GP_VECTOR;
> + }
> +
> + vcpu_args_set(vcpu, 4, ctr_msr, ret, idx, 1);
> +
> + return ret;
> +}
> +
> static void intel_check_arch_event_is_unavl(uint8_t idx)
> {
> const char *msg = "Unavailable arch event is counting.";
This test seems bogus to me. The event may not be available because it
is inaccurate. That doesn't imply that the count will always be zero.
> @@ -267,10 +316,23 @@ static void test_oob_gp_counter(uint8_t eax_gp_num, uint64_t perf_cap)
> free_vcpu(vcpu);
> }
>
> +static void intel_test_oob_fixed_ctr(uint8_t edx_fix_num, uint32_t fixed_bitmask)
> +{
> + const char *msg = "At least one unsupported Fixed counter is visible.";
This test seems bogus to me. Unsupported does not imply invisible.
> + struct kvm_vcpu *vcpu;
> + uint64_t ret;
> +
> + vcpu = new_vcpu(guest_wr_and_rd_msrs);
> + ret = test_oob_fixed_counter_setup(vcpu, edx_fix_num, fixed_bitmask);
> + run_vcpu(vcpu, msg, first_uc_arg_equals, (void *)ret);
> + free_vcpu(vcpu);
> +}
> +
> static void intel_test_counters_num(void)
> {
> uint8_t kvm_gp_num = kvm_gp_ctrs_num();
> unsigned int i;
> + uint32_t ecx;
>
> TEST_REQUIRE(kvm_gp_num > 2);
>
> @@ -289,6 +351,12 @@ static void intel_test_counters_num(void)
> /* KVM doesn't emulate more counters than it can support. */
> test_oob_gp_counter(kvm_gp_num + 1, perf_caps[i]);
> }
> +
> + for (ecx = 0; ecx <= kvm_fixed_ctrs_bitmask() + 1; ecx++) {
> + intel_test_oob_fixed_ctr(0, ecx);
> + intel_test_oob_fixed_ctr(kvm_fixed_ctrs_num(), ecx);
> + intel_test_oob_fixed_ctr(kvm_fixed_ctrs_num() + 1, ecx);
> + }
> }
>
> static void intel_test_arch_events(void)
> --
> 2.40.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 5/7] KVM: selftests: Test Intel supported fixed counters bit mask
2023-03-23 7:27 [PATCH 0/7] KVM: selftests: Test the consistency of the PMU's CPUID and its features Like Xu
` (3 preceding siblings ...)
2023-03-23 7:27 ` [PATCH 4/7] KVM: selftests: Test consistency of CPUID with num of Fixed counters Like Xu
@ 2023-03-23 7:27 ` Like Xu
2023-03-23 7:27 ` [PATCH 6/7] KVM: selftests: Test consistency of PMU MSRs with Intel PMU version Like Xu
` (2 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Like Xu @ 2023-03-23 7:27 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, Jinrong Liang, linux-kernel, kvm
From: Like Xu <likexu@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.
Signed-off-by: Like Xu <likexu@tencent.com>
---
.../selftests/kvm/x86_64/pmu_cpuid_test.c | 63 +++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
index c934144be287..79f2e144c6c6 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
@@ -199,6 +199,27 @@ static void intel_guest_run_arch_event(uint8_t version, uint8_t max_gp_num,
GUEST_DONE();
}
+static void intel_guest_run_fixed_counters(uint64_t supported_bitmask,
+ uint8_t max_fixed_num)
+{
+ unsigned int i;
+
+ for (i = 0; i < max_fixed_num; i++) {
+ if (!(supported_bitmask & BIT_ULL(i)))
+ continue;
+
+ 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));
+ }
+
+ GUEST_DONE();
+}
+
static void test_arch_events_setup(struct kvm_vcpu *vcpu, uint8_t evt_vector,
uint8_t unavl_mask, uint8_t idx)
{
@@ -279,6 +300,47 @@ static uint64_t test_oob_fixed_counter_setup(struct kvm_vcpu *vcpu,
return ret;
}
+static void test_fixed_counters_setup(struct kvm_vcpu *vcpu, uint8_t edx_fix_num,
+ uint32_t fixed_bitmask)
+{
+ struct kvm_cpuid_entry2 *entry;
+ uint8_t max_fixed_num = kvm_fixed_ctrs_num();
+ uint64_t supported_bitmask = 0;
+ unsigned int i;
+
+ entry = vcpu_get_cpuid_entry(vcpu, 0xa);
+ entry->ecx = fixed_bitmask;
+ entry->edx = (entry->edx & ~FIXED_CTR_NUM_MASK) | edx_fix_num;
+ vcpu_set_cpuid(vcpu);
+
+ for (i = 0; i < max_fixed_num; i++) {
+ if (entry->ecx & BIT_ULL(i) ||
+ ((entry->edx & FIXED_CTR_NUM_MASK) > i))
+ supported_bitmask |= BIT_ULL(i);
+ }
+
+ vcpu_args_set(vcpu, 2, supported_bitmask, max_fixed_num);
+ vm_install_exception_handler(vcpu->vm, GP_VECTOR, guest_gp_handler);
+}
+
+static void intel_test_fixed_counters(void)
+{
+ const char *msg = "At least one fixed counter is not working as expected";
+ uint8_t edx, num = kvm_fixed_ctrs_num();
+ struct kvm_vcpu *vcpu;
+ uint32_t ecx;
+
+ for (edx = 0; edx <= num; edx++) {
+ /* KVM doesn't emulate more fixed counters than it can support. */
+ for (ecx = 0; ecx <= (BIT_ULL(num) - 1); ecx++) {
+ vcpu = new_vcpu(intel_guest_run_fixed_counters);
+ test_fixed_counters_setup(vcpu, edx, ecx);
+ run_vcpu(vcpu, msg, first_uc_arg_equals, (void *)true);
+ free_vcpu(vcpu);
+ }
+ }
+}
+
static void intel_check_arch_event_is_unavl(uint8_t idx)
{
const char *msg = "Unavailable arch event is counting.";
@@ -383,6 +445,7 @@ static void intel_test_pmu_cpuid(void)
{
intel_test_arch_events();
intel_test_counters_num();
+ intel_test_fixed_counters();
}
int main(int argc, char *argv[])
--
2.40.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 6/7] KVM: selftests: Test consistency of PMU MSRs with Intel PMU version
2023-03-23 7:27 [PATCH 0/7] KVM: selftests: Test the consistency of the PMU's CPUID and its features Like Xu
` (4 preceding siblings ...)
2023-03-23 7:27 ` [PATCH 5/7] KVM: selftests: Test Intel supported fixed counters bit mask Like Xu
@ 2023-03-23 7:27 ` Like Xu
2023-03-23 7:27 ` [PATCH 7/7] KVM: selftests: Test Intel counters' bit width emulation Like Xu
2023-05-24 22:53 ` [PATCH 0/7] KVM: selftests: Test the consistency of the PMU's CPUID and its features Sean Christopherson
7 siblings, 0 replies; 16+ messages in thread
From: Like Xu @ 2023-03-23 7:27 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, Jinrong Liang, linux-kernel, kvm
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>
---
.../selftests/kvm/x86_64/pmu_cpuid_test.c | 57 +++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
index 79f2e144c6c6..caf0d98079c7 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
@@ -17,6 +17,7 @@
#define NUM_BRANCHES 10
#define EVENTSEL_OS BIT_ULL(17)
+#define EVENTSEL_ANY BIT_ULL(21)
#define EVENTSEL_EN BIT_ULL(22)
#define PMU_CAP_FW_WRITES BIT_ULL(13)
#define EVENTS_MASK GENMASK_ULL(7, 0)
@@ -90,6 +91,14 @@ static uint32_t kvm_fixed_ctrs_bitmask(void)
return kvm_entry->ecx;
}
+static uint32_t kvm_max_pmu_version(void)
+{
+ const struct kvm_cpuid_entry2 *kvm_entry;
+
+ kvm_entry = get_cpuid_entry(kvm_get_supported_cpuid(), 0xa, 0);
+ return kvm_entry->eax & PMU_VERSION_MASK;
+}
+
static struct kvm_vcpu *new_vcpu(void *guest_code)
{
struct kvm_vm *vm;
@@ -220,6 +229,25 @@ static void intel_guest_run_fixed_counters(uint64_t supported_bitmask,
GUEST_DONE();
}
+static void intel_guest_check_pmu_version(uint8_t version)
+{
+ switch (version) {
+ case 0:
+ wrmsr(MSR_INTEL_ARCH_PMU_GPCTR, 0xffffull);
+ case 1:
+ wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0x1ull);
+ case 2:
+ /* AnyThread Bit is only supported in version 3 */
+ wrmsr(MSR_P6_EVNTSEL0, EVENTSEL_ANY);
+ break;
+ default:
+ /* KVM currently supports up to pmu version 2 */
+ GUEST_SYNC(GP_VECTOR);
+ }
+
+ GUEST_DONE();
+}
+
static void test_arch_events_setup(struct kvm_vcpu *vcpu, uint8_t evt_vector,
uint8_t unavl_mask, uint8_t idx)
{
@@ -341,6 +369,18 @@ static void intel_test_fixed_counters(void)
}
}
+static void test_pmu_version_setup(struct kvm_vcpu *vcpu, uint8_t version)
+{
+ struct kvm_cpuid_entry2 *entry;
+
+ entry = vcpu_get_cpuid_entry(vcpu, 0xa);
+ entry->eax = (entry->eax & ~PMU_VERSION_MASK) | version;
+ vcpu_set_cpuid(vcpu);
+
+ vcpu_args_set(vcpu, 1, version);
+ vm_install_exception_handler(vcpu->vm, GP_VECTOR, guest_gp_handler);
+}
+
static void intel_check_arch_event_is_unavl(uint8_t idx)
{
const char *msg = "Unavailable arch event is counting.";
@@ -441,11 +481,28 @@ static void intel_test_arch_events(void)
}
}
+static void intel_test_pmu_version(void)
+{
+ const char *msg = "Something beyond this PMU version is leaked.";
+ uint8_t version, unsupported_version = kvm_max_pmu_version() + 1;
+ struct kvm_vcpu *vcpu;
+
+ TEST_REQUIRE(kvm_gp_ctrs_num() > 2);
+
+ for (version = 0; version <= unsupported_version; version++) {
+ vcpu = new_vcpu(intel_guest_check_pmu_version);
+ test_pmu_version_setup(vcpu, version);
+ run_vcpu(vcpu, msg, first_uc_arg_equals, (void *)GP_VECTOR);
+ free_vcpu(vcpu);
+ }
+}
+
static void intel_test_pmu_cpuid(void)
{
intel_test_arch_events();
intel_test_counters_num();
intel_test_fixed_counters();
+ intel_test_pmu_version();
}
int main(int argc, char *argv[])
--
2.40.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 7/7] KVM: selftests: Test Intel counters' bit width emulation
2023-03-23 7:27 [PATCH 0/7] KVM: selftests: Test the consistency of the PMU's CPUID and its features Like Xu
` (5 preceding siblings ...)
2023-03-23 7:27 ` [PATCH 6/7] KVM: selftests: Test consistency of PMU MSRs with Intel PMU version Like Xu
@ 2023-03-23 7:27 ` Like Xu
2023-05-24 22:52 ` Sean Christopherson
2023-05-24 22:53 ` [PATCH 0/7] KVM: selftests: Test the consistency of the PMU's CPUID and its features Sean Christopherson
7 siblings, 1 reply; 16+ messages in thread
From: Like Xu @ 2023-03-23 7:27 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, Jinrong Liang, linux-kernel, kvm
From: Like Xu <likexu@tencent.com>
Add tests to cover Intel counters' bit-width emulation. When the VM has
FW_WRITES bit, the bitwidth of the gp and fixed counters will be specified
by the CPUID (no less than 32 bits and no greater than the host bitwidth)
and accessing bits that are not within the bitwidth will generate #GP.
However, when it does not have FW_WRITES bit, only the low 32-bits
signed data will be in effect and naturally #GP will not be generated.
Co-developed-by: Jinrong Liang <cloudliang@tencent.com>
Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
Signed-off-by: Like Xu <likexu@tencent.com>
---
.../selftests/kvm/x86_64/pmu_cpuid_test.c | 105 ++++++++++++++++++
1 file changed, 105 insertions(+)
diff --git a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
index caf0d98079c7..e7465b01178a 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_cpuid_test.c
@@ -29,6 +29,10 @@
#define INTEL_PMC_IDX_FIXED 32
#define RDPMC_FIXED_BASE BIT_ULL(30)
#define FIXED_CTR_NUM_MASK GENMASK_ULL(4, 0)
+#define GP_WIDTH_OFS_BIT 16
+#define GP_WIDTH_MASK GENMASK_ULL(23, GP_WIDTH_OFS_BIT)
+#define FIXED_WIDTH_OFS_BIT 5
+#define FIXED_WIDTH_MASK GENMASK_ULL(12, FIXED_WIDTH_OFS_BIT)
#define ARCH_EVENT(select, umask) (((select) & 0xff) | ((umask) & 0xff) << 8)
@@ -62,6 +66,16 @@ static const uint64_t perf_caps[] = {
*/
#define MSR_INTEL_ARCH_PMU_GPCTR (MSR_IA32_PERFCTR0 + 2)
+static const uint32_t msr_bases[] = {
+ MSR_INTEL_ARCH_PMU_GPCTR,
+ MSR_IA32_PMC0,
+ MSR_CORE_PERF_FIXED_CTR0,
+};
+
+static const uint64_t bit_widths[] = {
+ 0, 1, 31, 32, 47, 48, 63, 64,
+};
+
static uint64_t evt_code_for_fixed_ctr(uint8_t idx)
{
return arch_events[fixed_events[idx]];
@@ -99,6 +113,22 @@ static uint32_t kvm_max_pmu_version(void)
return kvm_entry->eax & PMU_VERSION_MASK;
}
+static uint32_t kvm_gp_ctr_bit_width(void)
+{
+ const struct kvm_cpuid_entry2 *kvm_entry;
+
+ kvm_entry = get_cpuid_entry(kvm_get_supported_cpuid(), 0xa, 0);
+ return (kvm_entry->eax & GP_WIDTH_MASK) >> GP_WIDTH_OFS_BIT;
+}
+
+static uint32_t kvm_fixed_ctr_bit_width(void)
+{
+ const struct kvm_cpuid_entry2 *kvm_entry;
+
+ kvm_entry = get_cpuid_entry(kvm_get_supported_cpuid(), 0xa, 0);
+ return (kvm_entry->edx & FIXED_WIDTH_MASK) >> FIXED_WIDTH_OFS_BIT;
+}
+
static struct kvm_vcpu *new_vcpu(void *guest_code)
{
struct kvm_vm *vm;
@@ -381,6 +411,50 @@ static void test_pmu_version_setup(struct kvm_vcpu *vcpu, uint8_t version)
vm_install_exception_handler(vcpu->vm, GP_VECTOR, guest_gp_handler);
}
+static uint64_t test_ctrs_bit_width_setup(struct kvm_vcpu *vcpu,
+ uint8_t bit_width,
+ uint64_t perf_cap,
+ uint32_t msr_base)
+{
+ struct kvm_cpuid_entry2 *entry;
+ bool fw_wr = perf_cap & PMU_CAP_FW_WRITES;
+ uint64_t kvm_width;
+ uint64_t value;
+
+ entry = vcpu_get_cpuid_entry(vcpu, 0xa);
+ if (msr_base != MSR_CORE_PERF_FIXED_CTR0) {
+ kvm_width = kvm_gp_ctr_bit_width();
+ entry->eax = (entry->eax & ~GP_WIDTH_MASK) |
+ (bit_width << GP_WIDTH_OFS_BIT);
+ } else {
+ kvm_width = kvm_fixed_ctr_bit_width();
+ entry->edx = (entry->edx & ~FIXED_WIDTH_MASK) |
+ (bit_width << FIXED_WIDTH_OFS_BIT);
+ }
+ TEST_REQUIRE(kvm_width > 31);
+
+ vcpu_set_cpuid(vcpu);
+ vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, perf_cap);
+
+ /* No less than 32 bits and no greater than the host bitwidth */
+ bit_width = fw_wr ? max_t(int, 32, bit_width) : 32;
+ bit_width = min_t(int, bit_width, kvm_width);
+
+ /* Unconditionally set signed bit 31 for the case w/o FW_WRITES */
+ value = BIT_ULL(bit_width) | 0x1234567ull | BIT_ULL(31);
+ vcpu_args_set(vcpu, 4, msr_base, value, 1, 1);
+
+ if (fw_wr && msr_base != MSR_INTEL_ARCH_PMU_GPCTR) {
+ vm_install_exception_handler(vcpu->vm, GP_VECTOR,
+ guest_gp_handler);
+ return GP_VECTOR;
+ } else if (msr_base == MSR_INTEL_ARCH_PMU_GPCTR) {
+ value = (s32)(value & (BIT_ULL(32) - 1));
+ }
+
+ return value & (BIT_ULL(bit_width) - 1);
+}
+
static void intel_check_arch_event_is_unavl(uint8_t idx)
{
const char *msg = "Unavailable arch event is counting.";
@@ -497,12 +571,43 @@ static void intel_test_pmu_version(void)
}
}
+static void vcpu_run_bit_width(uint8_t bit_width, uint64_t perf_cap,
+ uint32_t msr_base)
+{
+ const char *msg = "Fail to emulate counters' bit width.";
+ struct kvm_vcpu *vcpu;
+ uint64_t ret;
+
+ vcpu = new_vcpu(guest_wr_and_rd_msrs);
+ ret = test_ctrs_bit_width_setup(vcpu, bit_width, perf_cap, msr_base);
+ run_vcpu(vcpu, msg, first_uc_arg_equals, (void *)ret);
+ free_vcpu(vcpu);
+}
+
+static void intel_test_counters_bit_width(void)
+{
+ uint8_t i, j, k;
+
+ for (i = 0; i < ARRAY_SIZE(perf_caps); i++) {
+ for (j = 0; j < ARRAY_SIZE(msr_bases); j++) {
+ if (!(perf_caps[i] & PMU_CAP_FW_WRITES) &&
+ msr_bases[j] == MSR_IA32_PMC0)
+ continue;
+
+ for (k = 0; k < ARRAY_SIZE(bit_widths); k++)
+ vcpu_run_bit_width(bit_widths[k], perf_caps[i],
+ msr_bases[j]);
+ }
+ }
+}
+
static void intel_test_pmu_cpuid(void)
{
intel_test_arch_events();
intel_test_counters_num();
intel_test_fixed_counters();
intel_test_pmu_version();
+ intel_test_counters_bit_width();
}
int main(int argc, char *argv[])
--
2.40.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 7/7] KVM: selftests: Test Intel counters' bit width emulation
2023-03-23 7:27 ` [PATCH 7/7] KVM: selftests: Test Intel counters' bit width emulation Like Xu
@ 2023-05-24 22:52 ` Sean Christopherson
0 siblings, 0 replies; 16+ messages in thread
From: Sean Christopherson @ 2023-05-24 22:52 UTC (permalink / raw)
To: Like Xu; +Cc: Paolo Bonzini, Jinrong Liang, linux-kernel, kvm
On Thu, Mar 23, 2023, Like Xu wrote:
> +static uint64_t test_ctrs_bit_width_setup(struct kvm_vcpu *vcpu,
> + uint8_t bit_width,
> + uint64_t perf_cap,
> + uint32_t msr_base)
> +{
> + struct kvm_cpuid_entry2 *entry;
> + bool fw_wr = perf_cap & PMU_CAP_FW_WRITES;
> + uint64_t kvm_width;
> + uint64_t value;
> +
> + entry = vcpu_get_cpuid_entry(vcpu, 0xa);
> + if (msr_base != MSR_CORE_PERF_FIXED_CTR0) {
> + kvm_width = kvm_gp_ctr_bit_width();
> + entry->eax = (entry->eax & ~GP_WIDTH_MASK) |
> + (bit_width << GP_WIDTH_OFS_BIT);
> + } else {
> + kvm_width = kvm_fixed_ctr_bit_width();
> + entry->edx = (entry->edx & ~FIXED_WIDTH_MASK) |
> + (bit_width << FIXED_WIDTH_OFS_BIT);
> + }
> + TEST_REQUIRE(kvm_width > 31);
Unfortunately, using TEST_REQUIRE() in a subtest is generally a bad idea. This
will skip _all_ tests if the requirement isn't met. That might be a signal that
the test is doing too much, i.e. should be split into multiple tests. Unlike KUT,
selftests are more geared towards lots of small tests, not a handful of massive
tests.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/7] KVM: selftests: Test the consistency of the PMU's CPUID and its features
2023-03-23 7:27 [PATCH 0/7] KVM: selftests: Test the consistency of the PMU's CPUID and its features Like Xu
` (6 preceding siblings ...)
2023-03-23 7:27 ` [PATCH 7/7] KVM: selftests: Test Intel counters' bit width emulation Like Xu
@ 2023-05-24 22:53 ` Sean Christopherson
7 siblings, 0 replies; 16+ messages in thread
From: Sean Christopherson @ 2023-05-24 22:53 UTC (permalink / raw)
To: Like Xu; +Cc: Paolo Bonzini, Jinrong Liang, linux-kernel, kvm
On Thu, Mar 23, 2023, Like Xu wrote:
> Hi,
>
> The KVM selfstests shows advantages over the KUT in terms of finding
> defects through flexible and varied guest settings form KVM user space.
>
> This patch set tests whether Intel vPMU works properly with different
> Intel CPUID.0xA configurations, in which three issues were identified.
> It also provides test scaffolding and a sufficient number of pmu test cases
> to subsequently provide adequate code coverage of AMD vPMU or Intel
> complex features such as LBR or PEBS in selftests.
>
> Please feel free to add more tests or share valuable comments.
>
> Related bugs:
> KVM: x86/pmu: Fix emulation on Intel counters' bit width
> (https://lore.kernel.org/kvm/20230322093117.48335-1-likexu@tencent.com/)
> KVM: x86/pmu: Add Intel PMU supported fixed counters bit mask
> (https://lore.kernel.org/kvm/20230321112742.25255-1-likexu@tencent.com/)
Can you send a single combined series for these fixes, plus the tests in this
series? I expect to apply the fixes before the tests, but I want to make it as
unlikely as possible that I forget and apply tests that fail.
Thanks!
^ permalink raw reply [flat|nested] 16+ messages in thread