Stefan Hajnoczi <stefa...@gmail.com> writes:
> On Mon, 28 Nov 2022 at 11:42, Alex Bennée <alex.ben...@linaro.org> wrote: >> >> There was a disconnect here because vdev->host_features was set to >> random rubbish. This caused a weird negotiation between the driver and >> device that took no account of the features provided by the backend. >> To fix this we must set vdev->host_features once we have initialised >> the vhost backend. >> >> [AJB: however this is confusing because AFAICT none of the other >> vhost-user devices do this.] >> >> Signed-off-by: Alex Bennée <alex.ben...@linaro.org> >> --- >> hw/virtio/vhost-user-gpio.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/hw/virtio/vhost-user-gpio.c b/hw/virtio/vhost-user-gpio.c >> index 5851cb3bc9..b2496c824c 100644 >> --- a/hw/virtio/vhost-user-gpio.c >> +++ b/hw/virtio/vhost-user-gpio.c >> @@ -228,6 +228,12 @@ static int vu_gpio_connect(DeviceState *dev, Error >> **errp) >> return ret; >> } >> >> + /* >> + * Once we have initialised the vhost backend we can finally set >> + * the what host features are available for this device. >> + */ >> + vdev->host_features = vhost_dev->features; > > vdev->host_feature is already set by virtio_bus_device_plugged -> > vu_gpio_get_features. > > Something is still wrong. > > My understanding is: ->realize() performs a blocking connect so when > it returns and virtio_bus_device_plugged() runs, we'll be able to > fetch the backend features from ->get_features(). The assumption is > that the backend features don't change across reconnection (I think). > > vhost-user-gpio seems to follow the same flow as the other vhost-user > devices (vhost-user-net is special, so let's ignore it), so I don't > understand why it's necessary to manually assign ->host_features here? I think you are right. I suspect I got confused while chasing down the missing high bits (due to the legacy VirtIO negotiation). Also slightly thrown off by the appearance of virtio-net (I assume because of a machine default?): ➜ env QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon QTEST_QEMU_IMG=./qemu-img QTEST_QEMU_BINARY=./qemu-system-aarch64 G_TEST_DBUS_DAEMON=/home/alex/ lsrc/qemu.git/tests/dbus-vmstate-daemon.sh MALLOC_PERTURB_=177 ./tests/qtest/qos-test --tap -p /aarch64/virt/generic-pcihost/pci-bus-generic/pci-bus/vhost-user-gpio-pci/vhost-user-gpio/vhost-user-gpio-tests/read-guest-mem/memfile # random seed: R02S235ee9d31e87e0159f810979be62563e # starting QEMU: exec ./qemu-system-aarch64 -qtest unix:/tmp/qtest-1276289.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-1276289.qmp,id=char0 -mon chardev=char0,mode=control -display none -machine none -accel qtest # Start of aarch64 tests # Start of virt tests # Start of generic-pcihost tests # Start of pci-bus-generic tests # Start of pci-bus tests # Start of vhost-user-gpio-pci tests # Start of vhost-user-gpio tests # Start of vhost-user-gpio-tests tests # Start of read-guest-mem tests virtio_bus_device_plugged: virtio-net host_features = 0x10179bf8064 vu_gpio_connect: pre host_features = 10039000000 vu_gpio_connect: post host_features = 140000001 virtio_bus_device_plugged: virtio-gpio host_features = 0x140000001 qemu-system-aarch64: Failed to set msg fds. qemu-system-aarch64: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-aarch64: Failed to set msg fds. qemu-system-aarch64: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-aarch64: Failed to set msg fds. qemu-system-aarch64: vhost_set_vring_call failed: Invalid argument (22) qemu-system-aarch64: Failed to set msg fds. qemu-system-aarch64: vhost_set_vring_call failed: Invalid argument (22) # qos_test running single test in subprocess # set_protocol_features: 0x200 # set_owner: start of session # vhost-user: un-handled message: 14 # vhost-user: un-handled message: 14 # set_vring_num: 0/256 # set_vring_addr: 0x7f55b0000000/0x7f55affff000/0x7f55b0001000 ok 1 /aarch64/virt/generic-pcihost/pci-bus-generic/pci-bus/vhost-user-gpio-pci/vhost-user-gpio/vhost-user-gpio-tests/read-guest-mem/memfile # Start of memfile tests # End of memfile tests # End of read-guest-mem tests # End of vhost-user-gpio-tests tests # End of vhost-user-gpio tests # End of vhost-user-gpio-pci tests # End of pci-bus tests # End of pci-bus-generic tests # End of generic-pcihost tests # End of virt tests # End of aarch64 tests 1..1 and for mmio: env MALLOC_PERTURB_=235 QTEST_QEMU_IMG=./qemu-img QTEST_QEMU_BINARY="./qemu-system-arm" QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon G_TEST_DBUS_DAEMON=/home/alex/lsrc/qemu.git/tests/dbus-vmstate-daemon.sh ./tests/qtest/qos-test --tap -p /arm/virt/virtio-mmio/virtio-bus/vhost-user-gpio-device/vhost-user-gpio/vhost-user-gpio-tests/read-guest-mem/memfile # random seed: R02Sac48bb073367f77c1c1a1db6b5245c9e # starting QEMU: exec ./qemu-system-arm -qtest unix:/tmp/qtest-1276963.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-1276963.qmp,id=char0 -mon chardev=char0,mode=control -display none -machine none -accel qtest # Start of arm tests # Start of virt tests # Start of virtio-mmio tests # Start of virtio-bus tests # Start of vhost-user-gpio-device tests # Start of vhost-user-gpio tests # Start of vhost-user-gpio-tests tests # Start of read-guest-mem tests virtio_bus_device_plugged: virtio-net host_features = 0x10179bf8064 vu_gpio_connect: pre host_features = 10039000000 vu_gpio_connect: post host_features = 140000001 virtio_bus_device_plugged: virtio-gpio host_features = 0x140000001 qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost_set_vring_call failed: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost_set_vring_call failed: Invalid argument (22) # qos_test running single test in subprocess # set_protocol_features: 0x200 # set_owner: start of session # vhost-user: un-handled message: 14 # vhost-user: un-handled message: 14 ok 1 /arm/virt/virtio-mmio/virtio-bus/vhost-user-gpio-device/vhost-user-gpio/vhost-user-gpio-tests/read-guest-mem/memfile # Start of memfile tests # End of memfile tests # End of read-guest-mem tests # End of vhost-user-gpio-tests tests # End of vhost-user-gpio tests # End of vhost-user-gpio-device tests # End of virtio-bus tests # End of virtio-mmio tests # End of virt tests # End of arm tests 1..1 I'll drop this patch for now and re-test. > > Stefan -- Alex Bennée
