On 15.10.2025 23:04, Jason Andryuk wrote: > io_apic.c has a lot of ad-hoc for(;;) and while(1) loops for iterating > over irq_pin_list entries. Replace them with a standardized > for loop using next_entry() to advance entry. > > Signed-off-by: Jason Andryuk <[email protected]> > --- > xen/arch/x86/io_apic.c | 49 ++++++++++++------------------------------ > 1 file changed, 14 insertions(+), 35 deletions(-) > > diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c > index c35d611ecf..73b48a9cb8 100644 > --- a/xen/arch/x86/io_apic.c > +++ b/xen/arch/x86/io_apic.c > @@ -191,6 +191,14 @@ static void remove_pin_from_irq(unsigned int irq, int > apic, int pin) > irq_2_pin_free_entry = entry - irq_2_pin; > } > > +static struct irq_pin_list *next_entry(const struct irq_pin_list *entry) > +{ > + if ( !entry->next ) > + return NULL; > + > + return irq_2_pin + entry->next; > +}
Preferably with the function put in its final place right in patch 1: Acked-by: Jan Beulich <[email protected]> > @@ -482,7 +487,7 @@ static void modify_IO_APIC_irq(unsigned int irq, unsigned > int enable, > { > struct irq_pin_list *entry = irq_2_pin + irq; > > - for (;;) { > + for (; entry; entry = next_entry(entry)) { > unsigned int pin = entry->pin; > struct IO_APIC_route_entry rte; > > @@ -492,9 +497,6 @@ static void modify_IO_APIC_irq(unsigned int irq, unsigned > int enable, > rte.raw &= ~(uint64_t)disable; > rte.raw |= enable; > __ioapic_write_entry(entry->apic, pin, false, rte); > - if (!entry->next) > - break; > - entry = irq_2_pin + entry->next; > } > } I notice that within here there's also a "break" upon ->pin being -1. Seeing that io_apic_level_ack_pending() has continue there, I think we will want to be consistent. Which way isn't quite clear to me (yet). > @@ -545,14 +547,11 @@ static void __eoi_IO_APIC_irq(struct irq_desc *desc) > struct irq_pin_list *entry = irq_2_pin + desc->irq; > unsigned int pin, vector = desc->arch.vector; > > - for (;;) { > + for (; entry; entry = next_entry(entry)) { > pin = entry->pin; > if (pin == -1) > break; Same here. > @@ -632,7 +631,7 @@ set_ioapic_affinity_irq(struct irq_desc *desc, const > cpumask_t *mask) > if ( !iommu_intremap || !x2apic_enabled ) > dest = SET_APIC_LOGICAL_ID(dest); > entry = irq_2_pin + irq; > - for (;;) { > + for (; entry; entry = next_entry(entry)) { > struct IO_APIC_route_entry rte; > > pin = entry->pin; > @@ -643,10 +642,6 @@ set_ioapic_affinity_irq(struct irq_desc *desc, const > cpumask_t *mask) > rte.dest.dest32 = dest; > rte.vector = desc->arch.vector; > __ioapic_write_entry(entry->apic, pin, false, rte); > - > - if (!entry->next) > - break; > - entry = irq_2_pin + entry->next; > } > } And here. Jan
