On 06/06/2019 12:43, Jan Beulich wrote:
>>>> On 06.06.19 at 13:34, <andrew.coop...@citrix.com> wrote:
>> On 06/06/2019 09:17, Jan Beulich wrote:
>>>>>> On 05.06.19 at 19:15, <andrew.coop...@citrix.com> wrote:
>>>> On 08/05/2019 13:46, Jan Beulich wrote:
>>>>> @@ -1130,8 +1130,10 @@ static void irq_guest_eoi_timer_fn(void
>>>>> }
>>>>> }
>>>>>
>>>>> - if ( action->in_flight != 0 )
>>>>> - goto out;
>>>>> + if ( action->in_flight )
>>>>> + printk(XENLOG_G_WARNING
>>>>> + "IRQ%d: %d handlers still in flight at forced EOI\n",
>>>>> + desc->irq, action->in_flight);
>>>> AFACIT, this condition can be triggered by a buggy/malicious guest, by
>>>> it simply ignoring or masking the line interrupt at the vIO-APIC.
>>> I don't think it can, no. Or else the ASSERT_UNREACHABLE() below
>>> here would be invalid to add.
>> Which ASSERT_UNREACHABLE() ? I know Roger asked for one, but I don't
>> see it anywhere in the code.
> Because so far there was no real reason to re-post. It's right here,
> as Roger did ask for, and as I did (hesitantly) agree:
>
> if ( action->in_flight )
> {
> printk(XENLOG_G_WARNING
> "IRQ%u: %d/%d handler(s) still in flight at forced EOI\n",
> irq, action->in_flight, action->nr_guests);
> ASSERT_UNREACHABLE();
> }
>
>>>> The message would be far more useful if it identified the domain in
>>>> question, which looks like it can be obtained from the middle of the loop.
>>> That very loop has just taken care of decrementing ->in_flight for
>>> all such guests.
>>>
>>> Also note that there could be more than one offending domain, for
>>> shared IRQs. Plus the loop you're referring to can specifically _not_
>>> be used for identifying the domain(s), because for the ones
>>> processed there we _did_ decrement ->in_flight. If this message
>>> gets logged, we simply have no idea why ->in_flight is _still_ non-
>>> zero. This could be a BUG_ON(), but it seems more in line with our
>>> general idea of how we would like to deal with such cases to try
>>> and keep the system running here in release builds.
>> Ok - lets go with this for now. It is a net improvement, and we can
>> evaluate the guest-triggerability at a later point.
>>
>> Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>
> Thanks much. I'll assume this holds also for the adjustments
> requested by Roger.
Fine. At least that should make things obvious in a debug build.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
