Re: [PATCH v2 03/14] KVM: selftests: Add vfio_pci_irq_test

[Sean Christopherson <seanjc@google.com>]

On Tue, Mar 31, 2026, Josh Hilke wrote:
> +int main(int argc, char **argv)
> +{
> +	/*
> +	 * Pick a random vector and a random GSI to use for device IRQ.
> +	 *
> +	 * Pick an IRQ vector in range [32, UINT8_MAX]. Min value is 32 because
> +	 * Linux/x86 reserves vectors 0-31 for exceptions and architecture
> +	 * defined NMIs and interrupts.
> +	 *
> +	 * Pick a GSI in range [24, KVM_MAX_IRQ_ROUTES - 1]. The min value is 24
> +	 * because KVM reserves GSIs 0-15 for legacy ISA IRQs and 16-23 only go
> +	 * to the IOAPIC. The max is KVM_MAX_IRQ_ROUTES - 1, because
> +	 * KVM_MAX_IRQ_ROUTES is exclusive.
> +	*/
> +	u32 gsi = 24 + rand() % (KVM_MAX_IRQ_ROUTES - 1 - 24);
> +	u8 vector = 32 + rand() % (UINT8_MAX - 32);

Ahh, now I see why you included "Reproduce tests that rely on randomization" in
this series.

I think I'd prefer to tweak guest_random_state() to drop the "guest" in favor of
"kvm", and then use that framework in host code as well.  Then you wouldn't need
to open code the % fun.

> +	/* Test configuration (overridable by command line flags). */
> +	int nr_irqs = 1000;
> +	int nr_vcpus = 1;
> +
> +	struct kvm_vcpu *vcpus[KVM_MAX_VCPUS];
> +	pthread_t vcpu_threads[KVM_MAX_VCPUS];
> +	u64 irq_count, pin_count, piw_count;
> +	struct vfio_pci_device *device;
> +	struct iommu *iommu;
> +	const char *device_bdf;
> +	int i, j, c, msi, irq;
> +	struct kvm_vm *vm;
> +
> +	device_bdf = vfio_selftests_get_bdf(&argc, argv);
> +
> +	while ((c = getopt(argc, argv, "h")) != -1) {
> +		switch (c) {
> +		case 'h':
> +		default:
> +			help(argv[0]);
> +		}
> +	}
> +
> +	vm = vm_create_with_vcpus(nr_vcpus, guest_code, vcpus);
> +	vm_install_exception_handler(vm, vector, guest_irq_handler);
> +
> +	iommu = iommu_init(default_iommu_mode);
> +	device = vfio_pci_device_init(device_bdf, iommu);
> +	msi = setup_msi(device);
> +	irq = get_irq_number(device_bdf, msi);
> +
> +	irq_count = get_irq_count(irq);
> +	pin_count = get_irq_count_by_name("PIN:");
> +	piw_count = get_irq_count_by_name("PIW:");

I'm *very* skeptical of blindling printing PIN/PIW.  They're Intel specific and
are global to the entire system.  Outside of debugging specific issues, I don't
see them providing much value, while on the other hand, they could easily confuse
readers.  And unlike the IRQ counts, the data won't be lost when the test exits.

As for the get_irq_number() and get_irq_count(), those need to be much more clearly
scoped to /proc/interrupts.  E.g. "irq" in KVM context most often refers to the
guest IRQ vector, not the host Linux make-believe IRQ number.

> +	printf("%s %s MSI-X[%d] (IRQ-%d) %d times\n",
> +               "Notifying the eventfd for",

Where's the rest of this line?  Oh, this is the first %s.  LOL, that's ridiculous.

	printf("Notifying the eventfd for BDF %s, MSI-X[%d] (IRQ-%d) %d times\n",
	       device_bdf, msi, irq, nr_irqs);
	       
> +	       device_bdf, msi, irq, nr_irqs);
> +
> +	kvm_assign_irqfd(vm, gsi, device->msi_eventfds[msi]);
> +
> +	for (i = 0; i < nr_vcpus; i++)
> +		pthread_create(&vcpu_threads[i], NULL, vcpu_thread_main, vcpus[i]);
> +
> +	for (i = 0; i < nr_vcpus; i++) {
> +		struct kvm_vcpu *vcpu = vcpus[i];
> +
> +		while (!READ_FROM_GUEST(vm, guest_ready_for_irqs[vcpu->id]))
> +			continue;
> +	}
> +
> +	/* Set a consistent seed so that test are repeatable. */
> +	srand(0);

And using a pRNG instead of rand() should make this unnecessary.

> +	for (i = 0; i < nr_irqs; i++) {
> +		struct kvm_vcpu *vcpu = vcpus[i % nr_vcpus];
> +		struct timespec start;
> +
> +		kvm_route_msi(vm, gsi, vcpu, vector);
> +
> +		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);
> +		}
> +
> +		send_msi(device, msi);
> +
> +		clock_gettime(CLOCK_MONOTONIC, &start);
> +		for (;;) {
> +			if (READ_FROM_GUEST(vm, guest_received_irq[vcpu->id]))
> +				break;
> +
> +			if (timespec_to_ns(timespec_elapsed(start)) > TIMEOUT_NS) {
> +				printf("Timeout waiting for interrupt!\n");
> +				printf("  vCPU: %d\n", vcpu->id);
> +
> +				TEST_FAIL("vCPU never received IRQ!\n");

Which vCPU?  What BDF+MSI?  Some of that is available in earlier messages, but
failure messages should be standalone.

> +			}
> +		}
> +
> +		WRITE_TO_GUEST(vm, guest_received_irq[vcpu->id], false);
> +	}
> +
> +	WRITE_TO_GUEST(vm, done, true);
> +
> +	for (i = 0; i < nr_vcpus; i++) {

Unnecessary curly braces.

> +		pthread_join(vcpu_threads[i], NULL);
> +	}
> +
> +	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);
> +
> +	vfio_pci_device_cleanup(device);
> +	iommu_cleanup(iommu);
> +
> +	return 0;
> +}
> -- 
> 2.53.0.1118.gaef5881109-goog
>