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

[Michael S. Tsirkin]

On Thu, Mar 12, 2015 at 06:00:28PM +0800, Fam Zheng wrote:
> 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 <address@hidden>
> > 
> > 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

About that one, I still don't know why it's necessary.
reboot causes a system reset and that will eventually
deassert an irq, no?


> /*
> 
> 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
> >