Re: [PATCH v2 00/14] KVM: selftests: Link with VFIO selftests lib and test device interrupts

[Josh Hilke <jrhilke@google.com>]

On Wed, Apr 01, 2026 at 04:58:34PM -0700, David Matlack wrote:
> On Wed, Apr 1, 2026 at 4:42 PM Josh Hilke <jrhilke@google.com> wrote:
> >
> > On Wed, Apr 1, 2026 at 1:12 PM David Matlack <dmatlack@google.com> wrote:
> > >
> > > On 2026-04-01 12:07 PM, Sean Christopherson wrote:
> > > > On Wed, Apr 01, 2026, David Matlack wrote:
> > > > > On 2026-04-01 11:17 AM, Sean Christopherson wrote:
> > > > > > On Tue, Mar 31, 2026, Josh Hilke wrote:
> > >
> > > > > Agree this would be nice to have. Is this a blocker for this series?
> > > >
> > > > Yes.  KVM selftests need to assert there's a driver before trying to proceed.
> > >
> > > That exists in the new test:
> > >
> > > static int setup_msi(struct vfio_pci_device *device, bool use_device_msi)
> > > {
> > >         const int flags = MAP_SHARED | MAP_ANONYMOUS;
> > >         const int prot = PROT_READ | PROT_WRITE;
> > >         struct dma_region *region;
> > >
> > >         if (use_device_msi) {
> > >                 /* A driver is required to generate an MSI. */
> > >                 TEST_REQUIRE(device->driver.ops);
> > >
> > > But I think you are also asking to have a helper that prints out the
> > > list of available drivers in this case. That should be trivial to add.
> > >
> > > VFIO selftests could expose add a helper that handles checking for
> > > device->driver.ops and, if null, printing an error message and list of
> > > available drivers and exiting with KSFT_SKIP.
> > >
> > > One wrinkle here is we are probably going to merge a driver soon that
> > > does not support send_msi():
> > >
> > >   https://lore.kernel.org/kvm/20260331172241.50456-1-rubind@nvidia.com/
> > >
> > > So we should also have an API for that. e.g. Add 2 helpsers:
> > >
> > >   - vfio_pci_has_driver()
> > >   - vfio_pci_has_driver_send_msi()
> > >
> > > This test would use the latter.
> >
> > Ack. I'll add a patch in the series to add these functions to the vfio
> > library in v3.
> >
> > >
> > > > > > struct vfio_pci_device *kvm_vfio_device_init(const char *bdf)
> > > > > > {
> > > > > >   struct vfio_pci_device *device;
> > > > > >   struct iommu *iommu;
> > > > > >
> > > > > >   iommu = iommu_init(default_iommu_mode);
> > > > > >
> > > > > >   device = vfio_pci_device_init(bdf, iommu);
> > > > > >   if (!device) {
> > > > > >           vfio_pci_device_print_drivers(stderr);
> > > > > >           TEST_FAIL("No driver found for BDF '%s'", bdf);
> > > > > >   }
> > > > > >   return device;
> > > > > > }
> > > > > >
> > > > > > Because as is, this requires way too much a priori knowledge and magic for
> > > > > > upstream.  The user shouldn't have to dig through code just to understand what
> > > > > > devices are supported.
> > > > > >
> > > > > > Ideally, VFIO selftests would also provide a script, utility, and/or helper to
> > > > > > identify BDFs for devices it has drivers for.  In my experience, binding a device
> > > > > > to VFIO is trivial.  Finding the device in the first place is more annoying.
> > > > >
> > > > > Agree this would be nice to have. Is this a blocker for this series?
> > > >
> > > > Yes.  As I mentioned to Josh off-list, I'm not willing to accept a "we pinky-swear
> > > > we'll make this stuff user friendly".  It's not that I don't trust you and Josh
> > > > (and others), it's that for me, this level of user-friendly behavior isn't nice
> > > > to have, it's mandatory for these tests to be usable upstream.
> > > >
> > > > Internally we can squeak by with barebones, unhelpful tests, because the run
> > > > commands are often hardcoded somewhere and there is piles of documentation elsewhere.
> > > > But for upstream, none of that holds true.
> > > >
> > > > If it were weeks of effort, I would be more open to treating this as nice to have,
> > > > but AFAICT writing the code should be less than a days worth of work.
> > >
> > > Yeah that's fine, I don't think it adds too much extra work, and it's
> > > something I'd like to have anyway. I just wanted to confirm if it should
> > > be part (or merged ahead of this series) or can be separate. Thanks!
> >
> > So I need to add kvm_vfio_device_init() and it will call
> > vfio_pci_has_driver(). Then, kvm_vfio_device_init() will TEST_FAIL if
> > vfio_pci_has_driver() returns false, correct?
> >
> > Should kvm_vfio_device_init() live in
> > tools/testing/selftests/kvm/lib/kvm_util.c?
> 
> I was thinking the new test would do:
> 
>   TEST_REQUIRE(vfio_pci_has_driver_send_msi(device));
> 
> instead of:
> 
>   TEST_REQUIRE(device->driver.ops);
> 
> vfio_pci_has_driver_send_msi() would call vfio_pci_has_driver()
> internally and also check that send_msi op is non-null. Both functions
> would also log error messages if driver/send_msi is missing to help
> the user, as Sean requested.
> 
> And you'll also need to a patch to add a helper script to print the
> list of BDFs on the current host that have a VFIO selftests driver. It
> might make sense to send that as a standalone patch if Sean is ok with
> that.

That makes sense for setup_msi(). But in Sean's implementation of
kvm_vfio_device_init() above, it looks like he wants the test to fail if
there's no VFIO selftest driver for the BDF passed into the test.