From: [email protected] <[email protected]> Sent: Tuesday, October 20, 2026 11:45 AM > > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) > to > consider: > - [Medium] PCI: hv: early activation in hv_pci_vec_irq_domain_activate() > skews CPU0 load
This was intentional, and in my judgment, it's OK to skew against CPU 0. Low speed VMBus devices (i.e., not storvsc or netvsc) are assigned to CPU 0 outside of what pci-hyperv.c knows about, as is the emulated PL011 UART for the VM serial console. Even in aggregate, these don't generate a lot of interrupts, but they could still cause some interference with a high-speed NVMe completion queue (for example). Ultimately, the spreading is just a heuristic that is set up to be helpful in the all the normal cases. But if there are edge cases where it's not perfect, that's OK. I'll add a code comment in v2 describing the reasoning. > - [Medium] PCI: hv: cpu_spi_count corruption in hv_pci_arm64_set_affinity() > when inactive This is a good issue. It's an error in the tracking that can easily be fixed. I'll fix it in v2 by setting IRQD_AFFINITY_ON_ACTIVATE, which is what the arm64 ITS and x86 VECTOR domains do. Michael > -- > > commit 9d112d599453572154cd3fbc90e3b6ea7dedf0e3 > Author: Michael Kelley <[email protected]> > > PCI: hv: Spread load across CPUs when selecting effective affinity CPU > > This patch extends the arm64 Hyper-V vPCI MSI irq chip to track per-CPU > SPI assignments and select the least-loaded CPU from the affinity mask > before forwarding the single-CPU effective affinity to the parent GICv3 > chip. It also updates the domain activate and deactivate callbacks to > increment and decrement the per-CPU SPI counters. > > > diff --git a/drivers/pci/controller/pci-hyperv.c > > b/drivers/pci/controller/pci-hyperv.c > > index 824cf27e85e67..7c293e39c9208 100644 > > --- a/drivers/pci/controller/pci-hyperv.c > > +++ b/drivers/pci/controller/pci-hyperv.c > > [ ... ] > > > +/* > > + * Track the count of SPIs assigned to each CPU. Pick a single CPU from the > > + * affinity mask based on the SPI loading data, and have the parent IRQ > > + * chip (should be "GICv3") set the effective affinity to that single CPU. > > + */ > > +static int hv_pci_arm64_set_affinity(struct irq_data *d, > > + const struct cpumask *mask_val, > > + bool force) > > +{ > > + unsigned int cpu, prev_cpu; > > + int ret; > > + > > + prev_cpu = cpumask_first(irq_data_get_effective_affinity_mask(d)); > > + if (prev_cpu < nr_cpu_ids) > > + hv_pci_dec_spi_count(d, prev_cpu); > > + > > + if (force) > > + cpu = hv_pci_arm64_get_min_load(d, mask_val); > > + else > > + cpu = hv_pci_arm64_select_cpu(d, mask_val); > > + > > + if (cpu >= nr_cpu_ids) > > + goto err; > > + > > + ret = irq_chip_set_affinity_parent(d, cpumask_of(cpu), force); > > + if (ret < 0) > > + goto err; > > + > > + hv_pci_inc_spi_count(d, cpu); > > + return ret; > > + > > +err: > > + if (prev_cpu < nr_cpu_ids) > > + hv_pci_inc_spi_count(d, prev_cpu); > > + return -EINVAL; > > +} > > [Severity: Medium] > Can hv_pci_arm64_set_affinity() corrupt cpu_spi_count when affinity is > changed on a deactivated interrupt? > > When an allocated MSI or MSI-X interrupt is deactivated (for example, when > free_irq() -> __free_irq() calls irq_domain_deactivate_irq() while MSI > vectors remain allocated across ndo_stop/ndo_open or queue reconfiguration, > or when migrate_one_irq() calls irq_shutdown_and_deactivate()), > hv_pci_vec_irq_domain_deactivate() decrements cpu_spi_count for the CPU in > effective_affinity and IRQD_ACTIVATED is cleared, but the old CPU remains > set in the effective_affinity mask: > > drivers/pci/controller/pci-hyperv.c:hv_pci_vec_irq_domain_deactivate() { > cpu = cpumask_first(irq_data_get_effective_affinity_mask(irqd)); > if (cpu < nr_cpu_ids) > hv_pci_dec_spi_count(irqd, cpu); > } > > Because hv_pci_vec_irq_domain_alloc() does not set IRQD_AFFINITY_ON_ACTIVATE > (unlike its_irq_domain_alloc() in drivers/irqchip/irq-gic-v3-its.c), > irq_set_affinity_deactivated() does not defer affinity changes on a > deactivated interrupt: > > kernel/irq/manage.c:irq_set_affinity_deactivated() { > if (!IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) || > irqd_is_activated(data) || !irqd_affinity_on_activate(data)) > return false; > ... > } > > If irq_set_affinity() or irq_force_affinity() is called while the interrupt > is deactivated (for example, via /proc/irq/<irq>/smp_affinity, > irq_set_affinity_hint(), or irq_restore_affinity_of_irq()), > irq_set_affinity_locked() -> irq_do_set_affinity() invokes > hv_pci_arm64_set_affinity(). That reads the stale prev_cpu from > irq_data_get_effective_affinity_mask(d), decrements prev_cpu's > cpu_spi_count a second time, and increments the newly selected cpu's > cpu_spi_count while IRQD_ACTIVATED remains false. > > If the vector is later freed via pci_free_irq_vectors() -> > __msi_domain_free_irqs() in kernel/irq/msi.c, irq_domain_deactivate_irq() > is skipped because !irqd_is_activated(irqd), leaking the count on the new > CPU; or if the vector is re-activated via request_irq() -> irq_activate() > -> hv_pci_vec_irq_domain_activate(), effective_affinity is overwritten > with cpumask_first(cpu_online_mask) without decrementing the CPU selected > while deactivated. > > Should hv_pci_vec_irq_domain_alloc() set IRQD_AFFINITY_ON_ACTIVATE, or > should hv_pci_vec_irq_domain_deactivate() clear effective_affinity? > > [ ... ] > > > @@ -885,21 +1007,39 @@ static int hv_pci_vec_irq_domain_alloc(struct > > irq_domain > *domain, > > * composing MSI from the hypervisor. GIC will eventually set the right > > * affinity for the irq and the 'unmask' will retarget the interrupt to > > that > > * cpu. > > + * > > + * The first cpu is always used for initial activation because spreading > > the > > + * initial CPU would have to be done without information about affinity > > groups > > + * that may exist. The resulting assignments would pollute the "least > > loaded" > > + * data for making the final assignments. Everything would still work, but > > + * the spreading would be less even than it could be. > > */ > > static int hv_pci_vec_irq_domain_activate(struct irq_domain *domain, > > struct irq_data *irqd, bool reserve) > > { > > - int cpu = cpumask_first(cpu_present_mask); > > + int cpu = cpumask_first(cpu_online_mask); > > > > + hv_pci_inc_spi_count(irqd, cpu); > > irq_data_update_effective_affinity(irqd, cpumask_of(cpu)); > > > > return 0; > > } > > [Severity: Medium] > Won't incrementing cpu_spi_count in hv_pci_vec_irq_domain_activate() skew > the load calculation away from the first online CPU during multi-vector > MSI/MSI-X setup? > > Because pci_msi_template and pci_msix_template in > drivers/pci/msi/irqdomain.c set MSI_FLAG_ACTIVATE_EARLY, allocating N > vectors via pci_alloc_irq_vectors() or pci_alloc_irq_vectors_affinity() > causes __msi_domain_alloc_irqs() to activate all N vectors in a batch via > msi_init_virq() -> irq_domain_activate_irq() -> > hv_pci_vec_irq_domain_activate() before returning and before request_irq() > is called on any vector: > > kernel/irq/msi.c:__msi_domain_alloc_irqs() { > ... > for (i = 0; i < desc->nvec_used; i++) { > irq_set_msi_desc_off(virq, i, desc); > irq_debugfs_copy_devname(virq + i, dev); > ret = msi_init_virq(domain, virq + i, vflags); > if (ret) > return ret; > } > } > > This increments cpu_spi_count on cpumask_first(cpu_online_mask) (typically > CPU 0) for all N vectors up front, inflating CPU 0's count by N. > > When the driver then calls request_irq() for each vector i = 0..N-1 in > sequence (such as in nvme_setup_io_queues()), irq_startup() -> > irq_do_set_affinity() -> hv_pci_arm64_set_affinity() decrements CPU 0's > cpu_spi_count by only 1 for vector i, leaving CPU 0's count inflated by > N - 1 - i from the remaining unstarted vectors of the same device still > parked on CPU 0. When hv_pci_arm64_select_cpu() selects a CPU for the first > queue (whose affinity mask includes CPU 0), it sees CPU 0's load inflated > by N - 1 and avoids assigning interrupts to CPU 0 for the first N - 1 > devices probed. > > Furthermore, if a driver allocates M vectors and only calls request_irq() > on K < M of them (or defers request_irq() until queues or network ports are > opened), the M - K unrequested vectors remain activated on CPU 0 and keep > its cpu_spi_count inflated by M - K. > > Could hv_pci_vec_irq_domain_activate() leave effective_affinity empty and > avoid incrementing cpu_spi_count until hv_pci_arm64_set_affinity() selects > the actual target CPU? > > [ ... ] > > -- > Sashiko AI review * > https://sashiko.dev/ > %23%2Fpatchset%2F20261020182433.336637-1- > mhklinux%40outlook.com%3Fpart%3D1&data=05%7C02%7C%7Ca9adc6acf8e34ef04 > ebd08df2eda50ac%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C63928118 > 7342913040%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIw > LjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C > %7C&sdata=nT7Wvhc6rrXjaC%2FqpvQj99H2cNbdzGmYWs4AeKb2jlU%3D&reserved= > 0
