On Fri, Jul 26, 2024 at 11:58 AM Sahil Siddiq <icegambi...@gmail.com> wrote: > > Hi, > > I have made some progress in this project and thought I would > send these changes first before continuing. I split patch v1 [1] > into two commits (#1 and #2) to make it easy to review. There are > very few changes in the first commit. The second commit has not > changes. > > There are a few things that I am not entirely sure of in commit #3. > > Q1. > In virtio_ring.h [2], new aliases with memory alignment enforcement > such as "vring_desc_t" have been created. I am not sure if this > is required for the packed vq descriptor ring (vring_packed_desc) > as well. I don't see a type alias that enforces memory alignment > for "vring_packed_desc" in the linux kernel. I haven't used any > alias either. >
The alignment is required to be 16 for the descriptor ring and 4 for the device and driver ares by the standard [1]. In QEMU, this is solved by calling mmap, which always returns page-aligned addresses. > Q2. > I see that parts of the "vhost-vdpa" implementation is based on > the assumption that SVQ uses the split vq format. For example, > "vhost_vdpa_svq_map_rings" [3], calls "vhost_svq_device_area_size" > which is specific to split vqs. The "vhost_vring_addr" [4] struct > is also specific to split vqs. > > My idea is to have a generic "vhost_vring_addr" structure that > wraps around split and packed vq specific structures, rather > than using them directly in if-else conditions wherever the > vhost-vdpa functions require their usage. However, this will > involve checking their impact in several other places where this > struct is currently being used (eg.: "vhost-user", "vhost-backend", > "libvhost-user"). > Ok I've just found this is under-documented actually :). As you mention, vhost-user is already using this same struct for packed vqs [2], just translating the driver area from the avail vring and the device area from the used vring. So the best option is to stick with that, unless I'm missing something. > Is this approach alright or is there a better alternative? I would > like to get your thoughts on this before working on this portion of > the project. > > Thanks, > Sahil > [1] https://docs.oasis-open.org/virtio/virtio/v1.3/virtio-v1.3.html [2] https://github.com/DPDK/dpdk/blob/82c47f005b9a0a1e3a649664b7713443d18abe43/lib/vhost/vhost_user.c#L841C1-L841C25
