| Message ID | 1478695476-19272-1-git-send-email-felipe@nutanix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <felipe@nutanix.com> This also fixes vhost-net after reboot on s390/kvm for me Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> > > [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); > } >
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 <felipe@nutanix.com> > > 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/nvram@71000000 Populating /vdevice/vty@71000100 Populating /pci@800000020000000 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/nvram@71000000 Populating /vdevice/vty@71000100 Populating /pci@800000020000000 00 0000 (D) : 1af4 1000 virtio [ net ] No NVRAM common partition, re-initializing... Scanning USB Using default console: /vdevice/vty@71000100 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/vty@71000100 Preparing to boot Linux version 4.5.0-le_v4.5_aik@vpl2-kernel (aik@vpl2.ozlabs.ibm.com) (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 /pci@800000020000000 rtas_msi: found PE /pci@800000020000000 rtas_msi: counting /pci@800000020000000/ethernet@0 rtas_msi: calc quota for 0000:00:00.0, request 4 rtas_msi: found prop on dn /pci@800000020000000 rtas_msi: found PE /pci@800000020000000 rtas_msi: counting /pci@800000020000000/ethernet@0 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 <borntraeger@de.ibm.com> > >> >> [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); >> } >> > >
> On 16 Nov 2016, at 04:05, Alexey Kardashevskiy <aik@ozlabs.ru> 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 <felipe@nutanix.com> >> >> 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/nvram@71000000 > Populating /vdevice/vty@71000100 > Populating /pci@800000020000000 > 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/nvram@71000000 > Populating /vdevice/vty@71000100 > Populating /pci@800000020000000 > 00 0000 (D) : 1af4 1000 virtio [ net ] > No NVRAM common partition, re-initializing... > Scanning USB > Using default console: /vdevice/vty@71000100 > 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/vty@71000100 > Preparing to boot Linux version 4.5.0-le_v4.5_aik@vpl2-kernel > (aik@vpl2.ozlabs.ibm.com) (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 /pci@800000020000000 > rtas_msi: found PE /pci@800000020000000 > rtas_msi: counting /pci@800000020000000/ethernet@0 > rtas_msi: calc quota for 0000:00:00.0, request 4 > rtas_msi: found prop on dn /pci@800000020000000 > rtas_msi: found PE /pci@800000020000000 > rtas_msi: counting /pci@800000020000000/ethernet@0 > 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 <borntraeger@de.ibm.com> >> >>> >>> [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
On 16/11/16 19:38, Felipe Franciosi wrote: > >> On 16 Nov 2016, at 04:05, Alexey Kardashevskiy <aik@ozlabs.ru> 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 <felipe@nutanix.com> >>> >>> 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 This one does work, thanks!
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); }
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 <felipe@nutanix.com> [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(+)