RE: [virtio-dev] [PATCH v10 13/13] split-ring: in order feature

[Lars Ganrot <lga@napatech.com>]

Hi Michael et al

> Behalf Of Michael S. Tsirkin
> Sent: 9. marts 2018 22:24
> 
> For a split ring, require that drivers use descriptors in order too.
> This allows devices to skip reading the available ring.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
[snip]
> 
> +If VIRTIO_F_IN_ORDER has been negotiated, and when making a descriptor
> +with VRING_DESC_F_NEXT set in \field{flags} at offset $x$ in the table
> +available to the device, driver MUST set \field{next} to $0$ for the
> +last descriptor in the table (where $x = queue\_size - 1$) and to $x +
> +1$ for the rest of the descriptors.
> +
>  \subsubsection{Indirect Descriptors}\label{sec:Basic Facilities of a Virtio
> Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
> 
>  Some devices benefit by concurrently dispatching a large number @@ -247,6
> +257,10 @@ chained by \field{next}. An indirect descriptor without a valid
> \field{next}  A single indirect descriptor  table can include both device-
> readable and device-writable descriptors.
> 
> +If VIRTIO_F_IN_ORDER has been negotiated, indirect descriptors use
> +sequential indices, in-order: index 0 followed by index 1 followed by
> +index 2, etc.
> +
>  \drivernormative{\paragraph}{Indirect Descriptors}{Basic Facilities of a Virtio
> Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
> The driver MUST NOT set the VIRTQ_DESC_F_INDIRECT flag unless the
>  VIRTIO_F_INDIRECT_DESC feature was negotiated.   The driver MUST NOT
> @@ -259,6 +273,10 @@ the device.
>  A driver MUST NOT set both VIRTQ_DESC_F_INDIRECT and
> VIRTQ_DESC_F_NEXT  in \field{flags}.
> 
> +If VIRTIO_F_IN_ORDER has been negotiated, indirect descriptors MUST
> +appear sequentially, with \field{next} taking the value of 1 for the
> +1st descriptor, 2 for the 2nd one, etc.
> +
>  \devicenormative{\paragraph}{Indirect Descriptors}{Basic Facilities of a Virtio
> Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
> The device MUST ignore the write-only flag
> (\field{flags}\&VIRTQ_DESC_F_WRITE) in the descriptor that refers to an
> indirect table.
> 

The use of VIRTIO_F_IN_ORDER for split-ring can eliminate some accesses to the virtq_avail.ring and virtq_used.ring. However I'm wondering if the proposed descriptor ordering for multi-element buffers couldn't be tweaked to be more HW friendly.  Currently even with the VIRTIO_F_IN_ORDER negotiated, there is no way of knowing if, or how many chained descriptors follow the descriptor pointed to by the virtq_avail.idx. A chain has to be inspected one descriptor at a time until virtq_desc.flags[VIRTQ_DESC_F_NEXT]=0. This is awkward for HW offload, where you want to DMA all available descriptors in one shot, instead of iterating based on the contents of received DMA data. As currently defined, HW would have to find a compromise between likely chain length, and cost of additional DMA transfers. This leads to a performance penalty for all chained descriptors, and in case the length assumption is wrong the impact can be significant.

Now, what if the VIRTIO_F_IN_ORDER instead required chained buffers to place the last element at the lowest index, and the head-element (to which virtq_avail.idx points) at the highest index? Then all the chained element descriptors would be included in a DMA of the descriptor table from the previous virtq_avail.idx+1 to the current virtq_avail.idx. The "backward" order of the chained descriptors shouldn't pose an issue as such (at least not in HW).

Best Regards,

-Lars

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org