On Mon, 9 Mar 2020 20:38:08 -0400
Peter Xu <pet...@redhat.com> wrote:
> 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?
As Paolo replied in your previous discussion, INTx is always level
triggered.
> > 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.
Yep. Thanks,
Alex
