Re: [Qemu-devel] [PATCH v2] vhost: Update 'ioeventfd_started' with host notifiers

[Felipe Franciosi]

> On 16 Nov 2016, at 04:05, Alexey Kardashevskiy <address@hidden> wrote:
> 
> On 11/11/16 01:45, Christian Borntraeger wrote:
>> On 11/09/2016 01:44 PM, Felipe Franciosi wrote:
>>> Following the recent refactor of virtio notfiers [1], more specifically
>>> the patch that uses virtio_bus_set_host_notifier [2] by default, core
>>> virtio code requires 'ioeventfd_started' to be set to true/false when
>>> the host notifiers are configured. Because not all vhost devices were
>>> update (eg. vhost-scsi) to use the new interface, this value is always
>>> set to false.
>>> 
>>> When booting a guest with a vhost-scsi backend controller, SeaBIOS will
>>> initially configure the device which sets all notifiers. The guest will
>>> continue to boot fine until the kernel virtio-scsi driver reinitialises
>>> the device causing a stop followed by another start. Since
>>> ioeventfd_started was never set to true, the 'stop' operation triggered
>>> by virtio_bus_set_host_notifier() will not result in a call to
>>> virtio_pci_ioeventfd_assign(assign=false). This leaves the memory
>>> regions with stale notifiers and results on the next start triggering
>>> the following assertion:
>>> 
>>>  kvm_mem_ioeventfd_add: error adding ioeventfd: File exists
>>>  Aborted
>>> 
>>> This patch updates ioeventfd_started whenever the notifiers are set or
>>> cleared, fixing this issue.
>>> 
>>> Signed-off-by: Felipe Franciosi <address@hidden>
>> 
>> This also fixes vhost-net after reboot on s390/kvm for me
> 
> 
> It does not fix it (the original breakage from e616c2f "virtio: remove
> ioeventfd_disabled altogether") for me:

Can you try Paolo's latest patches for this issue?
http://lists.nongnu.org/archive/html/qemu-devel/2016-11/msg02834.html

Specifically this:
http://lists.nongnu.org/archive/html/qemu-devel/2016-11/msg02837.html

If that doesn't work, can you please plug a gdb on your qemu and print a stack 
trace once you hit the assertion?

Thanks,
Felipe


> 
> /home/aik/p/qemu/ppc64-softmmu/qemu-system-ppc64 -nodefaults \
> -chardev stdio,id=STDIO0,signal=off,mux=on \
> -device spapr-vty,id=svty0,chardev=STDIO0,reg=0x71000100 \
> -mon id=MON0,chardev=STDIO0,mode=readline -nographic -vga none \
> -enable-kvm -m 2G \
> -kernel /home/aik/t/vml450le \
> -initrd /home/aik/t/le.cpio \
> -netdev tap,id=TAP0,vhost=on,helper=/home/aik/qemu-bridge-helper \
> -device "virtio-net-pci,id=vnet0,mac=C0:41:49:4b:00:00,netdev=TAP0" \
> -smp 16,threads=8 \
> -trace events=qemu_trace_events \
> -machine pseries \
> -L /home/aik/t/qemu-ppc64-bios/
> QEMU PID = 22145
> QEMU 2.7.50 monitor - type 'help' for more information
> (qemu)
> 
> 
> SLOF **********************************************************************
> QEMU Starting
> Build Date = Nov 14 2016 19:13:53
> FW Version = git-9b8945ecbde65b06
> Press "s" to enter Open Firmware.
> 
> Populating /vdevice methods
> Populating /vdevice/address@hidden
> Populating /vdevice/address@hidden
> Populating /address@hidden
>                     00 0000 (D) : 1af4 1000    virtio [ net ]
> qemu-system-ppc64: /home/aik/p/qemu/memory.c:1940:
> memory_region_del_eventfd: Assertion `i != mr->ioeventfd_nb' failed.
> QEMU pid = 22145 returned -6
> 
> 
> 
> 
> Without this one, the breakage looked different (error would have happened
> lot later, when in the guest kernel):
> 
> 
> 
> SLOF **********************************************************************
> QEMU Starting
> Build Date = Nov 14 2016 19:13:53
> FW Version = git-9b8945ecbde65b06
> Press "s" to enter Open Firmware.
> 
> Populating /vdevice methods
> Populating /vdevice/address@hidden
> Populating /vdevice/address@hidden
> Populating /address@hidden
>                     00 0000 (D) : 1af4 1000    virtio [ net ]
> No NVRAM common partition, re-initializing...
> Scanning USB
> Using default console: /vdevice/address@hidden
>     ted RAM kernel at 400000 (16ef23c bytes) C08FF
>  Welcome to Open Firmware
> 
>  Copyright (c) 2004, 2011 IBM Corporation All rights reserved.
>  This program and the accompanying materials are made available
>  under the terms of the BSD License available at
>  http://www.opensource.org/licenses/bsd-license.php
> 
> Booting from memory...
> OF stdout device is: /vdevice/address@hidden
> Preparing to boot Linux version address@hidden
> (address@hidden) (gcc version 5.4.1 20160623 (GCC) ) #59 SMP
> 
> [skipping bunch of boring stuff]
> 
> virtio-pci 0000:00:00.0: enabling device (0100 -> 0103)
> HVCS: Driver registered.
> Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
> brd: module loaded
> loop: module loaded
> Uniform Multi-Platform E-IDE driver
> ide-gd driver 1.18
> ide-cd driver 5.00
> Loading iSCSI transport class v2.0-870.
> Emulex LightPulse Fibre Channel SCSI driver 11.0.0.10.
> Copyright(c) 2004-2015 Emulex.  All rights reserved.
> ipr: IBM Power RAID SCSI Device Driver version: 2.6.3 (October 17, 2015)
> ibmvfc: IBM Virtual Fibre Channel Driver version: 1.0.11 (April 12, 2013)
> rtas_msi: calc quota for 0000:00:00.0, request 3
> rtas_msi: found prop on dn /address@hidden
> rtas_msi: found PE /address@hidden
> rtas_msi: counting /address@hidden/address@hidden
> rtas_msi: calc quota for 0000:00:00.0, request 4
> rtas_msi: found prop on dn /address@hidden
> rtas_msi: found PE /address@hidden
> rtas_msi: counting /address@hidden/address@hidden
> rtas_msi: ibm,change_msi(func=4,num=4), got 3 rc = 3
> rtas_msi: ibm,change_msi(func=4,num=3), got 3 rc = 3
> virtio-pci 0000:00:00.0: rtas_msi: allocated virq 21
> virtio-pci 0000:00:00.0: rtas_msi: allocated virq 22
> virtio-pci 0000:00:00.0: rtas_msi: allocated virq 23
> kvm_mem_ioeventfd_add: error adding ioeventfd: File exists
> QEMU pid = 22411 returned -6
> 
> 
> 
>> 
>> Tested-by: Christian Borntraeger <address@hidden>
>> 
>>> 
>>> [1] http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg07748.html
>>> [2] http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg07760.html
>>> ---
>>> v1->v2:
>>>  - Update ioeventfd_started in vhost_dev_enable/disable_notifiers()
>>>    instead of vhost_scsi_start/stop().
>>>  - Reword the commit message accordingly.
>>> ---
>>> hw/virtio/vhost.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>> 
>>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>>> index 131f164..1290963 100644
>>> --- a/hw/virtio/vhost.c
>>> +++ b/hw/virtio/vhost.c
>>> @@ -1205,6 +1205,7 @@ int vhost_dev_enable_notifiers(struct vhost_dev 
>>> *hdev, VirtIODevice *vdev)
>>>             goto fail_vq;
>>>         }
>>>     }
>>> +    VIRTIO_BUS(qbus)->ioeventfd_started = true;
>>> 
>>>     return 0;
>>> fail_vq:
>>> @@ -1239,6 +1240,7 @@ void vhost_dev_disable_notifiers(struct vhost_dev 
>>> *hdev, VirtIODevice *vdev)
>>>         }
>>>         assert (r >= 0);
>>>     }
>>> +    VIRTIO_BUS(qbus)->ioeventfd_started = false;
>>>     virtio_device_start_ioeventfd(vdev);
>>> }
>>> 
>> 
>> 
> 
> 
> -- 
> Alexey