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
