Re: [PATCH v6 09/18] KVM: selftests: Verify interrupts are received when IRQ affinity changes in IRQ test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Using get_nprocs() to generate a random CPU ID for IRQ affinity can produce offline or non-existent CPU IDs, leading to test crashes.
- [Medium] The test picks `gsi` and `vector` using an unseeded RNG state, making the selections completely deterministic across runs.
- [Low] The `irq_cpu` variable is used uninitialized in a TEST_ASSERT message when the test is run with `-d` but without `-a`.
--

commit 49ffc3d4b49fbef72ecbe743669ae8e39951edb2
Author: David Matlack <dmatlack@google.com>

KVM: selftests: Verify interrupts are received when IRQ affinity changes in IRQ test

This commit extends the eventfd IRQ test with a '-a' flag to randomly
affinitize the device's host IRQ to different physical CPUs. This stresses
the kernel's ability to maintain correct interrupt routing and delivery
during dynamic affinity changes.

> diff --git a/tools/testing/selftests/kvm/irq_test.c b/tools/testing/selftests/kvm/irq_test.c
> index 6888be54ee4a9..9e22874902628 100644
> --- a/tools/testing/selftests/kvm/irq_test.c
> +++ b/tools/testing/selftests/kvm/irq_test.c
[ ... ]
> @@ -160,10 +163,13 @@ int main(int argc, char **argv)
>  	int i, j, c, msix, eventfd;
>  	struct iommu *iommu;
>  	struct kvm_vm *vm;
> -	int irq;
> +	int irq, irq_cpu;

[Severity: Medium]
Does this test pick gsi and vector using an unseeded rng state?

At the beginning of main(), kvm_random_u64_in_range() is called with kvm_rng.
Because kvm_rng is in BSS, its seed is 0. The actual RNG initialization
happens later during vm_create_with_vcpus(). This means the initial
randomization will predictably return the minimum bounds (24 for gsi and
32 for vector), defeating the intended coverage.

[ ... ]
> @@ -192,7 +198,12 @@ int main(int argc, char **argv)
>  		printf("Using device %s MSI-X[%d] (IRQ-%u)\n", device_bdf, msix,
>  		       irq);
>  	} else {
> +		TEST_ASSERT(!irq_affinity,
> +			    "Setting IRQ affinity (-a) requires a backing device (-d)");
> +
>  		eventfd = kvm_new_eventfd();
> +		irq = -1;
> +		irq_cpu = -1;
>  	}

[Severity: Low]
Is irq_cpu used uninitialized here when the test is run with -d but without -a?

When a backing device is specified (-d), the code enters the if (device_bdf)
block and irq_cpu remains uninitialized. If a vCPU times out waiting for an
interrupt, the TEST_ASSERT macro evaluates and passes the uninitialized
irq_cpu to the variadic format string, printing garbage values.

[ ... ]
> @@ -216,6 +227,11 @@ int main(int argc, char **argv)
>  
>  		kvm_route_msi(vm, gsi, vcpu, vector);
>  
> +		if (irq_affinity) {
> +			irq_cpu = kvm_random_u64(&kvm_rng) % get_nprocs();
> +			proc_irq_set_smp_affinity(irq, irq_cpu);
> +		}

[Severity: Medium]
Can get_nprocs() produce offline or non-existent CPU IDs, leading to test
crashes?

get_nprocs() returns the total number of currently online processors, not the
maximum CPU ID. In systems with sparse CPU topologies or offline CPUs (e.g.,
CPUs 0, 2, 3 online; CPU 1 offline), get_nprocs() returns 3.

The modulo calculation will produce indices from 0 to 2. If it produces 1, the
test writes '1' to /proc/irq/<irq>/smp_affinity_list. This will fail with
-EINVAL because CPU 1 is offline, triggering a spurious test failure via
TEST_ASSERT(r > 0) in proc_irq_set_smp_affinity().

[ ... ]
> @@ -229,8 +245,8 @@ int main(int argc, char **argv)
>  			cpu_relax();
>  
>  		TEST_ASSERT(GUEST_RECEIVED_IRQ(vcpu),
> -			    "vCPU %d timed out waiting for IRQ (vector 0x%x) from GSI %d\n",
> -			    vcpu->id, vector, gsi);
> +			    "vCPU %d timed out waiting for IRQ (vector 0x%x) from GSI %d (via CPU %d)\n",
> +			    vcpu->id, vector, gsi, irq_cpu);
>  
>  		WRITE_AND_SYNC_TO_GUEST(vm, guest_received_irq[vcpu->id], false);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610005338.2967132-1-seanjc@google.com?part=9