Migrate virtio-snd from semu - #761
Charlie-Tsai1123 wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
1 issue found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/riscv.c">
<violation number="1" location="src/riscv.c:1077">
P2: When vsnd is enabled, emu_update_vsnd_interrupts (which calls plic_update_interrupts) runs on every rv_step in the main loop, adding per-instruction overhead to the hot loop for the whole run. Throttle the async TX-completion poll (e.g. every N steps or on a time budget) instead of doing it at instruction granularity; the MMIO access path already refreshes interrupts synchronously.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| vrng_delete(attr->vrng); | ||
|
|
||
| #if RV32_HAS(VIRTIO_SND) | ||
| if (attr->vsnd) |
There was a problem hiding this comment.
P2: When vsnd is enabled, emu_update_vsnd_interrupts (which calls plic_update_interrupts) runs on every rv_step in the main loop, adding per-instruction overhead to the hot loop for the whole run. Throttle the async TX-completion poll (e.g. every N steps or on a time budget) instead of doing it at instruction granularity; the MMIO access path already refreshes interrupts synchronously.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/riscv.c, line 1077:
<comment>When vsnd is enabled, emu_update_vsnd_interrupts (which calls plic_update_interrupts) runs on every rv_step in the main loop, adding per-instruction overhead to the hot loop for the whole run. Throttle the async TX-completion poll (e.g. every N steps or on a time budget) instead of doing it at instruction granularity; the MMIO access path already refreshes interrupts synchronously.</comment>
<file context>
@@ -1009,6 +1072,11 @@ riscv_t *rv_create(riscv_user_t rv_attr)
vrng_delete(attr->vrng);
+
+#if RV32_HAS(VIRTIO_SND)
+ if (attr->vsnd)
+ vsnd_delete(attr->vsnd);
+#endif
</file context>
There was a problem hiding this comment.
Benchmarks
Details
| Benchmark suite | Current: d29ac78 | Previous: 0d6ecd1 | Ratio |
|---|---|---|---|
Dhrystone |
2330 DMIPS |
1777.75 DMIPS |
0.76 |
CoreMark |
1438.371 iterations/sec |
1367.387 iterations/sec |
0.95 |
This comment was automatically generated by workflow using github-action-benchmark.
3f4365b to
e849ebf
Compare
|
Thanks for the patch. |
| /* VirtIO sound is available only for kernel system emulation. */ | ||
| #if !RV32_FEATURE_SYSTEM_MMIO | ||
| #undef RV32_FEATURE_VIRTIO_SND | ||
| #define RV32_FEATURE_VIRTIO_SND 0 |
There was a problem hiding this comment.
Silently undefining virtio-snd might lead to unexpected behavior for users who thought they built with sound support. Maybe consider throwing a compile error instead?
Migrate virtio-snd from semu and adapt it to the rv32emu VirtIO device model while preserving the original producer/consumer TX completion behavior. The migrated implementation keeps the semu-style PCM playback flow: PCM payloads from a TX request are copied into a host-side pcm buffer queue, and the producer is paced by readable/writable condition variables. The default VSND_MAX_PENDING_BUFS limit is kept at one, matching semu's buf_ev_notify-based producer/consumer behavior. The TX request is still completed when its PCM data has been accepted into the host-side queue. Therefore, the used-ring entry may be published before the corresponding PCM data has been fully consumed by the PortAudio callback. This behavior is intentionally preserved in this commit as the migration baseline; TX completion mechanism is introduced in a later commit. This commit contains only the virtio-snd device-side implementation. The device is not yet wired into rv32emu system emulation, so sound playback is not available at this stage. System, build, and guest integration are added in the following commit. Co-authored-by: Cuda-Chen <clh960524@gmail.com>
This commit integrates the virtio-snd device migrated in the previous commit into rv32emu system emulation with the following modifications: 1. Add virtio-snd build and feature configuration Introduce CONFIG_VIRTIO_SND and HAVE_PORTAUDIO to control virtio-snd support through Kconfig. Enable virtio-snd only for system emulation with MMIO support and when PortAudio is available on the host. Add the corresponding system defconfig entry, build rules, and RV32_FEATURE_VIRTIO_SND handling. Extend the ENABLE_* compatibility layer so ENABLE_VIRTIO_SND is translated to CONFIG_VIRTIO_SND. Use the same effective build state for source selection, PortAudio linking, and the feature macro to avoid inconsistent configurations. 2. Integrate virtio-snd into system emulation Store the virtio-snd device state in vm_attr_t and initialize device during system creation. Add MMIO_VIRTIOSND routing so guest accesses to the VirtIO sound MMIO region are forwarded to virtio_snd_read() and virtio_snd_write(). The device is also released during system teardown before guest memory is destroyed. 3. Connect virtio-snd interrupts to the PLIC Assign an IRQ to the virtio-snd device and propagate its interrupt status to the PLIC. Interrupt status is read atomically because TX processing may update VirtIO used-ring interrupt from a worker thread. 4. Add the '-x vsnd' system option Introduce the vsnd system device option so virtio-snd can be enabled when launching rv32emu. When enabled, rv32emu creates a virtio-mmio device-tree node for the sound device and assigns the corresponding MMIO address and interrupt. 5. Support coexistence with existing VirtIO devices Update dynamic VirtIO-MMIO allocation path so virtio-snd can coexist with existing devices such as virtio-blk and virtio-rng. 6. Add Linux guest virtio-snd support Enable CONFIG_SND_VIRTIO in the Linux configuration so the guest kernel can bind the VirtIO sound device. Update the Buildroot configuration with the userspace audio utilities required to inspect and exercise the ALSA sound device. 7. Make virtio-snd available to the Linux guest With this integration, the virtio-snd implementation from the previous commit can be instantiated by rv32emu and used by the Linux guest. At this stage, the device still uses the semu-style host PCM buffering and immediate TX completion model. The TX completion path is reworked in the following commit.
This commit reworks virtio-snd TX completion path to keep descriptor ownership aligned with host-side PCM consumption, with the following modifications: 1. Defer TX used-ring completion Rework PCM TX completion semantics. semu uses a single-buffer path and completes TX descriptors once PCM data has been copied into the host-side buffer. When applied to rv32emu, this design resulted in unstable playback, including stuttering and playback stopping before the stream was fully played. In the new implementation, a TX request remains owned by the device until its PCM data has actually been consumed by the audio backend. This also prevents the guest from recycling descriptors ahead of backend consumption. 2. Add a completed-buffer queue and a dedicated completion thread The TX thread only copies PCM data into the pending PCM buffer queue and does not publish the TX used-ring entry immediately. After the PortAudio callback fully consumes a buffer, the request is moved to the completed-buffer queue. The completion thread then updates the PCM status, publishes the TX used-ring entry, and raises the VirtIO interrupt. This provides backpressure based on actual backend consumption. 3. Synchronize asynchronous TX processing Add synchronization for the asynchronous playback and completion path. props->lock.lock protects the pending/completed PCM queues, pending_pcm_bytes, inflight_count, and stream release state. p->tx_mutex and p->tx_cond coordinate QueueNotify events with the TX worker thread, while p->tx_process_mutex serializes TX avail-ring processing and p->tx_used_mutex serializes deferred used-ring updates. Completion condition variables are used to wake the completion thread and allow teardown paths to wait until all device-owned TX requests have been completed. 4. Handle STOP and RELEASE with outstanding TX requests STOP stops the PortAudio stream while preserving pending PCM data so stream can be started again without prematurely discarding device-owned requests. RELEASE prevents new PCM requests from being accepted, processes guest-published TX descriptors, moves remaining pending PCM requests to the completion path, and waits until all outstanding requests have been returned to the guest before releasing stream resources. 5. Avoid blocking the PortAudio callback Do not wait for the PCM queue mutex from the PortAudio callback. If the queue cannot be accessed immediately, or if insufficient PCM data is available, output silence for the unavailable portion and continue the audio stream. This avoids blocking the host real-time audio callback on the TX or completion threads. 6. Account for ALSA period and buffer behavior ALSA --period-size and --buffer-size settings indirectly affect the PCM buffer queue and playback behavior. Each VirtIO PCM TX request corresponds to an ALSA period, while ALSA buffer determines how many periods can remain outstanding. With consumer-side completion, TX descriptor is not returned until its complete period has been consumed. Therefore, the number and size of outstanding periods affect both the amount of queued PCM data and whether ALSA receives a completion before its blocking write timeout. This is especially relevant for small ALSA buffers. For more details about the write timeout, see wait_time for avail in: https://github.com/torvalds/linux/blob/master/sound/core/pcm_lib.c The resulting TX flow becomes: Guest TX descriptor | v TX thread | v pending PCM buffer queue | | PortAudio consumes the complete node v completed-buffer queue | v completion thread | +--> update PCM status +--> publish TX used-ring entry +--> raise VirtIO interrupt This replaces the immediate TX completion model introduced by the migration baseline while keeping the VirtIO sound system integration unchanged.
e849ebf to
0d6ecd1
Compare
Introduce .ci/test-sound.sh to boot a Linux guest with '-x vsnd' and verify basic device integration. The test checks that: - the Linux virtio_snd driver is bound to the VirtIO sound device; - the VirtIO SoundCard is visible through /proc/asound/cards; - an ALSA PCM playback device is enumerated by aplay. The test only verifies device and ALSA enumeration and does not require audible playback from the host audio device. The smoke test is kept separate from the main GitHub Actions workflow for now because the currently published guest Linux image used by CI may not yet contain the virtio-snd driver and ALSA utilities required by the test.
0d6ecd1 to
d29ac78
Compare
|
Thanks for the suggestion. I have split the original large commit into four logical commits to make the review easier:
I also addressed the review comment about silently disabling VirtIO sound by replacing it with a compile-time error for invalid feature combinations. |
Description
This PR migrates the VirtIO sound playback implementation from semu.
In addition to the migration, the PCM TX completion path, buffering, synchronization, PortAudio callback behavior, build configuration, and guest Linux configuration are reworked to provide more stable playback under rv32emu.
PCM TX completion design
The original semu design uses a single-buffer style flow:
guest PCM data is passed to virtio_snd_stream_cb, and the TX request can be completed once the data has been handed to the pcm buffer queue.
In the current rv32emu design, TX requests are first stored in a PCM buffer queue. PortAudio consumes data from that queue, and once a buffer has actually been consumed, it is moved to a complete buffer queue and wake up complete thread. A dedicated completion thread then publishes the used-ring entry.
Using the original semu-style structure directly in rv32emu caused unstable playback. In practice, blocking producer/consumer synchronization can stall the PortAudio callback and make playback timing unstable, which made speaker-test playback unstable. The following video shows the result:
https://youtu.be/T5NduRhwVj0?si=ZevcrT1AYzJYZv-T
Simply extending the PCM buffer queue did make playback smoother. However, it was still insufficient: the TX thread would publish the used ring immediately after pushing PCM data into the PCM buffer queue, even though the host side might not have finished playing it yet.
As a result, the guest could observe the request as completed before the audio had actually finished playing, which could cause playback to stop halfway.
For this reason, rv32emu uses the current two-stage design:
PCM data is first queued for playback, and only after it has actually been consumed is the request moved to the complete buffer queue and finished by the completion thread.
The following video shows the result after modification:
https://youtu.be/hRR-VepwyWI?si=gLwI-OOgesJMAH1m
Play music compare semu to rv32emu:
semu:
https://youtu.be/QIN-FOWNjWU?si=g3vCaIY6EQTpr2jh
rv32emu:
https://youtu.be/hK_aWG_lJHc?si=1LoRCLUhZz62EhhS
PortAudio underrun handling
The PortAudio callback must not wait for the producer when PCM data is temporarily unavailable. eg. at the begining, host side starts to play, however guest side haven't finished preparing pcm yet would cause underrun.
Therefore, I use
memset(output, 0, out_bytes);when pcm buffer queue is empty PortAudio would silent.ALSA period/buffer size and TX completion
large
--period-sizecombined with a small--buffer-sizecan make playback fail with an I/O timeout. eg.buffer-sizedirectly affects the Linux PCM blocking wait timeout.period-sizedoes not directly appear in the timeout calculation. However,period-sizedetermines how much PCM data must be consumed before one VirtIO TX request can be completed and returned through the used ring.In Linux, the default wait timeout used by
wait_for_avail()is:where$N_{\text{buffer}}$ is the ALSA $f_s$ is the sample rate.
buffer-sizein frames andSource:
https://github.com/torvalds/linux/blob/master/sound/core/pcm_lib.c
Therefore, a smaller
buffer-sizeshortens the time Linux waits for more available PCM space, while a largerperiod-sizeincreases the amount of PCM that must be consumed before a TX completion can be generated.When the buffer is full, at least one outstanding period must be fully consumed by the PortAudio callback and its TX completion must be published to the used ring before Linux can reclaim that space.
Synchronization
props->lock.lockpending_pcm_bytes,inflight_count, and release statep->tx_mutex+p->tx_condtx_ev_notifyand TX-thread wakeupp->tx_process_mutexp->tx_used_mutexprops->lock.completedprops->lock.all_completedTest
Kconfig integration
Users can enable or disable the feature through:
Choose
Execution Modesthen user can determine whether enable or disableEnable VirtIO sound device.test virtio-snd
I have modified
linux.configandbuildroot.config. Therefore, build linux image:Host
Guest
ctrl + cto stop speaker-testresult
Play music by virtio-snd and virtio-blk
Check if the music stops halfway.
transfer mp3 to wav
In rv32emu folder
guest
result
result
Problem
CI smoke test and guest Linux image
This PR also provides a VirtIO sound smoke-test script that checks:
The test is currently not wired into
.github/workflows/main.yml.The existing rv32emu system CI downloads the prebuilt Linux-Image artifact. With the current prebuilt guest image, rv32emu successfully exposes the sound device:
but the guest contains only:
and does not contain
virtio_snd. The current prebuilt root filesystem also does not provide the ALSA utilities required by the test.This PR updates:
to enable the VirtIO sound guest driver and provide aplay /speaker-test.
rv32emu already rebuilds and publishes a new Linux-Image after relevant guest configuration changes are merged to master. However, PR CI still uses the previously published artifact, which creates a bootstrap problem:
the sound smoke test cannot pass until the new guest artifact exists. I would appreciate the maintainers' preference on how this test should be integrated into PR CI.
There is also an ongoing CI refactoring in:
#638
In particular:
bda4649
This turns .ci/boot-linux.sh into a driver for individual Linux test scripts. If that approach is merged, the VirtIO sound smoke test could be integrated through the same mechanism instead of adding a separate GitHub Actions step.
Summary by cubic
Migrates VirtIO sound playback from semu and makes it an optional system device enabled with -x vsnd. TX completion now occurs only after PortAudio consumes PCM (was: on queue push), adding backpressure, fixing stutter, and outputting silence on underrun.
Device: one playback stream (mono S16) with standard rates; enable with -x vsnd. MMIO, DT, and PLIC IRQ are assigned dynamically alongside
vblk/vrng.Path: TX worker processes avail-ring; PortAudio callback does not block and fills silence if empty; a completion thread updates used-ring entries and raises interrupts. STOP/RELEASE drain pending buffers and complete requests before teardown.
Build/config: new
CONFIG_VIRTIO_SND(default in system defconfig), only for native system emulation (notELF_LOADER/WASM). Gated byHAVE_PORTAUDIOviapkg-configdetection ofportaudio-2.0;virtio-snd.oand PortAudio link flags are excluded when disabled; definesRV32_FEATURE_VIRTIO_SND.CLI/DTB: adds
-x vsnd; device-tree node and IRQ are allocated next to other VirtIO devices.Guest/CI/docs: Linux enables sound core and
CONFIG_SND_VIRTIO; Buildroot adds ALSA tools. Adds docs at docs/sound.md and a headless smoke test.ci/test-sound.sh(driver bind and ALSA enumeration).Migration actions:
portaudiodev headers andpkg-config(packageportaudio-2.0) before building withCONFIG_VIRTIO_SND=y.virtio_sndand ALSA tools.Written for commit d29ac78. Summary will update on new commits.