[RFC PATCH 00/27] Virtio sound card implementation

[RFC PATCH 00/27] Virtio sound card implementation

[Shreyansh Chouhan]

This patch series aims to implement the virtio sound card
as defined in the virtio specs (v8). The specs can be found
at the following github repo:
https://github.com/oasis-tcs/virtio-spec

This patch series is not complete yet, but here is what's
already been done:

    - The device is initialized properly and is recognized
      by the guest as a sound card device.
    - Output stream works but the output is very noisy. (Which
      is what I wanted coments on.)

What remains to be done:

    - Input streams yet to be done.
    - The jacks are initialized with a default config, but
      they are not mapped to any streams for now.
    - Channel maps are yet to be implemented.

I'd like to request some comments on the following points:

    - The output from the sound card is accompanied by periodic
      white noise. I do not know why this is happening. I tried
      debugging it by writing the buffers to a new wav file and
      sure enough the contents of the file were different at
      some places, but I couldn't find what must be causing it.
      (Relevant patches: #19, #20, #21 and #25.) What steps should
      I take for debugging this?

    - If I try and output a wav file of a different size, that
      sets the period_bytes to 4004, I get an assert failure in
      the object_unref function defined in qom/object.c. (Function
      defined on line #681, assert on line #690.)
                assert(obj->parent == NULL);
      I tried taking a look at the stack trace for when this failure
      happens. In the stacktrace I found out that this happened
      when QEMU was trying to unmap the out_sg from the VirtQueue
      element. This failure doesn't happen if I am using a different
      wav file, that sets the period_bytes to something else.
      (Relevant patches: #19, #20, #21 and #25.)
      What could be causing this problem?

    - What is the suggested way of waiting? When the driver issues
      the VIRTIO_SND_PCM_STOP ctrl command I want to wait for the
      buffers existing in tx vq to be consumed before closing the
      stream.

Shreyansh Chouhan (27):
  virtio-snd: Add virtio sound header file
  virtio-snd: Add jack control structures
  virtio-snd: Add PCM control structures
  virtio-snd: Add chmap control structures
  virtio-snd: Add device implementation structures
  virtio-snd: Add PCI wrapper code for VirtIOSound
  virtio-snd: Add properties for class init
  virtio-snd: Add code for get config function
  virtio-snd: Add code for set config function
  virtio-snd: Add code for the realize function
  virtio-snd: Add macros for logging
  virtio-snd: Add control virtqueue handler
  virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler
  virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler
  virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler
  virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle
  virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler
  virtio-snd: Add default configs to realize fn
  virtio-snd: Add callback for SWVoiceOut
  virtio-snd: Add VIRITO_SND_R_PCM_START handler
  virtio-snd: Add VIRTIO_SND_R_PCM_STOP handler
  virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler
  virtio-snd: Replaced goto with if else
  virtio-snd: Add code to device unrealize function
  virtio-snd: Add tx vq and handler
  virtio-snd: Add event vq and a handler stub
  virtio-snd: Add rx vq and stub handler

 hw/audio/Kconfig               |    5 +
 hw/audio/meson.build           |    1 +
 hw/audio/virtio-snd.c          | 1168 ++++++++++++++++++++++++++++++++
 hw/virtio/meson.build          |    1 +
 hw/virtio/virtio-snd-pci.c     |   72 ++
 include/hw/virtio/virtio-snd.h |  393 +++++++++++
 6 files changed, 1640 insertions(+)
 create mode 100644 hw/audio/virtio-snd.c
 create mode 100644 hw/virtio/virtio-snd-pci.c
 create mode 100644 include/hw/virtio/virtio-snd.h

-- 
2.25.1

[RFC PATCH 01/27] virtio-snd: Add virtio sound header file

[Shreyansh Chouhan]

Added device configuration and common definitions to the header
file.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 include/hw/virtio/virtio-snd.h | 97 ++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 include/hw/virtio/virtio-snd.h

diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
new file mode 100644
index 0000000000..bbbf174c51
--- /dev/null
+++ b/include/hw/virtio/virtio-snd.h
@@ -0,0 +1,97 @@
+/*
+ * Virtio Sound Device
+ */
+
+#ifndef QEMU_VIRTIO_SOUND_H
+#define QEMU_VIRTIO_SOUND_H
+
+#include "qemu/units.h"
+#include "hw/virtio/virtio.h"
+#include "qemu/queue.h"
+#include "audio/audio.h"
+#include "audio/audio_int.h"
+
+#define VIRTIO_ID_SOUND 25
+
+/* CONFIGURATION SPACE */
+
+typedef struct virtio_snd_config {
+    /* # of jacks available */
+    uint32_t jacks;
+    /* # of streams avalable */
+    uint32_t streams;
+    /* # chmaps available */
+    uint32_t chmaps;
+} virtio_snd_config;
+
+/* COMMON DEFINITIONS */
+
+/* supported sample data directions. */
+enum {
+    VIRTIO_SND_D_OUTPUT = 0,
+    VIRTIO_SND_D_INPUT
+};
+
+enum {
+    /* jack control request types */
+    VIRTIO_SND_R_JACK_INFO = 1,
+    VIRTIO_SND_R_JACK_REMAP,
+
+    /* PCM control request types */
+    VIRTIO_SND_R_PCM_INFO = 0x0100,
+    VIRTIO_SND_R_PCM_SET_PARAMS,
+    VIRTIO_SND_R_PCM_PREPARE,
+    VIRTIO_SND_R_PCM_RELEASE,
+    VIRTIO_SND_R_PCM_START,
+    VIRTIO_SND_R_PCM_STOP,
+
+    /* channel map control request type */
+    VIRTIO_SND_R_CHMAP_INFO = 0x200,
+
+    /* jack event types */
+    VIRTIO_SND_EVT_JACK_CONNECTED = 0x1000,
+    VIRTIO_SND_EVT_JACK_DISCONNECTED,
+
+    /* PCM event types */
+    VIRTIO_SND_EVT_PCM_PERIOD_ELAPSED = 0x1100,
+    VIRTIO_SND_EVT_PCM_XRUN,
+
+    /* common status codes */
+    VIRTIO_SND_S_OK = 0x8000,
+    VIRTIO_SND_S_BAD_MSG,
+    VIRTIO_SND_S_NOT_SUPP,
+    VIRTIO_SND_S_IO_ERR
+};
+
+/* common header for request/response*/
+typedef struct virtio_snd_hdr {
+    uint32_t code;
+} virtio_snd_hdr;
+
+/* event notification */
+typedef struct virtio_snd_event {
+    /* VIRTIO_SND_EVT_* */
+    virtio_snd_hdr hdr;
+    /* Optional event data */
+    uint32_t data;
+} virtio_snd_event;
+
+/* common control request to query an item information */
+typedef struct virtio_snd_query_info {
+    /* VIRTIO_SND_R_*_INFO */
+    struct virtio_snd_hdr hdr;
+    /* item start identifier */
+    uint32_t start_id;
+    /* # of items to query */
+    uint32_t count;
+    /* size of a single item information in bytes */
+    uint32_t size;
+} virtio_snd_query_info;
+
+/* common item information header */
+typedef struct virtio_snd_info {
+    /* functional group node id (HDA Spec 7.1.2) */
+    uint32_t hda_fn_nid;
+} virtio_snd_info;
+
+#endif
-- 
2.25.1

Re: [RFC PATCH 01/27] virtio-snd: Add virtio sound header file

[Gerd Hoffmann]

On Thu, Apr 29, 2021 at 05:34:19PM +0530, Shreyansh Chouhan wrote:
> Added device configuration and common definitions to the header
> file.
> 
> Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
> ---
>  include/hw/virtio/virtio-snd.h | 97 ++++++++++++++++++++++++++++++++++

This is the place for the data structures used by qemu internally.

The structs and #defines for the guest/host protocol should go to
include/standard-headers/linux/ instead.

There is a script (scripts/update-linux-headers.sh) to sync the files
with the linux source tree content.  As far I know the linux guest
driver is scheduled for merge in the 5.13 merge window, so once 5.13-rc1
is out of the door this should start working.

Syncing against a development tree is possible too as temporary stopgap,
but some extra care is needed then to avoid unwanted changes to
non-virtio-sound files slipping in.

Possibly it makes sense to also sync other linux header files that way
(for hda jacks maybe?).  We also import the input headers to get all the
KEY_* #defines for example.

take care,
  Gerd

[RFC PATCH 02/27] virtio-snd: Add jack control structures

[Shreyansh Chouhan]

Added structures for handling jack control requests
to the header file.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 include/hw/virtio/virtio-snd.h | 40 ++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
index bbbf174c51..f58b06a949 100644
--- a/include/hw/virtio/virtio-snd.h
+++ b/include/hw/virtio/virtio-snd.h
@@ -94,4 +94,44 @@ typedef struct virtio_snd_info {
     uint32_t hda_fn_nid;
 } virtio_snd_info;
 
+/* JACK CONTROL MESSAGES */
+
+typedef struct virtio_snd_jack_hdr {
+    /* VIRTIO_SND_R_JACK_* */
+    virtio_snd_hdr hdr;
+    /* 0 to (virtio_snd_config.jacks - 1) */
+    uint32_t jack_id;
+} virtio_snd_jack_hdr;
+
+/* Supported jack features */
+enum {
+    VIRTIO_SND_F_JACK_REMAP = 0
+};
+
+/* jack information structure */
+typedef struct virtio_snd_jack_info {
+    /* common header */
+    virtio_snd_info hdr;
+    /* 1 << VIRTIO_SND_JACK_F_* */
+    uint32_t features;
+    /* pin default configuration from HDA spec */
+    uint32_t hda_reg_defconf;
+    /* pin capabilities from HDA spec */
+    uint32_t hda_reg_caps;
+    /* connection status (0: disconnected, 1: connected) */
+    uint8_t connected;
+
+    uint8_t padding[7];
+} virtio_snd_jack_info;
+
+/* jack remapping control request */
+typedef struct virtio_snd_jack_remap {
+    /* .code = VIRTIO_SND_R_JACK_REMAP */
+    virtio_snd_jack_hdr hdr;
+    /* selected association number */
+    uint32_t association;
+    /* selected sequence number */
+    uint32_t sequence;
+} virtio_snd_jack_remap;
+
 #endif
-- 
2.25.1

[RFC PATCH 03/27] virtio-snd: Add PCM control structures

[Shreyansh Chouhan]

Added structures for handle PCM control requests
to the heaer file.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 include/hw/virtio/virtio-snd.h | 128 +++++++++++++++++++++++++++++++++
 1 file changed, 128 insertions(+)

diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
index f58b06a949..e9a4fe3c5d 100644
--- a/include/hw/virtio/virtio-snd.h
+++ b/include/hw/virtio/virtio-snd.h
@@ -134,4 +134,132 @@ typedef struct virtio_snd_jack_remap {
     uint32_t sequence;
 } virtio_snd_jack_remap;
 
+/* PCM CONTROL MESSAGES */
+
+typedef struct virtio_snd_pcm_hdr {
+    /* .code = VIRTIO_SND_R_PCM_* */
+    virtio_snd_hdr hdr;
+    /* 0 to (virtio_snd_config.streams - 1) */
+    uint32_t stream_id;
+} virtio_snd_pcm_hdr;
+
+
+/* Supported PCM stream features */
+enum {
+    VIRTIO_SND_PCM_F_SHMEM_HOST = 0,
+    VIRTIO_SND_PCM_F_SHMEM_GUEST,
+    VIRTIO_SND_PCM_F_MSG_POLLING,
+    VIRTIO_SND_PCM_F_EVT_SHMEM_PERIODS,
+    VIRTIO_SNDPCM_F_EVT_XRUNS
+};
+
+/* PCM stream flags */
+enum {
+    VIRTIO_SND_PCM_FL_CHMAP = 0
+};
+
+/* Supported sample formats */
+enum {
+    /* analog formats (width / physical width) */
+    VIRTIO_SND_PCM_FMT_IMA_ADPCM = 0,   /*  4 /  4 bits */
+    VIRTIO_SND_PCM_FMT_MU_LAW,          /*  8 /  8 bits */
+    VIRTIO_SND_PCM_FMT_A_LAW,           /*  8 /  8 bits */
+    VIRTIO_SND_PCM_FMT_S8,              /*  8 /  8 bits */
+    VIRTIO_SND_PCM_FMT_U8,              /*  8 /  8 bits */
+    VIRTIO_SND_PCM_FMT_S16,             /* 16 / 16 bits */
+    VIRTIO_SND_PCM_FMT_U16,             /* 16 / 16 bits */
+    VIRTIO_SND_PCM_FMT_S18_3,           /* 18 / 24 bits */
+    VIRTIO_SND_PCM_FMT_U18_3,           /* 18 / 24 bits */
+    VIRTIO_SND_PCM_FMT_S20_3,           /* 20 / 24 bits */
+    VIRTIO_SND_PCM_FMT_U20_3,           /* 20 / 24 bits */
+    VIRTIO_SND_PCM_FMT_S24_3,           /* 24 / 24 bits */
+    VIRTIO_SND_PCM_FMT_U24_3,           /* 24 / 24 bits */
+    VIRTIO_SND_PCM_FMT_S20,             /* 20 / 32 bits */
+    VIRTIO_SND_PCM_FMT_U20,             /* 20 / 32 bits */
+    VIRTIO_SND_PCM_FMT_S24,             /* 24 / 32 bits */
+    VIRTIO_SND_PCM_FMT_U24,             /* 24 / 32 bits */
+    VIRTIO_SND_PCM_FMT_S32,             /* 32 / 32 bits */
+    VIRTIO_SND_PCM_FMT_U32,             /* 32 / 32 bits */
+    VIRTIO_SND_PCM_FMT_FLOAT,           /* 32 / 32 bits */
+    VIRTIO_SND_PCM_FMT_FLOAT64,         /* 64 / 64 bits */
+    /* digital formats (width / physical width) */
+    VIRTIO_SND_PCM_FMT_DSD_U8,          /*  8 /  8 bits */
+    VIRTIO_SND_PCM_FMT_DSD_U16,         /* 16 / 16 bits */
+    VIRTIO_SND_PCM_FMT_DSD_U32,         /* 32 / 32 bits */
+    VIRTIO_SND_PCM_FMT_IEC958_SUBFRAME  /* 32 / 32 bits */
+};
+
+/* Supported PCM frame rates */
+enum {
+    VIRTIO_SND_PCM_RATE_5512 = 0,
+    VIRTIO_SND_PCM_RATE_8000,
+    VIRTIO_SND_PCM_RATE_11025,
+    VIRTIO_SND_PCM_RATE_16000,
+    VIRTIO_SND_PCM_RATE_22050,
+    VIRTIO_SND_PCM_RATE_32000,
+    VIRTIO_SND_PCM_RATE_44100,
+    VIRTIO_SND_PCM_RATE_48000,
+    VIRTIO_SND_PCM_RATE_64000,
+    VIRTIO_SND_PCM_RATE_88200,
+    VIRTIO_SND_PCM_RATE_96000,
+    VIRTIO_SND_PCM_RATE_176399,
+    VIRTIO_SND_PCM_RATE_192000,
+    VIRTIO_SND_PCM_RATE_384000
+};
+
+/* PCM stream info structure */
+typedef struct virtio_snd_pcm_info {
+    /* common header */
+    virtio_snd_info hdr;
+    /* supported features bitmap (1 << VIRTIO_SND_PCM_F_*) */
+    uint32_t features;
+    /* supported sample formats bitmap (1 << VIRTIO_SND_PCM_FMT_*) */
+    uint64_t formats;
+    /* supported sample rates bitmap (1 << VIRTIO_SND_PCM_RATE_*) */
+    uint64_t rates;
+    /* direction of the stream (VIRTIO_SND_D_*) */
+    uint8_t direction;
+    /* min # of supported channels */
+    uint8_t channels_min;
+    /* max # of supported channels */
+    uint8_t channels_max;
+
+    uint8_t padding[5];
+} virtio_snd_pcm_info;
+
+/* set PCM stream params */
+typedef struct virtio_snd_pcm_set_params {
+    virtio_snd_pcm_hdr hdr;
+    /* size of hardware buffer in bytes */
+    uint32_t buffer_bytes;
+    /* size of hardware period in bytes */
+    uint32_t period_bytes;
+    /* selected feature bitmap */
+    uint32_t features;
+    /* number of channels */
+    uint8_t channel;
+    /* VIRTIO_SND_PCM_FMT_* */
+    uint8_t format;
+    /* VIRTIO_SND_PCM_RATE_* */
+    uint8_t rate;
+
+    uint8_t padding;
+} virtio_snd_pcm_set_params;
+
+/* PCM I/O MESSAGES */
+
+/* I/O request header */
+typedef struct virtio_snd_pcm_xfer {
+    /* 0 to (virtio_snd_config.stream - 1 */
+    uint32_t stream_id;
+} virtio_snd_pcm_xfer;
+
+/* I/O request status */
+typedef struct virtio_snd_pcm_status {
+    /* VIRTIO_SND_S_* */
+    uint32_t status;
+    /* current device latency */
+    uint32_t latency_bytes;
+} virtio_snd_pcm_status;
+
 #endif
-- 
2.25.1

[RFC PATCH 04/27] virtio-snd: Add chmap control structures

[Shreyansh Chouhan]

Added structures for handling channel map control
requests to the header file.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 include/hw/virtio/virtio-snd.h | 64 ++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
index e9a4fe3c5d..ad068e5893 100644
--- a/include/hw/virtio/virtio-snd.h
+++ b/include/hw/virtio/virtio-snd.h
@@ -262,4 +262,68 @@ typedef struct virtio_snd_pcm_status {
     uint32_t latency_bytes;
 } virtio_snd_pcm_status;
 
+/* CHANNEL MAP CONTROL MESSAGES */
+
+typedef struct virtio_snd_chmap_hdr {
+    /* .code = VIRTIO_SND_R_CHMAP_* */
+    virtio_snd_hdr hdr;
+    /* 0 to (virtio_snd_config.chmaps - 1) */
+    uint32_t chmap_id;
+} virtio_snd_chmap_hdr;
+
+/* standard channel position definition */
+enum {
+    VIRTIO_SND_CHMAP_NONE = 0,  /* undefined */
+    VIRTIO_SND_CHMAP_NA,        /* silent */
+    VIRTIO_SND_CHMAP_MONO,      /* mono stream */
+    VIRTIO_SND_CHMAP_FL,        /* front left */
+    VIRTIO_SND_CHMAP_FR,        /* front right */
+    VIRTIO_SND_CHMAP_RL,        /* rear left */
+    VIRTIO_SND_CHMAP_RR,        /* rear right */
+    VIRTIO_SND_CHMAP_FC,        /* front center */
+    VIRTIO_SND_CHMAP_LFE,       /* low frequency (LFE) */
+    VIRTIO_SND_CHMAP_SL,        /* side left */
+    VIRTIO_SND_CHMAP_SR,        /* side right */
+    VIRTIO_SND_CHMAP_RC,        /* rear center */
+    VIRTIO_SND_CHMAP_FLC,       /* front left center */
+    VIRTIO_SND_CHMAP_FRC,       /* front right center */
+    VIRTIO_SND_CHMAP_RLC,       /* rear left center */
+    VIRTIO_SND_CHMAP_RRC,       /* rear right center */
+    VIRTIO_SND_CHMAP_FLW,       /* front left wide */
+    VIRTIO_SND_CHMAP_FRW,       /* front right wide */
+    VIRTIO_SND_CHMAP_FLH,       /* front left high */
+    VIRTIO_SND_CHMAP_FCH,       /* front center high */
+    VIRTIO_SND_CHMAP_FRH,       /* front right high */
+    VIRTIO_SND_CHMAP_TC,        /* top center */
+    VIRTIO_SND_CHMAP_TFL,       /* top front left */
+    VIRTIO_SND_CHMAP_TFR,       /* top front right */
+    VIRTIO_SND_CHMAP_TFC,       /* top front center */
+    VIRTIO_SND_CHMAP_TRL,       /* top rear left */
+    VIRTIO_SND_CHMAP_TRR,       /* top rear right */
+    VIRTIO_SND_CHMAP_TRC,       /* top rear center */
+    VIRTIO_SND_CHMAP_TFLC,      /* top front left center */
+    VIRTIO_SND_CHMAP_TFRC,      /* top front right center */
+    VIRTIO_SND_CHMAP_TSL,       /* top side left */
+    VIRTIO_SND_CHMAP_TSR,       /* top side right */
+    VIRTIO_SND_CHMAP_LLFE,      /* left LFE */
+    VIRTIO_SND_CHMAP_RLFE,      /* right LFE */
+    VIRTIO_SND_CHMAP_BC,        /* bottom center */
+    VIRTIO_SND_CHMAP_BLC,       /* bottom left center */
+    VIRTIO_SND_CHMAP_BRC        /* bottom right center */
+};
+
+/* maximum possible number of channels */
+#define VIRTIO_SND_CHMAP_MAX_SIZE   18
+
+typedef struct virtio_snd_chmap_info {
+    /* common header */
+    virtio_snd_info hdr;
+    /* direction */
+    uint8_t direction;
+    /* # of valid channel position values */
+    uint8_t channels;
+    /* channel position values (VIRTIO_SND_CHMAP_*) */
+    uint8_t positions[VIRTIO_SND_CHMAP_MAX_SIZE];
+} virtio_snd_chmap_info;
+
 #endif
-- 
2.25.1

[RFC PATCH 05/27] virtio-snd: Add device implementation structures

[Shreyansh Chouhan]

Added jacks, pcm streams and the VirtIOSound structure for actual
device implementation.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 include/hw/virtio/virtio-snd.h | 64 ++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
index ad068e5893..6ab131db50 100644
--- a/include/hw/virtio/virtio-snd.h
+++ b/include/hw/virtio/virtio-snd.h
@@ -13,6 +13,9 @@
 
 #define VIRTIO_ID_SOUND 25
 
+#define TYPE_VIRTIO_SOUND "virtio-sound-device"
+OBJECT_DECLARE_SIMPLE_TYPE(VirtIOSound, VIRTIO_SOUND)
+
 /* CONFIGURATION SPACE */
 
 typedef struct virtio_snd_config {
@@ -326,4 +329,65 @@ typedef struct virtio_snd_chmap_info {
     uint8_t positions[VIRTIO_SND_CHMAP_MAX_SIZE];
 } virtio_snd_chmap_info;
 
+/* VIRTIO SOUND DEVICE */
+
+/* Jacks */
+typedef struct virtio_snd_jack {
+    uint32_t features; /* 1 << VIRTIO_SND_JACK_F_XXX */
+    uint32_t hda_fn_nid;
+    uint32_t hda_reg_defconf;
+    uint32_t hda_reg_caps;
+    uint8_t connected;
+} virtio_snd_jack;
+
+/* Streams */
+typedef struct virtio_snd_pcm_stream {
+    uint32_t hda_fn_nid;
+    uint32_t buffer_bytes;
+    uint32_t period_bytes;
+    uint32_t features; /* 1 << VIRTIO_SND_PCM_F_XXX */
+    uint32_t flags; /* 1 << VIRTIO_SND_PCM_FL_XXX */
+    uint32_t direction;
+    uint8_t channels_min;
+    uint8_t channels_max;
+    uint64_t formats; /* 1 << VIRTIO_SND_PCM_FMT_XXX */
+    uint64_t rates; /* 1 << VIRTIO_SND_PCM_RATE_XXX */
+    int tail, r_pos, w_pos;
+    VirtQueueElement **elems;
+    VirtIOSound *s;
+    union {
+        SWVoiceIn *in;
+        SWVoiceOut *out;
+    } voice;
+} virtio_snd_pcm_stream;
+
+/* Stream params */
+typedef struct virtio_snd_pcm_params {
+    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 channel;
+    uint8_t format;
+    uint8_t rate;
+} virtio_snd_pcm_params;
+
+/* Sound device */
+struct VirtIOSound {
+    /* Parent VirtIODevice object */
+    VirtIODevice parent_obj;
+    virtio_snd_config snd_conf;
+
+    VirtQueue *ctrl_vq;
+    VirtQueue *event_vq;
+    VirtQueue *tx_vq;
+    VirtQueue *rx_vq;
+
+    QEMUSoundCard card;
+    size_t config_size;
+
+    virtio_snd_pcm_params **pcm_params;
+    virtio_snd_pcm_stream **streams;
+    virtio_snd_jack **jacks;
+};
+
 #endif
-- 
2.25.1

[RFC PATCH 06/27] virtio-snd: Add PCI wrapper code for VirtIOSound

[Shreyansh Chouhan]

Added the virito-snd.c file which contains a wrapper
for combining the device with the VirtIOPCIProxy.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/virtio/meson.build      |  1 +
 hw/virtio/virtio-snd-pci.c | 72 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)
 create mode 100644 hw/virtio/virtio-snd-pci.c

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index fbff9bc9d4..a4593f7631 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -45,6 +45,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_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
 
diff --git a/hw/virtio/virtio-snd-pci.c b/hw/virtio/virtio-snd-pci.c
new file mode 100644
index 0000000000..8d8e4ffa51
--- /dev/null
+++ b/hw/virtio/virtio-snd-pci.c
@@ -0,0 +1,72 @@
+/*
+ * Virtio sound PCI Bindings
+ */
+
+#include "qemu/osdep.h"
+#include "hw/virtio/virtio-snd.h"
+#include "hw/virtio/virtio-pci.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+#include "qom/object.h"
+
+typedef struct VirtIOSoundPCI VirtIOSoundPCI;
+
+/*
+ * virtio-snd-pci: This extends VirtioPCIProxy.
+ */
+#define TYPE_VIRTIO_SOUND_PCI "virtio-sound-pci-base"
+DECLARE_INSTANCE_CHECKER(VirtIOSoundPCI, VIRTIO_SOUND_PCI,
+                         TYPE_VIRTIO_SOUND_PCI)
+
+struct VirtIOSoundPCI {
+    VirtIOPCIProxy parent_obj;
+    VirtIOSound vdev;
+};
+
+static Property virtio_sound_properties[] = {
+    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_snd_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+{
+    VirtIOSoundPCI *dev = VIRTIO_SOUND_PCI(vpci_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
+
+    qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
+}
+
+static void virtio_snd_pci_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    VirtioPCIClass *vpciklass = VIRTIO_PCI_CLASS(klass);
+
+    k->class_id = PCI_CLASS_MULTIMEDIA_AUDIO;
+    set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
+    device_class_set_props(dc, virtio_sound_properties);
+    vpciklass->realize = virtio_snd_pci_realize;
+}
+
+static void virtio_snd_pci_instance_init(Object *obj)
+{
+    VirtIOSoundPCI *dev = VIRTIO_SOUND_PCI(obj);
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_SOUND);
+}
+
+static const VirtioPCIDeviceTypeInfo virtio_snd_pci_info = {
+    .base_name              = TYPE_VIRTIO_SOUND_PCI,
+    .generic_name           = "virtio-snd-pci",
+    .instance_size = sizeof(VirtIOSoundPCI),
+    .instance_init = virtio_snd_pci_instance_init,
+    .class_init    = virtio_snd_pci_class_init,
+};
+
+static void virtio_snd_pci_register(void)
+{
+    virtio_pci_types_register(&virtio_snd_pci_info);
+}
+
+type_init(virtio_snd_pci_register);
-- 
2.25.1

[RFC PATCH 07/27] virtio-snd: Add properties for class init

[Shreyansh Chouhan]

Added properties and function stubs for virtio sound device class
init.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/Kconfig      |   5 ++
 hw/audio/meson.build  |   1 +
 hw/audio/virtio-snd.c | 126 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 132 insertions(+)
 create mode 100644 hw/audio/virtio-snd.c

diff --git a/hw/audio/Kconfig b/hw/audio/Kconfig
index e9c6fed826..00b4e1ca88 100644
--- a/hw/audio/Kconfig
+++ b/hw/audio/Kconfig
@@ -50,3 +50,8 @@ config CS4231
 
 config MARVELL_88W8618
     bool
+
+config VIRTIO_SND
+    bool
+    default y if PCI_DEVICES
+    depends on PCI
diff --git a/hw/audio/meson.build b/hw/audio/meson.build
index 32c42bdebe..8c7b8a1e46 100644
--- a/hw/audio/meson.build
+++ b/hw/audio/meson.build
@@ -13,3 +13,4 @@ softmmu_ss.add(when: 'CONFIG_PL041', if_true: files('pl041.c', 'lm4549.c'))
 softmmu_ss.add(when: 'CONFIG_SB16', if_true: files('sb16.c'))
 softmmu_ss.add(when: 'CONFIG_VT82C686', if_true: files('via-ac97.c'))
 softmmu_ss.add(when: 'CONFIG_WM8750', if_true: files('wm8750.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO_SND', if_true: files('virtio-snd.c'))
diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
new file mode 100644
index 0000000000..ae438aa7ec
--- /dev/null
+++ b/hw/audio/virtio-snd.c
@@ -0,0 +1,126 @@
+/*
+ * Virtio Sound device
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/atomic.h"
+#include "qemu/iov.h"
+#include "qemu/main-loop.h"
+#include "qemu/module.h"
+#include "hw/virtio/virtio.h"
+#include "audio/audio.h"
+#include "qemu/error-report.h"
+#include "qemu/timer.h"
+#include "qemu/option.h"
+#include "qemu/option_int.h"
+#include "qemu/config-file.h"
+#include "qapi/qmp/qdict.h"
+#include "hw/virtio/virtio-snd.h"
+#include "hw/virtio/virtio-bus.h"
+#include "qapi/error.h"
+#include "qapi/qapi-events-audio.h"
+#include "hw/qdev-properties.h"
+#include "qapi/qapi-types-migration.h"
+#include "qapi/qapi-events-migration.h"
+#include "migration/misc.h"
+#include "standard-headers/linux/ethtool.h"
+#include "sysemu/sysemu.h"
+#include "trace.h"
+#include "monitor/qdev.h"
+#include "hw/pci/pci.h"
+#include "intel-hda-defs.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_OUT 0
+#define VIRTIO_SOUND_HDA_FN_NID_IN 1
+
+static void virtio_snd_get_config(VirtIODevice *vdev, uint8_t *config)
+{
+}
+
+static void virtio_snd_set_config(VirtIODevice *vdev, const uint8_t *config)
+{
+}
+
+static const VMStateDescription vmstate_virtio_snd_device = {
+    .name = "virtio-snd-device",
+    .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_AUDIO_PROPERTIES(VirtIOSound, card),
+    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 uint64_t virtio_snd_get_features(VirtIODevice *vdev, uint64_t features,
+                                        Error **errp)
+{
+    return vdev->host_features;
+}
+
+static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
+{
+}
+
+static void virtio_snd_device_unrealize(DeviceState *dev)
+{
+}
+
+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);
+
+    device_class_set_props(dc, virtio_snd_properties);
+    dc->vmsd = &vmstate_virtio_snd;
+    set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
+    vdc->realize = virtio_snd_device_realize;
+    vdc->unrealize = virtio_snd_device_unrealize;
+    vdc->get_config = virtio_snd_get_config;
+    vdc->set_config = virtio_snd_set_config;
+    vdc->get_features = virtio_snd_get_features;
+    vdc->reset = virtio_snd_reset;
+    vdc->legacy_features = 0;
+    vdc->vmsd = &vmstate_virtio_snd_device;
+}
+
+
+static const TypeInfo virtio_snd_dev_info = {
+    .name = TYPE_VIRTIO_SOUND,
+    .parent = TYPE_VIRTIO_DEVICE,
+    .instance_size = sizeof(VirtIOSound),
+    .class_init = virtio_snd_class_init,
+};
+
+static void virtio_register_types(void)
+{
+    type_register_static(&virtio_snd_dev_info);
+}
+
+type_init(virtio_register_types)
-- 
2.25.1

Re: [RFC PATCH 07/27] virtio-snd: Add properties for class init

[Laurent Vivier]

Le 29/04/2021 à 14:04, Shreyansh Chouhan a écrit :
> Added properties and function stubs for virtio sound device class
> init.
>
> Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
> ---
>  hw/audio/Kconfig      |   5 ++
>  hw/audio/meson.build  |   1 +
>  hw/audio/virtio-snd.c | 126 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 132 insertions(+)
>  create mode 100644 hw/audio/virtio-snd.c
>
> diff --git a/hw/audio/Kconfig b/hw/audio/Kconfig
> index e9c6fed826..00b4e1ca88 100644
> --- a/hw/audio/Kconfig
> +++ b/hw/audio/Kconfig
> @@ -50,3 +50,8 @@ config CS4231
>  
>  config MARVELL_88W8618
>      bool
> +
> +config VIRTIO_SND
> +    bool
> +    default y if PCI_DEVICES
> +    depends on PCI

There is nothing specific to PCI in that code, why do you prevent the use of virtio-snd as a MMIO
device?

Thanks,

Laurent

Re: [RFC PATCH 07/27] virtio-snd: Add properties for class init

[Shreyansh Chouhan]

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

On Tue, 4 May 2021 at 19:02, Laurent Vivier <laurent@vivier.eu> wrote:

> There is nothing specific to PCI in that code, why do you prevent the use
> of virtio-snd as a MMIO
> device?
>
> I am sorry I do not understand your question completely. If by preventing
the use of virtio-snd, you mean
why did I add the PCI dependencies to the Kconfig file, then I think I must
have been a bit confused
while writing it. VIRTIO_PCI already includes those dependencies, I will
change the dependency to
VIRTIO. (Which is what it is for other virtio devices too.)

However if you mean why did I not add an MMIO binding for this device, then
there is no
specific reason. I simply followed what QEMU had been doing for the other
virtio devices.
Will there be any advantages to implementing the device as a MMIO device?

> Thanks,
>
> Laurent
>
> --
Regards
Shreyansh

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

Re: [RFC PATCH 07/27] virtio-snd: Add properties for class init

[Laurent Vivier]

Hi Shreyansh,

First of all, thank you for your work, I was expecting a virtio sound device for some time...

Le 04/05/2021 à 21:35, Shreyansh Chouhan a écrit :
> On Tue, 4 May 2021 at 19:02, Laurent Vivier <laurent@vivier.eu <mailto:laurent@vivier.eu>> wrote:
> 
>     There is nothing specific to PCI in that code, why do you prevent the use of virtio-snd as a MMIO
>     device?
> 
> I am sorry I do not understand your question completely. If by preventing the use of virtio-snd, you
> mean
> why did I add the PCI dependencies to the Kconfig file, then I think I must have been a bit confused
> while writing it. VIRTIO_PCI already includes those dependencies, I will change the dependency to
> VIRTIO. (Which is what it is for other virtio devices too.)
> 
> However if you mean why did I not add an MMIO binding for this device, then there is no
> specific reason. I simply followed what QEMU had been doing for the other virtio devices.
> Will there be any advantages to implementing the device as a MMIO device? 

No, the question was only about the dependencies, generally a a virtio device is binded to a virtio
bus, and virtio PCI is a PCI card providing a virtio bus with the virtio device attached to it.

For instance, for virtio-net-pci:

HOST

  -> PCI Host controller

    -> PCI virtio net device (TYPE_VIRTIO_NET_PCI)

      -> virtio Bus (TYPE_VIRTIO_BUS)

        -> virtio net device (TYPE_VIRTIO_NET)

TYPE_VIRTIO_NET_PCI is created by hw/virtio/virtio-net-pci.c and TYPE_VIRTIO_NET by hw/net/vrtio-net.c:

hw/virtio/meson.build:

virtio_pci_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('virtio-net-pci.c'))
virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)

hw/net/meson.build
specific_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('virtio-net.c'))

hw/net/Kconfig:

config VIRTIO_NET
    bool
    default y
    depends on VIRTIO

So:

the virtio-net device is built when VIRTIO_NET is set,
the virtio-net-pci device is build when VIRTIO_NET and VIRTIO_PCI are set.

So what I expect for virtio-snd:

hw/virtio/meson.build:

virtio_pci_ss.add(when: 'CONFIG_VIRTIO_SND', if_true: files('virtio-snd-pci.c'))

hw/audio/meson.build:

softmmu_ss.add(when: 'CONFIG_VIRTIO_SND', if_true: files('virtio-snd.c'))

hw/audio/Kconfig

config VIRTIO_SND
    bool
    default y
    depends on VIRTIO

With that kind of config, a machine without PCI bus will be able to create a virtio bus to add your
virtio device (like s390x with virtio-ccw of any other MMIO machine like the virt machines).

In short: update your hw/audio/config, and all will be fine.

Thanks,
Laurent

Re: [RFC PATCH 07/27] virtio-snd: Add properties for class init

[Shreyansh Chouhan]

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

On Wed, 5 May 2021 at 02:00, Laurent Vivier <laurent@vivier.eu> wrote:

> Hi Shreyansh,
>
> First of all, thank you for your work, I was expecting a virtio sound
> device for some time...
>
> You're welcome :)

> Le 04/05/2021 à 21:35, Shreyansh Chouhan a écrit :
> > On Tue, 4 May 2021 at 19:02, Laurent Vivier <laurent@vivier.eu <mailto:
> laurent@vivier.eu>> wrote:
> >
> >     There is nothing specific to PCI in that code, why do you prevent
> the use of virtio-snd as a MMIO
> >     device?
> >
> > I am sorry I do not understand your question completely. If by
> preventing the use of virtio-snd, you
> > mean
> > why did I add the PCI dependencies to the Kconfig file, then I think I
> must have been a bit confused
> > while writing it. VIRTIO_PCI already includes those dependencies, I will
> change the dependency to
> > VIRTIO. (Which is what it is for other virtio devices too.)
> >
> > However if you mean why did I not add an MMIO binding for this device,
> then there is no
> > specific reason. I simply followed what QEMU had been doing for the
> other virtio devices.
> > Will there be any advantages to implementing the device as a MMIO
> device?
>
> No, the question was only about the dependencies, generally a a virtio
> device is binded to a virtio
> bus, and virtio PCI is a PCI card providing a virtio bus with the virtio
> device attached to it.
>
> For instance, for virtio-net-pci:
>
> HOST
>
>   -> PCI Host controller
>
>     -> PCI virtio net device (TYPE_VIRTIO_NET_PCI)
>
>       -> virtio Bus (TYPE_VIRTIO_BUS)
>
>         -> virtio net device (TYPE_VIRTIO_NET)
>
> TYPE_VIRTIO_NET_PCI is created by hw/virtio/virtio-net-pci.c and
> TYPE_VIRTIO_NET by hw/net/vrtio-net.c:
>
> hw/virtio/meson.build:
>
> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_NET', if_true:
> files('virtio-net-pci.c'))
> virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
>
> hw/net/meson.build
> specific_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('virtio-net.c'))
>
> hw/net/Kconfig:
>
> config VIRTIO_NET
>     bool
>     default y
>     depends on VIRTIO
>
> So:
>
> the virtio-net device is built when VIRTIO_NET is set,
> the virtio-net-pci device is build when VIRTIO_NET and VIRTIO_PCI are set.
>
> So what I expect for virtio-snd:
>
> hw/virtio/meson.build:
>
> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_SND', if_true:
> files('virtio-snd-pci.c'))
>
> hw/audio/meson.build:
>
> softmmu_ss.add(when: 'CONFIG_VIRTIO_SND', if_true: files('virtio-snd.c'))
>
> hw/audio/Kconfig
>
> config VIRTIO_SND
>     bool
>     default y
>     depends on VIRTIO
>
> With that kind of config, a machine without PCI bus will be able to create
> a virtio bus to add your
> virtio device (like s390x with virtio-ccw of any other MMIO machine like
> the virt machines).
>
> Thanks a lot for the detailed explanation. It clarifies everything
regarding the separate
pci and device source files and how they are built, which was confusing me
a little bit.
I will fix the meson.build and Kconfig files in the upcoming versions of
these patches.

In short: update your hw/audio/config, and all will be fine.


>
Thanks,
> Laurent
>
--
Thanks
Shreyansh

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

[RFC PATCH 08/27] virtio-snd: Add code for get config function

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index ae438aa7ec..afa38adee7 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -41,6 +41,15 @@
 
 static void virtio_snd_get_config(VirtIODevice *vdev, uint8_t *config)
 {
+    VirtIOSound *s = VIRTIO_SOUND(vdev);
+    virtio_snd_config sndcfg;
+
+    memset(&sndcfg, 0, sizeof(virtio_snd_config));
+    stl_le_p(&(sndcfg.jacks), s->snd_conf.jacks);
+    stl_le_p(&(sndcfg.streams), s->snd_conf.streams);
+    stl_le_p(&(sndcfg.chmaps), s->snd_conf.chmaps);
+
+    memcpy(config, &sndcfg, sizeof(virtio_snd_config));
 }
 
 static void virtio_snd_set_config(VirtIODevice *vdev, const uint8_t *config)
-- 
2.25.1

[RFC PATCH 09/27] virtio-snd: Add code for set config function

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index afa38adee7..edaeffd6b7 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -54,6 +54,14 @@ static void virtio_snd_get_config(VirtIODevice *vdev, uint8_t *config)
 
 static void virtio_snd_set_config(VirtIODevice *vdev, const uint8_t *config)
 {
+    VirtIOSound *s = VIRTIO_SOUND(vdev);
+    virtio_snd_config sndcfg;
+
+    memcpy(&sndcfg, config, sizeof(virtio_snd_config));
+
+    memcpy(&s->snd_conf.jacks, &sndcfg.jacks, sizeof(uint32_t));
+    memcpy(&s->snd_conf.streams, &sndcfg.streams, sizeof(uint32_t));
+    memcpy(&s->snd_conf.chmaps, &sndcfg.streams, sizeof(uint32_t));
 }
 
 static const VMStateDescription vmstate_virtio_snd_device = {
-- 
2.25.1

Re: [RFC PATCH 09/27] virtio-snd: Add code for set config function

[Gerd Hoffmann]

On Thu, Apr 29, 2021 at 05:34:27PM +0530, Shreyansh Chouhan wrote:
> Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
> ---
>  hw/audio/virtio-snd.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index afa38adee7..edaeffd6b7 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -54,6 +54,14 @@ static void virtio_snd_get_config(VirtIODevice *vdev, uint8_t *config)
>  
>  static void virtio_snd_set_config(VirtIODevice *vdev, const uint8_t *config)
>  {
> +    VirtIOSound *s = VIRTIO_SOUND(vdev);
> +    virtio_snd_config sndcfg;
> +
> +    memcpy(&sndcfg, config, sizeof(virtio_snd_config));
> +
> +    memcpy(&s->snd_conf.jacks, &sndcfg.jacks, sizeof(uint32_t));
> +    memcpy(&s->snd_conf.streams, &sndcfg.streams, sizeof(uint32_t));
> +    memcpy(&s->snd_conf.chmaps, &sndcfg.streams, sizeof(uint32_t));

This is static device information for the driver, the driver should not
be able to change those.  I think you can simply leave the set_config
empty (i.e. just drop this patch).

take care,
  Gerd

[RFC PATCH 10/27] virtio-snd: Add code for the realize function

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index edaeffd6b7..caad157705 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -97,8 +97,43 @@ static uint64_t virtio_snd_get_features(VirtIODevice *vdev, uint64_t features,
     return vdev->host_features;
 }
 
+/*
+ * Initializes the VirtIOSound card device. Validates the configuration
+ * passed by the command line. Initializes the virtqueues. Allocates resources
+ * for and initializes streams, jacks and chmaps.
+ *
+ * @dev: VirtIOSound card device
+ * @errp: Set if there is an error
+ */
 static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
 {
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOSound *s = VIRTIO_SOUND(dev);
+
+    virtio_init(vdev, "virtio-snd", VIRTIO_ID_SOUND, sizeof(virtio_snd_config));
+
+    /* set number of jacks and streams */
+    if (s->snd_conf.jacks > 8) {
+        error_setg(errp, "Invalid number of jacks: %d", s->snd_conf.jacks);
+        return;
+    }
+    if (s->snd_conf.streams < 1 || s->snd_conf.streams > 10) {
+        error_setg(errp, "Invalid number of streams: %d", s->snd_conf.streams);
+        return;
+    }
+
+    if (s->snd_conf.chmaps > VIRTIO_SND_CHMAP_MAX_SIZE) {
+        error_setg(errp, "Invalid number of channel maps: %d",
+                   s->snd_conf.chmaps);
+        return;
+    }
+
+    /* set up QEMUSoundCard and audiodev */
+    AUD_register_card ("virtio_snd_card", &s->card);
+
+    s->streams = g_new0(virtio_snd_pcm_stream *, s->snd_conf.streams);
+    s->pcm_params = g_new0(virtio_snd_pcm_params *, s->snd_conf.streams);
+    s->jacks = g_new0(virtio_snd_jack *, s->snd_conf.jacks);
 }
 
 static void virtio_snd_device_unrealize(DeviceState *dev)
-- 
2.25.1

Re: [RFC PATCH 10/27] virtio-snd: Add code for the realize function

[Shreyansh Chouhan]

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

Hi,

On Thu, 22 Jul 2021 at 07:48, Deepa gowda <deepa.gouda@gmail.com> wrote:

> Hi, Shreyansh,
>
> When is virtio-snd expected to be completed and available in Qemu GitHub?
>
>
Sorry for the recent absence of activity on this patch series. I have the
sound card
working with alsa. The output works just fine. The input needs a little bit
of polishing to do.
To answer your question, it is still going to take some time because I
recently got selected
for an internship/mentorship program and I cannot give as much time to the
patch as I
would like to. It could still take me over a month to complete this.

Hope you understand.

(I've cc'd the mailing list and Gerd so that they too can know about this.)

Thanks,
Shreyansh Chouhan

> Regards Deepa
>
> On Thu, 29 Apr 2021, 17:58 Shreyansh Chouhan, <
> chouhan.shreyansh2702@gmail.com> wrote:
>
>> Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
>> ---
>>  hw/audio/virtio-snd.c | 35 +++++++++++++++++++++++++++++++++++
>>  1 file changed, 35 insertions(+)
>>
>> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
>> index edaeffd6b7..caad157705 100644
>> --- a/hw/audio/virtio-snd.c
>> +++ b/hw/audio/virtio-snd.c
>> @@ -97,8 +97,43 @@ static uint64_t virtio_snd_get_features(VirtIODevice
>> *vdev, uint64_t features,
>>      return vdev->host_features;
>>  }
>>
>> +/*
>> + * Initializes the VirtIOSound card device. Validates the configuration
>> + * passed by the command line. Initializes the virtqueues. Allocates
>> resources
>> + * for and initializes streams, jacks and chmaps.
>> + *
>> + * @dev: VirtIOSound card device
>> + * @errp: Set if there is an error
>> + */
>>  static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
>>  {
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>> +    VirtIOSound *s = VIRTIO_SOUND(dev);
>> +
>> +    virtio_init(vdev, "virtio-snd", VIRTIO_ID_SOUND,
>> sizeof(virtio_snd_config));
>> +
>> +    /* set number of jacks and streams */
>> +    if (s->snd_conf.jacks > 8) {
>> +        error_setg(errp, "Invalid number of jacks: %d",
>> s->snd_conf.jacks);
>> +        return;
>> +    }
>> +    if (s->snd_conf.streams < 1 || s->snd_conf.streams > 10) {
>> +        error_setg(errp, "Invalid number of streams: %d",
>> s->snd_conf.streams);
>> +        return;
>> +    }
>> +
>> +    if (s->snd_conf.chmaps > VIRTIO_SND_CHMAP_MAX_SIZE) {
>> +        error_setg(errp, "Invalid number of channel maps: %d",
>> +                   s->snd_conf.chmaps);
>> +        return;
>> +    }
>> +
>> +    /* set up QEMUSoundCard and audiodev */
>> +    AUD_register_card ("virtio_snd_card", &s->card);
>> +
>> +    s->streams = g_new0(virtio_snd_pcm_stream *, s->snd_conf.streams);
>> +    s->pcm_params = g_new0(virtio_snd_pcm_params *, s->snd_conf.streams);
>> +    s->jacks = g_new0(virtio_snd_jack *, s->snd_conf.jacks);
>>  }
>>
>>  static void virtio_snd_device_unrealize(DeviceState *dev)
>> --
>> 2.25.1
>>
>>
>>

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

[RFC PATCH 11/27] virtio-snd: Add macros for logging

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index caad157705..a93674ea72 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -39,6 +39,10 @@
 #define VIRTIO_SOUND_HDA_FN_NID_OUT 0
 #define VIRTIO_SOUND_HDA_FN_NID_IN 1
 
+#define virtio_snd_log(...) AUD_log("virtio sound info", __VA_ARGS__)
+#define virtio_snd_warn(...) AUD_log("virtio sound warn", __VA_ARGS__)
+#define virtio_snd_err(...) AUD_log("virtio sound err", __VA_ARGS__)
+
 static void virtio_snd_get_config(VirtIODevice *vdev, uint8_t *config)
 {
     VirtIOSound *s = VIRTIO_SOUND(vdev);
@@ -175,4 +179,8 @@ static void virtio_register_types(void)
     type_register_static(&virtio_snd_dev_info);
 }
 
+#undef virtio_snd_log
+#undef virtio_snd_warn
+#undef virtio_snd_err
+
 type_init(virtio_register_types)
-- 
2.25.1

Re: [RFC PATCH 11/27] virtio-snd: Add macros for logging

[Gerd Hoffmann]

On Thu, Apr 29, 2021 at 05:34:29PM +0530, Shreyansh Chouhan wrote:
> Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
> ---
>  hw/audio/virtio-snd.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index caad157705..a93674ea72 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -39,6 +39,10 @@
>  #define VIRTIO_SOUND_HDA_FN_NID_OUT 0
>  #define VIRTIO_SOUND_HDA_FN_NID_IN 1
>  
> +#define virtio_snd_log(...) AUD_log("virtio sound info", __VA_ARGS__)
> +#define virtio_snd_warn(...) AUD_log("virtio sound warn", __VA_ARGS__)
> +#define virtio_snd_err(...) AUD_log("virtio sound err", __VA_ARGS__)

Warnings and errors meant for the user should use error_report() and
warn_report().

For most debug messages it is better to use tracepoints instead.

take care,
  Gerd

[RFC PATCH 12/27] virtio-snd: Add control virtqueue handler

[Shreyansh Chouhan]

The handler prints the requests that came in the ctrl virtqueue.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 71 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index a93674ea72..435870e3ba 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -101,6 +101,75 @@ static uint64_t virtio_snd_get_features(VirtIODevice *vdev, uint64_t features,
     return vdev->host_features;
 }
 
+/* The control queue handler. Pops an element from the control virtqueue,
+ * checks the header and performs the requested action. Finally marks the
+ * element as used.
+ *
+ * @vdev: VirtIOSound card device
+ * @vq: Control virtqueue
+ */
+static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+{
+    virtio_snd_hdr ctrl;
+
+    VirtQueueElement *elem = NULL;
+    size_t sz;
+    struct iovec *iov, *iov2;
+    unsigned int iov_cnt;
+
+    for (;;) {
+        elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+        if (!elem) {
+            break;
+        }
+        if (iov_size(elem->in_sg, elem->in_num) < sizeof(ctrl) ||
+                iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
+            virtio_snd_err("virtio-snd ctrl missing headers\n");
+            virtqueue_detach_element(vq, elem, 0);
+            g_free(elem);
+            break;
+        }
+
+        iov_cnt = elem->out_num;
+        iov2 = iov = g_memdup(elem->out_sg,
+                              sizeof(struct iovec) * elem->out_num);
+        sz = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl));
+        iov_discard_front(&iov, &iov_cnt, sizeof(ctrl));
+        if (sz != sizeof(ctrl)) {
+            /* error */
+            virtio_snd_err("virtio snd ctrl could not read header\n");
+        } else if (ctrl.code == VIRTIO_SND_R_JACK_INFO) {
+            virtio_snd_log("VIRTIO_SND_R_JACK_INFO");
+        } else if (ctrl.code == VIRTIO_SND_R_JACK_REMAP) {
+            virtio_snd_log("VIRTIO_SND_R_JACK_REMAP");
+        } else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
+            virtio_snd_log("VIRTIO_SND_R_PCM_INFO");
+        } else if (ctrl.code == VIRTIO_SND_R_PCM_SET_PARAMS) {
+            virtio_snd_log("VIRTIO_SND_R_PCM_SET_PARAMS");
+        } else if (ctrl.code == VIRTIO_SND_R_PCM_PREPARE) {
+            virtio_snd_log("VIRTIO_SND_R_PCM_PREPARE");
+        } else if (ctrl.code == VIRTIO_SND_R_PCM_START) {
+            virtio_snd_log("VIRTIO_SND_R_PCM_START");
+        } else if (ctrl.code == VIRTIO_SND_R_PCM_STOP) {
+            virtio_snd_log("VIRTIO_SND_R_PCM_STOP");
+        } else if (ctrl.code == VIRTIO_SND_R_PCM_RELEASE) {
+            virtio_snd_log("VIRTIO_SND_R_PCM_RELEASE");
+        } else {
+            /* error */
+            virtio_snd_err("virtio snd header not recognized: %d\n", ctrl.code);
+        }
+
+        virtio_snd_hdr resp;
+        resp.code = VIRTIO_SND_S_OK;
+        sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+        virtqueue_push(vq, elem, sz);
+
+        virtio_notify(vdev, vq);
+        g_free(iov2);
+        g_free(elem);
+    }
+}
+
 /*
  * Initializes the VirtIOSound card device. Validates the configuration
  * passed by the command line. Initializes the virtqueues. Allocates resources
@@ -135,6 +204,8 @@ static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
     /* set up QEMUSoundCard and audiodev */
     AUD_register_card ("virtio_snd_card", &s->card);
 
+    s->ctrl_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_ctrl);
+
     s->streams = g_new0(virtio_snd_pcm_stream *, s->snd_conf.streams);
     s->pcm_params = g_new0(virtio_snd_pcm_params *, s->snd_conf.streams);
     s->jacks = g_new0(virtio_snd_jack *, s->snd_conf.jacks);
