[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFg_LQVA=u+WM3_xK5+KiAz5R=-nZ3jdQbX0jsBmrw7_6+ZWew@mail.gmail.com>
Date: Wed, 19 Jul 2023 11:20:54 +0800
From: Jinrong Liang <ljr.kernel@...il.com>
To: Isaku Yamahata <isaku.yamahata@...il.com>
Cc: Sean Christopherson <seanjc@...gle.com>,
Paolo Bonzini <pbonzini@...hat.com>,
Jim Mattson <jmattson@...gle.com>,
Shuah Khan <shuah@...nel.org>,
Aaron Lewis <aaronlewis@...gle.com>,
David Matlack <dmatlack@...gle.com>,
Vishal Annapurve <vannapurve@...gle.com>,
Wanpeng Li <wanpengli@...cent.com>,
Like Xu <like.xu.linux@...il.com>,
Jinrong Liang <cloudliang@...cent.com>,
linux-kselftest@...r.kernel.org, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 4/6] KVM: selftests: Add test cases for unsupported PMU
event filter input values
Isaku Yamahata <isaku.yamahata@...il.com> 于2023年7月19日周三 09:17写道:
>
> On Mon, Jul 17, 2023 at 02:23:41PM +0800,
> Jinrong Liang <ljr.kernel@...il.com> wrote:
>
> > From: Jinrong Liang <cloudliang@...cent.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 scenario where setting non-existent
> > fixed counters in the fixed bitmap does not fail.
> >
> > Signed-off-by: Jinrong Liang <cloudliang@...cent.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_DENY + 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 = 1, this happens to get wanted result, 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_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,
> > @@ -757,6 +761,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;
> >
> > @@ -777,6 +783,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
> >
>
> --
> Isaku Yamahata <isaku.yamahata@...il.com>
Powered by blists - more mailing lists