Re: [PATCH v2 4/5] KVM: Kick resamplefd for split kernel irqchip

[Peter Xu]

On Mon, Mar 09, 2020 at 04:33:59PM -0600, Alex Williamson wrote:

[...]

> > > diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
> > > index 15747fe2c2..13921b333d 100644
> > > --- a/hw/intc/ioapic.c
> > > +++ b/hw/intc/ioapic.c
> > > @@ -236,8 +236,27 @@ void ioapic_eoi_broadcast(int vector)
> > >          for (n = 0; n < IOAPIC_NUM_PINS; n++) {
> > >              entry = s->ioredtbl[n];
> > >  
> > > -            if ((entry & IOAPIC_VECTOR_MASK) != vector ||
> > > -                ((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) != 
> > > IOAPIC_TRIGGER_LEVEL) {
> > > +            if ((entry & IOAPIC_VECTOR_MASK) != vector) {
> > > +                continue;
> > > +            }
> > > +
> > > +            /*
> > > +             * When IOAPIC is in the userspace while APIC is still in
> > > +             * the kernel (i.e., split irqchip), we have a trick to
> > > +             * kick the resamplefd logic for registered irqfds from
> > > +             * userspace to deactivate the IRQ.  When that happens, it
> > > +             * means the irq bypassed userspace IOAPIC (so the irr and
> > > +             * remote-irr of the table entry should be bypassed too
> > > +             * even if interrupt come), then we don't need to clear
> > > +             * the remote-IRR and check irr again because they'll
> > > +             * always be zeros.
> > > +             */
> > > +            if (kvm_resample_fd_notify(n)) {
> > > +                continue;
> > > +            }  
> > 
> > It seems the problem I reported is here.  In my configuration virtio-blk
> > and an assigned e1000e share an interrupt.  virtio-blk is initializing
> > and apparently triggers an interrupt.  The vfio-pci device is
> > configured for INTx though not active yet, but kvm_resample_fd_notify()
> > kicks the fd here, so we continue.  If I remove the continue here both
> > devices seem to work, but I don't claim to understand the condition
> > we're trying to continue for here yet.  This series needs more testing
> > with shared interrupts.  Thanks,
> 
> I'm also curious how this ended up between testing whether the vector
> is masked and testing that it's level triggered.  We shouldn't have any
> edge triggered resamplers.

We had a similar discussion in V1 (with Paolo):

https://patchwork.kernel.org/patch/11407441/#23190891

So my understanding is that VFIO will unmask the intx IRQ when it
comes, and register the resamplefd too, no matter whether it's level
triggered (at least from what the code does).  Am I right?

> I find however that if I move the resampler
> notify to after the remote IRR test, my NIC gets starved of interrupts.
> So empirically, it seems kvm_resample_fd_notify() should be a void
> function called unconditionally between the original mask+level check
> removed above and the IRR check below.  Thanks,

Yes IMHO we can't move that notify() to be after the remote IRR check
because the IRR and remote IRR will be completely bypassed for the
assigned device.  In other words, even if the interrupt has arrived
for the assigned device, both IRR and remote IRR should always be zero
(assuming the virtio-blk device doesn't generate any IRQ).  So we
probably still need to do the notify even if remote-irr is not set.

Thanks,

-- 
Peter Xu