Re: [PATCH 3/4] virtio-pci: return IRQ_HANDLED for config-change interrupts

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Apr 07, 2026 at 01:35:26PM -0400, Andrew Stellman wrote:
> Fixes: 77cf524654a8 ("virtio_pci: split up vp_interrupt")
> 
> Should I resend as v2 with the tag included, or is adding it on your end
> easier?


how was this tested? I don't think a unit test is sufficient here, FYI.


> On Tue, Apr 7, 2026 at 12:20 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> 
>     On Tue, Apr 07, 2026 at 08:39:03AM -0400, Andrew Stellman wrote:
>     > vp_interrupt() unconditionally returns the result of
>     > vp_vring_interrupt(). When a config-change interrupt fires but no vring
>     > activity is pending, vp_vring_interrupt() returns IRQ_NONE — even
>     > though the interrupt was legitimately handled by vp_config_changed().
>     > Over time this causes the IRQ subsystem to flag the line as spurious.
>     >
>     > Track the return value explicitly: set ret to IRQ_HANDLED when the
>     > config-change bit is set, OR it with the vring result, and return the
>     > combined value.
>     >
>     > Signed-off-by: Andrew Stellman <astellman@stellman-greene.com>
> 
>     Fixes tag?
> 
>     > ---
>     >  drivers/virtio/virtio_pci_common.c | 10 ++++++++--
>     >  1 file changed, 8 insertions(+), 2 deletions(-)
>     >
>     > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/
>     virtio_pci_common.c
>     > index 0b9d66b..1d1ab02 100644
>     > --- a/drivers/virtio/virtio_pci_common.c
>     > +++ b/drivers/virtio/virtio_pci_common.c
>     > @@ -106,6 +106,7 @@ static irqreturn_t vp_vring_interrupt(int irq, void
>     *opaque)
>     >  static irqreturn_t vp_interrupt(int irq, void *opaque)
>     >  {
>     >       struct virtio_pci_device *vp_dev = opaque;
>     > +     irqreturn_t ret = IRQ_NONE;
>     >       u8 isr;
>     > 
>     >       /* reading the ISR has the effect of also clearing it so it's very
>     > @@ -117,10 +118,15 @@ static irqreturn_t vp_interrupt(int irq, void
>     *opaque)
>     >               return IRQ_NONE;
>     > 
>     >       /* Configuration change?  Tell driver if it wants to know. */
>     > -     if (isr & VIRTIO_PCI_ISR_CONFIG)
>     > +     if (isr & VIRTIO_PCI_ISR_CONFIG) {
>     >               vp_config_changed(irq, opaque);
>     > +             ret = IRQ_HANDLED;
>     > +     }
>     > 
>     > -     return vp_vring_interrupt(irq, opaque);
>     > +     if (vp_vring_interrupt(irq, opaque) == IRQ_HANDLED)
>     > +             ret = IRQ_HANDLED;
>     > +
>     > +     return ret;
>     >  }
>     > 
>     >  static int vp_request_msix_vectors(struct virtio_device *vdev, int
>     nvectors,
>     > --
>     > 2.34.1
> 
>