On Wed, May 26, 2021 at 09:44:56AM +0100, Marc Zyngier wrote:
> On Tue, 25 May 2021 21:09:22 +0100,
> Ricardo Koller <ricarkol@xxxxxxxxxx> wrote:
> >
> > On Wed, May 19, 2021 at 04:07:26PM +0200, Andrew Jones wrote:
> > > Since KVM commit 11663111cd49 ("KVM: arm64: Hide PMU registers from
> > > userspace when not available") the get-reg-list* tests have been
> > > failing with
> > >
> > > ...
> > > ... There are 74 missing registers.
> > > The following lines are missing registers:
> > > ...
> > >
> > > where the 74 missing registers are all PMU registers. This isn't a
> > > bug in KVM that the selftest found, even though it's true that a
> > > KVM userspace that wasn't setting the KVM_ARM_VCPU_PMU_V3 VCPU
> > > flag, but still expecting the PMU registers to be in the reg-list,
> > > would suddenly no longer have their expectations met. In that case,
> > > the expectations were wrong, though, so that KVM userspace needs to
> > > be fixed, and so does this selftest. The fix for this selftest is to
> > > pull the PMU registers out of the base register sublist into their
> > > own sublist and then create new, pmu-enabled vcpu configs which can
> > > be tested.
> > >
> > > Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx>
> > > ---
> > > .../selftests/kvm/aarch64/get-reg-list.c | 46 +++++++++++++++----
> > > 1 file changed, 38 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > > index dc06a28bfb74..78d8949bddbd 100644
> > > --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > > +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > > @@ -47,6 +47,7 @@ struct reg_sublist {
> > > struct vcpu_config {
> > > const char *name;
> > > bool sve;
> > > + bool pmu;
> > > struct reg_sublist sublists[];
> > > };
> >
> > I think it's possible that the number of sublists keeps increasing: it
> > would be very nice/useful if KVM allowed enabling/disabling more
> > features from userspace (besides SVE, PMU etc).
>
> [tangential semi-rant]
>
> While this is a very noble goal, it also doubles the validation space
> each time you add an option. Given how little testing gets done
> relative to the diversity of features and implementations, that's a
> *big* problem.
It's my hope that this test, especially now after its refactoring, will
allow us to test all configurations easily and therefore frequently.
>
> I'm not against it for big ticket items that result in a substantial
> amount of state to be context-switched (SVE, NV). However, doing that
> for more discrete features would require a radical change in the way
> we develop, review and test KVM/arm64.
>
I'm not sure I understand how we should change the development and
review processes. As for testing, with simple tests like this one,
we can actually achieve exhaustive configuration testing fast, at
least with respect to checking for expected registers and checking
that we can get/set_one_reg on them. If we were to try and setup
QEMU migration tests for all the possible configurations, then it
would take way too long.
Thanks,
drew
