Re: [RFC 1/8] virtio: Define InOrderVQElement
On Mon, Mar 25, 2024 at 6:08 PM Jonah Palmer wrote: > > > > On 3/22/24 5:45 AM, Eugenio Perez Martin wrote: > > On Thu, Mar 21, 2024 at 4:57 PM Jonah Palmer > > 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 > >> --- > >> 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 0x > >> -- > >> 2.39.3 > >> > > >
Re: [RFC 1/8] virtio: Define InOrderVQElement
On 3/22/24 5:45 AM, Eugenio Perez Martin wrote: On Thu, Mar 21, 2024 at 4:57 PM Jonah Palmer 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 --- 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. +} InOrderVQElement; + #define VIRTIO_QUEUE_MAX 1024 #define VIRTIO_NO_VECTOR 0x -- 2.39.3
Re: [RFC 1/8] virtio: Define InOrderVQElement
On Thu, Mar 21, 2024 at 4:57 PM Jonah Palmer 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 > --- > 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? > +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) ? > +} InOrderVQElement; > + > #define VIRTIO_QUEUE_MAX 1024 > > #define VIRTIO_NO_VECTOR 0x > -- > 2.39.3 >
[RFC 1/8] virtio: Define InOrderVQElement
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 --- 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; +unsigned int len; +unsigned int idx; +unsigned int count; +} InOrderVQElement; + #define VIRTIO_QUEUE_MAX 1024 #define VIRTIO_NO_VECTOR 0x -- 2.39.3