Virtualization - Sep 2023 - virtio-sound linux driver conformance to spec

Hello,

This email is to report a behavior of the Linux virtio-sound driver that
looks like it is not conforming to the VirtIO specification. The kernel
driver is moving buffers from the used ring to the available ring
without knowing if the content has been updated from the user. If the
device picks up buffers from the available ring just after it is
notified, it happens that the content is old. This problem can be fixed
by waiting a period of time after the device dequeues a buffer from the
available ring. The driver should not be allowed to change the content
of a buffer in the available ring. When buffers are enqueued in the
available ring, the device can consume them immediately.

1. Is the actual driver implementation following the spec?  
2. Shall the spec be extended to support such a use-case?

Thanks, Matias

On Wed, Sep 13, 2023 at 5:05?PM Matias Ezequiel Vara Larsen
<mvaralar at redhat.com> wrote:
>
> Hello,
>
> This email is to report a behavior of the Linux virtio-sound driver that
> looks like it is not conforming to the VirtIO specification. The kernel
> driver is moving buffers from the used ring to the available ring
> without knowing if the content has been updated from the user. If the
> device picks up buffers from the available ring just after it is
> notified, it happens that the content is old. This problem can be fixed
> by waiting a period of time after the device dequeues a buffer from the
> available ring. The driver should not be allowed to change the content
> of a buffer in the available ring. When buffers are enqueued in the
> available ring, the device can consume them immediately.
>
> 1. Is the actual driver implementation following the spec?
If I understand the issue correctly, it's not. As you say, absent any
clarification a buffer that has been placed in the available ring
should be unmodifiable; the driver should operate as if the data in
the available ring is copied to an internal buffer instantly when the
buffer id is added to the ring.

I am assuming this is a playback buffer. To clarify, does the driver
expect buffers to be read only as needed, which is a fraction of a
second before the data is played back?
> 2. Shall the spec be extended to support such a use-case?
If it places N buffers, I think it's a reasonable expectation (for the
sound device only!) that the Nth isn't read until the (N-1)th has
started playing. With two buffers only, the behavior you specify would
not be permissible (because as soon as buffer 1 starts playing, the
device can read buffer 2; there is never an idle buffer). With three
buffers, you could write buffer 3 while buffer 1 plays; write buffer 1
while buffer 2 plays; and write buffer 2 while buffer 3 plays. Is this
enough?

That said, being reasonable isn't enough for virtio-sound to do it and
deviate from other virtio devices. Why does the Linux driver behave
like this? Is it somehow constrained by the kernel->userspace APIs?

Paolo

Hello Matias,

On 14.09.2023 00:04, Matias Ezequiel Vara Larsen wrote:
> Hello,
> 
> This email is to report a behavior of the Linux virtio-sound driver that
> looks like it is not conforming to the VirtIO specification. The kernel
> driver is moving buffers from the used ring to the available ring
> without knowing if the content has been updated from the user. If the
> device picks up buffers from the available ring just after it is
> notified, it happens that the content is old. This problem can be fixed
> by waiting a period of time after the device dequeues a buffer from the
> available ring. The driver should not be allowed to change the content
> of a buffer in the available ring. When buffers are enqueued in the
> available ring, the device can consume them immediately.
> 
> 1. Is the actual driver implementation following the spec?
If the Linux virtio sound driver violates a specification, then there must be
a conformance statement that the driver does not follow. As far as I know,
there is no such thing at the moment.

During the design discussion, it was decided that the driver should implement
typical cyclic DMA logic. And the main idea behind was that, unlike many other
virtio devices, the sound device is isochronous. It consumes or supplies data
at a fixed rate. Which means that the device (in an idealized case) should
access buffers in virtqueues not as soon as they are available, but when
required. Whether this is a good idea or not is a debatable question.

> 2. Shall the spec be extended to support such a use-case?
There are some things you can try without having to change the spec.

The driver now handles RW and MMAP substream access modes in the same way.
Someone could try to change the handling of RW mode exactly as you describe,
because the UAPI allows it. But this logic cannot be reliably applied to MMAP
mode.


Best regards,
-- 
Anton Yakovlev
Senior Software Engineer

OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin

On Wed, Sep 13, 2023 at 05:04:24PM +0200, Matias Ezequiel Vara Larsen
wrote:
> Hello,
> 
> This email is to report a behavior of the Linux virtio-sound driver that
> looks like it is not conforming to the VirtIO specification. The kernel
> driver is moving buffers from the used ring to the available ring
> without knowing if the content has been updated from the user. If the
> device picks up buffers from the available ring just after it is
> notified, it happens that the content is old.
Then, what happens, exactly? Do things still work?
> This problem can be fixed
> by waiting a period of time after the device dequeues a buffer from the
> available ring. The driver should not be allowed to change the content
> of a buffer in the available ring. When buffers are enqueued in the
> available ring, the device can consume them immediately.
> 
> 1. Is the actual driver implementation following the spec?  
> 2. Shall the spec be extended to support such a use-case?
> 
> Thanks, Matias