Re: [PATCH v2] kvm: ioapic: Introduce arch-specific check for lazy update EOI mechanism

[Paolo Bonzini]

On 04/05/20 08:31, Suravee Suthikulpanit wrote:
>>
>>
>> The re-entrancy happens because the irq state is the OR of
>> the interrupt state and the resamplefd state.  That is, we don't
>> want to show the state as 0 until we've had a chance to set the
>> resamplefd.  But if the interrupt has _not_ gone low then we get an
>> infinite loop.
> 
> I'm not too familiar w/ the resamplefd.  I must have missed this part.
> Could you please point out to me where the OR logic is?

It's because kvm_ioapic_set_irq gets the actual state of the interrupt
line from __kvm_irq_line_state before calling ioapic_set_irq.

>> But in the case of level-triggered interrupts the VMEXIT already
>> happens because TMR is set; only edge-triggered interrupts need
>> the lazy invocation of the ack notifier.  
> 
> For AVIC, EOI write for level-triggered would have also be trapped.

Yes, I was talking about AVIC only there.

> And yes, edge-triggered needs lazy ack notifier.
> 
>> So this should be the fix:
>>
>> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
>> index 7668fed1ce65..ca2d73cd00a3 100644
>> --- a/arch/x86/kvm/ioapic.c
>> +++ b/arch/x86/kvm/ioapic.c
>> @@ -225,12 +225,12 @@ static int ioapic_set_irq(struct kvm_ioapic
>> *ioapic, unsigned int irq,
>>       }
>>         /*
>> -     * AMD SVM AVIC accelerate EOI write and do not trap,
>> -     * in-kernel IOAPIC will not be able to receive the EOI.
>> +     * AMD SVM AVIC accelerate EOI write iff the interrupt is level
>> +     * triggered, in-kernel IOAPIC will not be able to receive the EOI.
> 
> Actually, it should be "AMD SVM AVIC accelerate EOI write iff the
> interrupt is _edge_ triggered".

Of course, thanks.

Paolo