Re: [PATCH v3] KVM: x86/xen: Implement hvm_op/HVMOP_flush_tlbs hypercall

[Sean Christopherson <seanjc@google.com>]

On Tue, Jul 25, 2023, David Woodhouse wrote:
> On Fri, 2023-05-26 at 13:32 -0700, Sean Christopherson wrote:
> >   : Aha!  And QEMU appears to have Xen emulation support.  That means KVM-Unit-Tests
> >   : is an option.  Specifically, extend the "access" test to use this hypercall instead
> >   : of INVLPG.  That'll verify that the flush is actually being performed as expteced.
> 
> That works. Metin has a better version that actually sets up the
> hypercall page properly and uses it, but that one bails out when Xen
> support isn't present, and doesn't show the failure mode quite so
> clearly. This is the simple version:

IIUC, y'all have already written both tests, so why not post both?  I certainly
won't object to more tests if they provide different coverage.

> > Let me be more explicit this time: I am not applying this without a test.  I don't
> > care how trivial a patch may seem, I'm done taking patches without test coverage
> > unless there's a *really* good reason for me to do so.
> 
> Understood.
> 
> So, we know it *works*, but the above is a one-off and not a
> *regression* test.
> 
> It would definitely be nice to have regression tests that cover
> absolutely everything, but adding Xen guest support to the generic KVM-
> Unit-Tests might be considered overkill, because this *particular*
> thing is fairly unlikely to regress? It really is just calling
> kvm_flush_remote_tlbs(), and if that goes wrong we're probably likely
> to notice it anyway.
> 
> What do you think?

I'm a-ok with just having basic test coverage.  We don't have anywhere near 100%
(or even 10%...) coverage of KVM's TLB management, so it would be ridiculous to
require that for Xen PV.

I'd definitely love to change that, and raise the bar for test coverage in general,
but that'll take time (and effort).