[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications |
Date: |
Thu, 17 Nov 2016 04:04:26 -0500 (EST) |
> > > > > /*
> > > > > * virtio spec 1.0 says ISR bit 0 should be ignored with MSI, but
> > > > > * windows drivers included in virtio-win 1.8.0 (circa 2015)
> > > > > * for Windows 8.1 only are incorrectly polling this bit during
> > > > > shutdown
> > > > ^^^^^^^^^^^^^^^^
> > > >
> > > > Not sure it's only for Windows 8.1, in fact probably not.
> > >
> > > 8.1 on shutdown and others on crashdump or hibernation?
> >
> > Even 8.1 is really a hibernation hidden behind a "Shut down" menu item.
> >
> > Paolo
>
> what does "hang during shutdown" in your commit log refer to then?
The full text from the commit log is:
recent releases of
Windows do not really shut down, but rather log out and hibernate to
make the next startup faster. Hence, this manifested as a more serious
hang during shutdown with e.g. Windows 8.1 and virtio-win 1.8.0 RPMs.
Shutdown in the commit log just means "clicking Shut down". The previous
sentence explains why. Also note the "e.g.", I've not tested other versions
of Windows.
Paolo
> > > > Looks good if you replace this line with
> > > >
> > > > "are incorrectly polling this bit during crashdump or hibernation"
> > > >
> > > > Paolo
> > > >
> > > > > * in MSI mode, causing a hang if this bit is never updated.
> > > > > * Next driver release from 2016 fixed this problem, so working
> > > > > around it
> > > > > * is not a must, but it's easy to do so let's do it here.
> > > > > *
> > > > > * Note: it's safe to update ISR from any thread as it was switched
> > > > > * to an atomic operation.
> > > > > */
> > > >
> > > >
> > > > >
> > > > >
> > > > >
> > > > > > + event_notifier_set(&vq->guest_notifier);
> > > > > > +}
> > > > > > +
> > > > > > void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
> > > > > > {
> > > > > > if (!virtio_should_notify(vdev, vq)) {
> > > > > > @@ -1990,7 +1994,7 @@ static void
> > > > > > virtio_queue_guest_notifier_read(EventNotifier *n)
> > > > > > {
> > > > > > VirtQueue *vq = container_of(n, VirtQueue, guest_notifier);
> > > > > > if (event_notifier_test_and_clear(n)) {
> > > > > > - virtio_irq(vq);
> > > > > > + virtio_notify_vector(vq->vdev, vq->vector);
> > > > > > }
> > > > > > }
> > > > > >
> > > > > > diff --git a/include/hw/virtio/virtio-scsi.h
> > > > > > b/include/hw/virtio/virtio-scsi.h
> > > > > > index 9fbc7d7..7375196 100644
> > > > > > --- a/include/hw/virtio/virtio-scsi.h
> > > > > > +++ b/include/hw/virtio/virtio-scsi.h
> > > > > > @@ -137,6 +137,5 @@ void virtio_scsi_push_event(VirtIOSCSI *s,
> > > > > > SCSIDevice
> > > > > > *dev,
> > > > > > void virtio_scsi_dataplane_setup(VirtIOSCSI *s, Error **errp);
> > > > > > int virtio_scsi_dataplane_start(VirtIODevice *s);
> > > > > > void virtio_scsi_dataplane_stop(VirtIODevice *s);
> > > > > > -void virtio_scsi_dataplane_notify(VirtIODevice *vdev,
> > > > > > VirtIOSCSIReq
> > > > > > *req);
> > > > > >
> > > > > > #endif /* QEMU_VIRTIO_SCSI_H */
> > > > > > diff --git a/include/hw/virtio/virtio.h
> > > > > > b/include/hw/virtio/virtio.h
> > > > > > index 835b085..ab0e030 100644
> > > > > > --- a/include/hw/virtio/virtio.h
> > > > > > +++ b/include/hw/virtio/virtio.h
> > > > > > @@ -181,6 +181,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq,
> > > > > > unsigned
> > > > > > int *in_bytes,
> > > > > > unsigned max_in_bytes, unsigned
> > > > > > max_out_bytes);
> > > > > >
> > > > > > bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq);
> > > > > > +void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq);
> > > > > > void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
> > > > > >
> > > > > > void virtio_save(VirtIODevice *vdev, QEMUFile *f);
> > > > > > @@ -280,7 +281,6 @@ void
> > > > > > virtio_queue_host_notifier_read(EventNotifier
> > > > > > *n);
> > > > > > void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq,
> > > > > > AioContext
> > > > > > *ctx,
> > > > > > void
> > > > > > (*fn)(VirtIODevice
> > > > > > *,
> > > > > >
> > > > > > VirtQueue
> > > > > > *));
> > > > > > -void virtio_irq(VirtQueue *vq);
> > > > > > VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t
> > > > > > vector);
> > > > > > VirtQueue *virtio_vector_next_queue(VirtQueue *vq);
> > > > > >
> > > > > > --
> > > > > > 2.9.3
> > > > >
> > >
>
- [Qemu-devel] [PATCH 2/3] virtio: access ISR atomically, (continued)
- [Qemu-devel] [PATCH 2/3] virtio: access ISR atomically, Paolo Bonzini, 2016/11/16
- [Qemu-devel] [PATCH 1/3] virtio: introduce grab/release_ioeventfd to fix vhost, Paolo Bonzini, 2016/11/16
- [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications, Paolo Bonzini, 2016/11/16
- Re: [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications, Michael S. Tsirkin, 2016/11/16
- Re: [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications, Paolo Bonzini, 2016/11/16
- Re: [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications, Michael S. Tsirkin, 2016/11/16
- Re: [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications, Paolo Bonzini, 2016/11/16
- Re: [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications, Michael S. Tsirkin, 2016/11/16
- Re: [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications,
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications, Michael S. Tsirkin, 2016/11/17
- Re: [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications, Stefan Hajnoczi, 2016/11/17
Re: [Qemu-devel] [PATCH v2 for-2.8 0/3] virtio fixes, no-reply, 2016/11/16