-- 
2.25.1

Re: [RFC PATCH 12/27] virtio-snd: Add control virtqueue handler

[Gerd Hoffmann]

  Hi,

> +        } else if (ctrl.code == VIRTIO_SND_R_JACK_INFO) {
> +            virtio_snd_log("VIRTIO_SND_R_JACK_INFO");

For that kind of tracing / debug logging it is useful to have
enum -> string mapping helper function(s).

take care,
  Gerd

[RFC PATCH 13/27] virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 81 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 79 insertions(+), 2 deletions(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 435870e3ba..d50234f9a8 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -100,6 +100,80 @@ static uint64_t virtio_snd_get_features(VirtIODevice *vdev, uint64_t features,
 {
     return vdev->host_features;
 }
+/*
+ * Get a specific jack from the VirtIOSound card.
+ *
+ * @s: VirtIOSound card device.
+ * @id: Jack id
+ */
+static virtio_snd_jack *virtio_snd_get_jack(VirtIOSound *s, uint32_t id)
+{
+    if (id >= s->snd_conf.jacks) {
+        return NULL;
+    }
+    return s->jacks[id];
+}
+
+/*
+ * Handles VIRTIO_SND_R_JACK_INFO.
+ * The function writes the info structs and response to the virtqueue element.
+ * Returns the used size in bytes.
+ *
+ * @s: VirtIOSound card
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_jack_info(VirtIOSound *s,
+                                            VirtQueueElement *elem)
+{
+    virtio_snd_query_info req;
+    size_t sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+    assert(sz == sizeof(virtio_snd_query_info));
+
+    virtio_snd_hdr resp;
+
+    if (iov_size(elem->in_sg, elem->in_num) <
+        sizeof(virtio_snd_hdr) + req.count * req.size) {
+        virtio_snd_err("jack info: buffer too small got: %lu needed: %lu\n",
+                       iov_size(elem->in_sg, elem->in_num),
+                       sizeof(virtio_snd_hdr) + req.count * req.size);
+        resp.code = VIRTIO_SND_S_BAD_MSG;
+        goto done;
+    }
+
+    virtio_snd_jack_info *jack_info = g_new0(virtio_snd_jack_info, req.count);
+    for (int i = req.start_id; i < req.count + req.start_id; i++) {
+        virtio_snd_jack *jack = virtio_snd_get_jack(s, i);
+        if (!jack) {
+            virtio_snd_err("Invalid jack id: %d\n", i);
+            resp.code = VIRTIO_SND_S_BAD_MSG;
+            goto done;
+        }
+
+        jack_info[i - req.start_id].hdr.hda_fn_nid = jack->hda_fn_nid;
+        jack_info[i - req.start_id].features = jack->features;
+        jack_info[i - req.start_id].hda_reg_defconf = jack->hda_reg_defconf;
+        jack_info[i - req.start_id].hda_reg_caps = jack->hda_reg_caps;
+        jack_info[i - req.start_id].connected = jack->connected;
+        memset(jack_info[i - req.start_id].padding, 0,
+               sizeof(jack_info[i - req.start_id].padding));
+    }
+
+    resp.code = VIRTIO_SND_S_OK;
+done:
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+
+    if (resp.code == VIRTIO_SND_S_BAD_MSG) {
+        g_free(jack_info);
+        return sz;
+    }
+
+    sz = iov_from_buf(elem->in_sg, elem->in_num, sizeof(virtio_snd_hdr),
+                      jack_info, sizeof(virtio_snd_jack_info) * req.count);
+    assert(sz == req.count * req.size);
+    g_free(jack_info);
+    return sizeof(virtio_snd_hdr) + sz;
+}
 
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
@@ -110,6 +184,7 @@ static uint64_t virtio_snd_get_features(VirtIODevice *vdev, uint64_t features,
  */
 static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
+    VirtIOSound *s = VIRTIO_SOUND(vdev);
     virtio_snd_hdr ctrl;
 
     VirtQueueElement *elem = NULL;
@@ -139,7 +214,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             /* error */
             virtio_snd_err("virtio snd ctrl could not read header\n");
         } else if (ctrl.code == VIRTIO_SND_R_JACK_INFO) {
-            virtio_snd_log("VIRTIO_SND_R_JACK_INFO");
+            sz = virtio_snd_handle_jack_info(s, elem);
+            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_JACK_REMAP) {
             virtio_snd_log("VIRTIO_SND_R_JACK_REMAP");
         } else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
@@ -162,8 +238,9 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         virtio_snd_hdr resp;
         resp.code = VIRTIO_SND_S_OK;
         sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
-        virtqueue_push(vq, elem, sz);
 
+done:
+        virtqueue_push(vq, elem, sz);
         virtio_notify(vdev, vq);
         g_free(iov2);
         g_free(elem);
-- 
2.25.1

Re: [RFC PATCH 13/27] virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler

[Gerd Hoffmann]

  Hi,

> +    virtio_snd_query_info req;
> +    size_t sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
> +    assert(sz == sizeof(virtio_snd_query_info));

This assert looks like the guest can trigger it by sending broken
messages.  This should be avoided, the guest should not be able to
kill qemu that way.

> +        jack_info[i - req.start_id].hdr.hda_fn_nid = jack->hda_fn_nid;
> +        jack_info[i - req.start_id].features = jack->features;
> +        jack_info[i - req.start_id].hda_reg_defconf = jack->hda_reg_defconf;
> +        jack_info[i - req.start_id].hda_reg_caps = jack->hda_reg_caps;
> +        jack_info[i - req.start_id].connected = jack->connected;

Disclaimer: didn't check the structs.

If any of these fields is larger than a byte you need to take care of
byte ordering here.  virtio is little endian, so cpu_to_le{16,32}() will
do the job here (if needed).

Same thing elsewhere I suspect.

>          } else if (ctrl.code == VIRTIO_SND_R_JACK_INFO) {
> -            virtio_snd_log("VIRTIO_SND_R_JACK_INFO");
> +            sz = virtio_snd_handle_jack_info(s, elem);
> +            goto done;

Ah, you add the actual command handing here.  Hmm.  I guess a tracepoint
in virtio_snd_handle_jack_info() would be good for debugging.  You could
also log the jack id then.

Also: I'd suggest using "switch(ctrl.code)" here.  Is more readable than
else-if chains (personal opinion though).  Also has the advantage that
gcc will warn in case you forget to handle one of the enums in the
switch.

take care,
  Gerd

[RFC PATCH 14/27] virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index d50234f9a8..527eed6132 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -175,6 +175,27 @@ done:
     return sizeof(virtio_snd_hdr) + sz;
 }
 
+/*
+ * Handles VIRTIO_SND_R_JACK_REMAP.
+ * Not implemented yet.
+ *
+ * @s: VirtIOSound card
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_jack_remap(VirtIOSound *s,
+                                             VirtQueueElement *elem)
+{
+    virtio_snd_hdr resp;
+    resp.code = VIRTIO_SND_S_OK;
+
+    /* TODO: implement remap */
+
+    size_t sz;
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+    return sz;
+}
+
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
  * element as used.
