On 05.09.2025 22:34, Jason Andryuk wrote:
> On 2025-09-05 03:39, Jan Beulich wrote:
>> On 04.09.2025 23:51, Jason Andryuk wrote:
>>> io_apic_level_ack_pending() will end up in an infinite loop if
>>> entry->pin == -1. entry does not change, so it will keep reading -1.
>>>
>>> Switched to breaking out of the loop.
>>>
>>> Fixes: f821102450a1 ("x86: IRQ Migration logic enhancement.")
>>> Signed-off-by: Jason Andryuk <[email protected]>
>>> ---
>>> Noticed during code inspection.
>>
>> Well spotted, just that ...
>>
>>> --- a/xen/arch/x86/io_apic.c
>>> +++ b/xen/arch/x86/io_apic.c
>>> @@ -1715,7 +1715,7 @@ static bool io_apic_level_ack_pending(unsigned int
>>> irq)
>>>
>>> pin = entry->pin;
>>> if (pin == -1)
>>> - continue;
>>> + break;
>>
>> ... we shouldn't terminate the loop here, but rather continue with the next
>> entry in the list (if any). Hence presumably why "continue" was used, without
>> achieving the intended effect.
>
> Ok, makes sense. Though after the sending the patch, I was wondering if
> it was an unreachable condition, and we should not end up here with pin
> == -1.
I can't exclude that, but other code (e.g. add_pin_to_irq()) also deal with
this case.
Jan
