Re: [PATCH v4 08/19] KVM: selftests: Verify interrupts are received when IRQ affinity changes in IRQ test

[Sean Christopherson <seanjc@google.com>]

On Sat, May 30, 2026, Josh Hilke wrote:
> @@ -134,17 +137,21 @@ int main(int argc, char **argv)
>  	u32 gsi = kvm_random_u64_in_range(&kvm_rng, 24, KVM_MAX_IRQ_ROUTES - 1);
>  	u8 vector = kvm_random_u64_in_range(&kvm_rng, 32, UINT8_MAX);
>  
> +	int i, j, c, msi, irq, eventfd, irq_cpu;

This fails to build with -Werror:

irq_test.c: In function ‘main’:
irq_test.c:183:35: error: ‘irq’ may be used uninitialized [-Werror=maybe-uninitialized]
  183 |                 irq_affinity_fp = open_proc_irq_affinity(irq);
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
irq_test.c:140:27: note: ‘irq’ was declared here
  140 |         int i, j, c, msi, irq, eventfd, irq_cpu;
      |                           ^~~
irq_test.c:228:41: error: ‘irq_cpu’ may be used uninitialized [-Werror=maybe-uninitialized]
  228 |                                         printf("  irq_cpu: %d\n", irq_cpu);
      |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
irq_test.c:140:41: note: ‘irq_cpu’ was declared here
  140 |         int i, j, c, msi, irq, eventfd, irq_cpu;
      |                                         ^~~~~~~

The first one (of three distinct uses) is easy enough to solve:

diff --git a/tools/testing/selftests/kvm/irq_test.c b/tools/testing/selftests/kvm/irq_test.c
index 29f54a435c8e..23c82ddee6b9 100644
--- a/tools/testing/selftests/kvm/irq_test.c
+++ b/tools/testing/selftests/kvm/irq_test.c
@@ -251,15 +251,16 @@ int main(int argc, char **argv)
                eventfd = device->msi_eventfds[msi];
                printf("Using device %s MSI-X[%d] (IRQ-%d)\n", device_bdf, msi,
                       irq);
+
+               if (irq_affinity)
+                       irq_affinity_fp = open_proc_irq_affinity(irq);
        } else {
+               TEST_ASSERT(!irq_affinity,
+                           "Setting IRQ affinity (-a) requires a backing device (-d)");
+
                eventfd = kvm_new_eventfd();
        }
 
-       if (irq_affinity) {
-               TEST_ASSERT(device_bdf, "-a requires -d");
-               irq_affinity_fp = open_proc_irq_affinity(irq);
-       }
-
        printf("Injecting interrupts for GSI %d (Vector 0x%x) %d times\n",
               gsi, vector, nr_irqs);
 

And the second can be addressed by checking irq_affinity_fp instead of irq_affinity:

@@ -298,7 +299,7 @@ int main(int argc, char **argv)
 
                kvm_route_msi(vm, gsi, vcpu, vector, do_use_nmi);
 
-               if (irq_affinity && vcpu->id == 0) {
+               if (irq_affinity_fp && vcpu->id == 0) {
                        irq_cpu = kvm_random_u64(&kvm_rng) % get_nprocs();
                        write_proc_irq_affinity(irq_affinity_fp, irq, irq_cpu);
                }

but I don't think that's quite correct.  Keying off vCPU0 for task migration
makes sense, because the test round-robins IRQs to each vCPU, so it's logical to
redo task/vCPU affinity after every round.

But IRQs are being spread over _all_ host CPUs, and there is no round-robin
behavior, it's completely random.  So rather than rate-limit based on number of
vCPUs, I think it makes more sense to hardcode the changes in affinity, e.g.

		if (irq_affinity_fp && !(i % 10)) {
			irq_cpu = kvm_random_u64(&kvm_rng) % get_nprocs();
			write_proc_irq_affinity(irq_affinity_fp, irq, irq_cpu);
		}


Sadly, even that doesn't address the third "used uninitialized" issue:

irq_test.c:334:41: error: ‘irq_cpu’ may be used uninitialized [-Werror=maybe-uninitialized]
  334 |                                         printf("  irq_cpu: %d\n", irq_cpu);
      |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


because apparently gcc isn't smart enough to understand that "!(i % 10)" (or
even "!i" is guaranteed to run on the first iteration.  However, that's false
positive probably a blessing in disguise, because it highlights that printing
only what userspace has _requested_ could be very misleading, given that the
whole point of the offending printf() is to spit out information when the IRQ
got lost.

Writing smp_affinity_list (or smp_affinity) does NOT guarantee the new affinity
is realized in hardware.  E.g. the kernel typically "lazily" migrates IRQs, and
only actually changes the affinity when an IRQ actually arrives.  To see what
CPU an IRQ is actually wired up to, you need to read /proc/<irq>/effective_affinity{,list}.

As it pertains to the "used uninitialized" flaw, I think the test should spit
out three things:

 1. irq_cpu, i.e. what the test thought it set the affinity to
 2. /proc/<irq>/smp_affinity, i.e. what the kernel has for the allowed list
 3. /proc/<irq>/effective_affinity, i.e. what the actual, current affinity is

And then _if_ you want to ratelimit changes in affinity, fix the "used uninitialized"
issue by explicitly initializing irq_cpu, with a comment calling out that the
compiler is being stupid.

@@ -287,6 +288,12 @@ int main(int argc, char **argv)
                }
        }
 
+       /*
+        * Suppress a false positive maybe-uninitialized compiler warning due
+        * to ratelimiting changes in IRQ affinity.
+        */
+       irq_cpu = -1;
+
        for (i = 0; i < nr_irqs; i++) {
                const bool do_clear_routes = clear_routes && (i & BIT(3));
                const bool do_use_nmi = use_nmi && (i & BIT(2));
@@ -298,7 +305,7 @@ int main(int argc, char **argv)
 
                kvm_route_msi(vm, gsi, vcpu, vector, do_use_nmi);
 
-               if (irq_affinity && vcpu->id == 0) {
+               if (irq_affinity_fp && !(i % 10)) {
                        irq_cpu = kvm_random_u64(&kvm_rng) % get_nprocs();
                        write_proc_irq_affinity(irq_affinity_fp, irq, irq_cpu);
                }
@@ -329,7 +336,7 @@ int main(int argc, char **argv)
                                printf("Timeout waiting for interrupt!\n");
                                printf("  vCPU: %d\n", vcpu->id);
                                printf("  is interrupt NMI: %s\n", do_use_nmi ? "true" : "false");
-                               if (irq_affinity)
+                               if (irq_affinity_fp)
                                        printf("  irq_cpu: %d\n", irq_cpu);
                                if (migrate_vcpus)
                                        kvm_print_vcpu_affinity(vcpu, vcpu_tids[vcpu->id]);

But honestly, unless it impacts the runtime, I don't see much value in ratelimiting
changes in IRQ affinity.  Putting that much stress on the IRQ subsystem is actually
probably interesting.

And no matter what, /proc/<irq>/effective_affinity needs to be printed on failure.