@@ -217,7 +238,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             sz = virtio_snd_handle_jack_info(s, elem);
             goto done;
         } else if (ctrl.code == VIRTIO_SND_R_JACK_REMAP) {
-            virtio_snd_log("VIRTIO_SND_R_JACK_REMAP");
+            sz = virtio_snd_handle_jack_remap(s, elem);
+            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
             virtio_snd_log("VIRTIO_SND_R_PCM_INFO");
         } else if (ctrl.code == VIRTIO_SND_R_PCM_SET_PARAMS) {
-- 
2.25.1

[RFC PATCH 15/27] virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 88 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 527eed6132..bc99cd58d6 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -196,6 +196,91 @@ static uint32_t virtio_snd_handle_jack_remap(VirtIOSound *s,
     return sz;
 }
 
+/*
+ * Get a specific stream from the virtio sound card device.
+ *
+ * @s: VirtIOSound card device
+ * @stream: Stream id
+ *
+ * Returns NULL if function fails.
+ * TODO: Make failure more explicit. Output can be NULL if the stream number
+ *       is valid but the stream hasn't been allocated yet.
+ */
+static virtio_snd_pcm_stream *virtio_snd_pcm_get_stream(VirtIOSound *s,
+                                                        uint32_t stream)
+{
+    if (stream >= s->snd_conf.streams) {
+        virtio_snd_err("Invalid stream request %d\n", stream);
+        return NULL;
+    }
+    return s->streams[stream];
+}
+
+/*
+ * Handle the VIRTIO_SND_R_PCM_INFO request.
+ * The function writes the info structs to the request element.
+ * Returns the used size in bytes.
+ *
+ * @s: VirtIOSound card device
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_pcm_info(VirtIOSound *s,
+                                           VirtQueueElement *elem)
+{
+    virtio_snd_query_info req;
+    uint32_t sz;
+    sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+    assert(sz == sizeof(virtio_snd_query_info));
+
+    virtio_snd_hdr resp;
+    if (iov_size(elem->in_sg, elem->in_num) <
+        sizeof(virtio_snd_hdr) + req.size * req.count) {
+        virtio_snd_err("pcm info: buffer too small, got: %lu, needed: %lu\n",
+                iov_size(elem->in_sg, elem->in_num),
+                sizeof(virtio_snd_pcm_info));
+        resp.code = VIRTIO_SND_S_BAD_MSG;
+        goto done;
+    }
+
+    virtio_snd_pcm_stream *stream;
+    virtio_snd_pcm_info *pcm_info = g_new0(virtio_snd_pcm_info, req.count);
+    for (int i = req.start_id; i < req.start_id + req.count; i++) {
+        stream = virtio_snd_pcm_get_stream(s, i);
+
+        if (!stream) {
+            virtio_snd_err("Invalid stream id: %d\n", i);
+            resp.code = VIRTIO_SND_S_BAD_MSG;
+            goto done;
+        }
+
+        pcm_info[i - req.start_id].hdr.hda_fn_nid = stream->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;
+
+        memset(&pcm_info[i].padding, 0, sizeof(pcm_info[i].padding));
+    }
+
+    resp.code = VIRTIO_SND_S_OK;
+done:
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+
+    if (resp.code == VIRTIO_SND_S_BAD_MSG) {
+        g_free(pcm_info);
+        return sz;
+    }
+
+    sz = iov_from_buf(elem->in_sg, elem->in_num, sizeof(virtio_snd_hdr),
+                      pcm_info, sizeof(virtio_snd_pcm_info) * req.count);
+    assert(sz == req.size * req.count);
+    g_free(pcm_info);
+    return sizeof(virtio_snd_hdr) + sz;
+}
+
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
  * element as used.
@@ -241,7 +326,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             sz = virtio_snd_handle_jack_remap(s, elem);
             goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
-            virtio_snd_log("VIRTIO_SND_R_PCM_INFO");
+            sz = virtio_snd_handle_pcm_info(s, elem);
+            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_SET_PARAMS) {
             virtio_snd_log("VIRTIO_SND_R_PCM_SET_PARAMS");
         } else if (ctrl.code == VIRTIO_SND_R_PCM_PREPARE) {
-- 
2.25.1

[RFC PATCH 16/27] virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle

[Shreyansh Chouhan]

Added handler for the VIRTIO_SND_R_PCM_SET_PARAMS control request.
The handler was split up into two functions in so that the code
could be reused in the realize function.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 100 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 99 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index bc99cd58d6..4e9764de08 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -281,6 +281,103 @@ done:
     return sizeof(virtio_snd_hdr) + sz;
 }
 
+/*
+ * Set the given stream params.
+ * Called by both virtio_snd_handle_pcm_set_params and during device
+ * initialization.
+ * Returns a virtio sound status VIRTIO_SND_S_*
+ *
+ * @s: VirtIOSound card device
+ * @params: The PCM params as defined in the virtio specification
+ */
+static uint32_t virtio_snd_pcm_set_params_impl(VirtIOSound *s,
+                                               virtio_snd_pcm_set_params *params)
+{
+    uint32_t st = params->hdr.stream_id;
+    if (st > s->snd_conf.streams || !(s->pcm_params)) {
+        virtio_error(VIRTIO_DEVICE(s), "Streams not initalized\n");
+        return VIRTIO_SND_S_BAD_MSG;
+    }
+
+    if (!s->pcm_params[st]) {
+        s->pcm_params[st] = g_new0(virtio_snd_pcm_params, 1);
+    }
+    virtio_snd_pcm_params *st_params = s->pcm_params[st];
+
+    st_params->features = params->features;
+    st_params->buffer_bytes = params->buffer_bytes;
+    st_params->period_bytes = params->period_bytes;
+
+    if (params->channel < 1 || params->channel > AUDIO_MAX_CHANNELS) {
+        virtio_snd_err("Number of channels not supported\n");
+        return VIRTIO_SND_S_NOT_SUPP;
+    }
+    st_params->channel = params->channel;
+
+    uint32_t supported_formats = 1 << VIRTIO_SND_PCM_FMT_S8 |
+                                 1 << VIRTIO_SND_PCM_FMT_U8 |
+                                 1 << VIRTIO_SND_PCM_FMT_S16 |
+                                 1 << VIRTIO_SND_PCM_FMT_U16 |
+                                 1 << VIRTIO_SND_PCM_FMT_S32 |
+                                 1 << VIRTIO_SND_PCM_FMT_U32 |
+                                 1 << VIRTIO_SND_PCM_FMT_FLOAT;
+
+    uint32_t supported_rates = 1 << VIRTIO_SND_PCM_RATE_5512 |
+                               1 << VIRTIO_SND_PCM_RATE_8000 |
+                               1 << VIRTIO_SND_PCM_RATE_11025 |
+                               1 << VIRTIO_SND_PCM_RATE_16000 |
+                               1 << VIRTIO_SND_PCM_RATE_22050 |
+                               1 << VIRTIO_SND_PCM_RATE_32000 |
+                               1 << VIRTIO_SND_PCM_RATE_44100 |
+                               1 << VIRTIO_SND_PCM_RATE_48000 |
+                               1 << VIRTIO_SND_PCM_RATE_64000 |
+                               1 << VIRTIO_SND_PCM_RATE_88200 |
+                               1 << VIRTIO_SND_PCM_RATE_96000 |
+                               1 << VIRTIO_SND_PCM_RATE_176399 |
+                               1 << VIRTIO_SND_PCM_RATE_192000 |
+                               1 << VIRTIO_SND_PCM_RATE_384000;
+
+    if (!(supported_formats & (1 << params->format))) {
+        virtio_snd_err("Stream format not supported\n");
+        return VIRTIO_SND_S_NOT_SUPP;
+    }
+    st_params->format = params->format;
+
+    if (!(supported_rates & (1 << params->rate))) {
+        virtio_snd_err("Stream rate not supported\n");
+        return VIRTIO_SND_S_NOT_SUPP;
+    }
+    st_params->rate = params->rate;
+
+    st_params->period_bytes = params->period_bytes;
+    st_params->buffer_bytes = params->buffer_bytes;
+    return VIRTIO_SND_S_OK;
+}
+
+/*
+ * Handles the VIRTIO_SND_R_PCM_SET_PARAMS request.
+ * The function writes the response to the virtqueue element.
+ * Returns the used size in bytes.
+ *
+ * @s: VirtIOSound card device
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_pcm_set_params(VirtIOSound *s,
+                                                 VirtQueueElement *elem)
+{
+    virtio_snd_pcm_set_params req;
+    uint32_t sz;
+    sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+    assert(sz == sizeof(virtio_snd_pcm_set_params));
+
+    virtio_snd_hdr resp;
+    resp.code = virtio_snd_pcm_set_params_impl(s, &req);
+
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+    return sz;
+}
+
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
  * element as used.
@@ -329,7 +426,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             sz = virtio_snd_handle_pcm_info(s, elem);
             goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_SET_PARAMS) {
-            virtio_snd_log("VIRTIO_SND_R_PCM_SET_PARAMS");
+            sz = virtio_snd_handle_pcm_set_params(s, elem);
+            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_PREPARE) {
             virtio_snd_log("VIRTIO_SND_R_PCM_PREPARE");
         } else if (ctrl.code == VIRTIO_SND_R_PCM_START) {
-- 
2.25.1

[RFC PATCH 17/27] virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler

[Shreyansh Chouhan]

The handler doesn't intialize the SWVoiceOut streams for now.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 198 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 197 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 4e9764de08..dba90cc4a7 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -378,6 +378,201 @@ static uint32_t virtio_snd_handle_pcm_set_params(VirtIOSound *s,
     return sz;
 }
 
+/*
+ * Get a QEMU Audiosystem compatible format value from a VIRTIO_SND_PCM_FMT_*
+ */
+static AudioFormat virtio_snd_get_qemu_format(uint32_t format)
+{
+    switch (format) {
+    case VIRTIO_SND_PCM_FMT_U8:
+        return AUDIO_FORMAT_U8;
+    case VIRTIO_SND_PCM_FMT_S8:
+        return AUDIO_FORMAT_S8;
+    case VIRTIO_SND_PCM_FMT_U16:
+        return AUDIO_FORMAT_U16;
+    case VIRTIO_SND_PCM_FMT_S16:
+        return AUDIO_FORMAT_S16;
+    case VIRTIO_SND_PCM_FMT_U32:
+        return AUDIO_FORMAT_U32;
+    case VIRTIO_SND_PCM_FMT_S32:
+        return AUDIO_FORMAT_S32;
+    case VIRTIO_SND_PCM_FMT_FLOAT:
+        return AUDIO_FORMAT_F32;
+    default:
+        return -1;
+    }
+}
+
+/*
+ * Get a QEMU Audiosystem compatible frequency value from a
+ * VIRTIO_SND_PCM_RATE_*
+ */
+static uint32_t virtio_snd_get_qemu_freq(uint32_t rate)
+{
+    switch (rate) {
+    case VIRTIO_SND_PCM_RATE_5512:
+        return 5512;
+    case VIRTIO_SND_PCM_RATE_8000:
+        return 8000;
+    case VIRTIO_SND_PCM_RATE_11025:
+        return 11025;
+    case VIRTIO_SND_PCM_RATE_16000:
+        return 16000;
+    case VIRTIO_SND_PCM_RATE_22050:
+        return 22050;
+    case VIRTIO_SND_PCM_RATE_32000:
+        return 32000;
+    case VIRTIO_SND_PCM_RATE_44100:
+        return 44100;
+    case VIRTIO_SND_PCM_RATE_48000:
+        return 48000;
+    case VIRTIO_SND_PCM_RATE_64000:
+        return 64000;
+    case VIRTIO_SND_PCM_RATE_88200:
+        return 88200;
+    case VIRTIO_SND_PCM_RATE_96000:
+        return 96000;
+    case VIRTIO_SND_PCM_RATE_176399:
+        return 176399;
+    case VIRTIO_SND_PCM_RATE_192000:
+        return 192000;
+    case VIRTIO_SND_PCM_RATE_384000:
+        return 384000;
+    default:
+        return -1;
+    }
+}
+
+/*
+ * Get QEMU Audiosystem compatible audsettings from virtio based pcm stream
+ * params.
+ */
+static void virtio_snd_get_qemu_audsettings(audsettings *as,
+                                            virtio_snd_pcm_params *params)
+{
+    as->nchannels = params->channel;
+    as->fmt = virtio_snd_get_qemu_format(params->format);
+    as->freq = virtio_snd_get_qemu_freq(params->rate);
+    as->endianness = AUDIO_HOST_ENDIANNESS;
+}
+
+/*
+ * Get the maximum number of virtqueue elements that can be inserted
+ * into a virtio sound pcm stream
+ *
+ * @st: virtio sound pcm stream
+ */
+static int virtio_snd_pcm_get_nelems(virtio_snd_pcm_stream *st)
+{
+    return st->buffer_bytes / st->period_bytes
+           + !!(st->buffer_bytes % st->period_bytes);
+}
+
+/*
+ * Prepares a VirtIOSound card stream.
+ * Returns a virtio sound status (VIRTIO_SND_S_*).
+ *
+ * @s: VirtIOSound card
+ * @stream: stream id
+ */
+static uint32_t virtio_snd_pcm_prepare_impl(VirtIOSound *s, uint32_t stream)
+{
+    if (!s->streams || !s->pcm_params || !s->pcm_params[stream]) {
+        virtio_snd_err("Cannot prepare stream %d without params.\n", stream);
+        return VIRTIO_SND_S_BAD_MSG;
+    }
+
+    uint32_t supported_formats = 1 << VIRTIO_SND_PCM_FMT_S8 |
+                                 1 << VIRTIO_SND_PCM_FMT_U8 |
+                                 1 << VIRTIO_SND_PCM_FMT_S16 |
+                                 1 << VIRTIO_SND_PCM_FMT_U16 |
+                                 1 << VIRTIO_SND_PCM_FMT_S32 |
+                                 1 << VIRTIO_SND_PCM_FMT_U32 |
+                                 1 << VIRTIO_SND_PCM_FMT_FLOAT;
+
+    uint32_t supported_rates = 1 << VIRTIO_SND_PCM_RATE_5512 |
+                               1 << VIRTIO_SND_PCM_RATE_8000 |
+                               1 << VIRTIO_SND_PCM_RATE_11025 |
+                               1 << VIRTIO_SND_PCM_RATE_16000 |
+                               1 << VIRTIO_SND_PCM_RATE_22050 |
+                               1 << VIRTIO_SND_PCM_RATE_32000 |
+                               1 << VIRTIO_SND_PCM_RATE_44100 |
+                               1 << VIRTIO_SND_PCM_RATE_48000 |
+                               1 << VIRTIO_SND_PCM_RATE_64000 |
+                               1 << VIRTIO_SND_PCM_RATE_88200 |
+                               1 << VIRTIO_SND_PCM_RATE_96000 |
+                               1 << VIRTIO_SND_PCM_RATE_176399 |
+                               1 << VIRTIO_SND_PCM_RATE_192000 |
+                               1 << VIRTIO_SND_PCM_RATE_384000;
+
+    virtio_snd_pcm_stream *st = g_new0(virtio_snd_pcm_stream, 1);
+    st->hda_fn_nid = VIRTIO_SOUND_HDA_FN_NID_OUT;
+    st->features = 0;
+    st->direction = stream <= s->snd_conf.streams / 2 ?
+                    VIRTIO_SND_D_OUTPUT : VIRTIO_SND_D_INPUT;
+    st->channels_min = 1;
+    st->channels_max = AUDIO_MAX_CHANNELS;
+    st->formats = supported_formats;
+    st->rates = supported_rates;
+    st->s = s;
+
+    st->buffer_bytes = s->pcm_params[stream]->buffer_bytes;
+    st->period_bytes = s->pcm_params[stream]->period_bytes;
+
+    audsettings as;
+    virtio_snd_get_qemu_audsettings(&as, s->pcm_params[stream]);
+
+    if (st->direction == VIRTIO_SND_D_OUTPUT) {
+        /* st->voice.out = AUD_open_out(&s->card,
+         *                              st->voice.out,
+         *                              "virtio_snd_card",
+         *                              st,
+         *                              virtio_snd_output_cb, &as);
+         */
+    } else {
+        /* st->voice.in = AUD_open_in(&s->card,
+         *                            st->voice.in,
+         *                            "virtio_snd_card",
+         *                            st,
+         *                            virtio_snd_input_cb,
+         *                            &as);
+         */
+    }
+
+    uint32_t nelems = virtio_snd_pcm_get_nelems(st);
+    st->elems = g_new0(VirtQueueElement *, nelems);
+    st->tail = -1;
+    st->w_pos = 0;
+    st->r_pos = 0;
+    s->streams[stream] = st;
+
+    return VIRTIO_SND_S_OK;
+}
+
+/*
+ * Handles VIRTIO_SND_R_PCM_PREPARE.
+ * The function writes the response to the virtqueue element.
+ * Returns the used size in bytes.
+ *
+ * @s: VirtIOSound card
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_pcm_prepare(VirtIOSound *s,
+                                              VirtQueueElement *elem)
+{
+    virtio_snd_pcm_hdr req;
+    size_t sz;
+
+    sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+    assert(sz == sizeof(virtio_snd_pcm_hdr));
+
+    virtio_snd_hdr resp;
+    resp.code = virtio_snd_pcm_prepare_impl(s, req.stream_id);
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+    return sz;
+}
+
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
  * element as used.
@@ -429,7 +624,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             sz = virtio_snd_handle_pcm_set_params(s, elem);
             goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_PREPARE) {
-            virtio_snd_log("VIRTIO_SND_R_PCM_PREPARE");
+            sz = virtio_snd_handle_pcm_prepare(s, elem);
+            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_START) {
             virtio_snd_log("VIRTIO_SND_R_PCM_START");
         } else if (ctrl.code == VIRTIO_SND_R_PCM_STOP) {
-- 
2.25.1

[RFC PATCH 18/27] virtio-snd: Add default configs to realize fn

[Shreyansh Chouhan]

Added default configurations for streams and jacks in the
realize function.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index dba90cc4a7..d0ab19a04a 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -683,11 +683,51 @@ static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
     /* set up QEMUSoundCard and audiodev */
     AUD_register_card ("virtio_snd_card", &s->card);
 
+    // set default params for all streams
+    virtio_snd_pcm_set_params default_params;
+    default_params.features = 0;
+    default_params.buffer_bytes = 8192;
+    default_params.period_bytes = 4096;
+    default_params.channel = 2;
+    default_params.format = VIRTIO_SND_PCM_FMT_S16;
+    default_params.rate = VIRTIO_SND_PCM_RATE_44100;
+
     s->ctrl_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_ctrl);
 
     s->streams = g_new0(virtio_snd_pcm_stream *, s->snd_conf.streams);
     s->pcm_params = g_new0(virtio_snd_pcm_params *, s->snd_conf.streams);
     s->jacks = g_new0(virtio_snd_jack *, s->snd_conf.jacks);
+
+    uint32_t status;
+    for (int i = 0; i < s->snd_conf.streams; i++) {
+        default_params.hdr.stream_id = i;
+        status = virtio_snd_pcm_set_params_impl(s, &default_params);
+        if (status != VIRTIO_SND_S_OK) {
+            error_setg(errp, "Can't initalize stream params.\n");
+            return;
+        }
+        status = virtio_snd_pcm_prepare_impl(s, i);
+        if (status != VIRTIO_SND_S_OK) {
+            error_setg(errp, "Can't prepare streams.\n");
+            return;
+        }
+    }
+
+    for (int i = 0; i < s->snd_conf.jacks; i++) {
+        // TODO: For now the hda_fn_nid connects the starting streams to these
+        // jacks. This isn't working for now since the directions will be wrong
+        // for a few jacks. Similarly the capabilities are just placeholders.
+        s->jacks[i] = (virtio_snd_jack *)g_malloc0(sizeof(virtio_snd_jack));
+        s->jacks[i]->features = 0;
+        s->jacks[i]->hda_fn_nid = i;
+        s->jacks[i]->hda_reg_defconf = ((AC_JACK_PORT_COMPLEX << AC_DEFCFG_PORT_CONN_SHIFT) |
+                                       (AC_JACK_LINE_OUT     << AC_DEFCFG_DEVICE_SHIFT)    |
+                                       (AC_JACK_CONN_1_8     << AC_DEFCFG_CONN_TYPE_SHIFT) |
+                                       (AC_JACK_COLOR_GREEN  << AC_DEFCFG_COLOR_SHIFT)     |
+                                       0x10);
+        s->jacks[i]->hda_reg_caps = AC_PINCAP_OUT;
+        s->jacks[i]->connected = false;
+    }
 }
 
 static void virtio_snd_device_unrealize(DeviceState *dev)
-- 
2.25.1

[RFC PATCH 19/27] virtio-snd: Add callback for SWVoiceOut

[Shreyansh Chouhan]

Added the callback for writing audio using AUD_write. The callback uses
two helper functions for reading the buffers from the streams and
handling the buffers that were written. initialized the
SWVoiceOut using this callback.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 171 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 165 insertions(+), 6 deletions(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index d0ab19a04a..e5042caf9c 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -468,6 +468,165 @@ static int virtio_snd_pcm_get_nelems(virtio_snd_pcm_stream *st)
            + !!(st->buffer_bytes % st->period_bytes);
 }
 
