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

[Sean Christopherson <seanjc@google.com>]

On Wed, Apr 01, 2026, Josh Hilke wrote:
> On Wed, Apr 1, 2026 at 5:38 PM Josh Hilke <jrhilke@google.com> wrote:
> > > > > > > > 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.
> 
> Whoops, never mind. I missed part of the discussion above. We're only
> requiring a VFIO selftest driver if we're using device MSIs, correct?

From a user-friendliness perspective, I don't see the point in trying to support
devices for which there is no selftest driver.  The *best* case scenario is that
the user gets lucky and picks a device that has MSI-X capabilities, and thus is
able to run the test in a degraded mode.

If the device doesn't have MSI-X capabilities, the test will skip thanks to this
check:

	TEST_REQUIRE(device->msix_info.count > 0);
	vfio_pci_msix_enable(device, 0, 1);

Which is "fine", but IMO, the extra complexity, subtlety and potential for
confusion doesn't justify the coverage provided, because the coverage provided
is basically zero relative to what coverage is provided by dedicated KVM_IRQFD
and VFIO IRQ tests.  And most importantly requiring VFIO makes it practically
impossible to run the test by default.

Doing VFIO_DEVICE_SET_IRQS via vfio_pci_irq_trigger() is just a convoluted,
comically slow way of doing eventfd_signal().  The VFIO => eventfd_signal() path
can be fully tested by having userspace listen on the eventfd, and the
eventfd_signal() => KVM IRQ injection path can be fully tested by having userspace
signal the eventfd directly.

I'm a-ok doing a short pass of the test with vfio_pci_irq_trigger() to smoke
test things before proceeding to the "real" test, but IMO making the
vfio_pci_irq_trigger() path a standalone mode for the test is pointless.

However!  We _do_ have a KVM testing gap that can be trivially filled as there
is no existing eventfd_signal() => KVM IRQ injection selftest.  And this test is
pretty much exactly that.  I just don't want to deal with jumping through VFIO
hoops for effectively no coverage (for KVM).

If we strip out that fluff, then the test is runnable without _any_ VFIO
involvement, which means it can run by default, e.g. in n environments without
VFIO or VFIO-able devices, e.g. in automated CI testgrids.

Something like so.  And then we should also rename the test to drop vfio_pci from
the name, as the VFIO aspect becomes purely optional.

---
 .../testing/selftests/kvm/vfio_pci_irq_test.c | 105 ++++++++----------
 1 file changed, 48 insertions(+), 57 deletions(-)

diff --git a/tools/testing/selftests/kvm/vfio_pci_irq_test.c b/tools/testing/selftests/kvm/vfio_pci_irq_test.c
index 807e9aecc887..70c61fe9f4d0 100644
--- a/tools/testing/selftests/kvm/vfio_pci_irq_test.c
+++ b/tools/testing/selftests/kvm/vfio_pci_irq_test.c
@@ -156,42 +156,34 @@ static void kvm_route_msi(struct kvm_vm *vm, u32 gsi, struct kvm_vcpu *vcpu,
 	vm_ioctl(vm, KVM_SET_GSI_ROUTING, routes);
 }
 
-static int setup_msi(struct vfio_pci_device *device, bool use_device_msi)
+static int vfio_setup_msi(struct vfio_pci_device *device)
 {
 	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);
+	/* A driver is required to generate an MSI. */
+	TEST_REQUIRE(device->driver.ops);
 
-		/* Set up a DMA-able region for the driver to use. */
-		region = &device->driver.region;
-		region->iova = 0;
-		region->size = SZ_2M;
-		region->vaddr = kvm_mmap(region->size, prot, flags, -1);
-		TEST_ASSERT(region->vaddr != MAP_FAILED, "mmap() failed\n");
-		iommu_map(device->iommu, region);
+	/* Set up a DMA-able region for the driver to use. */
+	region = &device->driver.region;
+	region->iova = 0;
+	region->size = SZ_2M;
+	region->vaddr = kvm_mmap(region->size, prot, flags, -1);
+	TEST_ASSERT(region->vaddr != MAP_FAILED, "mmap() failed\n");
+	iommu_map(device->iommu, region);
 
-		vfio_pci_driver_init(device);
+	vfio_pci_driver_init(device);
 
-		return device->driver.msi;
-	}
-
-	TEST_REQUIRE(device->msix_info.count > 0);
-	vfio_pci_msix_enable(device, 0, 1);
-	return 0;
+	return device->driver.msi;
 }
 
-static void send_msi(struct vfio_pci_device *device, bool use_device_msi, int msi)
+static void trigger_interrupt(struct vfio_pci_device *device, int eventfd)
 {
-	if (use_device_msi) {
-		TEST_ASSERT_EQ(msi, device->driver.msi);
+	if (device)
 		vfio_pci_driver_send_msi(device);
-	} else {
-		vfio_pci_irq_trigger(device, VFIO_PCI_MSIX_IRQ_INDEX, msi);
-	}
+	else
+		eventfd_signal(eventfd);
 }
 
 static void help(const char *name)
@@ -236,7 +228,7 @@ int main(int argc, char **argv)
 	u8 vector = 32 + rand() % (UINT8_MAX - 32);
 
 	/* Test configuration (overridable by command line flags). */
-	bool use_device_msi = false, irq_affinity = false, pin_vcpus = false;
+	bool irq_affinity = false, pin_vcpus = false;
 	bool empty = false, nmi = false;
 	int nr_irqs = 1000;
 	int nr_vcpus = 1;
@@ -244,19 +236,18 @@ int main(int argc, char **argv)
 	struct kvm_vcpu *vcpus[KVM_MAX_VCPUS];
 	pthread_t vcpu_threads[KVM_MAX_VCPUS];
 	u64 irq_count, pin_count, piw_count;
