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

[Sahil Siddiq <icegambit91@gmail.com>]

Hi,

On 11/13/24 5:00 PM, Eugenio Perez Martin wrote:
> On Wed, Nov 13, 2024 at 6:11 AM Sahil Siddiq <icegambit91@gmail.com> wrote:
>>
>> Hi,
>>
>> On 10/28/24 11:07 AM, Sahil Siddiq wrote:
>>> [...]
>>> 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,
>>> };
>>>
>>> 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.
>>
>> I made a mistake here. "virtqueue_get_buf_ctx_packed" [1] in the linux
>> kernel also throws the same error. I think the issue might be because
>> hw/virtio/vhost-vdpa.c:vhost_vdpa_svq_map_rings [2] does not handle
>> mapping packed virtqueues at the moment.
>>
>> Probably because of this, vq->packed.desc_state[id].data [1] is NULL in the
>> kernel.
>>
>> Regarding one of the earlier reviews in the same thread [3]:
>>
> 
> I think it is a good first step, yes. Looking forward to your findings!

I faced a few more issues while testing my changes. Instead of explaining
the issues here, I thought I would explain them in a new thread along with
the next version of the patch series in case that helps with troubleshooting.

Thanks,
Sahil