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.