From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45209) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c6fcu-0006jn-OK for qemu-devel@nongnu.org; Tue, 15 Nov 2016 10:26:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c6fcp-0002xn-Q3 for qemu-devel@nongnu.org; Tue, 15 Nov 2016 10:26:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33662) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c6fcp-0002xd-Kk for qemu-devel@nongnu.org; Tue, 15 Nov 2016 10:26:19 -0500 Date: Tue, 15 Nov 2016 17:26:17 +0200 From: "Michael S. Tsirkin" Message-ID: <20161115172336-mutt-send-email-mst@kernel.org> References: <20161115134629.23161-1-pbonzini@redhat.com> <20161115134629.23161-4-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161115134629.23161-4-pbonzini@redhat.com> 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: Paolo Bonzini Cc: qemu-devel@nongnu.org, alex.williamson@redhat.com, borntraeger@de.ibm.com, felipe@nutanix.com On Tue, Nov 15, 2016 at 02:46:29PM +0100, Paolo Bonzini wrote: > Dataplane has been omitting forever the step of setting ISR when an > interrupt is raised. This causes surprisingly little breakage, > because most polling-mode drivers look at the used ring's index field > rather than the ISR register. > > Some versions of the Windows drivers are an exception---and they use > polling mode with ISR for crashdump and hibernation. And because > recent releases of Windows do not really shut down, but rather log > out and hibernate to make the next startup faster, this manifested > as a hang during shutdown with e.g. Windows 8.1 and virtio-win 1.8.0 > RPMs. Newer versions probably poll the used index; older versions > do not use MSI and therefore go through the emulated irqfd path > (virtio_queue_guest_notifier_read), which handled ISR correctly. Confused. virtio spec says ISR shouldn't be set on ring activity in MSI mode. Is this a driver bug? > The failure has always been there for virtio dataplane, but it > became visible after commits 9ffe337 ("virtio-blk: always use > dataplane path if ioeventfd is active", 2016-10-30) and > ad07cd6 ("virtio-scsi: always use dataplane path if ioeventfd > is active", 2016-10-30), which removed the non-dataplane ioeventfd > path for virtio-blk and virtio-scsi. The good news therefore > is that it was not a bug in the patches---they did exactly what they > were meant for, i.e. shake out remaining dataplane bugs. > > The fix is not hard. The virtio_should_notify+event_notifier_set > pair that is common to virtio-blk and virtio-scsi dataplane > is replaced with a new public function virtio_notify_irqfd > that also sets ISR. The irqfd emulation code now need not > set ISR anymore, so virtio_irq is removed. > > Signed-off-by: Paolo Bonzini