Re: [PATCH v5 05/21] KVM: selftests: Add IRQ injection test

[Sean Christopherson <seanjc@google.com>]

On Thu, Jun 04, 2026, Josh Hilke wrote:
> +static void kvm_route_msi(struct kvm_vm *vm, u32 gsi, struct kvm_vcpu *vcpu,
> +			  u8 vector)
> +{
> +	struct {
> +		struct kvm_irq_routing head;

s/head/header, because I read this as "head entry", not "header of the struct".

> +		struct kvm_irq_routing_entry entry;
> +	} routing_data = {};
> +
> +	struct kvm_irq_routing *routes = &routing_data.head;
> +
> +	routes->nr = 1;
> +	routes->entries[0].gsi = gsi;

This is silly (and confusing), just initialize the information in the struct
declaration:

	struct {
		struct kvm_irq_routing header;
		struct kvm_irq_routing_entry entry;
	} routing = {
		.header.nr = 1,
		.entry = {
			.gsi = gsi,
			.type = KVM_IRQ_ROUTING_MSI,
			.u.msi.address_lo = 0xFEE00000 | (vcpu->id << 12),
			.u.msi.data = use_nmi ? NMI_VECTOR | (4 << 8) : vector,
		},
	};

	vm_ioctl(vm, KVM_SET_GSI_ROUTING, &routing.header);

> +	routes->entries[0].type = KVM_IRQ_ROUTING_MSI;
> +	routes->entries[0].u.msi.address_lo = 0xFEE00000 | (vcpu->id << 12);
> +	routes->entries[0].u.msi.data = vector;
> +
> +	vm_ioctl(vm, KVM_SET_GSI_ROUTING, routes);
> +}
> +
> +static void help(const char *name)
> +{
> +	printf("Usage: %s [-h]\n", name);
> +	printf("\n");
> +	printf("Tests KVM IRQ injection via irqfd using an emulated eventfd.\n");

This is all kinds of misleading and wrong.  IRQ *injection* is not guaranteed.
KVM will use CPU-to-CPU posted interrupts to deliver the interrupt, if supported.
And it's not an "emulated eventfd"; the test isn't emulated anything.  It's
manually *signaling* the eventfd but there's no emulation of any kind.  And that
becomes stale when the test adds support for VFIO devices.

	Tests KVM interrupt routing and delivery via irqfd.

> +	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(!GUEST_RECEIVED_IRQ(vcpus[j]),
> +				    "IRQ flag for vCPU %d not clear prior to test",
> +				    vcpus[j]->id);
> +
> +		/* Trigger interrupt */

Meh, not helpful.  And it goes away in a few patches.

> +		eventfd_write(eventfd, 1);
> +
> +		clock_gettime(CLOCK_MONOTONIC, &start);
> +		for (;;) {
> +			if (GUEST_RECEIVED_IRQ(vcpu))
> +				break;

This is a kludgy and confusing way of writing:

		clock_gettime(CLOCK_MONOTONIC, &start);
		while (GUEST_RECEIVED_INTERRUPT(vcpu, do_use_nmi) &&
		       timespec_to_ns(timespec_elapsed(start)) <= timeout_ns)
			cpu_relax();

> +
> +			if (timespec_to_ns(timespec_elapsed(start)) > timeout_ns)
> +				TEST_FAIL("vCPU %d timed out waiting for IRQ from GSI %d (Vector 0x%x) !\n",
> +					  vcpu->id, gsi, vector);

Don't do an if-statement and then TEST_FAIL(), just TEST_ASSERT().  And with the
above, hoist this out of the loop:

		TEST_ASSERT(GUEST_RECEIVED_INTERRUPT(vcpu, do_use_nmi),
			    "vCPU %d timed out waiting for %s (vector 0x%x) from GSI %d (via CPU %d)\n",
			    vcpu->id, do_use_nmi ? "NMI" : "IRQ",
			    do_use_nmi ? NMI_VECTOR : vector, gsi, irq_cpu);

And as above, this should cpu_relax().

> +		}
> +
> +		WRITE_AND_SYNC_TO_GUEST(vm, guest_received_irq[vcpu->id], false);
> +	}
> +
> +	WRITE_AND_SYNC_TO_GUEST(vm, done, true);
> +
> +	for (i = 0; i < nr_vcpus; i++)
> +		pthread_join(vcpu_threads[i], NULL);
> +
> +	printf("Test passed!\n");

This is redundant, the return code provides this information.

> +
> +	return 0;
> +}
> -- 
> 2.54.0.1032.g2f8565e1d1-goog
>