[PATCH v6 0/6] KVM: selftests: Improve PMU event filter settings and add test cases

[PATCH v6 0/6] KVM: selftests: Improve PMU event filter settings and add test cases

[Jinrong Liang]

Hi,

This patch series aims to improve the PMU event filter settings with a cleaner
and more organized structure and adds several test cases related to PMU event
filters.

These changes help to ensure that KVM's PMU event filter functions as expected
in all supported use cases.

Any feedback or suggestions are greatly appreciated.

Sincerely,
Jinrong Liang

Changes log:

v6:
- Rebased to 74c2185c5b74(tag: kvm-x86-next-2023.08.02);
- Use TEST_ASSERT_EQ() instead of ASSERT_EQ();
- Fix a bug about x86 properties for Intel PMU in patch v5;

v5:
https://lore.kernel.org/kvm/20230720114714.34079-1-cloudliang@tencent.com/T/


Jinrong Liang (6):
  KVM: selftests: Add x86 properties for Intel PMU in processor.h
  KVM: selftests: Drop the return of remove_event()
  KVM: selftests: Introduce __kvm_pmu_event_filter to improved event
    filter settings
  KVM: selftests: Add test cases for unsupported PMU event filter input
    values
  KVM: selftests: Test if event filter meets expectations on fixed
    counters
  KVM: selftests: Test gp event filters don't affect fixed event filters

 .../selftests/kvm/include/x86_64/processor.h  |   5 +
 .../kvm/x86_64/pmu_event_filter_test.c        | 317 ++++++++++++------
 2 files changed, 228 insertions(+), 94 deletions(-)


base-commit: 74c2185c5b74fd0ae91133ad5afe8684f6a02b91
-- 
2.39.3

[PATCH v6 1/6] KVM: selftests: Add x86 properties for Intel PMU in processor.h

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

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

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

diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index aa434c8f19c5..4fd042112526 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -239,7 +239,12 @@ struct kvm_x86_cpu_property {
 #define X86_PROPERTY_MAX_BASIC_LEAF		KVM_X86_CPU_PROPERTY(0, 0, EAX, 0, 31)
 #define X86_PROPERTY_PMU_VERSION		KVM_X86_CPU_PROPERTY(0xa, 0, EAX, 0, 7)
 #define X86_PROPERTY_PMU_NR_GP_COUNTERS		KVM_X86_CPU_PROPERTY(0xa, 0, EAX, 8, 15)
+#define X86_PROPERTY_PMU_GP_COUNTERS_BIT_WIDTH	KVM_X86_CPU_PROPERTY(0xa, 0, EAX, 16, 23)
 #define X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH	KVM_X86_CPU_PROPERTY(0xa, 0, EAX, 24, 31)
+#define X86_PROPERTY_PMU_EVENTS_MASK		KVM_X86_CPU_PROPERTY(0xa, 0, EBX, 0, 7)
+#define X86_PROPERTY_PMU_FIXED_COUNTERS_BITMASK	KVM_X86_CPU_PROPERTY(0xa, 0, ECX, 0, 31)
+#define X86_PROPERTY_PMU_NR_FIXED_COUNTERS	KVM_X86_CPU_PROPERTY(0xa, 0, EDX, 0, 4)
+#define X86_PROPERTY_PMU_FIXED_COUNTERS_BIT_WIDTH	KVM_X86_CPU_PROPERTY(0xa, 0, EDX, 5, 12)
 
 #define X86_PROPERTY_SUPPORTED_XCR0_LO		KVM_X86_CPU_PROPERTY(0xd,  0, EAX,  0, 31)
 #define X86_PROPERTY_XSTATE_MAX_SIZE_XCR0	KVM_X86_CPU_PROPERTY(0xd,  0, EBX,  0, 31)
-- 
2.39.3

[PATCH v6 2/6] KVM: selftests: Drop the return of remove_event()

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

None of the callers consume remove_event(), and it incorrectly implies
that the incoming filter isn't modified. Drop the return.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
index 40507ed9fe8a..5ac05e64bec9 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
@@ -265,8 +265,7 @@ static struct kvm_pmu_event_filter *event_filter(uint32_t action)
  * Remove the first occurrence of 'event' (if any) from the filter's
  * event list.
  */
-static struct kvm_pmu_event_filter *remove_event(struct kvm_pmu_event_filter *f,
-						 uint64_t event)
+static void remove_event(struct kvm_pmu_event_filter *f, uint64_t event)
 {
 	bool found = false;
 	int i;
@@ -279,7 +278,6 @@ static struct kvm_pmu_event_filter *remove_event(struct kvm_pmu_event_filter *f,
 	}
 	if (found)
 		f->nevents--;
-	return f;
 }
 
 #define ASSERT_PMC_COUNTING_INSTRUCTIONS()						\
-- 
2.39.3

[PATCH v6 3/6] KVM: selftests: Introduce __kvm_pmu_event_filter to improved event filter settings

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

Add custom "__kvm_pmu_event_filter" structure to improve pmu event
filter settings. Simplifies event filter setup by organizing event
filter parameters in a cleaner, more organized way.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 .../kvm/x86_64/pmu_event_filter_test.c        | 182 +++++++++---------
 1 file changed, 90 insertions(+), 92 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
index 5ac05e64bec9..94f5a89aac40 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
@@ -28,6 +28,10 @@
 
 #define NUM_BRANCHES 42
 
+/* Matches KVM_PMU_EVENT_FILTER_MAX_EVENTS in pmu.c */
+#define MAX_FILTER_EVENTS		300
+#define MAX_TEST_EVENTS		10
+
 /*
  * This is how the event selector and unit mask are stored in an AMD
  * core performance event-select register. Intel's format is similar,
@@ -69,21 +73,33 @@
 
 #define INST_RETIRED EVENT(0xc0, 0)
 
+struct __kvm_pmu_event_filter {
+	__u32 action;
+	__u32 nevents;
+	__u32 fixed_counter_bitmap;
+	__u32 flags;
+	__u32 pad[4];
+	__u64 events[MAX_FILTER_EVENTS];
+};
+
 /*
  * This event list comprises Intel's eight architectural events plus
  * AMD's "retired branch instructions" for Zen[123] (and possibly
  * other AMD CPUs).
  */
-static const uint64_t event_list[] = {
-	EVENT(0x3c, 0),
-	INST_RETIRED,
-	EVENT(0x3c, 1),
-	EVENT(0x2e, 0x4f),
-	EVENT(0x2e, 0x41),
-	EVENT(0xc4, 0),
-	EVENT(0xc5, 0),
-	EVENT(0xa4, 1),
-	AMD_ZEN_BR_RETIRED,
+static const struct __kvm_pmu_event_filter base_event_filter = {
+	.nevents = ARRAY_SIZE(base_event_filter.events),
+	.events = {
+		EVENT(0x3c, 0),
+		INST_RETIRED,
+		EVENT(0x3c, 1),
+		EVENT(0x2e, 0x4f),
+		EVENT(0x2e, 0x41),
+		EVENT(0xc4, 0),
+		EVENT(0xc5, 0),
+		EVENT(0xa4, 1),
+		AMD_ZEN_BR_RETIRED,
+	},
 };
 
 struct {
@@ -225,47 +241,11 @@ static bool sanity_check_pmu(struct kvm_vcpu *vcpu)
 	return !r;
 }
 
-static struct kvm_pmu_event_filter *alloc_pmu_event_filter(uint32_t nevents)
-{
-	struct kvm_pmu_event_filter *f;
-	int size = sizeof(*f) + nevents * sizeof(f->events[0]);
-
-	f = malloc(size);
-	TEST_ASSERT(f, "Out of memory");
-	memset(f, 0, size);
-	f->nevents = nevents;
-	return f;
-}
-
-
-static struct kvm_pmu_event_filter *
-create_pmu_event_filter(const uint64_t event_list[], int nevents,
-			uint32_t action, uint32_t flags)
-{
-	struct kvm_pmu_event_filter *f;
-	int i;
-
-	f = alloc_pmu_event_filter(nevents);
-	f->action = action;
-	f->flags = flags;
-	for (i = 0; i < nevents; i++)
-		f->events[i] = event_list[i];
-
-	return f;
-}
-
-static struct kvm_pmu_event_filter *event_filter(uint32_t action)
-{
-	return create_pmu_event_filter(event_list,
-				       ARRAY_SIZE(event_list),
-				       action, 0);
-}
-
 /*
  * Remove the first occurrence of 'event' (if any) from the filter's
  * event list.
  */
-static void remove_event(struct kvm_pmu_event_filter *f, uint64_t event)
+static void remove_event(struct __kvm_pmu_event_filter *f, uint64_t event)
 {
 	bool found = false;
 	int i;
@@ -313,66 +293,73 @@ static void test_without_filter(struct kvm_vcpu *vcpu)
 }
 
 static void test_with_filter(struct kvm_vcpu *vcpu,
-			     struct kvm_pmu_event_filter *f)
+			     struct __kvm_pmu_event_filter *__f)
 {
+	struct kvm_pmu_event_filter *f = (void *)__f;
+
 	vm_ioctl(vcpu->vm, KVM_SET_PMU_EVENT_FILTER, f);
 	run_vcpu_and_sync_pmc_results(vcpu);
 }
 
 static void test_amd_deny_list(struct kvm_vcpu *vcpu)
 {
-	uint64_t event = EVENT(0x1C2, 0);
-	struct kvm_pmu_event_filter *f;
+	struct __kvm_pmu_event_filter f = {
+		.action = KVM_PMU_EVENT_DENY,
+		.nevents = 1,
+		.events = {
+			EVENT(0x1C2, 0),
+		},
+	};
 
-	f = create_pmu_event_filter(&event, 1, KVM_PMU_EVENT_DENY, 0);
-	test_with_filter(vcpu, f);
-	free(f);
+	test_with_filter(vcpu, &f);
 
 	ASSERT_PMC_COUNTING_INSTRUCTIONS();
 }
 
 static void test_member_deny_list(struct kvm_vcpu *vcpu)
 {
-	struct kvm_pmu_event_filter *f = event_filter(KVM_PMU_EVENT_DENY);
+	struct __kvm_pmu_event_filter f = base_event_filter;
 
-	test_with_filter(vcpu, f);
-	free(f);
+	f.action = KVM_PMU_EVENT_DENY;
+	test_with_filter(vcpu, &f);
 
 	ASSERT_PMC_NOT_COUNTING_INSTRUCTIONS();
 }
 
 static void test_member_allow_list(struct kvm_vcpu *vcpu)
 {
-	struct kvm_pmu_event_filter *f = event_filter(KVM_PMU_EVENT_ALLOW);
+	struct __kvm_pmu_event_filter f = base_event_filter;
 
-	test_with_filter(vcpu, f);
-	free(f);
+	f.action = KVM_PMU_EVENT_ALLOW;
+	test_with_filter(vcpu, &f);
 
 	ASSERT_PMC_COUNTING_INSTRUCTIONS();
 }
 
 static void test_not_member_deny_list(struct kvm_vcpu *vcpu)
 {
-	struct kvm_pmu_event_filter *f = event_filter(KVM_PMU_EVENT_DENY);
+	struct __kvm_pmu_event_filter f = base_event_filter;
 
-	remove_event(f, INST_RETIRED);
-	remove_event(f, INTEL_BR_RETIRED);
-	remove_event(f, AMD_ZEN_BR_RETIRED);
-	test_with_filter(vcpu, f);
-	free(f);
+	f.action = KVM_PMU_EVENT_DENY;
+
+	remove_event(&f, INST_RETIRED);
+	remove_event(&f, INTEL_BR_RETIRED);
+	remove_event(&f, AMD_ZEN_BR_RETIRED);
+	test_with_filter(vcpu, &f);
 
 	ASSERT_PMC_COUNTING_INSTRUCTIONS();
 }
 
 static void test_not_member_allow_list(struct kvm_vcpu *vcpu)
 {
-	struct kvm_pmu_event_filter *f = event_filter(KVM_PMU_EVENT_ALLOW);
+	struct __kvm_pmu_event_filter f = base_event_filter;
+
+	f.action = KVM_PMU_EVENT_ALLOW;
 
-	remove_event(f, INST_RETIRED);
-	remove_event(f, INTEL_BR_RETIRED);
-	remove_event(f, AMD_ZEN_BR_RETIRED);
-	test_with_filter(vcpu, f);
-	free(f);
+	remove_event(&f, INST_RETIRED);
+	remove_event(&f, INTEL_BR_RETIRED);
+	remove_event(&f, AMD_ZEN_BR_RETIRED);
+	test_with_filter(vcpu, &f);
 
 	ASSERT_PMC_NOT_COUNTING_INSTRUCTIONS();
 }
@@ -567,19 +554,16 @@ static void run_masked_events_test(struct kvm_vcpu *vcpu,
 				   const uint64_t masked_events[],
 				   const int nmasked_events)
 {
-	struct kvm_pmu_event_filter *f;
+	struct __kvm_pmu_event_filter f = {
+		.nevents = nmasked_events,
+		.action = KVM_PMU_EVENT_ALLOW,
+		.flags = KVM_PMU_EVENT_FLAG_MASKED_EVENTS,
+	};
 
-	f = create_pmu_event_filter(masked_events, nmasked_events,
-				    KVM_PMU_EVENT_ALLOW,
-				    KVM_PMU_EVENT_FLAG_MASKED_EVENTS);
-	test_with_filter(vcpu, f);
-	free(f);
+	memcpy(f.events, masked_events, sizeof(uint64_t) * nmasked_events);
+	test_with_filter(vcpu, &f);
 }
 
-/* Matches KVM_PMU_EVENT_FILTER_MAX_EVENTS in pmu.c */
-#define MAX_FILTER_EVENTS	300
-#define MAX_TEST_EVENTS		10
-
 #define ALLOW_LOADS		BIT(0)
 #define ALLOW_STORES		BIT(1)
 #define ALLOW_LOADS_STORES	BIT(2)
@@ -751,17 +735,27 @@ static void test_masked_events(struct kvm_vcpu *vcpu)
 	run_masked_events_tests(vcpu, events, nevents);
 }
 
-static int run_filter_test(struct kvm_vcpu *vcpu, const uint64_t *events,
-			   int nevents, uint32_t flags)
+static int do_vcpu_set_pmu_event_filter(struct kvm_vcpu *vcpu,
+					struct __kvm_pmu_event_filter *__f)
 {
-	struct kvm_pmu_event_filter *f;
-	int r;
+	struct kvm_pmu_event_filter *f = (void *)__f;
 
-	f = create_pmu_event_filter(events, nevents, KVM_PMU_EVENT_ALLOW, flags);
-	r = __vm_ioctl(vcpu->vm, KVM_SET_PMU_EVENT_FILTER, f);
-	free(f);
+	return __vm_ioctl(vcpu->vm, KVM_SET_PMU_EVENT_FILTER, f);
+}
+
+static int set_pmu_single_event_filter(struct kvm_vcpu *vcpu, uint64_t event,
+				       uint32_t flags, uint32_t action)
+{
+	struct __kvm_pmu_event_filter f = {
+		.nevents = 1,
+		.flags = flags,
+		.action = action,
+		.events = {
+			event,
+		},
+	};
 
-	return r;
+	return do_vcpu_set_pmu_event_filter(vcpu, &f);
 }
 
 static void test_filter_ioctl(struct kvm_vcpu *vcpu)