+/*
+ * Get the size in bytes of the buffer that still has to be written.
+ *
+ * @st: virtio sound pcm stream
+ */
+static int virtio_snd_pcm_get_pending_bytes(virtio_snd_pcm_stream *st)
+{
+    int pending = st->r_pos - st->w_pos;
+    return pending < 0 ? pending + st->buffer_bytes : pending;
+}
+
+/*
+ * Get data from a stream of the virtio sound device.
+ *
+ * @st: VirtIOSound card stream
+ * @offset: Start reading from this offseta in the stream (in bytes)
+ * @buffer: Write to this buffer
+ * @size: The number of bytes to read
+ */
+static void virtio_snd_pcm_get_buf(virtio_snd_pcm_stream *st, uint32_t offset,
+                                      void *buffer, uint32_t size)
+{
+    int nelems = virtio_snd_pcm_get_nelems(st);
+    int i = 0;
+    while (offset > st->period_bytes) {
+        offset -= st->period_bytes;
+        i++;
+        i %= nelems;
+    }
+
+
+    /*
+     * If the size spans beyond the current virtqueue element, read everything
+     * from the current virtqueue element and move to the next element. Repeat
+     * until we have read the required size.
+     */
+    while (size) {
+        int remaining = iov_size(st->elems[i]->out_sg, st->elems[i]->out_num)
+                        - sizeof(virtio_snd_pcm_xfer) - offset;
+        int to_read = MIN(remaining, size), wpos = 0;
+        size_t sz;
+        sz = iov_to_buf(st->elems[i]->out_sg, st->elems[i]->out_num,
+                        sizeof(virtio_snd_pcm_xfer) + offset, buffer + wpos,
+                        to_read);
+
+        assert(sz == to_read);
+
+        offset = 0;
+        size -= to_read;
+        wpos += to_read;
+        i++;
+        i %= nelems;
+    }
+}
+
+/*
+ * Handle a buffer after it has been written by AUD_write.
+ * It writes the status for the I/O messages that have been completed and
+ * marks the tx virtqueue elmenets as used. It notifies the device
+ * about I/O completion.
+ *
+ * @st: VirtIOSound card stream
+ * @size: Size that was written by AUD_write
+ *        If size = 0, write for the last element failed
+ */
+static void virtio_snd_pcm_handle_buf_written(virtio_snd_pcm_stream *st,
+                                              uint32_t size)
+{
+    int offset = st->w_pos, i = 0;
+    int nelems = virtio_snd_pcm_get_nelems(st);
+
+    while (offset >= st->period_bytes) {
+        offset -= st->period_bytes;
+        i++;
+        i %= nelems;
+    }
+
+    virtio_snd_pcm_status status;
+    size_t sz;
+    if (!size) {
+        status.status = VIRTIO_SND_S_IO_ERR;
+        status.latency_bytes = 0;
+
+        sz = iov_from_buf(st->elems[i]->in_sg, st->elems[i]->in_num, 0,
+                          &status, sizeof(status));
+        assert(sz == sizeof(virtio_snd_pcm_status));
+
+        int push_size = iov_size(st->elems[i]->out_sg, st->elems[i]->out_num)
+                        + sizeof(virtio_snd_pcm_status);
+
+        virtqueue_push(st->s->tx_vq, st->elems[i], push_size);
+        return;
+    }
+
+    status.status = VIRTIO_SND_S_OK;
+    status.latency_bytes = 0;
+
+    /*
+     * If the written size spans beyond current element, update the status for
+     * the current element, mark it as used and push it back on the tx
+     * virtqueue. Notify the device about the I/O completion. Repeat until
+     * the required bytes are handled.
+     */
+    while (size) {
+        int curr_elem_size = iov_size(st->elems[i]->out_sg, st->elems[i]->out_num)
+                             - sizeof(virtio_snd_pcm_xfer) - offset;
+        if (size >= curr_elem_size) {
+            sz = iov_from_buf(st->elems[i]->in_sg, st->elems[i]->in_num, 0,
+                              &status, sizeof(status));
+            assert(sz == sizeof(virtio_snd_pcm_status));
+
+            int push_size = sizeof(virtio_snd_pcm_xfer) + curr_elem_size
+                            + offset + sizeof(virtio_snd_pcm_status);
+
+            virtqueue_push(st->s->tx_vq, st->elems[i], push_size);
+            virtio_notify(VIRTIO_DEVICE(st->s), st->s->tx_vq);
+            g_free(st->elems[i]);
+            st->elems[i] = NULL;
+            size -= curr_elem_size;
+            virtio_snd_log("remains: %d\n", size);
+            st->w_pos += curr_elem_size;
+            st->w_pos %= st->buffer_bytes + 1;
+            i++;
+            i %= nelems;
+            offset = 0;
+        } else {
+            st->w_pos += size;
+            st->w_pos %= st->buffer_bytes + 1;
+            break;
+        }
+    }
+}
+
+/*
+ * Callback for AUD_open_out.
+ * Reads a buffer from the VirtIOSound card stream and writes it
+ * using AUD_write.
+ *
+ * @opaque: VirtIOSound card stream
+ * @free: Size in bytes that can be written via AUD_write
+ */
+static void virtio_snd_output_cb(void *opaque, int free)
+{
+    int to_play, pending;
+    virtio_snd_pcm_stream *st = opaque;
+
+    pending = virtio_snd_pcm_get_pending_bytes(st);
+    to_play = MIN(free, pending);
+
+    while (to_play) {
+        uint8_t *mixbuf = g_malloc0(to_play);
+        virtio_snd_pcm_get_buf(st, st->w_pos, mixbuf, to_play);
+        int wbytes = AUD_write(st->voice.out, mixbuf, to_play);
+        to_play -= wbytes;
+        virtio_snd_pcm_handle_buf_written(st, wbytes);
+        g_free(mixbuf);
+    }
+}
+
 /*
  * Prepares a VirtIOSound card stream.
  * Returns a virtio sound status (VIRTIO_SND_S_*).
@@ -523,12 +682,12 @@ static uint32_t virtio_snd_pcm_prepare_impl(VirtIOSound *s, uint32_t stream)
     virtio_snd_get_qemu_audsettings(&as, s->pcm_params[stream]);
 
     if (st->direction == VIRTIO_SND_D_OUTPUT) {
-        /* st->voice.out = AUD_open_out(&s->card,
-         *                              st->voice.out,
-         *                              "virtio_snd_card",
-         *                              st,
-         *                              virtio_snd_output_cb, &as);
-         */
+        st->voice.out = AUD_open_out(&s->card,
+                                     st->voice.out,
+                                     "virtio_snd_card",
+                                     st,
+                                     virtio_snd_output_cb, &as);
+
     } else {
         /* st->voice.in = AUD_open_in(&s->card,
          *                            st->voice.in,
-- 
2.25.1

[RFC PATCH 20/27] virtio-snd: Add VIRITO_SND_R_PCM_START handler

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index e5042caf9c..65104baba8 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -732,6 +732,34 @@ static uint32_t virtio_snd_handle_pcm_prepare(VirtIOSound *s,
     return sz;
 }
 
+/*
+ * Handles VIRTIO_SND_R_PCM_START.
+ * The function writes the response to the virtqueue element.
+ * Returns the used size in bytes.
+ *
+ * @s: VirtIOSound card
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_pcm_start(VirtIOSound *s,
+                                            VirtQueueElement *elem)
+{
+    virtio_snd_pcm_hdr req;
+    size_t sz;
+
+    sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+    assert(sz == sizeof(virtio_snd_pcm_hdr));
+
+    virtio_snd_hdr resp;
+    resp.code = VIRTIO_SND_S_OK;
+
+    virtio_snd_pcm_stream *st = virtio_snd_pcm_get_stream(s, req.stream_id);
+    AUD_set_active_out(st->voice.out, true);
+
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+    return sz;
+}
+
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
  * element as used.
@@ -786,7 +814,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             sz = virtio_snd_handle_pcm_prepare(s, elem);
             goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_START) {
-            virtio_snd_log("VIRTIO_SND_R_PCM_START");
+            sz = virtio_snd_handle_pcm_start(s, elem);
+            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_STOP) {
             virtio_snd_log("VIRTIO_SND_R_PCM_STOP");
         } else if (ctrl.code == VIRTIO_SND_R_PCM_RELEASE) {
-- 
2.25.1

[RFC PATCH 21/27] virtio-snd: Add VIRTIO_SND_R_PCM_STOP handler

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 65104baba8..8be3fb9bf5 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -760,6 +760,35 @@ static uint32_t virtio_snd_handle_pcm_start(VirtIOSound *s,
     return sz;
 }
 
+/*
+ * Handles VIRTIO_SND_R_PCM_STOP.
+ * The function writes the response to the virtqueue element.
+ * Returns the used size in bytes.
+ * TODO: Doesn't handle the stream buffers that are yet to be played.
+ *
+ * @s: VirtIOSound card
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_pcm_stop(VirtIOSound *s,
+                                           VirtQueueElement *elem)
+{
+    virtio_snd_pcm_hdr req;
+    size_t sz;
+
+    sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+    assert(sz == sizeof(virtio_snd_pcm_hdr));
+
+    virtio_snd_hdr resp;
+    resp.code = VIRTIO_SND_S_OK;
+
+    virtio_snd_pcm_stream *st = virtio_snd_pcm_get_stream(s, req.stream_id);
+    AUD_set_active_out(st->voice.out, false);
+
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+    return sz;
+}
+
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
  * element as used.
@@ -817,7 +846,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             sz = virtio_snd_handle_pcm_start(s, elem);
             goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_STOP) {
-            virtio_snd_log("VIRTIO_SND_R_PCM_STOP");
+            sz = virtio_snd_handle_pcm_stop(s, elem);
+            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_RELEASE) {
             virtio_snd_log("VIRTIO_SND_R_PCM_RELEASE");
         } else {
-- 
2.25.1

Re: [RFC PATCH 21/27] virtio-snd: Add VIRTIO_SND_R_PCM_STOP handler

[Gerd Hoffmann]

> +static uint32_t virtio_snd_handle_pcm_stop(VirtIOSound *s,
> +                                           VirtQueueElement *elem)

Looks very simliar to virtio_snd_handle_pcm_start.

Maybe it makes sense to have an
	virtio_snd_handle_pcm_start_stop(..., bool start)
function instead?

take care,
  Gerd

[RFC PATCH 22/27] virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 66 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 65 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 8be3fb9bf5..56e07a41f8 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -789,6 +789,69 @@ static uint32_t virtio_snd_handle_pcm_stop(VirtIOSound *s,
     return sz;
 }
 
+/*
+ * Releases the resources allocated to a stream. Sepearated from the handler
+ * so that the code could be reused in the unrealize function.
+ *
+ * TODO: Doesn't handle the stream buffers that are yet to be played.
+ *
+ * @s: VirtIOSound card
+ * @stream: stream id
+ */
+static uint32_t virtio_snd_pcm_release_impl(VirtIOSound *s, uint32_t stream)
+{
+    virtio_snd_pcm_stream *st = virtio_snd_pcm_get_stream(s, stream);
+    if (st) {
+        virtio_snd_err("already released %d\n", stream);
+        return VIRTIO_SND_S_BAD_MSG;
+    }
+
+    if (st->voice.out) {
+        AUD_close_out(&s->card, st->voice.out);
+    } else if (st->voice.in) {
+        AUD_close_in(&s->card, st->voice.in);
+    }
+
+    if (st->elems) {
+        int nelems = virtio_snd_pcm_get_nelems(st);
+        for (int i = 0; i < nelems; i++) {
+            g_free(st->elems[i]);
+            st->elems[i] = NULL;
+        }
+        g_free(st->elems);
+        st->elems = NULL;
+    }
+
+    g_free(s->streams[stream]);
+    s->streams[stream] = NULL;
+    return VIRTIO_SND_S_OK;
+}
+
+/*
+ * Handles VIRTIO_SND_R_PCM_RELEASE.
+ * The function writes the response to the virtqueue element.
+ * Returns the used size in bytes.
+ * TODO: Doesn't handle the stream buffers that are yet to be played.
+ *
+ * @s: VirtIOSound card
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_pcm_release(VirtIOSound *s,
+                                              VirtQueueElement *elem)
+{
+    virtio_snd_pcm_hdr req;
+    virtio_snd_hdr resp;
+    size_t sz;
+    sz = iov_from_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+    assert(sz == sizeof(virtio_snd_pcm_hdr));
+
+    virtio_snd_pcm_release_impl(s, req.stream_id);
+
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+    return sz;
+}
+
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
  * element as used.
@@ -849,7 +912,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             sz = virtio_snd_handle_pcm_stop(s, elem);
             goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_RELEASE) {
-            virtio_snd_log("VIRTIO_SND_R_PCM_RELEASE");
+            sz = virtio_snd_handle_pcm_release(s, elem);
+            goto done;
         } else {
             /* error */
             virtio_snd_err("virtio snd header not recognized: %d\n", ctrl.code);
-- 
2.25.1

[RFC PATCH 23/27] virtio-snd: Replaced goto with if else

[Shreyansh Chouhan]

Removed goto from the ctrl vq handler and added an if else
branch for error handling.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 56e07a41f8..5f12375d95 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -862,7 +862,7 @@ static uint32_t virtio_snd_handle_pcm_release(VirtIOSound *s,
 static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOSound *s = VIRTIO_SOUND(vdev);
-    virtio_snd_hdr ctrl;
+    virtio_snd_hdr ctrl, resp;
 
     VirtQueueElement *elem = NULL;
     size_t sz;
@@ -874,7 +874,7 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         if (!elem) {
             break;
         }
-        if (iov_size(elem->in_sg, elem->in_num) < sizeof(ctrl) ||
+        if (iov_size(elem->in_sg, elem->in_num) < sizeof(resp) ||
                 iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
             virtio_snd_err("virtio-snd ctrl missing headers\n");
             virtqueue_detach_element(vq, elem, 0);
@@ -890,40 +890,34 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         if (sz != sizeof(ctrl)) {
             /* error */
             virtio_snd_err("virtio snd ctrl could not read header\n");
+            resp.code = VIRTIO_SND_S_BAD_MSG;
         } else if (ctrl.code == VIRTIO_SND_R_JACK_INFO) {
             sz = virtio_snd_handle_jack_info(s, elem);
-            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_JACK_REMAP) {
             sz = virtio_snd_handle_jack_remap(s, elem);
-            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
             sz = virtio_snd_handle_pcm_info(s, elem);
-            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_SET_PARAMS) {
             sz = virtio_snd_handle_pcm_set_params(s, elem);
-            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_PREPARE) {
             sz = virtio_snd_handle_pcm_prepare(s, elem);
-            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_START) {
             sz = virtio_snd_handle_pcm_start(s, elem);
-            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_STOP) {
             sz = virtio_snd_handle_pcm_stop(s, elem);
-            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_RELEASE) {
             sz = virtio_snd_handle_pcm_release(s, elem);
-            goto done;
         } else {
             /* error */
             virtio_snd_err("virtio snd header not recognized: %d\n", ctrl.code);
+            resp.code = VIRTIO_SND_S_BAD_MSG;
         }
 
-        virtio_snd_hdr resp;
-        resp.code = VIRTIO_SND_S_OK;
-        sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+        if (resp.code == VIRTIO_SND_S_BAD_MSG) {
+            sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp,
+                              sizeof(resp));
+        }
 
-done:
         virtqueue_push(vq, elem, sz);
         virtio_notify(vdev, vq);
         g_free(iov2);
-- 
2.25.1

[RFC PATCH 24/27] virtio-snd: Add code to device unrealize function

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 5f12375d95..e2e47e9467 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -1006,8 +1006,34 @@ static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
     }
 }
 
+/*
+ * Frees the resources allocated to the device and then frees the device
+ * itself.
+ *
+ * @dev: VirtIOSound card device
+ */
 static void virtio_snd_device_unrealize(DeviceState *dev)
 {
+    VirtIOSound *s = VIRTIO_SOUND(dev);
+
+    for (int i = 0; i < s->snd_conf.streams; i++) {
+        virtio_snd_pcm_release_impl(s, i);
+        g_free(s->pcm_params[i]);
+        s->pcm_params[i] = NULL;
+    }
+    g_free(s->streams);
+    s->streams = NULL;
+    g_free(s->pcm_params);
+    s->pcm_params = NULL;
+
+    for (int i = 0; i < s->snd_conf.jacks; i++) {
+        g_free(s->jacks[i]);
+        s->jacks[i] = NULL;
+    }
+    g_free(s->jacks);
+    s->jacks = NULL;
+
+    virtio_delete_queue(s->ctrl_vq);
 }
 
 static void virtio_snd_reset(VirtIODevice *vdev)
-- 
2.25.1

[RFC PATCH 25/27] virtio-snd: Add tx vq and handler

[Shreyansh Chouhan]

The handler demultiplexes the buffers recieved in the
tx virtqueue. It uses a helper function for adding these
buffers, (along with the entire virtqueue element,) to
their respective streams.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 62 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index e2e47e9467..be9ea05e3f 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -925,6 +925,66 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
     }
 }
 
+/*
+ * Adds a virtqueue element to a VirtIOSound card stream. Makes the buffer
+ * available to the stream for consumption.
+ *
+ * @s: VirtIOSound card
+ * @stream: stream id
+ * @elem: The tx virtqueue element that contains the I/O message
+ */
+static void virtio_snd_pcm_add_buf(VirtIOSound *s, uint32_t stream,
+                                   VirtQueueElement *elem)
+{
+    virtio_snd_pcm_stream *st = virtio_snd_pcm_get_stream(s, stream);
+    int nelems = virtio_snd_pcm_get_nelems(st);
+    st->tail++;
+    st->tail %= nelems;
+
+    if (st->elems[st->tail]) {
+        return;
+    }
+
+    st->elems[st->tail] = elem;
+    st->r_pos += iov_size(elem->out_sg, elem->out_num)
+                 - sizeof(virtio_snd_pcm_xfer);
+    st->r_pos %= st->buffer_bytes + 1;
+}
+
+/*
+ * The tx virtqueue handler. Makes the buffers available to their respective
+ * streams for consumption.
+ *
+ * @vdev: VirtIOSound card
+ * @vq: tx virtqueue
+ */
+static void virtio_snd_handle_tx(VirtIODevice *vdev, VirtQueue *vq)
+{
+    VirtIOSound *s = VIRTIO_SOUND(vdev);
+    VirtQueueElement *elem;
+    size_t sz;
+    virtio_snd_pcm_xfer hdr;
+
+    for (;;) {
+        elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+        if (!elem) {
+            break;
+        }
+        if (iov_size(elem->in_sg, elem->in_num) < sizeof(virtio_snd_pcm_status) ||
+            iov_size(elem->out_sg, elem->out_num) < sizeof(virtio_snd_pcm_xfer)) {
+            virtqueue_detach_element(vq, elem, 0);
+            g_free(elem);
+            break;
+        }
+
+        /* get the message hdr object */
+        sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &hdr, sizeof(hdr));
+        assert(sz == sizeof(virtio_snd_pcm_xfer));
+
+        virtio_snd_pcm_add_buf(s, hdr.stream_id, elem);
+    }
+}
+
 /*
  * Initializes the VirtIOSound card device. Validates the configuration
  * passed by the command line. Initializes the virtqueues. Allocates resources
@@ -969,6 +1029,7 @@ static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
     default_params.rate = VIRTIO_SND_PCM_RATE_44100;
 
     s->ctrl_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_ctrl);
+    s->tx_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_tx);
 
     s->streams = g_new0(virtio_snd_pcm_stream *, s->snd_conf.streams);
     s->pcm_params = g_new0(virtio_snd_pcm_params *, s->snd_conf.streams);
@@ -1034,6 +1095,7 @@ static void virtio_snd_device_unrealize(DeviceState *dev)
     s->jacks = NULL;
 
     virtio_delete_queue(s->ctrl_vq);
+    virtio_delete_queue(s->tx_vq);
 }
 
 static void virtio_snd_reset(VirtIODevice *vdev)
-- 
2.25.1

[RFC PATCH 26/27] virtio-snd: Add event vq and a handler stub

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index be9ea05e3f..2d7b7ce39a 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -985,6 +985,18 @@ static void virtio_snd_handle_tx(VirtIODevice *vdev, VirtQueue *vq)
     }
 }
 
+/*
+ * The event virtqueue handler.
+ * Not implemented yet.
+ *
+ * @vdev: VirtIOSound card
+ * @vq: event vq
+ */
+static void virtio_snd_handle_event(VirtIODevice *vdev, VirtQueue *vq)
+{
+    virtio_snd_log("event queue callback called\n");
+}
+
 /*
  * Initializes the VirtIOSound card device. Validates the configuration
  * passed by the command line. Initializes the virtqueues. Allocates resources
@@ -1029,6 +1041,7 @@ static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
     default_params.rate = VIRTIO_SND_PCM_RATE_44100;
 
     s->ctrl_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_ctrl);
+    s->event_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_event);
     s->tx_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_tx);
 
     s->streams = g_new0(virtio_snd_pcm_stream *, s->snd_conf.streams);
@@ -1096,6 +1109,7 @@ static void virtio_snd_device_unrealize(DeviceState *dev)
 
     virtio_delete_queue(s->ctrl_vq);
     virtio_delete_queue(s->tx_vq);
+    virtio_delete_queue(s->event_vq);
 }
 
 static void virtio_snd_reset(VirtIODevice *vdev)
-- 
2.25.1

[RFC PATCH 27/27] virtio-snd: Add rx vq and stub handler

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 2d7b7ce39a..6a1d3e824c 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -997,6 +997,18 @@ static void virtio_snd_handle_event(VirtIODevice *vdev, VirtQueue *vq)
     virtio_snd_log("event queue callback called\n");
 }
 
+/*
+ * The rx virtqueue handler.
+ * Not implemented yet.
+ *
+ * @vdev: VirtIOSound card
+ * @vq: rx vq
+ */
+static void virtio_snd_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
+{
+    virtio_snd_log("rx queue callback called\n");
+}
+
 /*
  * Initializes the VirtIOSound card device. Validates the configuration
  * passed by the command line. Initializes the virtqueues. Allocates resources
@@ -1043,6 +1055,7 @@ static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
     s->ctrl_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_ctrl);
     s->event_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_event);
     s->tx_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_tx);
+    s->rx_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_rx);
 
     s->streams = g_new0(virtio_snd_pcm_stream *, s->snd_conf.streams);
     s->pcm_params = g_new0(virtio_snd_pcm_params *, s->snd_conf.streams);
@@ -1110,6 +1123,7 @@ static void virtio_snd_device_unrealize(DeviceState *dev)
     virtio_delete_queue(s->ctrl_vq);
     virtio_delete_queue(s->tx_vq);
     virtio_delete_queue(s->event_vq);
+    virtio_delete_queue(s->rx_vq);
 }
 
 static void virtio_snd_reset(VirtIODevice *vdev)
-- 
2.25.1

Re: [RFC PATCH 00/27] Virtio sound card implementation

[no-reply]

Patchew URL: https://patchew.org/QEMU/20210429120445.694420-1-chouhan.shreyansh2702@gmail.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210429120445.694420-1-chouhan.shreyansh2702@gmail.com
Subject: [RFC PATCH 00/27] Virtio sound card implementation

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210429120445.694420-1-chouhan.shreyansh2702@gmail.com -> patchew/20210429120445.694420-1-chouhan.shreyansh2702@gmail.com
Switched to a new branch 'test'
28acae8 virtio-snd: Add rx vq and stub handler
f130077 virtio-snd: Add event vq and a handler stub
ba43c58 virtio-snd: Add tx vq and handler
d1fae3c virtio-snd: Add code to device unrealize function
5633fc1 virtio-snd: Replaced goto with if else
df2aca8 virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler
4d5b976 virtio-snd: Add VIRTIO_SND_R_PCM_STOP handler
9c792b2 virtio-snd: Add VIRITO_SND_R_PCM_START handler
ad07e2c virtio-snd: Add callback for SWVoiceOut
6a9e47d virtio-snd: Add default configs to realize fn
75c62c8 virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler
64043bd virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle
bb5fe5d virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler
858de0e virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler
a1596e5 virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler
d303365 virtio-snd: Add control virtqueue handler
4c49f92 virtio-snd: Add macros for logging
504942a virtio-snd: Add code for the realize function
d7fad34 virtio-snd: Add code for set config function
f1d51f3 virtio-snd: Add code for get config function
cfc404f virtio-snd: Add properties for class init
b0b4fa2 virtio-snd: Add PCI wrapper code for VirtIOSound
0a0606a virtio-snd: Add device implementation structures
e57488a virtio-snd: Add chmap control structures
f8faf7e virtio-snd: Add PCM control structures
ba1cc8b virtio-snd: Add jack control structures
cc2413a virtio-snd: Add virtio sound header file

=== OUTPUT BEGIN ===
1/27 Checking commit cc2413a852b8 (virtio-snd: Add virtio sound header file)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100644

total: 0 errors, 1 warnings, 97 lines checked

Patch 1/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/27 Checking commit ba1cc8ba3399 (virtio-snd: Add jack control structures)
3/27 Checking commit f8faf7ec9a71 (virtio-snd: Add PCM control structures)
4/27 Checking commit e57488ae6767 (virtio-snd: Add chmap control structures)
5/27 Checking commit 0a0606abe96c (virtio-snd: Add device implementation structures)
6/27 Checking commit b0b4fa241458 (virtio-snd: Add PCI wrapper code for VirtIOSound)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#30: 
new file mode 100644

total: 0 errors, 1 warnings, 79 lines checked

Patch 6/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/27 Checking commit cfc404f0d2ff (virtio-snd: Add properties for class init)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#41: 
new file mode 100644

total: 0 errors, 1 warnings, 138 lines checked

Patch 7/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/27 Checking commit f1d51f3d3a6d (virtio-snd: Add code for get config function)
9/27 Checking commit d7fad34dacf1 (virtio-snd: Add code for set config function)
10/27 Checking commit 504942a2f235 (virtio-snd: Add code for the realize function)
11/27 Checking commit 4c49f923a5dd (virtio-snd: Add macros for logging)
12/27 Checking commit d3033655589b (virtio-snd: Add control virtqueue handler)
WARNING: Block comments use a leading /* on a separate line
#23: FILE: hw/audio/virtio-snd.c:104:
+/* The control queue handler. Pops an element from the control virtqueue,

total: 0 errors, 1 warnings, 83 lines checked

Patch 12/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/27 Checking commit a1596e59b05f (virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler)
14/27 Checking commit 858de0ed4483 (virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler)
15/27 Checking commit bb5fe5d45c23 (virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler)
16/27 Checking commit 64043bdef02c (virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle)
WARNING: line over 80 characters
#35: FILE: hw/audio/virtio-snd.c:294:
+                                               virtio_snd_pcm_set_params *params)

total: 0 errors, 1 warnings, 112 lines checked

Patch 16/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
17/27 Checking commit 75c62c857884 (virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler)
WARNING: Block comments use a leading /* on a separate line
#168: FILE: hw/audio/virtio-snd.c:526:
+        /* st->voice.out = AUD_open_out(&s->card,

WARNING: Block comments use a leading /* on a separate line
#175: FILE: hw/audio/virtio-snd.c:533:
+        /* st->voice.in = AUD_open_in(&s->card,

total: 0 errors, 2 warnings, 210 lines checked

Patch 17/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
18/27 Checking commit 6a9e47df0a16 (virtio-snd: Add default configs to realize fn)
Argument "m" isn't numeric in numeric eq (==) at ./scripts/checkpatch.pl line 2830.
Argument "m" isn't numeric in numeric eq (==) at ./scripts/checkpatch.pl line 2830.
Use of uninitialized value $1 in concatenation (.) or string at ./scripts/checkpatch.pl line 2831.
ERROR: do not use C99 // comments
#24: FILE: hw/audio/virtio-snd.c:686:
+    // set default params for all streams

ERROR: Error messages should not contain newlines
#44: FILE: hw/audio/virtio-snd.c:706:
+            error_setg(errp, "Can't initalize stream params.\n");

ERROR: Error messages should not contain newlines
#49: FILE: hw/audio/virtio-snd.c:711:
+            error_setg(errp, "Can't prepare streams.\n");

ERROR: do not use C99 // comments
#55: FILE: hw/audio/virtio-snd.c:717:
+        // TODO: For now the hda_fn_nid connects the starting streams to these

ERROR: do not use C99 // comments
#56: FILE: hw/audio/virtio-snd.c:718:
+        // jacks. This isn't working for now since the directions will be wrong

ERROR: do not use C99 // comments
#57: FILE: hw/audio/virtio-snd.c:719:
+        // for a few jacks. Similarly the capabilities are just placeholders.

ERROR: unnecessary cast may hide bugs, use g_new0 instead
#58: FILE: hw/audio/virtio-snd.c:720:
+        s->jacks[i] = (virtio_snd_jack *)g_malloc0(sizeof(virtio_snd_jack));

ERROR: line over 90 characters
#61: FILE: hw/audio/virtio-snd.c:723:
+        s->jacks[i]->hda_reg_defconf = ((AC_JACK_PORT_COMPLEX << AC_DEFCFG_PORT_CONN_SHIFT) |

ERROR: line over 90 characters
#62: FILE: hw/audio/virtio-snd.c:724:
+                                       (AC_JACK_LINE_OUT     << AC_DEFCFG_DEVICE_SHIFT)    |

ERROR: line over 90 characters
#63: FILE: hw/audio/virtio-snd.c:725:
+                                       (AC_JACK_CONN_1_8     << AC_DEFCFG_CONN_TYPE_SHIFT) |

ERROR: line over 90 characters
#64: FILE: hw/audio/virtio-snd.c:726:
+                                       (AC_JACK_COLOR_GREEN  << AC_DEFCFG_COLOR_SHIFT)     |

total: 11 errors, 0 warnings, 51 lines checked

Patch 18/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

19/27 Checking commit ad07e2c375df (virtio-snd: Add callback for SWVoiceOut)
WARNING: line over 80 characters
#129: FILE: hw/audio/virtio-snd.c:575:
+        int curr_elem_size = iov_size(st->elems[i]->out_sg, st->elems[i]->out_num)

total: 0 errors, 1 warnings, 183 lines checked

Patch 19/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
20/27 Checking commit 9c792b226bf4 (virtio-snd: Add VIRITO_SND_R_PCM_START handler)
21/27 Checking commit 4d5b9762480e (virtio-snd: Add VIRTIO_SND_R_PCM_STOP handler)
22/27 Checking commit df2aca848dfb (virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler)
23/27 Checking commit 5633fc14cfe7 (virtio-snd: Replaced goto with if else)
24/27 Checking commit d1fae3cd628b (virtio-snd: Add code to device unrealize function)
25/27 Checking commit ba43c5895b33 (virtio-snd: Add tx vq and handler)
WARNING: line over 80 characters
#70: FILE: hw/audio/virtio-snd.c:973:
+        if (iov_size(elem->in_sg, elem->in_num) < sizeof(virtio_snd_pcm_status) ||

WARNING: line over 80 characters
#71: FILE: hw/audio/virtio-snd.c:974:
+            iov_size(elem->out_sg, elem->out_num) < sizeof(virtio_snd_pcm_xfer)) {

total: 0 errors, 2 warnings, 80 lines checked

Patch 25/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
26/27 Checking commit f13007720379 (virtio-snd: Add event vq and a handler stub)
27/27 Checking commit 28acae83382f (virtio-snd: Add rx vq and stub handler)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210429120445.694420-1-chouhan.shreyansh2702@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [RFC PATCH 00/27] Virtio sound card implementation

[Gerd Hoffmann]

  Hi,

>     - The output from the sound card is accompanied by periodic
>       white noise. I do not know why this is happening. I tried
>       debugging it by writing the buffers to a new wav file and
>       sure enough the contents of the file were different at
>       some places, but I couldn't find what must be causing it.
>       (Relevant patches: #19, #20, #21 and #25.) What steps should
>       I take for debugging this?

Hmm, I'd try to simplify the code.  The two helper functions
virtio_snd_pcm_get_buf() + virtio_snd_pcm_handle_buf_written() look
rather complex to me.  I'd suggest to make them handle a single
virtqueue element only.  That should make the logic simpler.
The loop in virtio_snd_output_cb() will need to take a few more rounds
then, and virtio_snd_pcm_get_buf() would probably have to return the
number of bytes it actually placed in the buffer so you can pass on that
value to AUD_write().

For actual debugging I typically use trace points or temporary debug
printfs or a combination of both.  I'd suggest logging the buffer
handling, filling them from virt queue, writing to AUD, also log the
offset, maybe something goes wrong with partial writes.

>     - What is the suggested way of waiting? When the driver issues
>       the VIRTIO_SND_PCM_STOP ctrl command I want to wait for the
>       buffers existing in tx vq to be consumed before closing the
>       stream.

Store a pointer to the virtqueue element, then complete it when
virtio_snd_output_cb() processed all pending buffers.

take care,
  Gerd

[RFC PATCH v2 00/25] Virtio Sound card Implementation

[Shreyansh Chouhan]

The second RFC for implementing the VirtIO Sound card as described in
the virtio specs. Sorry for the absence of activity on this.

The output from the sound card works.

What remains to be done:
- Features defined in PCM features. (Eg message polling)
- Channel maps
- Jack remaps
- Input

I will work on the input after I have implemented the output
along with all the features since at that point it should just be a
matter of reversing a few things in the code that writes the audio.

I can work on this patchset mostly on weekends now but I will try to be
more regular with this.

Reviews are welcome :)

Shreyansh Chouhan (25):
  virtio-snd: Add virtio sound header file
  virtio-snd: Add jack control structures
  virtio-snd: Add PCM control structures
  virtio-snd: Add chmap control structures
  virtio-snd: Add device implementation structures
  virtio-snd: Add PCI wrapper code for VirtIOSound
  virtio-snd: Add properties for class init
  virtio-snd: Add code for get config function
  virtio-snd: Add code for the realize function
  virtio-snd: Add macros for logging
  virtio-snd: Add control virtqueue handler
  virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler
  virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler
  virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler
  virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle
  virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler
  virtio-snd: Add default configs to realize fn
  virtio-snd: Add callback for SWVoiceOut
  virtio-snd: Add start/stop handler
  virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler
  virtio-snd: Replaced goto with if else
  virtio-snd: Add code to device unrealize function
  virtio-snd: Add xfer handler
  virtio-snd: Add event vq and a handler stub
  virtio-snd: Replaced AUD_log with tracepoints

 hw/audio/Kconfig               |    5 +
 hw/audio/meson.build           |    1 +
 hw/audio/trace-events          |   14 +
 hw/audio/virtio-snd.c          | 1241 ++++++++++++++++++++++++++++++++
 hw/virtio/meson.build          |    1 +
 hw/virtio/virtio-snd-pci.c     |   72 ++
 include/hw/virtio/virtio-snd.h |  383 ++++++++++
 7 files changed, 1717 insertions(+)
 create mode 100644 hw/audio/virtio-snd.c
 create mode 100644 hw/virtio/virtio-snd-pci.c
 create mode 100644 include/hw/virtio/virtio-snd.h

-- 
2.31.1

Re: [RFC PATCH v2 00/25] Virtio Sound card Implementation

[Laurent Vivier]

Le 11/02/2022 à 23:12, Shreyansh Chouhan a écrit :
> The second RFC for implementing the VirtIO Sound card as described in
> the virtio specs. Sorry for the absence of activity on this.
> 
> The output from the sound card works.
> 
> What remains to be done:
> - Features defined in PCM features. (Eg message polling)
> - Channel maps
> - Jack remaps
> - Input
> 
> I will work on the input after I have implemented the output
> along with all the features since at that point it should just be a
> matter of reversing a few things in the code that writes the audio.
> 
> I can work on this patchset mostly on weekends now but I will try to be
> more regular with this.
> 
> Reviews are welcome :)
> 
> Shreyansh Chouhan (25):
>    virtio-snd: Add virtio sound header file
>    virtio-snd: Add jack control structures
>    virtio-snd: Add PCM control structures
>    virtio-snd: Add chmap control structures
>    virtio-snd: Add device implementation structures
>    virtio-snd: Add PCI wrapper code for VirtIOSound
>    virtio-snd: Add properties for class init
>    virtio-snd: Add code for get config function
>    virtio-snd: Add code for the realize function
>    virtio-snd: Add macros for logging
>    virtio-snd: Add control virtqueue handler
>    virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler
>    virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler
>    virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler
>    virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle
>    virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler
>    virtio-snd: Add default configs to realize fn
>    virtio-snd: Add callback for SWVoiceOut
>    virtio-snd: Add start/stop handler
>    virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler
>    virtio-snd: Replaced goto with if else
>    virtio-snd: Add code to device unrealize function
>    virtio-snd: Add xfer handler
>    virtio-snd: Add event vq and a handler stub
>    virtio-snd: Replaced AUD_log with tracepoints
> 
>   hw/audio/Kconfig               |    5 +
>   hw/audio/meson.build           |    1 +
>   hw/audio/trace-events          |   14 +
>   hw/audio/virtio-snd.c          | 1241 ++++++++++++++++++++++++++++++++
>   hw/virtio/meson.build          |    1 +
>   hw/virtio/virtio-snd-pci.c     |   72 ++
>   include/hw/virtio/virtio-snd.h |  383 ++++++++++
>   7 files changed, 1717 insertions(+)
>   create mode 100644 hw/audio/virtio-snd.c
>   create mode 100644 hw/virtio/virtio-snd-pci.c
>   create mode 100644 include/hw/virtio/virtio-snd.h
> 

Thank you for your work.

IMHO, all your patches can be merged in only one. Morever it would help for review as some patches 
remove code done in previous patches.

The "v2" tag is missing in the subject of the patches of your series.

And don't send a series as a reply of a previous one.

You can use "git-publish" it helps a lot when we have to send several versions of a series.

Thanks,
Laurent

Re: [RFC PATCH v2 00/25] Virtio Sound card Implementation

[Gerd Hoffmann]

  Hi,

> IMHO, all your patches can be merged in only one.

For the most part yes.  I'd keep the pci wrapper (aka -device
virtio-snd-pci) separate though.  Possibly also patches adding
significant functionality in the future (i.e. one patch with all
basics and playback support, one patch adding recording
functionality, ...).

> Morever it would help for
> review as some patches remove code done in previous patches.

Yes, squashing the incremental fixes at the end of the series makes
sense.

take care,
  Gerd

Re: [RFC PATCH v2 00/25] Virtio Sound card Implementation

[Laurent Vivier]

Le 14/02/2022 à 11:44, Gerd Hoffmann a écrit :
>    Hi,
> 
>> IMHO, all your patches can be merged in only one.
> 
> For the most part yes.  I'd keep the pci wrapper (aka -device
> virtio-snd-pci) separate though.  Possibly also patches adding
> significant functionality in the future (i.e. one patch with all
> basics and playback support, one patch adding recording
> functionality, ...).
> 

I agree.

Laurent

Re: [RFC PATCH v2 00/25] Virtio Sound card Implementation

[Stefano Garzarella]

Hi Shreyansh,

On Fri, Feb 11, 2022 at 11:18 PM Shreyansh Chouhan
<chouhan.shreyansh2702@gmail.com> wrote:
>
> The second RFC for implementing the VirtIO Sound card as described in
> the virtio specs. Sorry for the absence of activity on this.

Thanks for starting working on virtio-sound device for QEMU!
I'm interested in completing this work, but first I wanted to know if
you are still working on it or have any new updates.

Thanks,
Stefano

>
> The output from the sound card works.
>
> What remains to be done:
> - Features defined in PCM features. (Eg message polling)
> - Channel maps
> - Jack remaps
> - Input
>
> I will work on the input after I have implemented the output
> along with all the features since at that point it should just be a
> matter of reversing a few things in the code that writes the audio.
>
> I can work on this patchset mostly on weekends now but I will try to be
> more regular with this.
>
> Reviews are welcome :)
>
> Shreyansh Chouhan (25):
>   virtio-snd: Add virtio sound header file
>   virtio-snd: Add jack control structures
>   virtio-snd: Add PCM control structures
>   virtio-snd: Add chmap control structures
>   virtio-snd: Add device implementation structures
>   virtio-snd: Add PCI wrapper code for VirtIOSound
>   virtio-snd: Add properties for class init
>   virtio-snd: Add code for get config function
>   virtio-snd: Add code for the realize function
>   virtio-snd: Add macros for logging
>   virtio-snd: Add control virtqueue handler
>   virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler
>   virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler
>   virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler
>   virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle
>   virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler
>   virtio-snd: Add default configs to realize fn
>   virtio-snd: Add callback for SWVoiceOut
>   virtio-snd: Add start/stop handler
>   virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler
>   virtio-snd: Replaced goto with if else
>   virtio-snd: Add code to device unrealize function
>   virtio-snd: Add xfer handler
>   virtio-snd: Add event vq and a handler stub
>   virtio-snd: Replaced AUD_log with tracepoints
>
>  hw/audio/Kconfig               |    5 +
>  hw/audio/meson.build           |    1 +
>  hw/audio/trace-events          |   14 +
>  hw/audio/virtio-snd.c          | 1241 ++++++++++++++++++++++++++++++++
>  hw/virtio/meson.build          |    1 +
>  hw/virtio/virtio-snd-pci.c     |   72 ++
>  include/hw/virtio/virtio-snd.h |  383 ++++++++++
>  7 files changed, 1717 insertions(+)
>  create mode 100644 hw/audio/virtio-snd.c
>  create mode 100644 hw/virtio/virtio-snd-pci.c
>  create mode 100644 include/hw/virtio/virtio-snd.h
>
> --
> 2.31.1
>
>

[RFC PATCH 01/25] virtio-snd: Add virtio sound header file

[Shreyansh Chouhan]

Added device configuration and common definitions to the header
file.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 include/hw/virtio/virtio-snd.h | 97 ++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 include/hw/virtio/virtio-snd.h

diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
new file mode 100644
index 0000000000..bbbf174c51
--- /dev/null
+++ b/include/hw/virtio/virtio-snd.h
@@ -0,0 +1,97 @@
+/*
+ * Virtio Sound Device
+ */
+
+#ifndef QEMU_VIRTIO_SOUND_H
+#define QEMU_VIRTIO_SOUND_H
+
+#include "qemu/units.h"
+#include "hw/virtio/virtio.h"
+#include "qemu/queue.h"
+#include "audio/audio.h"
+#include "audio/audio_int.h"
+
+#define VIRTIO_ID_SOUND 25
+
+/* CONFIGURATION SPACE */
+
+typedef struct virtio_snd_config {
+    /* # of jacks available */
+    uint32_t jacks;
+    /* # of streams avalable */
+    uint32_t streams;
+    /* # chmaps available */
+    uint32_t chmaps;
+} virtio_snd_config;
+
+/* COMMON DEFINITIONS */
+
+/* supported sample data directions. */
+enum {
+    VIRTIO_SND_D_OUTPUT = 0,
+    VIRTIO_SND_D_INPUT
+};
+
+enum {
+    /* jack control request types */
+    VIRTIO_SND_R_JACK_INFO = 1,
+    VIRTIO_SND_R_JACK_REMAP,
+
+    /* PCM control request types */
+    VIRTIO_SND_R_PCM_INFO = 0x0100,
+    VIRTIO_SND_R_PCM_SET_PARAMS,
+    VIRTIO_SND_R_PCM_PREPARE,
+    VIRTIO_SND_R_PCM_RELEASE,
+    VIRTIO_SND_R_PCM_START,
+    VIRTIO_SND_R_PCM_STOP,
+
+    /* channel map control request type */
+    VIRTIO_SND_R_CHMAP_INFO = 0x200,
+
+    /* jack event types */
+    VIRTIO_SND_EVT_JACK_CONNECTED = 0x1000,
+    VIRTIO_SND_EVT_JACK_DISCONNECTED,
+
+    /* PCM event types */
+    VIRTIO_SND_EVT_PCM_PERIOD_ELAPSED = 0x1100,
+    VIRTIO_SND_EVT_PCM_XRUN,
+
+    /* common status codes */
+    VIRTIO_SND_S_OK = 0x8000,
+    VIRTIO_SND_S_BAD_MSG,
+    VIRTIO_SND_S_NOT_SUPP,
+    VIRTIO_SND_S_IO_ERR
+};
+
+/* common header for request/response*/
+typedef struct virtio_snd_hdr {
+    uint32_t code;
+} virtio_snd_hdr;
+
+/* event notification */
+typedef struct virtio_snd_event {
+    /* VIRTIO_SND_EVT_* */
+    virtio_snd_hdr hdr;
+    /* Optional event data */
+    uint32_t data;
+} virtio_snd_event;
+
+/* common control request to query an item information */
+typedef struct virtio_snd_query_info {
+    /* VIRTIO_SND_R_*_INFO */
+    struct virtio_snd_hdr hdr;
+    /* item start identifier */
+    uint32_t start_id;
+    /* # of items to query */
+    uint32_t count;
+    /* size of a single item information in bytes */
+    uint32_t size;
+} virtio_snd_query_info;
+
+/* common item information header */
+typedef struct virtio_snd_info {
+    /* functional group node id (HDA Spec 7.1.2) */
+    uint32_t hda_fn_nid;
+} virtio_snd_info;
+
+#endif
-- 
2.31.1

