Re: Intention to work on GSoC project

[Sahil <icegambit91@gmail.com>]

Hi,

On Wednesday, May 8, 2024 8:53:12 AM GMT+5:30 Sahil wrote:
> Hi,
> 
> On Tuesday, May 7, 2024 12:44:33 PM IST Eugenio Perez Martin wrote:
> > [...]
> > 
> > > Shall I start by implementing a mechanism to check if the feature bit
> > > "VIRTIO_F_RING_PACKED" is set (using "virtio_vdev_has_feature")? And
> > > if it's supported, "vhost_svq_add" should call "vhost_svq_add_packed".
> > > Following this, I can then start implementing "vhost_svq_add_packed"
> > > and progress from there.
> > > 
> > > What are your thoughts on this?
> > 
> > Yes, that's totally right.
> > 
> > I recommend you to also disable _F_EVENT_IDX to start, so the first
> > version is easier.
> > 
> > Also, you can send as many incomplete RFCs as you want. For example,
> > you can send a first version that only implements reading of the guest
> > avail ring, so we know we're aligned on that. Then, we can send
> > subsequents RFCs adding features on top.
>

I have started working on implementing packed virtqueue support in
vhost-shadow-virtqueue.c. The changes I have made so far are very
minimal. I have one confusion as well.

In "vhost_svq_add()" [1], a structure of type "VhostShadowVirtqueue"
is being used. My initial idea was to create a whole new structure (eg:
VhostShadowVirtqueuePacked). But I realized that "VhostShadowVirtqueue"
is being used in a lot of other places such as in "struct vhost_vdpa" [2]
(in "vhost-vdpa.h"). So maybe this isn't a good idea.

The problem is that "VhostShadowVirtqueue" has a member of type "struct
vring" [3] which represents a split virtqueue [4]. My idea now is to instead
wrap this member in a union so that the struct would look something like
this.

struct VhostShadowVirtqueue {
        union {
                struct vring vring;
                struct packed_vring vring;
        }
        ...
}

I am not entirely sure if this is a good idea. It is similar to what's been done
in linux's "drivers/virtio/virtio_ring.c" ("struct vring_virtqueue" [5]).

I thought I would ask this first before continuing further.

Thanks,
Sahil

[1] https://gitlab.com/qemu-project/qemu/-/blob/master/hw/virtio/vhost-shadow-virtqueue.c#L249
[2] https://gitlab.com/qemu-project/qemu/-/blob/master/include/hw/virtio/vhost-vdpa.h#L69
[3] https://gitlab.com/qemu-project/qemu/-/blob/master/hw/virtio/vhost-shadow-virtqueue.h#L52
[4] https://gitlab.com/qemu-project/qemu/-/blob/master/include/standard-headers/linux/virtio_ring.h#L156
[5] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/virtio/virtio_ring.c#n199