Re: [Qemu-devel] [PATCH 3/3] virtio: set ISR on dataplane notifications

[Michael S. Tsirkin]

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 <address@hidden>