Re: [RFC PATCH 01/25] virtio-snd: Add virtio sound header file

[Gerd Hoffmann]

On Sat, Feb 12, 2022 at 03:42:55AM +0530, Shreyansh Chouhan wrote:
> Added device configuration and common definitions to the header
> file.
> 
> Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
> ---
>  include/hw/virtio/virtio-snd.h | 97 ++++++++++++++++++++++++++++++++++
>  1 file changed, 97 insertions(+)
>  create mode 100644 include/hw/virtio/virtio-snd.h
> 
> diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
> new file mode 100644
> index 0000000000..bbbf174c51
> --- /dev/null
> +++ b/include/hw/virtio/virtio-snd.h

We already have include/standard-headers/linux/virtio_snd.h (synced from
linux kernel), you can (and should) just use that.

take care,
  Gerd

[RFC PATCH 02/25] virtio-snd: Add jack control structures

[Shreyansh Chouhan]

Added structures for handling jack control requests
to the header file.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 include/hw/virtio/virtio-snd.h | 40 ++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
index bbbf174c51..f58b06a949 100644
--- a/include/hw/virtio/virtio-snd.h
+++ b/include/hw/virtio/virtio-snd.h
@@ -94,4 +94,44 @@ typedef struct virtio_snd_info {
     uint32_t hda_fn_nid;
 } virtio_snd_info;
 
+/* JACK CONTROL MESSAGES */
+
+typedef struct virtio_snd_jack_hdr {
+    /* VIRTIO_SND_R_JACK_* */
+    virtio_snd_hdr hdr;
+    /* 0 to (virtio_snd_config.jacks - 1) */
+    uint32_t jack_id;
+} virtio_snd_jack_hdr;
+
+/* Supported jack features */
+enum {
+    VIRTIO_SND_F_JACK_REMAP = 0
+};
+
+/* jack information structure */
+typedef struct virtio_snd_jack_info {
+    /* common header */
+    virtio_snd_info hdr;
+    /* 1 << VIRTIO_SND_JACK_F_* */
+    uint32_t features;
+    /* pin default configuration from HDA spec */
+    uint32_t hda_reg_defconf;
+    /* pin capabilities from HDA spec */
+    uint32_t hda_reg_caps;
+    /* connection status (0: disconnected, 1: connected) */
+    uint8_t connected;
+
+    uint8_t padding[7];
+} virtio_snd_jack_info;
+
+/* jack remapping control request */
+typedef struct virtio_snd_jack_remap {
+    /* .code = VIRTIO_SND_R_JACK_REMAP */
+    virtio_snd_jack_hdr hdr;
+    /* selected association number */
+    uint32_t association;
+    /* selected sequence number */
+    uint32_t sequence;
+} virtio_snd_jack_remap;
+
 #endif
-- 
2.31.1

[RFC PATCH 03/25] virtio-snd: Add PCM control structures

[Shreyansh Chouhan]

Added structures for handle PCM control requests
to the heaer file.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 include/hw/virtio/virtio-snd.h | 123 +++++++++++++++++++++++++++++++++
 1 file changed, 123 insertions(+)

diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
index f58b06a949..a7828c4ab8 100644
--- a/include/hw/virtio/virtio-snd.h
+++ b/include/hw/virtio/virtio-snd.h
@@ -134,4 +134,127 @@ typedef struct virtio_snd_jack_remap {
     uint32_t sequence;
 } virtio_snd_jack_remap;
 
+/* PCM CONTROL MESSAGES */
+
+typedef struct virtio_snd_pcm_hdr {
+    /* .code = VIRTIO_SND_R_PCM_* */
+    virtio_snd_hdr hdr;
+    /* 0 to (virtio_snd_config.streams - 1) */
+    uint32_t stream_id;
+} virtio_snd_pcm_hdr;
+
+
+/* Supported PCM stream features */
+enum {
+    VIRTIO_SND_PCM_F_SHMEM_HOST = 0,
+    VIRTIO_SND_PCM_F_SHMEM_GUEST,
+    VIRTIO_SND_PCM_F_MSG_POLLING,
+    VIRTIO_SND_PCM_F_EVT_SHMEM_PERIODS,
+    VIRTIO_SNDPCM_F_EVT_XRUNS
+};
+
+/* Supported sample formats */
+enum {
+    /* analog formats (width / physical width) */
+    VIRTIO_SND_PCM_FMT_IMA_ADPCM = 0,   /*  4 /  4 bits */
+    VIRTIO_SND_PCM_FMT_MU_LAW,          /*  8 /  8 bits */
+    VIRTIO_SND_PCM_FMT_A_LAW,           /*  8 /  8 bits */
+    VIRTIO_SND_PCM_FMT_S8,              /*  8 /  8 bits */
+    VIRTIO_SND_PCM_FMT_U8,              /*  8 /  8 bits */
+    VIRTIO_SND_PCM_FMT_S16,             /* 16 / 16 bits */
+    VIRTIO_SND_PCM_FMT_U16,             /* 16 / 16 bits */
+    VIRTIO_SND_PCM_FMT_S18_3,           /* 18 / 24 bits */
+    VIRTIO_SND_PCM_FMT_U18_3,           /* 18 / 24 bits */
+    VIRTIO_SND_PCM_FMT_S20_3,           /* 20 / 24 bits */
+    VIRTIO_SND_PCM_FMT_U20_3,           /* 20 / 24 bits */
+    VIRTIO_SND_PCM_FMT_S24_3,           /* 24 / 24 bits */
+    VIRTIO_SND_PCM_FMT_U24_3,           /* 24 / 24 bits */
+    VIRTIO_SND_PCM_FMT_S20,             /* 20 / 32 bits */
+    VIRTIO_SND_PCM_FMT_U20,             /* 20 / 32 bits */
+    VIRTIO_SND_PCM_FMT_S24,             /* 24 / 32 bits */
+    VIRTIO_SND_PCM_FMT_U24,             /* 24 / 32 bits */
+    VIRTIO_SND_PCM_FMT_S32,             /* 32 / 32 bits */
+    VIRTIO_SND_PCM_FMT_U32,             /* 32 / 32 bits */
+    VIRTIO_SND_PCM_FMT_FLOAT,           /* 32 / 32 bits */
+    VIRTIO_SND_PCM_FMT_FLOAT64,         /* 64 / 64 bits */
+    /* digital formats (width / physical width) */
+    VIRTIO_SND_PCM_FMT_DSD_U8,          /*  8 /  8 bits */
+    VIRTIO_SND_PCM_FMT_DSD_U16,         /* 16 / 16 bits */
+    VIRTIO_SND_PCM_FMT_DSD_U32,         /* 32 / 32 bits */
+    VIRTIO_SND_PCM_FMT_IEC958_SUBFRAME  /* 32 / 32 bits */
+};
+
+/* Supported PCM frame rates */
+enum {
+    VIRTIO_SND_PCM_RATE_5512 = 0,
+    VIRTIO_SND_PCM_RATE_8000,
+    VIRTIO_SND_PCM_RATE_11025,
+    VIRTIO_SND_PCM_RATE_16000,
+    VIRTIO_SND_PCM_RATE_22050,
+    VIRTIO_SND_PCM_RATE_32000,
+    VIRTIO_SND_PCM_RATE_44100,
+    VIRTIO_SND_PCM_RATE_48000,
+    VIRTIO_SND_PCM_RATE_64000,
+    VIRTIO_SND_PCM_RATE_88200,
+    VIRTIO_SND_PCM_RATE_96000,
+    VIRTIO_SND_PCM_RATE_176399,
+    VIRTIO_SND_PCM_RATE_192000,
+    VIRTIO_SND_PCM_RATE_384000
+};
+
+/* PCM stream info structure */
+typedef struct virtio_snd_pcm_info {
+    /* common header */
+    virtio_snd_info hdr;
+    /* supported features bitmap (1 << VIRTIO_SND_PCM_F_*) */
+    uint32_t features;
+    /* supported sample formats bitmap (1 << VIRTIO_SND_PCM_FMT_*) */
+    uint64_t formats;
+    /* supported sample rates bitmap (1 << VIRTIO_SND_PCM_RATE_*) */
+    uint64_t rates;
+    /* direction of the stream (VIRTIO_SND_D_*) */
+    uint8_t direction;
+    /* min # of supported channels */
+    uint8_t channels_min;
+    /* max # of supported channels */
+    uint8_t channels_max;
+
+    uint8_t padding[5];
+} virtio_snd_pcm_info;
+
+/* set PCM stream params */
+typedef struct virtio_snd_pcm_set_params {
+    virtio_snd_pcm_hdr hdr;
+    /* size of hardware buffer in bytes */
+    uint32_t buffer_bytes;
+    /* size of hardware period in bytes */
+    uint32_t period_bytes;
+    /* selected feature bitmap */
+    uint32_t features;
+    /* number of channels */
+    uint8_t channel;
+    /* VIRTIO_SND_PCM_FMT_* */
+    uint8_t format;
+    /* VIRTIO_SND_PCM_RATE_* */
+    uint8_t rate;
+
+    uint8_t padding;
+} virtio_snd_pcm_set_params;
+
+/* PCM I/O MESSAGES */
+
+/* I/O request header */
+typedef struct virtio_snd_pcm_xfer {
+    /* 0 to (virtio_snd_config.stream - 1 */
+    uint32_t stream_id;
+} virtio_snd_pcm_xfer;
+
+/* I/O request status */
+typedef struct virtio_snd_pcm_status {
+    /* VIRTIO_SND_S_* */
+    uint32_t status;
+    /* current device latency */
+    uint32_t latency_bytes;
+} virtio_snd_pcm_status;
+
 #endif
-- 
2.31.1

[RFC PATCH 04/25] virtio-snd: Add chmap control structures

[Shreyansh Chouhan]

Added structures for handling channel map control
requests to the header file.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 include/hw/virtio/virtio-snd.h | 57 ++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
index a7828c4ab8..3c16609a25 100644
--- a/include/hw/virtio/virtio-snd.h
+++ b/include/hw/virtio/virtio-snd.h
@@ -257,4 +257,61 @@ typedef struct virtio_snd_pcm_status {
     uint32_t latency_bytes;
 } virtio_snd_pcm_status;
 
+/* CHANNEL MAP CONTROL MESSAGES */
+
+/* standard channel position definition */
+enum {
+    VIRTIO_SND_CHMAP_NONE = 0,  /* undefined */
+    VIRTIO_SND_CHMAP_NA,        /* silent */
+    VIRTIO_SND_CHMAP_MONO,      /* mono stream */
+    VIRTIO_SND_CHMAP_FL,        /* front left */
+    VIRTIO_SND_CHMAP_FR,        /* front right */
+    VIRTIO_SND_CHMAP_RL,        /* rear left */
+    VIRTIO_SND_CHMAP_RR,        /* rear right */
+    VIRTIO_SND_CHMAP_FC,        /* front center */
+    VIRTIO_SND_CHMAP_LFE,       /* low frequency (LFE) */
+    VIRTIO_SND_CHMAP_SL,        /* side left */
+    VIRTIO_SND_CHMAP_SR,        /* side right */
+    VIRTIO_SND_CHMAP_RC,        /* rear center */
+    VIRTIO_SND_CHMAP_FLC,       /* front left center */
+    VIRTIO_SND_CHMAP_FRC,       /* front right center */
+    VIRTIO_SND_CHMAP_RLC,       /* rear left center */
+    VIRTIO_SND_CHMAP_RRC,       /* rear right center */
+    VIRTIO_SND_CHMAP_FLW,       /* front left wide */
+    VIRTIO_SND_CHMAP_FRW,       /* front right wide */
+    VIRTIO_SND_CHMAP_FLH,       /* front left high */
+    VIRTIO_SND_CHMAP_FCH,       /* front center high */
+    VIRTIO_SND_CHMAP_FRH,       /* front right high */
+    VIRTIO_SND_CHMAP_TC,        /* top center */
+    VIRTIO_SND_CHMAP_TFL,       /* top front left */
+    VIRTIO_SND_CHMAP_TFR,       /* top front right */
+    VIRTIO_SND_CHMAP_TFC,       /* top front center */
+    VIRTIO_SND_CHMAP_TRL,       /* top rear left */
+    VIRTIO_SND_CHMAP_TRR,       /* top rear right */
+    VIRTIO_SND_CHMAP_TRC,       /* top rear center */
+    VIRTIO_SND_CHMAP_TFLC,      /* top front left center */
+    VIRTIO_SND_CHMAP_TFRC,      /* top front right center */
+    VIRTIO_SND_CHMAP_TSL,       /* top side left */
+    VIRTIO_SND_CHMAP_TSR,       /* top side right */
+    VIRTIO_SND_CHMAP_LLFE,      /* left LFE */
+    VIRTIO_SND_CHMAP_RLFE,      /* right LFE */
+    VIRTIO_SND_CHMAP_BC,        /* bottom center */
+    VIRTIO_SND_CHMAP_BLC,       /* bottom left center */
+    VIRTIO_SND_CHMAP_BRC        /* bottom right center */
+};
+
+/* maximum possible number of channels */
+#define VIRTIO_SND_CHMAP_MAX_SIZE   18
+
+typedef struct virtio_snd_chmap_info {
+    /* common header */
+    virtio_snd_info hdr;
+    /* direction */
+    uint8_t direction;
+    /* # of valid channel position values */
+    uint8_t channels;
+    /* channel position values (VIRTIO_SND_CHMAP_*) */
+    uint8_t positions[VIRTIO_SND_CHMAP_MAX_SIZE];
+} virtio_snd_chmap_info;
+
 #endif
-- 
2.31.1

[RFC PATCH 05/25] virtio-snd: Add device implementation structures

[Shreyansh Chouhan]

Added jacks, pcm streams and the VirtIOSound structure for actual
device implementation.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 include/hw/virtio/virtio-snd.h | 66 ++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/include/hw/virtio/virtio-snd.h b/include/hw/virtio/virtio-snd.h
index 3c16609a25..4d800a9626 100644
--- a/include/hw/virtio/virtio-snd.h
+++ b/include/hw/virtio/virtio-snd.h
@@ -13,6 +13,9 @@
 
 #define VIRTIO_ID_SOUND 25
 
+#define TYPE_VIRTIO_SOUND "virtio-sound-device"
+OBJECT_DECLARE_SIMPLE_TYPE(VirtIOSound, VIRTIO_SOUND)
+
 /* CONFIGURATION SPACE */
 
 typedef struct virtio_snd_config {
@@ -314,4 +317,67 @@ typedef struct virtio_snd_chmap_info {
     uint8_t positions[VIRTIO_SND_CHMAP_MAX_SIZE];
 } virtio_snd_chmap_info;
 
+/* VIRTIO SOUND DEVICE */
+
+/* Jacks */
+typedef struct virtio_snd_jack {
+    uint32_t features; /* 1 << VIRTIO_SND_JACK_F_XXX */
+    uint32_t hda_fn_nid;
+    uint32_t hda_reg_defconf;
+    uint32_t hda_reg_caps;
+    uint8_t connected;
+} virtio_snd_jack;
+
+/* Streams */
+typedef struct virtio_snd_pcm_stream {
+    uint32_t hda_fn_nid;
+    uint32_t buffer_bytes;
+    uint32_t period_bytes;
+    uint32_t features; /* 1 << VIRTIO_SND_PCM_F_XXX */
+    uint32_t flags; /* 1 << VIRTIO_SND_PCM_FL_XXX */
+    uint32_t direction;
+    uint8_t channels_min;
+    uint8_t channels_max;
+    uint64_t formats; /* 1 << VIRTIO_SND_PCM_FMT_XXX */
+    uint64_t rates; /* 1 << VIRTIO_SND_PCM_RATE_XXX */
+    uint32_t r_pos, w_pos;
+    bool flushing;
+    uint8_t chmap[VIRTIO_SND_CHMAP_MAX_SIZE];
+    VirtQueueElement **elems;
+    VirtIOSound *s;
+    union {
+        SWVoiceIn *in;
+        SWVoiceOut *out;
+    } voice;
+} virtio_snd_pcm_stream;
+
+/* Stream params */
+typedef struct virtio_snd_pcm_params {
+    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 channel;
+    uint8_t format;
+    uint8_t rate;
+} virtio_snd_pcm_params;
+
+/* Sound device */
+struct VirtIOSound {
+    /* Parent VirtIODevice object */
+    VirtIODevice parent_obj;
+    virtio_snd_config snd_conf;
+
+    VirtQueue *ctrl_vq;
+    VirtQueue *event_vq;
+    VirtQueue *tx_vq;
+    VirtQueue *rx_vq;
+
+    QEMUSoundCard card;
+    size_t config_size;
+
+    virtio_snd_pcm_params **pcm_params;
+    virtio_snd_pcm_stream **streams;
+    virtio_snd_jack **jacks;
+};
+
 #endif
-- 
2.31.1

[RFC PATCH 06/25] virtio-snd: Add PCI wrapper code for VirtIOSound

[Shreyansh Chouhan]

Added the virito-snd.c file which contains a wrapper
for combining the device with the VirtIOPCIProxy.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/virtio/meson.build      |  1 +
 hw/virtio/virtio-snd-pci.c | 72 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)
 create mode 100644 hw/virtio/virtio-snd-pci.c

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 521f7d64a8..86d6594c20 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -49,6 +49,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_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
 
diff --git a/hw/virtio/virtio-snd-pci.c b/hw/virtio/virtio-snd-pci.c
new file mode 100644
index 0000000000..8d8e4ffa51
--- /dev/null
+++ b/hw/virtio/virtio-snd-pci.c
@@ -0,0 +1,72 @@
+/*
+ * Virtio sound PCI Bindings
+ */
+
+#include "qemu/osdep.h"
+#include "hw/virtio/virtio-snd.h"
+#include "hw/virtio/virtio-pci.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+#include "qom/object.h"
+
+typedef struct VirtIOSoundPCI VirtIOSoundPCI;
+
+/*
+ * virtio-snd-pci: This extends VirtioPCIProxy.
+ */
+#define TYPE_VIRTIO_SOUND_PCI "virtio-sound-pci-base"
+DECLARE_INSTANCE_CHECKER(VirtIOSoundPCI, VIRTIO_SOUND_PCI,
+                         TYPE_VIRTIO_SOUND_PCI)
+
+struct VirtIOSoundPCI {
+    VirtIOPCIProxy parent_obj;
+    VirtIOSound vdev;
+};
+
+static Property virtio_sound_properties[] = {
+    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_snd_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+{
+    VirtIOSoundPCI *dev = VIRTIO_SOUND_PCI(vpci_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
+
+    qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
+}
+
+static void virtio_snd_pci_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    VirtioPCIClass *vpciklass = VIRTIO_PCI_CLASS(klass);
+
+    k->class_id = PCI_CLASS_MULTIMEDIA_AUDIO;
+    set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
+    device_class_set_props(dc, virtio_sound_properties);
+    vpciklass->realize = virtio_snd_pci_realize;
+}
+
+static void virtio_snd_pci_instance_init(Object *obj)
+{
+    VirtIOSoundPCI *dev = VIRTIO_SOUND_PCI(obj);
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_SOUND);
+}
+
+static const VirtioPCIDeviceTypeInfo virtio_snd_pci_info = {
+    .base_name              = TYPE_VIRTIO_SOUND_PCI,
+    .generic_name           = "virtio-snd-pci",
+    .instance_size = sizeof(VirtIOSoundPCI),
+    .instance_init = virtio_snd_pci_instance_init,
+    .class_init    = virtio_snd_pci_class_init,
+};
+
+static void virtio_snd_pci_register(void)
+{
+    virtio_pci_types_register(&virtio_snd_pci_info);
+}
+
+type_init(virtio_snd_pci_register);
-- 
2.31.1

