Re: [virtio-comment] virtio-sound linux driver conformance to spec

[Matias Ezequiel Vara Larsen <mvaralar@redhat.com>]

On Mon, Sep 18, 2023 at 1:13âPM Matias Ezequiel Vara Larsen
<mvaralar@redhat.com> wrote:
>
> On Wed, Sep 13, 2023 at 05:50:48PM +0200, Paolo Bonzini wrote:
> > On Wed, Sep 13, 2023 at 5:05âPM Matias Ezequiel Vara Larsen
> > <mvaralar@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?
> >
> The driver expects that device to read buffers a fraction of a second
> before playing them back. If the device reads it just when they are
> exposed in the available ring, the content is old. The device has to
> read it just when the audio engine is going to consume it.
>
> > > 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?
> >
> AFAIU, the driver sends four requests before starting playing, e.g.,
> aplay 'FrontLeft.wav', each with PERIOD_SIZE bytes. PERIOD_SIZE is
> negotiated between the device and the driver before playback begins.
> The requests are split into multiple buffers.  After a PERIOD_SIZE is
> played, the device notifies the guest.  These buffers are part of a ring
> buffer shared with the user application.

I mean the user application in the guest.

> The device just picks the last
> used set of buffers and enqueues again in the available ring. For

In this sentence, I mean the driver, not the device.

Matias