Re: [Qemu-devel] [PATCH] ioapic: allow buggy guests mishandling level-triggered interrupts to make progress

[Vitaly Kuznetsov]

Paolo Bonzini <address@hidden> writes:

> On 01/04/19 15:36, Vitaly Kuznetsov wrote:
...
>>  static void ioapic_set_irq(void *opaque, int vector, int level)
>>  {
>>      IOAPICCommonState *s = opaque;
>> @@ -227,7 +236,28 @@ void ioapic_eoi_broadcast(int vector)
>>                  trace_ioapic_clear_remote_irr(n, vector);
>>                  s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
>>                  if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) {
>> -                    ioapic_service(s);
>> +                    bool level = ((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) 
>> & 1)
>> +                        == IOAPIC_TRIGGER_LEVEL;
>> +
>> +                    ++s->irq_reassert[vector];
>> +                    if (!level ||
>> +                        s->irq_reassert[vector] < SUCCESSIVE_IRQ_MAX_COUNT) 
>> {
>> +                        ioapic_service(s);
>> +                    } else {
>> +                        /*
>> +                         * Real hardware does not deliver the interrupt
>> +                         * immediately during eoi broadcast, and this lets a
>> +                         * buggy guest make slow progress even if it does 
>> not
>> +                         * correctly handle a level-triggered interrupt. 
>> Emulate
>> +                         * this behavior if we detect an interrupt storm.
>> +                         */
>> +                        trace_ioapic_eoi_delayed_reassert(vector);
>> +                        timer_mod(s->timer,
>> +                                  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
>> +                                  NANOSECONDS_PER_SECOND / 100);
>
> Should this be done only if the timer isn't pending?

Hm, maybe ... but how can this happen? To get here we need remote IRR
bit and we clear it so someone needs to re-set it. The source won't
probably be doing this (it is a level-triggered interrupt and it is
already pending - why re-asserting?) but even if it does
ioapic_service(s) will be called and when our timer fires we will just
do nothing (consequitive ioapic_service() doesn't hurt).

-- 
Vitaly