[RFC PATCH 07/25] virtio-snd: Add properties for class init

[Shreyansh Chouhan]

Added properties and function stubs for virtio sound device class
init.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/Kconfig      |   5 ++
 hw/audio/meson.build  |   1 +
 hw/audio/virtio-snd.c | 126 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 132 insertions(+)
 create mode 100644 hw/audio/virtio-snd.c

diff --git a/hw/audio/Kconfig b/hw/audio/Kconfig
index e9c6fed826..d12df06699 100644
--- a/hw/audio/Kconfig
+++ b/hw/audio/Kconfig
@@ -50,3 +50,8 @@ config CS4231
 
 config MARVELL_88W8618
     bool
+
+config VIRTIO_SND
+    bool
+    default y
+    depends on VIRTIO
diff --git a/hw/audio/meson.build b/hw/audio/meson.build
index e48a9fc73d..455e6a1501 100644
--- a/hw/audio/meson.build
+++ b/hw/audio/meson.build
@@ -12,3 +12,4 @@ softmmu_ss.add(when: 'CONFIG_PL041', if_true: files('pl041.c', 'lm4549.c'))
 softmmu_ss.add(when: 'CONFIG_SB16', if_true: files('sb16.c'))
 softmmu_ss.add(when: 'CONFIG_VT82C686', if_true: files('via-ac97.c'))
 softmmu_ss.add(when: 'CONFIG_WM8750', if_true: files('wm8750.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO_SND', if_true: files('virtio-snd.c'))
diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
new file mode 100644
index 0000000000..ae438aa7ec
--- /dev/null
+++ b/hw/audio/virtio-snd.c
@@ -0,0 +1,126 @@
+/*
+ * Virtio Sound device
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/atomic.h"
+#include "qemu/iov.h"
+#include "qemu/main-loop.h"
+#include "qemu/module.h"
+#include "hw/virtio/virtio.h"
+#include "audio/audio.h"
+#include "qemu/error-report.h"
+#include "qemu/timer.h"
+#include "qemu/option.h"
+#include "qemu/option_int.h"
+#include "qemu/config-file.h"
+#include "qapi/qmp/qdict.h"
+#include "hw/virtio/virtio-snd.h"
+#include "hw/virtio/virtio-bus.h"
+#include "qapi/error.h"
+#include "qapi/qapi-events-audio.h"
+#include "hw/qdev-properties.h"
+#include "qapi/qapi-types-migration.h"
+#include "qapi/qapi-events-migration.h"
+#include "migration/misc.h"
+#include "standard-headers/linux/ethtool.h"
+#include "sysemu/sysemu.h"
+#include "trace.h"
+#include "monitor/qdev.h"
+#include "hw/pci/pci.h"
+#include "intel-hda-defs.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_OUT 0
+#define VIRTIO_SOUND_HDA_FN_NID_IN 1
+
+static void virtio_snd_get_config(VirtIODevice *vdev, uint8_t *config)
+{
+}
+
+static void virtio_snd_set_config(VirtIODevice *vdev, const uint8_t *config)
+{
+}
+
+static const VMStateDescription vmstate_virtio_snd_device = {
+    .name = "virtio-snd-device",
+    .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_AUDIO_PROPERTIES(VirtIOSound, card),
+    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 uint64_t virtio_snd_get_features(VirtIODevice *vdev, uint64_t features,
+                                        Error **errp)
+{
+    return vdev->host_features;
+}
+
+static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
+{
+}
+
+static void virtio_snd_device_unrealize(DeviceState *dev)
+{
+}
+
+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);
+
+    device_class_set_props(dc, virtio_snd_properties);
+    dc->vmsd = &vmstate_virtio_snd;
+    set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
+    vdc->realize = virtio_snd_device_realize;
+    vdc->unrealize = virtio_snd_device_unrealize;
+    vdc->get_config = virtio_snd_get_config;
+    vdc->set_config = virtio_snd_set_config;
+    vdc->get_features = virtio_snd_get_features;
+    vdc->reset = virtio_snd_reset;
+    vdc->legacy_features = 0;
+    vdc->vmsd = &vmstate_virtio_snd_device;
+}
+
+
+static const TypeInfo virtio_snd_dev_info = {
+    .name = TYPE_VIRTIO_SOUND,
+    .parent = TYPE_VIRTIO_DEVICE,
+    .instance_size = sizeof(VirtIOSound),
+    .class_init = virtio_snd_class_init,
+};
+
+static void virtio_register_types(void)
+{
+    type_register_static(&virtio_snd_dev_info);
+}
+
+type_init(virtio_register_types)
-- 
2.31.1

[RFC PATCH 08/25] virtio-snd: Add code for get config function

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index ae438aa7ec..afa38adee7 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -41,6 +41,15 @@
 
 static void virtio_snd_get_config(VirtIODevice *vdev, uint8_t *config)
 {
+    VirtIOSound *s = VIRTIO_SOUND(vdev);
+    virtio_snd_config sndcfg;
+
+    memset(&sndcfg, 0, sizeof(virtio_snd_config));
+    stl_le_p(&(sndcfg.jacks), s->snd_conf.jacks);
+    stl_le_p(&(sndcfg.streams), s->snd_conf.streams);
+    stl_le_p(&(sndcfg.chmaps), s->snd_conf.chmaps);
+
+    memcpy(config, &sndcfg, sizeof(virtio_snd_config));
 }
 
 static void virtio_snd_set_config(VirtIODevice *vdev, const uint8_t *config)
-- 
2.31.1

[RFC PATCH 09/25] virtio-snd: Add code for the realize function

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index afa38adee7..b51f6c7523 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -89,8 +89,43 @@ static uint64_t virtio_snd_get_features(VirtIODevice *vdev, uint64_t features,
     return vdev->host_features;
 }
 
+/*
+ * Initializes the VirtIOSound card device. Validates the configuration
+ * passed by the command line. Initializes the virtqueues. Allocates resources
+ * for and initializes streams, jacks and chmaps.
+ *
+ * @dev: VirtIOSound card device
+ * @errp: Set if there is an error
+ */
 static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
 {
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOSound *s = VIRTIO_SOUND(dev);
+
+    virtio_init(vdev, "virtio-snd", VIRTIO_ID_SOUND, sizeof(virtio_snd_config));
+
+    /* set number of jacks and streams */
+    if (s->snd_conf.jacks > 8) {
+        error_setg(errp, "Invalid number of jacks: %d", s->snd_conf.jacks);
+        return;
+    }
+    if (s->snd_conf.streams < 1 || s->snd_conf.streams > 10) {
+        error_setg(errp, "Invalid number of streams: %d", s->snd_conf.streams);
+        return;
+    }
+
+    if (s->snd_conf.chmaps > VIRTIO_SND_CHMAP_MAX_SIZE) {
+        error_setg(errp, "Invalid number of channel maps: %d",
+                   s->snd_conf.chmaps);
+        return;
+    }
+
+    /* set up QEMUSoundCard and audiodev */
+    AUD_register_card ("virtio_snd_card", &s->card);
+
+    s->streams = g_new0(virtio_snd_pcm_stream *, s->snd_conf.streams);
+    s->pcm_params = g_new0(virtio_snd_pcm_params *, s->snd_conf.streams);
+    s->jacks = g_new0(virtio_snd_jack *, s->snd_conf.jacks);
 }
 
 static void virtio_snd_device_unrealize(DeviceState *dev)
-- 
2.31.1

[RFC PATCH 10/25] virtio-snd: Add macros for logging

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index b51f6c7523..40829fa329 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -39,6 +39,10 @@
 #define VIRTIO_SOUND_HDA_FN_NID_OUT 0
 #define VIRTIO_SOUND_HDA_FN_NID_IN 1
 
+#define virtio_snd_log(...) AUD_log("virtio sound info", __VA_ARGS__)
+#define virtio_snd_warn(...) AUD_log("virtio sound warn", __VA_ARGS__)
+#define virtio_snd_err(...) AUD_log("virtio sound err", __VA_ARGS__)
+
 static void virtio_snd_get_config(VirtIODevice *vdev, uint8_t *config)
 {
     VirtIOSound *s = VIRTIO_SOUND(vdev);
@@ -167,4 +171,8 @@ static void virtio_register_types(void)
     type_register_static(&virtio_snd_dev_info);
 }
 
+#undef virtio_snd_log
+#undef virtio_snd_warn
+#undef virtio_snd_err
+
 type_init(virtio_register_types)
-- 
2.31.1

[RFC PATCH 11/25] virtio-snd: Add control virtqueue handler

[Shreyansh Chouhan]

The handler prints the requests that came in the ctrl virtqueue.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 73 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 40829fa329..a87922f91b 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -93,6 +93,77 @@ static uint64_t virtio_snd_get_features(VirtIODevice *vdev, uint64_t features,
     return vdev->host_features;
 }
 
+/* The control queue handler. Pops an element from the control virtqueue,
+ * checks the header and performs the requested action. Finally marks the
+ * element as used.
+ *
+ * @vdev: VirtIOSound card device
+ * @vq: Control virtqueue
+ */
+static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+{
+    virtio_snd_hdr ctrl;
+
+    VirtQueueElement *elem = NULL;
+    size_t sz;
+    struct iovec *iov, *iov2;
+    unsigned int iov_cnt;
+
+    for (;;) {
+        elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+        if (!elem) {
+            break;
+        }
+        if (iov_size(elem->in_sg, elem->in_num) < sizeof(ctrl) ||
+                iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
+            virtio_snd_err("virtio-snd ctrl missing headers\n");
+            virtqueue_detach_element(vq, elem, 0);
+            g_free(elem);
+            break;
+        }
+
+        iov_cnt = elem->out_num;
+        iov2 = iov = g_memdup(elem->out_sg,
+                              sizeof(struct iovec) * elem->out_num);
+        sz = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl));
+        iov_discard_front(&iov, &iov_cnt, sizeof(ctrl));
+        if (sz != sizeof(ctrl)) {
+            /* error */
+            virtio_snd_err("virtio snd ctrl could not read header\n");
+        } else if (ctrl.code == VIRTIO_SND_R_JACK_INFO) {
+            virtio_snd_log("VIRTIO_SND_R_JACK_INFO");
+        } else if (ctrl.code == VIRTIO_SND_R_JACK_REMAP) {
+            virtio_snd_log("VIRTIO_SND_R_JACK_REMAP");
+        } else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
+            virtio_snd_log("VIRTIO_SND_R_PCM_INFO");
+        } else if (ctrl.code == VIRTIO_SND_R_PCM_SET_PARAMS) {
+            virtio_snd_log("VIRTIO_SND_R_PCM_SET_PARAMS");
+        } else if (ctrl.code == VIRTIO_SND_R_PCM_PREPARE) {
+            virtio_snd_log("VIRTIO_SND_R_PCM_PREPARE");
+        } else if (ctrl.code == VIRTIO_SND_R_PCM_START) {
+            virtio_snd_log("VIRTIO_SND_R_PCM_START");
+        } else if (ctrl.code == VIRTIO_SND_R_PCM_STOP) {
+            virtio_snd_log("VIRTIO_SND_R_PCM_STOP");
+        } else if (ctrl.code == VIRTIO_SND_R_PCM_RELEASE) {
+            virtio_snd_log("VIRTIO_SND_R_PCM_RELEASE");
+        } else if (ctrl.code == VIRTIO_SND_R_CHMAP_INFO) {
+            virtio_snd_log("VIRTIO_SND_R_CHMAP_INFO");
+        } else {
+            /* error */
+            virtio_snd_err("virtio snd header not recognized: %d\n", ctrl.code);
+        }
+
+        virtio_snd_hdr resp;
+        resp.code = VIRTIO_SND_S_OK;
+        sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+        virtqueue_push(vq, elem, sz);
+
+        virtio_notify(vdev, vq);
+        g_free(iov2);
+        g_free(elem);
+    }
+}
+
 /*
  * Initializes the VirtIOSound card device. Validates the configuration
  * passed by the command line. Initializes the virtqueues. Allocates resources
@@ -127,6 +198,8 @@ static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
     /* set up QEMUSoundCard and audiodev */
     AUD_register_card ("virtio_snd_card", &s->card);
 
+    s->ctrl_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_ctrl);
+
     s->streams = g_new0(virtio_snd_pcm_stream *, s->snd_conf.streams);
     s->pcm_params = g_new0(virtio_snd_pcm_params *, s->snd_conf.streams);
     s->jacks = g_new0(virtio_snd_jack *, s->snd_conf.jacks);
-- 
2.31.1

[RFC PATCH 12/25] virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 81 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 79 insertions(+), 2 deletions(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index a87922f91b..c2af26f3cb 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -92,6 +92,80 @@ static uint64_t virtio_snd_get_features(VirtIODevice *vdev, uint64_t features,
 {
     return vdev->host_features;
 }
+/*
+ * Get a specific jack from the VirtIOSound card.
+ *
+ * @s: VirtIOSound card device.
+ * @id: Jack id
+ */
+static virtio_snd_jack *virtio_snd_get_jack(VirtIOSound *s, uint32_t id)
+{
+    if (id >= s->snd_conf.jacks) {
+        return NULL;
+    }
+    return s->jacks[id];
+}
+
+/*
+ * Handles VIRTIO_SND_R_JACK_INFO.
+ * The function writes the info structs and response to the virtqueue element.
+ * Returns the used size in bytes.
+ *
+ * @s: VirtIOSound card
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_jack_info(VirtIOSound *s,
+                                            VirtQueueElement *elem)
+{
+    virtio_snd_query_info req;
+    size_t sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+    assert(sz == sizeof(virtio_snd_query_info));
+
+    virtio_snd_hdr resp;
+
+    if (iov_size(elem->in_sg, elem->in_num) <
+        sizeof(virtio_snd_hdr) + req.count * req.size) {
+        virtio_snd_err("jack info: buffer too small got: %lu needed: %lu\n",
+                       iov_size(elem->in_sg, elem->in_num),
+                       sizeof(virtio_snd_hdr) + req.count * req.size);
+        resp.code = VIRTIO_SND_S_BAD_MSG;
+        goto done;
+    }
+
+    virtio_snd_jack_info *jack_info = g_new0(virtio_snd_jack_info, req.count);
+    for (int i = req.start_id; i < req.count + req.start_id; i++) {
+        virtio_snd_jack *jack = virtio_snd_get_jack(s, i);
+        if (!jack) {
+            virtio_snd_err("Invalid jack id: %d\n", i);
+            resp.code = VIRTIO_SND_S_BAD_MSG;
+            goto done;
+        }
+
+        jack_info[i - req.start_id].hdr.hda_fn_nid = jack->hda_fn_nid;
+        jack_info[i - req.start_id].features = jack->features;
+        jack_info[i - req.start_id].hda_reg_defconf = jack->hda_reg_defconf;
+        jack_info[i - req.start_id].hda_reg_caps = jack->hda_reg_caps;
+        jack_info[i - req.start_id].connected = jack->connected;
+        memset(jack_info[i - req.start_id].padding, 0,
+               sizeof(jack_info[i - req.start_id].padding));
+    }
+
+    resp.code = VIRTIO_SND_S_OK;
+done:
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+
+    if (resp.code == VIRTIO_SND_S_BAD_MSG) {
+        g_free(jack_info);
+        return sz;
+    }
+
+    sz = iov_from_buf(elem->in_sg, elem->in_num, sizeof(virtio_snd_hdr),
+                      jack_info, sizeof(virtio_snd_jack_info) * req.count);
+    assert(sz == req.count * req.size);
+    g_free(jack_info);
+    return sizeof(virtio_snd_hdr) + sz;
+}
 
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
@@ -102,6 +176,7 @@ static uint64_t virtio_snd_get_features(VirtIODevice *vdev, uint64_t features,
  */
 static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
+    VirtIOSound *s = VIRTIO_SOUND(vdev);
     virtio_snd_hdr ctrl;
 
     VirtQueueElement *elem = NULL;
@@ -131,7 +206,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             /* error */
             virtio_snd_err("virtio snd ctrl could not read header\n");
         } else if (ctrl.code == VIRTIO_SND_R_JACK_INFO) {
-            virtio_snd_log("VIRTIO_SND_R_JACK_INFO");
+            sz = virtio_snd_handle_jack_info(s, elem);
+            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_JACK_REMAP) {
             virtio_snd_log("VIRTIO_SND_R_JACK_REMAP");
         } else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
@@ -156,8 +232,9 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         virtio_snd_hdr resp;
         resp.code = VIRTIO_SND_S_OK;
         sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
-        virtqueue_push(vq, elem, sz);
 
+done:
+        virtqueue_push(vq, elem, sz);
         virtio_notify(vdev, vq);
         g_free(iov2);
         g_free(elem);
-- 
2.31.1

Re: [RFC PATCH 12/25] virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler

[Laurent Vivier]

Le 11/02/2022 à 23:13, Shreyansh Chouhan a écrit :
> Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
> ---
>   hw/audio/virtio-snd.c | 81 +++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 79 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index a87922f91b..c2af26f3cb 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -92,6 +92,80 @@ static uint64_t virtio_snd_get_features(VirtIODevice *vdev, uint64_t features,
>   {
>       return vdev->host_features;
>   }
> +/*
> + * Get a specific jack from the VirtIOSound card.
> + *
> + * @s: VirtIOSound card device.
> + * @id: Jack id
> + */
> +static virtio_snd_jack *virtio_snd_get_jack(VirtIOSound *s, uint32_t id)
> +{
> +    if (id >= s->snd_conf.jacks) {
> +        return NULL;
> +    }
> +    return s->jacks[id];
> +}
> +
> +/*
> + * Handles VIRTIO_SND_R_JACK_INFO.
> + * The function writes the info structs and response to the virtqueue element.
> + * Returns the used size in bytes.
> + *
> + * @s: VirtIOSound card
> + * @elem: The request element from control queue
> + */
> +static uint32_t virtio_snd_handle_jack_info(VirtIOSound *s,
> +                                            VirtQueueElement *elem)
> +{
> +    virtio_snd_query_info req;
> +    size_t sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
> +    assert(sz == sizeof(virtio_snd_query_info));
> +
> +    virtio_snd_hdr resp;
> +
> +    if (iov_size(elem->in_sg, elem->in_num) <
> +        sizeof(virtio_snd_hdr) + req.count * req.size) {
> +        virtio_snd_err("jack info: buffer too small got: %lu needed: %lu\n",
> +                       iov_size(elem->in_sg, elem->in_num),
> +                       sizeof(virtio_snd_hdr) + req.count * req.size);
> +        resp.code = VIRTIO_SND_S_BAD_MSG;
> +        goto done;
> +    }
> +
> +    virtio_snd_jack_info *jack_info = g_new0(virtio_snd_jack_info, req.count);
> +    for (int i = req.start_id; i < req.count + req.start_id; i++) {
> +        virtio_snd_jack *jack = virtio_snd_get_jack(s, i);
> +        if (!jack) {
> +            virtio_snd_err("Invalid jack id: %d\n", i);
> +            resp.code = VIRTIO_SND_S_BAD_MSG;
> +            goto done;
> +        }
> +
> +        jack_info[i - req.start_id].hdr.hda_fn_nid = jack->hda_fn_nid;
> +        jack_info[i - req.start_id].features = jack->features;
> +        jack_info[i - req.start_id].hda_reg_defconf = jack->hda_reg_defconf;
> +        jack_info[i - req.start_id].hda_reg_caps = jack->hda_reg_caps;
> +        jack_info[i - req.start_id].connected = jack->connected;
> +        memset(jack_info[i - req.start_id].padding, 0,
> +               sizeof(jack_info[i - req.start_id].padding));
> +    }
> +
> +    resp.code = VIRTIO_SND_S_OK;
> +done:
> +    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
> +    assert(sz == sizeof(virtio_snd_hdr));
> +
> +    if (resp.code == VIRTIO_SND_S_BAD_MSG) {
> +        g_free(jack_info);
> +        return sz;
> +    }
> +
> +    sz = iov_from_buf(elem->in_sg, elem->in_num, sizeof(virtio_snd_hdr),
> +                      jack_info, sizeof(virtio_snd_jack_info) * req.count);
> +    assert(sz == req.count * req.size);
> +    g_free(jack_info);
> +    return sizeof(virtio_snd_hdr) + sz;
> +}
>   
>   /* The control queue handler. Pops an element from the control virtqueue,
>    * checks the header and performs the requested action. Finally marks the
> @@ -102,6 +176,7 @@ static uint64_t virtio_snd_get_features(VirtIODevice *vdev, uint64_t features,
>    */
>   static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>   {
> +    VirtIOSound *s = VIRTIO_SOUND(vdev);
>       virtio_snd_hdr ctrl;
>   
>       VirtQueueElement *elem = NULL;
> @@ -131,7 +206,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>               /* error */
>               virtio_snd_err("virtio snd ctrl could not read header\n");
>           } else if (ctrl.code == VIRTIO_SND_R_JACK_INFO) {
> -            virtio_snd_log("VIRTIO_SND_R_JACK_INFO");
> +            sz = virtio_snd_handle_jack_info(s, elem);
> +            goto done;
>           } else if (ctrl.code == VIRTIO_SND_R_JACK_REMAP) {
>               virtio_snd_log("VIRTIO_SND_R_JACK_REMAP");
>           } else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
> @@ -156,8 +232,9 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>           virtio_snd_hdr resp;
>           resp.code = VIRTIO_SND_S_OK;
>           sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
> -        virtqueue_push(vq, elem, sz);
>   
> +done:
> +        virtqueue_push(vq, elem, sz);
>           virtio_notify(vdev, vq);
>           g_free(iov2);
>           g_free(elem);

This patch has a warning:

.../hw/audio/virtio-snd.c: In function 'virtio_snd_handle_ctrl':
.../include/qemu/iov.h:49:16: error: 'jack_info' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]
    49 |         return iov_from_buf_full(iov, iov_cnt, offset, buf, bytes);
       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...hw/audio/virtio-snd.c:135:27: note: 'jack_info' was declared here
   135 |     virtio_snd_jack_info *jack_info = g_new0(virtio_snd_jack_info, req.count);
       |                           ^~~~~~~~~

Thanks,
Laurent

[RFC PATCH 13/25] virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index c2af26f3cb..aec3e86db2 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -167,6 +167,27 @@ done:
     return sizeof(virtio_snd_hdr) + sz;
 }
 
+/*
+ * Handles VIRTIO_SND_R_JACK_REMAP.
+ * Not implemented yet.
+ *
+ * @s: VirtIOSound card
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_jack_remap(VirtIOSound *s,
+                                             VirtQueueElement *elem)
+{
+    virtio_snd_hdr resp;
+    resp.code = VIRTIO_SND_S_OK;
+
+    /* TODO: implement remap */
+
+    size_t sz;
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+    return sz;
+}
+
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
  * element as used.