+	const char *device_bdf = NULL;
 	struct vfio_pci_device *device;
 	struct iommu *iommu;
 	cpu_set_t available_cpus;
-	const char *device_bdf;
 	FILE *irq_affinity_fp;
 	int i, j, c, msi, irq;
 	struct kvm_vm *vm;
 	int irq_cpu;
 	int ret;
 
-	device_bdf = vfio_selftests_get_bdf(&argc, argv);
 
-	while ((c = getopt(argc, argv, "abdehi:npv:xt:")) != -1) {
+	while ((c = getopt(argc, argv, "abd:ehi:npv:xt:")) != -1) {
 		switch (c) {
 		case 'a':
 			irq_affinity = true;
@@ -265,7 +256,7 @@ int main(int argc, char **argv)
 			block = true;
 			break;
 		case 'd':
-			use_device_msi = true;
+			device_bdf = optarg;
 			break;
 		case 'e':
 			empty = true;
@@ -301,20 +292,23 @@ int main(int argc, char **argv)
 	if (!x2apic)
 		virt_pg_map(vm, APIC_DEFAULT_GPA, APIC_DEFAULT_GPA);
 
-	iommu = iommu_init(default_iommu_mode);
-	device = vfio_pci_device_init(device_bdf, iommu);
-	msi = setup_msi(device, use_device_msi);
-	irq = get_irq_number(device_bdf, msi);
+	if (device_bdf) {
+		iommu = iommu_init(default_iommu_mode);
+		device = vfio_pci_device_init(device_bdf, iommu);
+		msi = vfio_setup_msi(device);
+		irq = get_irq_number(device_bdf, msi);
+		irq_count = get_irq_count(irq);
 
-	irq_count = get_irq_count(irq);
-	pin_count = get_irq_count_by_name("PIN:");
-	piw_count = get_irq_count_by_name("PIW:");
+		eventfd = device->msi_eventfds[msi];
 
-	printf("%s %s MSI-X[%d] (IRQ-%d) %d times\n",
-	       use_device_msi ? "Triggering" : "Notifying the eventfd for",
-	       device_bdf, msi, irq, nr_irqs);
+		printf("Using device %s MSI-X[%d] (IRQ-%d)\n",
+		       device_bdf, msi, irq);
+	} else {
+		device = NULL;
+		eventfd = kvm_new_eventfd();
+	}
 
-	kvm_assign_irqfd(vm, gsi, device->msi_eventfds[msi]);
+	kvm_assign_irqfd(vm, gsi, eventfd);
 
 	sync_global_to_guest(vm, x2apic);
 	sync_global_to_guest(vm, block);
@@ -341,6 +335,8 @@ int main(int argc, char **argv)
 	if (irq_affinity) {
 		char path[PATH_MAX];
 
+		TEST_ASSERT(device, "Manipulating IRQ affinity requires a device");
+
 		snprintf(path, sizeof(path), "/proc/irq/%d/smp_affinity_list", irq);
 		irq_affinity_fp = fopen(path, "w");
 		TEST_ASSERT(irq_affinity_fp, "fopen(%s) failed", path);
@@ -371,17 +367,14 @@ int main(int argc, char **argv)
 			pin_vcpu_threads(nr_vcpus, rand() % get_nprocs(), &available_cpus);
 
 		for (j = 0; j < nr_vcpus; j++) {
-			TEST_ASSERT(
-				!READ_FROM_GUEST(vm, guest_received_irq[vcpu->id]),
-				"IRQ flag for vCPU %d not clear prior to test",
-				vcpu->id);
-			TEST_ASSERT(
-				!READ_FROM_GUEST(vm, guest_received_nmi[vcpu->id]),
-				"NMI flag for vCPU %d not clear prior to test",
+			TEST_ASSERT(!READ_FROM_GUEST(vm, guest_received_irq[vcpu->id]),
+					"IRQ flag for vCPU %d not clear prior to test",
 				vcpu->id);
+			TEST_ASSERT(!READ_FROM_GUEST(vm, guest_received_nmi[vcpu->id]),
+				    "NMI flag for vCPU %d not clear prior to test", vcpu->id);
 		}
 
-		send_msi(device, use_device_msi, msi);
+		trigger_interrupt(device, eventfd);
 
 		clock_gettime(CLOCK_MONOTONIC, &start);
 		for (;;) {
@@ -416,7 +409,7 @@ int main(int argc, char **argv)
 	for (i = 0; i < nr_vcpus; i++) {
 		if (block) {
 			kvm_route_msi(vm, gsi, vcpus[i], vector, false);
-			send_msi(device, false, msi);
+			trigger_interrupt(device, eventfd);
 		}
 
 		pthread_join(vcpu_threads[i], NULL);
@@ -425,15 +418,13 @@ int main(int argc, char **argv)
 	if (irq_affinity)
 		fclose(irq_affinity_fp);
 
-	printf("Host interrupts handled:\n");
-	printf("  IRQ-%d: %lu\n", irq, get_irq_count(irq) - irq_count);
-	printf("  Posted-interrupt notification events: %lu\n",
-	       get_irq_count_by_name("PIN:") - pin_count);
-	printf("  Posted-interrupt wakeup events: %lu\n",
-	       get_irq_count_by_name("PIW:") - piw_count);
+	if (device) {
+		printf("Host interrupts handled:\n");
+		printf("  IRQ-%d: %lu\n", irq, get_irq_count(irq) - irq_count);
 
-	vfio_pci_device_cleanup(device);
-	iommu_cleanup(iommu);
+		vfio_pci_device_cleanup(device);
+		iommu_cleanup(iommu);
+	}
 
 	return 0;
 }

base-commit: 97efbd75a6c9ef0946234df78d3b656e28035883
--