Implement VirtIO sound device #53

Cuda-Chen · 2024-08-31T08:17:47Z

Implement VirtIO sound device supporting these operations (the item with checked box checked means it is implemented right now):

For setting up the device:
- VIRTIO_SND_R_JACK_INFO
- VIRTIO_SND_R_PCM_INFO
- VIRTIO_SND_R_CHMAP_INFO
For playing the sound (PCM):
- 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

Test Cases

The test cases are subjected to be altered.

boot up test

test procedures

Execute make check to run semu.
Check kernel message (dmesg).

expected results

The following message should appear while booting up:

[    4.011962] ALSA device list:
[    4.015962]   #0: Loopback 1
[    4.015962]   #1: VirtIO SoundCard at platform/f4400000.virtio/virtio2

check driver configuration

test procedures

Execute aplay -l in emulator.
Check the output messages.

expected results

The following message should appear after Step 1:

$ aplay -l          
**** List of PLAYBACK Hardware Devices ****

<other sound device here>

card 1: SoundCard [VirtIO SoundCard], device 0: virtio-snd [VirtIO PCM 0]
  Subdevices: 1/1
  Subdevice #0: subdevice #0

play sound

test procedures

Execute speaker-test in emulator.
Check the host speaker.

expected results

A white noise will be played by host speaker while speaker-test is executing.

play sequence sound

test procedures

Execute aplay /usr/share/sounds/alsa/Front_Center.wav in emulator.
Check the host speaker.

expected results

A female sound with "Front Center" speaking should be played.

shengwen-tw · 2024-08-31T09:06:12Z

Please provide testing procedures once the change is ready.

CNFA_sf.h

virtio-snd.c

configs/linux.config

os_generic.h

Makefile

configs/linux.config

virtio-snd.c

jserv

Run clang-format before submitting.

.gitmodules

Makefile

jserv

Consult https://github.com/cntools/cnfa/blob/master/.github/workflows/build-cnfa.yml and mention the build dependency in top-level README.md.

configs/linux.config

feature.h

README.md

configs/linux.config

virtio-snd.c

jserv · 2024-12-15T13:29:49Z

device.h

+    uint32_t Status;
+    uint32_t InterruptStatus;
+    /* supplied by environment */
+    uint32_t *ram;


I am not sure ram is proper here. Should it be mem?

I just copy-and-paste from virtio-net.c. Maybe we can make a difference in this commit?

jserv

Move the content of queue.h to utils.h as rv32emu does. Then, enforce the Linux-like naming scheme.

virtio-snd.c

jserv · 2024-12-15T13:36:48Z

virtio-snd.c

+    uint32_t *ram = vsnd->ram;
+    void *priv = vsnd->priv;
+    uint32_t jacks = PRIV(vsnd)->jacks;
+    uint32_t streams = PRIV(vsnd)->streams;
+    uint32_t chmaps = PRIV(vsnd)->chmaps;
+    uint32_t controls = PRIV(vsnd)->controls;
+    memset(vsnd, 0, sizeof(*vsnd));
+    vsnd->ram = ram;
+    vsnd->priv = priv;
+    PRIV(vsnd)->jacks = jacks;
+    PRIV(vsnd)->streams = streams;
+    PRIV(vsnd)->chmaps = chmaps;
+    PRIV(vsnd)->controls = controls;


Can you use memcpy instead?

Hi @jserv , it seems that copying certain members of a struct using dst->foo = src->foo will be the sane way according this StackOverflow answer.

jserv · 2024-12-15T13:38:05Z

virtio-snd.c

+    }
+
+    /* Control the callback to prepare the buffer */
+    /* TODO: add lock to avoid race condition */


You may consolidate the testing for producer-consumer implementation in advance.

queue.h

jserv · 2024-12-17T16:46:17Z

utils.h

+void semu_timer_rebase(semu_timer_t *timer, uint64_t time);
+
+/* Linux-like queue API */


Let's enforce the Linux kernel style naming scheme.

jserv · 2024-12-17T16:48:36Z

virtio-snd.c

+        memcpy(props->ring.buffer + idx, payload, size - idx);
+        memcpy(props->ring.buffer, payload, n - (size - idx));
+    }
+    asm("" ::: "memory");


I create a compiler barrier to make sure the data has to been written to ring buffer thus prevent race condition (see the DPDK counterpart).

If this is necessary, I will suggest wrapping it with macro for readability.

jserv · 2024-12-18T04:45:58Z

For playing the sound (PCM):

VIRTIO_SND_R_PCM_START

VIRTIO_SND_R_PCM_STOP

I can build on macOS with CoreAudio support now. What is missing to complete PCM functionality?

Cuda-Chen · 2024-12-18T04:58:29Z

For playing the sound (PCM):

VIRTIO_SND_R_PCM_START

VIRTIO_SND_R_PCM_STOP

I can build on macOS with CoreAudio support now. What is missing to complete PCM functionality?

Hi @jserv , I list the messing parts for completing PCM functionality:

Clean the allocated resources after issuing VIRTIO_SND_R_PCM_RELEASE: on my Linux testing machine, because of PulseAudio, the guest OS hangs when closing the created ALSA device. However, we can't create ALSA device after disabling PulseAudio.
- Edit: The checked list of VIRTIO_SND_R_PCM_RELEASE is wrong, I am still finding solutions of co-working with PulseAudio.
No sound after VIRTIO_SND_R_PCM_START: I do not hear any sound after issuing VIRTIO_SND_R_PCM_START state on my Linux testing machine.

For testing, I come up with the following two methods and the corresponding expected results:

speaker-test
- expected result: a white noise should be played.
aplay <any audio file>
- expected results: the audio should be played without any artifacts (e.g., distortion, lagging, etc.).

Enable ALSA driver and System V IPC in Linux Kernel. For ALSA, the debug is enabled and will be removed once this commit is ready. Add description of descriptor chaining, yet need refactoring the description as well as code (exists uncertainty of the query struct). Succeed to initialize virtio-snd. Handle requests in control and TX queue, and print the address and length of each virtq element to check the validness of self-implement queue. For macOS, an experimental core audio library check is implemented. As the driver sends the PCM frames asynchronously, use a dedicated thread for receiving frames from driver. The reason that pcm_release state hangs is because of PulseAudio, need to address this later. Add ring buffer struct so that we don't need to allocate space frequently. Can play guest sound. However, the sound only plays a fraction of the beginning.

jserv · 2024-12-27T17:33:55Z

Is it time to review?

Cuda-Chen force-pushed the add-virtio-snd branch from 4a8a6fc to 0902227 Compare August 31, 2024 08:21

jserv requested a review from shengwen-tw August 31, 2024 08:59

jserv reviewed Aug 31, 2024

View reviewed changes

CNFA_sf.h Outdated Show resolved Hide resolved

jserv reviewed Aug 31, 2024

View reviewed changes

virtio-snd.c Outdated Show resolved Hide resolved

Cuda-Chen force-pushed the add-virtio-snd branch from 0902227 to ebba2c7 Compare September 1, 2024 06:05