@@ -209,7 +230,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             sz = virtio_snd_handle_jack_info(s, elem);
             goto done;
         } else if (ctrl.code == VIRTIO_SND_R_JACK_REMAP) {
-            virtio_snd_log("VIRTIO_SND_R_JACK_REMAP");
+            sz = virtio_snd_handle_jack_remap(s, elem);
+            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
             virtio_snd_log("VIRTIO_SND_R_PCM_INFO");
         } else if (ctrl.code == VIRTIO_SND_R_PCM_SET_PARAMS) {
-- 
2.31.1

[RFC PATCH 14/25] virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 88 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index aec3e86db2..a53a6be168 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -188,6 +188,91 @@ static uint32_t virtio_snd_handle_jack_remap(VirtIOSound *s,
     return sz;
 }
 
+/*
+ * Get a specific stream from the virtio sound card device.
+ *
+ * @s: VirtIOSound card device
+ * @stream: Stream id
+ *
+ * Returns NULL if function fails.
+ * TODO: Make failure more explicit. Output can be NULL if the stream number
+ *       is valid but the stream hasn't been allocated yet.
+ */
+static virtio_snd_pcm_stream *virtio_snd_pcm_get_stream(VirtIOSound *s,
+                                                        uint32_t stream)
+{
+    if (stream >= s->snd_conf.streams) {
+        virtio_snd_err("Invalid stream request %d\n", stream);
+        return NULL;
+    }
+    return s->streams[stream];
+}
+
+/*
+ * Handle the VIRTIO_SND_R_PCM_INFO request.
+ * The function writes the info structs to the request element.
+ * Returns the used size in bytes.
+ *
+ * @s: VirtIOSound card device
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_pcm_info(VirtIOSound *s,
+                                           VirtQueueElement *elem)
+{
+    virtio_snd_query_info req;
+    uint32_t sz;
+    sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+    assert(sz == sizeof(virtio_snd_query_info));
+
+    virtio_snd_hdr resp;
+    if (iov_size(elem->in_sg, elem->in_num) <
+        sizeof(virtio_snd_hdr) + req.size * req.count) {
+        virtio_snd_err("pcm info: buffer too small, got: %lu, needed: %lu\n",
+                iov_size(elem->in_sg, elem->in_num),
+                sizeof(virtio_snd_pcm_info));
+        resp.code = VIRTIO_SND_S_BAD_MSG;
+        goto done;
+    }
+
+    virtio_snd_pcm_stream *stream;
+    virtio_snd_pcm_info *pcm_info = g_new0(virtio_snd_pcm_info, req.count);
+    for (int i = req.start_id; i < req.start_id + req.count; i++) {
+        stream = virtio_snd_pcm_get_stream(s, i);
+
+        if (!stream) {
+            virtio_snd_err("Invalid stream id: %d\n", i);
+            resp.code = VIRTIO_SND_S_BAD_MSG;
+            goto done;
+        }
+
+        pcm_info[i - req.start_id].hdr.hda_fn_nid = stream->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;
+
+        memset(&pcm_info[i].padding, 0, sizeof(pcm_info[i].padding));
+    }
+
+    resp.code = VIRTIO_SND_S_OK;
+done:
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+
+    if (resp.code == VIRTIO_SND_S_BAD_MSG) {
+        g_free(pcm_info);
+        return sz;
+    }
+
+    sz = iov_from_buf(elem->in_sg, elem->in_num, sizeof(virtio_snd_hdr),
+                      pcm_info, sizeof(virtio_snd_pcm_info) * req.count);
+    assert(sz == req.size * req.count);
+    g_free(pcm_info);
+    return sizeof(virtio_snd_hdr) + sz;
+}
+
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
  * element as used.
@@ -233,7 +318,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             sz = virtio_snd_handle_jack_remap(s, elem);
             goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
-            virtio_snd_log("VIRTIO_SND_R_PCM_INFO");
+            sz = virtio_snd_handle_pcm_info(s, elem);
+            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_SET_PARAMS) {
             virtio_snd_log("VIRTIO_SND_R_PCM_SET_PARAMS");
         } else if (ctrl.code == VIRTIO_SND_R_PCM_PREPARE) {
-- 
2.31.1

Re: [RFC PATCH 14/25] virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler

[Laurent Vivier]

Le 11/02/2022 à 23:13, Shreyansh Chouhan a écrit :
> Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
> ---
>   hw/audio/virtio-snd.c | 88 ++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 87 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index aec3e86db2..a53a6be168 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -188,6 +188,91 @@ static uint32_t virtio_snd_handle_jack_remap(VirtIOSound *s,
>       return sz;
>   }
>   
> +/*
> + * Get a specific stream from the virtio sound card device.
> + *
> + * @s: VirtIOSound card device
> + * @stream: Stream id
> + *
> + * Returns NULL if function fails.
> + * TODO: Make failure more explicit. Output can be NULL if the stream number
> + *       is valid but the stream hasn't been allocated yet.
> + */
> +static virtio_snd_pcm_stream *virtio_snd_pcm_get_stream(VirtIOSound *s,
> +                                                        uint32_t stream)
> +{
> +    if (stream >= s->snd_conf.streams) {
> +        virtio_snd_err("Invalid stream request %d\n", stream);
> +        return NULL;
> +    }
> +    return s->streams[stream];
> +}
> +
> +/*
> + * Handle the VIRTIO_SND_R_PCM_INFO request.
> + * The function writes the info structs to the request element.
> + * Returns the used size in bytes.
> + *
> + * @s: VirtIOSound card device
> + * @elem: The request element from control queue
> + */
> +static uint32_t virtio_snd_handle_pcm_info(VirtIOSound *s,
> +                                           VirtQueueElement *elem)
> +{
> +    virtio_snd_query_info req;
> +    uint32_t sz;
> +    sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
> +    assert(sz == sizeof(virtio_snd_query_info));
> +
> +    virtio_snd_hdr resp;
> +    if (iov_size(elem->in_sg, elem->in_num) <
> +        sizeof(virtio_snd_hdr) + req.size * req.count) {
> +        virtio_snd_err("pcm info: buffer too small, got: %lu, needed: %lu\n",
> +                iov_size(elem->in_sg, elem->in_num),
> +                sizeof(virtio_snd_pcm_info));
> +        resp.code = VIRTIO_SND_S_BAD_MSG;
> +        goto done;
> +    }
> +
> +    virtio_snd_pcm_stream *stream;
> +    virtio_snd_pcm_info *pcm_info = g_new0(virtio_snd_pcm_info, req.count);
> +    for (int i = req.start_id; i < req.start_id + req.count; i++) {
> +        stream = virtio_snd_pcm_get_stream(s, i);
> +
> +        if (!stream) {
> +            virtio_snd_err("Invalid stream id: %d\n", i);
> +            resp.code = VIRTIO_SND_S_BAD_MSG;
> +            goto done;
> +        }
> +
> +        pcm_info[i - req.start_id].hdr.hda_fn_nid = stream->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;
> +
> +        memset(&pcm_info[i].padding, 0, sizeof(pcm_info[i].padding));
> +    }
> +
> +    resp.code = VIRTIO_SND_S_OK;
> +done:
> +    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
> +    assert(sz == sizeof(virtio_snd_hdr));
> +
> +    if (resp.code == VIRTIO_SND_S_BAD_MSG) {
> +        g_free(pcm_info);
> +        return sz;
> +    }
> +
> +    sz = iov_from_buf(elem->in_sg, elem->in_num, sizeof(virtio_snd_hdr),
> +                      pcm_info, sizeof(virtio_snd_pcm_info) * req.count);

Same problem here:

In file included from ../../../Projects/qemu-next/hw/audio/virtio-snd.c:7:
.../hw/audio/virtio-snd.c: In function 'virtio_snd_handle_ctrl':
.../include/qemu/iov.h:49:16: error: 'pcm_info' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]
    49 |         return iov_from_buf_full(iov, iov_cnt, offset, buf, bytes);
       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.../hw/audio/virtio-snd.c:238:26: note: 'pcm_info' was declared here
   238 |     virtio_snd_pcm_info *pcm_info = g_new0(virtio_snd_pcm_info, req.count);
       |                          ^~~~~~~~

> +    assert(sz == req.size * req.count);
> +    g_free(pcm_info);
> +    return sizeof(virtio_snd_hdr) + sz;
> +}
> +
>   /* The control queue handler. Pops an element from the control virtqueue,
>    * checks the header and performs the requested action. Finally marks the
>    * element as used.
> @@ -233,7 +318,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>               sz = virtio_snd_handle_jack_remap(s, elem);
>               goto done;
>           } else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
> -            virtio_snd_log("VIRTIO_SND_R_PCM_INFO");
> +            sz = virtio_snd_handle_pcm_info(s, elem);
> +            goto done;
>           } else if (ctrl.code == VIRTIO_SND_R_PCM_SET_PARAMS) {
>               virtio_snd_log("VIRTIO_SND_R_PCM_SET_PARAMS");
>           } else if (ctrl.code == VIRTIO_SND_R_PCM_PREPARE) {

[RFC PATCH 15/25] virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle

[Shreyansh Chouhan]

Added handler for the VIRTIO_SND_R_PCM_SET_PARAMS control request.
The handler was split up into two functions in so that the code
could be reused in the realize function.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 116 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 115 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index a53a6be168..acb126f392 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -208,6 +208,22 @@ static virtio_snd_pcm_stream *virtio_snd_pcm_get_stream(VirtIOSound *s,
     return s->streams[stream];
 }
 
+/*
+ * Get params for a sepific stream.
+ *
+ * @s: VirtIOSound card device
+ * @stream: Stream id
+ */
+static virtio_snd_pcm_params *virtio_snd_pcm_get_params(VirtIOSound *s,
+                                                        uint32_t stream)
+{
+    if (stream >= s->snd_conf.streams) {
+        virtio_snd_err("Invalid stream request %d\n", stream);
+        return NULL;
+    }
+    return s->pcm_params[stream];
+}
+
 /*
  * Handle the VIRTIO_SND_R_PCM_INFO request.
  * The function writes the info structs to the request element.
@@ -273,6 +289,103 @@ done:
     return sizeof(virtio_snd_hdr) + sz;
 }
 
+/*
+ * Set the given stream params.
+ * Called by both virtio_snd_handle_pcm_set_params and during device
+ * initialization.
+ * Returns a virtio sound status VIRTIO_SND_S_*
+ *
+ * @s: VirtIOSound card device
+ * @params: The PCM params as defined in the virtio specification
+ */
+static uint32_t virtio_snd_pcm_set_params_impl(VirtIOSound *s,
+                                               virtio_snd_pcm_set_params *params)
+{
+    uint32_t st = params->hdr.stream_id;
+    if (st > s->snd_conf.streams || !(s->pcm_params)) {
+        virtio_error(VIRTIO_DEVICE(s), "Streams not initalized\n");
+        return VIRTIO_SND_S_BAD_MSG;
+    }
+
+    if (!s->pcm_params[st]) {
+        s->pcm_params[st] = g_new0(virtio_snd_pcm_params, 1);
+    }
+    virtio_snd_pcm_params *st_params = virtio_snd_pcm_get_params(s, st);
+
+    st_params->features = params->features;
+    st_params->buffer_bytes = params->buffer_bytes;
+    st_params->period_bytes = params->period_bytes;
+
+    if (params->channel < 1 || params->channel > AUDIO_MAX_CHANNELS) {
+        virtio_snd_err("Number of channels not supported\n");
+        return VIRTIO_SND_S_NOT_SUPP;
+    }
+    st_params->channel = params->channel;
+
+    uint32_t supported_formats = 1 << VIRTIO_SND_PCM_FMT_S8 |
+                                 1 << VIRTIO_SND_PCM_FMT_U8 |
+                                 1 << VIRTIO_SND_PCM_FMT_S16 |
+                                 1 << VIRTIO_SND_PCM_FMT_U16 |
+                                 1 << VIRTIO_SND_PCM_FMT_S32 |
+                                 1 << VIRTIO_SND_PCM_FMT_U32 |
+                                 1 << VIRTIO_SND_PCM_FMT_FLOAT;
+
+    uint32_t supported_rates = 1 << VIRTIO_SND_PCM_RATE_5512 |
+                               1 << VIRTIO_SND_PCM_RATE_8000 |
+                               1 << VIRTIO_SND_PCM_RATE_11025 |
+                               1 << VIRTIO_SND_PCM_RATE_16000 |
+                               1 << VIRTIO_SND_PCM_RATE_22050 |
+                               1 << VIRTIO_SND_PCM_RATE_32000 |
+                               1 << VIRTIO_SND_PCM_RATE_44100 |
+                               1 << VIRTIO_SND_PCM_RATE_48000 |
+                               1 << VIRTIO_SND_PCM_RATE_64000 |
+                               1 << VIRTIO_SND_PCM_RATE_88200 |
+                               1 << VIRTIO_SND_PCM_RATE_96000 |
+                               1 << VIRTIO_SND_PCM_RATE_176399 |
+                               1 << VIRTIO_SND_PCM_RATE_192000 |
+                               1 << VIRTIO_SND_PCM_RATE_384000;
+
+    if (!(supported_formats & (1 << params->format))) {
+        virtio_snd_err("Stream format not supported\n");
+        return VIRTIO_SND_S_NOT_SUPP;
+    }
+    st_params->format = params->format;
+
+    if (!(supported_rates & (1 << params->rate))) {
+        virtio_snd_err("Stream rate not supported\n");
+        return VIRTIO_SND_S_NOT_SUPP;
+    }
+    st_params->rate = params->rate;
+
+    st_params->period_bytes = params->period_bytes;
+    st_params->buffer_bytes = params->buffer_bytes;
+    return VIRTIO_SND_S_OK;
+}
+
+/*
+ * Handles the VIRTIO_SND_R_PCM_SET_PARAMS request.
+ * The function writes the response to the virtqueue element.
+ * Returns the used size in bytes.
+ *
+ * @s: VirtIOSound card device
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_pcm_set_params(VirtIOSound *s,
+                                                 VirtQueueElement *elem)
+{
+    virtio_snd_pcm_set_params req;
+    uint32_t sz;
+    sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+    assert(sz == sizeof(virtio_snd_pcm_set_params));
+
+    virtio_snd_hdr resp;
+    resp.code = virtio_snd_pcm_set_params_impl(s, &req);
+
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+    return sz;
+}
+
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
  * element as used.
@@ -321,7 +434,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             sz = virtio_snd_handle_pcm_info(s, elem);
             goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_SET_PARAMS) {
-            virtio_snd_log("VIRTIO_SND_R_PCM_SET_PARAMS");
+            sz = virtio_snd_handle_pcm_set_params(s, elem);
+            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_PREPARE) {
             virtio_snd_log("VIRTIO_SND_R_PCM_PREPARE");
         } else if (ctrl.code == VIRTIO_SND_R_PCM_START) {
-- 
2.31.1

[RFC PATCH 16/25] virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler

[Shreyansh Chouhan]

The handler doesn't intialize the SWVoiceOut streams for now.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 203 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 202 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index acb126f392..b7c4dc691d 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -386,6 +386,206 @@ static uint32_t virtio_snd_handle_pcm_set_params(VirtIOSound *s,
     return sz;
 }
 
+/*
+ * Get a QEMU Audiosystem compatible format value from a VIRTIO_SND_PCM_FMT_*
+ */
+static AudioFormat virtio_snd_get_qemu_format(uint32_t format)
+{
+    switch (format) {
+    case VIRTIO_SND_PCM_FMT_U8:
+        return AUDIO_FORMAT_U8;
+    case VIRTIO_SND_PCM_FMT_S8:
+        return AUDIO_FORMAT_S8;
+    case VIRTIO_SND_PCM_FMT_U16:
+        return AUDIO_FORMAT_U16;
+    case VIRTIO_SND_PCM_FMT_S16:
+        return AUDIO_FORMAT_S16;
+    case VIRTIO_SND_PCM_FMT_U32:
+        return AUDIO_FORMAT_U32;
+    case VIRTIO_SND_PCM_FMT_S32:
+        return AUDIO_FORMAT_S32;
+    case VIRTIO_SND_PCM_FMT_FLOAT:
+        return AUDIO_FORMAT_F32;
+    default:
+        return -1;
+    }
+}
+
+/*
+ * Get a QEMU Audiosystem compatible frequency value from a
+ * VIRTIO_SND_PCM_RATE_*
+ */
+static uint32_t virtio_snd_get_qemu_freq(uint32_t rate)
+{
+    switch (rate) {
+    case VIRTIO_SND_PCM_RATE_5512:
+        return 5512;
+    case VIRTIO_SND_PCM_RATE_8000:
+        return 8000;
+    case VIRTIO_SND_PCM_RATE_11025:
+        return 11025;
+    case VIRTIO_SND_PCM_RATE_16000:
+        return 16000;
+    case VIRTIO_SND_PCM_RATE_22050:
+        return 22050;
+    case VIRTIO_SND_PCM_RATE_32000:
+        return 32000;
+    case VIRTIO_SND_PCM_RATE_44100:
+        return 44100;
+    case VIRTIO_SND_PCM_RATE_48000:
+        return 48000;
+    case VIRTIO_SND_PCM_RATE_64000:
+        return 64000;
+    case VIRTIO_SND_PCM_RATE_88200:
+        return 88200;
+    case VIRTIO_SND_PCM_RATE_96000:
+        return 96000;
+    case VIRTIO_SND_PCM_RATE_176399:
+        return 176399;
+    case VIRTIO_SND_PCM_RATE_192000:
+        return 192000;
+    case VIRTIO_SND_PCM_RATE_384000:
+        return 384000;
+    default:
+        return -1;
+    }
+}
+
+/*
+ * Get QEMU Audiosystem compatible audsettings from virtio based pcm stream
+ * params.
+ */
+static void virtio_snd_get_qemu_audsettings(audsettings *as,
+                                            virtio_snd_pcm_params *params)
+{
+    as->nchannels = params->channel;
+    as->fmt = virtio_snd_get_qemu_format(params->format);
+    as->freq = virtio_snd_get_qemu_freq(params->rate);
+    as->endianness = AUDIO_HOST_ENDIANNESS;
+}
+
+/*
+ * Get the maximum number of virtqueue elements that can be inserted
+ * into a virtio sound pcm stream
+ *
+ * @st: virtio sound pcm stream
+ */
+static int virtio_snd_pcm_get_nelems(virtio_snd_pcm_stream *st)
+{
+    return st->buffer_bytes / st->period_bytes
+           + !!(st->buffer_bytes % st->period_bytes);
+}
+
+/*
+ * Prepares a VirtIOSound card stream.
+ * Returns a virtio sound status (VIRTIO_SND_S_*).
+ *
+ * @s: VirtIOSound card
+ * @stream: stream id
+ */
+static uint32_t virtio_snd_pcm_prepare_impl(VirtIOSound *s, uint32_t stream)
+{
+    if (!s->streams || !s->pcm_params || !s->pcm_params[stream]) {
+        virtio_snd_err("Cannot prepare stream %d without params.\n", stream);
+        return VIRTIO_SND_S_BAD_MSG;
+    }
+
+    uint32_t supported_formats = 1 << VIRTIO_SND_PCM_FMT_S8 |
+                                 1 << VIRTIO_SND_PCM_FMT_U8 |
+                                 1 << VIRTIO_SND_PCM_FMT_S16 |
+                                 1 << VIRTIO_SND_PCM_FMT_U16 |
+                                 1 << VIRTIO_SND_PCM_FMT_S32 |
+                                 1 << VIRTIO_SND_PCM_FMT_U32 |
+                                 1 << VIRTIO_SND_PCM_FMT_FLOAT;
+
+    uint32_t supported_rates = 1 << VIRTIO_SND_PCM_RATE_5512 |
+                               1 << VIRTIO_SND_PCM_RATE_8000 |
+                               1 << VIRTIO_SND_PCM_RATE_11025 |
+                               1 << VIRTIO_SND_PCM_RATE_16000 |
+                               1 << VIRTIO_SND_PCM_RATE_22050 |
+                               1 << VIRTIO_SND_PCM_RATE_32000 |
+                               1 << VIRTIO_SND_PCM_RATE_44100 |
+                               1 << VIRTIO_SND_PCM_RATE_48000 |
+                               1 << VIRTIO_SND_PCM_RATE_64000 |
+                               1 << VIRTIO_SND_PCM_RATE_88200 |
+                               1 << VIRTIO_SND_PCM_RATE_96000 |
+                               1 << VIRTIO_SND_PCM_RATE_176399 |
+                               1 << VIRTIO_SND_PCM_RATE_192000 |
+                               1 << VIRTIO_SND_PCM_RATE_384000;
+
+    virtio_snd_pcm_params *params = virtio_snd_pcm_get_params(s, stream);
+    virtio_snd_pcm_stream *st = g_new0(virtio_snd_pcm_stream, 1);
+    st->hda_fn_nid = VIRTIO_SOUND_HDA_FN_NID_OUT;
+    st->features = 0;
+    st->direction = stream < s->snd_conf.streams / 2 + (s->snd_conf.streams & 1) ?
+                    VIRTIO_SND_D_OUTPUT : VIRTIO_SND_D_INPUT;
+    st->channels_min = 1;
+    st->channels_max = AUDIO_MAX_CHANNELS;
+    st->formats = supported_formats;
+    st->rates = supported_rates;
+    st->s = s;
+
+    st->buffer_bytes = params->buffer_bytes;
+    st->period_bytes = params->period_bytes;
+
+    st->chmap[0] = VIRTIO_SND_CHMAP_FL;
+    st->chmap[1] = VIRTIO_SND_CHMAP_FR;
+
+    st->flushing = false;
+
+    audsettings as;
+    virtio_snd_get_qemu_audsettings(&as, params);
+
+    if (st->direction == VIRTIO_SND_D_OUTPUT) {
+        /* st->voice.out = AUD_open_out(&s->card,
+         *                              st->voice.out,
+         *                              "virtio_snd_card",
+         *                              st,
+         *                              virtio_snd_output_cb, &as);
+         */
+    } else {
+        /* st->voice.in = AUD_open_in(&s->card,
+         *                            st->voice.in,
+         *                            "virtio_snd_card",
+         *                            st,
+         *                            virtio_snd_input_cb,
+         *                            &as);
+         */
+    }
+
+    uint32_t nelems = virtio_snd_pcm_get_nelems(st);
+    st->elems = g_new0(VirtQueueElement *, nelems);
+    st->w_pos = 0;
+    st->r_pos = 0;
+    s->streams[stream] = st;
+
+    return VIRTIO_SND_S_OK;
+}
+
+/*
+ * Handles VIRTIO_SND_R_PCM_PREPARE.
+ * The function writes the response to the virtqueue element.
+ * Returns the used size in bytes.
+ *
+ * @s: VirtIOSound card
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_pcm_prepare(VirtIOSound *s,
+                                              VirtQueueElement *elem)
+{
+    virtio_snd_pcm_hdr req;
+    size_t sz;
+
+    sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+    assert(sz == sizeof(virtio_snd_pcm_hdr));
+
+    virtio_snd_hdr resp;
+    resp.code = virtio_snd_pcm_prepare_impl(s, req.stream_id);
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+    return sz;
+}
+
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
  * element as used.
@@ -437,7 +637,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             sz = virtio_snd_handle_pcm_set_params(s, elem);
             goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_PREPARE) {
-            virtio_snd_log("VIRTIO_SND_R_PCM_PREPARE");
+            sz = virtio_snd_handle_pcm_prepare(s, elem);
+            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_START) {
             virtio_snd_log("VIRTIO_SND_R_PCM_START");
         } else if (ctrl.code == VIRTIO_SND_R_PCM_STOP) {
-- 
2.31.1

[RFC PATCH 17/25] virtio-snd: Add default configs to realize fn

[Shreyansh Chouhan]

Added default configurations for streams and jacks in the
realize function.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index b7c4dc691d..5ed8e524a6 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -698,11 +698,51 @@ static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
     /* set up QEMUSoundCard and audiodev */
     AUD_register_card ("virtio_snd_card", &s->card);
 
+    // set default params for all streams
+    virtio_snd_pcm_set_params default_params;
+    default_params.features = 0;
+    default_params.buffer_bytes = 8192;
+    default_params.period_bytes = 4096;
+    default_params.channel = 2;
+    default_params.format = VIRTIO_SND_PCM_FMT_S16;
+    default_params.rate = VIRTIO_SND_PCM_RATE_44100;
+
     s->ctrl_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_ctrl);
 
     s->streams = g_new0(virtio_snd_pcm_stream *, s->snd_conf.streams);
     s->pcm_params = g_new0(virtio_snd_pcm_params *, s->snd_conf.streams);
     s->jacks = g_new0(virtio_snd_jack *, s->snd_conf.jacks);
+
+    uint32_t status;
+    for (int i = 0; i < s->snd_conf.streams; i++) {
+        default_params.hdr.stream_id = i;
+        status = virtio_snd_pcm_set_params_impl(s, &default_params);
+        if (status != VIRTIO_SND_S_OK) {
+            error_setg(errp, "Can't initalize stream params.\n");
+            return;
+        }
+        status = virtio_snd_pcm_prepare_impl(s, i);
+        if (status != VIRTIO_SND_S_OK) {
+            error_setg(errp, "Can't prepare streams.\n");
+            return;
+        }
+    }
+
+    for (int i = 0; i < s->snd_conf.jacks; i++) {
+        // TODO: For now the hda_fn_nid connects the starting streams to these
+        // jacks. This isn't working for now since the directions will be wrong
+        // for a few jacks. Similarly the capabilities are just placeholders.
+        s->jacks[i] = (virtio_snd_jack *)g_malloc0(sizeof(virtio_snd_jack));
+        s->jacks[i]->features = 0;
+        s->jacks[i]->hda_fn_nid = i;
+        s->jacks[i]->hda_reg_defconf = ((AC_JACK_PORT_COMPLEX << AC_DEFCFG_PORT_CONN_SHIFT) |
+                                       (AC_JACK_LINE_OUT     << AC_DEFCFG_DEVICE_SHIFT)    |
+                                       (AC_JACK_CONN_1_8     << AC_DEFCFG_CONN_TYPE_SHIFT) |
+                                       (AC_JACK_COLOR_GREEN  << AC_DEFCFG_COLOR_SHIFT)     |
+                                       0x10);
+        s->jacks[i]->hda_reg_caps = AC_PINCAP_OUT;
+        s->jacks[i]->connected = false;
+    }
 }
 
 static void virtio_snd_device_unrealize(DeviceState *dev)
-- 
2.31.1

[RFC PATCH 18/25] virtio-snd: Add callback for SWVoiceOut

[Shreyansh Chouhan]

Added the callback for writing audio using AUD_write. The callback uses
two helper functions for reading the buffers from the streams and
handling the buffers that were written. initialized the
SWVoiceOut using this callback.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 254 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 248 insertions(+), 6 deletions(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 5ed8e524a6..b74c9e4a1f 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -476,6 +476,248 @@ static int virtio_snd_pcm_get_nelems(virtio_snd_pcm_stream *st)
            + !!(st->buffer_bytes % st->period_bytes);
 }
 
