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

[Alexey Kardashevskiy]

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:

/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