@@ -773,14 +767,18 @@ static void test_filter_ioctl(struct kvm_vcpu *vcpu)
 	 * Unfortunately having invalid bits set in event data is expected to
 	 * pass when flags == 0 (bits other than eventsel+umask).
 	 */
-	r = run_filter_test(vcpu, &e, 1, 0);
+	r = set_pmu_single_event_filter(vcpu, e, 0, KVM_PMU_EVENT_ALLOW);
 	TEST_ASSERT(r == 0, "Valid PMU Event Filter is failing");
 
-	r = run_filter_test(vcpu, &e, 1, KVM_PMU_EVENT_FLAG_MASKED_EVENTS);
+	r = set_pmu_single_event_filter(vcpu, e,
+					KVM_PMU_EVENT_FLAG_MASKED_EVENTS,
+					KVM_PMU_EVENT_ALLOW);
 	TEST_ASSERT(r != 0, "Invalid PMU Event Filter is expected to fail");
 
 	e = KVM_PMU_ENCODE_MASKED_ENTRY(0xff, 0xff, 0xff, 0xf);
-	r = run_filter_test(vcpu, &e, 1, KVM_PMU_EVENT_FLAG_MASKED_EVENTS);
+	r = set_pmu_single_event_filter(vcpu, e,
+					KVM_PMU_EVENT_FLAG_MASKED_EVENTS,
+					KVM_PMU_EVENT_ALLOW);
 	TEST_ASSERT(r == 0, "Valid PMU Event Filter is failing");
 }
 
-- 
2.39.3

Re: [PATCH v6 3/6] KVM: selftests: Introduce __kvm_pmu_event_filter to improved event filter settings

[JinrongLiang]

在 2023/8/10 17:09, Jinrong Liang 写道:
> From: Jinrong Liang <cloudliang@tencent.com>
> 
> Add custom "__kvm_pmu_event_filter" structure to improve pmu event
> filter settings. Simplifies event filter setup by organizing event
> filter parameters in a cleaner, more organized way.

I apologize for the oversight in this patch submission. I forgot to 
include the "Reviewed-by" tag. Please find the updated information below:

Reviewed-by: Isaku Yamahata <isaku.yamahata@intel.com>

When reviewing the patch, please take this tag into account.

> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
> ---
>   .../kvm/x86_64/pmu_event_filter_test.c        | 182 +++++++++---------
>   1 file changed, 90 insertions(+), 92 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> index 5ac05e64bec9..94f5a89aac40 100644
> --- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> @@ -28,6 +28,10 @@
>   
>   #define NUM_BRANCHES 42
>   
> +/* Matches KVM_PMU_EVENT_FILTER_MAX_EVENTS in pmu.c */
> +#define MAX_FILTER_EVENTS		300
> +#define MAX_TEST_EVENTS		10
> +
>   /*
>    * This is how the event selector and unit mask are stored in an AMD
>    * core performance event-select register. Intel's format is similar,
> @@ -69,21 +73,33 @@
>   
>   #define INST_RETIRED EVENT(0xc0, 0)
>   
> +struct __kvm_pmu_event_filter {
> +	__u32 action;
> +	__u32 nevents;
> +	__u32 fixed_counter_bitmap;
> +	__u32 flags;
> +	__u32 pad[4];
> +	__u64 events[MAX_FILTER_EVENTS];
> +};
> +
>   /*
>    * This event list comprises Intel's eight architectural events plus
>    * AMD's "retired branch instructions" for Zen[123] (and possibly
>    * other AMD CPUs).
>    */
> -static const uint64_t event_list[] = {
> -	EVENT(0x3c, 0),
> -	INST_RETIRED,
> -	EVENT(0x3c, 1),
> -	EVENT(0x2e, 0x4f),
> -	EVENT(0x2e, 0x41),
> -	EVENT(0xc4, 0),
> -	EVENT(0xc5, 0),
> -	EVENT(0xa4, 1),
> -	AMD_ZEN_BR_RETIRED,
> +static const struct __kvm_pmu_event_filter base_event_filter = {
> +	.nevents = ARRAY_SIZE(base_event_filter.events),
> +	.events = {
> +		EVENT(0x3c, 0),
> +		INST_RETIRED,
> +		EVENT(0x3c, 1),
> +		EVENT(0x2e, 0x4f),
> +		EVENT(0x2e, 0x41),
> +		EVENT(0xc4, 0),
> +		EVENT(0xc5, 0),
> +		EVENT(0xa4, 1),
> +		AMD_ZEN_BR_RETIRED,
> +	},
>   };
>   
>   struct {
> @@ -225,47 +241,11 @@ static bool sanity_check_pmu(struct kvm_vcpu *vcpu)
>   	return !r;
>   }
>   
> -static struct kvm_pmu_event_filter *alloc_pmu_event_filter(uint32_t nevents)
> -{
> -	struct kvm_pmu_event_filter *f;
> -	int size = sizeof(*f) + nevents * sizeof(f->events[0]);
> -
> -	f = malloc(size);
> -	TEST_ASSERT(f, "Out of memory");
> -	memset(f, 0, size);
> -	f->nevents = nevents;
> -	return f;
> -}
> -
> -
> -static struct kvm_pmu_event_filter *
> -create_pmu_event_filter(const uint64_t event_list[], int nevents,
> -			uint32_t action, uint32_t flags)
> -{
> -	struct kvm_pmu_event_filter *f;
> -	int i;
> -
> -	f = alloc_pmu_event_filter(nevents);
> -	f->action = action;
> -	f->flags = flags;
> -	for (i = 0; i < nevents; i++)
> -		f->events[i] = event_list[i];
> -
> -	return f;
> -}
> -
> -static struct kvm_pmu_event_filter *event_filter(uint32_t action)
> -{
> -	return create_pmu_event_filter(event_list,
> -				       ARRAY_SIZE(event_list),
> -				       action, 0);
> -}
> -
>   /*
>    * Remove the first occurrence of 'event' (if any) from the filter's
>    * event list.
>    */
> -static void remove_event(struct kvm_pmu_event_filter *f, uint64_t event)
> +static void remove_event(struct __kvm_pmu_event_filter *f, uint64_t event)
>   {
>   	bool found = false;
>   	int i;
> @@ -313,66 +293,73 @@ static void test_without_filter(struct kvm_vcpu *vcpu)
>   }
>   
>   static void test_with_filter(struct kvm_vcpu *vcpu,
> -			     struct kvm_pmu_event_filter *f)
> +			     struct __kvm_pmu_event_filter *__f)
>   {
> +	struct kvm_pmu_event_filter *f = (void *)__f;
> +
>   	vm_ioctl(vcpu->vm, KVM_SET_PMU_EVENT_FILTER, f);
>   	run_vcpu_and_sync_pmc_results(vcpu);
>   }
>   
>   static void test_amd_deny_list(struct kvm_vcpu *vcpu)
>   {
> -	uint64_t event = EVENT(0x1C2, 0);
> -	struct kvm_pmu_event_filter *f;
> +	struct __kvm_pmu_event_filter f = {
> +		.action = KVM_PMU_EVENT_DENY,
> +		.nevents = 1,
> +		.events = {
> +			EVENT(0x1C2, 0),
> +		},
> +	};
>   
> -	f = create_pmu_event_filter(&event, 1, KVM_PMU_EVENT_DENY, 0);
> -	test_with_filter(vcpu, f);
> -	free(f);
> +	test_with_filter(vcpu, &f);
>   
>   	ASSERT_PMC_COUNTING_INSTRUCTIONS();
>   }
>   
>   static void test_member_deny_list(struct kvm_vcpu *vcpu)
>   {
> -	struct kvm_pmu_event_filter *f = event_filter(KVM_PMU_EVENT_DENY);
> +	struct __kvm_pmu_event_filter f = base_event_filter;
>   
> -	test_with_filter(vcpu, f);
> -	free(f);
> +	f.action = KVM_PMU_EVENT_DENY;
> +	test_with_filter(vcpu, &f);
>   
>   	ASSERT_PMC_NOT_COUNTING_INSTRUCTIONS();
>   }
>   
>   static void test_member_allow_list(struct kvm_vcpu *vcpu)
>   {
> -	struct kvm_pmu_event_filter *f = event_filter(KVM_PMU_EVENT_ALLOW);
> +	struct __kvm_pmu_event_filter f = base_event_filter;
>   
> -	test_with_filter(vcpu, f);
> -	free(f);
> +	f.action = KVM_PMU_EVENT_ALLOW;
> +	test_with_filter(vcpu, &f);
>   
>   	ASSERT_PMC_COUNTING_INSTRUCTIONS();
>   }
>   
>   static void test_not_member_deny_list(struct kvm_vcpu *vcpu)
>   {
> -	struct kvm_pmu_event_filter *f = event_filter(KVM_PMU_EVENT_DENY);
> +	struct __kvm_pmu_event_filter f = base_event_filter;
>   
> -	remove_event(f, INST_RETIRED);
> -	remove_event(f, INTEL_BR_RETIRED);
> -	remove_event(f, AMD_ZEN_BR_RETIRED);
> -	test_with_filter(vcpu, f);
> -	free(f);
> +	f.action = KVM_PMU_EVENT_DENY;
> +
> +	remove_event(&f, INST_RETIRED);
> +	remove_event(&f, INTEL_BR_RETIRED);
> +	remove_event(&f, AMD_ZEN_BR_RETIRED);
> +	test_with_filter(vcpu, &f);
>   
>   	ASSERT_PMC_COUNTING_INSTRUCTIONS();
>   }
>   
>   static void test_not_member_allow_list(struct kvm_vcpu *vcpu)
>   {
> -	struct kvm_pmu_event_filter *f = event_filter(KVM_PMU_EVENT_ALLOW);
> +	struct __kvm_pmu_event_filter f = base_event_filter;
> +
> +	f.action = KVM_PMU_EVENT_ALLOW;
>   
> -	remove_event(f, INST_RETIRED);
> -	remove_event(f, INTEL_BR_RETIRED);
> -	remove_event(f, AMD_ZEN_BR_RETIRED);
> -	test_with_filter(vcpu, f);
> -	free(f);
> +	remove_event(&f, INST_RETIRED);
> +	remove_event(&f, INTEL_BR_RETIRED);
> +	remove_event(&f, AMD_ZEN_BR_RETIRED);
> +	test_with_filter(vcpu, &f);
>   
>   	ASSERT_PMC_NOT_COUNTING_INSTRUCTIONS();
>   }
> @@ -567,19 +554,16 @@ static void run_masked_events_test(struct kvm_vcpu *vcpu,
>   				   const uint64_t masked_events[],
>   				   const int nmasked_events)
>   {
> -	struct kvm_pmu_event_filter *f;
> +	struct __kvm_pmu_event_filter f = {
> +		.nevents = nmasked_events,
> +		.action = KVM_PMU_EVENT_ALLOW,
> +		.flags = KVM_PMU_EVENT_FLAG_MASKED_EVENTS,
> +	};
>   
> -	f = create_pmu_event_filter(masked_events, nmasked_events,
> -				    KVM_PMU_EVENT_ALLOW,
> -				    KVM_PMU_EVENT_FLAG_MASKED_EVENTS);
> -	test_with_filter(vcpu, f);
> -	free(f);
> +	memcpy(f.events, masked_events, sizeof(uint64_t) * nmasked_events);
> +	test_with_filter(vcpu, &f);
>   }
>   
> -/* Matches KVM_PMU_EVENT_FILTER_MAX_EVENTS in pmu.c */
> -#define MAX_FILTER_EVENTS	300
> -#define MAX_TEST_EVENTS		10
> -
>   #define ALLOW_LOADS		BIT(0)
>   #define ALLOW_STORES		BIT(1)
>   #define ALLOW_LOADS_STORES	BIT(2)
> @@ -751,17 +735,27 @@ static void test_masked_events(struct kvm_vcpu *vcpu)
>   	run_masked_events_tests(vcpu, events, nevents);
>   }
>   
> -static int run_filter_test(struct kvm_vcpu *vcpu, const uint64_t *events,
> -			   int nevents, uint32_t flags)
> +static int do_vcpu_set_pmu_event_filter(struct kvm_vcpu *vcpu,
> +					struct __kvm_pmu_event_filter *__f)
>   {
> -	struct kvm_pmu_event_filter *f;
> -	int r;
> +	struct kvm_pmu_event_filter *f = (void *)__f;
>   
> -	f = create_pmu_event_filter(events, nevents, KVM_PMU_EVENT_ALLOW, flags);
> -	r = __vm_ioctl(vcpu->vm, KVM_SET_PMU_EVENT_FILTER, f);
> -	free(f);
> +	return __vm_ioctl(vcpu->vm, KVM_SET_PMU_EVENT_FILTER, f);
> +}
> +
> +static int set_pmu_single_event_filter(struct kvm_vcpu *vcpu, uint64_t event,
> +				       uint32_t flags, uint32_t action)
> +{
> +	struct __kvm_pmu_event_filter f = {
> +		.nevents = 1,
> +		.flags = flags,
> +		.action = action,
> +		.events = {
> +			event,
> +		},
> +	};
>   
> -	return r;
> +	return do_vcpu_set_pmu_event_filter(vcpu, &f);
>   }
>   
>   static void test_filter_ioctl(struct kvm_vcpu *vcpu)
> @@ -773,14 +767,18 @@ static void test_filter_ioctl(struct kvm_vcpu *vcpu)
>   	 * Unfortunately having invalid bits set in event data is expected to
>   	 * pass when flags == 0 (bits other than eventsel+umask).
>   	 */
> -	r = run_filter_test(vcpu, &e, 1, 0);
> +	r = set_pmu_single_event_filter(vcpu, e, 0, KVM_PMU_EVENT_ALLOW);
>   	TEST_ASSERT(r == 0, "Valid PMU Event Filter is failing");
>   
> -	r = run_filter_test(vcpu, &e, 1, KVM_PMU_EVENT_FLAG_MASKED_EVENTS);
> +	r = set_pmu_single_event_filter(vcpu, e,
> +					KVM_PMU_EVENT_FLAG_MASKED_EVENTS,
> +					KVM_PMU_EVENT_ALLOW);
>   	TEST_ASSERT(r != 0, "Invalid PMU Event Filter is expected to fail");
>   
>   	e = KVM_PMU_ENCODE_MASKED_ENTRY(0xff, 0xff, 0xff, 0xf);
> -	r = run_filter_test(vcpu, &e, 1, KVM_PMU_EVENT_FLAG_MASKED_EVENTS);
> +	r = set_pmu_single_event_filter(vcpu, e,
> +					KVM_PMU_EVENT_FLAG_MASKED_EVENTS,
> +					KVM_PMU_EVENT_ALLOW);
>   	TEST_ASSERT(r == 0, "Valid PMU Event Filter is failing");
>   }
>   

