Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230717062343.3743-1-cloudliang@tencent.com> <20230717062343.3743-5-cloudliang@tencent.com>
 <20230719011752.GD25699@ls.amr.corp.intel.com>
In-Reply-To: <20230719011752.GD25699@ls.amr.corp.intel.com>
From:   Jinrong Liang <ljr.kernel@gmail.com>
Date:   Wed, 19 Jul 2023 11:20:54 +0800
Message-ID: <CAFg_LQVA=u+WM3_xK5+KiAz5R=-nZ3jdQbX0jsBmrw7_6+ZWew@mail.gmail.com>
Subject: Re: [PATCH v4 4/6] KVM: selftests: Add test cases for unsupported PMU
 event filter input values
To:     Isaku Yamahata <isaku.yamahata@gmail.com>
Cc:     Sean Christopherson <seanjc@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Jim Mattson <jmattson@google.com>,
        Shuah Khan <shuah@kernel.org>,
        Aaron Lewis <aaronlewis@google.com>,
        David Matlack <dmatlack@google.com>,
        Vishal Annapurve <vannapurve@google.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Like Xu <like.xu.linux@gmail.com>,
        Jinrong Liang <cloudliang@tencent.com>,
        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

Isaku Yamahata <isaku.yamahata@gmail.com> =E4=BA=8E2023=E5=B9=B47=E6=9C=881=
9=E6=97=A5=E5=91=A8=E4=B8=89 09:17=E5=86=99=E9=81=93=EF=BC=9A
>
> On Mon, Jul 17, 2023 at 02:23:41PM +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 t=
he
> > PMU event filter. The tests cover unsupported "action" values, unsuppor=
ted
> > "flags" values, and unsupported "nevents" values. All these cases shoul=
d
> > return an error, as they are currently not supported by the filter.
> > Furthermore, the tests also cover the scenario where setting non-existe=
nt
> > 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 ffcbbf25b29b..63f85f583ef8 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_DE=
NY + 1)
> > +#define PMU_EVENT_FILTER_INVALID_FLAGS                       (KVM_PMU_=
EVENT_FLAG_MASKED_EVENTS + 1)
>
> flag is a bit mask. Not number. So +1 sounds weird.
> As KVM_PMU_EVENT_FLAGS_VALID_MASK =3D 1,  this happens to get wanted resu=
lt, though.

We need an invalid flags, KVM_PMU_EVENT_FLAGS_VALID_MASK is actually
equal to KVM_PMU_EVENT_FLAG_MASKED_EVENTS.

In kvm.h:

#define KVM_PMU_EVENT_FLAG_MASKED_EVENTS BIT(0)
#define KVM_PMU_EVENT_FLAGS_VALID_MASK (KVM_PMU_EVENT_FLAG_MASKED_EVENTS)

How about this modification:

#define PMU_EVENT_FILTER_INVALID_FLAGS
(KVM_PMU_EVENT_FLAGS_VALID_MASK << 1)

>
>
> > +#define PMU_EVENT_FILTER_INVALID_NEVENTS             (MAX_FILTER_EVENT=
S + 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,
> > @@ -757,6 +761,8 @@ static int set_pmu_single_event_filter(struct kvm_v=
cpu *vcpu, uint64_t event,
> >
> >  static void test_filter_ioctl(struct kvm_vcpu *vcpu)
> >  {
> > +     uint8_t nr_fixed_counters =3D kvm_cpu_property(X86_PROPERTY_PMU_N=
R_FIXED_COUNTERS);
> > +     struct __kvm_pmu_event_filter f;
> >       uint64_t e =3D ~0ul;
> >       int r;
> >
> > @@ -777,6 +783,26 @@ static void test_filter_ioctl(struct kvm_vcpu *vcp=
u)
> >                                       KVM_PMU_EVENT_FLAG_MASKED_EVENTS,
> >                                       KVM_PMU_EVENT_ALLOW);
> >       TEST_ASSERT(r =3D=3D 0, "Valid PMU Event Filter is failing");
> > +
> > +     f =3D base_event_filter;
> > +     f.action =3D PMU_EVENT_FILTER_INVALID_ACTION;
> > +     r =3D do_vcpu_set_pmu_event_filter(vcpu, &f);
> > +     TEST_ASSERT(r, "Set invalid action is expected to fail");
> > +
> > +     f =3D base_event_filter;
> > +     f.flags =3D PMU_EVENT_FILTER_INVALID_FLAGS;
> > +     r =3D do_vcpu_set_pmu_event_filter(vcpu, &f);
> > +     TEST_ASSERT(r, "Set invalid flags is expected to fail");
> > +
> > +     f =3D base_event_filter;
> > +     f.nevents =3D PMU_EVENT_FILTER_INVALID_NEVENTS;
> > +     r =3D do_vcpu_set_pmu_event_filter(vcpu, &f);
> > +     TEST_ASSERT(r, "Exceeding the max number of filter events should =
fail");
> > +
> > +     f =3D base_event_filter;
> > +     f.fixed_counter_bitmap =3D ~GENMASK_ULL(nr_fixed_counters, 0);
> > +     r =3D do_vcpu_set_pmu_event_filter(vcpu, &f);
> > +     TEST_ASSERT(!r, "Masking non-existent fixed counters should be al=
lowed");
> >  }
> >
> >  int main(int argc, char *argv[])
> > --
> > 2.39.3
> >
>
> --
> Isaku Yamahata <isaku.yamahata@gmail.com>