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

[Suravee Suthikulpanit]

Paolo,

On 4/25/20 4:52 PM, Paolo Bonzini wrote:
> On 24/04/20 07:08, Suravee Suthikulpanit wrote:
> > commit f458d039db7e ("kvm: ioapic: Lazy update IOAPIC EOI") introduces
> > the following regression on Intel VMX APICv.
> >
> > BUG: stack guard page was hit at 000000008f595917 \
> > (stack is 00000000bdefe5a4..00000000ae2b06f5)
> > kernel stack overflow (double-fault): 0000 [#1] SMP NOPTI
> > RIP: 0010:kvm_set_irq+0x51/0x160 [kvm]
> > Call Trace:
> >   irqfd_resampler_ack+0x32/0x90 [kvm]
> >   kvm_notify_acked_irq+0x62/0xd0 [kvm]
> >   kvm_ioapic_update_eoi_one.isra.0+0x30/0x120 [kvm]
> >   ioapic_set_irq+0x20e/0x240 [kvm]
> >   kvm_ioapic_set_irq+0x5c/0x80 [kvm]
> >   kvm_set_irq+0xbb/0x160 [kvm]
> >   ? kvm_hv_set_sint+0x20/0x20 [kvm]
> >   irqfd_resampler_ack+0x32/0x90 [kvm]
> >   kvm_notify_acked_irq+0x62/0xd0 [kvm]
> >   kvm_ioapic_update_eoi_one.isra.0+0x30/0x120 [kvm]
> >   ioapic_set_irq+0x20e/0x240 [kvm]
> >   kvm_ioapic_set_irq+0x5c/0x80 [kvm]
> >   kvm_set_irq+0xbb/0x160 [kvm]
> >   ? kvm_hv_set_sint+0x20/0x20 [kvm]
> > ....
> >
> > This is due to the logic always force IOAPIC lazy update EOI mechanism
> > when APICv is activated, which is only needed by AMD SVM AVIC.
> >
> > Fixes by introducing struct kvm_arch.use_lazy_eoi variable to specify
> > whether the architecture needs lazy update EOI support.
>
> You are not explaining why the same infinite loop cannot happen on AMD.
>   It seems to me that it is also fixed by adding a check for re-entrancy
> in ioapic_lazy_update_eoi.  It's easy to add one since
> ioapic_lazy_update_eoi is called with the ioapic->lock taken.
>
> Paolo

I finally reproduced on AMD system as well. I'll send out a new patch for this based on your suggestion.

Suravee