On 2025-10-16 02:53, Jan Beulich wrote:
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).
Right. I don't know. It seems like ->pin == -1 indicates an unused
entry, so stopping in the case makes sense. I've wondered if
io_apic_level_ack_pending() continues just in case an entry->next points
to another pin to ack. i.e. it's not the expected case, but it also
might help if entry->next points to something valid.
Regards,
Jason