Re: [PATCH v6 3/6] KVM: selftests: Introduce __kvm_pmu_event_filter to improved event filter settings

[Isaku Yamahata]

On Thu, Aug 10, 2023 at 05:09:42PM +0800,
Jinrong Liang <ljr.kernel@gmail.com> wrote:

> From: Jinrong Liang <cloudliang@tencent.com>
> 
> Add custom "__kvm_pmu_event_filter" structure to improve pmu event
> filter settings. Simplifies event filter setup by organizing event
> filter parameters in a cleaner, more organized way.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
> ---
>  .../kvm/x86_64/pmu_event_filter_test.c        | 182 +++++++++---------
>  1 file changed, 90 insertions(+), 92 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> index 5ac05e64bec9..94f5a89aac40 100644
> --- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> @@ -28,6 +28,10 @@
>  
>  #define NUM_BRANCHES 42
>  
> +/* Matches KVM_PMU_EVENT_FILTER_MAX_EVENTS in pmu.c */
> +#define MAX_FILTER_EVENTS		300

Can we simply use KVM_PMU_EVENT_FILTER_MAX_EVENTS and remove MAX_FILTER_EVENTS?


> +#define MAX_TEST_EVENTS		10
> +
>  /*
>   * This is how the event selector and unit mask are stored in an AMD
>   * core performance event-select register. Intel's format is similar,
> @@ -69,21 +73,33 @@
>  
>  #define INST_RETIRED EVENT(0xc0, 0)
>  
> +struct __kvm_pmu_event_filter {
> +	__u32 action;
> +	__u32 nevents;
> +	__u32 fixed_counter_bitmap;
> +	__u32 flags;
> +	__u32 pad[4];
> +	__u64 events[MAX_FILTER_EVENTS];
> +};
> +

Is this same to struct kvm_pmu_event_filter?

Except two trivial issue, looks good to me.
-- 
Isaku Yamahata <isaku.yamahata@gmail.com>

Re: [PATCH v6 3/6] KVM: selftests: Introduce __kvm_pmu_event_filter to improved event filter settings

[Jinrong Liang]

Isaku Yamahata <isaku.yamahata@gmail.com> 于2023年8月15日周二 07:49写道：
>
> On Thu, Aug 10, 2023 at 05:09:42PM +0800,
> Jinrong Liang <ljr.kernel@gmail.com> wrote:
>
> > From: Jinrong Liang <cloudliang@tencent.com>
> >
> > Add custom "__kvm_pmu_event_filter" structure to improve pmu event
> > filter settings. Simplifies event filter setup by organizing event
> > filter parameters in a cleaner, more organized way.
> >
> > Suggested-by: Sean Christopherson <seanjc@google.com>
> > Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
> > ---
> >  .../kvm/x86_64/pmu_event_filter_test.c        | 182 +++++++++---------
> >  1 file changed, 90 insertions(+), 92 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> > index 5ac05e64bec9..94f5a89aac40 100644
> > --- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> > +++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> > @@ -28,6 +28,10 @@
> >
> >  #define NUM_BRANCHES 42
> >
> > +/* Matches KVM_PMU_EVENT_FILTER_MAX_EVENTS in pmu.c */
> > +#define MAX_FILTER_EVENTS            300
>
> Can we simply use KVM_PMU_EVENT_FILTER_MAX_EVENTS and remove MAX_FILTER_EVENTS?

I didn't find the definition of KVM_PMU_EVENT_FILTER_MAX_EVENTS in
selftests. KVM_PMU_EVENT_FILTER_MAX_EVENTS is defined in pmu.c. To use
it, we need to define it in selftests.

>
>
> > +#define MAX_TEST_EVENTS              10
> > +
> >  /*
> >   * This is how the event selector and unit mask are stored in an AMD
> >   * core performance event-select register. Intel's format is similar,
> > @@ -69,21 +73,33 @@
> >
> >  #define INST_RETIRED EVENT(0xc0, 0)
> >
> > +struct __kvm_pmu_event_filter {
> > +     __u32 action;
> > +     __u32 nevents;
> > +     __u32 fixed_counter_bitmap;
> > +     __u32 flags;
> > +     __u32 pad[4];
> > +     __u64 events[MAX_FILTER_EVENTS];
> > +};
> > +
>
> Is this same to struct kvm_pmu_event_filter?

In tools/arch/x86/include/uapi/asm/kvm.h

/* for KVM_CAP_PMU_EVENT_FILTER */
struct kvm_pmu_event_filter {
__u32 action;
__u32 nevents;
__u32 fixed_counter_bitmap;
__u32 flags;
__u32 pad[4];
__u64 events[];
};

>
> Except two trivial issue, looks good to me.
> --
> Isaku Yamahata <isaku.yamahata@gmail.com>

Re: [PATCH v6 3/6] KVM: selftests: Introduce __kvm_pmu_event_filter to improved event filter settings

[Sean Christopherson]

On Tue, Aug 15, 2023, Jinrong Liang wrote:
> Isaku Yamahata <isaku.yamahata@gmail.com> 于2023年8月15日周二 07:49写道：
> >
> > On Thu, Aug 10, 2023 at 05:09:42PM +0800,
> > Jinrong Liang <ljr.kernel@gmail.com> wrote:
> >
> > > From: Jinrong Liang <cloudliang@tencent.com>
> > >
> > > Add custom "__kvm_pmu_event_filter" structure to improve pmu event
> > > filter settings. Simplifies event filter setup by organizing event
> > > filter parameters in a cleaner, more organized way.
> > >
> > > Suggested-by: Sean Christopherson <seanjc@google.com>
> > > Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
> > > ---
> > >  .../kvm/x86_64/pmu_event_filter_test.c        | 182 +++++++++---------
> > >  1 file changed, 90 insertions(+), 92 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> > > index 5ac05e64bec9..94f5a89aac40 100644
> > > --- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> > > +++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> > > @@ -28,6 +28,10 @@
> > >
> > >  #define NUM_BRANCHES 42
> > >
> > > +/* Matches KVM_PMU_EVENT_FILTER_MAX_EVENTS in pmu.c */
> > > +#define MAX_FILTER_EVENTS            300
> >
> > Can we simply use KVM_PMU_EVENT_FILTER_MAX_EVENTS and remove MAX_FILTER_EVENTS?
> 
> I didn't find the definition of KVM_PMU_EVENT_FILTER_MAX_EVENTS in
> selftests. KVM_PMU_EVENT_FILTER_MAX_EVENTS is defined in pmu.c. To use
> it, we need to define it in selftests.

Huh.  That seems like something that should be enumerated to userspace.

> > > +#define MAX_TEST_EVENTS              10
> > > +
> > >  /*
> > >   * This is how the event selector and unit mask are stored in an AMD
> > >   * core performance event-select register. Intel's format is similar,
> > > @@ -69,21 +73,33 @@
> > >
> > >  #define INST_RETIRED EVENT(0xc0, 0)
> > >
> > > +struct __kvm_pmu_event_filter {
> > > +     __u32 action;
> > > +     __u32 nevents;
> > > +     __u32 fixed_counter_bitmap;
> > > +     __u32 flags;
> > > +     __u32 pad[4];
> > > +     __u64 events[MAX_FILTER_EVENTS];
> > > +};
> > > +
> >
> > Is this same to struct kvm_pmu_event_filter?
> 
> In tools/arch/x86/include/uapi/asm/kvm.h
> 
> /* for KVM_CAP_PMU_EVENT_FILTER */
> struct kvm_pmu_event_filter {
> __u32 action;
> __u32 nevents;
> __u32 fixed_counter_bitmap;
> __u32 flags;
> __u32 pad[4];
> __u64 events[];
> };

