OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]

Subject: Re: [virtio-dev] [PATCH] snd: Add virtio sound device specification

On 12.11.2019 15:20, Liam Girdwood wrote:
On Tue, 2019-11-12 at 12:09 +0100, Jean-Philippe Brucker wrote:

On Mon, Nov 11, 2019 at 03:20:55PM +0000, Liam Girdwood wrote:
+struct virtio_snd_ctl_msg {
+    /* device-read-only data */
+    le32 request_code;
+    u8 request_payload[];

How do I know how much data to read here ? Is size embedded in
request_code ?

Yes, you are right. Actual request/response length can be easily
derived from
a request_code.

Nak, please use a separate size and type code otherwise we will
real problems in the future when we come to add new features (with
codes or bigger sizes).

Other virtio devices rely on the buffer length already provided by
for this. For example see the virtio-net control virtqueue
(virtio_net_ctrl*), which is extended with new commands from time to
Most recently, the "receive-side scaling" feature introduces a large
structure as payload to virtio_net_ctrl.

Sorry, not following, what does this have to do with combining type and
size into request_code ? Or are you saying the size is encoded
elsewhere in the virtio transport data ?

A virtqueue operates with sg-lists, where every entry has additional property specifying whether it refers to RO or WO memory. Thus, as with any sg-list, it's possible to calculate its buffer size.

+struct virtio_snd_pcm_xfer {
+    le32 stream;
+    u8 data[];
+    le32 status;
+    le32 actual_length;

Not following this, is actual_length the size of data[]. If so,
preceed data[].

No, the actual_length field is supposed to be used by device side
actual amount of bytes read from/written to a buffer.

By the way the written size is already provided by virtio (field len
the used descriptor), but not the read size.

In real world
if an I/O request contains N bytes, a device can play/capture *up
N bytes.
Thus, it's required to report this length back to a driver.

This is not how PCM audio buffering works. I don't copy extra
if I dont need to, I only copy when my buffer is full. See the need
period size in earlier comments.

2) Zero copy of audio data and stream positions. Essential for
latency audio use cases (if supported by HW, hypervisor, VM

What do you mean by zero-copy? Shared memory between device and
driver sides?

Yes, but I see this is now a separate thread as it used by others

The virtio transport already permits zero-copy, because one buffer
can be
described with a chain of descriptors, each pointing to a different
in memory. The driver can split the virtio_snd_pcm_xfer into header

     desc ring
     |       |
     |       |---------> header (copied)
     +- - - -+
     |       |---------> PCM data (mapped)
     |       |

So it might be a good idea to put all the metadata (stream, status,
actual_length) at the beginning of the virtio_snd_pcm_xfer struct,
otherwise the driver will need three descriptors instead of two to

This would be a good improvement, it's less copying and would likely
improve user experience, however the buffer ptr still suffers latency
as it's queued (same for stream positions going the other way).

It will violate virtio spec.

<quote> Driver Requirements: Message Framing

The driver MUST place any device-writable descriptor elements after any device-readable descriptor elements.


2.6.5 The Virtqueue Descriptor Table

... Each descriptor describes a buffer which is read-only for the device (âdevice-readableâ) or write-only for the device (âdevice-writableâ), ...

It means, that a device can "map" this memory as RO or WO and will be right. Thus, you can not put in one descriptor both readable and writable fields. You can take a look into virtio-blk spec. There is a reason, why they put status field at the very end of a request.

Anton Yakovlev
Senior Software Engineer

OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]