Re: [RFC v3 3/3] vhost: Allocate memory for packed vring

[Sahil Siddiq <icegambit91@gmail.com>]

Hi,

Thank you for your reply.

On 10/28/24 1:40 PM, Eugenio Perez Martin wrote:
> On Mon, Oct 28, 2024 at 6:38 AM Sahil Siddiq <icegambit91@gmail.com> wrote:
>> [...]
>> 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!
> 
> [...]
>> 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 :).
> 

Sure thing. I'll do that while debugging the kernel in parallel.

Thanks,
Sahil