To more directly answer Isaku's question:

They're *basically* the same, and have an identical layout, but the struct defined
by KVM uses a flexible array because the number of events comes from userspace
and forcing userspace to create an 1KiB+ object just to define a single event
filter would be obnoxious.

There are alternatives, e.g. using an struct overlay to set a single entry:

	struct {
		struct kvm_msrs header;
		struct kvm_msr_entry entry;
	} buffer = {};

	memset(&buffer, 0, sizeof(buffer));
	buffer.header.nmsrs = 1;
	buffer.entry.index = msr_index;
	buffer.entry.data = msr_value;

but that gets annoying (and IMO confusing) because of the nested structs.

I'll massage the changelog to callout the alternative, and why it's undesirable.

Re: [PATCH v6 3/6] KVM: selftests: Introduce __kvm_pmu_event_filter to improved event filter settings

[Sean Christopherson]

On Thu, Aug 10, 2023, Jinrong Liang wrote:
> -static int run_filter_test(struct kvm_vcpu *vcpu, const uint64_t *events,
> -			   int nevents, uint32_t flags)
> +static int do_vcpu_set_pmu_event_filter(struct kvm_vcpu *vcpu,
> +					struct __kvm_pmu_event_filter *__f)

Just "set_pmu_event_filter()" please.  The "do" is pointless, and the "vcpu" part
is confusing since the filter is per-VM, the selftest just happens to pass around
the vCPU.  And to be consistent with set_pmu_single_event_filter().

No need for a new version, I'll fixup when applying.

[PATCH v6 4/6] KVM: selftests: Add test cases for unsupported PMU event filter input values

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

Add test cases to verify the handling of unsupported input values for the
PMU event filter. The tests cover unsupported "action" values, unsupported
"flags" values, and unsupported "nevents" values. All these cases should
return an error, as they are currently not supported by the filter.
Furthermore, the tests also cover the case where setting non-existent
fixed counters in the fixed bitmap does not fail.

Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 .../kvm/x86_64/pmu_event_filter_test.c        | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
index 94f5a89aac40..8b8bfee11016 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
@@ -32,6 +32,10 @@
 #define MAX_FILTER_EVENTS		300
 #define MAX_TEST_EVENTS		10
 
+#define PMU_EVENT_FILTER_INVALID_ACTION		(KVM_PMU_EVENT_DENY + 1)
+#define PMU_EVENT_FILTER_INVALID_FLAGS			(KVM_PMU_EVENT_FLAGS_VALID_MASK << 1)
+#define PMU_EVENT_FILTER_INVALID_NEVENTS		(MAX_FILTER_EVENTS + 1)
+
 /*
  * This is how the event selector and unit mask are stored in an AMD
  * core performance event-select register. Intel's format is similar,
@@ -760,6 +764,8 @@ static int set_pmu_single_event_filter(struct kvm_vcpu *vcpu, uint64_t event,
 
 static void test_filter_ioctl(struct kvm_vcpu *vcpu)
 {
+	uint8_t nr_fixed_counters = kvm_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS);
+	struct __kvm_pmu_event_filter f;
 	uint64_t e = ~0ul;
 	int r;
 
@@ -780,6 +786,26 @@ static void test_filter_ioctl(struct kvm_vcpu *vcpu)
 					KVM_PMU_EVENT_FLAG_MASKED_EVENTS,
 					KVM_PMU_EVENT_ALLOW);
 	TEST_ASSERT(r == 0, "Valid PMU Event Filter is failing");
+
+	f = base_event_filter;
+	f.action = PMU_EVENT_FILTER_INVALID_ACTION;
+	r = do_vcpu_set_pmu_event_filter(vcpu, &f);
+	TEST_ASSERT(r, "Set invalid action is expected to fail");
+
+	f = base_event_filter;
+	f.flags = PMU_EVENT_FILTER_INVALID_FLAGS;
+	r = do_vcpu_set_pmu_event_filter(vcpu, &f);
+	TEST_ASSERT(r, "Set invalid flags is expected to fail");
+
+	f = base_event_filter;
+	f.nevents = PMU_EVENT_FILTER_INVALID_NEVENTS;
+	r = do_vcpu_set_pmu_event_filter(vcpu, &f);
+	TEST_ASSERT(r, "Exceeding the max number of filter events should fail");
+
+	f = base_event_filter;
+	f.fixed_counter_bitmap = ~GENMASK_ULL(nr_fixed_counters, 0);
+	r = do_vcpu_set_pmu_event_filter(vcpu, &f);
+	TEST_ASSERT(!r, "Masking non-existent fixed counters should be allowed");
 }
 
 int main(int argc, char *argv[])
-- 
2.39.3

Re: [PATCH v6 4/6] KVM: selftests: Add test cases for unsupported PMU event filter input values

[Isaku Yamahata]

On Thu, Aug 10, 2023 at 05:09:43PM +0800,
Jinrong Liang <ljr.kernel@gmail.com> wrote:

> From: Jinrong Liang <cloudliang@tencent.com>
> 
> Add test cases to verify the handling of unsupported input values for the
> PMU event filter. The tests cover unsupported "action" values, unsupported
> "flags" values, and unsupported "nevents" values. All these cases should
> return an error, as they are currently not supported by the filter.
> Furthermore, the tests also cover the case where setting non-existent
> fixed counters in the fixed bitmap does not fail.
> 
> Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
> ---
>  .../kvm/x86_64/pmu_event_filter_test.c        | 26 +++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> index 94f5a89aac40..8b8bfee11016 100644
> --- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> @@ -32,6 +32,10 @@
>  #define MAX_FILTER_EVENTS		300
>  #define MAX_TEST_EVENTS		10
>  
> +#define PMU_EVENT_FILTER_INVALID_ACTION		(KVM_PMU_EVENT_DENY + 1)
> +#define PMU_EVENT_FILTER_INVALID_FLAGS			(KVM_PMU_EVENT_FLAGS_VALID_MASK << 1)
> +#define PMU_EVENT_FILTER_INVALID_NEVENTS		(MAX_FILTER_EVENTS + 1)
> +
>  /*
>   * This is how the event selector and unit mask are stored in an AMD
>   * core performance event-select register. Intel's format is similar,
> @@ -760,6 +764,8 @@ static int set_pmu_single_event_filter(struct kvm_vcpu *vcpu, uint64_t event,
>  
>  static void test_filter_ioctl(struct kvm_vcpu *vcpu)
>  {
> +	uint8_t nr_fixed_counters = kvm_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS);
> +	struct __kvm_pmu_event_filter f;
>  	uint64_t e = ~0ul;
>  	int r;
>  
> @@ -780,6 +786,26 @@ static void test_filter_ioctl(struct kvm_vcpu *vcpu)
>  					KVM_PMU_EVENT_FLAG_MASKED_EVENTS,
>  					KVM_PMU_EVENT_ALLOW);
>  	TEST_ASSERT(r == 0, "Valid PMU Event Filter is failing");
> +
> +	f = base_event_filter;
> +	f.action = PMU_EVENT_FILTER_INVALID_ACTION;
> +	r = do_vcpu_set_pmu_event_filter(vcpu, &f);
> +	TEST_ASSERT(r, "Set invalid action is expected to fail");
> +
> +	f = base_event_filter;
> +	f.flags = PMU_EVENT_FILTER_INVALID_FLAGS;
> +	r = do_vcpu_set_pmu_event_filter(vcpu, &f);
> +	TEST_ASSERT(r, "Set invalid flags is expected to fail");
> +
> +	f = base_event_filter;
> +	f.nevents = PMU_EVENT_FILTER_INVALID_NEVENTS;
> +	r = do_vcpu_set_pmu_event_filter(vcpu, &f);
> +	TEST_ASSERT(r, "Exceeding the max number of filter events should fail");
> +
> +	f = base_event_filter;
> +	f.fixed_counter_bitmap = ~GENMASK_ULL(nr_fixed_counters, 0);
> +	r = do_vcpu_set_pmu_event_filter(vcpu, &f);
> +	TEST_ASSERT(!r, "Masking non-existent fixed counters should be allowed");
>  }
>  
>  int main(int argc, char *argv[])
> -- 
> 2.39.3
> 

Reviewed-by: Isaku Yamahata <isaku.yamahata@intel.com>
-- 
Isaku Yamahata <isaku.yamahata@gmail.com>

[PATCH v6 5/6] KVM: selftests: Test if event filter meets expectations on fixed counters

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

Add tests to cover that pmu event_filter works as expected when it's
applied to fixed performance counters, even if there is none fixed
counter exists (e.g. Intel guest pmu version=1 or AMD guest).

Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 .../kvm/x86_64/pmu_event_filter_test.c        | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
index 8b8bfee11016..732c76c41bb0 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
@@ -27,6 +27,7 @@
 #define ARCH_PERFMON_BRANCHES_RETIRED		5
 
 #define NUM_BRANCHES 42
+#define INTEL_PMC_IDX_FIXED		32
 
 /* Matches KVM_PMU_EVENT_FILTER_MAX_EVENTS in pmu.c */
 #define MAX_FILTER_EVENTS		300
