On Wed, Mar 28, 2018 at 08:23:38AM +0000, Lars Ganrot wrote:
> 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 <[email protected]>
> > Reviewed-by: Cornelia Huck <[email protected]>
> > Reviewed-by: Stefan Hajnoczi <[email protected]>
> > ---
> [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

virtq_avail.idx is still an index into the available ring.

I don't really see how you can use virtq_avail.idx to guess the
placement of a descriptor.

I suspect the best way to optimize this is to include the
relevant data with the VIRTIO_F_NOTIFICATION_DATA feature.


> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to