On 15/11/2016 16:26, Michael S. Tsirkin wrote:
> 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?
Huh, then probably it is, and this is why it works with newer versions
of the driver. They probably forgot to disable MSI mode when entering
hibernation mode.
But in the end QEMU is doing different things in non-dataplane vs.
dataplane mode, at least for virtio-blk and virtio-scsi (I'm sure
virtio-net vs. vhost would have the same issue, just no driver that is
affected). Is it SHOULD NOT or MUST NOT or MAY NOT?
Paolo
>
>> 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 <pbonz...@redhat.com>
