[virtio-dev] virtio-snd spec question

[virtio-dev] virtio-snd spec question

[Roman Kiryanov]

[-- Attachment #1: Type: text/plain, Size: 969 bytes --]

Hello,

I work in Android Studio Emulator and I am currently implementing a
virtio-snd device. We found a spec draft here:

https://github.com/oasis-tcs/virtio-spec/commit/e73c8cdf3e822fd83c26c6de964a947670f93cc3#diff-73045e70aeaf45f93087610437b705e2d320c82a9d29b4027721f5f5f3918dc5

It mentions four virtqueues: ctl, event, rx and tx. It is not very clear
where a virtio-snd device should put responses to the ctl requests from the
linux kernel driver. There is a kernel driver implementation and we have a
virtio-snd device implemented in another emulator, it uses the same
virtqueue (ctl) to put ctl responses and the current kernel driver seems
happy with this.

Do you know if this is expected behavior? I am far from an expert here, but
I believe the device and the kernel will race here by reading from the same
virtqueue: the device could read VirtQueueElement produced by itself before
the kernel if the kernel is not fast enough.

Thank you.

Regards,
Roman.

[-- Attachment #2: Type: text/html, Size: 1323 bytes --]

Re: [virtio-dev] virtio-snd spec question

[Michael S. Tsirkin]

On Tue, Feb 01, 2022 at 03:57:10PM -0800, Roman Kiryanov wrote:
> Hello,
> 
> I work in Android Studio Emulator and I am currently implementing a virtio-snd
> device. We found a spec draft here:
> 
> https://github.com/oasis-tcs/virtio-spec/commit/
> e73c8cdf3e822fd83c26c6de964a947670f93cc3#
> diff-73045e70aeaf45f93087610437b705e2d320c82a9d29b4027721f5f5f3918dc5
> 
> It mentions four virtqueues: ctl, event, rx and tx. It is not very clear where
> a virtio-snd device should put responses to the ctl requests from the linux
> kernel driver. There is a kernel driver implementation and we have a
> virtio-snd device implemented in another emulator, it uses the same virtqueue
> (ctl) to put ctl responses and the current kernel driver seems happy with this.
> 
> Do you know if this is expected behavior? I am far from an expert here, but I
> believe the device and the kernel will race here by reading from the same
> virtqueue: the device could read VirtQueueElement produced by itself before the
> kernel if the kernel is not fast enough.
> 
> Thank you.
> 
> Regards,
> Roman.

Could you explain a bit more? which function in the kernel driver are
you referring to?



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

Re: [virtio-dev] virtio-snd spec question

[Roman Kiryanov]

[-- Attachment #1: Type: text/plain, Size: 1740 bytes --]

Sure. This is how I expect the device side ctl listening loop should look
like:

while (...) {
    request = virtqueue_pop(ctl_vq, ...);  //1
        response = virtqueue_pop(ctl_vq, ...);  //2
        virtqueue_push(ctl_vq, response, response_size):  //3
    virtqueue_push(ctl_vq, request, 0):  //4
}

My understanding is that line 1 could read output from line 3 if the kernel
is not fast enough fetching the response.

On Tue, Feb 1, 2022 at 11:44 PM Michael S. Tsirkin <mst@redhat.com> wrote:

> On Tue, Feb 01, 2022 at 03:57:10PM -0800, Roman Kiryanov wrote:
> > Hello,
> >
> > I work in Android Studio Emulator and I am currently implementing a
> virtio-snd
> > device. We found a spec draft here:
> >
> > https://github.com/oasis-tcs/virtio-spec/commit/
> > e73c8cdf3e822fd83c26c6de964a947670f93cc3#
> > diff-73045e70aeaf45f93087610437b705e2d320c82a9d29b4027721f5f5f3918dc5
> >
> > It mentions four virtqueues: ctl, event, rx and tx. It is not very clear
> where
> > a virtio-snd device should put responses to the ctl requests from the
> linux
> > kernel driver. There is a kernel driver implementation and we have a
> > virtio-snd device implemented in another emulator, it uses the same
> virtqueue
> > (ctl) to put ctl responses and the current kernel driver seems happy
> with this.
> >
> > Do you know if this is expected behavior? I am far from an expert here,
> but I
> > believe the device and the kernel will race here by reading from the same
> > virtqueue: the device could read VirtQueueElement produced by itself
> before the
> > kernel if the kernel is not fast enough.
> >
> > Thank you.
> >
> > Regards,
> > Roman.
>
> Could you explain a bit more? which function in the kernel driver are
> you referring to?
>
>
>

[-- Attachment #2: Type: text/html, Size: 2350 bytes --]

Re: [virtio-dev] virtio-snd spec question

[Michael S. Tsirkin]

On Wed, Feb 02, 2022 at 10:43:38AM -0800, Roman Kiryanov wrote:
> Sure. This is how I expect the device side ctl listening loop should look like:
> 
> while (...) {
>     request = virtqueue_pop(ctl_vq, ...);  //1
>         response = virtqueue_pop(ctl_vq, ...);  //2
>         virtqueue_push(ctl_vq, response, response_size):  //3
>     virtqueue_push(ctl_vq, request, 0):  //4
> }
> 
> My understanding is that line 1 could read output from line 3 if the kernel is
> not fast enough fetching the response.

Is this code from qemu then? In that case I don't see how. virtqueue_pop gets
an available buffer. virtqueue_push adds a used buffer, not an available
one. I think it's just the API that's confusing.

> On Tue, Feb 1, 2022 at 11:44 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> 
>     On Tue, Feb 01, 2022 at 03:57:10PM -0800, Roman Kiryanov wrote:
>     > Hello,
>     >
>     > I work in Android Studio Emulator and I am currently implementing a
>     virtio-snd
>     > device. We found a spec draft here:
>     >
>     > https://github.com/oasis-tcs/virtio-spec/commit/
>     > e73c8cdf3e822fd83c26c6de964a947670f93cc3#
>     > diff-73045e70aeaf45f93087610437b705e2d320c82a9d29b4027721f5f5f3918dc5
>     >
>     > It mentions four virtqueues: ctl, event, rx and tx. It is not very clear
>     where
>     > a virtio-snd device should put responses to the ctl requests from the
>     linux
>     > kernel driver. There is a kernel driver implementation and we have a
>     > virtio-snd device implemented in another emulator, it uses the same
>     virtqueue
>     > (ctl) to put ctl responses and the current kernel driver seems happy with
>     this.
>     >
>     > Do you know if this is expected behavior? I am far from an expert here,
>     but I
>     > believe the device and the kernel will race here by reading from the same
>     > virtqueue: the device could read VirtQueueElement produced by itself
>     before the
>     > kernel if the kernel is not fast enough.
>     >
>     > Thank you.
>     >
>     > Regards,
>     > Roman.
> 
>     Could you explain a bit more? which function in the kernel driver are
>     you referring to?
> 
> 
> 


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

Re: [virtio-dev] virtio-snd spec question

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]

On Tue, Feb 01, 2022 at 03:57:10PM -0800, Roman Kiryanov wrote:
> Hello,
> 
> I work in Android Studio Emulator and I am currently implementing a
> virtio-snd device. We found a spec draft here:
> 
> https://github.com/oasis-tcs/virtio-spec/commit/e73c8cdf3e822fd83c26c6de964a947670f93cc3#diff-73045e70aeaf45f93087610437b705e2d320c82a9d29b4027721f5f5f3918dc5
> 
> It mentions four virtqueues: ctl, event, rx and tx. It is not very clear
> where a virtio-snd device should put responses to the ctl requests from the
> linux kernel driver. There is a kernel driver implementation and we have a
> virtio-snd device implemented in another emulator, it uses the same
> virtqueue (ctl) to put ctl responses and the current kernel driver seems
> happy with this.
> 
> Do you know if this is expected behavior? I am far from an expert here, but
> I believe the device and the kernel will race here by reading from the same
> virtqueue: the device could read VirtQueueElement produced by itself before
> the kernel if the kernel is not fast enough.

CCing Anton and Shreyansh.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [virtio-dev] virtio-snd spec question

[Roman Kiryanov]

[-- Attachment #1: Type: text/plain, Size: 3222 bytes --]

On Sun, Feb 6, 2022 at 9:41 PM Shreyansh Chouhan <
chouhan.shreyansh2702@gmail.com> wrote:

> Hi,
>
> On Thu, 3 Feb 2022 at 16:06, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Tue, Feb 01, 2022 at 03:57:10PM -0800, Roman Kiryanov wrote:
> > > Hello,
> > >
> > > I work in Android Studio Emulator and I am currently implementing a
> > > virtio-snd device. We found a spec draft here:
> > >
> > >
> https://github.com/oasis-tcs/virtio-spec/commit/e73c8cdf3e822fd83c26c6de964a947670f93cc3#diff-73045e70aeaf45f93087610437b705e2d320c82a9d29b4027721f5f5f3918dc5
> > >
> > > It mentions four virtqueues: ctl, event, rx and tx. It is not very
> clear
> > > where a virtio-snd device should put responses to the ctl requests
> from the
> > > linux kernel driver. There is a kernel driver implementation and we
> have a
> > > virtio-snd device implemented in another emulator, it uses the same
> > > virtqueue (ctl) to put ctl responses and the current kernel driver
> seems
> > > happy with this.
> > >
> > > Do you know if this is expected behavior? I am far from an expert
> here, but
> > > I believe the device and the kernel will race here by reading from the
> same
> > > virtqueue: the device could read VirtQueueElement produced by itself
> before
> > > the kernel if the kernel is not fast enough.
>
> I think that is the intended behavior. Each virtqueue is supposed to have
> device and driver writable areas. (Separate used ring and avail ring
> in case of split
> virtqueues, and in case of packed virtqueues we have something similar
> but within a single circular buffer.)
>
> I don't think there will be any races. In case of split virtqueues we
> have separate
> avail and used rings and device and the driver look for buffers to consume
> from
> these different rings. In case of packed virtqueues we have flags and
> a wrap counting
> bit and we use these to make sure we don't read anything not intended for
> us.
>
> You can check these explanations out for more details, these are what I
> used
> when I started the sound card implementation.
>
> Split Virtqueue:
> https://www.redhat.com/en/blog/virtqueues-and-virtio-ring-how-data-travels
> Packed Virtqueue:
> https://www.redhat.com/en/blog/packed-virtqueue-how-reduce-overhead-virtio


Hi,
thank you all for looking into this. This helped me to make some progress.
My device is able to play audio, sometimes clean and sometimes I see the
guest issues a lot of PCM_PREPARE/PCM_START/PCM_STOP/PCM_RELEASE requests
for no reason. Maybe it is not happy with my replies in the tx virtqueue.

Do you happen to have a more recent patch to the spec for virtio-snd? The
patch (
https://github.com/oasis-tcs/virtio-spec/commit/e73c8cdf3e822fd83c26c6de964a947670f93cc3
) says "The driver MUST NOT place device-writable buffers into the tx
queue" while it seems expect for the device to write status messages there.
Also, it is never mentioned which status messages to write into the tx
virtqueue (I guess it is virtio_snd_pcm_status) and
the virtio_snd_pcm_status::latency_bytes explanation is not very elaborate.
I believe it is the amount of bytes buffered in the device waiting to go
to AUD_write.

Could you please clarify? Thank you.

Regards,
Roman.

[-- Attachment #2: Type: text/html, Size: 4487 bytes --]

Re: [virtio-dev] virtio-snd spec question

[Anton Yakovlev]

Hello Roman,

On 02.02.2022 00:57, Roman Kiryanov wrote:
> Hello,
> 
> I work in Android Studio Emulator and I am currently implementing a 
> virtio-snd device. We found a spec draft here:
> 
> https://github.com/oasis-tcs/virtio-spec/commit/e73c8cdf3e822fd83c26c6de964a947670f93cc3#diff-73045e70aeaf45f93087610437b705e2d320c82a9d29b4027721f5f5f3918dc5 
> <https://github.com/oasis-tcs/virtio-spec/commit/e73c8cdf3e822fd83c26c6de964a947670f93cc3#diff-73045e70aeaf45f93087610437b705e2d320c82a9d29b4027721f5f5f3918dc5>
> 
> It mentions four virtqueues: ctl, event, rx and tx. It is not very clear 
> where a virtio-snd device should put responses to the ctl requests from 
> the linux kernel driver. There is a kernel driver implementation and we 
> have a virtio-snd device implemented in another emulator, it uses the 
> same virtqueue (ctl) to put ctl responses and the current kernel driver 
> seems happy with this.
> 
> Do you know if this is expected behavior? I am far from an expert here, 
> but I believe the device and the kernel will race here by reading from 
> the same virtqueue: the device could read VirtQueueElement produced by 
> itself before the kernel if the kernel is not fast enough.

Yes, this is the expected behavior of the device and you are doing
everything right.

The virtio specification describes kind of single-producer single-
consumer lockless queue way to work with virtqueues. And if both the
device and the driver (in this case, the Linux kernel) follow the
specification, then you don't have to worry about a possible race
condition.

-- 
Anton Yakovlev
Senior Software Engineer

OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin

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

Re: [virtio-dev] virtio-snd spec question

[Roman Kiryanov]

On Sun, Feb 6, 2022 at 7:23 AM Anton Yakovlev
<anton.yakovlev@opensynergy.com> wrote:
>
> Hello Roman,
>
> On 02.02.2022 00:57, Roman Kiryanov wrote:
> > Hello,
> >
> > I work in Android Studio Emulator and I am currently implementing a
> > virtio-snd device. We found a spec draft here:
> >
> > https://github.com/oasis-tcs/virtio-spec/commit/e73c8cdf3e822fd83c26c6de964a947670f93cc3#diff-73045e70aeaf45f93087610437b705e2d320c82a9d29b4027721f5f5f3918dc5
> > <https://github.com/oasis-tcs/virtio-spec/commit/e73c8cdf3e822fd83c26c6de964a947670f93cc3#diff-73045e70aeaf45f93087610437b705e2d320c82a9d29b4027721f5f5f3918dc5>
> >
> > It mentions four virtqueues: ctl, event, rx and tx. It is not very clear
> > where a virtio-snd device should put responses to the ctl requests from
> > the linux kernel driver. There is a kernel driver implementation and we
> > have a virtio-snd device implemented in another emulator, it uses the
> > same virtqueue (ctl) to put ctl responses and the current kernel driver
> > seems happy with this.
> >
> > Do you know if this is expected behavior? I am far from an expert here,
> > but I believe the device and the kernel will race here by reading from
> > the same virtqueue: the device could read VirtQueueElement produced by
> > itself before the kernel if the kernel is not fast enough.
>
> Yes, this is the expected behavior of the device and you are doing
> everything right.
>
> The virtio specification describes kind of single-producer single-
> consumer lockless queue way to work with virtqueues. And if both the
> device and the driver (in this case, the Linux kernel) follow the
> specification, then you don't have to worry about a possible race
> condition.

Hello Anton. Thank you for looking into this. I was able to make the
ctl vq working.

Could you please help with the tx vq? The patch (
https://github.com/oasis-tcs/virtio-spec/commit/e73c8cdf3e822fd83c26c6de964a947670f93cc3
) says "The driver MUST NOT place device-writable buffers into the tx
queue" while it seems to expect for the device to write status
messages there. Also, it is never mentioned which status messages to
write into the tx virtqueue (I guess it is virtio_snd_pcm_status) and
the virtio_snd_pcm_status::latency_bytes explanation is not very
elaborate. I believe it is the amount of bytes buffered in the device
waiting to go to AUD_write.

Also, it is not very clear if the device is expected to process tx
requests right away or to keep them in the vq until they are
completely sent to AUD_write. When I tried to do the former I noticed
the driver keeps sending me tx requests indefinitely, when I tried to
do the latter, the driver often goes to the XRUN state.

Thank you.

Regards,
Roman.

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