Re: [PATCH] KVM: selftests: fix supported_flags for aarch64

[Paolo Bonzini]

On 12/13/23 18:21, Sean Christopherson wrote:
> On Tue, Dec 12, 2023, Paolo Bonzini wrote:
> > On 12/9/23 03:29, Sean Christopherson wrote:
> > > On Fri, Dec 08, 2023, Paolo Bonzini wrote:
> > > > KVM/Arm supports readonly memslots; fix the calculation of
> > > > supported_flags in set_memory_region_test.c, otherwise the
> > > > test fails.
> > >
> > > You got beat by a few hours, and by a better solution ;-)
> > >
> > > https://lore.kernel.org/all/20231208033505.2930064-1-shahuang@xxxxxxxxxx
> >
> > Better but also wrong---and my patch has the debatable merit of more
> > clearly exposing the wrongness.  Testing individual architectures is bad,
> > but testing __KVM_HAVE_READONLY_MEM makes the test fail when running a new
> > test on an old kernel.
>
> But we already crossed that bridge and burned it for good measure by switching
> to KVM_SET_USER_MEMORY_REGION2, i.e. as of commit
>
>    8d99e347c097 ("KVM: selftests: Convert lib's mem regions to KVM_SET_USER_MEMORY_REGION2")
>
> selftests built against a new kernel can't run on an old kernel.  Building KVM
> selftests requires kernel headers, so while not having a hard requirement that
> the uapi headers are fresh would be nice, I don't think it buys all that much.
>
> If we wanted to assert that x86, arm64, etc. enumerate __KVM_HAVE_READONLY_MEM,
> i.e. ensure that read-only memory is supported as expected, then that can be done
> as a completely unrelated test.

selftests have the luxury of having sync-ed kernel headers, but in 
general userspace won't, and that means __KVM_HAVE_READONLY_MEM would be 
a very poor userspace API.  Fortunately it has "__" so it is not 
userspace API at all, and I don't want selftests to treat it as one.

> IMO, one of the big selling points of selftests over KUT is that we can punt on
> supporting old kernels since selftests are in-tree.  I don't think it's at all
> unreasonable to require that selftests be built against the target kernel, and
> by doing so we can signficantly reduce the maintenance burden.  The kernel needs
> to be backwards compatibile, but I don't see why selftests need to be backwards
> compatible.

It does help sometimes to be able to run old tests on new kernel or vice 
versa.  So even without making that a requirement, it is a nice thing to 
have whenever possible.

Paolo