On Mon, Nov 28, 2022 at 07:39:29PM +0000, Alex Bennée wrote: > > 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.
So I should expect v4? And I guess we can split out documentation things then? > > > > Stefan > > > -- > Alex Bennée