@@ -808,6 +809,84 @@ static void test_filter_ioctl(struct kvm_vcpu *vcpu)
 	TEST_ASSERT(!r, "Masking non-existent fixed counters should be allowed");
 }
 
+static void intel_run_fixed_counter_guest_code(uint8_t fixed_ctr_idx)
+{
+	for (;;) {
+		wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
+		wrmsr(MSR_CORE_PERF_FIXED_CTR0 + fixed_ctr_idx, 0);
+
+		/* Only OS_EN bit is enabled for fixed counter[idx]. */
+		wrmsr(MSR_CORE_PERF_FIXED_CTR_CTRL, BIT_ULL(4 * fixed_ctr_idx));
+		wrmsr(MSR_CORE_PERF_GLOBAL_CTRL,
+		      BIT_ULL(INTEL_PMC_IDX_FIXED + fixed_ctr_idx));
+		__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
+		wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
+
+		GUEST_SYNC(rdmsr(MSR_CORE_PERF_FIXED_CTR0 + fixed_ctr_idx));
+	}
+}
+
+static uint64_t test_with_fixed_counter_filter(struct kvm_vcpu *vcpu,
+					       uint32_t action, uint32_t bitmap)
+{
+	struct __kvm_pmu_event_filter f = {
+		.action = action,
+		.fixed_counter_bitmap = bitmap,
+	};
+	do_vcpu_set_pmu_event_filter(vcpu, &f);
+
+	return run_vcpu_to_sync(vcpu);
+}
+
+static void __test_fixed_counter_bitmap(struct kvm_vcpu *vcpu, uint8_t idx,
+					uint8_t nr_fixed_counters)
+{
+	unsigned int i;
+	uint32_t bitmap;
+	uint64_t count;
+
+	TEST_ASSERT(nr_fixed_counters < sizeof(bitmap) * 8,
+		    "Invalid nr_fixed_counters");
+
+	/*
+	 * Check the fixed performance counter can count normally when KVM
+	 * userspace doesn't set any pmu filter.
+	 */
+	count = run_vcpu_to_sync(vcpu);
+	TEST_ASSERT(count, "Unexpected count value: %ld\n", count);
+
+	for (i = 0; i < BIT(nr_fixed_counters); i++) {
+		bitmap = BIT(i);
+		count = test_with_fixed_counter_filter(vcpu, KVM_PMU_EVENT_ALLOW,
+						       bitmap);
+		TEST_ASSERT_EQ(!!count, !!(bitmap & BIT(idx)));
+
+		count = test_with_fixed_counter_filter(vcpu, KVM_PMU_EVENT_DENY,
+						       bitmap);
+		TEST_ASSERT_EQ(!!count, !(bitmap & BIT(idx)));
+	}
+}
+
+static void test_fixed_counter_bitmap(void)
+{
+	uint8_t nr_fixed_counters = kvm_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS);
+	struct kvm_vm *vm;
+	struct kvm_vcpu *vcpu;
+	uint8_t idx;
+
+	/*
+	 * Check that pmu_event_filter works as expected when it's applied to
+	 * fixed performance counters.
+	 */
+	for (idx = 0; idx < nr_fixed_counters; idx++) {
+		vm = vm_create_with_one_vcpu(&vcpu,
+					     intel_run_fixed_counter_guest_code);
+		vcpu_args_set(vcpu, 1, idx);
+		__test_fixed_counter_bitmap(vcpu, idx, nr_fixed_counters);
+		kvm_vm_free(vm);
+	}
+}
+
 int main(int argc, char *argv[])
 {
 	void (*guest_code)(void);
@@ -851,6 +930,7 @@ int main(int argc, char *argv[])
 	kvm_vm_free(vm);
 
 	test_pmu_config_disable(guest_code);
+	test_fixed_counter_bitmap();
 
 	return 0;
 }
-- 
2.39.3

Re: [PATCH v6 5/6] KVM: selftests: Test if event filter meets expectations on fixed counters

[Isaku Yamahata]

On Thu, Aug 10, 2023 at 05:09:44PM +0800,
Jinrong Liang <ljr.kernel@gmail.com> wrote:

