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

From: Jim Mattson <jmattson@google.com>
To: Ricardo Koller <ricarkol@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Sean Christopherson <seanjc@google.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Joerg Roedel <joro@8bytes.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	linux-kernel@vger.kernel.org, clang-built-linux@googlegroups.com
Subject: Re: [PATCH 0/5] KVM: x86: Use kernel x86 cpuid utilities in KVM selftests
Date: Tue, 29 Jun 2021 10:28:46 -0700	[thread overview]
Message-ID: <CALMp9eSfpdWF0OROsOqxohxMoFrrY=Gt7FYfB1_31D7no4JYLw@mail.gmail.com> (raw)
In-Reply-To: <YIm7iWxggvoN9riz@google.com>

On Wed, Apr 28, 2021 at 12:46 PM Ricardo Koller <ricarkol@google.com> 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?