Re: [PATCH v3 0/5] KVM: arm64: selftests: Fix get-reg-list

[Marc Zyngier <maz@kernel.org>]

Hi Andrew,

On Tue, 22 Jun 2021 08:07:32 +0100,
Andrew Jones <drjones@redhat.com> wrote:
> 
> On Mon, May 31, 2021 at 12:33:39PM +0200, Andrew Jones wrote:
> > v3:
> >  - Took Ricardo's suggestions in order to avoid needing to update
> >    prepare_vcpu_init, finalize_vcpu, and check_supported when adding
> >    new register sublists by better associating the sublists with their
> >    vcpu feature bits and caps [Ricardo]
> >  - We now dynamically generate the vcpu config name by creating them
> >    from its sublist names [drew]
> > 
> > v2:
> >  - Removed some cruft left over from a previous more complex design of the
> >    config command line parser
> >  - Dropped the list printing factor out patch as it's not necessary
> >  - Added a 'PASS' output for passing tests to allow testers to feel good
> >  - Changed the "up to date with kernel" comment to reference 5.13.0-rc2
> > 
> > 
> > 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.
> > 
> > We could fix the test with a one-liner since we just need a
> > 
> >   init->features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
> > 
> > in prepare_vcpu_init(), but that's too easy, so here's a 5 patch patch
> > series instead :-)  The reason for all the patches and the heavy diffstat
> > is to prepare for other vcpu configuration testing, e.g. ptrauth and mte.
> > With the refactoring done in this series, we should now be able to easily
> > add register sublists and vcpu configs to the get-reg-list test, as the
> > last patch demonstrates with the pmu fix.
> > 
> > Thanks,
> > drew
> > 
> > 
> > Andrew Jones (5):
> >   KVM: arm64: selftests: get-reg-list: Introduce vcpu configs
> >   KVM: arm64: selftests: get-reg-list: Prepare to run multiple configs
> >     at once
> >   KVM: arm64: selftests: get-reg-list: Provide config selection option
> >   KVM: arm64: selftests: get-reg-list: Remove get-reg-list-sve
> >   KVM: arm64: selftests: get-reg-list: Split base and pmu registers
> > 
> >  tools/testing/selftests/kvm/.gitignore        |   1 -
> >  tools/testing/selftests/kvm/Makefile          |   1 -
> >  .../selftests/kvm/aarch64/get-reg-list-sve.c  |   3 -
> >  .../selftests/kvm/aarch64/get-reg-list.c      | 439 +++++++++++++-----
> >  4 files changed, 321 insertions(+), 123 deletions(-)
> >  delete mode 100644 tools/testing/selftests/kvm/aarch64/get-reg-list-sve.c
> > 
> > -- 
> > 2.31.1
> >
> 
> Gentle ping.
> 
> I'm not sure if I'm pinging Marc or Paolo though. MAINTAINERS shows Paolo
> as all kvm selftests, but I think Marc has started picking up the AArch64
> specific kvm selftests.

I'm happy to queue this series.

> Marc, if you've decided to maintain AArch64 kvm selftests, would you be
> opposed to adding
> 
>   F: tools/testing/selftests/kvm/*/aarch64/
>   F: tools/testing/selftests/kvm/aarch64/
> 
> to "KERNEL VIRTUAL MACHINE FOR ARM64 (KVM/arm64)"?

No problem to add this, but I *will* rely on you (and whoever wants to
part-take) to do the bulk of the reviewing. Do we have a deal?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm