Re: [PATCH 0/5] KVM: x86: Use kernel x86 cpuid utilities in KVM selftests

[Jim Mattson]

On Wed, Apr 28, 2021 at 12:46 PM Ricardo Koller <ricarkol@xxxxxxxxxx> wrote:
>
> On Thu, Apr 22, 2021 at 09:02:09AM +0200, Paolo Bonzini wrote:
> > On 22/04/21 02:56, Ricardo Koller wrote:
> > > The kernel has a set of utilities and definitions to deal with x86 cpu
> > > features.  The x86 KVM selftests don't use them, and instead have
> > > evolved to use differing and ad-hoc methods for checking features. The
> > > advantage of the kernel feature definitions is that they use a format
> > > that embeds the info needed to extract them from cpuid (function, index,
> > > and register to use).
> > >
> > > The first 3 patches massage the related cpuid header files in the kernel
> > > side, then copy them into tools/ so they can be included by selftests.
> > > The last 2 patches replace the tests checking for cpu features to use
> > > the definitions and utilities introduced from the kernel.
> >
> > I queued the first, but I am not sure about the rest.
> >
> > An alternative is to copy over the code from kvm-unit-tests which encodes
> > the leaf/subleaf/register/bit values into the X86_FEATURE_* value.  Sharing
> > code with kvm-unit-tests is probably simpler than adding #ifdef __KERNEL__
> > and keeping the headers in sync.
> >
> > Paolo
> >
>
> Thanks. I was thinking about kvm-unit-tests, but the issue is that it
> would also be a copy. And just like with kernel headers, it would be
> ideal to keep them in-sync. The advantage of the kernel headers is that
> it's much easier to check and fix diffs with them. On the other hand, as
> you say, there would not be any #ifdef stuff with kvm=unit-tests. Please
> let me know what you think.

I think the kvm-unit-tests implementation is superior to the kernel
implementation, but that's probably because I suggested it. Still, I
think there's an argument to be made that selftests, unlike
kvm-unit-tests, are part of the kernel distribution and should be
consistent with the kernel where possible.

Paolo?