+static uint32_t virtio_snd_pcm_get_buf_pos(virtio_snd_pcm_stream *st,
+                                           uint32_t dir)
+{
+    return (dir == VIRTIO_SND_D_OUTPUT) ? st->w_pos : st->r_pos;
+}
+
+static uint32_t virtio_snd_pcm_get_curr_elem(virtio_snd_pcm_stream *st,
+                                            uint32_t dir)
+{
+    uint32_t pos;
+    int nelems;
+
+    nelems = virtio_snd_pcm_get_nelems(st);
+    pos = virtio_snd_pcm_get_buf_pos(st, dir);
+
+    return (pos / st->period_bytes) % nelems;
+}
+
+static uint32_t virtio_snd_pcm_get_curr_elem_cap(virtio_snd_pcm_stream *st,
+                                                 uint32_t dir)
+{
+    uint32_t cap_bytes;
+    int i;
+
+    i = virtio_snd_pcm_get_curr_elem(st, dir);
+
+    if (dir == VIRTIO_SND_D_OUTPUT)
+        cap_bytes = iov_size(st->elems[i]->out_sg, st->elems[i]->out_num)
+                    - sizeof(virtio_snd_pcm_xfer);
+    else
+        cap_bytes = iov_size(st->elems[i]->in_sg, st->elems[i]->in_num)
+                    - sizeof(virtio_snd_pcm_status);
+
+    return cap_bytes;
+}
+
+static uint32_t virtio_snd_pcm_get_curr_elem_used(virtio_snd_pcm_stream *st,
+                                                  uint32_t dir)
+{
+    uint32_t pos;
+
+    pos = virtio_snd_pcm_get_buf_pos(st, dir);
+
+    return pos % st->period_bytes;
+}
+
+static uint32_t virtio_snd_pcm_get_curr_elem_free(virtio_snd_pcm_stream *st,
+                                                  uint32_t dir)
+{
+    uint32_t free_bytes, used;
+
+    used = virtio_snd_pcm_get_curr_elem_used(st, dir);
+    free_bytes = virtio_snd_pcm_get_curr_elem_cap(st, dir) - used;
+
+    return free_bytes;
+}
+
+/*
+ * Get the size in bytes of the buffer that still has to be written.
+ *
+ * @st: virtio sound pcm stream
+ */
+static uint32_t virtio_snd_pcm_get_pending_bytes(virtio_snd_pcm_stream *st)
+{
+    return (st->direction == VIRTIO_SND_D_OUTPUT) ?
+            st->r_pos - st->w_pos :
+            st->w_pos - st->r_pos;
+}
+
+static uint32_t virtio_snd_pcm_get_buf_to_proc(virtio_snd_pcm_stream *st,
+                                               size_t size)
+{
+    uint32_t to_proc, elem_free;
+
+    elem_free = virtio_snd_pcm_get_curr_elem_free(st, st->direction);
+    to_proc = MIN(elem_free, size);
+
+    return to_proc;
+}
+
+static void virtio_snd_pcm_update_buf_pos(virtio_snd_pcm_stream *st, uint32_t dir,
+                                          uint32_t size)
+{
+    if (dir == VIRTIO_SND_D_OUTPUT)
+        st->w_pos += size;
+    else
+        st->r_pos += size;
+}
+
+/*
+ * Get data from a stream of the virtio sound device. Only reads upto the
+ * end of the current virtqueue element. Returns the number of bytes read.
+ *
+ * @buffer: Write to this buffer
+ * @size: The number of bytes to read
+ * @st: VirtIOSound card stream
+ * @offset: Start reading from this offseta in the stream (in bytes)
+ */
+static size_t virtio_snd_pcm_buf_read(void *buf, size_t to_read,
+                                  virtio_snd_pcm_stream *st)
+{
+    size_t sz;
+    int i, used;
+
+    used = virtio_snd_pcm_get_curr_elem_used(st, st->direction);
+    i = virtio_snd_pcm_get_curr_elem(st, st->direction);
+
+    sz = iov_to_buf(st->elems[i]->out_sg, st->elems[i]->out_num,
+                    sizeof(virtio_snd_pcm_xfer) + used, buf, to_read);
+
+    assert(sz == to_read);
+    return sz;
+}
+
+/*
+ * Marks an element as used, pushes it to queue and notifies the device.
+ * Also frees the element
+ */
+static void virtio_snd_pcm_handle_elem_used(virtio_snd_pcm_stream *st)
+{
+    int elem_size, i;
+    size_t sz, offset;
+
+    virtio_snd_pcm_status status;
+    status.status = VIRTIO_SND_S_OK;
+    status.latency_bytes = 0;
+
+    i = virtio_snd_pcm_get_curr_elem(st, st->direction);
+    elem_size = iov_size(st->elems[i]->out_sg, st->elems[i]->out_num)
+                + iov_size(st->elems[i]->in_sg, st->elems[i]->in_num);
+    offset = iov_size(st->elems[i]->in_sg, st->elems[i]->in_num)
+             - sizeof(virtio_snd_pcm_status);
+
+    sz = iov_from_buf(st->elems[i]->in_sg, st->elems[i]->in_num, offset,
+                      &status, sizeof(status));
+    assert(sz == sizeof(virtio_snd_pcm_status));
+
+    virtqueue_push(st->s->tx_vq, st->elems[i], elem_size);
+    virtio_notify(VIRTIO_DEVICE(st->s), st->s->tx_vq);
+
+    g_free(st->elems[i]);
+    st->elems[i] = NULL;
+}
+
+/*
+ * Handle a buffer after it has been written by AUD_write.
+ * It writes the status for the I/O messages that have been completed and
+ * marks the tx virtqueue elmenets as used. It notifies the device
+ * about I/O completion. Returns the number of bytes handled.
+ *
+ * @st: VirtIOSound card stream
+ * @size: Size that was written by AUD_write
+ *        If size = 0, write for the last element failed
+ */
+static size_t virtio_snd_pcm_handle_used(virtio_snd_pcm_stream *st,
+                                         size_t size)
+{
+    if (size == 0)
+        // error
+        return 0;
+
+    size_t used, elem_buf_size;
+
+    used = virtio_snd_pcm_get_curr_elem_used(st, st->direction);
+    elem_buf_size = virtio_snd_pcm_get_curr_elem_cap(st, st->direction);
+
+    if (used + size == elem_buf_size)
+        virtio_snd_pcm_handle_elem_used(st);
+
+    virtio_snd_pcm_update_buf_pos(st, st->direction, size);
+
+    return size;
+}
+
+/*
+ * Writes upto the end of the current virtqueue element.
+ * Returns the numbre of bytes written.
+ *
+ * @st: Virtio sound card stream
+ * @size: Number of bytes to write to HWVoiceOut
+ */
+static size_t write_audio(virtio_snd_pcm_stream *st,
+                                     uint32_t size)
+{
+    size_t to_write, handled, written;
+    void *mixbuf;
+
+    to_write = virtio_snd_pcm_get_buf_to_proc(st, size);
+    mixbuf = g_malloc0(to_write);
+    to_write = virtio_snd_pcm_buf_read(mixbuf, to_write, st);
+    written = AUD_write(st->voice.out, mixbuf, to_write);
+    handled = virtio_snd_pcm_handle_used(st, written);
+    assert(handled == written);
+
+    g_free(mixbuf);
+    return written;
+}
+
+/*
+ * Callback for AUD_open_out.
+ * Reads a buffer from the VirtIOSound card stream and writes it
+ * using AUD_write.
+ *
+ * @opaque: VirtIOSound card stream
+ * @free: Size in bytes that can be written via AUD_write
+ */
+static void virtio_snd_output_cb(void *opaque, int free)
+{
+    int to_play, pending;
+    virtio_snd_pcm_stream *st = opaque;
+    int written = 0;
+
+    pending = virtio_snd_pcm_get_pending_bytes(st);
+
+    if (!pending && st->flushing) {
+        st->flushing = false;
+
+        if (st->direction == VIRTIO_SND_D_OUTPUT)
+            AUD_set_active_out(st->voice.out, false);
+        else
+            AUD_set_active_in(st->voice.in, false);
+        return;
+    }
+
+    to_play = MIN(free, pending);
+
+    virtio_snd_log("to_play: %d, free: %d, pending: %d\n", to_play, free, pending);
+    while (to_play) {
+        int wbytes = write_audio(st, to_play);
+
+        if (!wbytes)
+            break;
+
+        to_play -= wbytes;
+        free -= wbytes;
+        written += wbytes;
+        pending -= wbytes;
+    }
+
+    virtio_snd_log("written: %d\n", written);
+}
+
 /*
  * Prepares a VirtIOSound card stream.
  * Returns a virtio sound status (VIRTIO_SND_S_*).
@@ -537,12 +779,12 @@ static uint32_t virtio_snd_pcm_prepare_impl(VirtIOSound *s, uint32_t stream)
     virtio_snd_get_qemu_audsettings(&as, params);
 
     if (st->direction == VIRTIO_SND_D_OUTPUT) {
-        /* st->voice.out = AUD_open_out(&s->card,
-         *                              st->voice.out,
-         *                              "virtio_snd_card",
-         *                              st,
-         *                              virtio_snd_output_cb, &as);
-         */
+        st->voice.out = AUD_open_out(&s->card,
+                                     st->voice.out,
+                                     "virtio_snd_card",
+                                     st,
+                                     virtio_snd_output_cb, &as);
+
     } else {
         /* st->voice.in = AUD_open_in(&s->card,
          *                            st->voice.in,
-- 
2.31.1

[RFC PATCH 19/25] virtio-snd: Add start/stop handler

[Shreyansh Chouhan]

Added handlers for VIRTIO_SND_PCM_START and VIRTIO_SND_PCM_STOP.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index b74c9e4a1f..1b3e1f75f4 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -828,6 +828,36 @@ static uint32_t virtio_snd_handle_pcm_prepare(VirtIOSound *s,
     return sz;
 }
 
+/*
+ * Handles VIRTIO_SND_R_PCM_START.
+ * The function writes the response to the virtqueue element.
+ * Returns the used size in bytes.
+ *
+ * @s: VirtIOSound card
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_pcm_start_stop(VirtIOSound *s,
+                                                 VirtQueueElement *elem,
+                                                 bool start)
+{
+    virtio_snd_pcm_hdr req;
+    size_t sz;
+
+    sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+    assert(sz == sizeof(virtio_snd_pcm_hdr));
+
+    virtio_snd_hdr resp;
+    resp.code = VIRTIO_SND_S_OK;
+
+    virtio_snd_pcm_stream *st = virtio_snd_pcm_get_stream(s, req.stream_id);
+    if (st->direction == VIRTIO_SND_D_OUTPUT)
+        AUD_set_active_out(st->voice.out, start);
+
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+    return sz;
+}
+
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
  * element as used.
@@ -882,9 +912,10 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             sz = virtio_snd_handle_pcm_prepare(s, elem);
             goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_START) {
-            virtio_snd_log("VIRTIO_SND_R_PCM_START");
+            sz = virtio_snd_handle_pcm_start_stop(s, elem, true);
+            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_STOP) {
-            virtio_snd_log("VIRTIO_SND_R_PCM_STOP");
+            sz = virtio_snd_handle_pcm_start_stop(s, elem, false);
         } else if (ctrl.code == VIRTIO_SND_R_PCM_RELEASE) {
             virtio_snd_log("VIRTIO_SND_R_PCM_RELEASE");
         } else if (ctrl.code == VIRTIO_SND_R_CHMAP_INFO) {
-- 
2.31.1

[RFC PATCH 20/25] virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 82 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 81 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 1b3e1f75f4..7b80a92737 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -858,6 +858,85 @@ static uint32_t virtio_snd_handle_pcm_start_stop(VirtIOSound *s,
     return sz;
 }
 
+/*
+ * Releases the resources allocated to a stream. Sepearated from the handler
+ * so that the code could be reused in the unrealize function.
+ *
+ * TODO: Doesn't handle the stream buffers that are yet to be played.
+ *
+ * @s: VirtIOSound card
+ * @stream: stream id
+ */
+static uint32_t virtio_snd_pcm_release_impl(virtio_snd_pcm_stream *st, uint32_t stream)
+{
+    // if there are still pending io messages do nothing
+    if (virtio_snd_pcm_get_pending_bytes(st)) {
+        // flush the stream
+        virtio_snd_log("Started flushing stream");
+
+        // set flushing to true, the callback will automatically close the
+        // stream once the flushing is done
+        st->flushing = true;
+
+        if (st->direction == VIRTIO_SND_D_OUTPUT)
+            AUD_set_active_out(st->voice.out, true);
+        else
+            AUD_set_active_in(st->voice.in, true);
+        return VIRTIO_SND_S_OK;
+    }
+
+    if (st->direction == VIRTIO_SND_D_OUTPUT)
+        AUD_close_out(&st->s->card, st->voice.out);
+    else
+        AUD_close_in(&st->s->card, st->voice.in);
+
+    if (st->elems) {
+        int nelems = virtio_snd_pcm_get_nelems(st);
+        for (int i = 0; i < nelems; i++) {
+            g_free(st->elems[i]);
+            st->elems[i] = NULL;
+        }
+        g_free(st->elems);
+        st->elems = NULL;
+    }
+
+    g_free(st->s->streams[stream]);
+    st->s->streams[stream] = NULL;
+    return VIRTIO_SND_S_OK;
+}
+
+/*
+ * Handles VIRTIO_SND_R_PCM_RELEASE.
+ * The function writes the response to the virtqueue element.
+ * Returns the used size in bytes.
+ * TODO: Doesn't handle the stream buffers that are yet to be played.
+ *
+ * @s: VirtIOSound card
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_pcm_release(VirtIOSound *s,
+                                              VirtQueueElement *elem)
+{
+    virtio_snd_pcm_hdr req;
+    virtio_snd_hdr resp;
+    size_t sz;
+    sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+    assert(sz == sizeof(virtio_snd_pcm_hdr));
+
+    virtio_snd_log("Release called\n");
+
+    virtio_snd_pcm_stream *st = virtio_snd_pcm_get_stream(s, req.stream_id);
+    if (!st) {
+        virtio_snd_err("already released %d\n", req.stream_id);
+        return VIRTIO_SND_S_BAD_MSG;
+    }
+
+    resp.code = virtio_snd_pcm_release_impl(st, req.stream_id);
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+    assert(sz == sizeof(virtio_snd_hdr));
+    return sz;
+}
+
 /* The control queue handler. Pops an element from the control virtqueue,
  * checks the header and performs the requested action. Finally marks the
  * element as used.
@@ -917,9 +996,10 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         } else if (ctrl.code == VIRTIO_SND_R_PCM_STOP) {
             sz = virtio_snd_handle_pcm_start_stop(s, elem, false);
         } else if (ctrl.code == VIRTIO_SND_R_PCM_RELEASE) {
-            virtio_snd_log("VIRTIO_SND_R_PCM_RELEASE");
+            sz = virtio_snd_handle_pcm_release(s, elem);
         } else if (ctrl.code == VIRTIO_SND_R_CHMAP_INFO) {
             virtio_snd_log("VIRTIO_SND_R_CHMAP_INFO");
+            goto done;
         } else {
             /* error */
             virtio_snd_err("virtio snd header not recognized: %d\n", ctrl.code);
-- 
2.31.1

[RFC PATCH 21/25] virtio-snd: Replaced goto with if else

[Shreyansh Chouhan]

Removed goto from the ctrl vq handler and added an if else
branch for error handling.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 7b80a92737..cb83db0e89 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -947,7 +947,7 @@ static uint32_t virtio_snd_handle_pcm_release(VirtIOSound *s,
 static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOSound *s = VIRTIO_SOUND(vdev);
-    virtio_snd_hdr ctrl;
+    virtio_snd_hdr ctrl, resp;
 
     VirtQueueElement *elem = NULL;
     size_t sz;
@@ -959,7 +959,7 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         if (!elem) {
             break;
         }
-        if (iov_size(elem->in_sg, elem->in_num) < sizeof(ctrl) ||
+        if (iov_size(elem->in_sg, elem->in_num) < sizeof(resp) ||
                 iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
             virtio_snd_err("virtio-snd ctrl missing headers\n");
             virtqueue_detach_element(vq, elem, 0);
@@ -975,41 +975,36 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         if (sz != sizeof(ctrl)) {
             /* error */
             virtio_snd_err("virtio snd ctrl could not read header\n");
+            resp.code = VIRTIO_SND_S_BAD_MSG;
         } else if (ctrl.code == VIRTIO_SND_R_JACK_INFO) {
             sz = virtio_snd_handle_jack_info(s, elem);
-            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_JACK_REMAP) {
             sz = virtio_snd_handle_jack_remap(s, elem);
-            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
             sz = virtio_snd_handle_pcm_info(s, elem);
-            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_SET_PARAMS) {
             sz = virtio_snd_handle_pcm_set_params(s, elem);
-            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_PREPARE) {
             sz = virtio_snd_handle_pcm_prepare(s, elem);
-            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_START) {
             sz = virtio_snd_handle_pcm_start_stop(s, elem, true);
-            goto done;
         } else if (ctrl.code == VIRTIO_SND_R_PCM_STOP) {
             sz = virtio_snd_handle_pcm_start_stop(s, elem, false);
         } else if (ctrl.code == VIRTIO_SND_R_PCM_RELEASE) {
             sz = virtio_snd_handle_pcm_release(s, elem);
         } else if (ctrl.code == VIRTIO_SND_R_CHMAP_INFO) {
             virtio_snd_log("VIRTIO_SND_R_CHMAP_INFO");
-            goto done;
         } else {
             /* error */
             virtio_snd_err("virtio snd header not recognized: %d\n", ctrl.code);
+            resp.code = VIRTIO_SND_S_BAD_MSG;
         }
 
-        virtio_snd_hdr resp;
-        resp.code = VIRTIO_SND_S_OK;
-        sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+        if (resp.code == VIRTIO_SND_S_BAD_MSG) {
+            sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp,
+                              sizeof(resp));
+        }
 
-done:
         virtqueue_push(vq, elem, sz);
         virtio_notify(vdev, vq);
         g_free(iov2);
-- 
2.31.1

[RFC PATCH 22/25] virtio-snd: Add code to device unrealize function

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index cb83db0e89..7dd89c444b 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -1093,8 +1093,38 @@ static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
     }
 }
 
+/*
+ * Frees the resources allocated to the device and then frees the device
+ * itself.
+ *
+ * @dev: VirtIOSound card device
+ */
 static void virtio_snd_device_unrealize(DeviceState *dev)
 {
+    VirtIOSound *s = VIRTIO_SOUND(dev);
+
+    for (int i = 0; i < s->snd_conf.streams; i++) {
+        virtio_snd_pcm_stream *st = virtio_snd_pcm_get_stream(s, i);
+        virtio_snd_pcm_release_impl(st, i);
+        g_free(s->pcm_params[i]);
+        s->pcm_params[i] = NULL;
+    }
+    g_free(s->streams);
+    s->streams = NULL;
+    g_free(s->pcm_params);
+    s->pcm_params = NULL;
+
+    for (int i = 0; i < s->snd_conf.jacks; i++) {
+        g_free(s->jacks[i]);
+        s->jacks[i] = NULL;
+    }
+    g_free(s->jacks);
+    s->jacks = NULL;
+
+    virtio_delete_queue(s->ctrl_vq);
+    virtio_delete_queue(s->tx_vq);
+    virtio_delete_queue(s->event_vq);
+    virtio_delete_queue(s->rx_vq);
 }
 
 static void virtio_snd_reset(VirtIODevice *vdev)
-- 
2.31.1

[RFC PATCH 23/25] virtio-snd: Add xfer handler

[Shreyansh Chouhan]

The handler demultiplexes the buffers recieved in the
tx/rx virtqueue. It uses a helper function for adding these
buffers, (along with the entire virtqueue element,) to
their respective streams.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 71 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 7dd89c444b..80a34e1207 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -1012,6 +1012,74 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
     }
 }
 
+/*
+ * Adds a virtqueue element to a VirtIOSound card stream. Makes the buffer
+ * available to the stream for consumption.
+ *
+ * @s: VirtIOSound card
+ * @stream: stream id
+ * @elem: The tx virtqueue element that contains the I/O message
+ */
+static void virtio_snd_pcm_add_buf(VirtIOSound *s, uint32_t stream,
+                                   VirtQueueElement *elem)
+{
+    virtio_snd_log("add_buf called\n");
+
+    virtio_snd_pcm_stream *st = virtio_snd_pcm_get_stream(s, stream);
+    uint32_t buf_size, dir;
+    int i;
+
+    // get the direction opposite to the stream. We need read position if we are
+    // writing because we want to add data to the buffer and not consume it.
+    dir = VIRTIO_SND_D_INPUT ^ VIRTIO_SND_D_OUTPUT ^ st->direction;
+    i = virtio_snd_pcm_get_curr_elem(st, dir);
+
+    if (st->elems[i]) {
+        return;
+    }
+
+    buf_size = iov_size(elem->out_sg, elem->out_num)
+               - sizeof(virtio_snd_pcm_xfer);
+
+    st->elems[i] = elem;
+    virtio_snd_pcm_update_buf_pos(st, dir, buf_size);
+}
+
+/*
+ * The tx virtqueue handler. Makes the buffers available to their respective
+ * streams for consumption.
+ *
+ * @vdev: VirtIOSound card
+ * @vq: tx virtqueue
+ */
+static void virtio_snd_handle_xfer(VirtIODevice *vdev, VirtQueue *vq)
+{
+    virtio_snd_log("tx/rx queue callback called\n");
+    VirtIOSound *s = VIRTIO_SOUND(vdev);
+    VirtQueueElement *elem;
+    size_t sz;
+    virtio_snd_pcm_xfer hdr;
+
+    for (;;) {
+        elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+        if (!elem) {
+            break;
+        }
+        if (iov_size(elem->in_sg, elem->in_num) < sizeof(virtio_snd_pcm_status) ||
+            iov_size(elem->out_sg, elem->out_num) < sizeof(virtio_snd_pcm_xfer)) {
+            virtqueue_detach_element(vq, elem, 0);
+            g_free(elem);
+            break;
+        }
+
+        /* get the message hdr object */
+        sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &hdr, sizeof(hdr));
+        assert(sz == sizeof(hdr));
+
+        virtio_snd_pcm_add_buf(s, hdr.stream_id, elem);
+    }
+}
+
 /*
  * Initializes the VirtIOSound card device. Validates the configuration
  * passed by the command line. Initializes the virtqueues. Allocates resources
@@ -1056,6 +1124,8 @@ static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
     default_params.rate = VIRTIO_SND_PCM_RATE_44100;
 
     s->ctrl_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_ctrl);
+    s->tx_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_xfer);
+    s->rx_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_xfer);
 
     s->streams = g_new0(virtio_snd_pcm_stream *, s->snd_conf.streams);
     s->pcm_params = g_new0(virtio_snd_pcm_params *, s->snd_conf.streams);
@@ -1123,7 +1193,6 @@ static void virtio_snd_device_unrealize(DeviceState *dev)
 
     virtio_delete_queue(s->ctrl_vq);
     virtio_delete_queue(s->tx_vq);
-    virtio_delete_queue(s->event_vq);
     virtio_delete_queue(s->rx_vq);
 }
 
-- 
2.31.1

[RFC PATCH 24/25] virtio-snd: Add event vq and a handler stub

[Shreyansh Chouhan]

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/virtio-snd.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 80a34e1207..81a478d039 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -1080,6 +1080,18 @@ static void virtio_snd_handle_xfer(VirtIODevice *vdev, VirtQueue *vq)
     }
 }
 
+/*
+ * The event virtqueue handler.
+ * Not implemented yet.
+ *
+ * @vdev: VirtIOSound card
+ * @vq: event vq
+ */
+static void virtio_snd_handle_event(VirtIODevice *vdev, VirtQueue *vq)
+{
+    virtio_snd_log("event queue callback called\n");
+}
+
 /*
  * Initializes the VirtIOSound card device. Validates the configuration
  * passed by the command line. Initializes the virtqueues. Allocates resources
@@ -1124,6 +1136,7 @@ static void virtio_snd_device_realize(DeviceState *dev, Error **errp)
     default_params.rate = VIRTIO_SND_PCM_RATE_44100;
 
     s->ctrl_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_ctrl);
+    s->event_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_event);
     s->tx_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_xfer);
     s->rx_vq = virtio_add_queue(vdev, 64, virtio_snd_handle_xfer);
 
@@ -1192,6 +1205,7 @@ static void virtio_snd_device_unrealize(DeviceState *dev)
     s->jacks = NULL;
 
     virtio_delete_queue(s->ctrl_vq);
+    virtio_delete_queue(s->event_vq);
     virtio_delete_queue(s->tx_vq);
     virtio_delete_queue(s->rx_vq);
 }
-- 
2.31.1

[RFC PATCH 25/25] virtio-snd: Replaced AUD_log with tracepoints

[Shreyansh Chouhan]

Replaced the use of AUD_log via macros in virtio sound with
tracepoints.

Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh2702@gmail.com>
---
 hw/audio/trace-events | 14 +++++++++++
 hw/audio/virtio-snd.c | 55 +++++++++++++++++--------------------------
 2 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/hw/audio/trace-events b/hw/audio/trace-events
index e0e71cd9b1..a747648e4d 100644
--- a/hw/audio/trace-events
+++ b/hw/audio/trace-events
@@ -11,3 +11,17 @@ hda_audio_running(const char *stream, int nr, bool running) "st %s, nr %d, run %
 hda_audio_format(const char *stream, int chan, const char *fmt, int freq) "st %s, %d x %s @ %d Hz"
 hda_audio_adjust(const char *stream, int pos) "st %s, pos %d"
 hda_audio_overrun(const char *stream) "st %s"
+
+#virtio-snd.c
+virtio_snd_pcm_stream_flush(int stream) "flushing st %d"
+virtio_snd_handle_jack_info(int jack) "VIRTIO_SND_JACK_INFO called for jack %d"
+virtio_snd_handle_jack_remap(void) "VIRTIO_SND_PCM_JACK_REMAP called"
+virtio_snd_handle_pcm_info(int stream) "VIRTIO_SND_PCM_INFO called for stream %d"
+virtio_snd_handle_pcm_set_params(int stream) "VIRTIO_SND_PCM_SET_PARAMS called for stream %d"
+virtio_snd_handle_pcm_start(int stream) "VIRTIO_SND_PCM_START called for stream %d"
+virtio_snd_handle_pcm_stop(int stream) "VIRTIO_SND_PCM_STOP called for stream %id"
+virtio_snd_handle_pcm_release(int stream) "VIRTIO_SND_PCM_RELEASE called for stream %id"
+virtio_snd_handle_chmap_info(void) "VIRTIO_SND_CHMAP_INFO called"
+virtio_snd_output_cb(int to_play, int written) "to play: %d, written: %d"
+virtio_snd_handle_xfer(void) "tx/rx queue callback called"
+virtio_snd_handle_event(void) "event queue callback called"
diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 81a478d039..38a3b5e555 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -39,10 +39,6 @@
 #define VIRTIO_SOUND_HDA_FN_NID_OUT 0
 #define VIRTIO_SOUND_HDA_FN_NID_IN 1
 
-#define virtio_snd_log(...) AUD_log("virtio sound info", __VA_ARGS__)
-#define virtio_snd_warn(...) AUD_log("virtio sound warn", __VA_ARGS__)
-#define virtio_snd_err(...) AUD_log("virtio sound err", __VA_ARGS__)
-
 static void virtio_snd_get_config(VirtIODevice *vdev, uint8_t *config)
 {
     VirtIOSound *s = VIRTIO_SOUND(vdev);
@@ -125,7 +121,7 @@ static uint32_t virtio_snd_handle_jack_info(VirtIOSound *s,
 
     if (iov_size(elem->in_sg, elem->in_num) <
         sizeof(virtio_snd_hdr) + req.count * req.size) {
-        virtio_snd_err("jack info: buffer too small got: %lu needed: %lu\n",
+        error_report("jack info: buffer too small got: %lu needed: %lu\n",
                        iov_size(elem->in_sg, elem->in_num),
                        sizeof(virtio_snd_hdr) + req.count * req.size);
         resp.code = VIRTIO_SND_S_BAD_MSG;
@@ -134,9 +130,10 @@ static uint32_t virtio_snd_handle_jack_info(VirtIOSound *s,
 
     virtio_snd_jack_info *jack_info = g_new0(virtio_snd_jack_info, req.count);
     for (int i = req.start_id; i < req.count + req.start_id; i++) {
+        trace_virtio_snd_handle_jack_info(i);
         virtio_snd_jack *jack = virtio_snd_get_jack(s, i);
         if (!jack) {
-            virtio_snd_err("Invalid jack id: %d\n", i);
+            error_report("Invalid jack id: %d\n", i);
             resp.code = VIRTIO_SND_S_BAD_MSG;
             goto done;
         }
@@ -180,6 +177,7 @@ static uint32_t virtio_snd_handle_jack_remap(VirtIOSound *s,
     virtio_snd_hdr resp;
     resp.code = VIRTIO_SND_S_OK;
 
+    trace_virtio_snd_handle_jack_remap();
     /* TODO: implement remap */
 
     size_t sz;
@@ -202,7 +200,6 @@ static virtio_snd_pcm_stream *virtio_snd_pcm_get_stream(VirtIOSound *s,
                                                         uint32_t stream)
 {
     if (stream >= s->snd_conf.streams) {
-        virtio_snd_err("Invalid stream request %d\n", stream);
         return NULL;
     }
     return s->streams[stream];
@@ -218,7 +215,6 @@ static virtio_snd_pcm_params *virtio_snd_pcm_get_params(VirtIOSound *s,
                                                         uint32_t stream)
 {
     if (stream >= s->snd_conf.streams) {
-        virtio_snd_err("Invalid stream request %d\n", stream);
         return NULL;
     }
     return s->pcm_params[stream];
@@ -243,7 +239,7 @@ static uint32_t virtio_snd_handle_pcm_info(VirtIOSound *s,
     virtio_snd_hdr resp;
     if (iov_size(elem->in_sg, elem->in_num) <
         sizeof(virtio_snd_hdr) + req.size * req.count) {
-        virtio_snd_err("pcm info: buffer too small, got: %lu, needed: %lu\n",
+        error_report("pcm info: buffer too small, got: %lu, needed: %lu\n",
                 iov_size(elem->in_sg, elem->in_num),
                 sizeof(virtio_snd_pcm_info));
         resp.code = VIRTIO_SND_S_BAD_MSG;
@@ -253,10 +249,11 @@ static uint32_t virtio_snd_handle_pcm_info(VirtIOSound *s,
     virtio_snd_pcm_stream *stream;
     virtio_snd_pcm_info *pcm_info = g_new0(virtio_snd_pcm_info, req.count);
     for (int 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) {
-            virtio_snd_err("Invalid stream id: %d\n", i);
+            error_report("Invalid stream id: %d\n", i);
             resp.code = VIRTIO_SND_S_BAD_MSG;
             goto done;
         }
@@ -317,7 +314,7 @@ static uint32_t virtio_snd_pcm_set_params_impl(VirtIOSound *s,
     st_params->period_bytes = params->period_bytes;
 
     if (params->channel < 1 || params->channel > AUDIO_MAX_CHANNELS) {
-        virtio_snd_err("Number of channels not supported\n");
+        error_report("Number of channels not supported\n");
         return VIRTIO_SND_S_NOT_SUPP;
     }
     st_params->channel = params->channel;
@@ -346,13 +343,13 @@ static uint32_t virtio_snd_pcm_set_params_impl(VirtIOSound *s,
                                1 << VIRTIO_SND_PCM_RATE_384000;
 
     if (!(supported_formats & (1 << params->format))) {
-        virtio_snd_err("Stream format not supported\n");
+        error_report("Stream format not supported\n");
         return VIRTIO_SND_S_NOT_SUPP;
     }
     st_params->format = params->format;
 
     if (!(supported_rates & (1 << params->rate))) {
-        virtio_snd_err("Stream rate not supported\n");
+        error_report("Stream rate not supported\n");
         return VIRTIO_SND_S_NOT_SUPP;
     }
     st_params->rate = params->rate;
@@ -378,6 +375,7 @@ static uint32_t virtio_snd_handle_pcm_set_params(VirtIOSound *s,
     sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
     assert(sz == sizeof(virtio_snd_pcm_set_params));
 
+    trace_virtio_snd_handle_pcm_set_params(req.hdr.stream_id);
     virtio_snd_hdr resp;
     resp.code = virtio_snd_pcm_set_params_impl(s, &req);
 
@@ -702,7 +700,6 @@ static void virtio_snd_output_cb(void *opaque, int free)
 
     to_play = MIN(free, pending);
 
-    virtio_snd_log("to_play: %d, free: %d, pending: %d\n", to_play, free, pending);
     while (to_play) {
         int wbytes = write_audio(st, to_play);
 
@@ -715,7 +712,7 @@ static void virtio_snd_output_cb(void *opaque, int free)
         pending -= wbytes;
     }
 
-    virtio_snd_log("written: %d\n", written);
+    trace_virtio_snd_output_cb(to_play, written);
 }
 
 /*
@@ -728,7 +725,6 @@ static void virtio_snd_output_cb(void *opaque, int free)
 static uint32_t virtio_snd_pcm_prepare_impl(VirtIOSound *s, uint32_t stream)
 {
     if (!s->streams || !s->pcm_params || !s->pcm_params[stream]) {
-        virtio_snd_err("Cannot prepare stream %d without params.\n", stream);
         return VIRTIO_SND_S_BAD_MSG;
     }
 
@@ -849,6 +845,7 @@ static uint32_t virtio_snd_handle_pcm_start_stop(VirtIOSound *s,
     virtio_snd_hdr resp;
     resp.code = VIRTIO_SND_S_OK;
 
+    trace_virtio_snd_handle_pcm_stop(req.stream_id);
     virtio_snd_pcm_stream *st = virtio_snd_pcm_get_stream(s, req.stream_id);
     if (st->direction == VIRTIO_SND_D_OUTPUT)
         AUD_set_active_out(st->voice.out, start);
@@ -871,9 +868,6 @@ static uint32_t virtio_snd_pcm_release_impl(virtio_snd_pcm_stream *st, uint32_t
 {
     // if there are still pending io messages do nothing
     if (virtio_snd_pcm_get_pending_bytes(st)) {
-        // flush the stream
-        virtio_snd_log("Started flushing stream");
-
         // set flushing to true, the callback will automatically close the
         // stream once the flushing is done
         st->flushing = true;
@@ -923,11 +917,11 @@ static uint32_t virtio_snd_handle_pcm_release(VirtIOSound *s,
     sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
     assert(sz == sizeof(virtio_snd_pcm_hdr));
 
-    virtio_snd_log("Release called\n");
+    trace_virtio_snd_handle_pcm_release(req.stream_id);
 
     virtio_snd_pcm_stream *st = virtio_snd_pcm_get_stream(s, req.stream_id);
     if (!st) {
-        virtio_snd_err("already released %d\n", req.stream_id);
+        error_report("already released stream %d", req.stream_id);
         return VIRTIO_SND_S_BAD_MSG;
     }
 
@@ -961,7 +955,7 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         }
         if (iov_size(elem->in_sg, elem->in_num) < sizeof(resp) ||
                 iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
-            virtio_snd_err("virtio-snd ctrl missing headers\n");
+            error_report("virtio-snd ctrl missing headers\n");
             virtqueue_detach_element(vq, elem, 0);
             g_free(elem);
             break;
@@ -974,7 +968,7 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         iov_discard_front(&iov, &iov_cnt, sizeof(ctrl));
         if (sz != sizeof(ctrl)) {
             /* error */
-            virtio_snd_err("virtio snd ctrl could not read header\n");
+            error_report("virtio snd ctrl could not read header\n");
             resp.code = VIRTIO_SND_S_BAD_MSG;
         } else if (ctrl.code == VIRTIO_SND_R_JACK_INFO) {
             sz = virtio_snd_handle_jack_info(s, elem);
@@ -993,10 +987,10 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         } else if (ctrl.code == VIRTIO_SND_R_PCM_RELEASE) {
             sz = virtio_snd_handle_pcm_release(s, elem);
         } else if (ctrl.code == VIRTIO_SND_R_CHMAP_INFO) {
-            virtio_snd_log("VIRTIO_SND_R_CHMAP_INFO");
+            trace_virtio_snd_handle_chmap_info();
         } else {
             /* error */
-            virtio_snd_err("virtio snd header not recognized: %d\n", ctrl.code);
+            error_report("virtio snd header not recognized: %d\n", ctrl.code);
             resp.code = VIRTIO_SND_S_BAD_MSG;
         }
 
@@ -1023,8 +1017,6 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 static void virtio_snd_pcm_add_buf(VirtIOSound *s, uint32_t stream,
                                    VirtQueueElement *elem)
 {
-    virtio_snd_log("add_buf called\n");
-
     virtio_snd_pcm_stream *st = virtio_snd_pcm_get_stream(s, stream);
     uint32_t buf_size, dir;
     int i;
@@ -1054,12 +1046,13 @@ static void virtio_snd_pcm_add_buf(VirtIOSound *s, uint32_t stream,
  */
 static void virtio_snd_handle_xfer(VirtIODevice *vdev, VirtQueue *vq)
 {
-    virtio_snd_log("tx/rx queue callback called\n");
     VirtIOSound *s = VIRTIO_SOUND(vdev);
     VirtQueueElement *elem;
     size_t sz;
     virtio_snd_pcm_xfer hdr;
 
+    trace_virtio_snd_handle_xfer();
+
     for (;;) {
         elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
         if (!elem) {
@@ -1089,7 +1082,7 @@ static void virtio_snd_handle_xfer(VirtIODevice *vdev, VirtQueue *vq)
  */
 static void virtio_snd_handle_event(VirtIODevice *vdev, VirtQueue *vq)
 {
-    virtio_snd_log("event queue callback called\n");
+    trace_virtio_snd_handle_event();
 }
 
 /*
@@ -1245,8 +1238,4 @@ static void virtio_register_types(void)
     type_register_static(&virtio_snd_dev_info);
 }
 
-#undef virtio_snd_log
-#undef virtio_snd_warn
-#undef virtio_snd_err
-
 type_init(virtio_register_types)
-- 
2.31.1