From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45162) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c7IcV-0006hX-IP for qemu-devel@nongnu.org; Thu, 17 Nov 2016 04:04:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c7IcQ-0008Ci-KF for qemu-devel@nongnu.org; Thu, 17 Nov 2016 04:04:35 -0500 Received: from mx6-phx2.redhat.com ([209.132.183.39]:42605) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c7IcQ-0008C0-Cv for qemu-devel@nongnu.org; Thu, 17 Nov 2016 04:04:30 -0500 Date: Thu, 17 Nov 2016 04:04:26 -0500 (EST) From: Paolo Bonzini Message-ID: <1913696176.64577.1479373466625.JavaMail.zimbra@redhat.com> In-Reply-To: <20161116232008-mutt-send-email-mst@kernel.org> References: <20161116180551.9611-1-pbonzini@redhat.com> <20161116180551.9611-4-pbonzini@redhat.com> <20161116220123-mutt-send-email-mst@kernel.org> <195787138.13215853.1479328691064.JavaMail.zimbra@redhat.com> <20161116223848-mutt-send-email-mst@kernel.org> <1250949559.13223324.1479330331523.JavaMail.zimbra@redhat.com> <20161116232008-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org, alex williamson , borntraeger@de.ibm.com, felipe@nutanix.com > > > > > /* > > > > > * 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 > > > > > > > > >