> From: Jinrong Liang <cloudliang@tencent.com>
> 
> Add tests to cover that pmu event_filter works as expected when it's
> applied to fixed performance counters, even if there is none fixed
> counter exists (e.g. Intel guest pmu version=1 or AMD guest).
> 
> Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
> ---
>  .../kvm/x86_64/pmu_event_filter_test.c        | 80 +++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> index 8b8bfee11016..732c76c41bb0 100644
> --- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> @@ -27,6 +27,7 @@
>  #define ARCH_PERFMON_BRANCHES_RETIRED		5
>  
>  #define NUM_BRANCHES 42
> +#define INTEL_PMC_IDX_FIXED		32
>  
>  /* Matches KVM_PMU_EVENT_FILTER_MAX_EVENTS in pmu.c */
>  #define MAX_FILTER_EVENTS		300
> @@ -808,6 +809,84 @@ static void test_filter_ioctl(struct kvm_vcpu *vcpu)
>  	TEST_ASSERT(!r, "Masking non-existent fixed counters should be allowed");
>  }
>  
> +static void intel_run_fixed_counter_guest_code(uint8_t fixed_ctr_idx)
> +{
> +	for (;;) {
> +		wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
> +		wrmsr(MSR_CORE_PERF_FIXED_CTR0 + fixed_ctr_idx, 0);
> +
> +		/* Only OS_EN bit is enabled for fixed counter[idx]. */
> +		wrmsr(MSR_CORE_PERF_FIXED_CTR_CTRL, BIT_ULL(4 * fixed_ctr_idx));
> +		wrmsr(MSR_CORE_PERF_GLOBAL_CTRL,
> +		      BIT_ULL(INTEL_PMC_IDX_FIXED + fixed_ctr_idx));
> +		__asm__ __volatile__("loop ." : "+c"((int){NUM_BRANCHES}));
> +		wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
> +
> +		GUEST_SYNC(rdmsr(MSR_CORE_PERF_FIXED_CTR0 + fixed_ctr_idx));
> +	}
> +}
> +
> +static uint64_t test_with_fixed_counter_filter(struct kvm_vcpu *vcpu,
> +					       uint32_t action, uint32_t bitmap)
> +{
> +	struct __kvm_pmu_event_filter f = {
> +		.action = action,
> +		.fixed_counter_bitmap = bitmap,
> +	};
> +	do_vcpu_set_pmu_event_filter(vcpu, &f);
> +
> +	return run_vcpu_to_sync(vcpu);
> +}
> +
> +static void __test_fixed_counter_bitmap(struct kvm_vcpu *vcpu, uint8_t idx,
> +					uint8_t nr_fixed_counters)
> +{
> +	unsigned int i;
> +	uint32_t bitmap;
> +	uint64_t count;
> +
> +	TEST_ASSERT(nr_fixed_counters < sizeof(bitmap) * 8,
> +		    "Invalid nr_fixed_counters");
> +
> +	/*
> +	 * Check the fixed performance counter can count normally when KVM
> +	 * userspace doesn't set any pmu filter.
> +	 */
> +	count = run_vcpu_to_sync(vcpu);
> +	TEST_ASSERT(count, "Unexpected count value: %ld\n", count);
> +
> +	for (i = 0; i < BIT(nr_fixed_counters); i++) {
> +		bitmap = BIT(i);
> +		count = test_with_fixed_counter_filter(vcpu, KVM_PMU_EVENT_ALLOW,
> +						       bitmap);
> +		TEST_ASSERT_EQ(!!count, !!(bitmap & BIT(idx)));
> +
> +		count = test_with_fixed_counter_filter(vcpu, KVM_PMU_EVENT_DENY,
> +						       bitmap);
> +		TEST_ASSERT_EQ(!!count, !(bitmap & BIT(idx)));
> +	}
> +}
> +
> +static void test_fixed_counter_bitmap(void)
> +{
> +	uint8_t nr_fixed_counters = kvm_cpu_property(X86_PROPERTY_PMU_NR_FIXED_COUNTERS);
> +	struct kvm_vm *vm;
> +	struct kvm_vcpu *vcpu;
> +	uint8_t idx;
> +
> +	/*
> +	 * Check that pmu_event_filter works as expected when it's applied to
> +	 * fixed performance counters.
> +	 */
> +	for (idx = 0; idx < nr_fixed_counters; idx++) {
> +		vm = vm_create_with_one_vcpu(&vcpu,
> +					     intel_run_fixed_counter_guest_code);
> +		vcpu_args_set(vcpu, 1, idx);
> +		__test_fixed_counter_bitmap(vcpu, idx, nr_fixed_counters);
> +		kvm_vm_free(vm);
> +	}
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  	void (*guest_code)(void);
> @@ -851,6 +930,7 @@ int main(int argc, char *argv[])
>  	kvm_vm_free(vm);
>  
>  	test_pmu_config_disable(guest_code);
> +	test_fixed_counter_bitmap();
>  
>  	return 0;
>  }
> -- 
> 2.39.3
> 

Reviewed-by: Isaku Yamahata <isaku.yamahata@intel.com>

-- 
Isaku Yamahata <isaku.yamahata@gmail.com>

[PATCH v6 6/6] KVM: selftests: Test gp event filters don't affect fixed event filters

[Jinrong Liang]

From: Jinrong Liang <cloudliang@tencent.com>

Add a test to ensure that setting both generic and fixed performance
event filters does not affect the consistency of the fixed event filter
behavior in KVM.

Signed-off-by: Jinrong Liang <cloudliang@tencent.com>
---
 .../kvm/x86_64/pmu_event_filter_test.c        | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
index 732c76c41bb0..2166b4085ca3 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
@@ -838,6 +838,19 @@ static uint64_t test_with_fixed_counter_filter(struct kvm_vcpu *vcpu,
 	return run_vcpu_to_sync(vcpu);
 }
 
+static uint64_t test_set_gp_and_fixed_event_filter(struct kvm_vcpu *vcpu,
+						   uint32_t action,
+						   uint32_t bitmap)
+{
+	struct __kvm_pmu_event_filter f = base_event_filter;
+
+	f.action = action;
+	f.fixed_counter_bitmap = bitmap;
+	do_vcpu_set_pmu_event_filter(vcpu, &f);
+
+	return run_vcpu_to_sync(vcpu);
+}
+
 static void __test_fixed_counter_bitmap(struct kvm_vcpu *vcpu, uint8_t idx,
 					uint8_t nr_fixed_counters)
 {
@@ -864,6 +877,20 @@ static void __test_fixed_counter_bitmap(struct kvm_vcpu *vcpu, uint8_t idx,
 		count = test_with_fixed_counter_filter(vcpu, KVM_PMU_EVENT_DENY,
 						       bitmap);
 		TEST_ASSERT_EQ(!!count, !(bitmap & BIT(idx)));
+
+		/*
+		 * Check that fixed_counter_bitmap has higher priority than
+		 * events[] when both are set.
+		 */
+		count = test_set_gp_and_fixed_event_filter(vcpu,
+							   KVM_PMU_EVENT_ALLOW,
+							   bitmap);
+		TEST_ASSERT_EQ(!!count, !!(bitmap & BIT(idx)));
+
+		count = test_set_gp_and_fixed_event_filter(vcpu,
+							   KVM_PMU_EVENT_DENY,
+							   bitmap);
+		TEST_ASSERT_EQ(!!count, !(bitmap & BIT(idx)));
 	}
 }
 
-- 
2.39.3

Re: [PATCH v6 0/6] KVM: selftests: Improve PMU event filter settings and add test cases

[Sean Christopherson]

On Thu, 10 Aug 2023 17:09:39 +0800, Jinrong Liang wrote:
> This patch series aims to improve the PMU event filter settings with a cleaner
> and more organized structure and adds several test cases related to PMU event
> filters.
> 
> These changes help to ensure that KVM's PMU event filter functions as expected
> in all supported use cases.
> 
> [...]

Applied to kvm-x86 selftests, thanks!

[1/6] KVM: selftests: Add x86 properties for Intel PMU in processor.h
      https://github.com/kvm-x86/linux/commit/cf6d80c0eb95
[2/6] KVM: selftests: Drop the return of remove_event()
      https://github.com/kvm-x86/linux/commit/c853be2265cc
[3/6] KVM: selftests: Introduce __kvm_pmu_event_filter to improved event filter settings
      https://github.com/kvm-x86/linux/commit/de527b1daf69
[4/6] KVM: selftests: Add test cases for unsupported PMU event filter input values
      https://github.com/kvm-x86/linux/commit/86ab6af8b96a
[5/6] KVM: selftests: Test if event filter meets expectations on fixed counters
      https://github.com/kvm-x86/linux/commit/d4e36166820a
[6/6] KVM: selftests: Test gp event filters don't affect fixed event filters
      https://github.com/kvm-x86/linux/commit/740d087e7fff

--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes