On Mon, Oct 28, 2024 at 6:38 AM Sahil Siddiq <icegambi...@gmail.com> wrote: > > Hi, > > It's been a while since I gave my last update. I have one more update > that I would like to give. > > > On Tue, Sep 24, 2024 at 7:31 AM Sahil <icegambi...@gmail.com> wrote: > > > And I booted L2 by running: > > > > > > # ./qemu/build/qemu-system-x86_64 \ > > > -nographic \ > > > -m 4G \ > > > -enable-kvm \ > > > -M q35 \ > > > -drive file=//root/L2.qcow2,media=disk,if=virtio \ > > > -netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-0,id=vhost-vdpa0 \ > > > -device > > > virtio-net-pci,netdev=vhost-vdpa0,disable-legacy=on,disable-modern=off,ev > > > ent_idx=off,bus=pcie.0,addr=0x7 \ -smp 4 \ > > > -cpu host \ > > > 2>&1 | tee vm.log > > > > With packed=on in the device option, I see that the packed feature bit is > > set in L2 :) > > > > However, I see that vhost shadow virtqueues are still not being used. I am > > currently trying to find the reason behind this. I have narrowed down the > > issue to hw/virtio/vhost-vdpa.c [1]. The "vhost_vdpa_svqs_start" function > > is being called but in the loop, vhost_svq_start is never called. I think it > > might be because there's an issue with "vhost_vdpa_svq_setup". > > > > I'll send an update once I find something. > > > > Thanks, > > Sahil > > > > [1] https://github.com/qemu/qemu/blob/master/hw/virtio/vhost-vdpa.c#L1243 > > I spent some time tinkering with the L0-L1-L2 test environment setup, > and understanding QEMU's hw/virtio/vhost-vdpa.c [1] as well as Linux's > drivers/vhost/vdpa.c [2] and /drivers/vhost/vhost.c [3]. I don't think there > is an issue with the environment itself. > > When I boot L2 with the following combinations of "x-svq" and > "packed", this is what I observe: > > 1. x-svq=on and packed=off > > The virtio device in L2 has the packed feature bit turned off. Vhost > shadow virtqueues are used as expected. > > 2. x-svq=off and packed=on > > The virtio device in L2 has the packed feature bit turned on. Vhost > shadow virtqueues are not used. > > I don't see any issues in either of the above environment > configurations. > > 3. x-svq=on and packed=on > > This is the configuration that I need for testing. The virtio device in > L2 has the packed feature bit turned on. However, vhost shadow > virtqueues are not being used. This is due to the > VHOST_SET_VRING_BASE ioctl call returning a EOPNOTSUPP in > hw/virtio/vhost-vdpa.c:vhost_vdpa_set_dev_vring_base() [4]. > > I spent some time going through the ioctl's implementation in Linux. > I used ftrace to trace the functions that were being called in the kernel. > With x-svq=on (regardless of whether split virtqueues are used or packed > virtqueues), I got the following trace: > > [...] > qemu-system-x86-1737 [001] ...1. 3613.371358: > vhost_vdpa_unlocked_ioctl <-__x64_sys_ioctl > qemu-system-x86-1737 [001] ...1. 3613.371358: vhost_vring_ioctl > <-vhost_vdpa_unlocked_ioctl > qemu-system-x86-1737 [001] ...1. 3613.371362: > vp_vdpa_set_vq_state <-vhost_vdpa_unlocked_ioctl > [...] > > There are 3 virtqueues that the vdpa device offers in L1. There were no > issues when using split virtqueues and the trace shown above appears > 3 times. With packed virtqueues, the first call to VHOST_SET_VRING_BASE > fails because drivers/vdpa/virtio_pci/vp_vdpa.c:vp_vdpa_set_vq_state_packed > [5] returns EOPNOTSUPP. > > The payload that VHOST_SET_VRING_BASE accepts depends on whether > split virtqueues or packed virtqueues are used [6]. In hw/virtio/vhost- > vdpa.c:vhost_vdpa_svq_setup() [7], the following payload is used which is > not suitable for packed virtqueues: > > struct vhost_vring_state s = { > .index = vq_index, > }; > > Based on the implementation in the linux kernel, the payload needs to > be as shown below for the ioctl to succeed for packed virtqueues: > > struct vhost_vring_state s = { > .index = vq_index, > .num = 0x80008000, > }; >
Wow, that's a great analysis, very good catch! > After making these changes, it looks like QEMU is able to set up the > virtqueues > and shadow virtqueues are enabled as well. > > Unfortunately, before the L2 VM can finish booting the kernel crashes. > The reason is that even though packed virtqueues are to be used, the > kernel tries to run > drivers/virtio/virtio_ring.c:virtqueue_get_buf_ctx_split() [8] > (instead of virtqueue_get_buf_ctx_packed) and throws an "invalid vring > head" error. I am still investigating this issue. > That's interesting. It's been a while since I haven't tested that code, maybe you also discovered a regression here :). > I'll send an update once I resolve this issue. I'll also send a patch that > crafts the payload correctly based on the format of the virtqueue in > vhost_vdpa_svq_setup(). > The QEMU's vhost_vdpa_svq_setup is a valid patch so if you have the bandwith please send it ASAP and we move it forward :). Thanks! > Thanks, > Sahil > > [1] https://gitlab.com/qemu-project/qemu/-/blob/master/hw/virtio/vhost-vdpa.c > [2] https://github.com/torvalds/linux/blob/master/drivers/vhost/vdpa.c > [3] https://github.com/torvalds/linux/blob/master/drivers/vhost/vhost.c > [4] > https://gitlab.com/qemu-project/qemu/-/blob/master/hw/virtio/vhost-vdpa.c#L1002 > [5] > https://github.com/torvalds/linux/blob/master/drivers/vdpa/virtio_pci/vp_vdpa.c#L278 > [6] > https://qemu-project.gitlab.io/qemu/interop/vhost-user.html#front-end-message-types > [7] > https://gitlab.com/qemu-project/qemu/-/blob/master/hw/virtio/vhost-vdpa.c#L1223 > [8] > https://github.com/torvalds/linux/blob/master/drivers/virtio/virtio_ring.c#L823 >