Re: [Qemu-devel] [PATCH v4 0/2] virtio: Move host features to backends

[Christian Borntraeger]

Am 29.04.2015 um 22:19 schrieb Michael S. Tsirkin:
[...]
>>
>> This commit made it work.
>>
>> commit 7a11370e5e6c26566904bb7f08281093a3002ff2
>> Author: Michael S. Tsirkin <address@hidden>
>> Date:   Wed Oct 15 10:22:30 2014 +1030
>>
>>     virtio_blk: enable VQs early
>>     
>>     virtio spec requires drivers to set DRIVER_OK before using VQs.
>>     This is set automatically after probe returns, virtio block violated this
>>     rule by calling add_disk, which causes the VQ to be used directly within
>>     probe.
>>     
>>     To fix, call virtio_device_ready before using VQs.
>>     
>>     Signed-off-by: Michael S. Tsirkin <address@hidden>
>>     Reviewed-by: Cornelia Huck <address@hidden>
>>     Signed-off-by: Rusty Russell <address@hidden>
> 
> I guess this means s390 code somehow lost kicks that happened before
> DRIVER_OK. Without event index you would typically get another one on
> the next request.

The problem is that feature updates are not a synchronous op for this 
transport. This transport syncs the guest feature bits (those from finalize)
on the set_status call. Before that qemu thinks that features are zero, which
means QEMU will not write the event index thus the 2nd kick will be lost.

This quick hack makes the problem go away with older kernels

diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 15b2c0f..25abb10 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -85,6 +85,14 @@ static int s390_virtio_hcall_notify(const uint64_t *args)
     if (mem > ram_size) {
         VirtIOS390Device *dev = s390_virtio_bus_find_vring(s390_bus, mem, &i);
         if (dev) {
+            /*
+             * older kernels will use the virtqueue before setting DRIVER_OK.
+             * In this case the feature bits are not yet up to date, meaning
+             * that several funny things can happen, e.g. the guest thinks
+             * EVENT_IDX is on and QEMU thinks its off. Force a feature sync.
+             */
+            if (dev->vdev->status != VIRTIO_CONFIG_S_DRIVER_OK)
+                s390_virtio_device_update_status(dev);
             virtio_queue_notify(dev->vdev, i);
         } else {
             r = -EINVAL;


Unless there are better ideas, I will respin that as a proper patch.

Christian