* [PATCH v8 01/12] Add virtio-sound device stub
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
@ 2023-08-28 19:54 ` Emmanouil Pitsidianakis
2023-08-28 19:54 ` [PATCH v8 02/12] Add virtio-sound-pci device Emmanouil Pitsidianakis
` (13 subsequent siblings)
14 siblings, 0 replies; 51+ messages in thread
From: Emmanouil Pitsidianakis @ 2023-08-28 19:54 UTC (permalink / raw)
To: qemu-devel
Cc: Emmanouil Pitsidianakis, Igor Skalkin, Anton Yakovlev,
Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin,
Marcel Apfelbaum, Daniel P. Berrangé, Eduardo Habkost,
Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
Add a new VIRTIO device for the virtio sound device id. Functionality
will be added in the following commits.
Based-on: https://github.com/OpenSynergy/qemu/commit/5a2f350eec5d157b90d9c7b40a8e603f4da92471
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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>
---
MAINTAINERS | 6 +
hw/virtio/Kconfig | 5 +
hw/virtio/meson.build | 1 +
hw/virtio/trace-events | 9 ++
hw/virtio/virtio-snd.c | 219 +++++++++++++++++++++++++++++++++
include/hw/virtio/virtio-snd.h | 78 ++++++++++++
6 files changed, 318 insertions(+)
create mode 100644 hw/virtio/virtio-snd.c
create mode 100644 include/hw/virtio/virtio-snd.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 6111b6b4d9..ba365d621c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2245,6 +2245,12 @@ F: hw/virtio/virtio-mem-pci.h
F: hw/virtio/virtio-mem-pci.c
F: include/hw/virtio/virtio-mem.h
+virtio-snd
+M: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+S: Supported
+F: hw/virtio/virtio-snd*.c
+F: include/hw/virtio/virtio-snd.h
+
nvme
M: Keith Busch <kbusch@kernel.org>
M: Klaus Jensen <its@irrelevant.dk>
diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index 92c9cf6c96..d6f20657b3 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -17,6 +17,11 @@ config VIRTIO_PCI
depends on PCI
select VIRTIO
+config VIRTIO_SND
+ bool
+ default y
+ depends on VIRTIO
+
config VIRTIO_MMIO
bool
select VIRTIO
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 13e7c6c272..120d4bfa0a 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -31,6 +31,7 @@ specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c
specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.c'))
specific_virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
specific_virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
+specific_virtio_ss.add(when: 'CONFIG_VIRTIO_SND', if_true: files('virtio-snd.c'))
specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_GPIO', if_true: files('vhost-user-gpio.c'))
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 7109cf1a3b..3ed7da35f2 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -154,3 +154,12 @@ virtio_pmem_flush_done(int type) "fsync return=%d"
virtio_gpio_start(void) "start"
virtio_gpio_stop(void) "stop"
virtio_gpio_set_status(uint8_t status) "0x%x"
+
+#virtio-snd.c
+virtio_snd_get_config(void *vdev, uint32_t jacks, uint32_t streams, uint32_t chmaps) "snd %p: get_config jacks=%"PRIu32" streams=%"PRIu32" chmaps=%"PRIu32""
+virtio_snd_set_config(void *vdev, uint32_t jacks, uint32_t new_jacks, uint32_t streams, uint32_t new_streams, uint32_t chmaps, uint32_t new_chmaps) "snd %p: set_config jacks from %"PRIu32"->%"PRIu32", streams from %"PRIu32"->%"PRIu32", chmaps from %"PRIu32"->%"PRIu32
+virtio_snd_get_features(void *vdev, uint64_t features) "snd %p: get_features 0x%"PRIx64
+virtio_snd_vm_state_running(void) "vm state running"
+virtio_snd_vm_state_stopped(void) "vm state stopped"
+virtio_snd_realize(void *snd) "snd %p: realize"
+virtio_snd_unrealize(void *snd) "snd %p: unrealize"
diff --git a/hw/virtio/virtio-snd.c b/hw/virtio/virtio-snd.c
new file mode 100644
index 0000000000..a056a7bcc6
--- /dev/null
+++ b/hw/virtio/virtio-snd.c
@@ -0,0 +1,219 @@
+/*
+ * VIRTIO Sound Device conforming to
+ *
+ * "Virtual I/O Device (VIRTIO) Version 1.2
+ * Committee Specification Draft 01
+ * 09 May 2022"
+ *
+ * <https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-52900014>
+ *
+ * Copyright (c) 2023 Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>
+ * Copyright (C) 2019 OpenSynergy GmbH
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version. See the COPYING file in the
+ * top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/iov.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+#include "include/qemu/lockable.h"
+#include "sysemu/runstate.h"
+#include "trace.h"
+#include "qapi/error.h"
+#include "hw/virtio/virtio-snd.h"
+
+#define VIRTIO_SOUND_VM_VERSION 1
+#define VIRTIO_SOUND_JACK_DEFAULT 0
+#define VIRTIO_SOUND_STREAM_DEFAULT 1
+#define VIRTIO_SOUND_CHMAP_DEFAULT 0
+#define VIRTIO_SOUND_HDA_FN_NID 0
+
+static const VMStateDescription vmstate_virtio_snd_device = {
+ .name = TYPE_VIRTIO_SND,
+ .version_id = VIRTIO_SOUND_VM_VERSION,
+ .minimum_version_id = VIRTIO_SOUND_VM_VERSION,
+};
+
+static const VMStateDescription vmstate_virtio_snd = {
+ .name = "virtio-sound",
+ .minimum_version_id = VIRTIO_SOUND_VM_VERSION,
+ .version_id = VIRTIO_SOUND_VM_VERSION,
+ .fields = (VMStateField[]) {
+ VMSTATE_VIRTIO_DEVICE,
+ VMSTATE_END_OF_LIST()
+ },
+};
+
+static Property virtio_snd_properties[] = {
+ DEFINE_PROP_UINT32("jacks", VirtIOSound, snd_conf.jacks,
+ VIRTIO_SOUND_JACK_DEFAULT),
+ DEFINE_PROP_UINT32("streams", VirtIOSound, snd_conf.streams,
+ VIRTIO_SOUND_STREAM_DEFAULT),
+ DEFINE_PROP_UINT32("chmaps", VirtIOSound, snd_conf.chmaps,
+ VIRTIO_SOUND_CHMAP_DEFAULT),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void
+virtio_snd_get_config(VirtIODevice *vdev, uint8_t *config)
+{
+ VirtIOSound *s = VIRTIO_SND(vdev);
+ trace_virtio_snd_get_config(vdev,
+ s->snd_conf.jacks,
+ s->snd_conf.streams,
+ s->snd_conf.chmaps);
+
+ memcpy(config, &s->snd_conf, sizeof(s->snd_conf));
+}
+
+static void
+virtio_snd_set_config(VirtIODevice *vdev, const uint8_t *config)
+{
+ VirtIOSound *s = VIRTIO_SND(vdev);
+ const virtio_snd_config *sndconfig =
+ (const virtio_snd_config *)config;
+
+
+ trace_virtio_snd_set_config(vdev,
+ s->snd_conf.jacks,
+ sndconfig->jacks,
+ s->snd_conf.streams,
+ sndconfig->streams,
+ s->snd_conf.chmaps,
+ sndconfig->chmaps);
+
+ memcpy(&s->snd_conf, sndconfig, sizeof(s->snd_conf));
+}
+
+/*
+ * Queue handler stub.
+ *
+ * @vdev: VirtIOSound device
+ * @vq: virtqueue
+ */
+static void virtio_snd_handle_queue(VirtIODevice *vdev, VirtQueue *vq) {}
+
+static uint64_t get_features(VirtIODevice *vdev, uint64_t features,
+ Error **errp)
+{
+ /*
+ * virtio-v1.2-csd01, 5.14.3,
+ * Feature Bits
+ * None currently defined.
+ */
+ VirtIOSound *s = VIRTIO_SND(vdev);
+ features |= s->features;
+
+ trace_virtio_snd_get_features(vdev, features);
+
+ return features;
+}
+
+static void
+virtio_snd_vm_state_change(void *opaque, bool running,
+ RunState state)
+{
+ if (running) {
+ trace_virtio_snd_vm_state_running();
+ } else {
+ trace_virtio_snd_vm_state_stopped();
+ }
+}
+
+static void virtio_snd_realize(DeviceState *dev, Error **errp)
+{
+ ERRP_GUARD();
+ VirtIOSound *vsnd = VIRTIO_SND(dev);
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+
+ vsnd->vmstate =
+ qemu_add_vm_change_state_handler(virtio_snd_vm_state_change, vsnd);
+
+ trace_virtio_snd_realize(vsnd);
+
+ virtio_init(vdev, VIRTIO_ID_SOUND, sizeof(virtio_snd_config));
+ virtio_add_feature(&vsnd->features, VIRTIO_F_VERSION_1);
+
+ /* set number of jacks and streams */
+ if (vsnd->snd_conf.jacks > 8) {
+ error_setg(errp,
+ "Invalid number of jacks: %"PRIu32,
+ vsnd->snd_conf.jacks);
+ return;
+ }
+ if (vsnd->snd_conf.streams < 1 || vsnd->snd_conf.streams > 10) {
+ error_setg(errp,
+ "Invalid number of streams: %"PRIu32,
+ vsnd->snd_conf.streams);
+ return;
+ }
+
+ if (vsnd->snd_conf.chmaps > VIRTIO_SND_CHMAP_MAX_SIZE) {
+ error_setg(errp,
+ "Invalid number of channel maps: %"PRIu32,
+ vsnd->snd_conf.chmaps);
+ return;
+ }
+
+ AUD_register_card("virtio-sound", &vsnd->card);
+
+ vsnd->queues[VIRTIO_SND_VQ_CONTROL] =
+ virtio_add_queue(vdev, 64, virtio_snd_handle_queue);
+ vsnd->queues[VIRTIO_SND_VQ_EVENT] =
+ virtio_add_queue(vdev, 64, virtio_snd_handle_queue);
+ vsnd->queues[VIRTIO_SND_VQ_TX] =
+ virtio_add_queue(vdev, 64, virtio_snd_handle_queue);
+ vsnd->queues[VIRTIO_SND_VQ_RX] =
+ virtio_add_queue(vdev, 64, virtio_snd_handle_queue);
+}
+
+static void virtio_snd_unrealize(DeviceState *dev)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VirtIOSound *vsnd = VIRTIO_SND(dev);
+
+ qemu_del_vm_change_state_handler(vsnd->vmstate);
+ virtio_del_queue(vdev, 0);
+
+ trace_virtio_snd_unrealize(vsnd);
+
+ AUD_remove_card(&vsnd->card);
+ virtio_cleanup(vdev);
+}
+
+
+static void virtio_snd_reset(VirtIODevice *vdev) {}
+
+static void virtio_snd_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+
+
+ set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
+ device_class_set_props(dc, virtio_snd_properties);
+
+ dc->vmsd = &vmstate_virtio_snd;
+ vdc->vmsd = &vmstate_virtio_snd_device;
+ vdc->realize = virtio_snd_realize;
+ vdc->unrealize = virtio_snd_unrealize;
+ vdc->get_config = virtio_snd_get_config;
+ vdc->set_config = virtio_snd_set_config;
+ vdc->get_features = get_features;
+ vdc->reset = virtio_snd_reset;
+ vdc->legacy_features = 0;
+}
+
+static const TypeInfo virtio_snd_types[] = {
+ {
+ .name = TYPE_VIRTIO_SND,
+ .parent = TYPE_VIRTIO_DEVICE,
+ .instance_size = sizeof(VirtIOSound),
+ .class_init = virtio_snd_class_init,
+ }
+};
+
+DEFINE_TYPES(virtio_snd_types)
diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
new file mode 100644
index 0000000000..b3c0e6f079
--- /dev/null
+++ b/include/hw/virtio/virtio-snd.h
@@ -0,0 +1,78 @@
+/*
+ * VIRTIO Sound Device conforming to
+ *
+ * "Virtual I/O Device (VIRTIO) Version 1.2
+ * Committee Specification Draft 01
+ * 09 May 2022"
+ *
+ * Copyright (c) 2023 Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>
+ * Copyright (C) 2019 OpenSynergy GmbH
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version. See the COPYING file in the
+ * top-level directory.
+ */
+
+#ifndef QEMU_VIRTIO_SOUND_H
+#define QEMU_VIRTIO_SOUND_H
+
+#include "hw/virtio/virtio.h"
+#include "audio/audio.h"
+#include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_snd.h"
+
+#define TYPE_VIRTIO_SND "virtio-sound"
+#define VIRTIO_SND(obj) \
+ OBJECT_CHECK(VirtIOSound, (obj), TYPE_VIRTIO_SND)
+
+/* CONFIGURATION SPACE */
+
+typedef struct virtio_snd_config virtio_snd_config;
+
+/* COMMON DEFINITIONS */
+
+/* common header for request/response*/
+typedef struct virtio_snd_hdr virtio_snd_hdr;
+
+/* event notification */
+typedef struct virtio_snd_event virtio_snd_event;
+
+/* common control request to query an item information */
+typedef struct virtio_snd_query_info virtio_snd_query_info;
+
+/* JACK CONTROL MESSAGES */
+
+typedef struct virtio_snd_jack_hdr virtio_snd_jack_hdr;
+
+/* jack information structure */
+typedef struct virtio_snd_jack_info virtio_snd_jack_info;
+
+/* jack remapping control request */
+typedef struct virtio_snd_jack_remap virtio_snd_jack_remap;
+
+/*
+ * PCM CONTROL MESSAGES
+ */
+typedef struct virtio_snd_pcm_hdr virtio_snd_pcm_hdr;
+
+/* PCM stream info structure */
+typedef struct virtio_snd_pcm_info virtio_snd_pcm_info;
+
+/* set PCM stream params */
+typedef struct virtio_snd_pcm_set_params virtio_snd_pcm_set_params;
+
+/* I/O request header */
+typedef struct virtio_snd_pcm_xfer virtio_snd_pcm_xfer;
+
+/* I/O request status */
+typedef struct virtio_snd_pcm_status virtio_snd_pcm_status;
+
+typedef struct VirtIOSound {
+ VirtIODevice parent_obj;
+ VirtQueue *queues[VIRTIO_SND_VQ_MAX];
+ uint64_t features;
+ QEMUSoundCard card;
+ VMChangeStateEntry *vmstate;
+ virtio_snd_config snd_conf;
+} VirtIOSound;
+#endif
--
2.39.2
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH v8 02/12] Add virtio-sound-pci device
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
2023-08-28 19:54 ` [PATCH v8 01/12] Add virtio-sound device stub Emmanouil Pitsidianakis
@ 2023-08-28 19:54 ` Emmanouil Pitsidianakis
2023-09-04 6:32 ` Volker Rümelin
2023-09-06 9:05 ` Mark Cave-Ayland
2023-08-28 19:55 ` [PATCH v8 03/12] virtio-sound: handle control messages and streams Emmanouil Pitsidianakis
` (12 subsequent siblings)
14 siblings, 2 replies; 51+ messages in thread
From: Emmanouil Pitsidianakis @ 2023-08-28 19:54 UTC (permalink / raw)
To: qemu-devel
Cc: Emmanouil Pitsidianakis, Igor Skalkin, Anton Yakovlev,
Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin,
Marcel Apfelbaum, Daniel P. Berrangé, Eduardo Habkost,
Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
This patch adds a PCI wrapper device for the virtio-sound device.
It is necessary to instantiate a virtio-snd device in a guest.
All sound logic will be added to the virtio-snd device in the following
commits.
To add this device with a guest, you'll need a >=5.13 kernel compiled
with CONFIG_SND_VIRTIO=y, which at the time of writing most distros have
off by default.
Use with following flags in the invocation:
Pulseaudio:
-audio driver=pa,model=virtio
or
-audio driver=pa,model=virtio,server=/run/user/1000/pulse/native
sdl:
-audio driver=sdl,model=virtio
coreaudio (macos/darwin):
-audio driver=coreaudio,model=virtio
etc.
Based-on: https://github.com/OpenSynergy/qemu/commit/5a2f350eec5d157b90d9c7b40a8e603f4da92471
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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/meson.build | 1 +
hw/virtio/virtio-snd-pci.c | 97 ++++++++++++++++++++++++++++++++++++++
softmmu/qdev-monitor.c | 1 +
3 files changed, 99 insertions(+)
create mode 100644 hw/virtio/virtio-snd-pci.c
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 120d4bfa0a..5e5a83a4ee 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -63,6 +63,7 @@ virtio_pci_ss.add(when: 'CONFIG_VIRTIO_SERIAL', if_true: files('virtio-serial-pc
virtio_pci_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem-pci.c'))
virtio_pci_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu-pci.c'))
virtio_pci_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem-pci.c'))
+virtio_pci_ss.add(when: 'CONFIG_VIRTIO_SND', if_true: files('virtio-snd-pci.c'))
virtio_pci_ss.add(when: 'CONFIG_VHOST_VDPA_DEV', if_true: files('vdpa-dev-pci.c'))
virtio_pci_ss.add(when: 'CONFIG_VIRTIO_MD', if_true: files('virtio-md-pci.c'))
diff --git a/hw/virtio/virtio-snd-pci.c b/hw/virtio/virtio-snd-pci.c
new file mode 100644
index 0000000000..a6a530d161
--- /dev/null
+++ b/hw/virtio/virtio-snd-pci.c
@@ -0,0 +1,97 @@
+/*
+ * VIRTIO Sound Device PCI Bindings
+ *
+ * Copyright (c) 2023 Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version. See the COPYING file in the
+ * top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/audio/soundhw.h"
+#include "hw/virtio/virtio-pci.h"
+#include "hw/virtio/virtio-snd.h"
+
+typedef struct VirtIOSoundPCI VirtIOSoundPCI;
+
+/*
+ * virtio-snd-pci: This extends VirtioPCIProxy.
+ */
+#define TYPE_VIRTIO_SND_PCI "virtio-sound-pci"
+DECLARE_INSTANCE_CHECKER(VirtIOSoundPCI, VIRTIO_SND_PCI,
+ TYPE_VIRTIO_SND_PCI)
+
+struct VirtIOSoundPCI {
+ VirtIOPCIProxy parent;
+ VirtIOSound vdev;
+};
+
+static Property virtio_snd_pci_properties[] = {
+ DEFINE_AUDIO_PROPERTIES(VirtIOSoundPCI, vdev.card),
+ DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
+ VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
+ DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
+ DEV_NVECTORS_UNSPECIFIED),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_snd_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+{
+ VirtIOSoundPCI *dev = VIRTIO_SND_PCI(vpci_dev);
+ DeviceState *vdev = DEVICE(&dev->vdev);
+
+ if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
+ vpci_dev->nvectors = 2;
+ }
+
+ virtio_pci_force_virtio_1(vpci_dev);
+ qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
+}
+
+static void virtio_snd_pci_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ VirtioPCIClass *vpciklass = VIRTIO_PCI_CLASS(klass);
+
+ device_class_set_props(dc, virtio_snd_pci_properties);
+ dc->desc = "Virtio Sound";
+ set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
+
+ vpciklass->realize = virtio_snd_pci_realize;
+}
+
+static void virtio_snd_pci_instance_init(Object *obj)
+{
+ VirtIOSoundPCI *dev = VIRTIO_SND_PCI(obj);
+
+ virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+ TYPE_VIRTIO_SND);
+}
+
+static const VirtioPCIDeviceTypeInfo virtio_snd_pci_info = {
+ .generic_name = TYPE_VIRTIO_SND_PCI,
+ .instance_size = sizeof(VirtIOSoundPCI),
+ .instance_init = virtio_snd_pci_instance_init,
+ .class_init = virtio_snd_pci_class_init,
+};
+
+/* Create a Virtio Sound PCI device, so '-audio driver,model=virtio' works. */
+static int virtio_snd_pci_init(PCIBus *bus, const char *audiodev)
+{
+ DeviceState *dev;
+
+ dev = qdev_new(TYPE_VIRTIO_SND_PCI);
+ qdev_prop_set_string(dev, "audiodev", audiodev);
+ qdev_realize_and_unref(dev, BUS(bus), &error_fatal);
+ return 0;
+}
+
+static void virtio_snd_pci_register(void)
+{
+ virtio_pci_types_register(&virtio_snd_pci_info);
+ pci_register_soundhw("virtio", "Virtio Sound", virtio_snd_pci_init);
+}
+
+type_init(virtio_snd_pci_register);
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 74f4e41338..2e9835ad88 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -108,6 +108,7 @@ static const QDevAlias qdev_alias_table[] = {
{ "virtio-serial-device", "virtio-serial", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-serial-pci", "virtio-serial", QEMU_ARCH_VIRTIO_PCI},
+ { "virtio-sound-pci", "virtio-sound", QEMU_ARCH_VIRTIO_PCI},
{ "virtio-tablet-device", "virtio-tablet", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_VIRTIO_PCI },
--
2.39.2
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v8 02/12] Add virtio-sound-pci device
2023-08-28 19:54 ` [PATCH v8 02/12] Add virtio-sound-pci device Emmanouil Pitsidianakis
@ 2023-09-04 6:32 ` Volker Rümelin
2023-09-04 10:26 ` Manos Pitsidianakis
2023-09-06 9:05 ` Mark Cave-Ayland
1 sibling, 1 reply; 51+ messages in thread
From: Volker Rümelin @ 2023-09-04 6:32 UTC (permalink / raw)
To: Emmanouil Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
Michael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
> This patch adds a PCI wrapper device for the virtio-sound device.
> It is necessary to instantiate a virtio-snd device in a guest.
> All sound logic will be added to the virtio-snd device in the following
> commits.
>
> To add this device with a guest, you'll need a >=5.13 kernel compiled
> with CONFIG_SND_VIRTIO=y, which at the time of writing most distros have
> off by default.
>
> Use with following flags in the invocation:
>
> Pulseaudio:
> -audio driver=pa,model=virtio
> or
> -audio driver=pa,model=virtio,server=/run/user/1000/pulse/native
> sdl:
> -audio driver=sdl,model=virtio
> coreaudio (macos/darwin):
> -audio driver=coreaudio,model=virtio
> etc.
>
> Based-on: https://github.com/OpenSynergy/qemu/commit/5a2f350eec5d157b90d9c7b40a8e603f4da92471
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> 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/meson.build | 1 +
> hw/virtio/virtio-snd-pci.c | 97 ++++++++++++++++++++++++++++++++++++++
> softmmu/qdev-monitor.c | 1 +
> 3 files changed, 99 insertions(+)
> create mode 100644 hw/virtio/virtio-snd-pci.c
> diff --git a/hw/virtio/virtio-snd-pci.c b/hw/virtio/virtio-snd-pci.c
> new file mode 100644
> index 0000000000..a6a530d161
> --- /dev/null
> +++ b/hw/virtio/virtio-snd-pci.c
> @@ -0,0 +1,97 @@
> +/*
> + * VIRTIO Sound Device PCI Bindings
> + *
> + * Copyright (c) 2023 Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version. See the COPYING file in the
> + * top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
Hi Manos,
the macro definition of DECLARE_INSTANCE_CHECKER is in the qom/object.h
header file. It's better to include it directly than to rely on another
header file to include it.
> +#include "hw/audio/soundhw.h"
> +#include "hw/virtio/virtio-pci.h"
> +#include "hw/virtio/virtio-snd.h"
> +
> +typedef struct VirtIOSoundPCI VirtIOSoundPCI;
> +
> +/*
> + * virtio-snd-pci: This extends VirtioPCIProxy.
> + */
> +#define TYPE_VIRTIO_SND_PCI "virtio-sound-pci"
> +DECLARE_INSTANCE_CHECKER(VirtIOSoundPCI, VIRTIO_SND_PCI,
> + TYPE_VIRTIO_SND_PCI)
> +
> +struct VirtIOSoundPCI {
> + VirtIOPCIProxy parent;
> + VirtIOSound vdev;
> +};
> +
> +static Property virtio_snd_pci_properties[] = {
> + DEFINE_AUDIO_PROPERTIES(VirtIOSoundPCI, vdev.card),
I think DEFINE_AUDIO_PROPERTIES should be moved back to virtio-snd.c.
The audiodev property is a virtio-sound property and not a
virtio-sound-pci property.
> + DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
> + VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
> + DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
> + DEV_NVECTORS_UNSPECIFIED),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void virtio_snd_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> +{
> + VirtIOSoundPCI *dev = VIRTIO_SND_PCI(vpci_dev);
> + DeviceState *vdev = DEVICE(&dev->vdev);
> +
> + if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> + vpci_dev->nvectors = 2;
> + }
Why do you need that intermediate step with DEV_NVECTORS_UNSPECIFIED?
Unlike e.g. virtio-scsi-pci and virtio-net-pci devices, the default
value of nvectors is already known at compile time and can be specified
in the property definition.
With best regards,
Volker
> +
> + virtio_pci_force_virtio_1(vpci_dev);
> + qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> +}
> +
> +static void virtio_snd_pci_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + VirtioPCIClass *vpciklass = VIRTIO_PCI_CLASS(klass);
> +
> + device_class_set_props(dc, virtio_snd_pci_properties);
> + dc->desc = "Virtio Sound";
> + set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
> +
> + vpciklass->realize = virtio_snd_pci_realize;
> +}
> +
> +static void virtio_snd_pci_instance_init(Object *obj)
> +{
> + VirtIOSoundPCI *dev = VIRTIO_SND_PCI(obj);
> +
> + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> + TYPE_VIRTIO_SND);
> +}
> +
> +static const VirtioPCIDeviceTypeInfo virtio_snd_pci_info = {
> + .generic_name = TYPE_VIRTIO_SND_PCI,
> + .instance_size = sizeof(VirtIOSoundPCI),
> + .instance_init = virtio_snd_pci_instance_init,
> + .class_init = virtio_snd_pci_class_init,
> +};
> +
> +/* Create a Virtio Sound PCI device, so '-audio driver,model=virtio' works. */
> +static int virtio_snd_pci_init(PCIBus *bus, const char *audiodev)
> +{
> + DeviceState *dev;
> +
> + dev = qdev_new(TYPE_VIRTIO_SND_PCI);
> + qdev_prop_set_string(dev, "audiodev", audiodev);
> + qdev_realize_and_unref(dev, BUS(bus), &error_fatal);
> + return 0;
> +}
> +
> +static void virtio_snd_pci_register(void)
> +{
> + virtio_pci_types_register(&virtio_snd_pci_info);
> + pci_register_soundhw("virtio", "Virtio Sound", virtio_snd_pci_init);
> +}
> +
> +type_init(virtio_snd_pci_register);
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 74f4e41338..2e9835ad88 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -108,6 +108,7 @@ static const QDevAlias qdev_alias_table[] = {
> { "virtio-serial-device", "virtio-serial", QEMU_ARCH_VIRTIO_MMIO },
> { "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_VIRTIO_CCW },
> { "virtio-serial-pci", "virtio-serial", QEMU_ARCH_VIRTIO_PCI},
> + { "virtio-sound-pci", "virtio-sound", QEMU_ARCH_VIRTIO_PCI},
> { "virtio-tablet-device", "virtio-tablet", QEMU_ARCH_VIRTIO_MMIO },
> { "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_VIRTIO_CCW },
> { "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_VIRTIO_PCI },
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 02/12] Add virtio-sound-pci device
2023-09-04 6:32 ` Volker Rümelin
@ 2023-09-04 10:26 ` Manos Pitsidianakis
2023-09-04 21:08 ` Volker Rümelin
0 siblings, 1 reply; 51+ messages in thread
From: Manos Pitsidianakis @ 2023-09-04 10:26 UTC (permalink / raw)
To: Volker Rü melin, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
ichael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé ,
Eduardo Habkost, Marc-André Lureau,
Kő vá gó , Zoltá n, Alex Benné e,
Philippe Mathieu-Daudé
On Mon, 04 Sep 2023 09:32, Volker Rümelin <vr_qemu@t-online.de> wrote:
>> +static Property virtio_snd_pci_properties[] = {
>> + DEFINE_AUDIO_PROPERTIES(VirtIOSoundPCI, vdev.card),
>
>I think DEFINE_AUDIO_PROPERTIES should be moved back to virtio-snd.c.
>The audiodev property is a virtio-sound property and not a
>virtio-sound-pci property.
Hm, is it? Can you instantiate a virtio-sound device without the PCI
wrapper? Under hw/audio, DEFINE_AUDIO_PROPERTIES is set in PCI devices
as well (e.g. ac97)
>
>> + DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
>> + VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
>> + DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
>> + DEV_NVECTORS_UNSPECIFIED),
>> + DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void virtio_snd_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>> +{
>> + VirtIOSoundPCI *dev = VIRTIO_SND_PCI(vpci_dev);
>> + DeviceState *vdev = DEVICE(&dev->vdev);
>> +
>> + if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
>> + vpci_dev->nvectors = 2;
>> + }
>
>Why do you need that intermediate step with DEV_NVECTORS_UNSPECIFIED?
>Unlike e.g. virtio-scsi-pci and virtio-net-pci devices, the default
>value of nvectors is already known at compile time and can be specified
>in the property definition.
I did not think this through properly, you are correct. Thank you!
Manos
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 02/12] Add virtio-sound-pci device
2023-09-04 10:26 ` Manos Pitsidianakis
@ 2023-09-04 21:08 ` Volker Rümelin
0 siblings, 0 replies; 51+ messages in thread
From: Volker Rümelin @ 2023-09-04 21:08 UTC (permalink / raw)
To: Manos Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
Michael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau,
Kővágó , Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
Am 04.09.23 um 12:26 schrieb Manos Pitsidianakis:
> On Mon, 04 Sep 2023 09:32, Volker Rümelin <vr_qemu@t-online.de> wrote:
>>> +static Property virtio_snd_pci_properties[] = {
>>> + DEFINE_AUDIO_PROPERTIES(VirtIOSoundPCI, vdev.card),
>>
>> I think DEFINE_AUDIO_PROPERTIES should be moved back to virtio-snd.c.
>> The audiodev property is a virtio-sound property and not a
>> virtio-sound-pci property.
>
> Hm, is it? Can you instantiate a virtio-sound device without the PCI
> wrapper? Under hw/audio, DEFINE_AUDIO_PROPERTIES is set in PCI devices
> as well (e.g. ac97)
>
Creating a virtio-sound device without the PCI wrapper is possible.
./qemu-system-x86_64 -M microvm -accel kvm -cpu host -m 512m -smp 2
-serial stdio -device virtio-sound,audiodev=audio0 -audiodev
pipewire,id=audio0 -display gtk
qemu-system-x86_64: -device virtio-sound,audiodev=audio0: Property
'virtio-sound.audiodev' not found
If you move DEFINE_AUDIO_PROPERTIES to virtio-snd.c you don't see this
error message and you can see a virtio-mmio sound device if you type
info qtree in the QEMU compat monitor.
Now that you asked this question I wonder if this should be #define
TYPE_VIRTIO_SND "virtio-sound-device". Other virtio devices have the
-device suffix.
With best regards,
Volker
>>
>>> + DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
>>> + VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
>>> + DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
>>> + DEV_NVECTORS_UNSPECIFIED),
>>> + DEFINE_PROP_END_OF_LIST(),
>>> +};
>>> +
>>> +static void virtio_snd_pci_realize(VirtIOPCIProxy *vpci_dev, Error
>>> **errp)
>>> +{
>>> + VirtIOSoundPCI *dev = VIRTIO_SND_PCI(vpci_dev);
>>> + DeviceState *vdev = DEVICE(&dev->vdev);
>>> +
>>> + if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
>>> + vpci_dev->nvectors = 2;
>>> + }
>>
>> Why do you need that intermediate step with DEV_NVECTORS_UNSPECIFIED?
>> Unlike e.g. virtio-scsi-pci and virtio-net-pci devices, the default
>> value of nvectors is already known at compile time and can be
>> specified in the property definition.
>
> I did not think this through properly, you are correct. Thank you!
>
> Manos
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v8 02/12] Add virtio-sound-pci device
2023-08-28 19:54 ` [PATCH v8 02/12] Add virtio-sound-pci device Emmanouil Pitsidianakis
2023-09-04 6:32 ` Volker Rümelin
@ 2023-09-06 9:05 ` Mark Cave-Ayland
1 sibling, 0 replies; 51+ messages in thread
From: Mark Cave-Ayland @ 2023-09-06 9:05 UTC (permalink / raw)
To: Emmanouil Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
Michael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
On 28/08/2023 20:54, Emmanouil Pitsidianakis wrote:
Hi Emmanouil,
> This patch adds a PCI wrapper device for the virtio-sound device.
> It is necessary to instantiate a virtio-snd device in a guest.
> All sound logic will be added to the virtio-snd device in the following
> commits.
>
> To add this device with a guest, you'll need a >=5.13 kernel compiled
> with CONFIG_SND_VIRTIO=y, which at the time of writing most distros have
> off by default.
>
> Use with following flags in the invocation:
>
> Pulseaudio:
> -audio driver=pa,model=virtio
> or
> -audio driver=pa,model=virtio,server=/run/user/1000/pulse/native
> sdl:
> -audio driver=sdl,model=virtio
> coreaudio (macos/darwin):
> -audio driver=coreaudio,model=virtio
> etc.
>
> Based-on: https://github.com/OpenSynergy/qemu/commit/5a2f350eec5d157b90d9c7b40a8e603f4da92471
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> 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/meson.build | 1 +
> hw/virtio/virtio-snd-pci.c | 97 ++++++++++++++++++++++++++++++++++++++
> softmmu/qdev-monitor.c | 1 +
> 3 files changed, 99 insertions(+)
> create mode 100644 hw/virtio/virtio-snd-pci.c
>
> diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
> index 120d4bfa0a..5e5a83a4ee 100644
> --- a/hw/virtio/meson.build
> +++ b/hw/virtio/meson.build
> @@ -63,6 +63,7 @@ virtio_pci_ss.add(when: 'CONFIG_VIRTIO_SERIAL', if_true: files('virtio-serial-pc
> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem-pci.c'))
> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu-pci.c'))
> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem-pci.c'))
> +virtio_pci_ss.add(when: 'CONFIG_VIRTIO_SND', if_true: files('virtio-snd-pci.c'))
> virtio_pci_ss.add(when: 'CONFIG_VHOST_VDPA_DEV', if_true: files('vdpa-dev-pci.c'))
> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_MD', if_true: files('virtio-md-pci.c'))
>
> diff --git a/hw/virtio/virtio-snd-pci.c b/hw/virtio/virtio-snd-pci.c
> new file mode 100644
> index 0000000000..a6a530d161
> --- /dev/null
> +++ b/hw/virtio/virtio-snd-pci.c
> @@ -0,0 +1,97 @@
> +/*
> + * VIRTIO Sound Device PCI Bindings
> + *
> + * Copyright (c) 2023 Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version. See the COPYING file in the
> + * top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/audio/soundhw.h"
> +#include "hw/virtio/virtio-pci.h"
> +#include "hw/virtio/virtio-snd.h"
> +
> +typedef struct VirtIOSoundPCI VirtIOSoundPCI;
You can drop the typedef here.
> +/*
> + * virtio-snd-pci: This extends VirtioPCIProxy.
> + */
> +#define TYPE_VIRTIO_SND_PCI "virtio-sound-pci"
> +DECLARE_INSTANCE_CHECKER(VirtIOSoundPCI, VIRTIO_SND_PCI,
> + TYPE_VIRTIO_SND_PCI)
and you should be able to use OBJECT_DECLARE_SIMPLE_TYPE() here instead.
> +struct VirtIOSoundPCI {
> + VirtIOPCIProxy parent;
This should be parent_obj, followed by an empty line (see
https://qemu.readthedocs.io/en/master/devel/style.html#qemu-object-model-declarations).
> + VirtIOSound vdev;
> +};
> +
> +static Property virtio_snd_pci_properties[] = {
> + DEFINE_AUDIO_PROPERTIES(VirtIOSoundPCI, vdev.card),
> + DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
> + VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
> + DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
> + DEV_NVECTORS_UNSPECIFIED),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void virtio_snd_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> +{
> + VirtIOSoundPCI *dev = VIRTIO_SND_PCI(vpci_dev);
> + DeviceState *vdev = DEVICE(&dev->vdev);
> +
> + if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> + vpci_dev->nvectors = 2;
> + }
> +
> + virtio_pci_force_virtio_1(vpci_dev);
> + qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> +}
> +
> +static void virtio_snd_pci_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + VirtioPCIClass *vpciklass = VIRTIO_PCI_CLASS(klass);
> +
> + device_class_set_props(dc, virtio_snd_pci_properties);
> + dc->desc = "Virtio Sound";
> + set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
> +
> + vpciklass->realize = virtio_snd_pci_realize;
> +}
> +
> +static void virtio_snd_pci_instance_init(Object *obj)
> +{
> + VirtIOSoundPCI *dev = VIRTIO_SND_PCI(obj);
> +
> + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> + TYPE_VIRTIO_SND);
> +}
> +
> +static const VirtioPCIDeviceTypeInfo virtio_snd_pci_info = {
> + .generic_name = TYPE_VIRTIO_SND_PCI,
> + .instance_size = sizeof(VirtIOSoundPCI),
> + .instance_init = virtio_snd_pci_instance_init,
> + .class_init = virtio_snd_pci_class_init,
> +};
> +
> +/* Create a Virtio Sound PCI device, so '-audio driver,model=virtio' works. */
> +static int virtio_snd_pci_init(PCIBus *bus, const char *audiodev)
> +{
> + DeviceState *dev;
> +
> + dev = qdev_new(TYPE_VIRTIO_SND_PCI);
> + qdev_prop_set_string(dev, "audiodev", audiodev);
> + qdev_realize_and_unref(dev, BUS(bus), &error_fatal);
> + return 0;
> +}
> +
> +static void virtio_snd_pci_register(void)
> +{
> + virtio_pci_types_register(&virtio_snd_pci_info);
> + pci_register_soundhw("virtio", "Virtio Sound", virtio_snd_pci_init);
> +}
> +
> +type_init(virtio_snd_pci_register);
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 74f4e41338..2e9835ad88 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -108,6 +108,7 @@ static const QDevAlias qdev_alias_table[] = {
> { "virtio-serial-device", "virtio-serial", QEMU_ARCH_VIRTIO_MMIO },
> { "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_VIRTIO_CCW },
> { "virtio-serial-pci", "virtio-serial", QEMU_ARCH_VIRTIO_PCI},
> + { "virtio-sound-pci", "virtio-sound", QEMU_ARCH_VIRTIO_PCI},
> { "virtio-tablet-device", "virtio-tablet", QEMU_ARCH_VIRTIO_MMIO },
> { "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_VIRTIO_CCW },
> { "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_VIRTIO_PCI },
ATB,
Mark.
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v8 03/12] virtio-sound: handle control messages and streams
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
2023-08-28 19:54 ` [PATCH v8 01/12] Add virtio-sound device stub Emmanouil Pitsidianakis
2023-08-28 19:54 ` [PATCH v8 02/12] Add virtio-sound-pci device Emmanouil Pitsidianakis
@ 2023-08-28 19:55 ` Emmanouil Pitsidianakis
2023-09-04 10:08 ` Philippe Mathieu-Daudé
` (2 more replies)
2023-08-28 19:55 ` [PATCH v8 04/12] virtio-sound: set PCM stream parameters Emmanouil Pitsidianakis
` (11 subsequent siblings)
14 siblings, 3 replies; 51+ messages in thread
From: Emmanouil Pitsidianakis @ 2023-08-28 19:55 UTC (permalink / raw)
To: qemu-devel
Cc: Emmanouil Pitsidianakis, Igor Skalkin, Anton Yakovlev,
Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin,
Marcel Apfelbaum, Daniel P. Berrangé, Eduardo Habkost,
Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
Receive guest requests in the control (CTRL) queue of the virtio sound
device and reply with a NOT SUPPORTED error to all control commands.
The receiving handler is virtio_snd_handle_ctrl(). It stores all control
messages in the queue in the device's command queue. Then it calls
virtio_snd_process_cmdq() to handle each message.
The handler is process_cmd() which replies with VIRTIO_SND_S_NOT_SUPP.
Based-on: https://github.com/OpenSynergy/qemu/commit/5a2f350eec5d157b90d9c7b40a8e603f4da92471
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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 | 4 +
hw/virtio/virtio-snd.c | 227 ++++++++++++++++++++++++++++++++-
include/hw/virtio/virtio-snd.h | 70 +++++++++-
3 files changed, 292 insertions(+), 9 deletions(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 3ed7da35f2..8a223e36e9 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -163,3 +163,7 @@ virtio_snd_vm_state_running(void) "vm state running"
virtio_snd_vm_state_stopped(void) "vm state stopped"
virtio_snd_realize(void *snd) "snd %p: realize"
virtio_snd_unrealize(void *snd) "snd %p: unrealize"
+virtio_snd_handle_ctrl(void *vdev, void *vq) "snd %p: handle ctrl event for queue %p"
+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"
diff --git a/hw/virtio/virtio-snd.c b/hw/virtio/virtio-snd.c
index a056a7bcc6..b921903905 100644
--- a/hw/virtio/virtio-snd.c
+++ b/hw/virtio/virtio-snd.c
@@ -31,6 +31,29 @@
#define VIRTIO_SOUND_CHMAP_DEFAULT 0
#define VIRTIO_SOUND_HDA_FN_NID 0
+static const char *print_code(uint32_t code)
+{
+ #define CASE(CODE) \
+ case VIRTIO_SND_R_##CODE: \
+ return "VIRTIO_SND_R_"#CODE
+
+ switch (code) {
+ CASE(JACK_INFO);
+ CASE(JACK_REMAP);
+ CASE(PCM_INFO);
+ CASE(PCM_SET_PARAMS);
+ CASE(PCM_PREPARE);
+ CASE(PCM_RELEASE);
+ CASE(PCM_START);
+ CASE(PCM_STOP);
+ CASE(CHMAP_INFO);
+ default:
+ return "invalid code";
+ }
+
+ #undef CASE
+};
+
static const VMStateDescription vmstate_virtio_snd_device = {
.name = TYPE_VIRTIO_SND,
.version_id = VIRTIO_SOUND_VM_VERSION,
@@ -89,12 +112,148 @@ virtio_snd_set_config(VirtIODevice *vdev, const uint8_t *config)
}
/*
- * Queue handler stub.
+ * The actual processing done in virtio_snd_process_cmdq().
+ *
+ * @s: VirtIOSound device
+ * @cmd: control command request
+ */
+static inline void
+process_cmd(VirtIOSound *s, virtio_snd_ctrl_command *cmd)
+{
+ size_t sz = iov_to_buf(cmd->elem->out_sg,
+ cmd->elem->out_num,
+ 0,
+ &cmd->ctrl,
+ sizeof(cmd->ctrl));
+ if (sz != sizeof(cmd->ctrl)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: virtio-snd command size incorrect %zu vs \
+ %zu\n", __func__, sz, sizeof(cmd->ctrl));
+ return;
+ }
+
+ trace_virtio_snd_handle_code(cmd->ctrl.code,
+ print_code(cmd->ctrl.code));
+
+ switch (cmd->ctrl.code) {
+ case VIRTIO_SND_R_JACK_INFO:
+ case VIRTIO_SND_R_JACK_REMAP:
+ qemu_log_mask(LOG_UNIMP,
+ "virtio_snd: jack functionality is unimplemented.");
+ cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
+ break;
+ case VIRTIO_SND_R_PCM_INFO:
+ case VIRTIO_SND_R_PCM_SET_PARAMS:
+ case VIRTIO_SND_R_PCM_PREPARE:
+ case VIRTIO_SND_R_PCM_START:
+ case VIRTIO_SND_R_PCM_STOP:
+ case VIRTIO_SND_R_PCM_RELEASE:
+ cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
+ break;
+ case VIRTIO_SND_R_CHMAP_INFO:
+ qemu_log_mask(LOG_UNIMP,
+ "virtio_snd: chmap info functionality is unimplemented.");
+ trace_virtio_snd_handle_chmap_info();
+ cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
+ break;
+ default:
+ /* error */
+ error_report("virtio snd header not recognized: %"PRIu32,
+ cmd->ctrl.code);
+ cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
+ }
+
+ iov_from_buf(cmd->elem->in_sg,
+ cmd->elem->in_num,
+ 0,
+ &cmd->resp,
+ sizeof(cmd->resp));
+ virtqueue_push(cmd->vq, cmd->elem, sizeof(cmd->elem));
+ virtio_notify(VIRTIO_DEVICE(s), cmd->vq);
+}
+
+/*
+ * Consume all elements in command queue.
+ *
+ * @s: VirtIOSound device
+ */
+static void virtio_snd_process_cmdq(VirtIOSound *s)
+{
+ virtio_snd_ctrl_command *cmd;
+
+ if (unlikely(qatomic_read(&s->processing_cmdq))) {
+ return;
+ }
+
+ WITH_QEMU_LOCK_GUARD(&s->cmdq_mutex) {
+ qatomic_set(&s->processing_cmdq, true);
+ while (!QTAILQ_EMPTY(&s->cmdq)) {
+ cmd = QTAILQ_FIRST(&s->cmdq);
+
+ /* process command */
+ process_cmd(s, cmd);
+
+ QTAILQ_REMOVE(&s->cmdq, cmd, next);
+
+ g_free(cmd);
+ }
+ qatomic_set(&s->processing_cmdq, false);
+ }
+}
+
+/*
+ * The control message handler. Pops an element from the control virtqueue,
+ * and stores them to VirtIOSound's cmdq queue and finally calls
+ * virtio_snd_process_cmdq() for processing.
+ *
+ * @vdev: VirtIOSound device
+ * @vq: Control virtqueue
+ */
+static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+{
+ VirtIOSound *s = VIRTIO_SND(vdev);
+ VirtQueueElement *elem;
+ virtio_snd_ctrl_command *cmd;
+
+ trace_virtio_snd_handle_ctrl(vdev, vq);
+
+ if (!virtio_queue_ready(vq)) {
+ return;
+ }
+
+ elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+ while (elem) {
+ cmd = g_new0(virtio_snd_ctrl_command, 1);
+ cmd->elem = elem;
+ cmd->vq = vq;
+ cmd->resp.code = VIRTIO_SND_S_OK;
+ QTAILQ_INSERT_TAIL(&s->cmdq, cmd, next);
+ elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+ }
+
+ virtio_snd_process_cmdq(s);
+}
+
+/*
+ * The event virtqueue handler.
+ * Not implemented yet.
+ *
+ * @vdev: VirtIOSound device
+ * @vq: event vq
+ */
+static void virtio_snd_handle_event(VirtIODevice *vdev, VirtQueue *vq)
+{
+ qemu_log_mask(LOG_UNIMP, "virtio_snd: event queue is unimplemented.");
+ trace_virtio_snd_handle_event();
+}
+
+/*
+ * Stub buffer virtqueue handler.
*
* @vdev: VirtIOSound device
* @vq: virtqueue
*/
-static void virtio_snd_handle_queue(VirtIODevice *vdev, VirtQueue *vq) {}
+static void virtio_snd_handle_xfer(VirtIODevice *vdev, VirtQueue *vq) {}
static uint64_t get_features(VirtIODevice *vdev, uint64_t features,
Error **errp)
@@ -123,17 +282,32 @@ virtio_snd_vm_state_change(void *opaque, bool running,
}
}
+static void virtio_snd_set_pcm(VirtIOSound *snd)
+{
+ VirtIOSoundPCM *pcm;
+
+ pcm = g_new0(VirtIOSoundPCM, 1);
+ pcm->snd = snd;
+ pcm->streams = g_new0(VirtIOSoundPCMStream *, snd->snd_conf.streams);
+ pcm->pcm_params = g_new0(VirtIOSoundPCMParams, snd->snd_conf.streams);
+
+ snd->pcm = pcm;
+}
+
static void virtio_snd_realize(DeviceState *dev, Error **errp)
{
ERRP_GUARD();
VirtIOSound *vsnd = VIRTIO_SND(dev);
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ vsnd->pcm = NULL;
vsnd->vmstate =
qemu_add_vm_change_state_handler(virtio_snd_vm_state_change, vsnd);
trace_virtio_snd_realize(vsnd);
+ virtio_snd_set_pcm(vsnd);
+
virtio_init(vdev, VIRTIO_ID_SOUND, sizeof(virtio_snd_config));
virtio_add_feature(&vsnd->features, VIRTIO_F_VERSION_1);
@@ -161,31 +335,70 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp)
AUD_register_card("virtio-sound", &vsnd->card);
vsnd->queues[VIRTIO_SND_VQ_CONTROL] =
- virtio_add_queue(vdev, 64, virtio_snd_handle_queue);
+ virtio_add_queue(vdev, 64, virtio_snd_handle_ctrl);
vsnd->queues[VIRTIO_SND_VQ_EVENT] =
- virtio_add_queue(vdev, 64, virtio_snd_handle_queue);
+ virtio_add_queue(vdev, 64, virtio_snd_handle_event);
vsnd->queues[VIRTIO_SND_VQ_TX] =
- virtio_add_queue(vdev, 64, virtio_snd_handle_queue);
+ virtio_add_queue(vdev, 64, virtio_snd_handle_xfer);
vsnd->queues[VIRTIO_SND_VQ_RX] =
- virtio_add_queue(vdev, 64, virtio_snd_handle_queue);
+ virtio_add_queue(vdev, 64, virtio_snd_handle_xfer);
+ qemu_mutex_init(&vsnd->cmdq_mutex);
+ QTAILQ_INIT(&vsnd->cmdq);
+}
+
+/*
+ * Close the stream and free its resources.
+ *
+ * @stream: VirtIOSoundPCMStream *stream
+ */
+static void virtio_snd_pcm_close(VirtIOSoundPCMStream *stream)
+{
}
static void virtio_snd_unrealize(DeviceState *dev)
{
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VirtIOSound *vsnd = VIRTIO_SND(dev);
+ VirtIOSoundPCMStream *stream;
qemu_del_vm_change_state_handler(vsnd->vmstate);
virtio_del_queue(vdev, 0);
trace_virtio_snd_unrealize(vsnd);
+ if (vsnd->pcm) {
+ if (vsnd->pcm->streams) {
+ for (uint32_t i = 0; i < vsnd->snd_conf.streams; i++) {
+ stream = vsnd->pcm->streams[i];
+ if (stream) {
+ virtio_snd_process_cmdq(stream->s);
+ virtio_snd_pcm_close(stream);
+ g_free(stream);
+ }
+ }
+ g_free(vsnd->pcm->streams);
+ }
+ g_free(vsnd->pcm);
+ vsnd->pcm = NULL;
+ }
AUD_remove_card(&vsnd->card);
virtio_cleanup(vdev);
}
-static void virtio_snd_reset(VirtIODevice *vdev) {}
+static void virtio_snd_reset(VirtIODevice *vdev)
+{
+ VirtIOSound *s = VIRTIO_SND(vdev);
+ virtio_snd_ctrl_command *cmd;
+
+ WITH_QEMU_LOCK_GUARD(&s->cmdq_mutex) {
+ while (!QTAILQ_EMPTY(&s->cmdq)) {
+ cmd = QTAILQ_FIRST(&s->cmdq);
+ QTAILQ_REMOVE(&s->cmdq, cmd, next);
+ g_free(cmd);
+ }
+ }
+}
static void virtio_snd_class_init(ObjectClass *klass, void *data)
{
diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
index b3c0e6f079..b7046418cf 100644
--- a/include/hw/virtio/virtio-snd.h
+++ b/include/hw/virtio/virtio-snd.h
@@ -67,12 +67,78 @@ typedef struct virtio_snd_pcm_xfer virtio_snd_pcm_xfer;
/* I/O request status */
typedef struct virtio_snd_pcm_status virtio_snd_pcm_status;
-typedef struct VirtIOSound {
+/* device structs */
+
+typedef struct VirtIOSound VirtIOSound;
+
+typedef struct VirtIOSoundPCMStream VirtIOSoundPCMStream;
+
+typedef struct virtio_snd_ctrl_command virtio_snd_ctrl_command;
+
+typedef struct VirtIOSoundPCMParams VirtIOSoundPCMParams;
+
+typedef struct VirtIOSoundPCM VirtIOSoundPCM;
+
+/* Stream params */
+struct VirtIOSoundPCMParams {
+ uint32_t features;
+ uint32_t buffer_bytes; /* size of hardware buffer in bytes */
+ uint32_t period_bytes; /* size of hardware period in bytes */
+ uint8_t channels;
+ uint8_t format;
+ uint8_t rate;
+};
+
+struct VirtIOSoundPCM {
+ VirtIOSound *snd;
+ VirtIOSoundPCMParams *pcm_params;
+ VirtIOSoundPCMStream **streams;
+};
+
+struct VirtIOSoundPCMStream {
+ VirtIOSoundPCM *pcm;
+ virtio_snd_pcm_info info;
+ uint32_t id;
+ uint32_t buffer_bytes;
+ uint32_t period_bytes;
+ /* channel position values (VIRTIO_SND_CHMAP_XXX) */
+ uint8_t positions[VIRTIO_SND_CHMAP_MAX_SIZE];
+ VirtIOSound *s;
+ uint32_t features; /* 1 << VIRTIO_SND_PCM_F_XXX */
+ uint64_t formats; /* 1 << VIRTIO_SND_PCM_FMT_XXX */
+ uint64_t rates; /* 1 << VIRTIO_SND_PCM_RATE_XXX */
+ uint8_t direction;
+ uint8_t channels_min;
+ uint8_t channels_max;
+ bool flushing;
+ audsettings as;
+ audsettings desired_as;
+ union {
+ SWVoiceIn *in;
+ SWVoiceOut *out;
+ } voice;
+ QemuMutex queue_mutex;
+ QSIMPLEQ_HEAD(, VirtIOSoundPCMBlock) queue;
+};
+
+struct VirtIOSound {
VirtIODevice parent_obj;
VirtQueue *queues[VIRTIO_SND_VQ_MAX];
uint64_t features;
+ VirtIOSoundPCM *pcm;
QEMUSoundCard card;
VMChangeStateEntry *vmstate;
virtio_snd_config snd_conf;
-} VirtIOSound;
+ QemuMutex cmdq_mutex;
+ QTAILQ_HEAD(, virtio_snd_ctrl_command) cmdq;
+ bool processing_cmdq;
+};
+
+struct virtio_snd_ctrl_command {
+ VirtQueueElement *elem;
+ VirtQueue *vq;
+ virtio_snd_hdr ctrl;
+ virtio_snd_hdr resp;
+ QTAILQ_ENTRY(virtio_snd_ctrl_command) next;
+};
#endif
--
2.39.2
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v8 03/12] virtio-sound: handle control messages and streams
2023-08-28 19:55 ` [PATCH v8 03/12] virtio-sound: handle control messages and streams Emmanouil Pitsidianakis
@ 2023-09-04 10:08 ` Philippe Mathieu-Daudé
2023-09-04 10:18 ` Manos Pitsidianakis
2023-09-04 10:46 ` Philippe Mathieu-Daudé
2023-09-06 9:29 ` Mark Cave-Ayland
2 siblings, 1 reply; 51+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-09-04 10:08 UTC (permalink / raw)
To: Emmanouil Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
Michael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée
Hi Emmanouil,
On 28/8/23 21:55, Emmanouil Pitsidianakis wrote:
> Receive guest requests in the control (CTRL) queue of the virtio sound
> device and reply with a NOT SUPPORTED error to all control commands.
>
> The receiving handler is virtio_snd_handle_ctrl(). It stores all control
> messages in the queue in the device's command queue. Then it calls
> virtio_snd_process_cmdq() to handle each message.
>
> The handler is process_cmd() which replies with VIRTIO_SND_S_NOT_SUPP.
>
> Based-on: https://github.com/OpenSynergy/qemu/commit/5a2f350eec5d157b90d9c7b40a8e603f4da92471
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> 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 | 4 +
> hw/virtio/virtio-snd.c | 227 ++++++++++++++++++++++++++++++++-
> include/hw/virtio/virtio-snd.h | 70 +++++++++-
> 3 files changed, 292 insertions(+), 9 deletions(-)
> /*
> - * Queue handler stub.
> + * The actual processing done in virtio_snd_process_cmdq().
> + *
> + * @s: VirtIOSound device
> + * @cmd: control command request
> + */
> +static inline void
> +process_cmd(VirtIOSound *s, virtio_snd_ctrl_command *cmd)
> +{
> + size_t sz = iov_to_buf(cmd->elem->out_sg,
> + cmd->elem->out_num,
> + 0,
> + &cmd->ctrl,
> + sizeof(cmd->ctrl));
> + if (sz != sizeof(cmd->ctrl)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: virtio-snd command size incorrect %zu vs \
> + %zu\n", __func__, sz, sizeof(cmd->ctrl));
> + return;
> + }
> +
> + trace_virtio_snd_handle_code(cmd->ctrl.code,
> + print_code(cmd->ctrl.code));
> +
> + switch (cmd->ctrl.code) {
> + case VIRTIO_SND_R_JACK_INFO:
> + case VIRTIO_SND_R_JACK_REMAP:
> + qemu_log_mask(LOG_UNIMP,
> + "virtio_snd: jack functionality is unimplemented.");
> + cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
> + break;
> + case VIRTIO_SND_R_PCM_INFO:
> + case VIRTIO_SND_R_PCM_SET_PARAMS:
> + case VIRTIO_SND_R_PCM_PREPARE:
> + case VIRTIO_SND_R_PCM_START:
> + case VIRTIO_SND_R_PCM_STOP:
> + case VIRTIO_SND_R_PCM_RELEASE:
> + cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
> + break;
> + case VIRTIO_SND_R_CHMAP_INFO:
> + qemu_log_mask(LOG_UNIMP,
> + "virtio_snd: chmap info functionality is unimplemented.");
> + trace_virtio_snd_handle_chmap_info();
> + cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
> + break;
> + default:
> + /* error */
> + error_report("virtio snd header not recognized: %"PRIu32,
> + cmd->ctrl.code);
> + cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
> + }
> +
> + iov_from_buf(cmd->elem->in_sg,
> + cmd->elem->in_num,
> + 0,
> + &cmd->resp,
> + sizeof(cmd->resp));
> + virtqueue_push(cmd->vq, cmd->elem, sizeof(cmd->elem));
> + virtio_notify(VIRTIO_DEVICE(s), cmd->vq);
I have very few understanding of virtio, but I'm wondering here,
since this function is called under cmdq_mutex(), could it be
useful to batch the queue by calling virtio_notify() only once
in the caller once the whole cmdq is processed ...
> +}
> +
> +/*
> + * Consume all elements in command queue.
> + *
> + * @s: VirtIOSound device
> + */
> +static void virtio_snd_process_cmdq(VirtIOSound *s)
> +{
> + virtio_snd_ctrl_command *cmd;
> +
> + if (unlikely(qatomic_read(&s->processing_cmdq))) {
> + return;
> + }
> +
> + WITH_QEMU_LOCK_GUARD(&s->cmdq_mutex) {
> + qatomic_set(&s->processing_cmdq, true);
> + while (!QTAILQ_EMPTY(&s->cmdq)) {
> + cmd = QTAILQ_FIRST(&s->cmdq);
> +
> + /* process command */
> + process_cmd(s, cmd);
> +
> + QTAILQ_REMOVE(&s->cmdq, cmd, next);
> +
> + g_free(cmd);
> + }
... here?
> + qatomic_set(&s->processing_cmdq, false);
> + }
> +}
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 03/12] virtio-sound: handle control messages and streams
2023-09-04 10:08 ` Philippe Mathieu-Daudé
@ 2023-09-04 10:18 ` Manos Pitsidianakis
2023-09-04 10:42 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 51+ messages in thread
From: Manos Pitsidianakis @ 2023-09-04 10:18 UTC (permalink / raw)
To: Philippe Mathieu-Daudé , qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
ichael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé ,
Eduardo Habkost, Marc-André Lureau, Volker Rü melin,
Kő vá gó , Zoltá n, Alex Benné e
Good morning Philippe,
On Mon, 04 Sep 2023 13:08, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>> + iov_from_buf(cmd->elem->in_sg,
>> + cmd->elem->in_num,
>> + 0,
>> + &cmd->resp,
>> + sizeof(cmd->resp));
>> + virtqueue_push(cmd->vq, cmd->elem, sizeof(cmd->elem));
>> + virtio_notify(VIRTIO_DEVICE(s), cmd->vq);
>
>I have very few understanding of virtio, but I'm wondering here,
>since this function is called under cmdq_mutex(), could it be
>useful to batch the queue by calling virtio_notify() only once
>in the caller once the whole cmdq is processed ...
In the linux driver (sound/virtio/virtio_ctl_msg.c), the guest has a
timeout for receiving the message. I found that if I did not notify as
fast as possible, I got timeout errors on the guest.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v8 03/12] virtio-sound: handle control messages and streams
2023-09-04 10:18 ` Manos Pitsidianakis
@ 2023-09-04 10:42 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 51+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-09-04 10:42 UTC (permalink / raw)
To: Manos Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
ichael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-Andr é Lureau, Volker R ü melin,
Kő vá gó , Zoltá n, Alex Benn é e
On 4/9/23 12:18, Manos Pitsidianakis wrote:
> Good morning Philippe,
>
> On Mon, 04 Sep 2023 13:08, Philippe Mathieu-Daudé <philmd@linaro.org>
> wrote:
>>> + iov_from_buf(cmd->elem->in_sg,
>>> + cmd->elem->in_num,
>>> + 0,
>>> + &cmd->resp,
>>> + sizeof(cmd->resp));
>>> + virtqueue_push(cmd->vq, cmd->elem, sizeof(cmd->elem));
>>> + virtio_notify(VIRTIO_DEVICE(s), cmd->vq);
>>
>> I have very few understanding of virtio, but I'm wondering here,
>> since this function is called under cmdq_mutex(), could it be
>> useful to batch the queue by calling virtio_notify() only once
>> in the caller once the whole cmdq is processed ...
>
> In the linux driver (sound/virtio/virtio_ctl_msg.c), the guest has a
> timeout for receiving the message. I found that if I did not notify as
> fast as possible, I got timeout errors on the guest.
Ah, I see, 1ms per default:
sound/virtio/virtio_card.c:14:u32 virtsnd_msg_timeout_ms = MSEC_PER_SEC;
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v8 03/12] virtio-sound: handle control messages and streams
2023-08-28 19:55 ` [PATCH v8 03/12] virtio-sound: handle control messages and streams Emmanouil Pitsidianakis
2023-09-04 10:08 ` Philippe Mathieu-Daudé
@ 2023-09-04 10:46 ` Philippe Mathieu-Daudé
2023-09-04 11:00 ` Manos Pitsidianakis
2023-09-06 9:29 ` Mark Cave-Ayland
2 siblings, 1 reply; 51+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-09-04 10:46 UTC (permalink / raw)
To: Emmanouil Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
Michael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée
On 28/8/23 21:55, Emmanouil Pitsidianakis wrote:
> Receive guest requests in the control (CTRL) queue of the virtio sound
> device and reply with a NOT SUPPORTED error to all control commands.
>
> The receiving handler is virtio_snd_handle_ctrl(). It stores all control
> messages in the queue in the device's command queue. Then it calls
> virtio_snd_process_cmdq() to handle each message.
>
> The handler is process_cmd() which replies with VIRTIO_SND_S_NOT_SUPP.
>
> Based-on: https://github.com/OpenSynergy/qemu/commit/5a2f350eec5d157b90d9c7b40a8e603f4da92471
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> 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 | 4 +
> hw/virtio/virtio-snd.c | 227 ++++++++++++++++++++++++++++++++-
> include/hw/virtio/virtio-snd.h | 70 +++++++++-
> 3 files changed, 292 insertions(+), 9 deletions(-)
> /*
> - * Queue handler stub.
> + * The actual processing done in virtio_snd_process_cmdq().
> + *
> + * @s: VirtIOSound device
> + * @cmd: control command request
> + */
> +static inline void
> +process_cmd(VirtIOSound *s, virtio_snd_ctrl_command *cmd)
> +{
> + size_t sz = iov_to_buf(cmd->elem->out_sg,
> + cmd->elem->out_num,
> + 0,
> + &cmd->ctrl,
> + sizeof(cmd->ctrl));
> + if (sz != sizeof(cmd->ctrl)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: virtio-snd command size incorrect %zu vs \
> + %zu\n", __func__, sz, sizeof(cmd->ctrl));
> + return;
> + }
> +
> + trace_virtio_snd_handle_code(cmd->ctrl.code,
IIUC the spec, this structure is in little endian, is that right?
So shouldn't swap various fields in this series?
> + print_code(cmd->ctrl.code));
> +
> + switch (cmd->ctrl.code) {
> + case VIRTIO_SND_R_JACK_INFO:
> + case VIRTIO_SND_R_JACK_REMAP:
> + qemu_log_mask(LOG_UNIMP,
> + "virtio_snd: jack functionality is unimplemented.");
> + cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
> + break;
> + case VIRTIO_SND_R_PCM_INFO:
> + case VIRTIO_SND_R_PCM_SET_PARAMS:
> + case VIRTIO_SND_R_PCM_PREPARE:
> + case VIRTIO_SND_R_PCM_START:
> + case VIRTIO_SND_R_PCM_STOP:
> + case VIRTIO_SND_R_PCM_RELEASE:
> + cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
> + break;
> + case VIRTIO_SND_R_CHMAP_INFO:
> + qemu_log_mask(LOG_UNIMP,
> + "virtio_snd: chmap info functionality is unimplemented.");
> + trace_virtio_snd_handle_chmap_info();
> + cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
> + break;
> + default:
> + /* error */
> + error_report("virtio snd header not recognized: %"PRIu32,
> + cmd->ctrl.code);
> + cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
> + }
> +
> + iov_from_buf(cmd->elem->in_sg,
> + cmd->elem->in_num,
> + 0,
> + &cmd->resp,
> + sizeof(cmd->resp));
> + virtqueue_push(cmd->vq, cmd->elem, sizeof(cmd->elem));
> + virtio_notify(VIRTIO_DEVICE(s), cmd->vq);
> +}
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 03/12] virtio-sound: handle control messages and streams
2023-09-04 10:46 ` Philippe Mathieu-Daudé
@ 2023-09-04 11:00 ` Manos Pitsidianakis
2023-09-04 11:30 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 51+ messages in thread
From: Manos Pitsidianakis @ 2023-09-04 11:00 UTC (permalink / raw)
To: Philippe Mathieu-Daudé , qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
ichael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé ,
Eduardo Habkost, Marc-André Lureau, Volker Rü melin,
Kő vá gó , Zoltá n, Alex Benné e
On Mon, 04 Sep 2023 13:46, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>> + size_t sz = iov_to_buf(cmd->elem->out_sg,
>> + cmd->elem->out_num,
>> + 0,
>> + &cmd->ctrl,
>> + sizeof(cmd->ctrl));
>> + if (sz != sizeof(cmd->ctrl)) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: virtio-snd command size incorrect %zu vs \
>> + %zu\n", __func__, sz, sizeof(cmd->ctrl));
>> + return;
>> + }
>> +
>> + trace_virtio_snd_handle_code(cmd->ctrl.code,
>
>IIUC the spec, this structure is in little endian, is that right?
>So shouldn't swap various fields in this series?
Not sure about the answer to this. Need input from someone more
knowledgeable in virtio.
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 03/12] virtio-sound: handle control messages and streams
2023-09-04 11:00 ` Manos Pitsidianakis
@ 2023-09-04 11:30 ` Philippe Mathieu-Daudé
2023-09-04 11:46 ` Manos Pitsidianakis
0 siblings, 1 reply; 51+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-09-04 11:30 UTC (permalink / raw)
To: Manos Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
ichael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-Andr é Lureau, Volker R ü melin,
Kő vá gó , Zoltá n, Alex Benn é e
On 4/9/23 13:00, Manos Pitsidianakis wrote:
> On Mon, 04 Sep 2023 13:46, Philippe Mathieu-Daudé <philmd@linaro.org>
> wrote:
>>> + size_t sz = iov_to_buf(cmd->elem->out_sg,
>>> + cmd->elem->out_num,
>>> + 0,
>>> + &cmd->ctrl,
>>> + sizeof(cmd->ctrl));
>>> + if (sz != sizeof(cmd->ctrl)) {
>>> + qemu_log_mask(LOG_GUEST_ERROR,
>>> + "%s: virtio-snd command size incorrect %zu vs \
>>> + %zu\n", __func__, sz, sizeof(cmd->ctrl));
>>> + return;
>>> + }
>>> +
>>> + trace_virtio_snd_handle_code(cmd->ctrl.code,
>>
>> IIUC the spec, this structure is in little endian, is that right?
>> So shouldn't swap various fields in this series?
>
> Not sure about the answer to this. Need input from someone more
> knowledgeable in virtio.
You can test running a big-endian guest (m68k, s390x, sparc64) on your
little-endian host.
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 03/12] virtio-sound: handle control messages and streams
2023-09-04 11:30 ` Philippe Mathieu-Daudé
@ 2023-09-04 11:46 ` Manos Pitsidianakis
2023-09-04 12:17 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 51+ messages in thread
From: Manos Pitsidianakis @ 2023-09-04 11:46 UTC (permalink / raw)
To: Philippe Mathieu-Daudé , qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
chael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé ,
Eduardo Habkost, Marc-Andr é Lureau, Volker R ü melin,
Kő vá gó , Zoltá n, Alex Benn é e
On Mon, 04 Sep 2023 14:30, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>On 4/9/23 13:00, Manos Pitsidianakis wrote:
>> On Mon, 04 Sep 2023 13:46, Philippe Mathieu-Daudé <philmd@linaro.org>
>> wrote:
>>>> + size_t sz = iov_to_buf(cmd->elem->out_sg,
>>>> + cmd->elem->out_num,
>>>> + 0,
>>>> + &cmd->ctrl,
>>>> + sizeof(cmd->ctrl));
>>>> + if (sz != sizeof(cmd->ctrl)) {
>>>> + qemu_log_mask(LOG_GUEST_ERROR,
>>>> + "%s: virtio-snd command size incorrect %zu vs \
>>>> + %zu\n", __func__, sz, sizeof(cmd->ctrl));
>>>> + return;
>>>> + }
>>>> +
>>>> + trace_virtio_snd_handle_code(cmd->ctrl.code,
>>>
>>> IIUC the spec, this structure is in little endian, is that right?
>>> So shouldn't swap various fields in this series?
>>
>> Not sure about the answer to this. Need input from someone more
>> knowledgeable in virtio.
>
>You can test running a big-endian guest (m68k, s390x, sparc64) on your
>little-endian host.
The linux driver uses le{32,64}_to_cpu for reading fields, if there's
any problem then it would be with the host?
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 03/12] virtio-sound: handle control messages and streams
2023-09-04 11:46 ` Manos Pitsidianakis
@ 2023-09-04 12:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 51+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-09-04 12:17 UTC (permalink / raw)
To: Manos Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
chael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-Andr é Lureau, Volker R ü melin,
Kő vá gó , Zoltá n, Alex Benn é e
On 4/9/23 13:46, Manos Pitsidianakis wrote:
> On Mon, 04 Sep 2023 14:30, Philippe Mathieu-Daudé <philmd@linaro.org>
> wrote:
>> On 4/9/23 13:00, Manos Pitsidianakis wrote:
>>> On Mon, 04 Sep 2023 13:46, Philippe Mathieu-Daudé <philmd@linaro.org>
>>> wrote:
>>>>> + size_t sz = iov_to_buf(cmd->elem->out_sg,
>>>>> + cmd->elem->out_num,
>>>>> + 0,
>>>>> + &cmd->ctrl,
>>>>> + sizeof(cmd->ctrl));
>>>>> + if (sz != sizeof(cmd->ctrl)) {
>>>>> + qemu_log_mask(LOG_GUEST_ERROR,
>>>>> + "%s: virtio-snd command size incorrect %zu vs \
>>>>> + %zu\n", __func__, sz, sizeof(cmd->ctrl));
>>>>> + return;
>>>>> + }
>>>>> +
>>>>> + trace_virtio_snd_handle_code(cmd->ctrl.code,
>>>>
>>>> IIUC the spec, this structure is in little endian, is that right?
>>>> So shouldn't swap various fields in this series?
>>>
>>> Not sure about the answer to this. Need input from someone more
>>> knowledgeable in virtio.
>>
>> You can test running a big-endian guest (m68k, s390x, sparc64) on your
>> little-endian host.
>
> The linux driver uses le{32,64}_to_cpu for reading fields, if there's
> any problem then it would be with the host?
Ah right. Since using a BE guest is often simpler for most developers
than accessing a BE host -- in particular to test recent kernel -- I was
hoping this would be an easy enough suggestion.
Anyhow Linux swapping the fields confirms the virtio-sound model has to
do the same.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v8 03/12] virtio-sound: handle control messages and streams
2023-08-28 19:55 ` [PATCH v8 03/12] virtio-sound: handle control messages and streams Emmanouil Pitsidianakis
2023-09-04 10:08 ` Philippe Mathieu-Daudé
2023-09-04 10:46 ` Philippe Mathieu-Daudé
@ 2023-09-06 9:29 ` Mark Cave-Ayland
2 siblings, 0 replies; 51+ messages in thread
From: Mark Cave-Ayland @ 2023-09-06 9:29 UTC (permalink / raw)
To: Emmanouil Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
Michael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
On 28/08/2023 20:55, Emmanouil Pitsidianakis wrote:
Hi Emmanouil,
> Receive guest requests in the control (CTRL) queue of the virtio sound
> device and reply with a NOT SUPPORTED error to all control commands.
>
> The receiving handler is virtio_snd_handle_ctrl(). It stores all control
> messages in the queue in the device's command queue. Then it calls
> virtio_snd_process_cmdq() to handle each message.
>
> The handler is process_cmd() which replies with VIRTIO_SND_S_NOT_SUPP.
>
> Based-on: https://github.com/OpenSynergy/qemu/commit/5a2f350eec5d157b90d9c7b40a8e603f4da92471
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> 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 | 4 +
> hw/virtio/virtio-snd.c | 227 ++++++++++++++++++++++++++++++++-
> include/hw/virtio/virtio-snd.h | 70 +++++++++-
> 3 files changed, 292 insertions(+), 9 deletions(-)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 3ed7da35f2..8a223e36e9 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -163,3 +163,7 @@ virtio_snd_vm_state_running(void) "vm state running"
> virtio_snd_vm_state_stopped(void) "vm state stopped"
> virtio_snd_realize(void *snd) "snd %p: realize"
> virtio_snd_unrealize(void *snd) "snd %p: unrealize"
> +virtio_snd_handle_ctrl(void *vdev, void *vq) "snd %p: handle ctrl event for queue %p"
> +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"
> diff --git a/hw/virtio/virtio-snd.c b/hw/virtio/virtio-snd.c
> index a056a7bcc6..b921903905 100644
> --- a/hw/virtio/virtio-snd.c
> +++ b/hw/virtio/virtio-snd.c
> @@ -31,6 +31,29 @@
> #define VIRTIO_SOUND_CHMAP_DEFAULT 0
> #define VIRTIO_SOUND_HDA_FN_NID 0
>
> +static const char *print_code(uint32_t code)
> +{
> + #define CASE(CODE) \
> + case VIRTIO_SND_R_##CODE: \
> + return "VIRTIO_SND_R_"#CODE
> +
> + switch (code) {
> + CASE(JACK_INFO);
> + CASE(JACK_REMAP);
> + CASE(PCM_INFO);
> + CASE(PCM_SET_PARAMS);
> + CASE(PCM_PREPARE);
> + CASE(PCM_RELEASE);
> + CASE(PCM_START);
> + CASE(PCM_STOP);
> + CASE(CHMAP_INFO);
> + default:
> + return "invalid code";
> + }
> +
> + #undef CASE
> +};
> +
> static const VMStateDescription vmstate_virtio_snd_device = {
> .name = TYPE_VIRTIO_SND,
> .version_id = VIRTIO_SOUND_VM_VERSION,
> @@ -89,12 +112,148 @@ virtio_snd_set_config(VirtIODevice *vdev, const uint8_t *config)
> }
>
> /*
> - * Queue handler stub.
> + * The actual processing done in virtio_snd_process_cmdq().
> + *
> + * @s: VirtIOSound device
> + * @cmd: control command request
> + */
> +static inline void
> +process_cmd(VirtIOSound *s, virtio_snd_ctrl_command *cmd)
> +{
> + size_t sz = iov_to_buf(cmd->elem->out_sg,
> + cmd->elem->out_num,
> + 0,
> + &cmd->ctrl,
> + sizeof(cmd->ctrl));
> + if (sz != sizeof(cmd->ctrl)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: virtio-snd command size incorrect %zu vs \
> + %zu\n", __func__, sz, sizeof(cmd->ctrl));
> + return;
> + }
> +
> + trace_virtio_snd_handle_code(cmd->ctrl.code,
> + print_code(cmd->ctrl.code));
> +
> + switch (cmd->ctrl.code) {
> + case VIRTIO_SND_R_JACK_INFO:
> + case VIRTIO_SND_R_JACK_REMAP:
> + qemu_log_mask(LOG_UNIMP,
> + "virtio_snd: jack functionality is unimplemented.");
> + cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
> + break;
> + case VIRTIO_SND_R_PCM_INFO:
> + case VIRTIO_SND_R_PCM_SET_PARAMS:
> + case VIRTIO_SND_R_PCM_PREPARE:
> + case VIRTIO_SND_R_PCM_START:
> + case VIRTIO_SND_R_PCM_STOP:
> + case VIRTIO_SND_R_PCM_RELEASE:
> + cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
> + break;
> + case VIRTIO_SND_R_CHMAP_INFO:
> + qemu_log_mask(LOG_UNIMP,
> + "virtio_snd: chmap info functionality is unimplemented.");
> + trace_virtio_snd_handle_chmap_info();
> + cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
> + break;
> + default:
> + /* error */
> + error_report("virtio snd header not recognized: %"PRIu32,
> + cmd->ctrl.code);
> + cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
> + }
> +
> + iov_from_buf(cmd->elem->in_sg,
> + cmd->elem->in_num,
> + 0,
> + &cmd->resp,
> + sizeof(cmd->resp));
> + virtqueue_push(cmd->vq, cmd->elem, sizeof(cmd->elem));
> + virtio_notify(VIRTIO_DEVICE(s), cmd->vq);
> +}
> +
> +/*
> + * Consume all elements in command queue.
> + *
> + * @s: VirtIOSound device
> + */
> +static void virtio_snd_process_cmdq(VirtIOSound *s)
> +{
> + virtio_snd_ctrl_command *cmd;
> +
> + if (unlikely(qatomic_read(&s->processing_cmdq))) {
> + return;
> + }
> +
> + WITH_QEMU_LOCK_GUARD(&s->cmdq_mutex) {
> + qatomic_set(&s->processing_cmdq, true);
> + while (!QTAILQ_EMPTY(&s->cmdq)) {
> + cmd = QTAILQ_FIRST(&s->cmdq);
> +
> + /* process command */
> + process_cmd(s, cmd);
> +
> + QTAILQ_REMOVE(&s->cmdq, cmd, next);
> +
> + g_free(cmd);
> + }
> + qatomic_set(&s->processing_cmdq, false);
> + }
> +}
> +
> +/*
> + * The control message handler. Pops an element from the control virtqueue,
> + * and stores them to VirtIOSound's cmdq queue and finally calls
> + * virtio_snd_process_cmdq() for processing.
> + *
> + * @vdev: VirtIOSound device
> + * @vq: Control virtqueue
> + */
> +static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> +{
> + VirtIOSound *s = VIRTIO_SND(vdev);
> + VirtQueueElement *elem;
> + virtio_snd_ctrl_command *cmd;
> +
> + trace_virtio_snd_handle_ctrl(vdev, vq);
> +
> + if (!virtio_queue_ready(vq)) {
> + return;
> + }
> +
> + elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> + while (elem) {
> + cmd = g_new0(virtio_snd_ctrl_command, 1);
> + cmd->elem = elem;
> + cmd->vq = vq;
> + cmd->resp.code = VIRTIO_SND_S_OK;
> + QTAILQ_INSERT_TAIL(&s->cmdq, cmd, next);
> + elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> + }
> +
> + virtio_snd_process_cmdq(s);
> +}
> +
> +/*
> + * The event virtqueue handler.
> + * Not implemented yet.
> + *
> + * @vdev: VirtIOSound device
> + * @vq: event vq
> + */
> +static void virtio_snd_handle_event(VirtIODevice *vdev, VirtQueue *vq)
> +{
> + qemu_log_mask(LOG_UNIMP, "virtio_snd: event queue is unimplemented.");
> + trace_virtio_snd_handle_event();
> +}
> +
> +/*
> + * Stub buffer virtqueue handler.
> *
> * @vdev: VirtIOSound device
> * @vq: virtqueue
> */
> -static void virtio_snd_handle_queue(VirtIODevice *vdev, VirtQueue *vq) {}
> +static void virtio_snd_handle_xfer(VirtIODevice *vdev, VirtQueue *vq) {}
>
> static uint64_t get_features(VirtIODevice *vdev, uint64_t features,
> Error **errp)
> @@ -123,17 +282,32 @@ virtio_snd_vm_state_change(void *opaque, bool running,
> }
> }
>
> +static void virtio_snd_set_pcm(VirtIOSound *snd)
> +{
> + VirtIOSoundPCM *pcm;
> +
> + pcm = g_new0(VirtIOSoundPCM, 1);
> + pcm->snd = snd;
> + pcm->streams = g_new0(VirtIOSoundPCMStream *, snd->snd_conf.streams);
> + pcm->pcm_params = g_new0(VirtIOSoundPCMParams, snd->snd_conf.streams);
> +
> + snd->pcm = pcm;
> +}
This modelling seems a bit odd: I'll comment a bit further below.
> static void virtio_snd_realize(DeviceState *dev, Error **errp)
> {
> ERRP_GUARD();
> VirtIOSound *vsnd = VIRTIO_SND(dev);
> VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>
> + vsnd->pcm = NULL;
> vsnd->vmstate =
> qemu_add_vm_change_state_handler(virtio_snd_vm_state_change, vsnd);
>
> trace_virtio_snd_realize(vsnd);
>
> + virtio_snd_set_pcm(vsnd);
> +
> virtio_init(vdev, VIRTIO_ID_SOUND, sizeof(virtio_snd_config));
> virtio_add_feature(&vsnd->features, VIRTIO_F_VERSION_1);
>
> @@ -161,31 +335,70 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp)
> AUD_register_card("virtio-sound", &vsnd->card);
>
> vsnd->queues[VIRTIO_SND_VQ_CONTROL] =
> - virtio_add_queue(vdev, 64, virtio_snd_handle_queue);
> + virtio_add_queue(vdev, 64, virtio_snd_handle_ctrl);
> vsnd->queues[VIRTIO_SND_VQ_EVENT] =
> - virtio_add_queue(vdev, 64, virtio_snd_handle_queue);
> + virtio_add_queue(vdev, 64, virtio_snd_handle_event);
> vsnd->queues[VIRTIO_SND_VQ_TX] =
> - virtio_add_queue(vdev, 64, virtio_snd_handle_queue);
> + virtio_add_queue(vdev, 64, virtio_snd_handle_xfer);
> vsnd->queues[VIRTIO_SND_VQ_RX] =
> - virtio_add_queue(vdev, 64, virtio_snd_handle_queue);
> + virtio_add_queue(vdev, 64, virtio_snd_handle_xfer);
> + qemu_mutex_init(&vsnd->cmdq_mutex);
> + QTAILQ_INIT(&vsnd->cmdq);
> +}
> +
> +/*
> + * Close the stream and free its resources.
> + *
> + * @stream: VirtIOSoundPCMStream *stream
> + */
> +static void virtio_snd_pcm_close(VirtIOSoundPCMStream *stream)
> +{
> }
>
> static void virtio_snd_unrealize(DeviceState *dev)
> {
> VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> VirtIOSound *vsnd = VIRTIO_SND(dev);
> + VirtIOSoundPCMStream *stream;
>
> qemu_del_vm_change_state_handler(vsnd->vmstate);
> virtio_del_queue(vdev, 0);
>
> trace_virtio_snd_unrealize(vsnd);
>
> + if (vsnd->pcm) {
> + if (vsnd->pcm->streams) {
> + for (uint32_t i = 0; i < vsnd->snd_conf.streams; i++) {
> + stream = vsnd->pcm->streams[i];
> + if (stream) {
> + virtio_snd_process_cmdq(stream->s);
> + virtio_snd_pcm_close(stream);
> + g_free(stream);
> + }
> + }
> + g_free(vsnd->pcm->streams);
> + }
> + g_free(vsnd->pcm);
> + vsnd->pcm = NULL;
> + }
> AUD_remove_card(&vsnd->card);
> virtio_cleanup(vdev);
> }
>
>
> -static void virtio_snd_reset(VirtIODevice *vdev) {}
> +static void virtio_snd_reset(VirtIODevice *vdev)
> +{
> + VirtIOSound *s = VIRTIO_SND(vdev);
> + virtio_snd_ctrl_command *cmd;
> +
> + WITH_QEMU_LOCK_GUARD(&s->cmdq_mutex) {
> + while (!QTAILQ_EMPTY(&s->cmdq)) {
> + cmd = QTAILQ_FIRST(&s->cmdq);
> + QTAILQ_REMOVE(&s->cmdq, cmd, next);
> + g_free(cmd);
> + }
> + }
> +}
>
> static void virtio_snd_class_init(ObjectClass *klass, void *data)
> {
> diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
> index b3c0e6f079..b7046418cf 100644
> --- a/include/hw/virtio/virtio-snd.h
> +++ b/include/hw/virtio/virtio-snd.h
> @@ -67,12 +67,78 @@ typedef struct virtio_snd_pcm_xfer virtio_snd_pcm_xfer;
> /* I/O request status */
> typedef struct virtio_snd_pcm_status virtio_snd_pcm_status;
>
> -typedef struct VirtIOSound {
> +/* device structs */
> +
> +typedef struct VirtIOSound VirtIOSound;
> +
> +typedef struct VirtIOSoundPCMStream VirtIOSoundPCMStream;
> +
> +typedef struct virtio_snd_ctrl_command virtio_snd_ctrl_command;
> +
> +typedef struct VirtIOSoundPCMParams VirtIOSoundPCMParams;
> +
> +typedef struct VirtIOSoundPCM VirtIOSoundPCM;
> +
> +/* Stream params */
> +struct VirtIOSoundPCMParams {
> + uint32_t features;
> + uint32_t buffer_bytes; /* size of hardware buffer in bytes */
> + uint32_t period_bytes; /* size of hardware period in bytes */
> + uint8_t channels;
> + uint8_t format;
> + uint8_t rate;
> +};
> +
> +struct VirtIOSoundPCM {
> + VirtIOSound *snd;
> + VirtIOSoundPCMParams *pcm_params;
> + VirtIOSoundPCMStream **streams;
> +};
> +
Both pcm_params and streams are set in virtio_snd_set_pcm() to match the number of
available streams.
> +struct VirtIOSoundPCMStream {
> + VirtIOSoundPCM *pcm;
> + virtio_snd_pcm_info info;
> + uint32_t id;
> + uint32_t buffer_bytes;
> + uint32_t period_bytes;
> + /* channel position values (VIRTIO_SND_CHMAP_XXX) */
> + uint8_t positions[VIRTIO_SND_CHMAP_MAX_SIZE];
> + VirtIOSound *s;
> + uint32_t features; /* 1 << VIRTIO_SND_PCM_F_XXX */
> + uint64_t formats; /* 1 << VIRTIO_SND_PCM_FMT_XXX */
> + uint64_t rates; /* 1 << VIRTIO_SND_PCM_RATE_XXX */
> + uint8_t direction;
> + uint8_t channels_min;
> + uint8_t channels_max;
> + bool flushing;
> + audsettings as;
> + audsettings desired_as;
> + union {
> + SWVoiceIn *in;
> + SWVoiceOut *out;
> + } voice;
> + QemuMutex queue_mutex;
> + QSIMPLEQ_HEAD(, VirtIOSoundPCMBlock) queue;
> +};
> +
> +struct VirtIOSound {
> VirtIODevice parent_obj;
Missing newline here.
> VirtQueue *queues[VIRTIO_SND_VQ_MAX];
> uint64_t features;
> + VirtIOSoundPCM *pcm;
See comment below.
> QEMUSoundCard card;
> VMChangeStateEntry *vmstate;
> virtio_snd_config snd_conf;
> -} VirtIOSound;
> + QemuMutex cmdq_mutex;
> + QTAILQ_HEAD(, virtio_snd_ctrl_command) cmdq;
> + bool processing_cmdq;
> +};
> +
> +struct virtio_snd_ctrl_command {
> + VirtQueueElement *elem;
> + VirtQueue *vq;
> + virtio_snd_hdr ctrl;
> + virtio_snd_hdr resp;
> + QTAILQ_ENTRY(virtio_snd_ctrl_command) next;
> +};
> #endif
On that basis it seems a reasonable idea to embed the config directly in the stream
and drop VirtIOSoundPCM i.e.:
struct VirtIOSoundPCMStream {
VirtIOSoundPCMParams pcm_param;
virtio_snd_pcm_info info;
uint32_t id;
uint32_t buffer_bytes;
uint32_t period_bytes;
...
...
};
You can then alter VirtIOSound to point to the streams directly i.e.
struct VirtIOSound {
VirtIODevice parent_obj;
VirtQueue *queues[VIRTIO_SND_VQ_MAX];
uint64_t features;
VirtIOSoundPCMStream **streams;
QEMUSoundCard card;
VMChangeStateEntry *vmstate;
virtio_snd_config snd_conf;
QemuMutex cmdq_mutex;
QTAILQ_HEAD(, virtio_snd_ctrl_command) cmdq;
bool processing_cmdq;
};
This should also save maintaining extra "back" pointers, and you can still obtain a
reference to elements in the parent object via container_of().
ATB,
Mark.
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v8 04/12] virtio-sound: set PCM stream parameters
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
` (2 preceding siblings ...)
2023-08-28 19:55 ` [PATCH v8 03/12] virtio-sound: handle control messages and streams Emmanouil Pitsidianakis
@ 2023-08-28 19:55 ` Emmanouil Pitsidianakis
2023-08-29 19:27 ` Alex Bennée
2023-08-28 19:55 ` [PATCH v8 05/12] virtio-sound: handle VIRTIO_SND_R_PCM_INFO request Emmanouil Pitsidianakis
` (10 subsequent siblings)
14 siblings, 1 reply; 51+ messages in thread
From: Emmanouil Pitsidianakis @ 2023-08-28 19:55 UTC (permalink / raw)
To: qemu-devel
Cc: Emmanouil Pitsidianakis, Igor Skalkin, Anton Yakovlev,
Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin,
Marcel Apfelbaum, Daniel P. Berrangé, Eduardo Habkost,
Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
This commit sets the virtio-snd device's default PCM parameters in
virtio_snd_pcm_set_params_impl(). The same function will be used to set
parameters from the guest with VIRTIO_SND_R_PCM_SET_PARAMS in a
follow-up commit.
PCM parameters describe the sound card parameters that the guest's
kernel sees as an ALSA device.
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/virtio-snd.c | 258 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 250 insertions(+), 8 deletions(-)
diff --git a/hw/virtio/virtio-snd.c b/hw/virtio/virtio-snd.c
index b921903905..25c3539509 100644
--- a/hw/virtio/virtio-snd.c
+++ b/hw/virtio/virtio-snd.c
@@ -31,6 +31,29 @@
#define VIRTIO_SOUND_CHMAP_DEFAULT 0
#define VIRTIO_SOUND_HDA_FN_NID 0
+static uint32_t supported_formats = BIT(VIRTIO_SND_PCM_FMT_S8)
+ | BIT(VIRTIO_SND_PCM_FMT_U8)
+ | BIT(VIRTIO_SND_PCM_FMT_S16)
+ | BIT(VIRTIO_SND_PCM_FMT_U16)
+ | BIT(VIRTIO_SND_PCM_FMT_S32)
+ | BIT(VIRTIO_SND_PCM_FMT_U32)
+ | BIT(VIRTIO_SND_PCM_FMT_FLOAT);
+
+static uint32_t supported_rates = BIT(VIRTIO_SND_PCM_RATE_5512)
+ | BIT(VIRTIO_SND_PCM_RATE_8000)
+ | BIT(VIRTIO_SND_PCM_RATE_11025)
+ | BIT(VIRTIO_SND_PCM_RATE_16000)
+ | BIT(VIRTIO_SND_PCM_RATE_22050)
+ | BIT(VIRTIO_SND_PCM_RATE_32000)
+ | BIT(VIRTIO_SND_PCM_RATE_44100)
+ | BIT(VIRTIO_SND_PCM_RATE_48000)
+ | BIT(VIRTIO_SND_PCM_RATE_64000)
+ | BIT(VIRTIO_SND_PCM_RATE_88200)
+ | BIT(VIRTIO_SND_PCM_RATE_96000)
+ | BIT(VIRTIO_SND_PCM_RATE_176400)
+ | BIT(VIRTIO_SND_PCM_RATE_192000)
+ | BIT(VIRTIO_SND_PCM_RATE_384000);
+
static const char *print_code(uint32_t code)
{
#define CASE(CODE) \
@@ -111,6 +134,206 @@ virtio_snd_set_config(VirtIODevice *vdev, const uint8_t *config)
memcpy(&s->snd_conf, sndconfig, sizeof(s->snd_conf));
}
+/*
+ * Get params for a specific stream.
+ *
+ * @s: VirtIOSound device
+ * @stream_id: stream id
+ */
+static VirtIOSoundPCMParams *virtio_snd_pcm_get_params(VirtIOSound *s,
+ uint32_t stream_id)
+{
+ return stream_id >= s->snd_conf.streams ? NULL
+ : &s->pcm->pcm_params[stream_id];
+}
+
+/*
+ * Set the given stream params.
+ * Called by both virtio_snd_handle_pcm_set_params and during device
+ * initialization.
+ * Returns the response status code. (VIRTIO_SND_S_*).
+ *
+ * @s: VirtIOSound device
+ * @params: The PCM params as defined in the virtio specification
+ */
+static
+uint32_t virtio_snd_set_pcm_params(VirtIOSound *s,
+ virtio_snd_pcm_set_params *params)
+{
+ VirtIOSoundPCMParams *st_params;
+ uint32_t stream_id = params->hdr.stream_id;
+
+ if (stream_id >= s->snd_conf.streams || !(s->pcm->pcm_params)) {
+ virtio_error(VIRTIO_DEVICE(s), "Streams have not been initialized.\n");
+ return VIRTIO_SND_S_BAD_MSG;
+ }
+
+ st_params = virtio_snd_pcm_get_params(s, stream_id);
+
+ if (params->channels < 1 || params->channels > AUDIO_MAX_CHANNELS) {
+ error_report("Number of channels is not supported.");
+ return VIRTIO_SND_S_NOT_SUPP;
+ }
+ if (!(supported_formats & BIT(params->format))) {
+ error_report("Stream format is not supported.");
+ return VIRTIO_SND_S_NOT_SUPP;
+ }
+ if (!(supported_rates & BIT(params->rate))) {
+ error_report("Stream rate is not supported.");
+ return VIRTIO_SND_S_NOT_SUPP;
+ }
+
+ st_params->buffer_bytes = params->buffer_bytes;
+ st_params->period_bytes = params->period_bytes;
+ st_params->features = params->features;
+ st_params->channels = params->channels;
+ st_params->format = params->format;
+ st_params->rate = params->rate;
+
+ return VIRTIO_SND_S_OK;
+}
+
+/*
+ * Get a QEMU Audiosystem compatible format value from a VIRTIO_SND_PCM_FMT_*
+ */
+static AudioFormat virtio_snd_get_qemu_format(uint32_t format)
+{
+ #define CASE(FMT) \
+ case VIRTIO_SND_PCM_FMT_##FMT: \
+ return AUDIO_FORMAT_##FMT;
+
+ switch (format) {
+ CASE(U8)
+ CASE(S8)
+ CASE(U16)
+ CASE(S16)
+ CASE(U32)
+ CASE(S32)
+ case VIRTIO_SND_PCM_FMT_FLOAT:
+ return AUDIO_FORMAT_F32;
+ default:
+ g_assert_not_reached();
+ }
+
+ #undef CASE
+}
+
+/*
+ * Get a QEMU Audiosystem compatible frequency value from a
+ * VIRTIO_SND_PCM_RATE_*
+ */
+static uint32_t virtio_snd_get_qemu_freq(uint32_t rate)
+{
+ #define CASE(RATE) \
+ case VIRTIO_SND_PCM_RATE_##RATE: \
+ return RATE;
+
+ switch (rate) {
+ CASE(5512)
+ CASE(8000)
+ CASE(11025)
+ CASE(16000)
+ CASE(22050)
+ CASE(32000)
+ CASE(44100)
+ CASE(48000)
+ CASE(64000)
+ CASE(88200)
+ CASE(96000)
+ CASE(176400)
+ CASE(192000)
+ CASE(384000)
+ default:
+ g_assert_not_reached();
+ }
+
+ #undef CASE
+}
+
+/*
+ * Get QEMU Audiosystem compatible audsettings from virtio based pcm stream
+ * params.
+ */
+static void virtio_snd_get_qemu_audsettings(audsettings *as,
+ VirtIOSoundPCMParams *params)
+{
+ as->nchannels = MIN(AUDIO_MAX_CHANNELS, params->channels);
+ as->fmt = virtio_snd_get_qemu_format(params->format);
+ as->freq = virtio_snd_get_qemu_freq(params->rate);
+ as->endianness = AUDIO_HOST_ENDIANNESS;
+}
+
+/*
+ * Close a stream and free all its resources.
+ *
+ * @stream: VirtIOSoundPCMStream *stream
+ */
+static void virtio_snd_pcm_close(VirtIOSoundPCMStream *stream)
+{
+ if (stream) {
+ qemu_mutex_destroy(&stream->queue_mutex);
+ g_free(stream);
+ }
+}
+
+/*
+ * Prepares a VirtIOSound card stream.
+ * Returns the response status code. (VIRTIO_SND_S_*).
+ *
+ * @s: VirtIOSound device
+ * @stream_id: stream id
+ */
+static uint32_t virtio_snd_pcm_prepare(VirtIOSound *s, uint32_t stream_id)
+{
+ audsettings as;
+ VirtIOSoundPCMParams *params;
+ VirtIOSoundPCMStream *stream;
+
+ if (!s->pcm->streams ||
+ !s->pcm->pcm_params ||
+ stream_id >= s->snd_conf.streams) {
+ return VIRTIO_SND_S_BAD_MSG;
+ }
+
+ params = virtio_snd_pcm_get_params(s, stream_id);
+ if (!params) {
+ return VIRTIO_SND_S_BAD_MSG;
+ }
+
+ virtio_snd_get_qemu_audsettings(&as, params);
+
+ virtio_snd_pcm_close(s->pcm->streams[stream_id]);
+
+ stream = g_new0(VirtIOSoundPCMStream, 1);
+
+ stream->id = stream_id;
+ stream->pcm = s->pcm;
+ stream->direction = stream_id < s->snd_conf.streams / 2 +
+ (s->snd_conf.streams & 1) ? VIRTIO_SND_D_OUTPUT : VIRTIO_SND_D_INPUT;
+ stream->info.hdr.hda_fn_nid = VIRTIO_SOUND_HDA_FN_NID;
+ stream->features = 0;
+ stream->channels_min = 1;
+ stream->channels_max = as.nchannels;
+ stream->formats = supported_formats;
+ stream->rates = supported_rates;
+ stream->s = s;
+
+ stream->buffer_bytes = params->buffer_bytes;
+ stream->period_bytes = params->period_bytes;
+
+ stream->positions[0] = VIRTIO_SND_CHMAP_FL;
+ stream->positions[1] = VIRTIO_SND_CHMAP_FR;
+
+ stream->as = as;
+ stream->desired_as = stream->as;
+ qemu_mutex_init(&stream->queue_mutex);
+ QSIMPLEQ_INIT(&stream->queue);
+
+ s->pcm->streams[stream_id] = stream;
+
+ return VIRTIO_SND_S_OK;
+}
+
/*
* The actual processing done in virtio_snd_process_cmdq().
*
@@ -299,6 +522,8 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp)
ERRP_GUARD();
VirtIOSound *vsnd = VIRTIO_SND(dev);
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ virtio_snd_pcm_set_params default_params = { 0 };
+ uint32_t status;
vsnd->pcm = NULL;
vsnd->vmstate =
@@ -334,6 +559,13 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp)
AUD_register_card("virtio-sound", &vsnd->card);
+ /* set default params for all streams */
+ default_params.features = 0;
+ default_params.buffer_bytes = 8192;
+ default_params.period_bytes = 2048;
+ default_params.channels = 2;
+ default_params.format = VIRTIO_SND_PCM_FMT_S16;
+ default_params.rate = VIRTIO_SND_PCM_RATE_48000;
vsnd->queues[VIRTIO_SND_VQ_CONTROL] =
virtio_add_queue(vdev, 64, virtio_snd_handle_ctrl);
vsnd->queues[VIRTIO_SND_VQ_EVENT] =
@@ -344,15 +576,24 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp)
virtio_add_queue(vdev, 64, virtio_snd_handle_xfer);
qemu_mutex_init(&vsnd->cmdq_mutex);
QTAILQ_INIT(&vsnd->cmdq);
-}
-/*
- * Close the stream and free its resources.
- *
- * @stream: VirtIOSoundPCMStream *stream
- */
-static void virtio_snd_pcm_close(VirtIOSoundPCMStream *stream)
-{
+ for (uint32_t i = 0; i < vsnd->snd_conf.streams; i++) {
+ default_params.hdr.stream_id = i;
+ status = virtio_snd_set_pcm_params(vsnd, &default_params);
+ if (status != VIRTIO_SND_S_OK) {
+ error_setg(errp,
+ "Can't initalize stream params, device responded with %s.",
+ print_code(status));
+ return;
+ }
+ status = virtio_snd_pcm_prepare(vsnd, i);
+ if (status != VIRTIO_SND_S_OK) {
+ error_setg(errp,
+ "Can't prepare streams, device responded with %s.",
+ print_code(status));
+ return;
+ }
+ }
}
static void virtio_snd_unrealize(DeviceState *dev)
@@ -382,6 +623,7 @@ static void virtio_snd_unrealize(DeviceState *dev)
vsnd->pcm = NULL;
}
AUD_remove_card(&vsnd->card);
+ qemu_mutex_destroy(&vsnd->cmdq_mutex);
virtio_cleanup(vdev);
}
--
2.39.2
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v8 04/12] virtio-sound: set PCM stream parameters
2023-08-28 19:55 ` [PATCH v8 04/12] virtio-sound: set PCM stream parameters Emmanouil Pitsidianakis
@ 2023-08-29 19:27 ` Alex Bennée
0 siblings, 0 replies; 51+ messages in thread
From: Alex Bennée @ 2023-08-29 19:27 UTC (permalink / raw)
To: Emmanouil Pitsidianakis
Cc: qemu-devel, Igor Skalkin, Anton Yakovlev, Paolo Bonzini,
Gerd Hoffmann, Michael S. Tsirkin, Marcel Apfelbaum,
Daniel P. Berrangé, Eduardo Habkost, Marc-André Lureau,
Volker Rümelin, Kővágó, Zoltán,
Philippe Mathieu-Daudé
Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> This commit sets the virtio-snd device's default PCM parameters in
> virtio_snd_pcm_set_params_impl(). The same function will be used to set
> parameters from the guest with VIRTIO_SND_R_PCM_SET_PARAMS in a
> follow-up commit.
>
> PCM parameters describe the sound card parameters that the guest's
> kernel sees as an ALSA device.
>
> 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>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v8 05/12] virtio-sound: handle VIRTIO_SND_R_PCM_INFO request
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
` (3 preceding siblings ...)
2023-08-28 19:55 ` [PATCH v8 04/12] virtio-sound: set PCM stream parameters Emmanouil Pitsidianakis
@ 2023-08-28 19:55 ` Emmanouil Pitsidianakis
2023-08-29 19:31 ` Alex Bennée
2023-09-04 10:13 ` Philippe Mathieu-Daudé
2023-08-28 19:55 ` [PATCH v8 06/12] virtio-sound: handle VIRTIO_SND_R_PCM_{START,STOP} Emmanouil Pitsidianakis
` (9 subsequent siblings)
14 siblings, 2 replies; 51+ messages in thread
From: Emmanouil Pitsidianakis @ 2023-08-28 19:55 UTC (permalink / raw)
To: qemu-devel
Cc: Emmanouil Pitsidianakis, Igor Skalkin, Anton Yakovlev,
Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin,
Marcel Apfelbaum, Daniel P. Berrangé, Eduardo Habkost,
Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
Respond to the VIRTIO_SND_R_PCM_INFO control request with the parameters
of each requested PCM stream.
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 | 1 +
hw/virtio/virtio-snd.c | 76 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 77 insertions(+)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 8a223e36e9..3e619f778b 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -164,6 +164,7 @@ virtio_snd_vm_state_stopped(void) "vm state stopped"
virtio_snd_realize(void *snd) "snd %p: realize"
virtio_snd_unrealize(void *snd) "snd %p: unrealize"
virtio_snd_handle_ctrl(void *vdev, void *vq) "snd %p: handle ctrl event for queue %p"
+virtio_snd_handle_pcm_info(uint32_t stream) "VIRTIO_SND_R_PCM_INFO called for stream %"PRIu32
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"
diff --git a/hw/virtio/virtio-snd.c b/hw/virtio/virtio-snd.c
index 25c3539509..ab82c786f2 100644
--- a/hw/virtio/virtio-snd.c
+++ b/hw/virtio/virtio-snd.c
@@ -134,6 +134,19 @@ virtio_snd_set_config(VirtIODevice *vdev, const uint8_t *config)
memcpy(&s->snd_conf, sndconfig, sizeof(s->snd_conf));
}
+/*
+ * Get a specific stream from the virtio sound card device.
+ * Returns NULL if @stream_id is invalid or not allocated.
+ *
+ * @s: VirtIOSound device
+ * @stream_id: stream id
+ */
+static VirtIOSoundPCMStream *virtio_snd_pcm_get_stream(VirtIOSound *s,
+ uint32_t stream_id)
+{
+ return stream_id >= s->snd_conf.streams ? NULL : s->pcm->streams[stream_id];
+}
+
/*
* Get params for a specific stream.
*
@@ -147,6 +160,67 @@ static VirtIOSoundPCMParams *virtio_snd_pcm_get_params(VirtIOSound *s,
: &s->pcm->pcm_params[stream_id];
}
+/*
+ * Handle the VIRTIO_SND_R_PCM_INFO request.
+ * The function writes the info structs to the request element.
+ *
+ * @s: VirtIOSound device
+ * @cmd: The request command queue element from VirtIOSound cmdq field
+ */
+static void virtio_snd_handle_pcm_info(VirtIOSound *s,
+ virtio_snd_ctrl_command *cmd)
+{
+ virtio_snd_query_info req;
+ VirtIOSoundPCMStream *stream = NULL;
+ g_autofree virtio_snd_pcm_info *pcm_info = NULL;
+ size_t sz = iov_to_buf(cmd->elem->out_sg,
+ cmd->elem->out_num,
+ 0,
+ &req,
+ sizeof(req));
+ if (sz != sizeof(virtio_snd_query_info)) {
+ cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
+ return;
+ }
+
+ if (iov_size(cmd->elem->in_sg, cmd->elem->in_num) <
+ sizeof(virtio_snd_hdr) + req.size * req.count) {
+ error_report("pcm info: buffer too small, got: %zu, needed: %zu",
+ iov_size(cmd->elem->in_sg, cmd->elem->in_num),
+ sizeof(virtio_snd_pcm_info));
+ cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
+ return;
+ }
+
+ pcm_info = g_new0(virtio_snd_pcm_info, req.count);
+ for (uint32_t i = req.start_id; i < req.start_id + req.count; i++) {
+ trace_virtio_snd_handle_pcm_info(i);
+ stream = virtio_snd_pcm_get_stream(s, i);
+
+ if (!stream) {
+ error_report("Invalid stream id: %"PRIu32, i);
+ cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
+ return;
+ }
+
+ pcm_info[i - req.start_id].hdr.hda_fn_nid = stream->info.hdr.hda_fn_nid;
+ pcm_info[i - req.start_id].features = stream->features;
+ pcm_info[i - req.start_id].formats = stream->formats;
+ pcm_info[i - req.start_id].rates = stream->rates;
+ pcm_info[i - req.start_id].direction = stream->direction;
+ pcm_info[i - req.start_id].channels_min = stream->channels_min;
+ pcm_info[i - req.start_id].channels_max = stream->channels_max;
+ }
+
+ cmd->resp.code = VIRTIO_SND_S_OK;
+
+ iov_from_buf(cmd->elem->in_sg,
+ cmd->elem->in_num,
+ sizeof(virtio_snd_hdr),
+ pcm_info,
+ sizeof(virtio_snd_pcm_info) * req.count);
+}
+
/*
* Set the given stream params.
* Called by both virtio_snd_handle_pcm_set_params and during device
@@ -366,6 +440,8 @@ process_cmd(VirtIOSound *s, virtio_snd_ctrl_command *cmd)
cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
break;
case VIRTIO_SND_R_PCM_INFO:
+ virtio_snd_handle_pcm_info(s, cmd);
+ break;
case VIRTIO_SND_R_PCM_SET_PARAMS:
case VIRTIO_SND_R_PCM_PREPARE:
case VIRTIO_SND_R_PCM_START:
--
2.39.2
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v8 05/12] virtio-sound: handle VIRTIO_SND_R_PCM_INFO request
2023-08-28 19:55 ` [PATCH v8 05/12] virtio-sound: handle VIRTIO_SND_R_PCM_INFO request Emmanouil Pitsidianakis
@ 2023-08-29 19:31 ` Alex Bennée
2023-09-04 10:13 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 51+ messages in thread
From: Alex Bennée @ 2023-08-29 19:31 UTC (permalink / raw)
To: Emmanouil Pitsidianakis
Cc: qemu-devel, Igor Skalkin, Anton Yakovlev, Paolo Bonzini,
Gerd Hoffmann, Michael S. Tsirkin, Marcel Apfelbaum,
Daniel P. Berrangé, Eduardo Habkost, Marc-André Lureau,
Volker Rümelin, Kővágó, Zoltán,
Philippe Mathieu-Daudé
Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> Respond to the VIRTIO_SND_R_PCM_INFO control request with the parameters
> of each requested PCM stream.
>
> 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>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v8 05/12] virtio-sound: handle VIRTIO_SND_R_PCM_INFO request
2023-08-28 19:55 ` [PATCH v8 05/12] virtio-sound: handle VIRTIO_SND_R_PCM_INFO request Emmanouil Pitsidianakis
2023-08-29 19:31 ` Alex Bennée
@ 2023-09-04 10:13 ` Philippe Mathieu-Daudé
2023-09-04 10:24 ` Manos Pitsidianakis
1 sibling, 1 reply; 51+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-09-04 10:13 UTC (permalink / raw)
To: Emmanouil Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
Michael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée
On 28/8/23 21:55, Emmanouil Pitsidianakis wrote:
> Respond to the VIRTIO_SND_R_PCM_INFO control request with the parameters
> of each requested PCM stream.
>
> 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 | 1 +
> hw/virtio/virtio-snd.c | 76 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 77 insertions(+)
> +/*
> + * Handle the VIRTIO_SND_R_PCM_INFO request.
> + * The function writes the info structs to the request element.
> + *
> + * @s: VirtIOSound device
> + * @cmd: The request command queue element from VirtIOSound cmdq field
> + */
> +static void virtio_snd_handle_pcm_info(VirtIOSound *s,
> + virtio_snd_ctrl_command *cmd)
> +{
> + virtio_snd_query_info req;
> + VirtIOSoundPCMStream *stream = NULL;
> + g_autofree virtio_snd_pcm_info *pcm_info = NULL;
> + size_t sz = iov_to_buf(cmd->elem->out_sg,
> + cmd->elem->out_num,
> + 0,
> + &req,
> + sizeof(req));
> + if (sz != sizeof(virtio_snd_query_info)) {
> + cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
> + return;
> + }
> +
> + if (iov_size(cmd->elem->in_sg, cmd->elem->in_num) <
> + sizeof(virtio_snd_hdr) + req.size * req.count) {
> + error_report("pcm info: buffer too small, got: %zu, needed: %zu",
> + iov_size(cmd->elem->in_sg, cmd->elem->in_num),
> + sizeof(virtio_snd_pcm_info));
> + cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
> + return;
> + }
> +
> + pcm_info = g_new0(virtio_snd_pcm_info, req.count);
> + for (uint32_t i = req.start_id; i < req.start_id + req.count; i++) {
Starting from req.start_id seems to increase this code complexity.
> + trace_virtio_snd_handle_pcm_info(i);
> + stream = virtio_snd_pcm_get_stream(s, i);
> +
> + if (!stream) {
> + error_report("Invalid stream id: %"PRIu32, i);
> + cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
> + return;
> + }
> +
> + pcm_info[i - req.start_id].hdr.hda_fn_nid = stream->info.hdr.hda_fn_nid;
> + pcm_info[i - req.start_id].features = stream->features;
> + pcm_info[i - req.start_id].formats = stream->formats;
> + pcm_info[i - req.start_id].rates = stream->rates;
> + pcm_info[i - req.start_id].direction = stream->direction;
> + pcm_info[i - req.start_id].channels_min = stream->channels_min;
> + pcm_info[i - req.start_id].channels_max = stream->channels_max;
> + }
> +
> + cmd->resp.code = VIRTIO_SND_S_OK;
> +
> + iov_from_buf(cmd->elem->in_sg,
> + cmd->elem->in_num,
> + sizeof(virtio_snd_hdr),
> + pcm_info,
> + sizeof(virtio_snd_pcm_info) * req.count);
> +}
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 05/12] virtio-sound: handle VIRTIO_SND_R_PCM_INFO request
2023-09-04 10:13 ` Philippe Mathieu-Daudé
@ 2023-09-04 10:24 ` Manos Pitsidianakis
0 siblings, 0 replies; 51+ messages in thread
From: Manos Pitsidianakis @ 2023-09-04 10:24 UTC (permalink / raw)
To: Philippe Mathieu-Daudé , qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
ichael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé ,
Eduardo Habkost, Marc-André Lureau, Volker Rü melin,
Kő vá gó , Zoltá n, Alex Benné e
On Mon, 04 Sep 2023 13:13, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>> +
>> + pcm_info = g_new0(virtio_snd_pcm_info, req.count);
>> + for (uint32_t i = req.start_id; i < req.start_id + req.count; i++) {
>
>Starting from req.start_id seems to increase this code complexity.
I see your point, will change it!
Manos
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v8 06/12] virtio-sound: handle VIRTIO_SND_R_PCM_{START,STOP}
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
` (4 preceding siblings ...)
2023-08-28 19:55 ` [PATCH v8 05/12] virtio-sound: handle VIRTIO_SND_R_PCM_INFO request Emmanouil Pitsidianakis
@ 2023-08-28 19:55 ` Emmanouil Pitsidianakis
2023-08-29 19:32 ` Alex Bennée
2023-08-28 19:55 ` [PATCH v8 07/12] virtio-sound: handle VIRTIO_SND_R_PCM_SET_PARAMS Emmanouil Pitsidianakis
` (8 subsequent siblings)
14 siblings, 1 reply; 51+ messages in thread
From: Emmanouil Pitsidianakis @ 2023-08-28 19:55 UTC (permalink / raw)
To: qemu-devel
Cc: Emmanouil Pitsidianakis, Igor Skalkin, Anton Yakovlev,
Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin,
Marcel Apfelbaum, Daniel P. Berrangé, Eduardo Habkost,
Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
Handle the start and stop control messages for a stream_id. This request
does nothing at the moment except for replying to it. Audio playback
or capture will be started/stopped here in follow-up commits.
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 | 1 +
hw/virtio/virtio-snd.c | 42 ++++++++++++++++++++++++++++++++++++++++--
2 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 3e619f778b..8eae1bf881 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -165,6 +165,7 @@ virtio_snd_realize(void *snd) "snd %p: realize"
virtio_snd_unrealize(void *snd) "snd %p: unrealize"
virtio_snd_handle_ctrl(void *vdev, void *vq) "snd %p: handle ctrl event for queue %p"
virtio_snd_handle_pcm_info(uint32_t stream) "VIRTIO_SND_R_PCM_INFO called for stream %"PRIu32
+virtio_snd_handle_pcm_start_stop(const char *code, uint32_t stream) "%s called for stream %"PRIu32
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"
diff --git a/hw/virtio/virtio-snd.c b/hw/virtio/virtio-snd.c
index ab82c786f2..ae014d946c 100644
--- a/hw/virtio/virtio-snd.c
+++ b/hw/virtio/virtio-snd.c
@@ -408,6 +408,40 @@ static uint32_t virtio_snd_pcm_prepare(VirtIOSound *s, uint32_t stream_id)
return VIRTIO_SND_S_OK;
}
+/*
+ * Handles VIRTIO_SND_R_PCM_START.
+ *
+ * @s: VirtIOSound device
+ * @cmd: The request command queue element from VirtIOSound cmdq field
+ * @start: whether to start or stop the device
+ */
+static void virtio_snd_handle_pcm_start_stop(VirtIOSound *s,
+ virtio_snd_ctrl_command *cmd,
+ bool start)
+{
+ VirtIOSoundPCMStream *stream;
+ virtio_snd_pcm_hdr req;
+ size_t sz = iov_to_buf(cmd->elem->out_sg,
+ cmd->elem->out_num,
+ 0,
+ &req,
+ sizeof(req));
+ if (sz != sizeof(virtio_snd_pcm_hdr)) {
+ cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
+ return;
+ }
+
+ cmd->resp.code = VIRTIO_SND_S_OK;
+ trace_virtio_snd_handle_pcm_start_stop(start ? "VIRTIO_SND_R_PCM_START" :
+ "VIRTIO_SND_R_PCM_STOP", req.stream_id);
+
+ stream = virtio_snd_pcm_get_stream(s, req.stream_id);
+ if (!stream) {
+ error_report("Invalid stream id: %"PRIu32, req.stream_id);
+ cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
+ }
+}
+
/*
* The actual processing done in virtio_snd_process_cmdq().
*
@@ -442,10 +476,14 @@ process_cmd(VirtIOSound *s, virtio_snd_ctrl_command *cmd)
case VIRTIO_SND_R_PCM_INFO:
virtio_snd_handle_pcm_info(s, cmd);
break;
- case VIRTIO_SND_R_PCM_SET_PARAMS:
- case VIRTIO_SND_R_PCM_PREPARE:
case VIRTIO_SND_R_PCM_START:
+ virtio_snd_handle_pcm_start_stop(s, cmd, true);
+ break;
case VIRTIO_SND_R_PCM_STOP:
+ virtio_snd_handle_pcm_start_stop(s, cmd, false);
+ break;
+ case VIRTIO_SND_R_PCM_SET_PARAMS:
+ case VIRTIO_SND_R_PCM_PREPARE:
case VIRTIO_SND_R_PCM_RELEASE:
cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
break;
--
2.39.2
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v8 06/12] virtio-sound: handle VIRTIO_SND_R_PCM_{START,STOP}
2023-08-28 19:55 ` [PATCH v8 06/12] virtio-sound: handle VIRTIO_SND_R_PCM_{START,STOP} Emmanouil Pitsidianakis
@ 2023-08-29 19:32 ` Alex Bennée
0 siblings, 0 replies; 51+ messages in thread
From: Alex Bennée @ 2023-08-29 19:32 UTC (permalink / raw)
To: Emmanouil Pitsidianakis
Cc: qemu-devel, Igor Skalkin, Anton Yakovlev, Paolo Bonzini,
Gerd Hoffmann, Michael S. Tsirkin, Marcel Apfelbaum,
Daniel P. Berrangé, Eduardo Habkost, Marc-André Lureau,
Volker Rümelin, Kővágó, Zoltán,
Philippe Mathieu-Daudé
Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> Handle the start and stop control messages for a stream_id. This request
> does nothing at the moment except for replying to it. Audio playback
> or capture will be started/stopped here in follow-up commits.
>
> 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>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v8 07/12] virtio-sound: handle VIRTIO_SND_R_PCM_SET_PARAMS
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
` (5 preceding siblings ...)
2023-08-28 19:55 ` [PATCH v8 06/12] virtio-sound: handle VIRTIO_SND_R_PCM_{START,STOP} Emmanouil Pitsidianakis
@ 2023-08-28 19:55 ` Emmanouil Pitsidianakis
2023-08-29 19:33 ` Alex Bennée
2023-08-28 19:55 ` [PATCH v8 08/12] virtio-sound: handle VIRTIO_SND_R_PCM_PREPARE Emmanouil Pitsidianakis
` (7 subsequent siblings)
14 siblings, 1 reply; 51+ messages in thread
From: Emmanouil Pitsidianakis @ 2023-08-28 19:55 UTC (permalink / raw)
To: qemu-devel
Cc: Emmanouil Pitsidianakis, Igor Skalkin, Anton Yakovlev,
Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin,
Marcel Apfelbaum, Daniel P. Berrangé, Eduardo Habkost,
Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
Handle the set parameters control request. It reconfigures a stream
based on a guest's preference if the values are valid and supported.
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 | 1 +
hw/virtio/virtio-snd.c | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 8eae1bf881..f70cde4f01 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -163,6 +163,7 @@ virtio_snd_vm_state_running(void) "vm state running"
virtio_snd_vm_state_stopped(void) "vm state stopped"
virtio_snd_realize(void *snd) "snd %p: realize"
virtio_snd_unrealize(void *snd) "snd %p: unrealize"
+virtio_snd_handle_pcm_set_params(uint32_t stream) "VIRTIO_SND_PCM_SET_PARAMS called for stream %"PRIu32
virtio_snd_handle_ctrl(void *vdev, void *vq) "snd %p: handle ctrl event for queue %p"
virtio_snd_handle_pcm_info(uint32_t stream) "VIRTIO_SND_R_PCM_INFO called for stream %"PRIu32
virtio_snd_handle_pcm_start_stop(const char *code, uint32_t stream) "%s called for stream %"PRIu32
diff --git a/hw/virtio/virtio-snd.c b/hw/virtio/virtio-snd.c
index ae014d946c..a05f637dc3 100644
--- a/hw/virtio/virtio-snd.c
+++ b/hw/virtio/virtio-snd.c
@@ -267,6 +267,30 @@ uint32_t virtio_snd_set_pcm_params(VirtIOSound *s,
return VIRTIO_SND_S_OK;
}
+/*
+ * Handles the VIRTIO_SND_R_PCM_SET_PARAMS request.
+ *
+ * @s: VirtIOSound device
+ * @cmd: The request command queue element from VirtIOSound cmdq field
+ */
+static void virtio_snd_handle_pcm_set_params(VirtIOSound *s,
+ virtio_snd_ctrl_command *cmd)
+{
+ virtio_snd_pcm_set_params req;
+ size_t sz = iov_to_buf(cmd->elem->out_sg,
+ cmd->elem->out_num,
+ 0,
+ &req,
+ sizeof(req));
+ if (sz != sizeof(virtio_snd_pcm_set_params)) {
+ cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
+ return;
+ }
+
+ trace_virtio_snd_handle_pcm_set_params(req.hdr.stream_id);
+ cmd->resp.code = virtio_snd_set_pcm_params(s, &req);
+}
+
/*
* Get a QEMU Audiosystem compatible format value from a VIRTIO_SND_PCM_FMT_*
*/
@@ -483,6 +507,8 @@ process_cmd(VirtIOSound *s, virtio_snd_ctrl_command *cmd)
virtio_snd_handle_pcm_start_stop(s, cmd, false);
break;
case VIRTIO_SND_R_PCM_SET_PARAMS:
+ virtio_snd_handle_pcm_set_params(s, cmd);
+ break;
case VIRTIO_SND_R_PCM_PREPARE:
case VIRTIO_SND_R_PCM_RELEASE:
cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
--
2.39.2
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v8 07/12] virtio-sound: handle VIRTIO_SND_R_PCM_SET_PARAMS
2023-08-28 19:55 ` [PATCH v8 07/12] virtio-sound: handle VIRTIO_SND_R_PCM_SET_PARAMS Emmanouil Pitsidianakis
@ 2023-08-29 19:33 ` Alex Bennée
0 siblings, 0 replies; 51+ messages in thread
From: Alex Bennée @ 2023-08-29 19:33 UTC (permalink / raw)
To: Emmanouil Pitsidianakis
Cc: qemu-devel, Igor Skalkin, Anton Yakovlev, Paolo Bonzini,
Gerd Hoffmann, Michael S. Tsirkin, Marcel Apfelbaum,
Daniel P. Berrangé, Eduardo Habkost, Marc-André Lureau,
Volker Rümelin, Kővágó, Zoltán,
Philippe Mathieu-Daudé
Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> Handle the set parameters control request. It reconfigures a stream
> based on a guest's preference if the values are valid and supported.
>
> 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>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v8 08/12] virtio-sound: handle VIRTIO_SND_R_PCM_PREPARE
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
` (6 preceding siblings ...)
2023-08-28 19:55 ` [PATCH v8 07/12] virtio-sound: handle VIRTIO_SND_R_PCM_SET_PARAMS Emmanouil Pitsidianakis
@ 2023-08-28 19:55 ` Emmanouil Pitsidianakis
2023-08-29 19:33 ` Alex Bennée
2023-08-28 19:55 ` [PATCH v8 09/12] virtio-sound: handle VIRTIO_SND_R_PCM_RELEASE Emmanouil Pitsidianakis
` (6 subsequent siblings)
14 siblings, 1 reply; 51+ messages in thread
From: Emmanouil Pitsidianakis @ 2023-08-28 19:55 UTC (permalink / raw)
To: qemu-devel
Cc: Emmanouil Pitsidianakis, Igor Skalkin, Anton Yakovlev,
Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin,
Marcel Apfelbaum, Daniel P. Berrangé, Eduardo Habkost,
Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
Handles the PCM prepare control request. It initializes a PCM stream
when the guests asks for it.
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/virtio-snd.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/hw/virtio/virtio-snd.c b/hw/virtio/virtio-snd.c
index a05f637dc3..be6eb2c36d 100644
--- a/hw/virtio/virtio-snd.c
+++ b/hw/virtio/virtio-snd.c
@@ -432,6 +432,27 @@ static uint32_t virtio_snd_pcm_prepare(VirtIOSound *s, uint32_t stream_id)
return VIRTIO_SND_S_OK;
}
+/*
+ * Handles VIRTIO_SND_R_PCM_PREPARE.
+ *
+ * @s: VirtIOSound device
+ * @cmd: The request command queue element from VirtIOSound cmdq field
+ */
+static void virtio_snd_handle_pcm_prepare(VirtIOSound *s,
+ virtio_snd_ctrl_command *cmd)
+{
+ uint32_t stream_id;
+ size_t sz = iov_to_buf(cmd->elem->out_sg,
+ cmd->elem->out_num,
+ sizeof(virtio_snd_hdr),
+ &stream_id,
+ sizeof(stream_id));
+
+ cmd->resp.code = sz == sizeof(uint32_t)
+ ? virtio_snd_pcm_prepare(s, stream_id)
+ : VIRTIO_SND_S_BAD_MSG;
+}
+
/*
* Handles VIRTIO_SND_R_PCM_START.
*
@@ -510,6 +531,8 @@ process_cmd(VirtIOSound *s, virtio_snd_ctrl_command *cmd)
virtio_snd_handle_pcm_set_params(s, cmd);
break;
case VIRTIO_SND_R_PCM_PREPARE:
+ virtio_snd_handle_pcm_prepare(s, cmd);
+ break;
case VIRTIO_SND_R_PCM_RELEASE:
cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
break;
--
2.39.2
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v8 08/12] virtio-sound: handle VIRTIO_SND_R_PCM_PREPARE
2023-08-28 19:55 ` [PATCH v8 08/12] virtio-sound: handle VIRTIO_SND_R_PCM_PREPARE Emmanouil Pitsidianakis
@ 2023-08-29 19:33 ` Alex Bennée
0 siblings, 0 replies; 51+ messages in thread
From: Alex Bennée @ 2023-08-29 19:33 UTC (permalink / raw)
To: Emmanouil Pitsidianakis
Cc: qemu-devel, Igor Skalkin, Anton Yakovlev, Paolo Bonzini,
Gerd Hoffmann, Michael S. Tsirkin, Marcel Apfelbaum,
Daniel P. Berrangé, Eduardo Habkost, Marc-André Lureau,
Volker Rümelin, Kővágó, Zoltán,
Philippe Mathieu-Daudé
Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> Handles the PCM prepare control request. It initializes a PCM stream
> when the guests asks for it.
>
> 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>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v8 09/12] virtio-sound: handle VIRTIO_SND_R_PCM_RELEASE
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
` (7 preceding siblings ...)
2023-08-28 19:55 ` [PATCH v8 08/12] virtio-sound: handle VIRTIO_SND_R_PCM_PREPARE Emmanouil Pitsidianakis
@ 2023-08-28 19:55 ` Emmanouil Pitsidianakis
2023-08-29 19:34 ` Alex Bennée
2023-08-28 19:55 ` [PATCH v8 10/12] virtio-sound: implement audio output (TX) Emmanouil Pitsidianakis
` (5 subsequent siblings)
14 siblings, 1 reply; 51+ messages in thread
From: Emmanouil Pitsidianakis @ 2023-08-28 19:55 UTC (permalink / raw)
To: qemu-devel
Cc: Emmanouil Pitsidianakis, Igor Skalkin, Anton Yakovlev,
Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin,
Marcel Apfelbaum, Daniel P. Berrangé, Eduardo Habkost,
Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
Handle the PCM release control request, which is necessary for flushing
pending sound IO. No IO is handled yet so currently it only replies to
the request.
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 | 1 +
hw/virtio/virtio-snd.c | 38 +++++++++++++++++++++++++++++++++++++-
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index f70cde4f01..60ab62a80d 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -167,6 +167,7 @@ virtio_snd_handle_pcm_set_params(uint32_t stream) "VIRTIO_SND_PCM_SET_PARAMS cal
virtio_snd_handle_ctrl(void *vdev, void *vq) "snd %p: handle ctrl event for queue %p"
virtio_snd_handle_pcm_info(uint32_t stream) "VIRTIO_SND_R_PCM_INFO called for stream %"PRIu32
virtio_snd_handle_pcm_start_stop(const char *code, uint32_t stream) "%s called for stream %"PRIu32
+virtio_snd_handle_pcm_release(uint32_t stream) "VIRTIO_SND_PCM_RELEASE called for stream %"PRIu32
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"
diff --git a/hw/virtio/virtio-snd.c b/hw/virtio/virtio-snd.c
index be6eb2c36d..e4fc832c39 100644
--- a/hw/virtio/virtio-snd.c
+++ b/hw/virtio/virtio-snd.c
@@ -487,6 +487,42 @@ static void virtio_snd_handle_pcm_start_stop(VirtIOSound *s,
}
}
+/*
+ * Handles VIRTIO_SND_R_PCM_RELEASE. Releases the buffer resources allocated to
+ * a stream.
+ *
+ * @s: VirtIOSound device
+ * @cmd: The request command queue element from VirtIOSound cmdq field
+ */
+static void virtio_snd_handle_pcm_release(VirtIOSound *s,
+ virtio_snd_ctrl_command *cmd)
+{
+ uint32_t stream_id;
+ VirtIOSoundPCMStream *stream;
+ size_t sz = iov_to_buf(cmd->elem->out_sg,
+ cmd->elem->out_num,
+ sizeof(virtio_snd_hdr),
+ &stream_id,
+ sizeof(stream_id));
+ if (sz != sizeof(uint32_t)) {
+ cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
+ return;
+ }
+
+ trace_virtio_snd_handle_pcm_release(stream_id);
+
+ stream = virtio_snd_pcm_get_stream(s, stream_id);
+ if (!stream) {
+ error_report("already released stream %"PRIu32, stream_id);
+ virtio_error(VIRTIO_DEVICE(s),
+ "already released stream %"PRIu32,
+ stream_id);
+ cmd->resp.code = VIRTIO_SND_S_BAD_MSG;
+ return;
+ }
+ cmd->resp.code = VIRTIO_SND_S_OK;
+}
+
/*
* The actual processing done in virtio_snd_process_cmdq().
*
@@ -534,7 +570,7 @@ process_cmd(VirtIOSound *s, virtio_snd_ctrl_command *cmd)
virtio_snd_handle_pcm_prepare(s, cmd);
break;
case VIRTIO_SND_R_PCM_RELEASE:
- cmd->resp.code = VIRTIO_SND_S_NOT_SUPP;
+ virtio_snd_handle_pcm_release(s, cmd);
break;
case VIRTIO_SND_R_CHMAP_INFO:
qemu_log_mask(LOG_UNIMP,
--
2.39.2
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v8 09/12] virtio-sound: handle VIRTIO_SND_R_PCM_RELEASE
2023-08-28 19:55 ` [PATCH v8 09/12] virtio-sound: handle VIRTIO_SND_R_PCM_RELEASE Emmanouil Pitsidianakis
@ 2023-08-29 19:34 ` Alex Bennée
0 siblings, 0 replies; 51+ messages in thread
From: Alex Bennée @ 2023-08-29 19:34 UTC (permalink / raw)
To: Emmanouil Pitsidianakis
Cc: qemu-devel, Igor Skalkin, Anton Yakovlev, Paolo Bonzini,
Gerd Hoffmann, Michael S. Tsirkin, Marcel Apfelbaum,
Daniel P. Berrangé, Eduardo Habkost, Marc-André Lureau,
Volker Rümelin, Kővágó, Zoltán,
Philippe Mathieu-Daudé
Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> Handle the PCM release control request, which is necessary for flushing
> pending sound IO. No IO is handled yet so currently it only replies to
> the request.
>
> 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>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v8 10/12] virtio-sound: implement audio output (TX)
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
` (8 preceding siblings ...)
2023-08-28 19:55 ` [PATCH v8 09/12] virtio-sound: handle VIRTIO_SND_R_PCM_RELEASE Emmanouil Pitsidianakis
@ 2023-08-28 19:55 ` Emmanouil Pitsidianakis
2023-08-30 13:39 ` Alex Bennée
2023-09-04 10:26 ` Philippe Mathieu-Daudé
2023-08-28 19:55 ` [PATCH v8 11/12] virtio-sound: implement audio capture (RX) Emmanouil Pitsidianakis
` (4 subsequent siblings)
14 siblings, 2 replies; 51+ messages in thread
From: Emmanouil Pitsidianakis @ 2023-08-28 19:55 UTC (permalink / raw)
To: qemu-devel
Cc: Emmanouil Pitsidianakis, Igor Skalkin, Anton Yakovlev,
Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin,
Marcel Apfelbaum, Daniel P. Berrangé, Eduardo Habkost,
Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
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(-)
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;
--
2.39.2
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v8 10/12] virtio-sound: implement audio output (TX)
2023-08-28 19:55 ` [PATCH v8 10/12] virtio-sound: implement audio output (TX) Emmanouil Pitsidianakis
@ 2023-08-30 13:39 ` Alex Bennée
2023-09-04 10:26 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 51+ messages in thread
From: Alex Bennée @ 2023-08-30 13:39 UTC (permalink / raw)
To: Emmanouil Pitsidianakis
Cc: qemu-devel, Igor Skalkin, Anton Yakovlev, Paolo Bonzini,
Gerd Hoffmann, Michael S. Tsirkin, Marcel Apfelbaum,
Daniel P. Berrangé, Eduardo Habkost, Marc-André Lureau,
Volker Rümelin, Kővágó, Zoltán,
Philippe Mathieu-Daudé
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>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v8 10/12] virtio-sound: implement audio output (TX)
2023-08-28 19:55 ` [PATCH v8 10/12] virtio-sound: implement audio output (TX) Emmanouil Pitsidianakis
2023-08-30 13:39 ` Alex Bennée
@ 2023-09-04 10:26 ` Philippe Mathieu-Daudé
2023-09-04 10:34 ` Manos Pitsidianakis
1 sibling, 1 reply; 51+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-09-04 10:26 UTC (permalink / raw)
To: Emmanouil Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
Michael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée
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);
> + }
> + }
> +}
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 10/12] virtio-sound: implement audio output (TX)
2023-09-04 10:26 ` Philippe Mathieu-Daudé
@ 2023-09-04 10:34 ` Manos Pitsidianakis
2023-09-04 11:39 ` Philippe Mathieu-Daudé
2023-09-04 21:34 ` Volker Rümelin
0 siblings, 2 replies; 51+ messages in thread
From: Manos Pitsidianakis @ 2023-09-04 10:34 UTC (permalink / raw)
To: Philippe Mathieu-Daudé , qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
ichael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé ,
Eduardo Habkost, Marc-André Lureau, Volker Rü melin,
Kő vá gó , Zoltá n, Alex Benné e
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
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 10/12] virtio-sound: implement audio output (TX)
2023-09-04 10:34 ` Manos Pitsidianakis
@ 2023-09-04 11:39 ` Philippe Mathieu-Daudé
2023-09-04 21:34 ` Volker Rümelin
1 sibling, 0 replies; 51+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-09-04 11:39 UTC (permalink / raw)
To: Manos Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
ichael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-Andr é Lureau, Volker R ü melin,
Kő vá gó , Zoltá n, Alex Benn é e
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.
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 10/12] virtio-sound: implement audio output (TX)
2023-09-04 10:34 ` Manos Pitsidianakis
2023-09-04 11:39 ` Philippe Mathieu-Daudé
@ 2023-09-04 21:34 ` Volker Rümelin
2023-09-05 7:10 ` Volker Rümelin
1 sibling, 1 reply; 51+ messages in thread
From: Volker Rümelin @ 2023-09-04 21:34 UTC (permalink / raw)
To: Manos Pitsidianakis, Philippe Mathieu-Daud é, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
Michael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau,
Kővágó , Zoltán, Alex Bennée
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);
>>
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 10/12] virtio-sound: implement audio output (TX)
2023-09-04 21:34 ` Volker Rümelin
@ 2023-09-05 7:10 ` Volker Rümelin
0 siblings, 0 replies; 51+ messages in thread
From: Volker Rümelin @ 2023-09-05 7:10 UTC (permalink / raw)
To: Manos Pitsidianakis, Philippe Mathieu-Daud é, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
Michael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau,
Kővágó , Zoltán, Alex Bennée
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);
>>>
>
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v8 11/12] virtio-sound: implement audio capture (RX)
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
` (9 preceding siblings ...)
2023-08-28 19:55 ` [PATCH v8 10/12] virtio-sound: implement audio output (TX) Emmanouil Pitsidianakis
@ 2023-08-28 19:55 ` Emmanouil Pitsidianakis
2023-08-30 13:40 ` Alex Bennée
` (2 more replies)
2023-08-28 19:55 ` [PATCH v8 12/12] docs/system: add basic virtio-snd documentation Emmanouil Pitsidianakis
` (3 subsequent siblings)
14 siblings, 3 replies; 51+ messages in thread
From: Emmanouil Pitsidianakis @ 2023-08-28 19:55 UTC (permalink / raw)
To: qemu-devel
Cc: Emmanouil Pitsidianakis, Igor Skalkin, Anton Yakovlev,
Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin,
Marcel Apfelbaum, Daniel P. Berrangé, Eduardo Habkost,
Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
To perform audio capture we duplicate the TX logic of the previous
commit with the following difference: we receive data from the QEMU
audio backend and write it in the virt queue IO buffers the guest sends
to QEMU. When they are full (i.e. they have `period_bytes` amount of
data) or when recording stops in QEMU's audio backend, the buffer is
returned to the guest by notifying it.
Signed-off-by: Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>
---
hw/virtio/trace-events | 3 +-
hw/virtio/virtio-snd.c | 245 +++++++++++++++++++++++++++++++++++------
2 files changed, 215 insertions(+), 33 deletions(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 3b95e745c2..9b7fbffedc 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -172,4 +172,5 @@ virtio_snd_handle_code(uint32_t val, const char *code) "ctrl code msg val = %"PR
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"
+virtio_snd_handle_tx_xfer(void) "tx queue callback called"
+virtio_snd_handle_rx_xfer(void) "rx queue callback called"
diff --git a/hw/virtio/virtio-snd.c b/hw/virtio/virtio-snd.c
index 4859ce4bf6..70e8a73072 100644
--- a/hw/virtio/virtio-snd.c
+++ b/hw/virtio/virtio-snd.c
@@ -27,18 +27,21 @@
#define VIRTIO_SOUND_VM_VERSION 1
#define VIRTIO_SOUND_JACK_DEFAULT 0
-#define VIRTIO_SOUND_STREAM_DEFAULT 1
+#define VIRTIO_SOUND_STREAM_DEFAULT 2
#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 void virtio_snd_pcm_out_flush(VirtIOSoundPCMStream *stream);
+static void virtio_snd_pcm_in_flush(VirtIOSoundPCMStream *stream);
+static void virtio_snd_pcm_in_cb(void *data, int available);
+static uint32_t virtio_snd_pcm_write(VirtIOSoundPCMStream *stream,
+ VirtQueue *vq,
+ VirtQueueElement *element);
+static uint32_t virtio_snd_pcm_read(VirtIOSoundPCMStream *stream,
+ VirtQueue *vq,
+ VirtQueueElement *element);
static uint32_t supported_formats = BIT(VIRTIO_SND_PCM_FMT_S8)
| BIT(VIRTIO_SND_PCM_FMT_U8)
@@ -394,6 +397,9 @@ static void virtio_snd_pcm_close(VirtIOSoundPCMStream *stream)
if (stream->direction == VIRTIO_SND_D_OUTPUT) {
AUD_close_out(&stream->pcm->snd->card, stream->voice.out);
stream->voice.out = NULL;
+ } else if (stream->direction == VIRTIO_SND_D_INPUT) {
+ AUD_close_in(&stream->pcm->snd->card, stream->voice.in);
+ stream->voice.in = NULL;
}
qemu_mutex_destroy(&stream->queue_mutex);
g_free(stream);
@@ -456,7 +462,12 @@ static uint32_t virtio_snd_pcm_prepare(VirtIOSound *s, uint32_t stream_id)
virtio_snd_pcm_out_cb,
&as);
} else {
- qemu_log_mask(LOG_UNIMP, "virtio_snd: input/capture is unimplemented.");
+ stream->voice.in = AUD_open_in(&s->card,
+ stream->voice.in,
+ "virtio-sound.in",
+ stream,
+ virtio_snd_pcm_in_cb,
+ &as);
}
stream->as = as;
@@ -522,6 +533,8 @@ static void virtio_snd_handle_pcm_start_stop(VirtIOSound *s,
if (stream) {
if (stream->direction == VIRTIO_SND_D_OUTPUT) {
AUD_set_active_out(stream->voice.out, start);
+ } else {
+ AUD_set_active_in(stream->voice.in, start);
}
/* remove previous buffers. */
WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) {
@@ -604,7 +617,11 @@ static void virtio_snd_handle_pcm_release(VirtIOSound *s,
*/
virtio_snd_process_cmdq(stream->s);
trace_virtio_snd_pcm_stream_flush(stream_id);
- virtio_snd_pcm_flush(stream);
+ if (stream->direction == VIRTIO_SND_D_OUTPUT) {
+ virtio_snd_pcm_out_flush(stream);
+ } else {
+ virtio_snd_pcm_in_flush(stream);
+ }
}
cmd->resp.code = VIRTIO_SND_S_OK;
@@ -763,7 +780,7 @@ static void virtio_snd_handle_event(VirtIODevice *vdev, VirtQueue *vq)
* @vdev: VirtIOSound device
* @vq: tx virtqueue
*/
-static void virtio_snd_handle_tx(VirtIODevice *vdev, VirtQueue *vq)
+static void virtio_snd_handle_tx_xfer(VirtIODevice *vdev, VirtQueue *vq)
{
VirtIOSound *s = VIRTIO_SND(vdev);
VirtIOSoundPCMStream *stream = NULL;
@@ -772,7 +789,7 @@ static void virtio_snd_handle_tx(VirtIODevice *vdev, VirtQueue *vq)
virtio_snd_pcm_xfer hdr;
virtio_snd_pcm_status resp = { 0 };
- trace_virtio_snd_handle_xfer();
+ trace_virtio_snd_handle_tx_xfer();
for (;;) {
elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
@@ -796,11 +813,11 @@ static void virtio_snd_handle_tx(VirtIODevice *vdev, VirtQueue *vq)
goto tx_err;
}
+ assert(hdr.stream_id != VIRTIO_SND_D_INPUT);
WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) {
- virtio_snd_pcm_read_write(stream,
+ virtio_snd_pcm_write(stream,
vq,
- elem,
- hdr.stream_id == VIRTIO_SND_D_INPUT);
+ elem);
resp.status = VIRTIO_SND_S_OK;
iov_from_buf(elem->in_sg,
@@ -830,12 +847,66 @@ tx_err:
}
/*
- * Stub buffer virtqueue handler.
+ * The rx virtqueue handler. Makes the buffers available to their respective
+ * streams for consumption.
*
* @vdev: VirtIOSound device
- * @vq: virtqueue
+ * @vq: tx virtqueue
*/
-static void virtio_snd_handle_xfer(VirtIODevice *vdev, VirtQueue *vq) {}
+static void virtio_snd_handle_rx_xfer(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_rx_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]) {
+ continue;
+ }
+
+ stream = s->pcm->streams[hdr.stream_id];
+ if (stream->direction != VIRTIO_SND_D_INPUT) {
+ goto rx_err;
+ }
+ WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) {
+ virtio_snd_pcm_read(stream, vq, elem);
+ }
+ continue;
+
+rx_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);
+}
static uint64_t get_features(VirtIODevice *vdev, uint64_t features,
Error **errp)
@@ -930,9 +1001,9 @@ 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_tx);
+ virtio_add_queue(vdev, 64, virtio_snd_handle_tx_xfer);
vsnd->queues[VIRTIO_SND_VQ_RX] =
- virtio_add_queue(vdev, 64, virtio_snd_handle_xfer);
+ virtio_add_queue(vdev, 64, virtio_snd_handle_rx_xfer);
qemu_mutex_init(&vsnd->cmdq_mutex);
QTAILQ_INIT(&vsnd->cmdq);
@@ -1002,26 +1073,119 @@ static void virtio_snd_pcm_out_cb(void *data, int available)
}
/*
- * Flush all buffer data from this stream's queue into the driver's virtual
- * queue.
+ * AUD_* input callback.
*
- * @stream: VirtIOSoundPCMStream *stream
+ * @data: VirtIOSoundPCMStream stream
+ * @available: number of bytes that can be read with AUD_read()
*/
-static void virtio_snd_pcm_flush(VirtIOSoundPCMStream *stream)
+static void virtio_snd_pcm_in_cb(void *data, int available)
{
+ VirtIOSoundPCMStream *stream = data;
VirtIOSoundPCMBlock *block;
- VirtIOSoundPCMBlock *next;
+ uint32_t sz;
+ virtio_snd_pcm_status resp = { 0 };
+ size_t size;
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);
+ while (!QSIMPLEQ_EMPTY(&stream->queue)) {
+ block = QSIMPLEQ_FIRST(&stream->queue);
+
+ for (;;) {
+ size = AUD_read(stream->voice.in,
+ block->data + block->offset,
+ MIN(stream->period_bytes - block->offset, available));
+ block->offset += size;
+ block->size += size;
+ if (size == 0 || block->size >= stream->period_bytes) {
+ resp.status = VIRTIO_SND_S_OK;
+ sz = iov_from_buf(block->elem->in_sg,
+ block->elem->in_num,
+ 0,
+ &resp,
+ sizeof(resp));
+
+ /* Copy data -if any- to guest */
+ if (block->size) {
+ iov_from_buf(block->elem->in_sg,
+ block->elem->in_num,
+ sz,
+ block->data,
+ MIN(stream->period_bytes, 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;
+ }
}
}
}
+#define virtio_snd_pcm_flush(AUD_CB) \
+ VirtIOSoundPCMBlock *block; \
+ VirtIOSoundPCMBlock *next; \
+ WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { \
+ QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) { \
+ do { \
+ AUD_CB; \
+ } while (0) \
+ ; \
+ virtqueue_push(block->vq, block->elem, sizeof(block->elem));\
+ virtio_notify(VIRTIO_DEVICE(stream->s), block->vq); \
+ QSIMPLEQ_REMOVE(&stream->queue, \
+ block, \
+ VirtIOSoundPCMBlock, \
+ entry); \
+ } \
+ } \
+
+
+/*
+ * Flush all buffer data from this output stream's queue into the driver's
+ * virtual queue.
+ *
+ * @stream: VirtIOSoundPCMStream *stream
+ */
+static void virtio_snd_pcm_out_flush(VirtIOSoundPCMStream *stream)
+{
+ virtio_snd_pcm_flush(
+ AUD_write(stream->voice.out,
+ block->data + block->offset,
+ block->size);
+ );
+}
+
+/*
+ * Flush all buffer data from this input stream's queue into the driver's
+ * virtual queue.
+ *
+ * @stream: VirtIOSoundPCMStream *stream
+ */
+static void virtio_snd_pcm_in_flush(VirtIOSoundPCMStream *stream)
+{
+ virtio_snd_pcm_flush(
+ iov_from_buf(block->elem->in_sg,
+ block->elem->in_num,
+ sizeof(virtio_snd_pcm_info),
+ block->data,
+ block->offset);
+ );
+}
+
static void virtio_snd_unrealize(DeviceState *dev)
{
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -1056,10 +1220,9 @@ static void virtio_snd_unrealize(DeviceState *dev)
static uint32_t
-virtio_snd_pcm_read_write(VirtIOSoundPCMStream *stream,
+virtio_snd_pcm_write(VirtIOSoundPCMStream *stream,
VirtQueue *vq,
- VirtQueueElement *element,
- bool read)
+ VirtQueueElement *element)
{
VirtIOSoundPCMBlock *fragment;
size_t size = iov_size(element->out_sg, element->out_num) -
@@ -1081,6 +1244,24 @@ virtio_snd_pcm_read_write(VirtIOSoundPCMStream *stream,
return fragment->size;
}
+static uint32_t
+virtio_snd_pcm_read(VirtIOSoundPCMStream *stream,
+ VirtQueue *vq,
+ VirtQueueElement *element)
+{
+ VirtIOSoundPCMBlock *fragment;
+
+ fragment = g_malloc0(sizeof(VirtIOSoundPCMBlock) + stream->period_bytes);
+ fragment->elem = element;
+ fragment->vq = vq;
+ fragment->size = 0;
+ fragment->offset = 0;
+
+ QSIMPLEQ_INSERT_TAIL(&stream->queue, fragment, entry);
+
+ return fragment->size;
+}
+
static void virtio_snd_reset(VirtIODevice *vdev)
{
VirtIOSound *s = VIRTIO_SND(vdev);
--
2.39.2
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v8 11/12] virtio-sound: implement audio capture (RX)
2023-08-28 19:55 ` [PATCH v8 11/12] virtio-sound: implement audio capture (RX) Emmanouil Pitsidianakis
@ 2023-08-30 13:40 ` Alex Bennée
2023-09-08 6:49 ` Volker Rümelin
2023-09-09 8:56 ` Volker Rümelin
2 siblings, 0 replies; 51+ messages in thread
From: Alex Bennée @ 2023-08-30 13:40 UTC (permalink / raw)
To: Emmanouil Pitsidianakis
Cc: qemu-devel, Igor Skalkin, Anton Yakovlev, Paolo Bonzini,
Gerd Hoffmann, Michael S. Tsirkin, Marcel Apfelbaum,
Daniel P. Berrangé, Eduardo Habkost, Marc-André Lureau,
Volker Rümelin, Kővágó, Zoltán,
Philippe Mathieu-Daudé
Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> To perform audio capture we duplicate the TX logic of the previous
> commit with the following difference: we receive data from the QEMU
> audio backend and write it in the virt queue IO buffers the guest sends
> to QEMU. When they are full (i.e. they have `period_bytes` amount of
> data) or when recording stops in QEMU's audio backend, the buffer is
> returned to the guest by notifying it.
>
> Signed-off-by: Emmanouil Pitsidianakis
> <manos.pitsidianakis@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v8 11/12] virtio-sound: implement audio capture (RX)
2023-08-28 19:55 ` [PATCH v8 11/12] virtio-sound: implement audio capture (RX) Emmanouil Pitsidianakis
2023-08-30 13:40 ` Alex Bennée
@ 2023-09-08 6:49 ` Volker Rümelin
2023-09-09 8:56 ` Volker Rümelin
2 siblings, 0 replies; 51+ messages in thread
From: Volker Rümelin @ 2023-09-08 6:49 UTC (permalink / raw)
To: Emmanouil Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
Michael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
Am 28.08.23 um 21:55 schrieb Emmanouil Pitsidianakis:
> To perform audio capture we duplicate the TX logic of the previous
> commit with the following difference: we receive data from the QEMU
> audio backend and write it in the virt queue IO buffers the guest sends
> to QEMU. When they are full (i.e. they have `period_bytes` amount of
> data) or when recording stops in QEMU's audio backend, the buffer is
> returned to the guest by notifying it.
>
> Signed-off-by: Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
> hw/virtio/trace-events | 3 +-
> hw/virtio/virtio-snd.c | 245 +++++++++++++++++++++++++++++++++++------
> 2 files changed, 215 insertions(+), 33 deletions(-)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 3b95e745c2..9b7fbffedc 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -172,4 +172,5 @@ virtio_snd_handle_code(uint32_t val, const char *code) "ctrl code msg val = %"PR
> 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"
> +virtio_snd_handle_tx_xfer(void) "tx queue callback called"
> +virtio_snd_handle_rx_xfer(void) "rx queue callback called"
> diff --git a/hw/virtio/virtio-snd.c b/hw/virtio/virtio-snd.c
> index 4859ce4bf6..70e8a73072 100644
> --- a/hw/virtio/virtio-snd.c
> +++ b/hw/virtio/virtio-snd.c
> @@ -1002,26 +1073,119 @@ static void virtio_snd_pcm_out_cb(void *data, int available)
> }
>
> /*
> - * Flush all buffer data from this stream's queue into the driver's virtual
> - * queue.
> + * AUD_* input callback.
> *
> - * @stream: VirtIOSoundPCMStream *stream
> + * @data: VirtIOSoundPCMStream stream
> + * @available: number of bytes that can be read with AUD_read()
> */
> -static void virtio_snd_pcm_flush(VirtIOSoundPCMStream *stream)
> +static void virtio_snd_pcm_in_cb(void *data, int available)
> {
> + VirtIOSoundPCMStream *stream = data;
> VirtIOSoundPCMBlock *block;
> - VirtIOSoundPCMBlock *next;
> + uint32_t sz;
> + virtio_snd_pcm_status resp = { 0 };
> + size_t size;
>
> 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);
> + while (!QSIMPLEQ_EMPTY(&stream->queue)) {
> + block = QSIMPLEQ_FIRST(&stream->queue);
> +
> + for (;;) {
> + size = AUD_read(stream->voice.in,
> + block->data + block->offset,
> + MIN(stream->period_bytes - block->offset, available));
For the -audiodev in.mixing-engine=off case, 'available' is much larger
than the available bytes AUD_read() can actually read. 'size' is 0 when
the upstream buffer is empty.
+ if (size == 0) {
+ available = 0;
+ break;
+ }
This fixes audio recording with -audiodev
pipewire,out.mixing-engine=off,in.mixing-engine=off,id=audio0. The issue
with two wrong audio frames every 25ms remains.
> + block->offset += size;
> + block->size += size;
> + if (size == 0 || block->size >= stream->period_bytes) {
- if (size == 0 || block->size >= stream->period_bytes) {
+ if (block->size >= stream->period_bytes) {
With best regards,
Volker
> + resp.status = VIRTIO_SND_S_OK;
> + sz = iov_from_buf(block->elem->in_sg,
> + block->elem->in_num,
> + 0,
> + &resp,
> + sizeof(resp));
> +
> + /* Copy data -if any- to guest */
> + if (block->size) {
> + iov_from_buf(block->elem->in_sg,
> + block->elem->in_num,
> + sz,
> + block->data,
> + MIN(stream->period_bytes, 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;
> + }
> }
> }
> }
>
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 11/12] virtio-sound: implement audio capture (RX)
2023-08-28 19:55 ` [PATCH v8 11/12] virtio-sound: implement audio capture (RX) Emmanouil Pitsidianakis
2023-08-30 13:40 ` Alex Bennée
2023-09-08 6:49 ` Volker Rümelin
@ 2023-09-09 8:56 ` Volker Rümelin
2 siblings, 0 replies; 51+ messages in thread
From: Volker Rümelin @ 2023-09-09 8:56 UTC (permalink / raw)
To: Emmanouil Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
Michael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
Am 28.08.23 um 21:55 schrieb Emmanouil Pitsidianakis:
> To perform audio capture we duplicate the TX logic of the previous
> commit with the following difference: we receive data from the QEMU
> audio backend and write it in the virt queue IO buffers the guest sends
> to QEMU. When they are full (i.e. they have `period_bytes` amount of
> data) or when recording stops in QEMU's audio backend, the buffer is
> returned to the guest by notifying it.
>
> Signed-off-by: Emmanouil Pitsidianakis<manos.pitsidianakis@linaro.org>
> ---
> hw/virtio/trace-events | 3 +-
> hw/virtio/virtio-snd.c | 245 +++++++++++++++++++++++++++++++++++------
> 2 files changed, 215 insertions(+), 33 deletions(-)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 3b95e745c2..9b7fbffedc 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -172,4 +172,5 @@ virtio_snd_handle_code(uint32_t val, const char *code) "ctrl code msg val = %"PR
> 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"
> +virtio_snd_handle_tx_xfer(void) "tx queue callback called"
> +virtio_snd_handle_rx_xfer(void) "rx queue callback called"
> diff --git a/hw/virtio/virtio-snd.c b/hw/virtio/virtio-snd.c
> index 4859ce4bf6..70e8a73072 100644
> --- a/hw/virtio/virtio-snd.c
> +++ b/hw/virtio/virtio-snd.c
> @@ -1002,26 +1073,119 @@ static void virtio_snd_pcm_out_cb(void *data, int available)
> }
>
> /*
> - * Flush all buffer data from this stream's queue into the driver's virtual
> - * queue.
> + * AUD_* input callback.
> *
> - * @stream: VirtIOSoundPCMStream *stream
> + * @data: VirtIOSoundPCMStream stream
> + * @available: number of bytes that can be read with AUD_read()
> */
> -static void virtio_snd_pcm_flush(VirtIOSoundPCMStream *stream)
> +static void virtio_snd_pcm_in_cb(void *data, int available)
> {
> + VirtIOSoundPCMStream *stream = data;
> VirtIOSoundPCMBlock *block;
> - VirtIOSoundPCMBlock *next;
> + uint32_t sz;
> + virtio_snd_pcm_status resp = { 0 };
> + size_t size;
>
> 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);
> + while (!QSIMPLEQ_EMPTY(&stream->queue)) {
> + block = QSIMPLEQ_FIRST(&stream->queue);
> +
> + for (;;) {
> + size = AUD_read(stream->voice.in,
> + block->data + block->offset,
> + MIN(stream->period_bytes - block->offset, available));
> + block->offset += size;
> + block->size += size;
> + if (size == 0 || block->size >= stream->period_bytes) {
> + resp.status = VIRTIO_SND_S_OK;
Hi Manos,
the type of resp.status is le32. I doubt that the virtio-sound device
will work on a big endian host at the moment. There are more issues like
this in virtio-snd.c.
> + sz = iov_from_buf(block->elem->in_sg,
> + block->elem->in_num,
> + 0,
> + &resp,
> + sizeof(resp));
> +
> + /* Copy data -if any- to guest */
> + if (block->size) {
> + iov_from_buf(block->elem->in_sg,
> + block->elem->in_num,
> + sz,
> + block->data,
> + MIN(stream->period_bytes, block->size));
> + }
The order of the parts of the PCM input stream message is wrong. The
buffer comes first, then the status part. See virtio-v1.2-cs01
5.14.6.8PCM I/O Messages. The correct order fixes the problem with the
wrong audio frames every 25ms. The two wrong audio frames were the
status part of the message.
> + virtqueue_push(block->vq,
> + block->elem,
> + sizeof(block->elem));
sizeof(block->elem)? This is the size of a pointer. The PCM I/O Message
is much larger. The other virtqueue_push() call sites also don't look right.
With best regards,
Volker
> + 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;
> + }
> }
> }
> }
>
>
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v8 12/12] docs/system: add basic virtio-snd documentation
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
` (10 preceding siblings ...)
2023-08-28 19:55 ` [PATCH v8 11/12] virtio-sound: implement audio capture (RX) Emmanouil Pitsidianakis
@ 2023-08-28 19:55 ` Emmanouil Pitsidianakis
2023-08-30 17:49 ` Alex Bennée
2023-08-30 13:40 ` [PATCH v8 00/12] Add VIRTIO sound card Alex Bennée
` (2 subsequent siblings)
14 siblings, 1 reply; 51+ messages in thread
From: Emmanouil Pitsidianakis @ 2023-08-28 19:55 UTC (permalink / raw)
To: qemu-devel
Cc: Emmanouil Pitsidianakis, Igor Skalkin, Anton Yakovlev,
Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin,
Marcel Apfelbaum, Daniel P. Berrangé, Eduardo Habkost,
Marc-André Lureau, Volker Rümelin,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
This commit adds basic documentation for using virtio-snd.
Signed-off-by: Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>
---
docs/system/device-emulation.rst | 1 +
docs/system/devices/virtio-snd.rst | 49 ++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
create mode 100644 docs/system/devices/virtio-snd.rst
diff --git a/docs/system/device-emulation.rst b/docs/system/device-emulation.rst
index 4491c4cbf7..dae19446e5 100644
--- a/docs/system/device-emulation.rst
+++ b/docs/system/device-emulation.rst
@@ -92,6 +92,7 @@ Emulated Devices
devices/usb.rst
devices/vhost-user.rst
devices/virtio-pmem.rst
+ devices/virtio-snd.rst
devices/vhost-user-rng.rst
devices/canokey.rst
devices/usb-u2f.rst
diff --git a/docs/system/devices/virtio-snd.rst b/docs/system/devices/virtio-snd.rst
new file mode 100644
index 0000000000..2a9187fd70
--- /dev/null
+++ b/docs/system/devices/virtio-snd.rst
@@ -0,0 +1,49 @@
+virtio sound
+============
+
+This document explains the setup and usage of the Virtio sound device.
+The Virtio sound device is a paravirtualized sound card device.
+
+Linux kernel support
+--------------------
+
+Virtio sound requires a guest Linux kernel built with the
+``CONFIG_SND_VIRTIO`` option.
+
+Description
+-----------
+
+Virtio sound implements capture and playback from inside a guest using the
+configured audio backend of the host machine.
+
+Device properties
+-----------------
+
+The Virtio sound device can be configured with the following properties:
+
+ * ``jacks`` number of physical jacks (Unimplemented).
+ * ``streams`` number of PCM streams. At the moment, no stream configuration is supported: the first one will always be a playback stream, an optional second will always be a capture stream. Adding more will cycle stream directions from playback to capture.
+ * ``chmaps`` number of channel maps (Unimplemented).
+
+All streams are stereo and have the default channel positions ``Front left, right``.
+
+Examples
+--------
+
+Add an audio device and an audio backend at once with ``-audio`` and ``model=virtio``:
+
+ * pulseaudio: ``-audio driver=pa,model=virtio``
+ or ``-audio driver=pa,model=virtio,server=/run/user/1000/pulse/native``
+ * sdl: ``-audio driver=sdl,model=virtio``
+ * coreaudio: ``-audio driver=coreaudio,model=virtio``
+
+etc.
+
+To specifically add virtualized sound devices, you have to specify a PCI device
+and an audio backend listed with ``-audio driver=help`` that works on your host
+machine, e.g.:
+
+::
+
+ -device virtio-sound-pci,audiodev=my_audiodev \
+ -audiodev alsa,id=my_audiodev
--
2.39.2
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v8 12/12] docs/system: add basic virtio-snd documentation
2023-08-28 19:55 ` [PATCH v8 12/12] docs/system: add basic virtio-snd documentation Emmanouil Pitsidianakis
@ 2023-08-30 17:49 ` Alex Bennée
0 siblings, 0 replies; 51+ messages in thread
From: Alex Bennée @ 2023-08-30 17:49 UTC (permalink / raw)
To: Emmanouil Pitsidianakis
Cc: qemu-devel, Igor Skalkin, Anton Yakovlev, Paolo Bonzini,
Gerd Hoffmann, Michael S. Tsirkin, Marcel Apfelbaum,
Daniel P. Berrangé, Eduardo Habkost, Marc-André Lureau,
Volker Rümelin, Kővágó, Zoltán,
Philippe Mathieu-Daudé
Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> This commit adds basic documentation for using virtio-snd.
>
> Signed-off-by: Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
> docs/system/device-emulation.rst | 1 +
> docs/system/devices/virtio-snd.rst | 49 ++++++++++++++++++++++++++++++
> 2 files changed, 50 insertions(+)
> create mode 100644 docs/system/devices/virtio-snd.rst
>
> diff --git a/docs/system/device-emulation.rst b/docs/system/device-emulation.rst
> index 4491c4cbf7..dae19446e5 100644
> --- a/docs/system/device-emulation.rst
> +++ b/docs/system/device-emulation.rst
> @@ -92,6 +92,7 @@ Emulated Devices
> devices/usb.rst
> devices/vhost-user.rst
> devices/virtio-pmem.rst
> + devices/virtio-snd.rst
> devices/vhost-user-rng.rst
> devices/canokey.rst
> devices/usb-u2f.rst
> diff --git a/docs/system/devices/virtio-snd.rst b/docs/system/devices/virtio-snd.rst
> new file mode 100644
> index 0000000000..2a9187fd70
> --- /dev/null
> +++ b/docs/system/devices/virtio-snd.rst
> @@ -0,0 +1,49 @@
> +virtio sound
> +============
> +
> +This document explains the setup and usage of the Virtio sound device.
> +The Virtio sound device is a paravirtualized sound card device.
> +
> +Linux kernel support
> +--------------------
> +
> +Virtio sound requires a guest Linux kernel built with the
> +``CONFIG_SND_VIRTIO`` option.
> +
> +Description
> +-----------
> +
> +Virtio sound implements capture and playback from inside a guest using the
> +configured audio backend of the host machine.
> +
> +Device properties
> +-----------------
> +
> +The Virtio sound device can be configured with the following properties:
> +
> + * ``jacks`` number of physical jacks (Unimplemented).
> + * ``streams`` number of PCM streams. At the moment, no stream configuration is supported: the first one will always be a playback stream, an optional second will always be a capture stream. Adding more will cycle stream directions from playback to capture.
> + * ``chmaps`` number of channel maps (Unimplemented).
I think you can drop the unimplemented properties from the docs and the
code. No point having them if they do nothing and you might change your
mind on the name when they are implemented ;-)
> +
> +All streams are stereo and have the default channel positions ``Front left, right``.
> +
> +Examples
> +--------
> +
> +Add an audio device and an audio backend at once with ``-audio`` and ``model=virtio``:
> +
> + * pulseaudio: ``-audio driver=pa,model=virtio``
> + or ``-audio driver=pa,model=virtio,server=/run/user/1000/pulse/native``
> + * sdl: ``-audio driver=sdl,model=virtio``
> + * coreaudio: ``-audio driver=coreaudio,model=virtio``
> +
> +etc.
> +
> +To specifically add virtualized sound devices, you have to specify a PCI device
> +and an audio backend listed with ``-audio driver=help`` that works on your host
> +machine, e.g.:
I'd be tempted to start with the specific example and then mention the
shortcut. I'm curious as to how model= resolves on various platforms or
how you would tell from the command line.
> +
> +::
> +
> + -device virtio-sound-pci,audiodev=my_audiodev \
> + -audiodev alsa,id=my_audiodev
Otherwise:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v8 00/12] Add VIRTIO sound card
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
` (11 preceding siblings ...)
2023-08-28 19:55 ` [PATCH v8 12/12] docs/system: add basic virtio-snd documentation Emmanouil Pitsidianakis
@ 2023-08-30 13:40 ` Alex Bennée
2023-09-04 7:20 ` Volker Rümelin
2023-09-06 8:39 ` Matias Ezequiel Vara Larsen
14 siblings, 0 replies; 51+ messages in thread
From: Alex Bennée @ 2023-08-30 13:40 UTC (permalink / raw)
To: Emmanouil Pitsidianakis
Cc: qemu-devel, Igor Skalkin, Anton Yakovlev, Paolo Bonzini,
Gerd Hoffmann, Michael S. Tsirkin, Marcel Apfelbaum,
Daniel P. Berrangé, Eduardo Habkost, Marc-André Lureau,
Volker Rümelin, Kővágó, Zoltán,
Philippe Mathieu-Daudé
Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> This patch series adds an audio device implementing the recent virtio
> sound spec (1.2) and a corresponding PCI wrapper device.
And you can have a:
Tested-by: Alex Bennée <alex.bennee@linaro.org>
for the whole series.
mst,
are you going to pull via your tree?
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 00/12] Add VIRTIO sound card
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
` (12 preceding siblings ...)
2023-08-30 13:40 ` [PATCH v8 00/12] Add VIRTIO sound card Alex Bennée
@ 2023-09-04 7:20 ` Volker Rümelin
2023-09-04 10:01 ` Manos Pitsidianakis
2023-09-06 8:39 ` Matias Ezequiel Vara Larsen
14 siblings, 1 reply; 51+ messages in thread
From: Volker Rümelin @ 2023-09-04 7:20 UTC (permalink / raw)
To: Emmanouil Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
Michael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
Am 28.08.23 um 21:54 schrieb Emmanouil Pitsidianakis:
> This patch series adds an audio device implementing the recent virtio
> sound spec (1.2) and a corresponding PCI wrapper device.
Hi Manos,
I have a few more general comments.
All qemu_log_mask() format strings need a trailing \n.
I still hear a lot of playback dropouts. I had planned to look at the
playback code, but I didn't have the time until now.
Compared to v6 audio recording has improved but there are bugs. When I
start QEMU with -audiodev
pipewire,out.frequency=48000,in.frequency=48000,id=audio0 there are two
either uninitialized or stale samples every 25ms in the recorded audio
stream.
To reproduce the issue start audacity on the host and generate a 2s
square wave tone with 315Hz and an amplitude of 0.8. Use pavucontrol to
select the monitor of your host playback device as QEMU recording
device. In the guest start recording with audacity. Start playback of
the generated square wave on the host. Stop recording in the guest and
have a look at a 200ms sequence of the recorded square wave and notice
the wrong samples every 25ms.
When I start QEMU with -audiodev
pipewire,out.mixing-engine=off,in.mixing-engine=off,id=audio0 audio
recording starts but the recorded stream immediately stalls.
With best regards,
Volker
> v8 can be found online at:
>
> https://gitlab.com/epilys/qemu/-/tree/virtio-snd-v8
>
> Ref 69eb5f4fbae731f5fc05dea8a5f4b656e0de127f
>
> Main differences with v7 patch series [^v7]
> <cover.1692731646.git.manos.pitsidianakis@linaro.org>:
>
> - Addressed [^v7] review comments.
> Functions that were called from more than one place for code re-use
> are not created until they are actually needed.
> - Fixed cases where block->offset was not respected in Playback
>
>
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v8 00/12] Add VIRTIO sound card
2023-09-04 7:20 ` Volker Rümelin
@ 2023-09-04 10:01 ` Manos Pitsidianakis
2023-09-04 12:11 ` Alex Bennée
2023-09-05 6:56 ` Volker Rümelin
0 siblings, 2 replies; 51+ messages in thread
From: Manos Pitsidianakis @ 2023-09-04 10:01 UTC (permalink / raw)
To: Volker Rü melin, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
ichael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé ,
Eduardo Habkost, Marc-André Lureau,
Kő vá gó , Zoltá n, Alex Benné e,
Philippe Mathieu-Daudé
Hello Volker :)
On Mon, 04 Sep 2023 10:20, Volker Rümelin <vr_qemu@t-online.de> wrote:
>All qemu_log_mask() format strings need a trailing \n.
Thank you, will fix it!
>I still hear a lot of playback dropouts. I had planned to look at the
>playback code, but I didn't have the time until now.
>
>Compared to v6 audio recording has improved but there are bugs. When I
>start QEMU with -audiodev
>pipewire,out.frequency=48000,in.frequency=48000,id=audio0 there are two
>either uninitialized or stale samples every 25ms in the recorded audio
>stream.
>
>To reproduce the issue start audacity on the host and generate a 2s
>square wave tone with 315Hz and an amplitude of 0.8. Use pavucontrol to
>select the monitor of your host playback device as QEMU recording
>device. In the guest start recording with audacity. Start playback of
>the generated square wave on the host. Stop recording in the guest and
>have a look at a 200ms sequence of the recorded square wave and notice
>the wrong samples every 25ms.
We've noticed this and decided to fix it in the future. I think the
problem lies when PCM release is called from the guest. Quoting the
spec:
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.
When RELEASE is received, buffers are simply dropped. This is pure
conjecture but I think creating an in-device buffer could solve this.
Unless the bug is found to be caused by something else, I settled on
accepting it for this patch series because it is spec conformant.
>When I start QEMU with -audiodev
>pipewire,out.mixing-engine=off,in.mixing-engine=off,id=audio0 audio
>recording starts but the recorded stream immediately stalls.
Can you elaborate? Do you mean you repeat the same process as before,
but the stall happens immediately? I personally rarely get any drops I
could notice, only one or two for many minutes of playback / capture. I
also could not reproduce exactly the same behavior you had in the
previous version. The bugs *were* there but it was not as severe. Maybe
it's a hardware performance issue? Can someone else test this too? It'd
be helpful.
Thank you very much for your help,
Manos
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v8 00/12] Add VIRTIO sound card
2023-09-04 10:01 ` Manos Pitsidianakis
@ 2023-09-04 12:11 ` Alex Bennée
2023-09-05 6:03 ` Volker Rümelin
2023-09-05 6:56 ` Volker Rümelin
1 sibling, 1 reply; 51+ messages in thread
From: Alex Bennée @ 2023-09-04 12:11 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: Volker Rü melin, qemu-devel, Igor Skalkin, Anton Yakovlev,
Paolo Bonzini, Gerd Hoffmann, ichael S. Tsirkin, Marcel Apfelbaum,
Daniel P. Berrangé, Eduardo Habkost, Marc-André Lureau,
Kő vá gó , Zoltá n,
Philippe Mathieu-Daudé
Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> Hello Volker :)
>
> On Mon, 04 Sep 2023 10:20, Volker Rümelin <vr_qemu@t-online.de> wrote:
>>All qemu_log_mask() format strings need a trailing \n.
>
> Thank you, will fix it!
>
>> I still hear a lot of playback dropouts. I had planned to look at
>> the playback code, but I didn't have the time until now.
>>
>> Compared to v6 audio recording has improved but there are bugs. When
>> I start QEMU with -audiodev
>> pipewire,out.frequency=48000,in.frequency=48000,id=audio0 there are
>> two either uninitialized or stale samples every 25ms in the recorded
>> audio stream.
>>
>> To reproduce the issue start audacity on the host and generate a 2s
>> square wave tone with 315Hz and an amplitude of 0.8. Use pavucontrol
>> to select the monitor of your host playback device as QEMU recording
>> device. In the guest start recording with audacity. Start playback
>> of the generated square wave on the host. Stop recording in the
>> guest and have a look at a 200ms sequence of the recorded square
>> wave and notice the wrong samples every 25ms.
>
> We've noticed this and decided to fix it in the future. I think the
> problem lies when PCM release is called from the guest. Quoting the
> spec:
>
> 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.
>
> When RELEASE is received, buffers are simply dropped. This is pure
> conjecture but I think creating an in-device buffer could solve this.
> Unless the bug is found to be caused by something else, I settled on
> accepting it for this patch series because it is spec conformant.
Volker,
Can you run with:
-d trace:virtio_snd\*
to confirm you are seeing the same behaviour. The experience I had with
ogg123 in an emulated guest was it would work fine but then the next run
I would get audio corruption. You can see this if you see lots of
START/STOP/RELEASE messages constantly restarting things. If you are
getting corruption without this pattern that is something else which we
should investigate before merging.
>
>> When I start QEMU with -audiodev
>> pipewire,out.mixing-engine=off,in.mixing-engine=off,id=audio0 audio
>> recording starts but the recorded stream immediately stalls.
>
> Can you elaborate? Do you mean you repeat the same process as before,
> but the stall happens immediately? I personally rarely get any drops I
> could notice, only one or two for many minutes of playback / capture.
> I also could not reproduce exactly the same behavior you had in the
> previous version. The bugs *were* there but it was not as severe.
> Maybe it's a hardware performance issue? Can someone else test this
> too? It'd be helpful.
>
> Thank you very much for your help,
> Manos
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v8 00/12] Add VIRTIO sound card
2023-09-04 12:11 ` Alex Bennée
@ 2023-09-05 6:03 ` Volker Rümelin
0 siblings, 0 replies; 51+ messages in thread
From: Volker Rümelin @ 2023-09-05 6:03 UTC (permalink / raw)
To: Alex Bennée, Manos Pitsidianakis
Cc: qemu-devel, Igor Skalkin, Anton Yakovlev, Paolo Bonzini,
Gerd Hoffmann, Michael S. Tsirkin, Marcel Apfelbaum,
Daniel P. Berrangé, Eduardo Habkost, Marc-André Lureau,
Kővágó, Zoltán, Philippe Mathieu-Daudé
Am 04.09.23 um 14:11 schrieb Alex Bennée:
> Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
>
>> Hello Volker :)
>>
>> On Mon, 04 Sep 2023 10:20, Volker Rümelin <vr_qemu@t-online.de> wrote:
>>> All qemu_log_mask() format strings need a trailing \n.
>> Thank you, will fix it!
>>
>>> I still hear a lot of playback dropouts. I had planned to look at
>>> the playback code, but I didn't have the time until now.
>>>
>>> Compared to v6 audio recording has improved but there are bugs. When
>>> I start QEMU with -audiodev
>>> pipewire,out.frequency=48000,in.frequency=48000,id=audio0 there are
>>> two either uninitialized or stale samples every 25ms in the recorded
>>> audio stream.
>>>
>>> To reproduce the issue start audacity on the host and generate a 2s
>>> square wave tone with 315Hz and an amplitude of 0.8. Use pavucontrol
>>> to select the monitor of your host playback device as QEMU recording
>>> device. In the guest start recording with audacity. Start playback
>>> of the generated square wave on the host. Stop recording in the
>>> guest and have a look at a 200ms sequence of the recorded square
>>> wave and notice the wrong samples every 25ms.
>> We've noticed this and decided to fix it in the future. I think the
>> problem lies when PCM release is called from the guest. Quoting the
>> spec:
>>
>> 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.
>>
>> When RELEASE is received, buffers are simply dropped. This is pure
>> conjecture but I think creating an in-device buffer could solve this.
>> Unless the bug is found to be caused by something else, I settled on
>> accepting it for this patch series because it is spec conformant.
> Volker,
>
> Can you run with:
>
> -d trace:virtio_snd\*
>
> to confirm you are seeing the same behaviour. The experience I had with
> ogg123 in an emulated guest was it would work fine but then the next run
> I would get audio corruption. You can see this if you see lots of
> START/STOP/RELEASE messages constantly restarting things. If you are
> getting corruption without this pattern that is something else which we
> should investigate before merging.
Hi Alex,
I only see a START message when I start recording with audacity and a
STOP message approximately 5s after I stop recording. This is when guest
PulseAudio disables the virtio-sound device.
9881@1693892230.558732:virtio_snd_handle_ctrl snd 0x55ac9e6eb590: handle
ctrl event for queue 0x55ac9e6f4160
9881@1693892230.558780:virtio_snd_handle_code ctrl code msg val = 257 ==
VIRTIO_SND_R_PCM_SET_PARAMS
9881@1693892230.558797:virtio_snd_handle_pcm_set_params
VIRTIO_SND_PCM_SET_PARAMS called for stream 1
9881@1693892230.559132:virtio_snd_handle_ctrl snd 0x55ac9e6eb590: handle
ctrl event for queue 0x55ac9e6f4160
9881@1693892230.559158:virtio_snd_handle_code ctrl code msg val = 258 ==
VIRTIO_SND_R_PCM_PREPARE
9881@1693892230.562202:virtio_snd_handle_rx_xfer rx queue callback called
9881@1693892230.562365:virtio_snd_handle_ctrl snd 0x55ac9e6eb590: handle
ctrl event for queue 0x55ac9e6f4160
9881@1693892230.562395:virtio_snd_handle_code ctrl code msg val = 260 ==
VIRTIO_SND_R_PCM_START
9881@1693892230.562411:virtio_snd_handle_pcm_start_stop
VIRTIO_SND_R_PCM_START called for stream 1
9881@1693892230.562557:virtio_snd_handle_rx_xfer rx queue callback called
9881@1693892230.810029:virtio_snd_handle_rx_xfer rx queue callback called
9881@1693892230.840194:virtio_snd_handle_rx_xfer rx queue callback called
9881@1693892230.860279:virtio_snd_handle_rx_xfer rx queue callback called
... a lot of 'virtio_snd_handle_rx_xfer rx queue callback called'
messages every 20ms - 30ms
9881@1693892238.510774:virtio_snd_handle_rx_xfer rx queue callback called
9881@1693892238.530895:virtio_snd_handle_rx_xfer rx queue callback called
9881@1693892238.561123:virtio_snd_handle_rx_xfer rx queue callback called
9881@1693892238.566280:virtio_snd_handle_ctrl snd 0x55ac9e6eb590: handle
ctrl event for queue 0x55ac9e6f4160
9881@1693892238.566290:virtio_snd_handle_code ctrl code msg val = 261 ==
VIRTIO_SND_R_PCM_STOP
9881@1693892238.566293:virtio_snd_handle_pcm_start_stop
VIRTIO_SND_R_PCM_STOP called for stream 1
9881@1693892238.566415:virtio_snd_handle_ctrl snd 0x55ac9e6eb590: handle
ctrl event for queue 0x55ac9e6f4160
9881@1693892238.566424:virtio_snd_handle_code ctrl code msg val = 259 ==
VIRTIO_SND_R_PCM_RELEASE
9881@1693892238.566428:virtio_snd_handle_pcm_release
VIRTIO_SND_PCM_RELEASE called for stream 1
With best regards,
Volker
>>> When I start QEMU with -audiodev
>>> pipewire,out.mixing-engine=off,in.mixing-engine=off,id=audio0 audio
>>> recording starts but the recorded stream immediately stalls.
>> Can you elaborate? Do you mean you repeat the same process as before,
>> but the stall happens immediately? I personally rarely get any drops I
>> could notice, only one or two for many minutes of playback / capture.
>> I also could not reproduce exactly the same behavior you had in the
>> previous version. The bugs *were* there but it was not as severe.
>> Maybe it's a hardware performance issue? Can someone else test this
>> too? It'd be helpful.
>>
>> Thank you very much for your help,
>> Manos
>
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v8 00/12] Add VIRTIO sound card
2023-09-04 10:01 ` Manos Pitsidianakis
2023-09-04 12:11 ` Alex Bennée
@ 2023-09-05 6:56 ` Volker Rümelin
1 sibling, 0 replies; 51+ messages in thread
From: Volker Rümelin @ 2023-09-05 6:56 UTC (permalink / raw)
To: Manos Pitsidianakis, qemu-devel
Cc: Igor Skalkin, Anton Yakovlev, Paolo Bonzini, Gerd Hoffmann,
Michael S. Tsirkin, Marcel Apfelbaum, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau,
Kővágó, Zoltán, Alex Bennée,
Philippe Mathieu-Daudé
Am 04.09.23 um 12:01 schrieb Manos Pitsidianakis:
> Hello Volker :)
>
> On Mon, 04 Sep 2023 10:20, Volker Rümelin <vr_qemu@t-online.de> wrote:
>> All qemu_log_mask() format strings need a trailing \n.
>
> Thank you, will fix it!
>
>> I still hear a lot of playback dropouts. I had planned to look at the
>> playback code, but I didn't have the time until now.
>>
>> Compared to v6 audio recording has improved but there are bugs. When
>> I start QEMU with -audiodev
>> pipewire,out.frequency=48000,in.frequency=48000,id=audio0 there are
>> two either uninitialized or stale samples every 25ms in the recorded
>> audio stream.
>>
>> To reproduce the issue start audacity on the host and generate a 2s
>> square wave tone with 315Hz and an amplitude of 0.8. Use pavucontrol
>> to select the monitor of your host playback device as QEMU recording
>> device. In the guest start recording with audacity. Start playback of
>> the generated square wave on the host. Stop recording in the guest
>> and have a look at a 200ms sequence of the recorded square wave and
>> notice the wrong samples every 25ms.
>
> We've noticed this and decided to fix it in the future. I think the
> problem lies when PCM release is called from the guest. Quoting the spec:
>
> 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.
>
> When RELEASE is received, buffers are simply dropped. This is pure
> conjecture but I think creating an in-device buffer could solve this.
> Unless the bug is found to be caused by something else, I settled on
> accepting it for this patch series because it is spec conformant.
>
>> When I start QEMU with -audiodev
>> pipewire,out.mixing-engine=off,in.mixing-engine=off,id=audio0 audio
>> recording starts but the recorded stream immediately stalls.
>
> Can you elaborate? Do you mean you repeat the same process as before,
> but the stall happens immediately? I personally rarely get any drops I
> could notice, only one or two for many minutes of playback / capture.
> I also could not reproduce exactly the same behavior you had in the
> previous version. The bugs *were* there but it was not as severe.
> Maybe it's a hardware performance issue? Can someone else test this
> too? It'd be helpful.
For this test I only start QEMU with the mixing engine disabled. When I
start recording with audacity, audacity reports 'Recording' in the
status line but it actually doesn't record. The recording marker in the
tracks window stays a 0.
I don't think it's a hardware performance issue. Other QEMU audio
devices don't show this behaviour.
With best regards,
Volker
>
> Thank you very much for your help,
> Manos
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v8 00/12] Add VIRTIO sound card
2023-08-28 19:54 [PATCH v8 00/12] Add VIRTIO sound card Emmanouil Pitsidianakis
` (13 preceding siblings ...)
2023-09-04 7:20 ` Volker Rümelin
@ 2023-09-06 8:39 ` Matias Ezequiel Vara Larsen
14 siblings, 0 replies; 51+ messages in thread
From: Matias Ezequiel Vara Larsen @ 2023-09-06 8:39 UTC (permalink / raw)
To: Emmanouil Pitsidianakis
Cc: qemu-devel, Igor Skalkin, Anton Yakovlev, Paolo Bonzini,
Gerd Hoffmann, Michael S. Tsirkin, Marcel Apfelbaum,
Daniel P. Berrangé, Eduardo Habkost, Marc-André Lureau,
Volker Rümelin, Kővágó, Zoltán,
Alex Bennée, Philippe Mathieu-Daudé
On Mon, Aug 28, 2023 at 10:54:57PM +0300, Emmanouil Pitsidianakis wrote:
> This patch series adds an audio device implementing the recent virtio
> sound spec (1.2) and a corresponding PCI wrapper device.
>
> v8 can be found online at:
>
> https://gitlab.com/epilys/qemu/-/tree/virtio-snd-v8
>
Hello Manos,
I have been testing these patches in a 6.4.6 Linux guest by using the
command:
`aplay /usr/share/sounds/alsa/Front_Left.wav`
There is some audio that is reproduced twice, such as "Front ... Front
Left". I have also observed this in the virtio-sound vhost-user-device.
I'm still trying to figure out why this is happening.
Thanks, Matias.
^ permalink raw reply [flat|nested] 51+ messages in thread