From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 73A55C61CE5 for ; Thu, 17 Aug 2023 20:17:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1354823AbjHQUR1 (ORCPT ); Thu, 17 Aug 2023 16:17:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52346 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1354819AbjHQURS (ORCPT ); Thu, 17 Aug 2023 16:17:18 -0400 Received: from mail-pg1-x54a.google.com (mail-pg1-x54a.google.com [IPv6:2607:f8b0:4864:20::54a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4DA32359B for ; Thu, 17 Aug 2023 13:17:16 -0700 (PDT) Received: by mail-pg1-x54a.google.com with SMTP id 41be03b00d2f7-5637a108d02so308060a12.2 for ; Thu, 17 Aug 2023 13:17:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1692303436; x=1692908236; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:from:to:cc:subject:date:message-id :reply-to; bh=KU+yygDiyP0ELbBFxmC0szeJUfwpE0TiBmbPQ8YXVAs=; b=zNA3vpDDz1KyBjk9Ob5BUHexoZIGyPiAf/jYnuCLTrIqL4j8d6uRJul5dmw74Bh5Hq SeyuGFaDDWqXuIHXDZA8/MCkbxdNniW3sClCvGAJwf+6uB9P9YBYZucziWA+uHZ9V3aQ ry1zwBbNsGxEkazo8EH9jBDiVKZ6gZuLtJp6BpHlGyFW2+HSz0J3LwOKAJBm5+Sbpzzh 1dOvxxvTnzFRJ0DUIV9Np2zj6Wx5KXBVohTo0UHZTaxclJriOTeM0q8Ral8VlimAaMPU HI0tbM5WA3rH/97lmAIvs2Vj+Px3wTzwhMfDsgg9wXUriOKOFynRHp9VmuR+kHhQtls9 snsA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692303436; x=1692908236; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=KU+yygDiyP0ELbBFxmC0szeJUfwpE0TiBmbPQ8YXVAs=; b=ZM+sFGK1r5ra0Sjci427pdOweNBxFnTJECRvxR/3af+H5vBMINKJh+I8C3d0wulQx5 tHFH4zL/AQtu9xEfvbw6zQUEQZwl3kURAAwxukuMB9RO3mv4QDAbtvHcTPhGtjkDgjAi 4SFVYfkgqcg4D8QjvPZKai9mh9d9keF9VL/FbJr+e8bbdNSYoo8Jb5aQcE80C1m9vmob WKUzlq9WDAKe2NehO4aROiHz0YPidskH12ui7o2CV8OpT8cB/+UI/odETdMYBOXvGJ2e 8T/nejD5MIwHMACRBcD7pcgh9kDJbg0YavrwTKKhQJ7mc8qR8Zt/AoeAPERWIksyybj7 H3Ig== X-Gm-Message-State: AOJu0Yz0aWpg089qDA7uVuqBIZ6A3yMpH6Z/UV/x1TOxkldGPGnDbQNc k5QGjo9vO6vDV8UXoJo/qbfhSd+44Y0= X-Google-Smtp-Source: AGHT+IFijHumwatVu7On9K+vpmmEsXcAinTeAqe7rw5Domph+e00oHTgIyy4LP9oPODs9OGpTVBlFLzjQh0= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a63:b00f:0:b0:565:dddd:1f65 with SMTP id h15-20020a63b00f000000b00565dddd1f65mr61039pgf.7.1692303435763; Thu, 17 Aug 2023 13:17:15 -0700 (PDT) Date: Thu, 17 Aug 2023 13:17:14 -0700 In-Reply-To: Mime-Version: 1.0 References: <20230810090945.16053-1-cloudliang@tencent.com> <20230810090945.16053-4-cloudliang@tencent.com> <20230814234926.GD2257301@ls.amr.corp.intel.com> Message-ID: Subject: Re: [PATCH v6 3/6] KVM: selftests: Introduce __kvm_pmu_event_filter to improved event filter settings From: Sean Christopherson To: Jinrong Liang Cc: Isaku Yamahata , Paolo Bonzini , Isaku Yamahata , Jim Mattson , Shuah Khan , Aaron Lewis , David Matlack , Vishal Annapurve , Wanpeng Li , Like Xu , Jinrong Liang , linux-kselftest@vger.kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Tue, Aug 15, 2023, Jinrong Liang wrote: > Isaku Yamahata =E4=BA=8E2023=E5=B9=B48=E6=9C= =8815=E6=97=A5=E5=91=A8=E4=BA=8C 07:49=E5=86=99=E9=81=93=EF=BC=9A > > > > On Thu, Aug 10, 2023 at 05:09:42PM +0800, > > Jinrong Liang wrote: > > > > > From: Jinrong Liang > > > > > > 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 > > > Signed-off-by: Jinrong Liang > > > --- > > > .../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? >=20 > 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? >=20 > In tools/arch/x86/include/uapi/asm/kvm.h >=20 > /* 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 usersp= ace and forcing userspace to create an 1KiB+ object just to define a single eve= nt 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 =3D {}; memset(&buffer, 0, sizeof(buffer)); buffer.header.nmsrs =3D 1; buffer.entry.index =3D msr_index; buffer.entry.data =3D 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 undesir= able.