| Message ID | c94a9c1e65bb6bb43c58e5ccb982948424a3f3f2.1693252037.git.manos.pitsidianakis@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add VIRTIO sound card | expand |
Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org> writes: > Handle output IO messages in the transmit (TX) virtqueue. > > It allocates a VirtIOSoundPCMBlock for each IO message and copies the > data buffer to it. When the IO buffer is written to the host's sound > card, the guest will be notified that it has been consumed. > > The lifetime of an IO message is: > > 1. Guest sends IO message to TX virtqueue. > 2. QEMU adds it to the appropriate stream's IO buffer queue. > 3. Sometime later, the host audio backend calls the output callback, > virtio_snd_pcm_out_cb(), which is defined with an AUD_open_out() > call. The callback gets an available number of bytes the backend can > receive. Then it writes data from the IO buffer queue to the backend. > If at any time a buffer is exhausted, it is returned to the guest as > completed. > 4. If the guest releases the stream, its buffer queue is flushed by > attempting to write any leftover data to the audio backend and > releasing all IO messages back to the guest. This is how according to > the spec the guest knows the release was successful. > > Based-on: https://github.com/OpenSynergy/qemu/commit/5a2f350eec5d157b90d9c7b40a8e603f4da92471 > Signed-off-by: Igor Skalkin <Igor.Skalkin@opensynergy.com> > Signed-off-by: Anton Yakovlev <Anton.Yakovlev@opensynergy.com> > Signed-off-by: Emmanouil Pitsidianakis > <manos.pitsidianakis@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
On 28/8/23 21:55, Emmanouil Pitsidianakis wrote: > Handle output IO messages in the transmit (TX) virtqueue. > > It allocates a VirtIOSoundPCMBlock for each IO message and copies the > data buffer to it. When the IO buffer is written to the host's sound > card, the guest will be notified that it has been consumed. > > The lifetime of an IO message is: > > 1. Guest sends IO message to TX virtqueue. > 2. QEMU adds it to the appropriate stream's IO buffer queue. > 3. Sometime later, the host audio backend calls the output callback, > virtio_snd_pcm_out_cb(), which is defined with an AUD_open_out() > call. The callback gets an available number of bytes the backend can > receive. Then it writes data from the IO buffer queue to the backend. > If at any time a buffer is exhausted, it is returned to the guest as > completed. > 4. If the guest releases the stream, its buffer queue is flushed by > attempting to write any leftover data to the audio backend and > releasing all IO messages back to the guest. This is how according to > the spec the guest knows the release was successful. > > Based-on: https://github.com/OpenSynergy/qemu/commit/5a2f350eec5d157b90d9c7b40a8e603f4da92471 > Signed-off-by: Igor Skalkin <Igor.Skalkin@opensynergy.com> > Signed-off-by: Anton Yakovlev <Anton.Yakovlev@opensynergy.com> > Signed-off-by: Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org> > --- > hw/virtio/trace-events | 2 + > hw/virtio/virtio-snd.c | 262 ++++++++++++++++++++++++++++++++- > include/hw/virtio/virtio-snd.h | 11 ++ > 3 files changed, 271 insertions(+), 4 deletions(-) > /* > - * Handles VIRTIO_SND_R_PCM_RELEASE. Releases the buffer resources allocated to > - * a stream. > + * Returns the number of I/O messages that are being processed. > + * > + * @stream: VirtIOSoundPCMStream > + */ > +static size_t virtio_snd_pcm_get_pending_io_msgs(VirtIOSoundPCMStream *stream) > +{ > + VirtIOSoundPCMBlock *block; > + VirtIOSoundPCMBlock *next; > + size_t size = 0; > + > + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { > + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { > + size += 1; Can you add a comment explaining this magic size? > + } > + } > + return size; > +} > +/* > + * The tx virtqueue handler. Makes the buffers available to their respective > + * streams for consumption. > + * > + * @vdev: VirtIOSound device > + * @vq: tx virtqueue > + */ > +static void virtio_snd_handle_tx(VirtIODevice *vdev, VirtQueue *vq) > +{ > + VirtIOSound *s = VIRTIO_SND(vdev); > + VirtIOSoundPCMStream *stream = NULL; > + VirtQueueElement *elem; > + size_t sz; > + virtio_snd_pcm_xfer hdr; > + virtio_snd_pcm_status resp = { 0 }; virtio_snd_pcm_status has multiple fields, so better zero-initialize all of them with '{ }'. > + > + trace_virtio_snd_handle_xfer(); > + > + for (;;) { > + elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); > + if (!elem) { > + break; > + } > + /* get the message hdr object */ > + sz = iov_to_buf(elem->out_sg, > + elem->out_num, > + 0, > + &hdr, > + sizeof(hdr)); > + if (sz != sizeof(hdr) > + || hdr.stream_id >= s->snd_conf.streams > + || !s->pcm->streams[hdr.stream_id]) { > + goto tx_err; > + } > + > + stream = s->pcm->streams[hdr.stream_id]; > + if (stream->direction != VIRTIO_SND_D_OUTPUT) { > + goto tx_err; > + } > + > + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { > + virtio_snd_pcm_read_write(stream, > + vq, > + elem, > + hdr.stream_id == VIRTIO_SND_D_INPUT); > + > + resp.status = VIRTIO_SND_S_OK; > + iov_from_buf(elem->in_sg, > + elem->in_num, > + 0, > + &resp, > + sizeof(resp)); > + } > + continue; > + > +tx_err: > + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { > + resp.status = VIRTIO_SND_S_BAD_MSG; > + iov_from_buf(elem->in_sg, > + elem->in_num, > + 0, > + &resp, > + sizeof(resp)); > + } > + } > + > + /* > + * Notify vq about virtio_snd_pcm_status responses. > + * Buffer responses must be notified separately later. > + */ > + virtio_notify(VIRTIO_DEVICE(s), vq); > +} > +/* > + * AUD_* output callback. > + * > + * @data: VirtIOSoundPCMStream stream > + * @available: number of bytes that can be written with AUD_write() > + */ > +static void virtio_snd_pcm_out_cb(void *data, int available) > +{ > + VirtIOSoundPCMStream *stream = data; > + VirtIOSoundPCMBlock *block; > + VirtIOSoundPCMBlock *next; > + size_t size; > + > + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { > + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { > + for (;;) { > + size = MIN(block->size, available); > + size = AUD_write(stream->voice.out, > + block->data + block->offset, > + size); If AUD_write() returns 0, is this an infinite loop? > + block->size -= size; > + block->offset += size; > + if (!block->size) { > + virtqueue_push(block->vq, > + block->elem, > + sizeof(block->elem)); > + virtio_notify(VIRTIO_DEVICE(stream->s), > + block->vq); > + QSIMPLEQ_REMOVE_HEAD(&stream->queue, entry); > + g_free(block); > + available -= size; > + break; > + } > + > + available -= size; > + if (!available) { > + break; > + } > + } > + if (!available) { > + break; > + } > + } > + } > +} > + > +/* > + * Flush all buffer data from this stream's queue into the driver's virtual > + * queue. > + * > + * @stream: VirtIOSoundPCMStream *stream > + */ > +static void virtio_snd_pcm_flush(VirtIOSoundPCMStream *stream) > +{ > + VirtIOSoundPCMBlock *block; > + VirtIOSoundPCMBlock *next; > + > + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { > + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { > + AUD_write(stream->voice.out, block->data + block->offset, block->size); Is it OK to ignore AUD_write() returning < block->size? If so, can you add a comment please? > + virtqueue_push(block->vq, block->elem, sizeof(block->elem)); > + virtio_notify(VIRTIO_DEVICE(stream->s), block->vq); > + QSIMPLEQ_REMOVE(&stream->queue, block, VirtIOSoundPCMBlock, entry); > + } > + } > +}
On Mon, 04 Sep 2023 13:26, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: >> /* >> - * Handles VIRTIO_SND_R_PCM_RELEASE. Releases the buffer resources allocated to >> - * a stream. >> + * Returns the number of I/O messages that are being processed. >> + * >> + * @stream: VirtIOSoundPCMStream >> + */ >> +static size_t virtio_snd_pcm_get_pending_io_msgs(VirtIOSoundPCMStream *stream) >> +{ >> + VirtIOSoundPCMBlock *block; >> + VirtIOSoundPCMBlock *next; >> + size_t size = 0; >> + >> + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { >> + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { >> + size += 1; > >Can you add a comment explaining this magic size? It's not magic, it's simply how many messages there are as explained in the function doc comment. This was previously bytes hence `size`. I will change the variable name to `count`. >> +static void virtio_snd_handle_tx(VirtIODevice *vdev, VirtQueue *vq) >> +{ >> + VirtIOSound *s = VIRTIO_SND(vdev); >> + VirtIOSoundPCMStream *stream = NULL; >> + VirtQueueElement *elem; >> + size_t sz; >> + virtio_snd_pcm_xfer hdr; >> + virtio_snd_pcm_status resp = { 0 }; > >virtio_snd_pcm_status has multiple fields, so better zero-initialize >all of them with '{ }'. I don't understand why, virtio_snd_pcm_status has two int fields hence { 0 } zero-initializes all of them. >> +/* >> + * AUD_* output callback. >> + * >> + * @data: VirtIOSoundPCMStream stream >> + * @available: number of bytes that can be written with AUD_write() >> + */ >> +static void virtio_snd_pcm_out_cb(void *data, int available) >> +{ >> + VirtIOSoundPCMStream *stream = data; >> + VirtIOSoundPCMBlock *block; >> + VirtIOSoundPCMBlock *next; >> + size_t size; >> + >> + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { >> + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { >> + for (;;) { >> + size = MIN(block->size, available); >> + size = AUD_write(stream->voice.out, >> + block->data + block->offset, >> + size); > >If AUD_write() returns 0, is this an infinite loop? Hm since we have available > 0 bytes this wouldn't theoretically happen, but I see there are code paths that return 0 on bugs/failures, I will add the check. >> + block->size -= size; >> + block->offset += size; >> + if (!block->size) { >> + virtqueue_push(block->vq, >> + block->elem, >> + sizeof(block->elem)); >> + virtio_notify(VIRTIO_DEVICE(stream->s), >> + block->vq); >> + QSIMPLEQ_REMOVE_HEAD(&stream->queue, entry); >> + g_free(block); >> + available -= size; >> + break; >> + } >> + >> + available -= size; >> + if (!available) { >> + break; >> + } >> + } >> + if (!available) { >> + break; >> + } >> + } >> + } >> +} >> + >> +/* >> + * Flush all buffer data from this stream's queue into the driver's virtual >> + * queue. >> + * >> + * @stream: VirtIOSoundPCMStream *stream >> + */ >> +static void virtio_snd_pcm_flush(VirtIOSoundPCMStream *stream) >> +{ >> + VirtIOSoundPCMBlock *block; >> + VirtIOSoundPCMBlock *next; >> + >> + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { >> + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { >> + AUD_write(stream->voice.out, block->data + block->offset, block->size); > >Is it OK to ignore AUD_write() returning < block->size? >If so, can you add a comment please? This is a flush event with a timeout so it should complete asap. As mentioned in another reply it might be better to copy the data to a buffer in order not to lose any audio bytes. Thank you for the feedback, Manos
On 4/9/23 12:34, Manos Pitsidianakis wrote: > On Mon, 04 Sep 2023 13:26, Philippe Mathieu-Daudé <philmd@linaro.org> > wrote: >>> /* >>> - * Handles VIRTIO_SND_R_PCM_RELEASE. Releases the buffer resources >>> allocated to >>> - * a stream. >>> + * Returns the number of I/O messages that are being processed. >>> + * >>> + * @stream: VirtIOSoundPCMStream >>> + */ >>> +static size_t >>> virtio_snd_pcm_get_pending_io_msgs(VirtIOSoundPCMStream *stream) >>> +{ >>> + VirtIOSoundPCMBlock *block; >>> + VirtIOSoundPCMBlock *next; >>> + size_t size = 0; >>> + >>> + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { >>> + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { >>> + size += 1; >> >> Can you add a comment explaining this magic size? > > It's not magic, it's simply how many messages there are as explained in > the function doc comment. This was previously bytes hence `size`. I will > change the variable name to `count`. Ah OK. 'msg_processed'? >>> +static void virtio_snd_handle_tx(VirtIODevice *vdev, VirtQueue *vq) >>> +{ >>> + VirtIOSound *s = VIRTIO_SND(vdev); >>> + VirtIOSoundPCMStream *stream = NULL; >>> + VirtQueueElement *elem; >>> + size_t sz; >>> + virtio_snd_pcm_xfer hdr; >>> + virtio_snd_pcm_status resp = { 0 }; >> >> virtio_snd_pcm_status has multiple fields, so better zero-initialize >> all of them with '{ }'. > > I don't understand why, virtio_snd_pcm_status has two int fields hence { > 0 } zero-initializes all of them. Hmm I'm too busy right now to check the C standard. Looking at QEMU code base, most of the time we use nothing or N * 0 to initialize N fields: $ git grep '{ }' | wc -l 536 $ git grep -E '{ ?0, 0 ?}' | wc -l 50 $ git grep '{ }' | wc -l 536 Anyway, not a blocker. >>> +/* >>> + * AUD_* output callback. >>> + * >>> + * @data: VirtIOSoundPCMStream stream >>> + * @available: number of bytes that can be written with AUD_write() >>> + */ >>> +static void virtio_snd_pcm_out_cb(void *data, int available) >>> +{ >>> + VirtIOSoundPCMStream *stream = data; >>> + VirtIOSoundPCMBlock *block; >>> + VirtIOSoundPCMBlock *next; >>> + size_t size; >>> + >>> + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { >>> + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { >>> + for (;;) { >>> + size = MIN(block->size, available); >>> + size = AUD_write(stream->voice.out, >>> + block->data + block->offset, >>> + size); >> >> If AUD_write() returns 0, is this an infinite loop? > > Hm since we have available > 0 bytes this wouldn't theoretically happen, > but I see there are code paths that return 0 on bugs/failures, I will > add the check. Thanks. >>> + block->size -= size; >>> + block->offset += size; >>> + if (!block->size) { >>> + virtqueue_push(block->vq, >>> + block->elem, >>> + sizeof(block->elem)); >>> + virtio_notify(VIRTIO_DEVICE(stream->s), >>> + block->vq); >>> + QSIMPLEQ_REMOVE_HEAD(&stream->queue, entry); >>> + g_free(block); >>> + available -= size; >>> + break; >>> + } >>> + >>> + available -= size; >>> + if (!available) { >>> + break; >>> + } >>> + } >>> + if (!available) { >>> + break; >>> + } >>> + } >>> + } >>> +} >>> + >>> +/* >>> + * Flush all buffer data from this stream's queue into the driver's >>> virtual >>> + * queue. >>> + * >>> + * @stream: VirtIOSoundPCMStream *stream >>> + */ >>> +static void virtio_snd_pcm_flush(VirtIOSoundPCMStream *stream) >>> +{ >>> + VirtIOSoundPCMBlock *block; >>> + VirtIOSoundPCMBlock *next; >>> + >>> + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { >>> + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { >>> + AUD_write(stream->voice.out, block->data + >>> block->offset, block->size); >> >> Is it OK to ignore AUD_write() returning < block->size? >> If so, can you add a comment please? > > This is a flush event with a timeout so it should complete asap. As > mentioned in another reply it might be better to copy the data to a > buffer in order not to lose any audio bytes. Maybe this could do: /* * This is a flush event with a timeout which should complete ASAP. * TODO: Copy the data to a buffer in order not to lose any audio * bytes. (Meanwhile we just discard any write failure). */ Regards, Phil.
Am 04.09.23 um 12:34 schrieb Manos Pitsidianakis: > On Mon, 04 Sep 2023 13:26, Philippe Mathieu-Daudé <philmd@linaro.org> > wrote: > >>> +/* >>> + * AUD_* output callback. >>> + * >>> + * @data: VirtIOSoundPCMStream stream >>> + * @available: number of bytes that can be written with AUD_write() >>> + */ >>> +static void virtio_snd_pcm_out_cb(void *data, int available) >>> +{ >>> + VirtIOSoundPCMStream *stream = data; >>> + VirtIOSoundPCMBlock *block; >>> + VirtIOSoundPCMBlock *next; >>> + size_t size; >>> + >>> + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { >>> + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { >>> + for (;;) { >>> + size = MIN(block->size, available); >>> + size = AUD_write(stream->voice.out, >>> + block->data + block->offset, >>> + size); >> >> If AUD_write() returns 0, is this an infinite loop? > > Hm since we have available > 0 bytes this wouldn't theoretically > happen, but I see there are code paths that return 0 on bugs/failures, > I will add the check. Before QEMU 8.0.0 it was possible that AUD_write() couldn't write the last audio frame and sometimes 'available' was just miscalculated. Since commit e1e6a6fcc9 ("audio: handle leftover audio frame from upsampling") AUD_write() writes all 'available' bytes. With best regards, Volker > >>> + block->size -= size; >>> + block->offset += size; >>> + if (!block->size) { >>> + virtqueue_push(block->vq, >>> + block->elem, >>> + sizeof(block->elem)); >>> + virtio_notify(VIRTIO_DEVICE(stream->s), >>> + block->vq); >>> + QSIMPLEQ_REMOVE_HEAD(&stream->queue, entry); >>> + g_free(block); >>> + available -= size; >>> + break; >>> + } >>> + >>> + available -= size; >>> + if (!available) { >>> + break; >>> + } >>> + } >>> + if (!available) { >>> + break; >>> + } >>> + } >>> + } >>> +} >>> + >>> +/* >>> + * Flush all buffer data from this stream's queue into the driver's >>> virtual >>> + * queue. >>> + * >>> + * @stream: VirtIOSoundPCMStream *stream >>> + */ >>> +static void virtio_snd_pcm_flush(VirtIOSoundPCMStream *stream) >>> +{ >>> + VirtIOSoundPCMBlock *block; >>> + VirtIOSoundPCMBlock *next; >>> + >>> + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { >>> + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { >>> + AUD_write(stream->voice.out, block->data + >>> block->offset, block->size); >>
Am 04.09.23 um 23:34 schrieb Volker Rümelin: > Am 04.09.23 um 12:34 schrieb Manos Pitsidianakis: >> On Mon, 04 Sep 2023 13:26, Philippe Mathieu-Daudé <philmd@linaro.org> >> wrote: >> >>>> +/* >>>> + * AUD_* output callback. >>>> + * >>>> + * @data: VirtIOSoundPCMStream stream >>>> + * @available: number of bytes that can be written with AUD_write() >>>> + */ >>>> +static void virtio_snd_pcm_out_cb(void *data, int available) >>>> +{ >>>> + VirtIOSoundPCMStream *stream = data; >>>> + VirtIOSoundPCMBlock *block; >>>> + VirtIOSoundPCMBlock *next; >>>> + size_t size; >>>> + >>>> + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { >>>> + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { >>>> + for (;;) { >>>> + size = MIN(block->size, available); >>>> + size = AUD_write(stream->voice.out, >>>> + block->data + block->offset, >>>> + size); >>> >>> If AUD_write() returns 0, is this an infinite loop? >> >> Hm since we have available > 0 bytes this wouldn't theoretically >> happen, but I see there are code paths that return 0 on >> bugs/failures, I will add the check. > > Before QEMU 8.0.0 it was possible that AUD_write() couldn't write the > last audio frame and sometimes 'available' was just miscalculated. > Since commit e1e6a6fcc9 ("audio: handle leftover audio frame from > upsampling") AUD_write() writes all 'available' bytes. > I thought about this again. The error check is necessary. > With best regards, > Volker > >> >>>> + block->size -= size; >>>> + block->offset += size; >>>> + if (!block->size) { >>>> + virtqueue_push(block->vq, >>>> + block->elem, >>>> + sizeof(block->elem)); >>>> + virtio_notify(VIRTIO_DEVICE(stream->s), >>>> + block->vq); >>>> + QSIMPLEQ_REMOVE_HEAD(&stream->queue, entry); >>>> + g_free(block); >>>> + available -= size; >>>> + break; >>>> + } >>>> + >>>> + available -= size; >>>> + if (!available) { >>>> + break; >>>> + } >>>> + } >>>> + if (!available) { >>>> + break; >>>> + } >>>> + } >>>> + } >>>> +} >>>> + >>>> +/* >>>> + * Flush all buffer data from this stream's queue into the >>>> driver's virtual >>>> + * queue. >>>> + * >>>> + * @stream: VirtIOSoundPCMStream *stream >>>> + */ >>>> +static void virtio_snd_pcm_flush(VirtIOSoundPCMStream *stream) >>>> +{ >>>> + VirtIOSoundPCMBlock *block; >>>> + VirtIOSoundPCMBlock *next; >>>> + >>>> + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { >>>> + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { >>>> + AUD_write(stream->voice.out, block->data + >>>> block->offset, block->size); >>> >
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events index 60ab62a80d..3b95e745c2 100644 --- a/hw/virtio/trace-events +++ b/hw/virtio/trace-events @@ -171,3 +171,5 @@ virtio_snd_handle_pcm_release(uint32_t stream) "VIRTIO_SND_PCM_RELEASE called fo virtio_snd_handle_code(uint32_t val, const char *code) "ctrl code msg val = %"PRIu32" == %s" virtio_snd_handle_chmap_info(void) "VIRTIO_SND_CHMAP_INFO called" virtio_snd_handle_event(void) "event queue callback called" +virtio_snd_pcm_stream_flush(uint32_t stream) "flushing stream %"PRIu32 +virtio_snd_handle_xfer(void) "tx/rx queue callback called" diff --git a/hw/virtio/virtio-snd.c b/hw/virtio/virtio-snd.c index e4fc832c39..4859ce4bf6 100644 --- a/hw/virtio/virtio-snd.c +++ b/hw/virtio/virtio-snd.c @@ -31,6 +31,15 @@ #define VIRTIO_SOUND_CHMAP_DEFAULT 0 #define VIRTIO_SOUND_HDA_FN_NID 0 +static void virtio_snd_pcm_out_cb(void *data, int available); +static void virtio_snd_process_cmdq(VirtIOSound *s); +static void virtio_snd_pcm_flush(VirtIOSoundPCMStream *stream); +static uint32_t +virtio_snd_pcm_read_write(VirtIOSoundPCMStream *stream, + VirtQueue *vq, + VirtQueueElement *element, + bool read); + static uint32_t supported_formats = BIT(VIRTIO_SND_PCM_FMT_S8) | BIT(VIRTIO_SND_PCM_FMT_U8) | BIT(VIRTIO_SND_PCM_FMT_S16) @@ -368,7 +377,24 @@ static void virtio_snd_get_qemu_audsettings(audsettings *as, */ static void virtio_snd_pcm_close(VirtIOSoundPCMStream *stream) { + VirtIOSoundPCMBlock *block, *next; + if (stream) { + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { + virtqueue_push(block->vq, + block->elem, + sizeof(block->elem)); + virtio_notify(VIRTIO_DEVICE(stream->s), + block->vq); + QSIMPLEQ_REMOVE_HEAD(&stream->queue, entry); + g_free(block); + } + } + if (stream->direction == VIRTIO_SND_D_OUTPUT) { + AUD_close_out(&stream->pcm->snd->card, stream->voice.out); + stream->voice.out = NULL; + } qemu_mutex_destroy(&stream->queue_mutex); g_free(stream); } @@ -422,6 +448,17 @@ static uint32_t virtio_snd_pcm_prepare(VirtIOSound *s, uint32_t stream_id) stream->positions[0] = VIRTIO_SND_CHMAP_FL; stream->positions[1] = VIRTIO_SND_CHMAP_FR; + if (stream->direction == VIRTIO_SND_D_OUTPUT) { + stream->voice.out = AUD_open_out(&s->card, + stream->voice.out, + "virtio-sound.out", + stream, + virtio_snd_pcm_out_cb, + &as); + } else { + qemu_log_mask(LOG_UNIMP, "virtio_snd: input/capture is unimplemented."); + } + stream->as = as; stream->desired_as = stream->as; qemu_mutex_init(&stream->queue_mutex); @@ -465,6 +502,7 @@ static void virtio_snd_handle_pcm_start_stop(VirtIOSound *s, bool start) { VirtIOSoundPCMStream *stream; + VirtIOSoundPCMBlock *block, *next; virtio_snd_pcm_hdr req; size_t sz = iov_to_buf(cmd->elem->out_sg, cmd->elem->out_num, @@ -481,15 +519,48 @@ static void virtio_snd_handle_pcm_start_stop(VirtIOSound *s, "VIRTIO_SND_R_PCM_STOP", req.stream_id); stream = virtio_snd_pcm_get_stream(s, req.stream_id); - if (!stream) { + if (stream) { + if (stream->direction == VIRTIO_SND_D_OUTPUT) { + AUD_set_active_out(stream->voice.out, start); + } + /* remove previous buffers. */ + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { + virtqueue_push(block->vq, + block->elem, + sizeof(block->elem)); + virtio_notify(VIRTIO_DEVICE(stream->s), block->vq); + QSIMPLEQ_REMOVE_HEAD(&stream->queue, entry); + g_free(block); + } + } + } else { error_report("Invalid stream id: %"PRIu32, req.stream_id); cmd->resp.code = VIRTIO_SND_S_BAD_MSG; } } /* - * Handles VIRTIO_SND_R_PCM_RELEASE. Releases the buffer resources allocated to - * a stream. + * Returns the number of I/O messages that are being processed. + * + * @stream: VirtIOSoundPCMStream + */ +static size_t virtio_snd_pcm_get_pending_io_msgs(VirtIOSoundPCMStream *stream) +{ + VirtIOSoundPCMBlock *block; + VirtIOSoundPCMBlock *next; + size_t size = 0; + + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { + size += 1; + } + } + return size; +} + +/* + * Handles VIRTIO_SND_R_PCM_RELEASE. * * @s: VirtIOSound device * @cmd: The request command queue element from VirtIOSound cmdq field @@ -520,6 +591,22 @@ static void virtio_snd_handle_pcm_release(VirtIOSound *s, cmd->resp.code = VIRTIO_SND_S_BAD_MSG; return; } + + if (virtio_snd_pcm_get_pending_io_msgs(stream)) { + /* + * virtio-v1.2-csd01, 5.14.6.6.5.1, + * Device Requirements: Stream Release + * + * - The device MUST complete all pending I/O messages for the + * specified stream ID. + * - The device MUST NOT complete the control request while there + * are pending I/O messages for the specified stream ID. + */ + virtio_snd_process_cmdq(stream->s); + trace_virtio_snd_pcm_stream_flush(stream_id); + virtio_snd_pcm_flush(stream); + } + cmd->resp.code = VIRTIO_SND_S_OK; } @@ -669,6 +756,79 @@ static void virtio_snd_handle_event(VirtIODevice *vdev, VirtQueue *vq) trace_virtio_snd_handle_event(); } +/* + * The tx virtqueue handler. Makes the buffers available to their respective + * streams for consumption. + * + * @vdev: VirtIOSound device + * @vq: tx virtqueue + */ +static void virtio_snd_handle_tx(VirtIODevice *vdev, VirtQueue *vq) +{ + VirtIOSound *s = VIRTIO_SND(vdev); + VirtIOSoundPCMStream *stream = NULL; + VirtQueueElement *elem; + size_t sz; + virtio_snd_pcm_xfer hdr; + virtio_snd_pcm_status resp = { 0 }; + + trace_virtio_snd_handle_xfer(); + + for (;;) { + elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); + if (!elem) { + break; + } + /* get the message hdr object */ + sz = iov_to_buf(elem->out_sg, + elem->out_num, + 0, + &hdr, + sizeof(hdr)); + if (sz != sizeof(hdr) + || hdr.stream_id >= s->snd_conf.streams + || !s->pcm->streams[hdr.stream_id]) { + goto tx_err; + } + + stream = s->pcm->streams[hdr.stream_id]; + if (stream->direction != VIRTIO_SND_D_OUTPUT) { + goto tx_err; + } + + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { + virtio_snd_pcm_read_write(stream, + vq, + elem, + hdr.stream_id == VIRTIO_SND_D_INPUT); + + resp.status = VIRTIO_SND_S_OK; + iov_from_buf(elem->in_sg, + elem->in_num, + 0, + &resp, + sizeof(resp)); + } + continue; + +tx_err: + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { + resp.status = VIRTIO_SND_S_BAD_MSG; + iov_from_buf(elem->in_sg, + elem->in_num, + 0, + &resp, + sizeof(resp)); + } + } + + /* + * Notify vq about virtio_snd_pcm_status responses. + * Buffer responses must be notified separately later. + */ + virtio_notify(VIRTIO_DEVICE(s), vq); +} + /* * Stub buffer virtqueue handler. * @@ -770,7 +930,7 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp) vsnd->queues[VIRTIO_SND_VQ_EVENT] = virtio_add_queue(vdev, 64, virtio_snd_handle_event); vsnd->queues[VIRTIO_SND_VQ_TX] = - virtio_add_queue(vdev, 64, virtio_snd_handle_xfer); + virtio_add_queue(vdev, 64, virtio_snd_handle_tx); vsnd->queues[VIRTIO_SND_VQ_RX] = virtio_add_queue(vdev, 64, virtio_snd_handle_xfer); qemu_mutex_init(&vsnd->cmdq_mutex); @@ -795,6 +955,73 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp) } } +/* + * AUD_* output callback. + * + * @data: VirtIOSoundPCMStream stream + * @available: number of bytes that can be written with AUD_write() + */ +static void virtio_snd_pcm_out_cb(void *data, int available) +{ + VirtIOSoundPCMStream *stream = data; + VirtIOSoundPCMBlock *block; + VirtIOSoundPCMBlock *next; + size_t size; + + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { + for (;;) { + size = MIN(block->size, available); + size = AUD_write(stream->voice.out, + block->data + block->offset, + size); + block->size -= size; + block->offset += size; + if (!block->size) { + virtqueue_push(block->vq, + block->elem, + sizeof(block->elem)); + virtio_notify(VIRTIO_DEVICE(stream->s), + block->vq); + QSIMPLEQ_REMOVE_HEAD(&stream->queue, entry); + g_free(block); + available -= size; + break; + } + + available -= size; + if (!available) { + break; + } + } + if (!available) { + break; + } + } + } +} + +/* + * Flush all buffer data from this stream's queue into the driver's virtual + * queue. + * + * @stream: VirtIOSoundPCMStream *stream + */ +static void virtio_snd_pcm_flush(VirtIOSoundPCMStream *stream) +{ + VirtIOSoundPCMBlock *block; + VirtIOSoundPCMBlock *next; + + WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { + QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { + AUD_write(stream->voice.out, block->data + block->offset, block->size); + virtqueue_push(block->vq, block->elem, sizeof(block->elem)); + virtio_notify(VIRTIO_DEVICE(stream->s), block->vq); + QSIMPLEQ_REMOVE(&stream->queue, block, VirtIOSoundPCMBlock, entry); + } + } +} + static void virtio_snd_unrealize(DeviceState *dev) { VirtIODevice *vdev = VIRTIO_DEVICE(dev); @@ -818,6 +1045,7 @@ static void virtio_snd_unrealize(DeviceState *dev) } g_free(vsnd->pcm->streams); } + g_free(vsnd->pcm->pcm_params); g_free(vsnd->pcm); vsnd->pcm = NULL; } @@ -827,6 +1055,32 @@ static void virtio_snd_unrealize(DeviceState *dev) } +static uint32_t +virtio_snd_pcm_read_write(VirtIOSoundPCMStream *stream, + VirtQueue *vq, + VirtQueueElement *element, + bool read) +{ + VirtIOSoundPCMBlock *fragment; + size_t size = iov_size(element->out_sg, element->out_num) - + sizeof(virtio_snd_pcm_xfer); + + fragment = g_malloc0(sizeof(VirtIOSoundPCMBlock) + size); + fragment->elem = element; + fragment->vq = vq; + fragment->size = size; + fragment->offset = 0; + + iov_to_buf(element->out_sg, element->out_num, + sizeof(virtio_snd_pcm_xfer), + fragment->data, + size); + + QSIMPLEQ_INSERT_TAIL(&stream->queue, fragment, entry); + + return fragment->size; +} + static void virtio_snd_reset(VirtIODevice *vdev) { VirtIOSound *s = VIRTIO_SND(vdev); diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h index b7046418cf..0350df9ab7 100644 --- a/include/hw/virtio/virtio-snd.h +++ b/include/hw/virtio/virtio-snd.h @@ -79,6 +79,8 @@ typedef struct VirtIOSoundPCMParams VirtIOSoundPCMParams; typedef struct VirtIOSoundPCM VirtIOSoundPCM; +typedef struct VirtIOSoundPCMBlock VirtIOSoundPCMBlock; + /* Stream params */ struct VirtIOSoundPCMParams { uint32_t features; @@ -89,6 +91,15 @@ struct VirtIOSoundPCMParams { uint8_t rate; }; +struct VirtIOSoundPCMBlock { + QSIMPLEQ_ENTRY(VirtIOSoundPCMBlock) entry; + VirtQueueElement *elem; + VirtQueue *vq; + size_t size; + uint64_t offset; + uint8_t data[]; +}; + struct VirtIOSoundPCM { VirtIOSound *snd; VirtIOSoundPCMParams *pcm_params;