On Mon, Mar 25, 2024 at 6:08 PM Jonah Palmer <jonah.pal...@oracle.com> wrote: > > > > On 3/22/24 5:45 AM, Eugenio Perez Martin wrote: > > On Thu, Mar 21, 2024 at 4:57 PM Jonah Palmer <jonah.pal...@oracle.com> > > wrote: > >> > >> Define the InOrderVQElement structure for the VIRTIO_F_IN_ORDER > >> transport feature implementation. > >> > >> The InOrderVQElement structure is used to encapsulate out-of-order > >> VirtQueueElement data that was processed by the host. This data > >> includes: > >> - The processed VirtQueueElement (elem) > >> - Length of data (len) > >> - VirtQueueElement array index (idx) > >> - Number of processed VirtQueueElements (count) > >> > >> InOrderVQElements will be stored in a buffering mechanism until an > >> order can be achieved. > >> > >> Signed-off-by: Jonah Palmer <jonah.pal...@oracle.com> > >> --- > >> include/hw/virtio/virtio.h | 7 +++++++ > >> 1 file changed, 7 insertions(+) > >> > >> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > >> index b3c74a1bca..c8aa435a5e 100644 > >> --- a/include/hw/virtio/virtio.h > >> +++ b/include/hw/virtio/virtio.h > >> @@ -77,6 +77,13 @@ typedef struct VirtQueueElement > >> struct iovec *out_sg; > >> } VirtQueueElement; > >> > >> +typedef struct InOrderVQElement { > >> + const VirtQueueElement *elem; > > > > Some subsystems allocate space for extra elements after > > VirtQueueElement, like VirtIOBlockReq. You can request virtqueue_pop > > to allocate this extra space by its second argument. Would it work for > > this? > > > > I don't see why not. Although this may not be necessary due to me > missing a key aspect mentioned in your comment below. > > >> + unsigned int len; > >> + unsigned int idx; > >> + unsigned int count; > > > > Now I don't get why these fields cannot be obtained from elem->(len, > > index, ndescs) ? > > > > Interesting. I didn't realize that these values are equivalent to a > VirtQueueElement's len, index, and ndescs fields. > > Is this always true? Else I would've expected, for example, > virtqueue_push to not need the 'unsigned int len' parameter if this > information is already included via. the VirtQueueElement being passed in. >
The code uses "len" to store the written length values of each used descriptor between virtqueue_fill and virtqueue_flush. But not all devices use these separately, only the ones that batches: virtio-net and SVQ. A smarter / less simpler implementation of virtqueue_push could certainly avoid storing elem->len. But the performance gain is probably tiny, and the code complexity grows. > >> +} InOrderVQElement; > >> + > >> #define VIRTIO_QUEUE_MAX 1024 > >> > >> #define VIRTIO_NO_VECTOR 0xffff > >> -- > >> 2.39.3 > >> > > >