Re: [Qemu-devel] [PATCH] virtio-pci: Clear IRQ at reset

[Fam Zheng <famz@redhat.com>]

On Thu, 03/12 10:41, Michael S. Tsirkin wrote:
> On Thu, Mar 12, 2015 at 02:40:55PM +0800, Fam Zheng wrote:
> > Currently we could leave PCI IRQ asserted even after reset, it is safer
> > to clear it.
> > 
> > In the case that a buggy driver has disabled MSI-X unintentially, we may
> > have already injected IRQ in previous virtio_pci_notify, which will not
> > be cleared by guest because it doesn't expect it (i.e. no irq handler).
> > However the driver may eventually notice the unresponsiveness and reset
> > the device, at that point, clearing the irq is meaningful.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> I don't think we care about buggy drivers being able to recover, but the
> same would apply to e.g. kdump when using virtio blk/scsi after a crash,
> correct?

OK, yes, get your point. And this also relates to the patch you replied
yesterday:

http://www.spinics.net/lists/kernel/msg1943182.html

/*

The hanging problem, which is due to irq not being acknowledged, can only be
fixed by your patch if the irq is cleared at reset.

The uninterruptable process doesn't really matter, the system would still
shutdown, as long as the wrong irq is cleard.

See also RHBZ https://bugzilla.redhat.com/show_bug.cgi?id=1199155

*/

Fam

> 
> > ---
> >  hw/virtio/virtio-pci.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index e7baf7b..2600f1e 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -988,6 +988,7 @@ static void virtio_pci_reset(DeviceState *qdev)
> >      virtio_pci_stop_ioeventfd(proxy);
> >      virtio_bus_reset(bus);
> >      msix_unuse_all_vectors(&proxy->pci_dev);
> > +    pci_irq_deassert(&proxy->pci_dev);
> >  }
> >  
> >  static Property virtio_pci_properties[] = {
> > -- 
> > 1.9.3
>