* [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization
@ 2026-10-29 5:43 Joonho Lee
2026-10-29 5:43 ` [PATCH v8 01/14] ALSA: virtio: Propagate PCM send failures through ACK Joonho Lee
` (13 more replies)
0 siblings, 14 replies; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc
Hi Takashi,
This is a local draft of the next complete revision of the staged capture
series, based on sound.git for-next at
a4311e9447bbb219044db38897f95be112f46a8a.
It supersedes the submitted v7 series and includes all of its unmerged
prerequisites. Please apply this series directly to the base, rather than
on top of v7. Targeting 7.5 or later still works for me.
I consolidated the staged capture implementation and its follow-up fixes
into patch 9. The request-lifetime and ACK changes remain separate, as do
the notification priority change and the three RESET changes at the end.
The capture compatibility options remain opt-in and disabled by default.
Series layout
1-6: ACK errors, START recovery and request/DMA lifetime.
7: Mark core stop-threshold accesses used by capture policy sampling.
8: Send STOP and synchronize teardown when leaving PAUSED.
9: Staged capture, dsnoop policy and lifecycle recovery.
10: Staged capture period-work priority.
11: Capture compatibility option documentation.
12: Optional PCM reset operation under the stream lock.
13: Staged capture RESET accounting.
14: Ordinary indirect capture RESET accounting.
Patch 8 extends the earlier paused-stop change to invoke the driver's
STOP trigger. Marking stop_operating alone left the device paused while
subsequent teardown expected it to have stopped. This is a core behavior
change affecting drivers beyond virtio-snd.
Patch 12 retains the common pointer update, time accounting and XRUN path
in snd_pcm_lib_reset(). The optional operation runs with the PCM stream
lock held through core position and appl_ptr completion. Drivers without
it keep their existing ioctl RESET path. The same operation runs after a
successful prepare.
A successful callback member completes its reset before its lock is
released. If a later linked member fails, the first error is returned;
there is no group rollback. Earlier members can already be reset, and a
failing callback may have changed state. Mixed legacy/callback groups
retain the legacy members' existing post_action policy. Applications must
recover the group before relying on its positions after an error.
Patch 13 preserves RX completions arriving after the common pointer
observation. A refill failure finishes the core rebase and then reports
XRUN through period work. Patch 14 fixes ordinary capture accounting after
RESET, including MMAP capture. It discards the requested unread samples
while preserving the device's ring offsets. A refill failure returns an
error and stops transfers, requiring drop/prepare or reopen recovery.
Validation
The full ARM64 Image and modules were built from the new base with KASAN,
lockdep and W=1. Core and virtio-snd objects also built at every one of the
14 intermediate stages. ARM32 LLVM core/virtio-snd object builds passed;
ARM32 was not booted.
The new-base QEMU runs covered normal and staged capture, playback, policy
changes, linked RESET/prepare partial failures, delayed control replies,
failed prepare, RELEASE retry, suspend and virtqueue reset recovery.
The linked synthetic fixtures cover atomic and nonatomic streams with
all callback combinations, including errors after pointer changes and
later-member prepare failure.
MMAP_INTERLEAVED capture was checked at 48 kHz, mono S16_LE with both
RUNNING and PAUSED RESET, managed and free-running stop thresholds, XRUN
recovery and reopen. Seven VM runs produced 21 capture segments and
8,016,000 frames matching the deterministic sample oracle, including two
70-second one-vCPU recordings. Fault hooks are confined to separate test
builds.
The two earlier TCG failures were followed by 22 comparison runs using
the previous and current kernel bases. Both used two vCPUs, 512 MiB, the
same QEMU and rootfs, four dsnoop clients at 16, 44.1, 48 and 96 kHz,
and a 48 kHz mono slave with a 160 ms buffer. Kernel configurations
differed only in LOCALVERSION. The main and initialization kernel text
ranges and the executable/text-relocation sections of virtio_snd, snd-
pcm and snd were identical.
In alternating 70-second TCG runs with two CPU busy loops, the previous
base failed three of four runs and the current base failed all four.
Lowering only the busy loops to nice 19 still failed both runs on each
base. Host load varied substantially and was not controlled. These
counts alone do not establish a performance regression.
The diagnostics distinguish two paths. A client can exceed its dsnoop
stop_threshold and receive -EPIPE while the hardware keeps running. With
more severe delay, the driver rejects completion when capture progress
since the last PCM pointer observation reaches a full ring
(pointer_window). dsnoop can recover the slave before reporting the
error to a client, so a later RUNNING state does not rule out a
preceding driver XRUN.
An instrumented 70-second TCG run on the previous base hit
client_threshold at 7680 or 8640 unread frames against a 7680-frame
limit. The current base completed all four rates in the corresponding
run, totaling 14,287,000 frames. In HVF controls both bases completed 20
seconds. Delaying only the 96 kHz reader by 240 ms produced
client_threshold XRUN on both bases while the other three readers
completed their requested frames.
These results support delayed consumption and pointer observation under
CPU contention as the observed failure conditions. They provide no
evidence of an executable-code performance regression in this
configuration, but do not establish identical performance under all host
loads. The original two failures had no branch-level instrumentation;
their exact return sites cannot be established retrospectively. All
failed runs are retained. The pointer-window protection and buffer sizes
were left unchanged.
All 22 runs completed input-integrity and module-hash checks without
KASAN or lockdep diagnostics. Four runs separately recorded
pointer_window warnings. The instrumented and uninstrumented fixtures
are reported separately because instrumentation can affect timing. This
is not a lossless-capture guarantee for overloaded TCG guests.
Scope and remaining review
This revision has only been run in QEMU. VZ/MOTU acceptance is still
pending. Other MMAP layouts, sample formats, multichannel capture and
concurrent RESET between mmap_begin and mmap_commit were not covered.
The new callback's linked partial-failure contract and the general
paused-STOP behavior need ALSA core maintainer review. NOMMU and 32-bit
runtime behavior are not established by the ARM64 tests.
The public Sashiko review criteria were used for a local audit. This
revision has not been reviewed by the Sashiko service. Physical-device
acceptance and ALSA core review remain outstanding.
Thanks,
Joonho
base-commit: a4311e9447bbb219044db38897f95be112f46a8a
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v8 01/14] ALSA: virtio: Propagate PCM send failures through ACK
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
@ 2026-10-29 5:43 ` Joonho Lee
2026-10-29 6:06 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 02/14] ALSA: virtio: Recover PCM state after a failed start Joonho Lee
` (12 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
virtsnd_pcm_msg_send() can fail while queueing a transfer. Restore the
failed message's length and notify the device about earlier messages
queued by the same call, so they do not remain stranded.
Propagate that failure through the indirect PCM transfer callback instead
of discarding it. Commit the updated indirect state only after successful
submission and stop further transfers when a send fails. Return an XRUN
error to ALSA so the stream can recover.
Keep the polling feature path unchanged.
Fixes: fe981e67568c ("ALSA: virtio: use ack callback")
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c
index 72103f606716..5371f4985fd0 100644
--- a/sound/virtio/virtio_pcm_msg.c
+++ b/sound/virtio/virtio_pcm_msg.c
@@ -260,6 +260,18 @@ int virtsnd_pcm_msg_send(struct virtio_pcm_substream *vss, unsigned long offset,
dev_err(&vdev->dev,
"SID %u: failed to send I/O message\n",
vss->sid);
+ msg->length -= n;
+
+ /*
+ * Do not leave earlier messages queued without notifying
+ * the device when a later message could not be added.
+ */
+ if (msg_count != vss->msg_count &&
+ !(vss->features &
+ (1U << VIRTIO_SND_PCM_F_MSG_POLLING)) &&
+ virtqueue_kick_prepare(vqueue))
+ virtqueue_notify(vqueue);
+
return rc;
}
diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
index 1091100a4c98..a40ea394bf17 100644
--- a/sound/virtio/virtio_pcm_ops.c
+++ b/sound/virtio/virtio_pcm_ops.c
@@ -466,36 +466,100 @@ virtsnd_pcm_cp_pointer(struct snd_pcm_substream *substream)
vss->hw_ptr);
}
+struct virtsnd_pcm_transfer {
+ struct snd_pcm_indirect indirect;
+ int error;
+};
+
static void virtsnd_pcm_trans_copy(struct snd_pcm_substream *substream,
struct snd_pcm_indirect *rec, size_t bytes)
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+ struct virtsnd_pcm_transfer *transfer =
+ container_of(rec, struct virtsnd_pcm_transfer, indirect);
+
+ if (transfer->error)
+ return;
- virtsnd_pcm_msg_send(vss, rec->sw_data, bytes);
+ transfer->error = virtsnd_pcm_msg_send(vss, rec->sw_data, bytes);
+}
+
+/* Caller holds vss->lock. */
+static void virtsnd_pcm_transfer_failed(struct virtio_pcm_substream *vss)
+{
+ vss->xfer_enabled = false;
+ vss->stopped = true;
+ vss->substream->runtime->stop_operating = true;
}
static int virtsnd_pcm_pb_ack(struct snd_pcm_substream *substream)
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss);
+ struct virtsnd_pcm_transfer transfer = {};
+ int rc;
- guard(spinlock_irqsave)(&queue->lock);
- guard(spinlock)(&vss->lock);
+ scoped_guard(spinlock_irqsave, &queue->lock) {
+ guard(spinlock)(&vss->lock);
+ transfer.indirect = vss->pcm_indirect;
+ if (vss->stopped) {
+ rc = -EPIPE;
+ } else {
+ rc = snd_pcm_indirect_playback_transfer(substream,
+ &transfer.indirect,
+ virtsnd_pcm_trans_copy);
+ }
+ if (!rc)
+ rc = transfer.error;
+ if (rc)
+ virtsnd_pcm_transfer_failed(vss);
+ else
+ vss->pcm_indirect = transfer.indirect;
+ }
+ if (rc) {
+ dev_err(&vss->snd->vdev->dev,
+ "SID %u: playback ACK transfer failed: %d\n",
+ vss->sid, rc);
+ /* ALSA treats -EPIPE as an XRUN in ACK and pointer paths. */
+ return -EPIPE;
+ }
- return snd_pcm_indirect_playback_transfer(substream, &vss->pcm_indirect,
- virtsnd_pcm_trans_copy);
+ return 0;
}
static int virtsnd_pcm_cp_ack(struct snd_pcm_substream *substream)
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss);
+ struct virtsnd_pcm_transfer transfer = {};
+ int rc;
- guard(spinlock_irqsave)(&queue->lock);
- guard(spinlock)(&vss->lock);
+ scoped_guard(spinlock_irqsave, &queue->lock) {
+ guard(spinlock)(&vss->lock);
+ transfer.indirect = vss->pcm_indirect;
+ if (vss->stopped) {
+ rc = -EPIPE;
+ } else {
+ rc = snd_pcm_indirect_capture_transfer(substream,
+ &transfer.indirect,
+ virtsnd_pcm_trans_copy);
+ }
+ if (!rc)
+ rc = transfer.error;
+ if (rc)
+ virtsnd_pcm_transfer_failed(vss);
+ else
+ vss->pcm_indirect = transfer.indirect;
+ }
+ if (rc) {
+ dev_err(&vss->snd->vdev->dev,
+ "SID %u: capture ACK transfer failed: %d\n",
+ vss->sid, rc);
+ /* ALSA treats -EPIPE as an XRUN in ACK and pointer paths. */
+ return -EPIPE;
+ }
- return snd_pcm_indirect_capture_transfer(substream, &vss->pcm_indirect,
- virtsnd_pcm_trans_copy);
+ return 0;
}
/* PCM substream operators map. */
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 02/14] ALSA: virtio: Recover PCM state after a failed start
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
2026-10-29 5:43 ` [PATCH v8 01/14] ALSA: virtio: Propagate PCM send failures through ACK Joonho Lee
@ 2026-10-29 5:43 ` Joonho Lee
2026-10-29 6:10 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 03/14] ALSA: virtio: Retain queued messages through device reset Joonho Lee
` (11 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
Capture trigger queues I/O messages before PCM_START. If the START
message cannot be allocated or sent, transfers may remain queued, and
ALSA does not invoke the action undo callback for the failed trigger.
Mark the stream stopped and disable transfers for every failed START
path. Arm runtime->stop_operating so sync_stop releases pending I/O and
prevents prepare, hw_params, or START retries until release succeeds.
The stopped flag is shared with ACK callbacks that can run without the
ALSA stream lock. Protect every stopped access with vss->lock. Read the
pending message count under the same lock in open and hw_params. In
sync_stop, decide whether RELEASE is needed and mark the stream stopped
in one locked transition; leave it stopped when release or draining fails.
Always cancel elapsed-period work in sync_stop, even when no RELEASE
is needed.
Fixes: da76e9f3e43a ("ALSA: virtio: PCM substream operators")
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
diff --git a/sound/virtio/virtio_pcm.h b/sound/virtio/virtio_pcm.h
index 8837dfc12828..8048b14e6c80 100644
--- a/sound/virtio/virtio_pcm.h
+++ b/sound/virtio/virtio_pcm.h
@@ -26,13 +26,13 @@ struct virtio_pcm_msg;
* @hw: Kernel ALSA substream hardware descriptor.
* @elapsed_period: Kernel work to handle the elapsed period state.
* @lock: Spinlock that protects fields shared by interrupt handlers and
- * substream operators.
+ * substream operators, including @stopped.
* @buffer_bytes: Current buffer size in bytes.
* @hw_ptr: Substream hardware pointer value in bytes [0 ... buffer_bytes).
* @xfer_enabled: Data transfer state (0 - off, 1 - on).
* @xfer_xrun: Data underflow/overflow state (0 - no xrun, 1 - xrun).
* @stopped: True if the substream is stopped and must be released on the device
- * side.
+ * side; protected by @lock.
* @suspended: True if the substream is suspended and must be reconfigured on
* the device side at resume.
* @msgs: Allocated I/O messages.
diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
index a40ea394bf17..970499fe419b 100644
--- a/sound/virtio/virtio_pcm_ops.c
+++ b/sound/virtio/virtio_pcm_ops.c
@@ -96,6 +96,15 @@ static const struct virtsnd_a2v_rate g_a2v_rate_map[] = {
static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream);
+static void virtsnd_pcm_start_failed(struct virtio_pcm_substream *vss)
+{
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ vss->xfer_enabled = false;
+ vss->stopped = true;
+ }
+ vss->substream->runtime->stop_operating = true;
+}
+
/**
* virtsnd_pcm_open() - Open the PCM substream.
* @substream: Kernel ALSA substream.
@@ -115,7 +124,8 @@ static int virtsnd_pcm_open(struct snd_pcm_substream *substream)
snd_pcm_hw_constraint_integer(substream->runtime,
SNDRV_PCM_HW_PARAM_PERIODS);
- vss->stopped = !!virtsnd_pcm_msg_pending_num(vss);
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ vss->stopped |= !!vss->msg_count;
vss->suspended = false;
/*
@@ -217,13 +227,25 @@ static int virtsnd_pcm_hw_params(struct snd_pcm_substream *substream,
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
struct virtio_device *vdev = vss->snd->vdev;
+ unsigned int msg_count;
+ bool stopped;
int rc;
- if (virtsnd_pcm_msg_pending_num(vss)) {
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ msg_count = vss->msg_count;
+ stopped = vss->stopped;
+ }
+ if (msg_count) {
dev_err(&vdev->dev, "SID %u: invalid I/O queue state\n",
vss->sid);
return -EBADFD;
}
+ if (stopped) {
+ dev_err(&vdev->dev, "SID %u: stream release failed\n",
+ vss->sid);
+ substream->runtime->stop_operating = true;
+ return -EBADFD;
+ }
rc = virtsnd_pcm_dev_set_params(vss, params_buffer_bytes(hw_params),
params_period_bytes(hw_params),
@@ -273,6 +295,16 @@ static int virtsnd_pcm_prepare(struct snd_pcm_substream *substream)
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
struct virtio_device *vdev = vss->snd->vdev;
struct virtio_snd_msg *msg;
+ bool stopped;
+
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ stopped = vss->stopped;
+ if (stopped) {
+ dev_err(&vdev->dev, "SID %u: stream release failed\n",
+ vss->sid);
+ substream->runtime->stop_operating = true;
+ return -EBADFD;
+ }
if (!vss->suspended) {
if (virtsnd_pcm_msg_pending_num(vss)) {
@@ -338,31 +370,40 @@ static int virtsnd_pcm_trigger(struct snd_pcm_substream *substream, int command)
scoped_guard(spinlock_irqsave, &queue->lock) {
guard(spinlock)(&vss->lock);
+ if (vss->stopped) {
+ substream->runtime->stop_operating = true;
+ return -EBADFD;
+ }
if (vss->direction == SNDRV_PCM_STREAM_CAPTURE)
rc = virtsnd_pcm_msg_send(vss, 0, vss->buffer_bytes);
- if (rc)
+ if (rc) {
+ vss->stopped = true;
+ substream->runtime->stop_operating = true;
return rc;
+ }
vss->xfer_enabled = true;
}
msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_START,
GFP_KERNEL);
if (!msg) {
- guard(spinlock_irqsave)(&vss->lock);
- vss->xfer_enabled = false;
-
+ virtsnd_pcm_start_failed(vss);
return -ENOMEM;
}
- return virtsnd_ctl_msg_send_sync(snd, msg);
+ rc = virtsnd_ctl_msg_send_sync(snd, msg);
+ if (rc)
+ virtsnd_pcm_start_failed(vss);
+
+ return rc;
case SNDRV_PCM_TRIGGER_SUSPEND:
vss->suspended = true;
fallthrough;
case SNDRV_PCM_TRIGGER_STOP:
- vss->stopped = true;
- fallthrough;
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (command != SNDRV_PCM_TRIGGER_PAUSE_PUSH)
+ vss->stopped = true;
vss->xfer_enabled = false;
}
@@ -393,21 +434,31 @@ static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream)
struct virtio_snd *snd = vss->snd;
struct virtio_snd_msg *msg;
unsigned int js = msecs_to_jiffies(virtsnd_msg_timeout_ms);
+ bool need_release;
int rc;
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ vss->xfer_enabled = false;
+ need_release = vss->stopped || vss->msg_count;
+ if (need_release)
+ vss->stopped = true;
+ }
cancel_work_sync(&vss->elapsed_period);
-
- if (!vss->stopped)
+ if (!need_release)
return 0;
msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_RELEASE,
GFP_KERNEL);
- if (!msg)
+ if (!msg) {
+ substream->runtime->stop_operating = true;
return -ENOMEM;
+ }
rc = virtsnd_ctl_msg_send_sync(snd, msg);
- if (rc)
+ if (rc) {
+ substream->runtime->stop_operating = true;
return rc;
+ }
/*
* The spec states that upon receipt of the RELEASE command "the device
@@ -421,11 +472,13 @@ static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream)
if (rc <= 0) {
dev_warn(&snd->vdev->dev, "SID %u: failed to flush I/O queue\n",
vss->sid);
+ substream->runtime->stop_operating = true;
return !rc ? -ETIMEDOUT : rc;
}
- vss->stopped = false;
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ vss->stopped = false;
return 0;
}
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 03/14] ALSA: virtio: Retain queued messages through device reset
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
2026-10-29 5:43 ` [PATCH v8 01/14] ALSA: virtio: Propagate PCM send failures through ACK Joonho Lee
2026-10-29 5:43 ` [PATCH v8 02/14] ALSA: virtio: Recover PCM state after a failed start Joonho Lee
@ 2026-10-29 5:43 ` Joonho Lee
2026-10-29 6:14 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 04/14] ALSA: virtio: Keep PCM DMA buffers alive through device ownership Joonho Lee
` (10 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
Keep the control queue available through snd_card_free() so close-time
STOP and RELEASE commands can still be submitted. Block new control
requests and mark pending messages canceled before reset, then release
their queue references only after reset and queue deletion.
For PCM queues, detach each remaining virtqueue buffer after the device
has stopped DMA and retire its message without reporting a normal
completion. Detach all queues before deleting them so outstanding DMA
mappings are unmapped and no token remains for a late device completion.
Fixes: 9d45e514da88 ("ALSA: virtio: handling control messages")
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
index 6f35276416fe..df7091b74a52 100644
--- a/sound/virtio/virtio_card.c
+++ b/sound/virtio/virtio_card.c
@@ -337,6 +337,38 @@ static int virtsnd_probe(struct virtio_device *vdev)
return rc;
}
+/**
+ * virtsnd_reset_vqs() - Stop device access and unmap outstanding buffers.
+ * @snd: VirtIO sound device.
+ *
+ * Context: Any context that permits to sleep.
+ */
+static void virtsnd_reset_vqs(struct virtio_snd *snd)
+{
+ struct virtio_device *vdev = snd->vdev;
+ unsigned int i;
+
+ virtio_reset_device(vdev);
+
+ /* Deleting a virtqueue does not unmap its outstanding descriptors. */
+ for (i = 0; i < VIRTIO_SND_VQ_MAX; ++i) {
+ struct virtio_snd_queue *queue = &snd->queues[i];
+ unsigned long flags;
+ void *msg;
+
+ spin_lock_irqsave(&queue->lock, flags);
+ if (queue->vqueue)
+ while ((msg = virtqueue_detach_unused_buf(queue->vqueue)))
+ if (i == VIRTIO_SND_VQ_TX || i == VIRTIO_SND_VQ_RX)
+ virtsnd_pcm_msg_cancel(msg);
+ spin_unlock_irqrestore(&queue->lock, flags);
+ }
+
+ vdev->config->del_vqs(vdev);
+ for (i = 0; i < VIRTIO_SND_VQ_MAX; ++i)
+ snd->queues[i].vqueue = NULL;
+}
+
/**
* virtsnd_remove() - Remove VirtIO and ALSA devices.
* @vdev: VirtIO parent device.
@@ -349,13 +381,14 @@ static void virtsnd_remove(struct virtio_device *vdev)
unsigned int i;
virtsnd_disable_event_vq(snd);
- virtsnd_ctl_msg_cancel_all(snd);
if (snd->card)
snd_card_free(snd->card);
- virtio_reset_device(vdev);
- vdev->config->del_vqs(vdev);
+ virtsnd_ctl_msg_cancel_all(snd);
+
+ virtsnd_reset_vqs(snd);
+ virtsnd_ctl_msg_cleanup(snd);
for (i = 0; snd->substreams && i < snd->nsubstreams; ++i) {
struct virtio_pcm_substream *vss = &snd->substreams[i];
@@ -383,8 +416,8 @@ static int virtsnd_freeze(struct virtio_device *vdev)
virtsnd_disable_event_vq(snd);
virtsnd_ctl_msg_cancel_all(snd);
- virtio_reset_device(vdev);
- vdev->config->del_vqs(vdev);
+ virtsnd_reset_vqs(snd);
+ virtsnd_ctl_msg_cleanup(snd);
for (i = 0; i < snd->nsubstreams; ++i)
cancel_work_sync(&snd->substreams[i].elapsed_period);
@@ -412,6 +445,7 @@ static int virtsnd_restore(struct virtio_device *vdev)
return rc;
virtio_device_ready(vdev);
+ virtsnd_ctl_msg_enable(snd);
virtsnd_enable_event_vq(snd);
diff --git a/sound/virtio/virtio_card.h b/sound/virtio/virtio_card.h
index 3ceee4e416fc..bd065bd00d25 100644
--- a/sound/virtio/virtio_card.h
+++ b/sound/virtio/virtio_card.h
@@ -47,6 +47,7 @@ struct virtio_kctl {
* @queues: Virtqueue wrappers.
* @card: ALSA sound card.
* @ctl_msgs: Pending control request list.
+ * @ctl_msgs_blocked: Control requests are blocked during teardown or suspend.
* @event_msgs: Device events.
* @pcm_list: VirtIO PCM device list.
* @jacks: VirtIO jacks.
@@ -64,6 +65,7 @@ struct virtio_snd {
struct virtio_snd_queue queues[VIRTIO_SND_VQ_MAX];
struct snd_card *card;
struct list_head ctl_msgs;
+ bool ctl_msgs_blocked;
struct virtio_snd_event *event_msgs;
struct list_head pcm_list;
struct virtio_jack *jacks;
diff --git a/sound/virtio/virtio_ctl_msg.c b/sound/virtio/virtio_ctl_msg.c
index 6433c870f271..7114bea4033a 100644
--- a/sound/virtio/virtio_ctl_msg.c
+++ b/sound/virtio/virtio_ctl_msg.c
@@ -15,6 +15,7 @@
* @list: Pending message list entry.
* @notify: Request completed notification.
* @ref_count: Reference count used to manage a message lifetime.
+ * @cancelled: Request canceled during device teardown.
*/
struct virtio_snd_msg {
struct scatterlist sg_request;
@@ -22,6 +23,7 @@ struct virtio_snd_msg {
struct list_head list;
struct completion notify;
refcount_t ref_count;
+ bool cancelled;
};
/**
@@ -100,7 +102,7 @@ struct virtio_snd_msg *virtsnd_ctl_msg_alloc(size_t request_size,
INIT_LIST_HEAD(&msg->list);
init_completion(&msg->notify);
- /* This reference is dropped in virtsnd_ctl_msg_complete(). */
+ /* The caller owns this reference; send adds a queue reference. */
refcount_set(&msg->ref_count, 1);
return msg;
@@ -134,6 +136,7 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
int rc;
virtsnd_ctl_msg_ref(msg);
+ /* This reference stays with the queue until completion or teardown. */
/* Set the default status in case the message was canceled. */
response->code = cpu_to_le32(VIRTIO_SND_S_IO_ERR);
@@ -147,12 +150,17 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
psgs[nouts + nins++] = in_sgs;
scoped_guard(spinlock_irqsave, &queue->lock) {
- rc = virtqueue_add_sgs(queue->vqueue, psgs, nouts, nins, msg,
- GFP_ATOMIC);
+ if (snd->ctl_msgs_blocked)
+ rc = -ENODEV;
+ else
+ rc = virtqueue_add_sgs(queue->vqueue, psgs, nouts, nins,
+ msg, GFP_ATOMIC);
if (!rc) {
notify = virtqueue_kick_prepare(queue->vqueue);
list_add_tail(&msg->list, &snd->ctl_msgs);
+ if (notify)
+ virtqueue_notify(queue->vqueue);
}
}
@@ -169,9 +177,6 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
goto on_exit;
}
- if (notify)
- virtqueue_notify(queue->vqueue);
-
if (nowait)
goto on_exit;
@@ -187,6 +192,11 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
goto on_exit;
}
+ if (READ_ONCE(msg->cancelled)) {
+ rc = -ENODEV;
+ goto on_exit;
+ }
+
switch (le32_to_cpu(response->code)) {
case VIRTIO_SND_S_OK:
rc = 0;
@@ -227,9 +237,33 @@ void virtsnd_ctl_msg_complete(struct virtio_snd_msg *msg)
* virtsnd_ctl_msg_cancel_all() - Cancel all pending control messages.
* @snd: VirtIO sound device.
*
- * Context: Any context.
+ * Context: Any context. Blocks new senders and marks pending requests canceled.
+ * Existing senders are woken when their request completes or after
+ * the device has been reset.
*/
void virtsnd_ctl_msg_cancel_all(struct virtio_snd *snd)
+{
+ struct virtio_snd_queue *queue = virtsnd_control_queue(snd);
+ struct virtio_snd_msg *msg;
+
+ guard(spinlock_irqsave)(&queue->lock);
+ snd->ctl_msgs_blocked = true;
+ list_for_each_entry(msg, &snd->ctl_msgs, list) {
+ WRITE_ONCE(msg->cancelled, true);
+ }
+}
+
+/**
+ * virtsnd_ctl_msg_cleanup() - Release messages left after queue teardown.
+ * @snd: VirtIO sound device.
+ *
+ * This must be called after device reset, buffer detachment, and del_vqs(),
+ * when no queue callback or DMA access can still reach a message. The queue
+ * object and its lock must remain alive until this function returns.
+ *
+ * Context: Any context.
+ */
+void virtsnd_ctl_msg_cleanup(struct virtio_snd *snd)
{
struct virtio_snd_queue *queue = virtsnd_control_queue(snd);
@@ -243,6 +277,20 @@ void virtsnd_ctl_msg_cancel_all(struct virtio_snd *snd)
}
}
+/**
+ * virtsnd_ctl_msg_enable() - Allow control requests after queue restoration.
+ * @snd: VirtIO sound device.
+ *
+ * Context: Any context.
+ */
+void virtsnd_ctl_msg_enable(struct virtio_snd *snd)
+{
+ struct virtio_snd_queue *queue = virtsnd_control_queue(snd);
+
+ guard(spinlock_irqsave)(&queue->lock);
+ snd->ctl_msgs_blocked = false;
+}
+
/**
* virtsnd_ctl_query_info() - Query the item configuration from the device.
* @snd: VirtIO sound device.
diff --git a/sound/virtio/virtio_ctl_msg.h b/sound/virtio/virtio_ctl_msg.h
index 7f4db044f28e..d917435409d0 100644
--- a/sound/virtio/virtio_ctl_msg.h
+++ b/sound/virtio/virtio_ctl_msg.h
@@ -68,6 +68,10 @@ static inline int virtsnd_ctl_msg_send_async(struct virtio_snd *snd,
void virtsnd_ctl_msg_cancel_all(struct virtio_snd *snd);
+void virtsnd_ctl_msg_cleanup(struct virtio_snd *snd);
+
+void virtsnd_ctl_msg_enable(struct virtio_snd *snd);
+
void virtsnd_ctl_msg_complete(struct virtio_snd_msg *msg);
int virtsnd_ctl_query_info(struct virtio_snd *snd, int command, int start_id,
diff --git a/sound/virtio/virtio_pcm.h b/sound/virtio/virtio_pcm.h
index 8048b14e6c80..e31f090a0124 100644
--- a/sound/virtio/virtio_pcm.h
+++ b/sound/virtio/virtio_pcm.h
@@ -120,6 +120,8 @@ void virtsnd_pcm_msg_free(struct virtio_pcm_substream *vss);
void virtsnd_pcm_msg_reset(struct virtio_pcm_substream *vss);
+void virtsnd_pcm_msg_cancel(struct virtio_pcm_msg *msg);
+
int virtsnd_pcm_msg_send(struct virtio_pcm_substream *vss, unsigned long offset,
unsigned long bytes);
diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c
index 5371f4985fd0..762480363b86 100644
--- a/sound/virtio/virtio_pcm_msg.c
+++ b/sound/virtio/virtio_pcm_msg.c
@@ -307,6 +307,35 @@ unsigned int virtsnd_pcm_msg_pending_num(struct virtio_pcm_substream *vss)
return vss->msg_count;
}
+/**
+ * virtsnd_pcm_msg_cancel() - Cancel a message detached during device reset.
+ * @msg: Message returned by virtqueue_detach_unused_buf().
+ *
+ * Reset cancellation retires the queue token without treating it as a device
+ * completion. In particular, it does not advance the PCM hardware pointer or
+ * schedule period work.
+ *
+ * Context: Process context with the PCM queue lock held.
+ */
+void virtsnd_pcm_msg_cancel(struct virtio_pcm_msg *msg)
+{
+ struct virtio_pcm_substream *vss = msg->substream;
+ bool wake;
+
+ {
+ guard(spinlock)(&vss->lock);
+ msg->length = 0;
+ vss->xfer_enabled = false;
+ if (WARN_ON_ONCE(!vss->msg_count))
+ return;
+ vss->msg_count--;
+ wake = !vss->msg_count;
+ }
+
+ if (wake)
+ wake_up_all(&vss->msg_empty);
+}
+
/**
* virtsnd_pcm_msg_complete() - Complete an I/O message.
* @msg: I/O message.
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 04/14] ALSA: virtio: Keep PCM DMA buffers alive through device ownership
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (2 preceding siblings ...)
2026-10-29 5:43 ` [PATCH v8 03/14] ALSA: virtio: Retain queued messages through device reset Joonho Lee
@ 2026-10-29 5:43 ` Joonho Lee
2026-10-29 6:07 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 05/14] ALSA: virtio: Keep control payloads alive through DMA completion Joonho Lee
` (9 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
Retain references to the pages backing ALSA's vmalloc buffer until its
queued I/O messages are freed. On NOMMU systems, use driver-owned memory
for DMA and copy playback data before submission and capture data after
completion, so ALSA buffer release cannot invalidate an active request.
Validate the PCM buffer and each period before building scatterlists.
Clamp device-reported capture lengths to the message capacity before
advancing the hardware pointer, preventing an oversized completion from
accounting bytes beyond the buffer actually submitted.
Fixes: f40a28679e0b ("ALSA: virtio: handling control and I/O messages for the PCM device")
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
diff --git a/sound/virtio/virtio_pcm.h b/sound/virtio/virtio_pcm.h
index e31f090a0124..61faf7b13cbc 100644
--- a/sound/virtio/virtio_pcm.h
+++ b/sound/virtio/virtio_pcm.h
@@ -13,6 +13,7 @@
struct virtio_pcm;
struct virtio_pcm_msg;
+struct page;
/**
* struct virtio_pcm_substream - VirtIO PCM substream.
@@ -38,6 +39,10 @@ struct virtio_pcm_msg;
* @msgs: Allocated I/O messages.
* @nmsgs: Number of allocated I/O messages.
* @msg_count: Number of pending I/O messages in the virtqueue.
+ * @dma_pages: Pages backing DMA messages, retained until messages are freed.
+ * @ndma_pages: Number of retained pages.
+ * @owned_dma_area: Driver-owned PCM buffer backing asynchronous messages.
+ * @owned_dma_bytes: Size of @owned_dma_area.
* @msg_empty: Notify when msg_count is zero.
*/
struct virtio_pcm_substream {
@@ -60,6 +65,12 @@ struct virtio_pcm_substream {
struct virtio_pcm_msg **msgs;
unsigned int nmsgs;
unsigned int msg_count;
+ struct page **dma_pages;
+ unsigned int ndma_pages;
+#ifndef CONFIG_MMU
+ u8 *owned_dma_area;
+ size_t owned_dma_bytes;
+#endif
wait_queue_head_t msg_empty;
};
diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c
index 762480363b86..b7f2334bb221 100644
--- a/sound/virtio/virtio_pcm_msg.c
+++ b/sound/virtio/virtio_pcm_msg.c
@@ -3,6 +3,9 @@
* virtio-snd: Virtio sound device
* Copyright (C) 2021 OpenSynergy GmbH
*/
+#include <linux/limits.h>
+#include <linux/mm.h>
+#include <linux/overflow.h>
#include <sound/pcm_params.h>
#include "virtio_card.h"
@@ -13,6 +16,7 @@
* @xfer: Request header payload.
* @status: Response header payload.
* @length: Data length in bytes.
+ * @data_offset: Offset in the driver-owned PCM buffer on NOMMU systems.
* @sgs: Payload scatter-gather table.
*/
struct virtio_pcm_msg {
@@ -20,6 +24,9 @@ struct virtio_pcm_msg {
struct virtio_snd_pcm_xfer xfer;
struct virtio_snd_pcm_status status;
size_t length;
+#ifndef CONFIG_MMU
+ size_t data_offset;
+#endif
struct scatterlist sgs[];
};
@@ -37,9 +44,8 @@ enum pcm_msg_sg_index {
};
/**
- * virtsnd_pcm_sg_num() - Count the number of sg-elements required to represent
- * vmalloc'ed buffer.
- * @data: Pointer to vmalloc'ed buffer.
+ * virtsnd_pcm_sg_num() - Count the sg-elements for a page-backed buffer.
+ * @data: Pointer to the page-backed buffer.
* @length: Buffer size.
*
* Context: Any context.
@@ -47,15 +53,19 @@ enum pcm_msg_sg_index {
*/
static int virtsnd_pcm_sg_num(u8 *data, unsigned int length)
{
- phys_addr_t sg_address;
- unsigned int sg_length;
+ phys_addr_t sg_address = 0;
+ unsigned int sg_length = 0;
int num = 0;
while (length) {
struct page *pg = vmalloc_to_page(data);
- phys_addr_t pg_address = page_to_phys(pg);
+ phys_addr_t pg_address;
size_t pg_length;
+ if (!pg)
+ return -EINVAL;
+ pg_address = page_to_phys(pg);
+
pg_length = PAGE_SIZE - offset_in_page(data);
if (pg_length > length)
pg_length = length;
@@ -75,11 +85,80 @@ static int virtsnd_pcm_sg_num(u8 *data, unsigned int length)
return num;
}
+#ifdef CONFIG_MMU
+/**
+ * virtsnd_pcm_dma_pages_put() - Release retained PCM buffer pages.
+ * @vss: VirtIO PCM substream.
+ *
+ * Context: Process context.
+ */
+static void virtsnd_pcm_dma_pages_put(struct virtio_pcm_substream *vss)
+{
+ while (vss->ndma_pages)
+ put_page(vss->dma_pages[--vss->ndma_pages]);
+ kfree(vss->dma_pages);
+ vss->dma_pages = NULL;
+}
+
/**
- * virtsnd_pcm_sg_from() - Build sg-list from vmalloc'ed buffer.
+ * virtsnd_pcm_dma_pages_get() - Retain the pages backing the PCM buffer.
+ * @vss: VirtIO PCM substream.
+ * @data: PCM buffer address.
+ * @length: PCM buffer size.
+ *
+ * Keep the backing pages alive while virtqueue descriptors can still refer to
+ * them, even if ALSA releases the vmalloc mapping after a failed drain.
+ *
+ * Context: Process context.
+ * Return: 0 on success, -errno on failure.
+ */
+static int virtsnd_pcm_dma_pages_get(struct virtio_pcm_substream *vss,
+ u8 *data, size_t length)
+{
+ size_t i;
+ size_t npages;
+ size_t span;
+
+ if (!data || !length || !is_vmalloc_addr(data))
+ return -EINVAL;
+ if (vss->dma_pages || vss->ndma_pages)
+ return -EBUSY;
+
+ if (check_add_overflow(length, (size_t)offset_in_page(data), &span))
+ return -EOVERFLOW;
+
+ npages = span / PAGE_SIZE + !!(span % PAGE_SIZE);
+ if (npages > UINT_MAX)
+ return -EOVERFLOW;
+
+ vss->dma_pages = kcalloc(npages, sizeof(*vss->dma_pages), GFP_KERNEL);
+ if (!vss->dma_pages)
+ return -ENOMEM;
+
+ for (i = 0; i < npages; i++) {
+ struct page *page = vmalloc_to_page(data + i * PAGE_SIZE);
+
+ if (!page)
+ goto err_put_pages;
+
+ get_page(page);
+ vss->dma_pages[vss->ndma_pages++] = page;
+ }
+
+ return 0;
+
+err_put_pages:
+ virtsnd_pcm_dma_pages_put(vss);
+
+ return -EINVAL;
+}
+#endif
+
+/**
+ * virtsnd_pcm_sg_from() - Build an sg-list from a page-backed buffer.
* @sgs: Preallocated sg-list to populate.
* @nsgs: The maximum number of elements in the @sgs.
- * @data: Pointer to vmalloc'ed buffer.
+ * @data: Pointer to the page-backed buffer.
* @length: Buffer size.
*
* Splits the buffer into physically contiguous parts and makes an sg-list of
@@ -132,36 +211,99 @@ static void virtsnd_pcm_sg_from(struct scatterlist *sgs, int nsgs, u8 *data,
int virtsnd_pcm_msg_alloc(struct virtio_pcm_substream *vss,
unsigned int periods, unsigned int period_bytes)
{
- struct snd_pcm_runtime *runtime = vss->substream->runtime;
+ struct snd_pcm_runtime *runtime;
+ size_t buffer_bytes;
unsigned int i;
+ int rc;
- vss->msgs = kzalloc_objs(*vss->msgs, periods);
- if (!vss->msgs)
+ if (vss->msgs || vss->nmsgs || vss->dma_pages || vss->ndma_pages)
+ return -EBUSY;
+#ifndef CONFIG_MMU
+ if (vss->owned_dma_area || vss->owned_dma_bytes)
+ return -EBUSY;
+#endif
+ if (!periods || !period_bytes)
+ return -EINVAL;
+
+ if (check_mul_overflow((size_t)periods, (size_t)period_bytes,
+ &buffer_bytes))
+ return -EOVERFLOW;
+
+ if (!vss->substream || !vss->substream->runtime)
+ return -EINVAL;
+ runtime = vss->substream->runtime;
+ if (!runtime->dma_area || buffer_bytes > runtime->dma_bytes)
+ return -EINVAL;
+
+#ifdef CONFIG_MMU
+ rc = virtsnd_pcm_dma_pages_get(vss, runtime->dma_area, buffer_bytes);
+ if (rc)
+ return rc;
+#else
+ vss->owned_dma_area = kzalloc(buffer_bytes, GFP_KERNEL);
+ if (!vss->owned_dma_area)
return -ENOMEM;
+ vss->owned_dma_bytes = buffer_bytes;
+#endif
+
+ vss->msgs = kzalloc_objs(*vss->msgs, periods);
+ if (!vss->msgs) {
+ rc = -ENOMEM;
+ goto err_free_pages;
+ }
vss->nmsgs = periods;
for (i = 0; i < periods; ++i) {
- u8 *data = runtime->dma_area + period_bytes * i;
- int sg_num = virtsnd_pcm_sg_num(data, period_bytes);
+ int sg_num;
struct virtio_pcm_msg *msg;
+ u8 *data;
+#ifdef CONFIG_MMU
+ data = runtime->dma_area + (size_t)period_bytes * i;
+#else
+ data = vss->owned_dma_area + (size_t)period_bytes * i;
+#endif
+ sg_num = virtsnd_pcm_sg_num(data, period_bytes);
+ if (sg_num < 0) {
+ rc = sg_num;
+ goto err_free_msgs;
+ }
msg = kzalloc_flex(*msg, sgs, sg_num + 2);
- if (!msg)
- return -ENOMEM;
+ if (!msg) {
+ rc = -ENOMEM;
+ goto err_free_msgs;
+ }
msg->substream = vss;
+ vss->msgs[i] = msg;
sg_init_one(&msg->sgs[PCM_MSG_SG_XFER], &msg->xfer,
sizeof(msg->xfer));
sg_init_one(&msg->sgs[PCM_MSG_SG_STATUS], &msg->status,
sizeof(msg->status));
+#ifndef CONFIG_MMU
+ msg->data_offset = (size_t)period_bytes * i;
+#endif
virtsnd_pcm_sg_from(&msg->sgs[PCM_MSG_SG_DATA], sg_num, data,
period_bytes);
-
- vss->msgs[i] = msg;
}
return 0;
+
+err_free_msgs:
+ virtsnd_pcm_msg_free(vss);
+ return rc;
+
+err_free_pages:
+#ifdef CONFIG_MMU
+ virtsnd_pcm_dma_pages_put(vss);
+#else
+ kfree(vss->owned_dma_area);
+ vss->owned_dma_area = NULL;
+ vss->owned_dma_bytes = 0;
+#endif
+
+ return rc;
}
/**
@@ -177,6 +319,13 @@ void virtsnd_pcm_msg_free(struct virtio_pcm_substream *vss)
for (i = 0; vss->msgs && i < vss->nmsgs; ++i)
kfree(vss->msgs[i]);
kfree(vss->msgs);
+#ifdef CONFIG_MMU
+ virtsnd_pcm_dma_pages_put(vss);
+#else
+ kfree(vss->owned_dma_area);
+ vss->owned_dma_area = NULL;
+ vss->owned_dma_bytes = 0;
+#endif
vss->msgs = NULL;
vss->nmsgs = 0;
@@ -249,6 +398,33 @@ int virtsnd_pcm_msg_send(struct virtio_pcm_substream *vss, unsigned long offset,
msg->xfer.stream_id = cpu_to_le32(vss->sid);
memset(&msg->status, 0, sizeof(msg->status));
+#ifndef CONFIG_MMU
+ if (vss->direction == SNDRV_PCM_STREAM_PLAYBACK) {
+ struct snd_pcm_runtime *runtime =
+ vss->substream ? vss->substream->runtime : NULL;
+ size_t data_bytes = msg->length;
+
+ if (!runtime || !runtime->dma_area || !vss->owned_dma_area ||
+ msg->data_offset > runtime->dma_bytes ||
+ data_bytes > runtime->dma_bytes - msg->data_offset ||
+ msg->data_offset > vss->owned_dma_bytes ||
+ data_bytes > vss->owned_dma_bytes - msg->data_offset) {
+ dev_err_ratelimited(&vdev->dev,
+ "SID %u: invalid playback buffer for I/O message\n",
+ vss->sid);
+ msg->length -= n;
+ if (msg_count != vss->msg_count &&
+ !(vss->features &
+ (1U << VIRTIO_SND_PCM_F_MSG_POLLING)) &&
+ virtqueue_kick_prepare(vqueue))
+ virtqueue_notify(vqueue);
+ return -EINVAL;
+ }
+ memcpy(vss->owned_dma_area + msg->data_offset,
+ runtime->dma_area + msg->data_offset, data_bytes);
+ }
+#endif
+
if (vss->direction == SNDRV_PCM_STREAM_PLAYBACK)
rc = virtqueue_add_sgs(vqueue, psgs, 2, 1, msg,
GFP_ATOMIC);
@@ -363,15 +539,48 @@ static void virtsnd_pcm_msg_complete(struct virtio_pcm_msg *msg,
* the hw_ptr value is unconditionally advanced.
*/
guard(spinlock)(&vss->lock);
+#ifndef CONFIG_MMU
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
+ vss->xfer_enabled && written_bytes > sizeof(msg->status)) {
+ struct snd_pcm_runtime *runtime =
+ vss->substream ? vss->substream->runtime : NULL;
+ size_t reported_bytes = written_bytes - sizeof(msg->status);
+ size_t data_bytes = min_t(size_t, reported_bytes, msg->length);
+
+ if (!runtime || !runtime->dma_area || !vss->owned_dma_area ||
+ msg->data_offset > runtime->dma_bytes ||
+ data_bytes > runtime->dma_bytes - msg->data_offset ||
+ msg->data_offset > vss->owned_dma_bytes ||
+ data_bytes > vss->owned_dma_bytes - msg->data_offset)
+ dev_err_ratelimited(&vss->snd->vdev->dev,
+ "SID %u: invalid capture buffer for I/O message\n",
+ vss->sid);
+ else
+ memcpy(runtime->dma_area + msg->data_offset,
+ vss->owned_dma_area + msg->data_offset, data_bytes);
+ }
+#endif
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
+ written_bytes > sizeof(msg->status) &&
+ written_bytes - sizeof(msg->status) > msg->length)
+ dev_err_ratelimited(&vss->snd->vdev->dev,
+ "SID %u: capture length exceeds I/O message capacity\n",
+ vss->sid);
/*
* If the capture substream returned an incorrect status, then just
- * increase the hw_ptr by the message size.
+ * increase the hw_ptr by the message size. Clamp valid capture lengths to
+ * the actual SG payload capacity before accounting them.
*/
if (vss->direction == SNDRV_PCM_STREAM_PLAYBACK ||
- written_bytes <= sizeof(msg->status))
+ written_bytes <= sizeof(msg->status)) {
vss->hw_ptr += msg->length;
- else
- vss->hw_ptr += written_bytes - sizeof(msg->status);
+ } else {
+ size_t data_bytes = min_t(size_t,
+ written_bytes - sizeof(msg->status),
+ msg->length);
+
+ vss->hw_ptr += data_bytes;
+ }
if (vss->hw_ptr >= vss->buffer_bytes)
vss->hw_ptr -= vss->buffer_bytes;
@@ -387,7 +596,6 @@ static void virtsnd_pcm_msg_complete(struct virtio_pcm_msg *msg,
runtime->delay =
bytes_to_frames(runtime,
le32_to_cpu(msg->status.latency_bytes));
-
schedule_work(&vss->elapsed_period);
} else if (!vss->msg_count) {
wake_up_all(&vss->msg_empty);
diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
index 970499fe419b..f74bf365065b 100644
--- a/sound/virtio/virtio_pcm_ops.c
+++ b/sound/virtio/virtio_pcm_ops.c
@@ -25,6 +25,8 @@
* signal or due to a timeout. In this case, the device can still access
* messages even after calling ops->hw_free(). It can also issue an interrupt,
* and the interrupt handler will also try to access message structures.
+ * The messages retain references to the PCM buffer pages until they are
+ * freed, so late DMA does not target pages released by ALSA.
*
* Therefore, freeing of already allocated messages occurs:
*
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 05/14] ALSA: virtio: Keep control payloads alive through DMA completion
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (3 preceding siblings ...)
2026-10-29 5:43 ` [PATCH v8 04/14] ALSA: virtio: Keep PCM DMA buffers alive through device ownership Joonho Lee
@ 2026-10-29 5:43 ` Joonho Lee
2026-10-29 5:54 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 06/14] ALSA: virtio: wait through signals for PCM teardown Joonho Lee
` (8 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
Control requests can expose caller-owned payloads to the device after a
synchronous send times out. Keep request and response headers and payloads
in the reference-counted message so caller cleanup cannot invalidate DMA.
Keep the queue reference until device completion or post-reset cleanup,
even when the caller times out or is interrupted. Consume the caller's
reference on every send path, and let response readers retain an extra
reference through copyout. Check payload sizes and allocation arithmetic
before building the scatterlists.
A control value payload is also read by the CPU as a typed structure. DMA
cacheline alignment alone does not guarantee the structure's CPU alignment.
Align regions to the greater of the DMA cacheline and control-value type
alignment, and reserve prefix slack from the buffer base so typed accesses
remain aligned while scatterlist regions stay on cacheline boundaries.
Fixes: 9d45e514da88 ("ALSA: virtio: handling control messages")
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
diff --git a/sound/virtio/virtio_ctl_msg.c b/sound/virtio/virtio_ctl_msg.c
index 7114bea4033a..2e0ff15e729d 100644
--- a/sound/virtio/virtio_ctl_msg.c
+++ b/sound/virtio/virtio_ctl_msg.c
@@ -3,7 +3,10 @@
* virtio-snd: Virtio sound device
* Copyright (C) 2021 OpenSynergy GmbH
*/
+#include <linux/dma-mapping.h>
+#include <linux/limits.h>
#include <linux/moduleparam.h>
+#include <linux/overflow.h>
#include <linux/virtio_config.h>
#include "virtio_card.h"
@@ -11,7 +14,12 @@
/**
* struct virtio_snd_msg - Control message.
* @sg_request: Scattergather list containing a device request (header).
+ * @sg_request_data: Scattergather list containing request data.
* @sg_response: Scattergather list containing a device response (status).
+ * @sg_response_data: Scattergather list containing response data.
+ * @dma_buffer: Message-owned storage for all DMA-visible SG regions.
+ * @request_data_size: Size of the request data buffer.
+ * @response_data_size: Size of the response data buffer.
* @list: Pending message list entry.
* @notify: Request completed notification.
* @ref_count: Reference count used to manage a message lifetime.
@@ -19,7 +27,12 @@
*/
struct virtio_snd_msg {
struct scatterlist sg_request;
+ struct scatterlist sg_request_data;
struct scatterlist sg_response;
+ struct scatterlist sg_response_data;
+ void *dma_buffer;
+ size_t request_data_size;
+ size_t response_data_size;
struct list_head list;
struct completion notify;
refcount_t ref_count;
@@ -47,8 +60,10 @@ void virtsnd_ctl_msg_ref(struct virtio_snd_msg *msg)
*/
void virtsnd_ctl_msg_unref(struct virtio_snd_msg *msg)
{
- if (refcount_dec_and_test(&msg->ref_count))
+ if (refcount_dec_and_test(&msg->ref_count)) {
+ kfree(msg->dma_buffer);
kfree(msg);
+ }
}
/**
@@ -73,10 +88,71 @@ void *virtsnd_ctl_msg_response(struct virtio_snd_msg *msg)
return sg_virt(&msg->sg_response);
}
+/**
+ * virtsnd_ctl_msg_request_data() - Get the request data buffer.
+ * @msg: Control message.
+ *
+ * Context: Any context.
+ */
+void *virtsnd_ctl_msg_request_data(struct virtio_snd_msg *msg)
+{
+ return msg->request_data_size ? sg_virt(&msg->sg_request_data) : NULL;
+}
+
+/**
+ * virtsnd_ctl_msg_response_data() - Get the response data buffer.
+ * @msg: Control message.
+ *
+ * Context: Any context.
+ */
+void *virtsnd_ctl_msg_response_data(struct virtio_snd_msg *msg)
+{
+ return msg->response_data_size ? sg_virt(&msg->sg_response_data) : NULL;
+}
+
+/**
+ * virtsnd_ctl_msg_sg_init() - Place one DMA region on cacheline boundaries.
+ * @sg: Scattergather entry for the region.
+ * @cursor: Current location in the message-owned DMA buffer.
+ * @size: Region length.
+ * @alignment: Alignment for DMA cachelines and typed CPU accesses.
+ *
+ * Context: Any context during message allocation.
+ * Return: 0 on success, -EOVERFLOW if the address cannot be represented.
+ */
+static int virtsnd_ctl_msg_sg_init(struct scatterlist *sg, u8 **cursor,
+ size_t size, size_t alignment)
+{
+ uintptr_t address = (uintptr_t)*cursor;
+ uintptr_t end;
+ size_t padding;
+
+ padding = address % alignment;
+ if (padding &&
+ check_add_overflow(address, (uintptr_t)(alignment - padding),
+ &address))
+ return -EOVERFLOW;
+
+ if (check_add_overflow(address, (uintptr_t)size, &end))
+ return -EOVERFLOW;
+
+ padding = end % alignment;
+ if (padding &&
+ check_add_overflow(end, (uintptr_t)(alignment - padding), &end))
+ return -EOVERFLOW;
+
+ sg_init_one(sg, (void *)address, (unsigned int)size);
+ *cursor = (u8 *)end;
+
+ return 0;
+}
+
/**
* virtsnd_ctl_msg_alloc() - Allocate and initialize a control message.
* @request_size: Size of request header.
* @response_size: Size of response header.
+ * @request_data_size: Size of request data buffer.
+ * @response_data_size: Size of response data buffer.
* @gfp: Kernel flags for memory allocation.
*
* The message will be automatically freed when the ref_count value is 0.
@@ -85,20 +161,85 @@ void *virtsnd_ctl_msg_response(struct virtio_snd_msg *msg)
* Return: Allocated message on success, NULL on failure.
*/
struct virtio_snd_msg *virtsnd_ctl_msg_alloc(size_t request_size,
- size_t response_size, gfp_t gfp)
+ size_t response_size,
+ size_t request_data_size,
+ size_t response_data_size,
+ gfp_t gfp)
{
struct virtio_snd_msg *msg;
+ struct scatterlist *sgs[4];
+ size_t sizes[4] = {
+ request_size,
+ response_size,
+ request_data_size,
+ response_data_size,
+ };
+ size_t alignment;
+ size_t dma_buffer_size;
+ size_t aligned_size;
+ size_t padding;
+ u8 *data;
+ unsigned int i;
+ int cache_alignment;
+ int rc;
- if (!request_size || !response_size)
+ if (!request_size || !response_size || request_size > UINT_MAX ||
+ response_size > UINT_MAX || request_data_size > UINT_MAX ||
+ response_data_size > UINT_MAX)
return NULL;
+ cache_alignment = dma_get_cache_alignment();
+ if (cache_alignment <= 0)
+ return NULL;
+ /* DMA coherence does not remove the CPU's type alignment requirement. */
+ alignment = max_t(size_t, cache_alignment,
+ __alignof__(struct virtio_snd_ctl_value));
+
+ /* Reserve prefix alignment slack before the first DMA region. */
+ dma_buffer_size = alignment - 1;
+ for (i = 0; i < ARRAY_SIZE(sizes); ++i) {
+ if (!sizes[i])
+ continue;
+
+ padding = sizes[i] % alignment;
+ aligned_size = sizes[i];
+ if (padding &&
+ check_add_overflow(aligned_size, alignment - padding,
+ &aligned_size))
+ return NULL;
+
+ if (check_add_overflow(dma_buffer_size, aligned_size,
+ &dma_buffer_size))
+ return NULL;
+ }
- msg = kzalloc(sizeof(*msg) + request_size + response_size, gfp);
+ msg = kzalloc(sizeof(*msg), gfp);
if (!msg)
return NULL;
- sg_init_one(&msg->sg_request, (u8 *)msg + sizeof(*msg), request_size);
- sg_init_one(&msg->sg_response, (u8 *)msg + sizeof(*msg) + request_size,
- response_size);
+ msg->dma_buffer = kzalloc(dma_buffer_size, gfp);
+ if (!msg->dma_buffer) {
+ kfree(msg);
+ return NULL;
+ }
+ data = msg->dma_buffer;
+ sgs[0] = &msg->sg_request;
+ sgs[1] = &msg->sg_response;
+ sgs[2] = &msg->sg_request_data;
+ sgs[3] = &msg->sg_response_data;
+ for (i = 0; i < ARRAY_SIZE(sizes); ++i) {
+ if (!sizes[i])
+ continue;
+
+ rc = virtsnd_ctl_msg_sg_init(sgs[i], &data, sizes[i], alignment);
+ if (rc) {
+ kfree(msg->dma_buffer);
+ kfree(msg);
+ return NULL;
+ }
+ }
+
+ msg->request_data_size = request_data_size;
+ msg->response_data_size = response_data_size;
INIT_LIST_HEAD(&msg->list);
init_completion(&msg->notify);
@@ -112,17 +253,15 @@ struct virtio_snd_msg *virtsnd_ctl_msg_alloc(size_t request_size,
* virtsnd_ctl_msg_send() - Send a control message.
* @snd: VirtIO sound device.
* @msg: Control message.
- * @out_sgs: Additional sg-list to attach to the request header (may be NULL).
- * @in_sgs: Additional sg-list to attach to the response header (may be NULL).
* @nowait: Flag indicating whether to wait for completion.
*
* Context: Any context. Takes and releases the control queue spinlock.
* May sleep if @nowait is false.
+ * Consumes the caller's reference on every return path.
* Return: 0 on success, -errno on failure.
*/
int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
- struct scatterlist *out_sgs,
- struct scatterlist *in_sgs, bool nowait)
+ bool nowait)
{
struct virtio_device *vdev = snd->vdev;
struct virtio_snd_queue *queue = virtsnd_control_queue(snd);
@@ -133,6 +272,7 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
unsigned int nins = 0;
struct scatterlist *psgs[4];
bool notify = false;
+ unsigned long flags;
int rc;
virtsnd_ctl_msg_ref(msg);
@@ -142,27 +282,27 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
response->code = cpu_to_le32(VIRTIO_SND_S_IO_ERR);
psgs[nouts++] = &msg->sg_request;
- if (out_sgs)
- psgs[nouts++] = out_sgs;
+ if (msg->request_data_size)
+ psgs[nouts++] = &msg->sg_request_data;
psgs[nouts + nins++] = &msg->sg_response;
- if (in_sgs)
- psgs[nouts + nins++] = in_sgs;
-
- scoped_guard(spinlock_irqsave, &queue->lock) {
- if (snd->ctl_msgs_blocked)
- rc = -ENODEV;
- else
- rc = virtqueue_add_sgs(queue->vqueue, psgs, nouts, nins,
- msg, GFP_ATOMIC);
- if (!rc) {
- notify = virtqueue_kick_prepare(queue->vqueue);
+ if (msg->response_data_size)
+ psgs[nouts + nins++] = &msg->sg_response_data;
- list_add_tail(&msg->list, &snd->ctl_msgs);
- if (notify)
- virtqueue_notify(queue->vqueue);
- }
+ spin_lock_irqsave(&queue->lock, flags);
+ if (snd->ctl_msgs_blocked)
+ rc = -ENODEV;
+ else
+ rc = virtqueue_add_sgs(queue->vqueue, psgs, nouts, nins, msg,
+ GFP_ATOMIC);
+ if (!rc) {
+ notify = virtqueue_kick_prepare(queue->vqueue);
+
+ list_add_tail(&msg->list, &snd->ctl_msgs);
+ if (notify)
+ virtqueue_notify(queue->vqueue);
}
+ spin_unlock_irqrestore(&queue->lock, flags);
if (rc) {
dev_err(&vdev->dev, "failed to send control message (0x%08x)\n",
@@ -173,46 +313,36 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
* called, it is necessary to decrement the reference count.
*/
virtsnd_ctl_msg_unref(msg);
-
- goto on_exit;
- }
-
- if (nowait)
- goto on_exit;
-
- rc = wait_for_completion_interruptible_timeout(&msg->notify, js);
- if (rc <= 0) {
+ } else if (!nowait) {
+ rc = wait_for_completion_interruptible_timeout(&msg->notify, js);
if (!rc) {
dev_err(&vdev->dev,
"control message (0x%08x) timeout\n",
le32_to_cpu(request->code));
rc = -ETIMEDOUT;
+ } else if (rc > 0) {
+ if (READ_ONCE(msg->cancelled)) {
+ rc = -ENODEV;
+ } else {
+ switch (le32_to_cpu(response->code)) {
+ case VIRTIO_SND_S_OK:
+ rc = 0;
+ break;
+ case VIRTIO_SND_S_NOT_SUPP:
+ rc = -EOPNOTSUPP;
+ break;
+ case VIRTIO_SND_S_IO_ERR:
+ rc = -EIO;
+ break;
+ default:
+ rc = -EINVAL;
+ break;
+ }
+ }
}
-
- goto on_exit;
- }
-
- if (READ_ONCE(msg->cancelled)) {
- rc = -ENODEV;
- goto on_exit;
- }
-
- switch (le32_to_cpu(response->code)) {
- case VIRTIO_SND_S_OK:
- rc = 0;
- break;
- case VIRTIO_SND_S_NOT_SUPP:
- rc = -EOPNOTSUPP;
- break;
- case VIRTIO_SND_S_IO_ERR:
- rc = -EIO;
- break;
- default:
- rc = -EINVAL;
- break;
}
-on_exit:
+ /* Drop the reference consumed by this send attempt. */
virtsnd_ctl_msg_unref(msg);
return rc;
@@ -308,10 +438,23 @@ int virtsnd_ctl_query_info(struct virtio_snd *snd, int command, int start_id,
{
struct virtio_snd_msg *msg;
struct virtio_snd_query_info *query;
- struct scatterlist sg;
+ size_t info_size;
+ int rc;
+
+ if (count < 0)
+ return -EINVAL;
+
+ if (size > UINT_MAX ||
+ check_mul_overflow((size_t)count, size, &info_size) ||
+ info_size > UINT_MAX)
+ return -EOVERFLOW;
+
+ if (info_size && !info)
+ return -EINVAL;
msg = virtsnd_ctl_msg_alloc(sizeof(*query),
- sizeof(struct virtio_snd_hdr), GFP_KERNEL);
+ sizeof(struct virtio_snd_hdr), 0, info_size,
+ GFP_KERNEL);
if (!msg)
return -ENOMEM;
@@ -321,9 +464,13 @@ int virtsnd_ctl_query_info(struct virtio_snd *snd, int command, int start_id,
query->count = cpu_to_le32(count);
query->size = cpu_to_le32(size);
- sg_init_one(&sg, info, count * size);
+ virtsnd_ctl_msg_ref(msg);
+ rc = virtsnd_ctl_msg_send(snd, msg, false);
+ if (!rc && info_size)
+ memcpy(info, virtsnd_ctl_msg_response_data(msg), info_size);
+ virtsnd_ctl_msg_unref(msg);
- return virtsnd_ctl_msg_send(snd, msg, NULL, &sg, false);
+ return rc;
}
/**
diff --git a/sound/virtio/virtio_ctl_msg.h b/sound/virtio/virtio_ctl_msg.h
index d917435409d0..f867b9b9233d 100644
--- a/sound/virtio/virtio_ctl_msg.h
+++ b/sound/virtio/virtio_ctl_msg.h
@@ -21,20 +21,28 @@ void *virtsnd_ctl_msg_request(struct virtio_snd_msg *msg);
void *virtsnd_ctl_msg_response(struct virtio_snd_msg *msg);
struct virtio_snd_msg *virtsnd_ctl_msg_alloc(size_t request_size,
- size_t response_size, gfp_t gfp);
+ size_t response_size,
+ size_t request_data_size,
+ size_t response_data_size,
+ gfp_t gfp);
+
+void *virtsnd_ctl_msg_request_data(struct virtio_snd_msg *msg);
+
+void *virtsnd_ctl_msg_response_data(struct virtio_snd_msg *msg);
int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
- struct scatterlist *out_sgs,
- struct scatterlist *in_sgs, bool nowait);
+ bool nowait);
/**
* virtsnd_ctl_msg_send_sync() - Simplified sending of synchronous message.
* @snd: VirtIO sound device.
* @msg: Control message.
*
- * After returning from this function, the message will be deleted. If message
- * content is still needed, the caller must additionally to
- * virtsnd_ctl_msg_ref/unref() it.
+ * This function consumes the caller's message reference on return. A message
+ * accepted by the control queue keeps its queue reference until completion or
+ * post-reset cleanup, including when this function returns on timeout or
+ * interruption. Callers that read the response after success must retain an
+ * additional reference through that access.
*
* The msg_timeout_ms module parameter defines the message completion timeout.
* If the message is not completed within this time, the function will return an
@@ -49,7 +57,7 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
static inline int virtsnd_ctl_msg_send_sync(struct virtio_snd *snd,
struct virtio_snd_msg *msg)
{
- return virtsnd_ctl_msg_send(snd, msg, NULL, NULL, false);
+ return virtsnd_ctl_msg_send(snd, msg, false);
}
/**
@@ -63,7 +71,7 @@ static inline int virtsnd_ctl_msg_send_sync(struct virtio_snd *snd,
static inline int virtsnd_ctl_msg_send_async(struct virtio_snd *snd,
struct virtio_snd_msg *msg)
{
- return virtsnd_ctl_msg_send(snd, msg, NULL, NULL, true);
+ return virtsnd_ctl_msg_send(snd, msg, true);
}
void virtsnd_ctl_msg_cancel_all(struct virtio_snd *snd);
diff --git a/sound/virtio/virtio_kctl.c b/sound/virtio/virtio_kctl.c
index 45f7b6a5b308..4cf75dedbe9a 100644
--- a/sound/virtio/virtio_kctl.c
+++ b/sound/virtio/virtio_kctl.c
@@ -3,8 +3,10 @@
* virtio-snd: Virtio sound device
* Copyright (C) 2022 OpenSynergy GmbH
*/
-#include <sound/control.h>
+#include <linux/limits.h>
+#include <linux/overflow.h>
#include <linux/virtio_config.h>
+#include <sound/control.h>
#include "virtio_card.h"
@@ -161,10 +163,12 @@ static int virtsnd_kctl_get(struct snd_kcontrol *kcontrol,
unsigned int i;
int rc;
- msg = virtsnd_ctl_msg_alloc(request_size, response_size, GFP_KERNEL);
+ msg = virtsnd_ctl_msg_alloc(request_size, response_size, 0, 0,
+ GFP_KERNEL);
if (!msg)
return -ENOMEM;
+ /* Response lifetime through ALSA value copyout. */
virtsnd_ctl_msg_ref(msg);
hdr = virtsnd_ctl_msg_request(msg);
@@ -233,7 +237,8 @@ static int virtsnd_kctl_put(struct snd_kcontrol *kcontrol,
size_t response_size = sizeof(struct virtio_snd_hdr);
unsigned int i;
- msg = virtsnd_ctl_msg_alloc(request_size, response_size, GFP_KERNEL);
+ msg = virtsnd_ctl_msg_alloc(request_size, response_size, 0, 0,
+ GFP_KERNEL);
if (!msg)
return -ENOMEM;
@@ -288,23 +293,28 @@ static int virtsnd_kctl_tlv_op(struct snd_kcontrol *kcontrol, int op_flag,
struct virtio_snd *snd = snd_kcontrol_chip(kcontrol);
struct virtio_snd_msg *msg;
struct virtio_snd_ctl_hdr *hdr;
- unsigned int *tlv;
- struct scatterlist sg;
+ size_t request_data_size = 0;
+ size_t response_data_size = 0;
int rc;
+ switch (op_flag) {
+ case SNDRV_CTL_TLV_OP_READ:
+ response_data_size = size;
+ break;
+ case SNDRV_CTL_TLV_OP_WRITE:
+ case SNDRV_CTL_TLV_OP_CMD:
+ request_data_size = size;
+ break;
+ default:
+ return -EINVAL;
+ }
+
msg = virtsnd_ctl_msg_alloc(sizeof(*hdr), sizeof(struct virtio_snd_hdr),
+ request_data_size, response_data_size,
GFP_KERNEL);
if (!msg)
return -ENOMEM;
- tlv = kzalloc(size, GFP_KERNEL);
- if (!tlv) {
- rc = -ENOMEM;
- goto on_msg_unref;
- }
-
- sg_init_one(&sg, tlv, size);
-
hdr = virtsnd_ctl_msg_request(msg);
hdr->control_id = cpu_to_le32(kcontrol->private_value);
@@ -312,13 +322,16 @@ static int virtsnd_kctl_tlv_op(struct snd_kcontrol *kcontrol, int op_flag,
case SNDRV_CTL_TLV_OP_READ:
hdr->hdr.code = cpu_to_le32(VIRTIO_SND_R_CTL_TLV_READ);
- rc = virtsnd_ctl_msg_send(snd, msg, NULL, &sg, false);
- if (!rc) {
- if (copy_to_user(utlv, tlv, size))
- rc = -EFAULT;
- }
+ /* Response lifetime through user copyout. */
+ virtsnd_ctl_msg_ref(msg);
+ rc = virtsnd_ctl_msg_send(snd, msg, false);
+ if (!rc && size &&
+ copy_to_user(utlv, virtsnd_ctl_msg_response_data(msg),
+ size))
+ rc = -EFAULT;
+ virtsnd_ctl_msg_unref(msg);
- break;
+ return rc;
case SNDRV_CTL_TLV_OP_WRITE:
case SNDRV_CTL_TLV_OP_CMD:
if (op_flag == SNDRV_CTL_TLV_OP_WRITE)
@@ -327,28 +340,16 @@ static int virtsnd_kctl_tlv_op(struct snd_kcontrol *kcontrol, int op_flag,
hdr->hdr.code =
cpu_to_le32(VIRTIO_SND_R_CTL_TLV_COMMAND);
- if (copy_from_user(tlv, utlv, size)) {
- rc = -EFAULT;
- goto on_msg_unref;
- } else {
- rc = virtsnd_ctl_msg_send(snd, msg, &sg, NULL, false);
+ if (size && copy_from_user(virtsnd_ctl_msg_request_data(msg), utlv,
+ size)) {
+ virtsnd_ctl_msg_unref(msg);
+ return -EFAULT;
}
- break;
- default:
- rc = -EINVAL;
- /* We never get here - we listed all values for op_flag */
- WARN_ON(1);
- goto on_msg_unref;
+ return virtsnd_ctl_msg_send(snd, msg, false);
}
- kfree(tlv);
- return rc;
-
-on_msg_unref:
- virtsnd_ctl_msg_unref(msg);
- kfree(tlv);
- return rc;
+ return -EINVAL;
}
/**
@@ -369,27 +370,36 @@ static int virtsnd_kctl_get_enum_items(struct virtio_snd *snd, unsigned int cid)
struct virtio_snd_msg *msg;
struct virtio_snd_ctl_hdr *hdr;
unsigned int n = le32_to_cpu(kinfo->value.enumerated.items);
- struct scatterlist sg;
+ size_t items_size;
+ int rc;
- msg = virtsnd_ctl_msg_alloc(sizeof(*hdr),
- sizeof(struct virtio_snd_hdr), GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
+ if (check_mul_overflow((size_t)n, sizeof(*kctl->items), &items_size) ||
+ items_size > UINT_MAX)
+ return -EOVERFLOW;
kctl->items = devm_kcalloc(&vdev->dev, n, sizeof(*kctl->items),
GFP_KERNEL);
- if (!kctl->items) {
- virtsnd_ctl_msg_unref(msg);
+ if (!kctl->items)
return -ENOMEM;
- }
- sg_init_one(&sg, kctl->items, n * sizeof(*kctl->items));
+ msg = virtsnd_ctl_msg_alloc(sizeof(*hdr),
+ sizeof(struct virtio_snd_hdr), 0, items_size,
+ GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
hdr = virtsnd_ctl_msg_request(msg);
hdr->hdr.code = cpu_to_le32(VIRTIO_SND_R_CTL_ENUM_ITEMS);
hdr->control_id = cpu_to_le32(cid);
- return virtsnd_ctl_msg_send(snd, msg, NULL, &sg, false);
+ /* Item-buffer lifetime through the control-list copyout. */
+ virtsnd_ctl_msg_ref(msg);
+ rc = virtsnd_ctl_msg_send(snd, msg, false);
+ if (!rc && items_size)
+ memcpy(kctl->items, virtsnd_ctl_msg_response_data(msg), items_size);
+ virtsnd_ctl_msg_unref(msg);
+
+ return rc;
}
/**
diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c
index b7f2334bb221..b54642a9521d 100644
--- a/sound/virtio/virtio_pcm_msg.c
+++ b/sound/virtio/virtio_pcm_msg.c
@@ -671,7 +671,7 @@ virtsnd_pcm_ctl_msg_alloc(struct virtio_pcm_substream *vss,
break;
}
- msg = virtsnd_ctl_msg_alloc(request_size, response_size, gfp);
+ msg = virtsnd_ctl_msg_alloc(request_size, response_size, 0, 0, gfp);
if (msg) {
struct virtio_snd_pcm_hdr *hdr = virtsnd_ctl_msg_request(msg);
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 06/14] ALSA: virtio: wait through signals for PCM teardown
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (4 preceding siblings ...)
2026-10-29 5:43 ` [PATCH v8 05/14] ALSA: virtio: Keep control payloads alive through DMA completion Joonho Lee
@ 2026-10-29 5:43 ` Joonho Lee
2026-10-29 6:00 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 07/14] ALSA: pcm: mark stop-threshold accesses for capture policy sampling Joonho Lee
` (7 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
A signal can end the interruptible wait while a STOP or RELEASE request is
still pending. A later teardown may then submit another RELEASE before the
first request completes.
Keep both waits non-interruptible and use the existing message timeout.
Other control requests remain interruptible. Device reset still cancels the
pending request and wakes the waiter.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
diff --git a/sound/virtio/virtio_ctl_msg.c b/sound/virtio/virtio_ctl_msg.c
index 2e0ff15e729d..e50e6c7f3b8d 100644
--- a/sound/virtio/virtio_ctl_msg.c
+++ b/sound/virtio/virtio_ctl_msg.c
@@ -314,7 +314,20 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
*/
virtsnd_ctl_msg_unref(msg);
} else if (!nowait) {
- rc = wait_for_completion_interruptible_timeout(&msg->notify, js);
+ /*
+ * Teardown must observe completion even with a pending signal.
+ * Otherwise a subsequent open can retry RELEASE while the device
+ * is still processing the previous teardown request.
+ */
+ switch (le32_to_cpu(request->code)) {
+ case VIRTIO_SND_R_PCM_STOP:
+ case VIRTIO_SND_R_PCM_RELEASE:
+ rc = wait_for_completion_timeout(&msg->notify, js);
+ break;
+ default:
+ rc = wait_for_completion_interruptible_timeout(&msg->notify, js);
+ break;
+ }
if (!rc) {
dev_err(&vdev->dev,
"control message (0x%08x) timeout\n",
diff --git a/sound/virtio/virtio_ctl_msg.h b/sound/virtio/virtio_ctl_msg.h
index f867b9b9233d..d84f0740fcb0 100644
--- a/sound/virtio/virtio_ctl_msg.h
+++ b/sound/virtio/virtio_ctl_msg.h
@@ -47,6 +47,9 @@ int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg,
* The msg_timeout_ms module parameter defines the message completion timeout.
* If the message is not completed within this time, the function will return an
* error.
+ * PCM STOP and RELEASE wait through pending signals within this timeout.
+ * Other requests remain interruptible. Device reset cancels and wakes either
+ * kind of waiter.
*
* Context: Any context that permits to sleep.
* Return: 0 on success, -errno on failure.
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 07/14] ALSA: pcm: mark stop-threshold accesses for capture policy sampling
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (5 preceding siblings ...)
2026-10-29 5:43 ` [PATCH v8 06/14] ALSA: virtio: wait through signals for PCM teardown Joonho Lee
@ 2026-10-29 5:43 ` Joonho Lee
2026-10-29 5:48 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 08/14] ALSA: pcm: stop paused streams before synchronizing teardown Joonho Lee
` (6 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
The staged virtio-snd capture path needs to sample stop_threshold from
completion context to distinguish free-running capture from bounded
capture. Software parameters can update this field while capture runs.
Mark the runtime stop-threshold assignments with WRITE_ONCE and the proc
status read with READ_ONCE. This pairs the assignments with the driver's
READ_ONCE policy sampling in the following patch.
These annotations do not provide a transaction for the entire sw_params
update. The driver serializes its cached policy and accounting with its
own substream lock.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -2958,7 +2958,7 @@
runtime->oss.mmap_bytes);
#endif
/* In mmap mode we never stop */
- runtime->stop_threshold = runtime->boundary;
+ WRITE_ONCE(runtime->stop_threshold, runtime->boundary);
return 0;
}
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -422,7 +422,8 @@
snd_iprintf(buffer, "period_step: %u\n", runtime->period_step);
snd_iprintf(buffer, "avail_min: %lu\n", runtime->control->avail_min);
snd_iprintf(buffer, "start_threshold: %lu\n", runtime->start_threshold);
- snd_iprintf(buffer, "stop_threshold: %lu\n", runtime->stop_threshold);
+ snd_iprintf(buffer, "stop_threshold: %lu\n",
+ READ_ONCE(runtime->stop_threshold));
snd_iprintf(buffer, "silence_threshold: %lu\n", runtime->silence_threshold);
snd_iprintf(buffer, "silence_size: %lu\n", runtime->silence_size);
snd_iprintf(buffer, "boundary: %lu\n", runtime->boundary);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -871,7 +871,7 @@
runtime->period_step = 1;
runtime->control->avail_min = runtime->period_size;
runtime->start_threshold = 1;
- runtime->stop_threshold = runtime->buffer_size;
+ WRITE_ONCE(runtime->stop_threshold, runtime->buffer_size);
runtime->silence_threshold = 0;
runtime->silence_size = 0;
runtime->boundary = runtime->buffer_size;
@@ -1062,7 +1062,8 @@
runtime->period_step = params->period_step;
runtime->control->avail_min = params->avail_min;
runtime->start_threshold = params->start_threshold;
- runtime->stop_threshold = params->stop_threshold;
+ /* Drivers may sample the stop policy from completion context. */
+ WRITE_ONCE(runtime->stop_threshold, params->stop_threshold);
runtime->silence_threshold = params->silence_threshold;
runtime->silence_size = params->silence_size;
params->boundary = runtime->boundary;
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v8 08/14] ALSA: pcm: stop paused streams before synchronizing teardown
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (6 preceding siblings ...)
2026-10-29 5:43 ` [PATCH v8 07/14] ALSA: pcm: mark stop-threshold accesses for capture policy sampling Joonho Lee
@ 2026-10-29 5:43 ` Joonho Lee
2026-10-29 6:04 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 09/14] ALSA: virtio: add staged capture accounting and lifecycle recovery Joonho Lee
` (5 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
A paused stream can retain outstanding driver requests. Moving it out of
PAUSED without the STOP trigger leaves device state paused, and skipping
stop_operating prevents the following sync_stop from cleaning it up.
Run the existing STOP and stop_operating path for PAUSED as well as
running streams. This replaces the earlier v7 proposal which only marked
stop_operating. Drivers now receive STOP when a paused stream is dropped
or stopped by an error. The change affects all PCM drivers.
Playback pause/drop/prepare/write and capture lifecycle tests exercise the
change. Physical drivers beyond virtio-snd still need review and testing.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1607,7 +1607,8 @@
snd_pcm_state_t state)
{
if (substream->runtime->trigger_master == substream &&
- snd_pcm_running(substream)) {
+ (snd_pcm_running(substream) ||
+ substream->runtime->state == SNDRV_PCM_STATE_PAUSED)) {
substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_STOP);
substream->runtime->stop_operating = true;
}
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v8 09/14] ALSA: virtio: add staged capture accounting and lifecycle recovery
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (7 preceding siblings ...)
2026-10-29 5:43 ` [PATCH v8 08/14] ALSA: pcm: stop paused streams before synchronizing teardown Joonho Lee
@ 2026-10-29 5:43 ` Joonho Lee
2026-10-29 6:23 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 10/14] ALSA: virtio: prioritize staged capture period work Joonho Lee
` (4 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
Staged capture needs request-owned DMA storage and capture accounting
which remains valid when dsnoop sets stop_threshold to the PCM boundary.
A managed-buffer acknowledgment policy alone can exhaust the RX queue
while shared capture clients are still reading.
Introduce staged capture with separate produced, observed and consumed
positions. Select free-running capture policy for boundary stop thresholds
and synchronize policy changes with RX completion and acknowledgment.
Keep ordinary capture and playback on their existing transfer paths.
Serialize prepare, sync_stop, hw_params and hw_free with a lifecycle mutex.
Track submitted control transitions, distinguish uncertain completion from
an immediate rejection, and reject stale software completions after a
virtqueue reset. Revoke prepared and runtime access before cleanup. Keep
DMA storage until device ownership ends, and preserve readable capture
tails without refilling released streams.
The pause-restart option uses STOP/RELEASE and SET_PARAMS/PREPARE/START to
support backends which cannot resume their capture requests after pause.
The short, packed and empty-response compatibility options are independent
and remain disabled by default. This consolidates v7 patches 7, 8, 11, 12,
13 and 16 with the subsequent local review corrections into one coherent
state machine. It supersedes those revisions.
The RESET callback conversion follows separately with its ALSA core
prerequisite. Notification priority is also a separate patch.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
--- a/sound/virtio/virtio_card.c
+++ b/sound/virtio/virtio_card.c
@@ -348,6 +348,18 @@
struct virtio_device *vdev = snd->vdev;
unsigned int i;
+ for (i = 0; snd->substreams && i < snd->nsubstreams; ++i) {
+ struct virtio_pcm_substream *vss = &snd->substreams[i];
+ struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss);
+
+ guard(spinlock_irqsave)(&queue->lock);
+ guard(spinlock)(&vss->lock);
+ vss->reset_epoch++;
+ vss->resetting = true;
+ vss->prepared = false;
+ vss->xfer_enabled = false;
+ vss->suspended = true;
+ }
virtio_reset_device(vdev);
/* Deleting a virtqueue does not unmap its outstanding descriptors. */
@@ -364,9 +376,20 @@
spin_unlock_irqrestore(&queue->lock, flags);
}
+ for (i = 0; i < VIRTIO_SND_VQ_MAX; ++i) {
+ guard(spinlock_irqsave)(&snd->queues[i].lock);
+ snd->queues[i].vqueue = NULL;
+ }
vdev->config->del_vqs(vdev);
- for (i = 0; i < VIRTIO_SND_VQ_MAX; ++i)
- snd->queues[i].vqueue = NULL;
+ for (i = 0; snd->substreams && i < snd->nsubstreams; ++i) {
+ struct virtio_pcm_substream *vss = &snd->substreams[i];
+
+ cancel_work_sync(&vss->elapsed_period);
+ guard(spinlock_irqsave)(&vss->lock);
+ vss->stopped = false;
+ vss->xfer_xrun = false;
+ vss->pause_restart_state = VIRTSND_PCM_PAUSE_RESTART_NONE;
+ }
}
/**
@@ -439,6 +462,7 @@
{
struct virtio_snd *snd = vdev->priv;
int rc;
+ unsigned int i;
rc = virtsnd_find_vqs(snd);
if (rc)
@@ -447,6 +471,12 @@
virtio_device_ready(vdev);
virtsnd_ctl_msg_enable(snd);
+ for (i = 0; i < snd->nsubstreams; ++i) {
+ struct virtio_pcm_substream *vss = &snd->substreams[i];
+
+ guard(spinlock_irqsave)(&vss->lock);
+ vss->resetting = false;
+ }
virtsnd_enable_event_vq(snd);
return 0;
diff --git a/sound/virtio/virtio_ctl_msg.c b/sound/virtio/virtio_ctl_msg.c
--- a/sound/virtio/virtio_ctl_msg.c
+++ b/sound/virtio/virtio_ctl_msg.c
@@ -23,6 +23,7 @@
* @list: Pending message list entry.
* @notify: Request completed notification.
* @ref_count: Reference count used to manage a message lifetime.
+ * @submitted: Request was handed to the device.
* @cancelled: Request canceled during device teardown.
*/
struct virtio_snd_msg {
@@ -37,7 +38,17 @@
struct completion notify;
refcount_t ref_count;
bool cancelled;
+ bool submitted;
};
+
+/* Caller retains a reference across the synchronous send. */
+bool virtsnd_ctl_msg_uncertain(struct virtio_snd_msg *msg, int rc)
+{
+ return msg->submitted &&
+ (rc == -ETIMEDOUT || rc == -ERESTARTSYS ||
+ READ_ONCE(msg->cancelled));
+}
+
/**
* virtsnd_ctl_msg_ref() - Increment reference counter for the message.
@@ -296,6 +307,7 @@
rc = virtqueue_add_sgs(queue->vqueue, psgs, nouts, nins, msg,
GFP_ATOMIC);
if (!rc) {
+ msg->submitted = true;
notify = virtqueue_kick_prepare(queue->vqueue);
list_add_tail(&msg->list, &snd->ctl_msgs);
@@ -315,11 +327,14 @@
virtsnd_ctl_msg_unref(msg);
} else if (!nowait) {
/*
- * Teardown must observe completion even with a pending signal.
- * Otherwise a subsequent open can retry RELEASE while the device
- * is still processing the previous teardown request.
+ * PCM state changes must observe completion with a pending signal.
+ * Otherwise recovery can issue a conflicting command while the
+ * device is still processing the previous transition.
*/
switch (le32_to_cpu(request->code)) {
+ case VIRTIO_SND_R_PCM_SET_PARAMS:
+ case VIRTIO_SND_R_PCM_PREPARE:
+ case VIRTIO_SND_R_PCM_START:
case VIRTIO_SND_R_PCM_STOP:
case VIRTIO_SND_R_PCM_RELEASE:
rc = wait_for_completion_timeout(&msg->notify, js);
diff --git a/sound/virtio/virtio_ctl_msg.h b/sound/virtio/virtio_ctl_msg.h
--- a/sound/virtio/virtio_ctl_msg.h
+++ b/sound/virtio/virtio_ctl_msg.h
@@ -11,6 +11,8 @@
struct virtio_snd;
struct virtio_snd_msg;
+
+bool virtsnd_ctl_msg_uncertain(struct virtio_snd_msg *msg, int rc);
void virtsnd_ctl_msg_ref(struct virtio_snd_msg *msg);
@@ -47,7 +49,7 @@
* The msg_timeout_ms module parameter defines the message completion timeout.
* If the message is not completed within this time, the function will return an
* error.
- * PCM STOP and RELEASE wait through pending signals within this timeout.
+ * PCM state-changing requests wait through pending signals within this timeout.
* Other requests remain interruptible. Device reset cancels and wakes either
* kind of waiter.
*
diff --git a/sound/virtio/virtio_kctl.c b/sound/virtio/virtio_kctl.c
--- a/sound/virtio/virtio_kctl.c
+++ b/sound/virtio/virtio_kctl.c
@@ -159,12 +159,12 @@
struct virtio_snd_ctl_hdr *hdr;
struct virtio_snd_ctl_value *kvalue;
size_t request_size = sizeof(*hdr);
- size_t response_size = sizeof(struct virtio_snd_hdr) + sizeof(*kvalue);
+ size_t response_size = sizeof(struct virtio_snd_hdr);
unsigned int i;
int rc;
- msg = virtsnd_ctl_msg_alloc(request_size, response_size, 0, 0,
- GFP_KERNEL);
+ msg = virtsnd_ctl_msg_alloc(request_size, response_size, 0,
+ sizeof(*kvalue), GFP_KERNEL);
if (!msg)
return -ENOMEM;
@@ -179,8 +179,7 @@
if (rc)
goto on_failure;
- kvalue = (void *)((u8 *)virtsnd_ctl_msg_response(msg) +
- sizeof(struct virtio_snd_hdr));
+ kvalue = virtsnd_ctl_msg_response_data(msg);
switch (type) {
case VIRTIO_SND_CTL_TYPE_BOOLEAN:
diff --git a/sound/virtio/virtio_pcm.c b/sound/virtio/virtio_pcm.c
--- a/sound/virtio/virtio_pcm.c
+++ b/sound/virtio/virtio_pcm.c
@@ -310,8 +310,98 @@
{
struct virtio_pcm_substream *vss =
container_of(work, struct virtio_pcm_substream, elapsed_period);
-
- snd_pcm_period_elapsed(vss->substream);
+ struct snd_pcm_substream *substream = vss->substream;
+ struct snd_pcm_runtime *runtime;
+ size_t period_bytes;
+ u64 period_floor;
+ u64 snapshot;
+ bool staged_capture;
+ bool paused_xrun = false;
+ bool notify = false;
+ bool requeue = false;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ staged_capture = vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
+ vss->capture_staging;
+ }
+
+ if (!staged_capture) {
+ snd_pcm_period_elapsed(substream);
+ return;
+ }
+
+ /* Serialize the pointer snapshot with ALSA's pointer and ACK callbacks. */
+ guard(pcm_stream_lock_irqsave)(substream);
+ if (!substream->runtime)
+ return;
+ if (!snd_pcm_running(substream)) {
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ runtime = substream->runtime;
+ paused_xrun = runtime &&
+ runtime->state == SNDRV_PCM_STATE_PAUSED &&
+ vss->runtime_active && vss->xfer_xrun;
+ if (paused_xrun)
+ virtsnd_pcm_capture_arm_paused_stop(vss);
+ }
+ if (paused_xrun) {
+ /* Stop paused transfers and wake blocked readers. */
+ snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
+ }
+ return;
+ }
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ runtime = substream->runtime;
+ if (!vss->runtime_active ||
+ (!vss->xfer_enabled && !vss->xfer_xrun) || !runtime)
+ return;
+
+ period_bytes = snd_pcm_lib_period_bytes(substream);
+ if (vss->xfer_xrun) {
+ notify = true;
+ } else if (!period_bytes || period_bytes >= vss->buffer_bytes ||
+ !virtsnd_pcm_capture_counters_valid(vss) ||
+ vss->capture_notified_periods >
+ div64_ul(vss->capture_copied_total, period_bytes)) {
+ vss->xfer_xrun = true;
+ notify = true;
+ } else {
+ period_floor = div64_ul(vss->capture_copied_total, period_bytes);
+ if (period_floor > vss->capture_notified_periods) {
+ /* Reserve crossed boundaries before the callback. */
+ vss->capture_notified_periods = period_floor;
+ notify = true;
+ }
+ }
+
+ if (notify) {
+ snapshot = vss->capture_copied_total;
+ vss->capture_callback_snapshot = snapshot;
+ vss->capture_callback_snapshot_active = true;
+ }
+ }
+
+ if (!notify)
+ return;
+
+ /* Pin the callback position while completions keep updating actual totals. */
+ snd_pcm_period_elapsed_under_stream_lock(substream);
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ vss->capture_callback_snapshot_active = false;
+ vss->capture_callback_snapshot = 0;
+ if (vss->runtime_active && vss->xfer_enabled &&
+ !vss->xfer_xrun && substream->runtime) {
+ period_bytes = snd_pcm_lib_period_bytes(substream);
+ if (period_bytes &&
+ div64_ul(vss->capture_copied_total, period_bytes) >
+ vss->capture_notified_periods)
+ requeue = true;
+ }
+ }
+
+ if (requeue)
+ virtsnd_pcm_schedule_period_elapsed(vss);
}
/**
@@ -353,6 +443,7 @@
INIT_WORK(&vss->elapsed_period, virtsnd_pcm_period_elapsed);
init_waitqueue_head(&vss->msg_empty);
spin_lock_init(&vss->lock);
+ mutex_init(&vss->lifecycle_lock);
}
info = kzalloc_objs(*info, snd->nsubstreams);
@@ -486,7 +577,7 @@
snd_pcm_set_ops(vpcm->pcm, i, &virtsnd_pcm_ops[i]);
}
- snd_pcm_set_managed_buffer_all(vpcm->pcm,
+ snd_pcm_lib_preallocate_pages_for_all(vpcm->pcm,
SNDRV_DMA_TYPE_VMALLOC, NULL,
0, 0);
}
@@ -517,8 +608,11 @@
break;
case VIRTIO_SND_EVT_PCM_XRUN:
scoped_guard(spinlock, &vss->lock) {
- if (vss->xfer_enabled)
+ if (vss->xfer_enabled) {
vss->xfer_xrun = true;
+ if (vss->capture_staging)
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ }
}
break;
}
diff --git a/sound/virtio/virtio_pcm.h b/sound/virtio/virtio_pcm.h
--- a/sound/virtio/virtio_pcm.h
+++ b/sound/virtio/virtio_pcm.h
@@ -7,13 +7,32 @@
#define VIRTIO_SND_PCM_H
#include <linux/atomic.h>
+#include <linux/mutex.h>
+#include <linux/math64.h>
#include <linux/virtio_config.h>
+#include <linux/workqueue.h>
#include <sound/pcm.h>
#include <sound/pcm-indirect.h>
struct virtio_pcm;
struct virtio_pcm_msg;
struct page;
+
+enum virtio_pcm_pause_restart_state {
+ VIRTSND_PCM_PAUSE_RESTART_NONE,
+ VIRTSND_PCM_PAUSE_RESTART_CONFIGURED,
+ VIRTSND_PCM_PAUSE_RESTART_SET_PARAMS_SUBMITTED,
+ VIRTSND_PCM_PAUSE_RESTART_PREPARE_SUBMITTED,
+ VIRTSND_PCM_PAUSE_RESTART_PREPARED,
+ VIRTSND_PCM_PAUSE_RESTART_START_SUBMITTED,
+ VIRTSND_PCM_PAUSE_RESTART_RUNNING,
+ VIRTSND_PCM_PAUSE_RESTART_STOP_SUBMITTED,
+ VIRTSND_PCM_PAUSE_RESTART_STOPPED,
+ VIRTSND_PCM_PAUSE_RESTART_RELEASE_SUBMITTED,
+ VIRTSND_PCM_PAUSE_RESTART_RELEASE_SENT,
+ VIRTSND_PCM_PAUSE_RESTART_RELEASED,
+ VIRTSND_PCM_PAUSE_RESTART_RESET_REQUIRED,
+};
/**
* struct virtio_pcm_substream - VirtIO PCM substream.
@@ -26,16 +45,35 @@
* @pcm_indirect: Kernel indirect pcm structure.
* @hw: Kernel ALSA substream hardware descriptor.
* @elapsed_period: Kernel work to handle the elapsed period state.
+ * @lifecycle_lock: Serializes prepare with synchronous release, outside trigger.
+ * @resetting: Device queues are unavailable until restore completes.
+ * @reset_epoch: Invalidates control replies from before device reset.
* @lock: Spinlock that protects fields shared by interrupt handlers and
* substream operators, including @stopped.
* @buffer_bytes: Current buffer size in bytes.
* @hw_ptr: Substream hardware pointer value in bytes [0 ... buffer_bytes).
+ * @capture_ready: Managed-mode bytes copied but not yet acknowledged by ALSA;
+ * always zero in free-running mode.
+ * @capture_boundary: ALSA boundary captured for this prepared epoch.
+ * @capture_copied_total: Monotonic bytes copied into the capture ring.
+ * @capture_acked_total: Monotonic bytes acknowledged by the capture client.
+ * @capture_pointer_seen_total: Last capture position sampled by ALSA.
+ * @capture_notified_periods: Highest full-period boundary reserved for notify.
+ * @capture_callback_snapshot: Capture position fixed for the period callback.
+ * @capture_callback_snapshot_active: The callback must use its fixed position.
+ * @runtime_active: ALSA runtime buffer may be accessed by the staged path.
+ * @capture_staging: Capture uses request-owned payload buffers.
+ * @capture_started: Non-empty capture data was received since prepare.
+ * @capture_empty: Consecutive empty startup completions.
+ * @capture_free_running: Cached staged-capture policy, protected by @lock.
* @xfer_enabled: Data transfer state (0 - off, 1 - on).
* @xfer_xrun: Data underflow/overflow state (0 - no xrun, 1 - xrun).
* @stopped: True if the substream is stopped and must be released on the device
* side; protected by @lock.
+ * @prepared: Device PREPARE succeeded; protected by @lock.
* @suspended: True if the substream is suspended and must be reconfigured on
* the device side at resume.
+ * @pause_restart_state: Device-side state for opt-in pause compatibility.
* @msgs: Allocated I/O messages.
* @nmsgs: Number of allocated I/O messages.
* @msg_count: Number of pending I/O messages in the virtqueue.
@@ -56,12 +94,30 @@
struct snd_pcm_hardware hw;
struct work_struct elapsed_period;
spinlock_t lock;
+ struct mutex lifecycle_lock;
+ u64 reset_epoch;
+ bool resetting;
size_t buffer_bytes;
size_t hw_ptr;
+ size_t capture_ready;
+ snd_pcm_uframes_t capture_boundary;
+ u64 capture_copied_total;
+ u64 capture_acked_total;
+ u64 capture_pointer_seen_total;
+ u64 capture_notified_periods;
+ u64 capture_callback_snapshot;
+ bool capture_callback_snapshot_active;
+ bool runtime_active;
+ bool capture_staging;
+ bool capture_started;
+ bool capture_free_running;
+ unsigned int capture_empty;
bool xfer_enabled;
bool xfer_xrun;
bool stopped;
+ bool prepared;
bool suspended;
+ enum virtio_pcm_pause_restart_state pause_restart_state;
struct virtio_pcm_msg **msgs;
unsigned int nmsgs;
unsigned int msg_count;
@@ -74,6 +130,64 @@
wait_queue_head_t msg_empty;
};
+/* Caller holds the PCM stream lock and vss->lock. */
+static inline void
+virtsnd_pcm_capture_arm_paused_stop(struct virtio_pcm_substream *vss)
+{
+ struct snd_pcm_runtime *runtime = vss->substream->runtime;
+
+ lockdep_assert_held(&vss->lock);
+ if (!runtime || runtime->state != SNDRV_PCM_STATE_PAUSED)
+ return;
+
+ /* ALSA skips trigger(STOP) when moving a paused stream to XRUN. */
+ vss->xfer_enabled = false;
+ vss->stopped = true;
+ runtime->stop_operating = true;
+}
+
+/* Use one notification helper for all producers. */
+static inline bool
+virtsnd_pcm_schedule_period_elapsed(struct virtio_pcm_substream *vss)
+{
+ return schedule_work(&vss->elapsed_period);
+}
+
+/*
+ * ALSA may observe a subperiod tail before its period callback. Keep actual
+ * unread occupancy separate from pointer observations and notification state;
+ * free-running streams account occupancy from monotonic totals instead.
+ */
+static inline size_t virtsnd_pcm_ring_offset(u64 total, size_t size)
+{
+ u64 remainder;
+
+ div64_u64_rem(total, size, &remainder);
+ return remainder;
+}
+
+static inline bool
+virtsnd_pcm_capture_counters_valid(const struct virtio_pcm_substream *vss)
+{
+ u64 outstanding;
+
+ if (!vss->buffer_bytes ||
+ vss->capture_copied_total < vss->capture_acked_total ||
+ vss->capture_pointer_seen_total > vss->capture_copied_total ||
+ vss->hw_ptr != virtsnd_pcm_ring_offset(vss->capture_copied_total, vss->buffer_bytes) ||
+ (vss->capture_callback_snapshot_active &&
+ (vss->capture_callback_snapshot < vss->capture_pointer_seen_total ||
+ vss->capture_callback_snapshot > vss->capture_copied_total)))
+ return false;
+
+ if (vss->capture_free_running)
+ return !vss->capture_ready;
+
+ outstanding = vss->capture_copied_total - vss->capture_acked_total;
+ return outstanding == (u64)vss->capture_ready &&
+ vss->capture_ready < vss->buffer_bytes;
+}
+
/**
* struct virtio_pcm_stream - VirtIO PCM stream.
* @substreams: VirtIO substreams belonging to the stream.
@@ -104,6 +218,11 @@
extern const struct snd_pcm_ops virtsnd_pcm_ops[];
+extern bool virtsnd_capture_staging;
+extern bool virtsnd_capture_packed_status;
+extern bool virtsnd_capture_allow_short;
+extern bool virtsnd_capture_allow_empty;
+
int virtsnd_pcm_validate(struct virtio_device *vdev);
int virtsnd_pcm_parse_cfg(struct virtio_snd *snd);
@@ -127,6 +246,11 @@
int virtsnd_pcm_msg_alloc(struct virtio_pcm_substream *vss,
unsigned int periods, unsigned int period_bytes);
+int virtsnd_pcm_capture_send(struct virtio_pcm_substream *vss);
+
+int virtsnd_pcm_capture_refresh_mode_locked(struct virtio_pcm_substream *vss,
+ bool *free_running);
+
void virtsnd_pcm_msg_free(struct virtio_pcm_substream *vss);
void virtsnd_pcm_msg_reset(struct virtio_pcm_substream *vss);
diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c
--- a/sound/virtio/virtio_pcm_msg.c
+++ b/sound/virtio/virtio_pcm_msg.c
@@ -3,9 +3,11 @@
* virtio-snd: Virtio sound device
* Copyright (C) 2021 OpenSynergy GmbH
*/
+#include <linux/compiler.h>
#include <linux/limits.h>
#include <linux/mm.h>
#include <linux/overflow.h>
+#include <linux/vmalloc.h>
#include <sound/pcm_params.h>
#include "virtio_card.h"
@@ -16,14 +18,16 @@
* @xfer: Request header payload.
* @status: Response header payload.
* @length: Data length in bytes.
+ * @data: Request-owned payload for staged capture.
* @data_offset: Offset in the driver-owned PCM buffer on NOMMU systems.
* @sgs: Payload scatter-gather table.
*/
struct virtio_pcm_msg {
struct virtio_pcm_substream *substream;
- struct virtio_snd_pcm_xfer xfer;
- struct virtio_snd_pcm_status status;
+ struct virtio_snd_pcm_xfer *xfer;
+ struct virtio_snd_pcm_status *status;
size_t length;
+ u8 *data;
#ifndef CONFIG_MMU
size_t data_offset;
#endif
@@ -228,6 +232,8 @@
if (check_mul_overflow((size_t)periods, (size_t)period_bytes,
&buffer_bytes))
return -EOVERFLOW;
+ if (vss->capture_staging && periods < 2)
+ return -EINVAL;
if (!vss->substream || !vss->substream->runtime)
return -EINVAL;
@@ -236,14 +242,19 @@
return -EINVAL;
#ifdef CONFIG_MMU
- rc = virtsnd_pcm_dma_pages_get(vss, runtime->dma_area, buffer_bytes);
- if (rc)
- return rc;
+ if (!vss->capture_staging) {
+ rc = virtsnd_pcm_dma_pages_get(vss, runtime->dma_area,
+ buffer_bytes);
+ if (rc)
+ return rc;
+ }
#else
- vss->owned_dma_area = kzalloc(buffer_bytes, GFP_KERNEL);
- if (!vss->owned_dma_area)
- return -ENOMEM;
- vss->owned_dma_bytes = buffer_bytes;
+ if (!vss->capture_staging) {
+ vss->owned_dma_area = kzalloc(buffer_bytes, GFP_KERNEL);
+ if (!vss->owned_dma_area)
+ return -ENOMEM;
+ vss->owned_dma_bytes = buffer_bytes;
+ }
#endif
vss->msgs = kzalloc_objs(*vss->msgs, periods);
@@ -255,32 +266,54 @@
vss->nmsgs = periods;
for (i = 0; i < periods; ++i) {
- int sg_num;
struct virtio_pcm_msg *msg;
u8 *data;
+ int sg_num;
+
+ if (vss->capture_staging) {
+ data = vzalloc(period_bytes);
+ } else {
#ifdef CONFIG_MMU
- data = runtime->dma_area + (size_t)period_bytes * i;
+ data = runtime->dma_area + (size_t)period_bytes * i;
#else
- data = vss->owned_dma_area + (size_t)period_bytes * i;
+ data = vss->owned_dma_area + (size_t)period_bytes * i;
#endif
+ }
+ if (!data) {
+ rc = -ENOMEM;
+ goto err_free_msgs;
+ }
+
sg_num = virtsnd_pcm_sg_num(data, period_bytes);
if (sg_num < 0) {
+ if (vss->capture_staging)
+ vfree(data);
rc = sg_num;
goto err_free_msgs;
}
msg = kzalloc_flex(*msg, sgs, sg_num + 2);
if (!msg) {
+ if (vss->capture_staging)
+ vfree(data);
rc = -ENOMEM;
goto err_free_msgs;
}
msg->substream = vss;
+ msg->data = vss->capture_staging ? data : NULL;
vss->msgs[i] = msg;
- sg_init_one(&msg->sgs[PCM_MSG_SG_XFER], &msg->xfer,
- sizeof(msg->xfer));
- sg_init_one(&msg->sgs[PCM_MSG_SG_STATUS], &msg->status,
- sizeof(msg->status));
+ /* DMA payloads must not share cachelines with CPU-owned fields. */
+ msg->xfer = kzalloc(sizeof(*msg->xfer), GFP_KERNEL);
+ msg->status = kzalloc(sizeof(*msg->status), GFP_KERNEL);
+ if (!msg->xfer || !msg->status) {
+ rc = -ENOMEM;
+ goto err_free_msgs;
+ }
+ sg_init_one(&msg->sgs[PCM_MSG_SG_XFER], msg->xfer,
+ sizeof(*msg->xfer));
+ sg_init_one(&msg->sgs[PCM_MSG_SG_STATUS], msg->status,
+ sizeof(*msg->status));
#ifndef CONFIG_MMU
msg->data_offset = (size_t)period_bytes * i;
#endif
@@ -316,8 +349,14 @@
{
unsigned int i;
- for (i = 0; vss->msgs && i < vss->nmsgs; ++i)
- kfree(vss->msgs[i]);
+ for (i = 0; vss->msgs && i < vss->nmsgs; ++i) {
+ if (vss->msgs[i]) {
+ vfree(vss->msgs[i]->data);
+ kfree(vss->msgs[i]->xfer);
+ kfree(vss->msgs[i]->status);
+ kfree(vss->msgs[i]);
+ }
+ }
kfree(vss->msgs);
#ifdef CONFIG_MMU
virtsnd_pcm_dma_pages_put(vss);
@@ -339,15 +378,26 @@
* substream was stopped are never sent to the device, so their length must be
* cleared before the substream is started again.
*
- * Context: Any context. Takes and releases the VirtIO substream spinlock.
+ * Context: Caller holds the VirtIO substream spinlock.
*/
void virtsnd_pcm_msg_reset(struct virtio_pcm_substream *vss)
{
unsigned int i;
- guard(spinlock_irqsave)(&vss->lock);
+ lockdep_assert_held(&vss->lock);
for (i = 0; vss->msgs && i < vss->nmsgs; ++i)
vss->msgs[i]->length = 0;
+ vss->capture_ready = 0;
+ vss->capture_copied_total = 0;
+ vss->capture_acked_total = 0;
+ vss->capture_pointer_seen_total = 0;
+ vss->capture_notified_periods = 0;
+ vss->capture_callback_snapshot = 0;
+ vss->capture_callback_snapshot_active = false;
+ vss->capture_boundary = 0;
+ vss->capture_free_running = false;
+ vss->capture_empty = 0;
+ vss->capture_started = false;
}
/**
@@ -377,6 +427,10 @@
bool notify = false;
int rc;
+ if (!vqueue || !vss->prepared || vss->suspended)
+ return -EBADFD;
+ if (!bytes)
+ return 0;
start = offset / period_bytes;
end = (offset + bytes - 1) / period_bytes;
@@ -395,8 +449,8 @@
msg->length += n;
if (msg->length == period_bytes) {
- msg->xfer.stream_id = cpu_to_le32(vss->sid);
- memset(&msg->status, 0, sizeof(msg->status));
+ msg->xfer->stream_id = cpu_to_le32(vss->sid);
+ memset(msg->status, 0, sizeof(*msg->status));
#ifndef CONFIG_MMU
if (vss->direction == SNDRV_PCM_STREAM_PLAYBACK) {
@@ -470,6 +524,290 @@
return 0;
}
+/* Caller holds vss->lock. */
+int virtsnd_pcm_capture_refresh_mode_locked(struct virtio_pcm_substream *vss,
+ bool *free_running)
+{
+ struct snd_pcm_runtime *runtime;
+ snd_pcm_uframes_t stop_threshold;
+ u64 outstanding;
+ bool new_free_running;
+
+ if (!vss->capture_staging || !vss->runtime_active || !vss->substream)
+ return -EBADFD;
+
+ runtime = vss->substream->runtime;
+ if (!runtime || !vss->buffer_bytes || !vss->capture_boundary)
+ return -EBADFD;
+
+ /* Validate the currently committed policy before observing a transition. */
+ if (!virtsnd_pcm_capture_counters_valid(vss))
+ return -EIO;
+
+ stop_threshold = READ_ONCE(runtime->stop_threshold);
+ new_free_running = stop_threshold == vss->capture_boundary;
+ if (new_free_running != vss->capture_free_running) {
+ outstanding = vss->capture_copied_total -
+ vss->capture_acked_total;
+ if (new_free_running) {
+ vss->capture_ready = 0;
+ } else {
+ if (outstanding >= vss->buffer_bytes)
+ return -EPIPE;
+ vss->capture_ready = outstanding;
+ }
+ vss->capture_free_running = new_free_running;
+ }
+
+ *free_running = new_free_running;
+ return 0;
+}
+
+/* Caller holds the RX queue and vss locks and has sampled the policy. */
+static int virtsnd_pcm_capture_enqueue(struct virtio_pcm_substream *vss,
+ bool free_running)
+{
+ struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss);
+ struct virtqueue *vqueue = queue->vqueue;
+ unsigned long period_bytes;
+ size_t free_bytes;
+ size_t request_limit = vss->nmsgs;
+ unsigned int i;
+ bool added = false;
+
+ if (!vqueue || !vss->prepared || vss->suspended ||
+ !vss->capture_staging || !vss->runtime_active)
+ return -EBADFD;
+
+ period_bytes = snd_pcm_lib_period_bytes(vss->substream);
+ if (!period_bytes || !vss->buffer_bytes ||
+ period_bytes >= vss->buffer_bytes ||
+ free_running != vss->capture_free_running ||
+ !virtsnd_pcm_capture_counters_valid(vss) ||
+ vss->msg_count > vss->nmsgs)
+ return -EIO;
+
+ if (!free_running) {
+ free_bytes = vss->buffer_bytes - vss->capture_ready;
+ request_limit = free_bytes / period_bytes +
+ !!(free_bytes % period_bytes);
+ request_limit = min_t(size_t, request_limit, vss->nmsgs);
+ }
+
+ for (i = 0; i < vss->nmsgs && vss->msg_count < request_limit; ++i) {
+ struct virtio_pcm_msg *msg = vss->msgs[i];
+ struct scatterlist *sgs[] = {
+ &msg->sgs[PCM_MSG_SG_XFER],
+ &msg->sgs[PCM_MSG_SG_DATA],
+ &msg->sgs[PCM_MSG_SG_STATUS]
+ };
+ int rc;
+
+ if (msg->length)
+ continue;
+
+ msg->xfer->stream_id = cpu_to_le32(vss->sid);
+ /* Do not let an absent packed trailer reuse old PCM bytes. */
+ memset(msg->data, 0, period_bytes);
+ memset(msg->status, 0, sizeof(*msg->status));
+ msg->status->status = cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
+ rc = virtqueue_add_sgs(vqueue, sgs, 1, 2, msg, GFP_ATOMIC);
+ if (rc) {
+ if (added && !(vss->features &
+ (1U << VIRTIO_SND_PCM_F_MSG_POLLING)) &&
+ virtqueue_kick_prepare(vqueue))
+ virtqueue_notify(vqueue);
+ return rc;
+ }
+
+ msg->length = period_bytes;
+ vss->msg_count++;
+ added = true;
+ }
+
+ if (added && !(vss->features &
+ (1U << VIRTIO_SND_PCM_F_MSG_POLLING)) &&
+ virtqueue_kick_prepare(vqueue))
+ virtqueue_notify(vqueue);
+
+ return 0;
+}
+
+/**
+ * virtsnd_pcm_capture_send() - Refresh policy and queue staged requests.
+ * @vss: VirtIO PCM substream.
+ *
+ * Context: Expects the RX queue and substream spinlocks to be held by caller.
+ * Return: 0 on success, -errno on failure.
+ */
+int virtsnd_pcm_capture_send(struct virtio_pcm_substream *vss)
+{
+ bool free_running;
+ int rc;
+
+ rc = virtsnd_pcm_capture_refresh_mode_locked(vss, &free_running);
+ if (rc)
+ return rc;
+
+ return virtsnd_pcm_capture_enqueue(vss, free_running);
+}
+
+static int virtsnd_pcm_capture_status(const struct virtio_pcm_msg *msg,
+ size_t written_bytes,
+ size_t frame_bytes,
+ size_t *audio_bytes,
+ u32 *latency_bytes)
+{
+ struct virtio_snd_pcm_status status;
+ size_t max_written;
+ size_t first;
+
+ if (!frame_bytes ||
+ check_add_overflow(msg->length, sizeof(status), &max_written) ||
+ written_bytes < sizeof(status) || written_bytes > max_written)
+ return -EIO;
+
+ *audio_bytes = written_bytes - sizeof(status);
+ if (*audio_bytes > msg->length || *audio_bytes % frame_bytes)
+ return -EIO;
+
+ if (virtsnd_capture_packed_status) {
+ first = min_t(size_t, sizeof(status),
+ msg->length - *audio_bytes);
+ if (first)
+ memcpy(&status, msg->data + *audio_bytes, first);
+ if (first < sizeof(status))
+ memcpy((u8 *)&status + first, msg->status,
+ sizeof(status) - first);
+ } else {
+ status = *msg->status;
+ }
+
+ if (le32_to_cpu(status.status) != VIRTIO_SND_S_OK)
+ return -EIO;
+
+ *latency_bytes = le32_to_cpu(status.latency_bytes);
+ return 0;
+}
+
+static void virtsnd_pcm_capture_xrun_locked(struct virtio_pcm_substream *vss)
+{
+ if (!vss->xfer_xrun) {
+ vss->xfer_xrun = true;
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ }
+}
+
+static void virtsnd_pcm_capture_complete(struct virtio_pcm_msg *msg,
+ size_t written_bytes)
+{
+ struct virtio_pcm_substream *vss = msg->substream;
+ size_t audio_bytes = 0;
+ size_t next_ready = 0;
+ u64 next_copied_total = 0;
+ bool free_running = false;
+ bool got_audio = false;
+
+ guard(spinlock)(&vss->lock);
+ vss->msg_count--;
+ if (vss->runtime_active && vss->xfer_enabled && !vss->xfer_xrun) {
+ struct snd_pcm_runtime *runtime = vss->substream->runtime;
+ size_t frame_bytes = runtime ? frames_to_bytes(runtime, 1) : 0;
+ u32 latency_bytes;
+ const char *invalid_reason = NULL;
+
+ /* A full ring is modulo-ambiguous, so fail closed before copying. */
+ if (!runtime || !runtime->dma_area)
+ invalid_reason = "runtime";
+ else if (virtsnd_pcm_capture_refresh_mode_locked(vss,
+ &free_running))
+ invalid_reason = "policy";
+ else if (!virtsnd_pcm_capture_counters_valid(vss))
+ invalid_reason = "counters";
+ else if (vss->hw_ptr !=
+ virtsnd_pcm_ring_offset(vss->capture_copied_total, vss->buffer_bytes))
+ invalid_reason = "hw_ptr";
+ else if (virtsnd_pcm_capture_status(msg, written_bytes,
+ frame_bytes, &audio_bytes,
+ &latency_bytes))
+ invalid_reason = "status";
+ else if (check_add_overflow(vss->capture_copied_total,
+ (u64)audio_bytes,
+ &next_copied_total))
+ invalid_reason = "total_overflow";
+ else if (free_running && audio_bytes >= vss->buffer_bytes)
+ invalid_reason = "payload_exceeds_ring";
+ else if (!free_running &&
+ audio_bytes >= vss->buffer_bytes - vss->capture_ready)
+ invalid_reason = "ring_capacity";
+ else if (!free_running &&
+ check_add_overflow(vss->capture_ready, audio_bytes,
+ &next_ready))
+ invalid_reason = "ready_overflow";
+ else if (!free_running && next_ready >= vss->buffer_bytes)
+ invalid_reason = "ring_full";
+ else if (free_running &&
+ (next_copied_total < vss->capture_pointer_seen_total ||
+ next_copied_total - vss->capture_pointer_seen_total >=
+ vss->buffer_bytes))
+ invalid_reason = "pointer_window";
+ else if (free_running)
+ next_ready = 0;
+
+ if (invalid_reason) {
+ virtsnd_pcm_capture_xrun_locked(vss);
+ dev_warn_ratelimited(&vss->snd->vdev->dev,
+ "SID %u: staged capture completion rejected (%s)\n",
+ vss->sid, invalid_reason);
+ } else if (!audio_bytes) {
+ unsigned int limit = min_t(unsigned int, vss->nmsgs,
+ UINT_MAX / 2) * 2;
+
+ if (!virtsnd_capture_allow_empty || vss->capture_started ||
+ vss->capture_empty >= limit) {
+ virtsnd_pcm_capture_xrun_locked(vss);
+ } else {
+ vss->capture_empty++;
+ }
+ } else if (audio_bytes < msg->length &&
+ !virtsnd_capture_allow_short) {
+ virtsnd_pcm_capture_xrun_locked(vss);
+ } else {
+ size_t ring_ptr = virtsnd_pcm_ring_offset(vss->capture_copied_total,
+ vss->buffer_bytes);
+ size_t first = min(audio_bytes,
+ vss->buffer_bytes - ring_ptr);
+
+ memcpy(runtime->dma_area + ring_ptr, msg->data, first);
+ if (audio_bytes > first)
+ memcpy(runtime->dma_area, msg->data + first,
+ audio_bytes - first);
+ vss->capture_copied_total = next_copied_total;
+ vss->hw_ptr = virtsnd_pcm_ring_offset(next_copied_total, vss->buffer_bytes);
+ vss->capture_ready = next_ready;
+ vss->capture_started = true;
+ vss->capture_empty = 0;
+ runtime->delay = bytes_to_frames(runtime, latency_bytes);
+ got_audio = true;
+ }
+ }
+
+ msg->length = 0;
+ if (vss->runtime_active && vss->xfer_enabled) {
+ if (!vss->xfer_xrun &&
+ virtsnd_pcm_capture_enqueue(vss, free_running)) {
+ dev_warn_ratelimited(&vss->snd->vdev->dev,
+ "SID %u: failed to refill capture queue\n",
+ vss->sid);
+ virtsnd_pcm_capture_xrun_locked(vss);
+ }
+ if (got_audio && !vss->xfer_xrun)
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ } else if (!vss->msg_count) {
+ wake_up_all(&vss->msg_empty);
+ }
+}
+
/**
* virtsnd_pcm_msg_pending_num() - Returns the number of pending I/O messages.
* @vss: VirtIO substream.
@@ -521,9 +859,9 @@
* allowed, then each completed message is immediately placed back at the end
* of the queue.
*
- * For the playback substream, @written_bytes is equal to sizeof(msg->status).
- *
- * For the capture substream, @written_bytes is equal to sizeof(msg->status)
+ * For the playback substream, @written_bytes is equal to sizeof(*msg->status).
+ *
+ * For the capture substream, @written_bytes is equal to sizeof(*msg->status)
* plus the number of captured bytes.
*
* Context: Interrupt context. Takes and releases the VirtIO substream spinlock.
@@ -532,6 +870,12 @@
size_t written_bytes)
{
struct virtio_pcm_substream *vss = msg->substream;
+
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
+ vss->capture_staging) {
+ virtsnd_pcm_capture_complete(msg, written_bytes);
+ return;
+ }
/*
* hw_ptr always indicates the buffer position of the first I/O message
@@ -541,10 +885,11 @@
guard(spinlock)(&vss->lock);
#ifndef CONFIG_MMU
if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
- vss->xfer_enabled && written_bytes > sizeof(msg->status)) {
+ vss->prepared && !vss->stopped &&
+ written_bytes > sizeof(*msg->status)) {
struct snd_pcm_runtime *runtime =
vss->substream ? vss->substream->runtime : NULL;
- size_t reported_bytes = written_bytes - sizeof(msg->status);
+ size_t reported_bytes = written_bytes - sizeof(*msg->status);
size_t data_bytes = min_t(size_t, reported_bytes, msg->length);
if (!runtime || !runtime->dma_area || !vss->owned_dma_area ||
@@ -561,8 +906,8 @@
}
#endif
if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
- written_bytes > sizeof(msg->status) &&
- written_bytes - sizeof(msg->status) > msg->length)
+ written_bytes > sizeof(*msg->status) &&
+ written_bytes - sizeof(*msg->status) > msg->length)
dev_err_ratelimited(&vss->snd->vdev->dev,
"SID %u: capture length exceeds I/O message capacity\n",
vss->sid);
@@ -572,11 +917,11 @@
* the actual SG payload capacity before accounting them.
*/
if (vss->direction == SNDRV_PCM_STREAM_PLAYBACK ||
- written_bytes <= sizeof(msg->status)) {
+ written_bytes <= sizeof(*msg->status)) {
vss->hw_ptr += msg->length;
} else {
size_t data_bytes = min_t(size_t,
- written_bytes - sizeof(msg->status),
+ written_bytes - sizeof(*msg->status),
msg->length);
vss->hw_ptr += data_bytes;
@@ -595,8 +940,8 @@
runtime->delay =
bytes_to_frames(runtime,
- le32_to_cpu(msg->status.latency_bytes));
- schedule_work(&vss->elapsed_period);
+ le32_to_cpu(msg->status->latency_bytes));
+ virtsnd_pcm_schedule_period_elapsed(vss);
} else if (!vss->msg_count) {
wake_up_all(&vss->msg_empty);
}
diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
--- a/sound/virtio/virtio_pcm_ops.c
+++ b/sound/virtio/virtio_pcm_ops.c
@@ -3,9 +3,40 @@
* virtio-snd: Virtio sound device
* Copyright (C) 2021 OpenSynergy GmbH
*/
+#include <linux/moduleparam.h>
+#include <linux/overflow.h>
#include <sound/pcm_params.h>
#include "virtio_card.h"
+
+bool virtsnd_capture_staging;
+module_param_named(capture_staging, virtsnd_capture_staging, bool, 0444);
+MODULE_PARM_DESC(capture_staging,
+ "Use request-owned buffers for experimental PCM capture");
+
+bool virtsnd_capture_packed_status;
+module_param_named(capture_packed_status, virtsnd_capture_packed_status,
+ bool, 0444);
+MODULE_PARM_DESC(capture_packed_status,
+ "Read staged capture status after the actual audio payload");
+
+bool virtsnd_capture_allow_short;
+module_param_named(capture_allow_short, virtsnd_capture_allow_short,
+ bool, 0444);
+MODULE_PARM_DESC(capture_allow_short,
+ "Accept non-empty short staged capture responses");
+
+bool virtsnd_capture_allow_empty;
+module_param_named(capture_allow_empty, virtsnd_capture_allow_empty,
+ bool, 0444);
+MODULE_PARM_DESC(capture_allow_empty,
+ "Retry bounded empty startup capture responses");
+
+static bool virtsnd_capture_pause_restart;
+module_param_named(capture_pause_restart, virtsnd_capture_pause_restart,
+ bool, 0444);
+MODULE_PARM_DESC(capture_pause_restart,
+ "Release and reprepare staged capture streams across pause");
/*
* I/O messages lifetime
@@ -97,14 +128,238 @@
};
static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream);
-
-static void virtsnd_pcm_start_failed(struct virtio_pcm_substream *vss)
-{
- scoped_guard(spinlock_irqsave, &vss->lock) {
+static int virtsnd_pcm_sync_stop_locked(struct snd_pcm_substream *substream);
+static int virtsnd_pcm_pause_restart_sync_stop(struct snd_pcm_substream *substream);
+
+static void virtsnd_pcm_start_failed(struct virtio_pcm_substream *vss, u64 epoch)
+{
+ bool notify;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return;
vss->xfer_enabled = false;
vss->stopped = true;
+ notify = vss->runtime_active && vss->capture_staging &&
+ vss->direction == SNDRV_PCM_STREAM_CAPTURE;
+ if (notify)
+ vss->xfer_xrun = true;
}
vss->substream->runtime->stop_operating = true;
+ if (notify)
+ virtsnd_pcm_schedule_period_elapsed(vss);
+}
+
+static bool
+virtsnd_pcm_pause_restart_enabled(struct virtio_pcm_substream *vss)
+{
+ return virtsnd_capture_pause_restart && vss->capture_staging &&
+ vss->direction == SNDRV_PCM_STREAM_CAPTURE;
+}
+
+static bool virtsnd_pcm_pause_restart_transitioning(enum virtio_pcm_pause_restart_state state)
+{
+ switch (state) {
+ case VIRTSND_PCM_PAUSE_RESTART_SET_PARAMS_SUBMITTED:
+ case VIRTSND_PCM_PAUSE_RESTART_PREPARE_SUBMITTED:
+ case VIRTSND_PCM_PAUSE_RESTART_START_SUBMITTED:
+ case VIRTSND_PCM_PAUSE_RESTART_STOP_SUBMITTED:
+ case VIRTSND_PCM_PAUSE_RESTART_RELEASE_SUBMITTED:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static int virtsnd_pcm_send_sync(struct virtio_pcm_substream *vss,
+ struct virtio_snd_msg *msg)
+{
+ u64 epoch;
+ int rc;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting) {
+ virtsnd_ctl_msg_unref(msg);
+ return -ECANCELED;
+ }
+ epoch = vss->reset_epoch;
+ }
+ rc = virtsnd_ctl_msg_send_sync(vss->snd, msg);
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (epoch != vss->reset_epoch || vss->resetting)
+ return -ECANCELED;
+ }
+ return rc;
+}
+
+static int
+virtsnd_pcm_pause_restart_send(struct virtio_pcm_substream *vss,
+ struct virtio_snd_msg *msg,
+ unsigned long allowed_states,
+ enum virtio_pcm_pause_restart_state submitted_state,
+ enum virtio_pcm_pause_restart_state success_state)
+{
+ enum virtio_pcm_pause_restart_state state;
+ bool uncertain;
+ u64 epoch;
+ int rc;
+
+ if (!virtsnd_pcm_pause_restart_enabled(vss))
+ return virtsnd_pcm_send_sync(vss, msg);
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ epoch = vss->reset_epoch;
+ state = vss->pause_restart_state;
+ if (vss->resetting) {
+ rc = -ECANCELED;
+ } else if (state == VIRTSND_PCM_PAUSE_RESTART_RESET_REQUIRED) {
+ rc = -EIO;
+ } else if (virtsnd_pcm_pause_restart_transitioning(state)) {
+ vss->pause_restart_state =
+ VIRTSND_PCM_PAUSE_RESTART_RESET_REQUIRED;
+ rc = -EIO;
+ } else if (!(allowed_states & BIT(state))) {
+ rc = -EBADFD;
+ } else {
+ vss->pause_restart_state = submitted_state;
+ rc = 0;
+ }
+ }
+
+ if (rc) {
+ virtsnd_ctl_msg_unref(msg);
+ return rc;
+ }
+
+ virtsnd_ctl_msg_ref(msg);
+ rc = virtsnd_ctl_msg_send_sync(vss->snd, msg);
+ uncertain = virtsnd_ctl_msg_uncertain(msg, rc);
+ virtsnd_ctl_msg_unref(msg);
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->reset_epoch != epoch || vss->resetting)
+ return -ECANCELED;
+ if ((rc && uncertain) || vss->pause_restart_state != submitted_state) {
+ vss->pause_restart_state =
+ VIRTSND_PCM_PAUSE_RESTART_RESET_REQUIRED;
+ if (!rc)
+ rc = -EIO;
+ } else if (rc) {
+ vss->pause_restart_state = state;
+ } else {
+ vss->pause_restart_state = success_state;
+ }
+ }
+
+ if (rc && uncertain)
+ dev_err(&vss->snd->vdev->dev,
+ "SID %u: PCM control state is unknown; device reprobe required\n",
+ vss->sid);
+
+ return rc;
+}
+
+static void
+virtsnd_pcm_pause_restart_fail(struct virtio_pcm_substream *vss,
+ bool reset_required, u64 epoch)
+{
+ bool notify_xrun;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return;
+ notify_xrun = vss->runtime_active;
+ vss->xfer_enabled = false;
+ vss->stopped = true;
+ vss->xfer_xrun = true;
+ if (reset_required)
+ vss->pause_restart_state =
+ VIRTSND_PCM_PAUSE_RESTART_RESET_REQUIRED;
+ }
+ if (notify_xrun)
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ vss->substream->runtime->stop_operating = true;
+}
+
+static int virtsnd_pcm_dev_prepare(struct virtio_pcm_substream *vss)
+{
+ struct virtio_snd_msg *msg;
+
+ msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_PREPARE,
+ GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ return virtsnd_pcm_pause_restart_send(vss, msg,
+ BIT(VIRTSND_PCM_PAUSE_RESTART_CONFIGURED) |
+ BIT(VIRTSND_PCM_PAUSE_RESTART_RELEASED) |
+ BIT(VIRTSND_PCM_PAUSE_RESTART_STOPPED) |
+ BIT(VIRTSND_PCM_PAUSE_RESTART_PREPARED),
+ VIRTSND_PCM_PAUSE_RESTART_PREPARE_SUBMITTED,
+ VIRTSND_PCM_PAUSE_RESTART_PREPARED);
+}
+
+static int virtsnd_pcm_pause_prepare(struct virtio_pcm_substream *vss)
+{
+ struct virtio_snd_msg *msg;
+
+ msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_PREPARE,
+ GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ return virtsnd_pcm_pause_restart_send(vss, msg,
+ BIT(VIRTSND_PCM_PAUSE_RESTART_RELEASED),
+ VIRTSND_PCM_PAUSE_RESTART_PREPARE_SUBMITTED,
+ VIRTSND_PCM_PAUSE_RESTART_PREPARED);
+}
+
+static int virtsnd_pcm_release_wait(struct virtio_pcm_substream *vss, u64 epoch)
+{
+ unsigned int js = msecs_to_jiffies(virtsnd_msg_timeout_ms);
+ long waited;
+
+ waited = wait_event_timeout(vss->msg_empty,
+ !virtsnd_pcm_msg_pending_num(vss), js);
+ guard(spinlock_irqsave)(&vss->lock);
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ if (!waited || vss->msg_count ||
+ vss->pause_restart_state != VIRTSND_PCM_PAUSE_RESTART_RELEASE_SENT)
+ return -ETIMEDOUT;
+ vss->prepared = false;
+ vss->pause_restart_state = VIRTSND_PCM_PAUSE_RESTART_RELEASED;
+ return 0;
+}
+
+static int
+virtsnd_pcm_pause_release_flush(struct virtio_pcm_substream *vss,
+ unsigned long allowed_states)
+{
+ struct virtio_snd_msg *msg;
+ u64 epoch;
+ int rc;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting)
+ return -ECANCELED;
+ epoch = vss->reset_epoch;
+ }
+ msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_RELEASE,
+ GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+ rc = virtsnd_pcm_pause_restart_send(vss, msg, allowed_states,
+ VIRTSND_PCM_PAUSE_RESTART_RELEASE_SUBMITTED,
+ VIRTSND_PCM_PAUSE_RESTART_RELEASE_SENT);
+ if (rc)
+ return rc;
+ return virtsnd_pcm_release_wait(vss, epoch);
+}
+
+static void virtsnd_pcm_pause_restart_failed(struct virtio_pcm_substream *vss,
+ u64 epoch)
+{
+ virtsnd_pcm_pause_restart_fail(vss, false, epoch);
}
/**
@@ -119,23 +374,34 @@
struct virtio_pcm *vpcm = snd_pcm_substream_chip(substream);
struct virtio_pcm_stream *vs = &vpcm->streams[substream->stream];
struct virtio_pcm_substream *vss = vs->substreams[substream->number];
+ int rc;
substream->runtime->hw = vss->hw;
substream->private_data = vss;
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ vss->capture_staging = virtsnd_capture_staging &&
+ vss->direction == SNDRV_PCM_STREAM_CAPTURE;
+ if (vss->capture_staging) {
+ vss->runtime_active = false;
+ vss->xfer_enabled = false;
+ }
+ }
snd_pcm_hw_constraint_integer(substream->runtime,
SNDRV_PCM_HW_PARAM_PERIODS);
scoped_guard(spinlock_irqsave, &vss->lock)
vss->stopped |= !!vss->msg_count;
- vss->suspended = false;
/*
* If the substream has already been used, then the I/O queue may be in
* an invalid state. Just in case, we do a check and try to return the
* queue to its original state, if necessary.
*/
- return virtsnd_pcm_sync_stop(substream);
+ rc = virtsnd_pcm_sync_stop(substream);
+ if (rc)
+ cancel_work_sync(&vss->elapsed_period);
+ return rc;
}
/**
@@ -147,6 +413,16 @@
*/
static int virtsnd_pcm_close(struct snd_pcm_substream *substream)
{
+ struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+
+ if (vss->capture_staging) {
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ vss->runtime_active = false;
+ vss->xfer_enabled = false;
+ }
+ cancel_work_sync(&vss->elapsed_period);
+ }
+
return 0;
}
@@ -213,7 +489,14 @@
request->features |=
cpu_to_le32(1U << VIRTIO_SND_PCM_F_EVT_XRUNS);
- return virtsnd_ctl_msg_send_sync(vss->snd, msg);
+ return virtsnd_pcm_pause_restart_send(vss, msg,
+ BIT(VIRTSND_PCM_PAUSE_RESTART_NONE) |
+ BIT(VIRTSND_PCM_PAUSE_RESTART_CONFIGURED) |
+ BIT(VIRTSND_PCM_PAUSE_RESTART_PREPARED) |
+ BIT(VIRTSND_PCM_PAUSE_RESTART_STOPPED) |
+ BIT(VIRTSND_PCM_PAUSE_RESTART_RELEASED),
+ VIRTSND_PCM_PAUSE_RESTART_SET_PARAMS_SUBMITTED,
+ VIRTSND_PCM_PAUSE_RESTART_CONFIGURED);
}
/**
@@ -233,6 +516,22 @@
bool stopped;
int rc;
+ guard(mutex)(&vss->lifecycle_lock);
+ /* Drain the previous buffer before ALSA can replace its mapping. */
+ rc = virtsnd_pcm_sync_stop_locked(substream);
+ if (rc)
+ return rc;
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ vss->prepared = false;
+
+ if (vss->capture_staging) {
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ vss->runtime_active = false;
+ vss->xfer_enabled = false;
+ }
+ cancel_work_sync(&vss->elapsed_period);
+ }
+
scoped_guard(spinlock_irqsave, &vss->lock) {
msg_count = vss->msg_count;
stopped = vss->stopped;
@@ -248,6 +547,11 @@
substream->runtime->stop_operating = true;
return -EBADFD;
}
+
+ rc = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(hw_params));
+ if (rc < 0)
+ return rc;
+ substream->runtime->buffer_changed = rc > 0;
rc = virtsnd_pcm_dev_set_params(vss, params_buffer_bytes(hw_params),
params_period_bytes(hw_params),
@@ -263,8 +567,14 @@
*/
virtsnd_pcm_msg_free(vss);
- return virtsnd_pcm_msg_alloc(vss, params_periods(hw_params),
- params_period_bytes(hw_params));
+ rc = virtsnd_pcm_msg_alloc(vss, params_periods(hw_params),
+ params_period_bytes(hw_params));
+ if (!rc && vss->capture_staging) {
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ vss->runtime_active = true;
+ }
+
+ return rc;
}
/**
@@ -277,27 +587,90 @@
static int virtsnd_pcm_hw_free(struct snd_pcm_substream *substream)
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+ int rc;
+
+ guard(mutex)(&vss->lifecycle_lock);
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ vss->stopped |= vss->prepared;
+ vss->prepared = false;
+ vss->runtime_active = false;
+ vss->xfer_enabled = false;
+ }
+ rc = virtsnd_pcm_sync_stop_locked(substream);
+
+ if (vss->capture_staging) {
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ vss->runtime_active = false;
+ vss->xfer_enabled = false;
+ }
+ cancel_work_sync(&vss->elapsed_period);
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ vss->hw_ptr = 0;
+ vss->capture_ready = 0;
+ vss->capture_copied_total = 0;
+ vss->capture_acked_total = 0;
+ vss->capture_pointer_seen_total = 0;
+ vss->capture_notified_periods = 0;
+ vss->capture_callback_snapshot = 0;
+ vss->capture_callback_snapshot_active = false;
+ vss->capture_boundary = 0;
+ vss->capture_free_running = false;
+ }
+ }
/* If the queue is flushed, we can safely free the messages here. */
if (!virtsnd_pcm_msg_pending_num(vss))
virtsnd_pcm_msg_free(vss);
- return 0;
+ snd_pcm_lib_free_pages(substream);
+ return rc;
}
/**
- * virtsnd_pcm_prepare() - Prepare the PCM substream.
+ * virtsnd_pcm_prepare_locked() - Prepare the PCM substream.
* @substream: Kernel ALSA substream.
*
* Context: Process context.
* Return: 0 on success, -errno on failure.
*/
-static int virtsnd_pcm_prepare(struct snd_pcm_substream *substream)
+static int virtsnd_pcm_prepare_locked(struct snd_pcm_substream *substream)
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
struct virtio_device *vdev = vss->snd->vdev;
- struct virtio_snd_msg *msg;
+ bool pause_restart = virtsnd_pcm_pause_restart_enabled(vss);
+ bool free_running;
+ int rc;
bool stopped;
+ bool suspended;
+ u64 epoch;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting)
+ return -ECANCELED;
+ stopped = vss->stopped;
+ }
+ if (stopped) {
+ rc = virtsnd_pcm_sync_stop_locked(substream);
+ if (rc)
+ return rc;
+ }
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ vss->prepared = false;
+ epoch = vss->reset_epoch;
+ suspended = vss->suspended;
+ }
+
+ if (pause_restart) {
+ enum virtio_pcm_pause_restart_state state;
+
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ state = vss->pause_restart_state;
+ if (state == VIRTSND_PCM_PAUSE_RESTART_RESET_REQUIRED ||
+ virtsnd_pcm_pause_restart_transitioning(state)) {
+ virtsnd_pcm_pause_restart_fail(vss, true, epoch);
+ return -EIO;
+ }
+ }
scoped_guard(spinlock_irqsave, &vss->lock)
stopped = vss->stopped;
@@ -308,44 +681,320 @@
return -EBADFD;
}
- if (!vss->suspended) {
+ if (!suspended) {
if (virtsnd_pcm_msg_pending_num(vss)) {
dev_err(&vdev->dev, "SID %u: invalid I/O queue state\n",
vss->sid);
return -EBADFD;
}
- vss->buffer_bytes = snd_pcm_lib_buffer_bytes(substream);
- vss->hw_ptr = 0;
} else {
struct snd_pcm_runtime *runtime = substream->runtime;
unsigned int buffer_bytes = snd_pcm_lib_buffer_bytes(substream);
unsigned int period_bytes = snd_pcm_lib_period_bytes(substream);
- int rc;
rc = virtsnd_pcm_dev_set_params(vss, buffer_bytes, period_bytes,
runtime->channels,
runtime->format, runtime->rate);
- if (rc)
+ if (rc) {
+ if (pause_restart && rc != -ECANCELED)
+ virtsnd_pcm_pause_restart_fail(vss, false, epoch);
return rc;
- }
-
- vss->xfer_xrun = false;
- vss->suspended = false;
- vss->msg_count = 0;
- virtsnd_pcm_msg_reset(vss);
-
- memset(&vss->pcm_indirect, 0, sizeof(vss->pcm_indirect));
- vss->pcm_indirect.sw_buffer_size =
- vss->pcm_indirect.hw_buffer_size =
- snd_pcm_lib_buffer_bytes(substream);
-
- msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_PREPARE,
- GFP_KERNEL);
- if (!msg)
+ }
+ }
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (epoch != vss->reset_epoch || vss->resetting)
+ return -ECANCELED;
+ vss->prepared = false;
+ vss->buffer_bytes = snd_pcm_lib_buffer_bytes(substream);
+ vss->hw_ptr = 0;
+ if (!pause_restart)
+ vss->xfer_xrun = false;
+ vss->suspended = false;
+ vss->msg_count = 0;
+ virtsnd_pcm_msg_reset(vss);
+ memset(&vss->pcm_indirect, 0, sizeof(vss->pcm_indirect));
+ vss->pcm_indirect.sw_buffer_size = vss->buffer_bytes;
+ vss->pcm_indirect.hw_buffer_size = vss->buffer_bytes;
+ if (vss->capture_staging) {
+ vss->capture_boundary = substream->runtime->boundary;
+ rc = virtsnd_pcm_capture_refresh_mode_locked(vss,
+ &free_running);
+ if (rc)
+ return rc;
+ }
+ }
+
+ rc = virtsnd_pcm_dev_prepare(vss);
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (epoch != vss->reset_epoch || vss->resetting)
+ return -ECANCELED;
+ vss->prepared = !rc;
+ }
+ if (pause_restart) {
+ if (rc) {
+ if (rc == -ECANCELED)
+ return rc;
+ virtsnd_pcm_pause_restart_fail(vss, false, epoch);
+ } else {
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ vss->stopped = false;
+ vss->xfer_xrun = false;
+ }
+ }
+ }
+
+ return rc;
+}
+
+static int virtsnd_pcm_prepare(struct snd_pcm_substream *substream)
+{
+ struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+
+ guard(mutex)(&vss->lifecycle_lock);
+ return virtsnd_pcm_prepare_locked(substream);
+}
+
+static int virtsnd_pcm_pause_restart_start(struct virtio_pcm_substream *vss,
+ bool resume)
+{
+ struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss);
+ struct virtio_snd_msg *msg;
+ enum virtio_pcm_pause_restart_state state;
+ bool reset_required;
+ bool invalid_state;
+ int rc = 0;
+ u64 epoch;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting)
+ return -ECANCELED;
+ epoch = vss->reset_epoch;
+ state = vss->pause_restart_state;
+ reset_required =
+ state == VIRTSND_PCM_PAUSE_RESTART_RESET_REQUIRED ||
+ virtsnd_pcm_pause_restart_transitioning(state);
+ invalid_state =
+ vss->xfer_xrun ||
+ (resume &&
+ (state != VIRTSND_PCM_PAUSE_RESTART_RELEASED ||
+ !vss->stopped || vss->msg_count ||
+ !vss->runtime_active)) ||
+ (!resume &&
+ (state != VIRTSND_PCM_PAUSE_RESTART_PREPARED ||
+ vss->stopped));
+ }
+ if (reset_required) {
+ virtsnd_pcm_pause_restart_fail(vss, true, epoch);
+ return -EIO;
+ }
+ if (invalid_state) {
+ virtsnd_pcm_pause_restart_fail(vss, false, epoch);
+ return -EBADFD;
+ }
+
+ if (resume) {
+ rc = virtsnd_pcm_pause_prepare(vss);
+ if (rc) {
+ if (rc == -ECANCELED)
+ return rc;
+ virtsnd_pcm_pause_restart_failed(vss, epoch);
+ return rc;
+ }
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting || vss->reset_epoch != epoch) {
+ rc = -ECANCELED;
+ } else if (vss->pause_restart_state !=
+ VIRTSND_PCM_PAUSE_RESTART_PREPARED ||
+ vss->msg_count || !vss->runtime_active) {
+ rc = -EBADFD;
+ } else {
+ /* Preserve ring accounting, restarting only startup grace. */
+ vss->prepared = true;
+ vss->capture_started = false;
+ vss->capture_empty = 0;
+ }
+ }
+ if (rc) {
+ if (rc == -ECANCELED)
+ return rc;
+ virtsnd_pcm_pause_restart_failed(vss, epoch);
+ return rc;
+ }
+ }
+
+ scoped_guard(spinlock_irqsave, &queue->lock) {
+ guard(spinlock)(&vss->lock);
+ if (vss->resetting || vss->reset_epoch != epoch) {
+ rc = -ECANCELED;
+ } else if (vss->pause_restart_state !=
+ VIRTSND_PCM_PAUSE_RESTART_PREPARED ||
+ vss->msg_count || !vss->runtime_active || vss->xfer_xrun) {
+ rc = -EBADFD;
+ } else {
+ rc = virtsnd_pcm_capture_send(vss);
+ if (!rc)
+ vss->xfer_enabled = true;
+ }
+ }
+ if (rc) {
+ if (rc == -ECANCELED)
+ return rc;
+ virtsnd_pcm_pause_restart_failed(vss, epoch);
+ return rc;
+ }
+
+ msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_START, GFP_KERNEL);
+ if (!msg) {
+ virtsnd_pcm_pause_restart_failed(vss, epoch);
return -ENOMEM;
-
- return virtsnd_ctl_msg_send_sync(vss->snd, msg);
+ }
+
+ rc = virtsnd_pcm_pause_restart_send(vss, msg,
+ BIT(VIRTSND_PCM_PAUSE_RESTART_PREPARED),
+ VIRTSND_PCM_PAUSE_RESTART_START_SUBMITTED,
+ VIRTSND_PCM_PAUSE_RESTART_RUNNING);
+ if (rc) {
+ if (rc == -ECANCELED)
+ return rc;
+ virtsnd_pcm_pause_restart_failed(vss, epoch);
+ return rc;
+ }
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ vss->stopped = false;
+ }
+ virtsnd_pcm_schedule_period_elapsed(vss);
+
+ return 0;
+}
+
+static int virtsnd_pcm_pause_restart_pause(struct virtio_pcm_substream *vss)
+{
+ struct virtio_snd_msg *msg;
+ enum virtio_pcm_pause_restart_state state;
+ int rc;
+ u64 epoch;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting)
+ return -ECANCELED;
+ epoch = vss->reset_epoch;
+ state = vss->pause_restart_state;
+ if (state == VIRTSND_PCM_PAUSE_RESTART_RELEASED) {
+ if (vss->xfer_xrun)
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ return vss->xfer_xrun ? -EPIPE : 0;
+ }
+ vss->xfer_enabled = false;
+ vss->stopped = true;
+ }
+
+ if (state == VIRTSND_PCM_PAUSE_RESTART_RELEASE_SENT) {
+ /* Trigger holds the PCM lock; this helper never cancels work. */
+ rc = virtsnd_pcm_release_wait(vss, epoch);
+ if (!rc) {
+ guard(spinlock_irqsave)(&vss->lock);
+ rc = vss->xfer_xrun ? -EPIPE : 0;
+ }
+ if (rc && rc != -ECANCELED)
+ virtsnd_pcm_pause_restart_failed(vss, epoch);
+ return rc;
+ }
+
+ if (state == VIRTSND_PCM_PAUSE_RESTART_RESET_REQUIRED ||
+ virtsnd_pcm_pause_restart_transitioning(state)) {
+ virtsnd_pcm_pause_restart_fail(vss, true, epoch);
+ return -EIO;
+ }
+
+ if (state == VIRTSND_PCM_PAUSE_RESTART_RUNNING) {
+ msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_STOP,
+ GFP_KERNEL);
+ if (!msg) {
+ virtsnd_pcm_pause_restart_failed(vss, epoch);
+ return -ENOMEM;
+ }
+
+ rc = virtsnd_pcm_pause_restart_send(vss, msg,
+ BIT(VIRTSND_PCM_PAUSE_RESTART_RUNNING),
+ VIRTSND_PCM_PAUSE_RESTART_STOP_SUBMITTED,
+ VIRTSND_PCM_PAUSE_RESTART_STOPPED);
+ if (rc) {
+ if (rc == -ECANCELED)
+ return rc;
+ virtsnd_pcm_pause_restart_failed(vss, epoch);
+ return rc;
+ }
+ state = VIRTSND_PCM_PAUSE_RESTART_STOPPED;
+ } else if (state != VIRTSND_PCM_PAUSE_RESTART_STOPPED &&
+ state != VIRTSND_PCM_PAUSE_RESTART_PREPARED) {
+ virtsnd_pcm_pause_restart_failed(vss, epoch);
+ return -EBADFD;
+ }
+
+ rc = virtsnd_pcm_pause_release_flush(vss,
+ BIT(VIRTSND_PCM_PAUSE_RESTART_STOPPED) |
+ BIT(VIRTSND_PCM_PAUSE_RESTART_PREPARED));
+ if (rc) {
+ if (rc == -ECANCELED)
+ return rc;
+ virtsnd_pcm_pause_restart_failed(vss, epoch);
+ return rc;
+ }
+
+ return 0;
+}
+
+static int virtsnd_pcm_pause_restart_stop(struct virtio_pcm_substream *vss,
+ bool suspend)
+{
+ struct virtio_snd_msg *msg;
+ enum virtio_pcm_pause_restart_state state;
+ int rc;
+ u64 epoch;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting)
+ return -ECANCELED;
+ epoch = vss->reset_epoch;
+ if (!vss->prepared && !vss->msg_count)
+ return 0;
+ if (suspend)
+ vss->suspended = true;
+ vss->xfer_enabled = false;
+ vss->stopped = true;
+ state = vss->pause_restart_state;
+ }
+
+ if (state == VIRTSND_PCM_PAUSE_RESTART_RESET_REQUIRED ||
+ virtsnd_pcm_pause_restart_transitioning(state)) {
+ virtsnd_pcm_pause_restart_fail(vss, true, epoch);
+ return -EIO;
+ }
+ if (state != VIRTSND_PCM_PAUSE_RESTART_RUNNING)
+ return 0;
+
+ msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_STOP, GFP_KERNEL);
+ if (!msg) {
+ virtsnd_pcm_pause_restart_failed(vss, epoch);
+ return -ENOMEM;
+ }
+
+ rc = virtsnd_pcm_pause_restart_send(vss, msg,
+ BIT(VIRTSND_PCM_PAUSE_RESTART_RUNNING),
+ VIRTSND_PCM_PAUSE_RESTART_STOP_SUBMITTED,
+ VIRTSND_PCM_PAUSE_RESTART_STOPPED);
+ if (rc && rc != -ECANCELED)
+ virtsnd_pcm_pause_restart_failed(vss, epoch);
+
+ return rc;
}
/**
@@ -360,10 +1009,31 @@
static int virtsnd_pcm_trigger(struct snd_pcm_substream *substream, int command)
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
- struct virtio_snd *snd = vss->snd;
struct virtio_snd_queue *queue;
struct virtio_snd_msg *msg;
int rc = 0;
+
+ u64 epoch;
+
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ epoch = vss->reset_epoch;
+
+ if (virtsnd_pcm_pause_restart_enabled(vss)) {
+ switch (command) {
+ case SNDRV_PCM_TRIGGER_START:
+ return virtsnd_pcm_pause_restart_start(vss, false);
+ case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+ return virtsnd_pcm_pause_restart_start(vss, true);
+ case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ return virtsnd_pcm_pause_restart_pause(vss);
+ case SNDRV_PCM_TRIGGER_STOP:
+ return virtsnd_pcm_pause_restart_stop(vss, false);
+ case SNDRV_PCM_TRIGGER_SUSPEND:
+ return virtsnd_pcm_pause_restart_stop(vss, true);
+ default:
+ return -EINVAL;
+ }
+ }
switch (command) {
case SNDRV_PCM_TRIGGER_START:
@@ -372,38 +1042,51 @@
scoped_guard(spinlock_irqsave, &queue->lock) {
guard(spinlock)(&vss->lock);
- if (vss->stopped) {
- substream->runtime->stop_operating = true;
- return -EBADFD;
+ if (vss->resetting)
+ return -ECANCELED;
+ if (vss->stopped || !vss->prepared || vss->xfer_xrun) {
+ rc = vss->xfer_xrun ? -EPIPE : -EBADFD;
+ goto start_failed;
}
- if (vss->direction == SNDRV_PCM_STREAM_CAPTURE)
- rc = virtsnd_pcm_msg_send(vss, 0, vss->buffer_bytes);
- if (rc) {
- vss->stopped = true;
- substream->runtime->stop_operating = true;
- return rc;
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE) {
+ if (vss->capture_staging) {
+ if (!vss->runtime_active)
+ rc = -EBADFD;
+ else
+ rc = virtsnd_pcm_capture_send(vss);
+ } else {
+ rc = virtsnd_pcm_msg_send(vss, 0,
+ vss->buffer_bytes);
+ }
}
+ if (rc)
+ goto start_failed;
vss->xfer_enabled = true;
}
msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_START,
GFP_KERNEL);
if (!msg) {
- virtsnd_pcm_start_failed(vss);
+ virtsnd_pcm_start_failed(vss, epoch);
return -ENOMEM;
}
- rc = virtsnd_ctl_msg_send_sync(snd, msg);
- if (rc)
- virtsnd_pcm_start_failed(vss);
-
+ rc = virtsnd_pcm_send_sync(vss, msg);
+ if (rc && rc != -ECANCELED)
+ virtsnd_pcm_start_failed(vss, epoch);
+
+ if (!rc && vss->capture_staging)
+ virtsnd_pcm_schedule_period_elapsed(vss);
return rc;
case SNDRV_PCM_TRIGGER_SUSPEND:
- vss->suspended = true;
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ vss->suspended = true;
fallthrough;
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (!vss->prepared)
+ return 0;
if (command != SNDRV_PCM_TRIGGER_PAUSE_PUSH)
vss->stopped = true;
vss->xfer_enabled = false;
@@ -414,14 +1097,136 @@
if (!msg)
return -ENOMEM;
- return virtsnd_ctl_msg_send_sync(snd, msg);
+ return virtsnd_pcm_send_sync(vss, msg);
default:
return -EINVAL;
}
+
+start_failed:
+ virtsnd_pcm_start_failed(vss, epoch);
+ return rc;
+}
+
+static int
+virtsnd_pcm_pause_restart_sync_stop(struct snd_pcm_substream *substream)
+{
+ struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+ unsigned int js = msecs_to_jiffies(virtsnd_msg_timeout_ms);
+ enum virtio_pcm_pause_restart_state state;
+ struct virtio_snd_msg *msg;
+ long waited;
+ int rc;
+ u64 epoch;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting)
+ return -ECANCELED;
+ epoch = vss->reset_epoch;
+ vss->xfer_enabled = false;
+ vss->stopped = true;
+ state = vss->pause_restart_state;
+ }
+ cancel_work_sync(&vss->elapsed_period);
+
+ if (state == VIRTSND_PCM_PAUSE_RESTART_RESET_REQUIRED ||
+ virtsnd_pcm_pause_restart_transitioning(state)) {
+ virtsnd_pcm_pause_restart_fail(vss, true, epoch);
+ dev_err(&vss->snd->vdev->dev,
+ "SID %u: PCM state is unknown; device reprobe required\n",
+ vss->sid);
+ return -EIO;
+ }
+
+ if (state == VIRTSND_PCM_PAUSE_RESTART_NONE ||
+ state == VIRTSND_PCM_PAUSE_RESTART_CONFIGURED) {
+ if (virtsnd_pcm_msg_pending_num(vss)) {
+ virtsnd_pcm_pause_restart_fail(vss, true, epoch);
+ return -EIO;
+ }
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ vss->stopped = false;
+ }
+ return 0;
+ }
+
+ if (state == VIRTSND_PCM_PAUSE_RESTART_RUNNING) {
+ msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_STOP,
+ GFP_KERNEL);
+ if (!msg) {
+ virtsnd_pcm_pause_restart_fail(vss, false, epoch);
+ return -ENOMEM;
+ }
+
+ rc = virtsnd_pcm_pause_restart_send(vss, msg,
+ BIT(VIRTSND_PCM_PAUSE_RESTART_RUNNING),
+ VIRTSND_PCM_PAUSE_RESTART_STOP_SUBMITTED,
+ VIRTSND_PCM_PAUSE_RESTART_STOPPED);
+ if (rc) {
+ if (rc == -ECANCELED)
+ return rc;
+ virtsnd_pcm_pause_restart_fail(vss, false, epoch);
+ return rc;
+ }
+ state = VIRTSND_PCM_PAUSE_RESTART_STOPPED;
+ }
+
+ if (state == VIRTSND_PCM_PAUSE_RESTART_RELEASE_SENT ||
+ state == VIRTSND_PCM_PAUSE_RESTART_RELEASED) {
+ waited = wait_event_timeout(vss->msg_empty,
+ !virtsnd_pcm_msg_pending_num(vss), js);
+ if (waited <= 0) {
+ rc = waited ? (int)waited : -ETIMEDOUT;
+ virtsnd_pcm_pause_restart_fail(vss, false, epoch);
+ return rc;
+ }
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ if (vss->msg_count) {
+ rc = -ETIMEDOUT;
+ } else {
+ vss->pause_restart_state =
+ VIRTSND_PCM_PAUSE_RESTART_RELEASED;
+ vss->prepared = false;
+ vss->stopped = false;
+ rc = 0;
+ }
+ }
+ if (rc)
+ virtsnd_pcm_pause_restart_fail(vss, false, epoch);
+ return rc;
+ }
+
+ if (state != VIRTSND_PCM_PAUSE_RESTART_PREPARED &&
+ state != VIRTSND_PCM_PAUSE_RESTART_STOPPED) {
+ virtsnd_pcm_pause_restart_fail(vss, true, epoch);
+ return -EIO;
+ }
+
+ rc = virtsnd_pcm_pause_release_flush(vss,
+ BIT(VIRTSND_PCM_PAUSE_RESTART_PREPARED) |
+ BIT(VIRTSND_PCM_PAUSE_RESTART_STOPPED));
+ if (rc) {
+ if (rc == -ECANCELED)
+ return rc;
+ virtsnd_pcm_pause_restart_fail(vss, false, epoch);
+ return rc;
+ }
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ vss->prepared = false;
+ vss->stopped = false;
+ }
+
+ return 0;
}
/**
- * virtsnd_pcm_sync_stop() - Synchronous PCM substream stop.
+ * virtsnd_pcm_sync_stop_locked() - Synchronous PCM substream stop.
* @substream: Kernel ALSA substream.
*
* The function can be called both from the upper level or from the driver
@@ -430,7 +1235,7 @@
* Context: Process context. Takes and releases the VirtIO substream spinlock.
* Return: 0 on success, -errno on failure.
*/
-static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream)
+static int virtsnd_pcm_sync_stop_locked(struct snd_pcm_substream *substream)
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
struct virtio_snd *snd = vss->snd;
@@ -438,10 +1243,17 @@
unsigned int js = msecs_to_jiffies(virtsnd_msg_timeout_ms);
bool need_release;
int rc;
-
- scoped_guard(spinlock_irqsave, &vss->lock) {
+ u64 epoch;
+
+ if (virtsnd_pcm_pause_restart_enabled(vss))
+ return virtsnd_pcm_pause_restart_sync_stop(substream);
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting)
+ return -ECANCELED;
+ epoch = vss->reset_epoch;
vss->xfer_enabled = false;
- need_release = vss->stopped || vss->msg_count;
+ need_release = vss->prepared || vss->stopped || vss->msg_count;
if (need_release)
vss->stopped = true;
}
@@ -456,7 +1268,7 @@
return -ENOMEM;
}
- rc = virtsnd_ctl_msg_send_sync(snd, msg);
+ rc = virtsnd_pcm_send_sync(vss, msg);
if (rc) {
substream->runtime->stop_operating = true;
return rc;
@@ -468,7 +1280,7 @@
* Thus, we consider the absence of I/O messages in the queue as an
* indication that the substream has been released.
*/
- rc = wait_event_interruptible_timeout(vss->msg_empty,
+ rc = wait_event_timeout(vss->msg_empty,
!virtsnd_pcm_msg_pending_num(vss),
js);
if (rc <= 0) {
@@ -479,8 +1291,12 @@
return !rc ? -ETIMEDOUT : rc;
}
- scoped_guard(spinlock_irqsave, &vss->lock)
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ vss->prepared = false;
vss->stopped = false;
+ }
return 0;
}
@@ -515,6 +1331,44 @@
virtsnd_pcm_cp_pointer(struct snd_pcm_substream *substream)
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+ size_t hw_ptr = 0;
+ u64 total;
+ bool free_running;
+ bool xrun;
+
+ if (vss->capture_staging) {
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ xrun = vss->xfer_xrun;
+ if (!xrun &&
+ virtsnd_pcm_capture_refresh_mode_locked(vss,
+ &free_running))
+ xrun = true;
+ if (!xrun && !virtsnd_pcm_capture_counters_valid(vss))
+ xrun = true;
+ if (!xrun) {
+ total = vss->capture_callback_snapshot_active ?
+ vss->capture_callback_snapshot :
+ vss->capture_copied_total;
+ /* A full-ring observation gap is modulo-ambiguous. */
+ if (total < vss->capture_pointer_seen_total ||
+ total - vss->capture_pointer_seen_total >=
+ vss->buffer_bytes) {
+ xrun = true;
+ vss->xfer_xrun = true;
+ } else {
+ vss->capture_pointer_seen_total = total;
+ hw_ptr = virtsnd_pcm_ring_offset(total, vss->buffer_bytes);
+ }
+ }
+ if (xrun)
+ vss->xfer_xrun = true;
+ }
+
+ if (xrun)
+ return SNDRV_PCM_POS_XRUN;
+
+ return bytes_to_frames(substream->runtime, hw_ptr);
+ }
return snd_pcm_indirect_capture_pointer(substream,
&vss->pcm_indirect,
@@ -533,7 +1387,9 @@
struct virtsnd_pcm_transfer *transfer =
container_of(rec, struct virtsnd_pcm_transfer, indirect);
- if (transfer->error)
+ if (transfer->error ||
+ (substream->stream == SNDRV_PCM_STREAM_CAPTURE &&
+ substream->runtime->state == SNDRV_PCM_STATE_DRAINING))
return;
transfer->error = virtsnd_pcm_msg_send(vss, rec->sw_data, bytes);
@@ -556,6 +1412,8 @@
scoped_guard(spinlock_irqsave, &queue->lock) {
guard(spinlock)(&vss->lock);
+ if (!vss->prepared)
+ return -EBADFD;
transfer.indirect = vss->pcm_indirect;
if (vss->stopped) {
rc = -EPIPE;
@@ -582,17 +1440,120 @@
return 0;
}
+static int virtsnd_pcm_capture_staged_ack(struct virtio_pcm_substream *vss,
+ struct snd_pcm_runtime *runtime)
+{
+ struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss);
+ int rc = 0;
+
+ /*
+ * The RX callback holds the queue lock while draining completions.
+ * Account ALSA's read under the substream lock alone so completions later
+ * in that drain observe the newly released ring space.
+ */
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ snd_pcm_uframes_t appl_ptr;
+ snd_pcm_sframes_t diff;
+ size_t bytes;
+ u64 outstanding;
+ u64 next_acked_total;
+ bool free_running;
+
+ if (!vss->runtime_active || !runtime || !runtime->control ||
+ !runtime->dma_area || vss->suspended)
+ return -EBADFD;
+ if (!vss->prepared && runtime->state != SNDRV_PCM_STATE_DRAINING &&
+ !(runtime->state == SNDRV_PCM_STATE_PAUSED &&
+ virtsnd_pcm_pause_restart_enabled(vss) &&
+ vss->pause_restart_state == VIRTSND_PCM_PAUSE_RESTART_RELEASED))
+ return -EBADFD;
+ if (vss->xfer_xrun)
+ return -EPIPE;
+ rc = virtsnd_pcm_capture_refresh_mode_locked(vss,
+ &free_running);
+ if (rc || !virtsnd_pcm_capture_counters_valid(vss)) {
+ vss->xfer_xrun = true;
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ return -EPIPE;
+ }
+
+ appl_ptr = runtime->control->appl_ptr;
+ diff = appl_ptr - vss->pcm_indirect.appl_ptr;
+ if (diff < -(snd_pcm_sframes_t)(runtime->boundary / 2))
+ diff += runtime->boundary;
+ if (diff < 0 ||
+ (snd_pcm_uframes_t)diff > runtime->buffer_size) {
+ vss->xfer_xrun = true;
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ return -EPIPE;
+ }
+
+ /* ACK includes any subperiod tail already copied to the ring. */
+ bytes = frames_to_bytes(runtime, diff);
+ outstanding = vss->capture_copied_total -
+ vss->capture_acked_total;
+ if ((!free_running && bytes > vss->capture_ready) ||
+ (free_running &&
+ (bytes > vss->buffer_bytes || bytes > outstanding)) ||
+ check_add_overflow(vss->capture_acked_total, (u64)bytes,
+ &next_acked_total) ||
+ next_acked_total > vss->capture_pointer_seen_total) {
+ vss->xfer_xrun = true;
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ return -EPIPE;
+ }
+
+ vss->capture_acked_total = next_acked_total;
+ if (!free_running)
+ vss->capture_ready -= bytes;
+ vss->pcm_indirect.appl_ptr = appl_ptr;
+ }
+
+ /* Preserve the queue -> substream lock order while refilling requests. */
+ scoped_guard(spinlock_irqsave, &queue->lock) {
+ guard(spinlock)(&vss->lock);
+ if (vss->runtime_active && vss->xfer_enabled &&
+ !vss->xfer_xrun) {
+ rc = virtsnd_pcm_capture_send(vss);
+ if (rc) {
+ vss->xfer_xrun = true;
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ }
+ }
+ }
+
+ if (rc)
+ dev_warn_ratelimited(&vss->snd->vdev->dev,
+ "SID %u: failed to refill capture queue after ACK: %d\n",
+ vss->sid, rc);
+
+ /* The ACK is committed; a refill failure is reported by the next XRUN. */
+ return 0;
+}
+
static int virtsnd_pcm_cp_ack(struct snd_pcm_substream *substream)
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss);
+ struct snd_pcm_runtime *runtime = substream->runtime;
struct virtsnd_pcm_transfer transfer = {};
int rc;
+ if (vss->capture_staging) {
+ rc = virtsnd_pcm_capture_staged_ack(vss, runtime);
+ if (rc == -EPIPE) {
+ guard(spinlock_irqsave)(&vss->lock);
+ virtsnd_pcm_capture_arm_paused_stop(vss);
+ }
+ return rc;
+ }
+
scoped_guard(spinlock_irqsave, &queue->lock) {
guard(spinlock)(&vss->lock);
+ if (!vss->prepared && runtime->state != SNDRV_PCM_STATE_DRAINING)
+ return -EBADFD;
transfer.indirect = vss->pcm_indirect;
- if (vss->stopped) {
+ if (vss->stopped && runtime->state != SNDRV_PCM_STATE_DRAINING) {
rc = -EPIPE;
} else {
rc = snd_pcm_indirect_capture_transfer(substream,
@@ -617,7 +1578,19 @@
return 0;
}
-/* PCM substream operators map. */
+static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream)
+{
+ struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+
+ guard(mutex)(&vss->lifecycle_lock);
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ /* A prepare may already have consumed this stopped generation. */
+ if (!vss->stopped)
+ return 0;
+ }
+ return virtsnd_pcm_sync_stop_locked(substream);
+}
+
const struct snd_pcm_ops virtsnd_pcm_ops[] = {
{
.open = virtsnd_pcm_open,
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v8 10/14] ALSA: virtio: prioritize staged capture period work
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (8 preceding siblings ...)
2026-10-29 5:43 ` [PATCH v8 09/14] ALSA: virtio: add staged capture accounting and lifecycle recovery Joonho Lee
@ 2026-10-29 5:43 ` Joonho Lee
2026-10-29 5:48 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 11/14] docs: describe virtio-sound capture options Joonho Lee
` (3 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
Queue staged capture period notifications on system_highpri_wq through
the common scheduling helper. This allows capture notifications to make
progress when ordinary workqueue workers are delayed by CPU contention.
Playback and ordinary capture keep schedule_work().
This preserves the existing priority proposal without mixing RESET or
lifecycle changes into it. It provides no hard real-time deadline and
cannot prevent a userspace reader from overrunning its buffer.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
diff --git a/sound/virtio/virtio_pcm.h b/sound/virtio/virtio_pcm.h
--- a/sound/virtio/virtio_pcm.h
+++ b/sound/virtio/virtio_pcm.h
@@ -146,10 +146,14 @@
runtime->stop_operating = true;
}
-/* Use one notification helper for all producers. */
+/* Keep non-staged PCM notifications on schedule_work()'s normal queue. */
static inline bool
virtsnd_pcm_schedule_period_elapsed(struct virtio_pcm_substream *vss)
{
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
+ vss->capture_staging)
+ return queue_work(system_highpri_wq, &vss->elapsed_period);
+
return schedule_work(&vss->elapsed_period);
}
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v8 11/14] docs: describe virtio-sound capture options
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (9 preceding siblings ...)
2026-10-29 5:43 ` [PATCH v8 10/14] ALSA: virtio: prioritize staged capture period work Joonho Lee
@ 2026-10-29 5:43 ` Joonho Lee
2026-10-29 5:47 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 12/14] ALSA: pcm: add an optional reset operation under the stream lock Joonho Lee
` (2 subsequent siblings)
13 siblings, 1 reply; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
Document the five opt-in capture module parameters introduced by this
series. All default to false. Describe the startup-empty, short-payload,
packed-status and pause-restart compatibility behavior alongside staging,
and require validation with the target device.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index e75344f..6c7b1b3 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -8328,6 +8328,36 @@ Kernel parameters
brightness level.
default: 1
+ virtio_snd.capture_allow_empty= [HW,SOUND]
+ Retry a bounded number of empty startup capture replies.
+ Format: <bool>
+ Default: false. Compatibility option for devices that
+ require staged capture; validate with the target device.
+
+ virtio_snd.capture_allow_short= [HW,SOUND]
+ Accept non-empty, frame-aligned short capture replies.
+ Format: <bool>
+ Default: false. Compatibility option for devices that
+ require staged capture; validate with the target device.
+
+ virtio_snd.capture_packed_status= [HW,SOUND]
+ Read status immediately after the returned capture data.
+ Format: <bool>
+ Default: false. Compatibility option for devices that
+ require staged capture; validate with the target device.
+
+ virtio_snd.capture_pause_restart= [HW,SOUND]
+ Release and reprepare capture streams across pause.
+ Format: <bool>
+ Default: false. Compatibility option for devices that
+ require staged capture; validate with the target device.
+
+ virtio_snd.capture_staging= [HW,SOUND]
+ Use request-owned buffers for capture data.
+ Format: <bool>
+ Default: false. Compatibility option for devices that
+ require staged capture; validate with the target device.
+
virtio_mmio.device=
[VMMIO] Memory mapped virtio (platform) device.
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 12/14] ALSA: pcm: add an optional reset operation under the stream lock
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (10 preceding siblings ...)
2026-10-29 5:43 ` [PATCH v8 11/14] docs: describe virtio-sound capture options Joonho Lee
@ 2026-10-29 5:43 ` Joonho Lee
2026-10-29 5:52 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 13/14] ALSA: virtio: synchronize staged capture reset with PCM positions Joonho Lee
2026-10-29 5:43 ` [PATCH v8 14/14] ALSA: virtio: synchronize indirect capture positions on reset Joonho Lee
13 siblings, 1 reply; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
A driver which keeps private capture counters needs to reset those
counters together with the PCM application's position. The ioctl RESET
path releases the stream lock before core completes the position rebase,
and appl_ptr is updated in a separate post_action. Period processing can
observe the intermediate state.
Add an optional reset operation whose stream lock remains held through
the core position and appl_ptr updates. Export the common pointer reset
as snd_pcm_lib_reset(), so drivers can reuse pointer validation, time
accounting and XRUN handling without reacquiring the stream lock.
Drivers without the new operation keep their existing ioctl path.
A successful callback member completes its reset immediately. If a later
linked member fails, the first error is returned and completed members
are not rolled back. Legacy members retain the existing post_action
behavior, including in mixed groups. An error can leave partial changes
in the failing member as well. Callers must recover the group before
relying on its capture positions again.
The operation also runs after a successful driver prepare. A later
prepare failure can leave an earlier member reset without the group's
post_prepare state transition. Document this together with the atomic
and nonatomic locking requirements.
Linked synthetic capture tests cover both locking modes, four callback
combinations, later-member errors and recovery. The lock-window concern
was found by inspection; the existing delayed v7 test did not reproduce
a user-visible failure. The new-candidate tests validate the
synchronization contract and recovery paths.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -77,6 +77,20 @@
unsigned long offset);
int (*mmap)(struct snd_pcm_substream *substream, struct vm_area_struct *vma);
int (*ack)(struct snd_pcm_substream *substream);
+ /*
+ * Optional RESET operation, also used after a successful prepare.
+ * Called with this substream's PCM lock held; atomic streams must not
+ * sleep. Do not reacquire the PCM lock through snd_pcm_lib_ioctl().
+ * snd_pcm_lib_reset() provides the common pointer update under this
+ * lock. The core completes the position and appl_ptr rebase before
+ * unlocking a successful member.
+ *
+ * Return zero on success or a negative error. Linked RESET/prepare
+ * stops at the first error without rollback. Earlier callback members
+ * retain their completed reset; legacy members retain the existing
+ * post_action policy. An error does not guarantee unchanged state.
+ */
+ int (*reset)(struct snd_pcm_substream *substream);
};
/*
@@ -1179,6 +1193,7 @@
{
substream->runtime->std_sync_id = true;
}
+int snd_pcm_lib_reset(struct snd_pcm_substream *substream);
int snd_pcm_lib_ioctl(struct snd_pcm_substream *substream,
unsigned int cmd, void *arg);
void snd_pcm_period_elapsed_under_stream_lock(struct snd_pcm_substream *substream);
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -1760,20 +1760,39 @@
}
EXPORT_SYMBOL(snd_pcm_hw_params_bits);
-static int snd_pcm_lib_ioctl_reset(struct snd_pcm_substream *substream,
- void *arg)
+/**
+ * snd_pcm_lib_reset - Reset the pointer while holding the PCM stream lock
+ * @substream: PCM substream
+ *
+ * Context: PCM stream lock held. The caller must not hold driver locks
+ * acquired by the pointer or XRUN callbacks.
+ *
+ * Update a running stream through the ordinary pointer path. Other states
+ * start at zero. Pointer errors keep the existing RESET return semantics;
+ * a zero return does not imply recovery from XRUN.
+ * This helper does not update appl_ptr or acquire the stream lock.
+ *
+ * Return: zero.
+ */
+int snd_pcm_lib_reset(struct snd_pcm_substream *substream)
{
struct snd_pcm_runtime *runtime = substream->runtime;
- guard(pcm_stream_lock_irqsave)(substream);
- if (snd_pcm_running(substream) &&
- snd_pcm_update_hw_ptr(substream) >= 0)
+ if (snd_pcm_running(substream) && snd_pcm_update_hw_ptr(substream) >= 0)
runtime->status->hw_ptr %= runtime->buffer_size;
else {
runtime->status->hw_ptr = 0;
runtime->hw_ptr_wrap = 0;
}
return 0;
+}
+EXPORT_SYMBOL(snd_pcm_lib_reset);
+
+static int snd_pcm_lib_ioctl_reset(struct snd_pcm_substream *substream,
+ void *arg)
+{
+ guard(pcm_stream_lock_irqsave)(substream);
+ return snd_pcm_lib_reset(substream);
}
static int snd_pcm_lib_ioctl_channel_info(struct snd_pcm_substream *substream,
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1989,7 +1989,31 @@
snd_pcm_state_t state)
{
struct snd_pcm_runtime *runtime = substream->runtime;
- int err = snd_pcm_ops_ioctl(substream, SNDRV_PCM_IOCTL1_RESET, NULL);
+ int err;
+
+ if (substream->ops->reset) {
+ /*
+ * Complete this member before releasing its PCM lock. A later
+ * linked member may fail; RESET has no group rollback operation.
+ */
+ guard(pcm_stream_lock_irq)(substream);
+
+ err = substream->ops->reset(substream);
+ if (err < 0)
+ return err;
+ runtime->hw_ptr_base = 0;
+ runtime->hw_ptr_interrupt = runtime->status->hw_ptr -
+ runtime->status->hw_ptr % runtime->period_size;
+ runtime->silence_start = runtime->status->hw_ptr;
+ runtime->silence_filled = 0;
+ runtime->control->appl_ptr = runtime->status->hw_ptr;
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
+ runtime->silence_size > 0)
+ snd_pcm_playback_silence(substream, ULONG_MAX);
+ return 0;
+ }
+
+ err = snd_pcm_ops_ioctl(substream, SNDRV_PCM_IOCTL1_RESET, NULL);
if (err < 0)
return err;
guard(pcm_stream_lock_irq)(substream);
@@ -2005,6 +2029,9 @@
snd_pcm_state_t state)
{
struct snd_pcm_runtime *runtime = substream->runtime;
+
+ if (substream->ops->reset)
+ return;
guard(pcm_stream_lock_irq)(substream);
runtime->control->appl_ptr = runtime->status->hw_ptr;
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v8 13/14] ALSA: virtio: synchronize staged capture reset with PCM positions
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (11 preceding siblings ...)
2026-10-29 5:43 ` [PATCH v8 12/14] ALSA: pcm: add an optional reset operation under the stream lock Joonho Lee
@ 2026-10-29 5:43 ` Joonho Lee
2026-10-29 6:05 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 14/14] ALSA: virtio: synchronize indirect capture positions on reset Joonho Lee
13 siblings, 1 reply; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
Staged capture maintains private copied and acknowledged byte totals.
Its ioctl RESET handler cannot keep the PCM lock held until core has
rebased appl_ptr. It also rebases the copied total, which loses the
relationship to completions arriving after the pointer observation.
Select the optional PCM reset operation when opening staged capture. At
this step, ordinary capture and playback keep the legacy RESET
completion policy, including when a later linked member fails. Call the
common snd_pcm_lib_reset() before acquiring the queue and substream
locks. For a running stream, retain copied_total and advance acked_total
only to pointer_seen_total. Completions after that observation remain
readable. For other states, align the private counters with the common
reset position. The operation table stays fixed for the opened runtime.
Complete the core rebase even if refilling RX fails, then report XRUN
through the period worker. A successful RESET return therefore does not
promise a usable running stream. A device-generation cancellation can
return -ECANCELED after common pointer processing; recovery still needs
the normal prepare path.
This patch depends on the core reset operation in patch 1 and the
pending staged capture and lifecycle work described in the cover letter.
Earlier narrow-candidate ARM64 QEMU tests forced RX progress between the
pointer observation and accounting, covering two capture policies and
10, 20 and 40 ms gaps. Two 1-vCPU runs crossed the ring boundary and
matched all 3,360,000 samples over the following 70 seconds, then
reopened successfully. Intentional-loss controls were detected. KASAN
and lockdep were quiet. The current narrow candidate has not been tested
with VZ and MOTU.
The ordinary capture RESET failure remains reproducible at this step.
Patch 3 handles its indirect accounting in a separate operation.
Playback continues to use the existing ioctl RESET completion path.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
--- a/sound/virtio/virtio_pcm_ops.c
+++ b/sound/virtio/virtio_pcm_ops.c
@@ -8,6 +8,8 @@
#include <sound/pcm_params.h>
#include "virtio_card.h"
+
+static const struct snd_pcm_ops virtsnd_pcm_capture_staged_ops;
bool virtsnd_capture_staging;
module_param_named(capture_staging, virtsnd_capture_staging, bool, 0444);
@@ -387,6 +389,11 @@
}
}
+ /* Keep the legacy RESET completion policy outside staged capture. */
+ substream->ops = vss->capture_staging ?
+ &virtsnd_pcm_capture_staged_ops :
+ &virtsnd_pcm_ops[substream->stream];
+
snd_pcm_hw_constraint_integer(substream->runtime,
SNDRV_PCM_HW_PARAM_PERIODS);
@@ -1591,6 +1598,58 @@
return virtsnd_pcm_sync_stop_locked(substream);
}
+/* The core holds the stream lock through the matching appl_ptr reset. */
+static int virtsnd_pcm_reset(struct snd_pcm_substream *substream)
+{
+ struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss);
+ size_t position;
+ size_t period;
+ int rc;
+
+ /* Keep ALSA's pointer validation, time accounting and XRUN processing. */
+ rc = snd_pcm_lib_reset(substream);
+ if (rc || !vss->capture_staging)
+ return rc;
+
+ guard(spinlock_irqsave)(&queue->lock);
+ guard(spinlock)(&vss->lock);
+ if (vss->resetting)
+ return -ECANCELED;
+ /* The common reset has already handled a pointer-reported XRUN. */
+ if (vss->xfer_xrun)
+ return 0;
+ if (snd_pcm_running(substream)) {
+ /* Keep completions arriving after the core's pointer observation. */
+ vss->capture_acked_total = vss->capture_pointer_seen_total;
+ vss->capture_ready = vss->capture_free_running ? 0 :
+ vss->capture_copied_total - vss->capture_acked_total;
+ } else {
+ position = frames_to_bytes(runtime, runtime->status->hw_ptr);
+ vss->hw_ptr = position;
+ period = snd_pcm_lib_period_bytes(substream);
+ vss->capture_copied_total = position;
+ vss->capture_acked_total = position;
+ vss->capture_pointer_seen_total = position;
+ vss->capture_ready = 0;
+ vss->capture_notified_periods = period ? position / period : 0;
+ }
+ vss->capture_callback_snapshot = 0;
+ vss->capture_callback_snapshot_active = false;
+ vss->pcm_indirect.appl_ptr = runtime->status->hw_ptr;
+ if (snd_pcm_running(substream) && vss->xfer_enabled && vss->prepared) {
+ rc = virtsnd_pcm_capture_send(vss);
+ if (rc) {
+ vss->xfer_xrun = true;
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ }
+ }
+ /* Complete the matching core pointer rebase before reporting XRUN. */
+ return 0;
+}
+
+/* PCM substream operators map. */
const struct snd_pcm_ops virtsnd_pcm_ops[] = {
{
.open = virtsnd_pcm_open,
@@ -1617,3 +1676,18 @@
.ack = virtsnd_pcm_cp_ack,
},
};
+
+/* Selected at open; the staging policy remains fixed for that runtime. */
+static const struct snd_pcm_ops virtsnd_pcm_capture_staged_ops = {
+ .open = virtsnd_pcm_open,
+ .close = virtsnd_pcm_close,
+ .ioctl = snd_pcm_lib_ioctl,
+ .reset = virtsnd_pcm_reset,
+ .hw_params = virtsnd_pcm_hw_params,
+ .hw_free = virtsnd_pcm_hw_free,
+ .prepare = virtsnd_pcm_prepare,
+ .trigger = virtsnd_pcm_trigger,
+ .sync_stop = virtsnd_pcm_sync_stop,
+ .pointer = virtsnd_pcm_cp_pointer,
+ .ack = virtsnd_pcm_cp_ack,
+};
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v8 14/14] ALSA: virtio: synchronize indirect capture positions on reset
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (12 preceding siblings ...)
2026-10-29 5:43 ` [PATCH v8 13/14] ALSA: virtio: synchronize staged capture reset with PCM positions Joonho Lee
@ 2026-10-29 5:43 ` Joonho Lee
2026-10-29 6:06 ` sashiko-bot
13 siblings, 1 reply; 29+ messages in thread
From: Joonho Lee @ 2026-10-29 5:43 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
RESET rebases the PCM application position without updating the indirect
capture accounting. The next ACK can interpret that change as a backward
application move and return -EPIPE. In a QEMU reproduction, the PCM
application position changed from 24000 to 2880 frames while the private
position stayed at 24000. Both running and paused capture failed with
free-running and buffer-sized stop thresholds.
Use a separate reset operation for indirect capture. The PCM stream lock
covers the common pointer reset, private accounting and core completion.
Discard the observed unread data and return its buffer space to RX. Keep
the hardware and software ring offsets so in-flight completions continue
at their existing DMA positions. Set the private application position to
the position core will publish on completion.
A paused device retains its fixed DMA positions. Keep the observed
software ring position as the reset position in this state, instead of
making userspace read from offset zero while the device continues at a
different offset. Completions after the observation are retained.
If returning RX space fails, stop further transfers and return the
error. The caller must drop and prepare or reopen the stream after this
partial failure. The existing staged reset operation and playback are
unchanged.
This patch depends on the proposed PCM reset operation and helper in
patch 1. The series diff applies after patch 2 because both touch the
operation tables. The indirect reset implementation also applies on top
of patch 1 alone; an alternate local diff is supplied for that order.
The reproduction uses the pending virtio-snd stack with local follow-up
corrections described in the cover letter.
The earlier RW capture tests covered running and paused RESET, both stop
thresholds, a 240 ms read backlog, refill-error recovery and reopen. The
core-only plus indirect variant also passed the four RW reset cases.
Additional direct MMAP_INTERLEAVED tests used 48 kHz mono S16_LE in
ARM64 QEMU. The pre-fix running and paused cases failed after RESET with
ACK -EPIPE. All 13 fixed runs passed, including two 1-vCPU 70-second
captures, refill -ENOSPC injection, managed overrun recovery and reopen.
All 9,072,000 captured frames matched the deterministic source at their
expected positions. RESET discards observed unread data; verification
starts at the resulting position. Recovery and reopen checks start at
the device's new START position.
KASAN and lockdep reported no errors. These MMAP runs used the combined
local stack with staging disabled. Physical VZ/MOTU, other MMAP layouts,
multi-channel input and concurrent RESET between mmap_begin and
mmap_commit remain untested for this candidate.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
--- a/sound/virtio/virtio_pcm_ops.c
+++ b/sound/virtio/virtio_pcm_ops.c
@@ -10,6 +10,7 @@
#include "virtio_card.h"
static const struct snd_pcm_ops virtsnd_pcm_capture_staged_ops;
+static const struct snd_pcm_ops virtsnd_pcm_capture_indirect_ops;
bool virtsnd_capture_staging;
module_param_named(capture_staging, virtsnd_capture_staging, bool, 0444);
@@ -389,10 +390,13 @@
}
}
- /* Keep the legacy RESET completion policy outside staged capture. */
+ /* Keep RESET operation selection fixed for the opened runtime. */
substream->ops = vss->capture_staging ?
&virtsnd_pcm_capture_staged_ops :
&virtsnd_pcm_ops[substream->stream];
+
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE && !vss->capture_staging)
+ substream->ops = &virtsnd_pcm_capture_indirect_ops;
snd_pcm_hw_constraint_integer(substream->runtime,
SNDRV_PCM_HW_PARAM_PERIODS);
@@ -1596,6 +1600,54 @@
return 0;
}
return virtsnd_pcm_sync_stop_locked(substream);
+}
+
+/* The PCM lock covers both indirect accounting and core completion. */
+static int virtsnd_pcm_indirect_reset(struct snd_pcm_substream *substream)
+{
+ struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss);
+ struct virtsnd_pcm_transfer transfer = {};
+ int rc;
+
+ rc = snd_pcm_lib_reset(substream);
+ if (rc)
+ return rc;
+ if (!snd_pcm_running(substream) &&
+ runtime->state != SNDRV_PCM_STATE_PAUSED)
+ return 0;
+
+ guard(spinlock_irqsave)(&queue->lock);
+ guard(spinlock)(&vss->lock);
+ if (vss->resetting)
+ return -ECANCELED;
+ if (!vss->prepared || vss->stopped)
+ return -EBADFD;
+
+ /* Pausing does not move the device's fixed DMA buffer positions. */
+ if (runtime->state == SNDRV_PCM_STATE_PAUSED)
+ runtime->status->hw_ptr =
+ bytes_to_frames(runtime, vss->pcm_indirect.sw_io);
+
+ transfer.indirect = vss->pcm_indirect;
+ transfer.indirect.sw_ready = 0;
+ /* Consume no application delta while returning discarded RX space. */
+ transfer.indirect.appl_ptr = runtime->control->appl_ptr;
+ rc = snd_pcm_indirect_capture_transfer(substream, &transfer.indirect,
+ virtsnd_pcm_trans_copy);
+ if (!rc)
+ rc = transfer.error;
+ if (rc) {
+ virtsnd_pcm_transfer_failed(vss);
+ return rc;
+ }
+
+ /* RESET discards all samples through the observed software pointer. */
+ transfer.indirect.sw_ready = 0;
+ transfer.indirect.appl_ptr = runtime->status->hw_ptr;
+ vss->pcm_indirect = transfer.indirect;
+ return 0;
}
/* The core holds the stream lock through the matching appl_ptr reset. */
@@ -1691,3 +1743,17 @@
.pointer = virtsnd_pcm_cp_pointer,
.ack = virtsnd_pcm_cp_ack,
};
+
+static const struct snd_pcm_ops virtsnd_pcm_capture_indirect_ops = {
+ .open = virtsnd_pcm_open,
+ .close = virtsnd_pcm_close,
+ .ioctl = snd_pcm_lib_ioctl,
+ .reset = virtsnd_pcm_indirect_reset,
+ .hw_params = virtsnd_pcm_hw_params,
+ .hw_free = virtsnd_pcm_hw_free,
+ .prepare = virtsnd_pcm_prepare,
+ .trigger = virtsnd_pcm_trigger,
+ .sync_stop = virtsnd_pcm_sync_stop,
+ .pointer = virtsnd_pcm_cp_pointer,
+ .ack = virtsnd_pcm_cp_ack,
+};
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 11/14] docs: describe virtio-sound capture options
2026-10-29 5:43 ` [PATCH v8 11/14] docs: describe virtio-sound capture options Joonho Lee
@ 2026-10-29 5:47 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-10-29 5:47 UTC (permalink / raw)
To: Joonho Lee; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> Document the five opt-in capture module parameters introduced by this
> series. All default to false. Describe the startup-empty, short-payload,
> packed-status and pause-restart compatibility behavior alongside staging,
> and require validation with the target device.
>
> Assisted-by: LLM
> Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029054354.2931-1-mynameisjune111@gmail.com?part=11
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 10/14] ALSA: virtio: prioritize staged capture period work
2026-10-29 5:43 ` [PATCH v8 10/14] ALSA: virtio: prioritize staged capture period work Joonho Lee
@ 2026-10-29 5:48 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-10-29 5:48 UTC (permalink / raw)
To: Joonho Lee; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> Queue staged capture period notifications on system_highpri_wq through
> the common scheduling helper. This allows capture notifications to make
> progress when ordinary workqueue workers are delayed by CPU contention.
> Playback and ordinary capture keep schedule_work().
>
> This preserves the existing priority proposal without mixing RESET or
> lifecycle changes into it. It provides no hard real-time deadline and
> cannot prevent a userspace reader from overrunning its buffer.
>
> Assisted-by: LLM
> Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029054354.2931-1-mynameisjune111@gmail.com?part=10
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 07/14] ALSA: pcm: mark stop-threshold accesses for capture policy sampling
2026-10-29 5:43 ` [PATCH v8 07/14] ALSA: pcm: mark stop-threshold accesses for capture policy sampling Joonho Lee
@ 2026-10-29 5:48 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-10-29 5:48 UTC (permalink / raw)
To: Joonho Lee; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> The staged virtio-snd capture path needs to sample stop_threshold from
> completion context to distinguish free-running capture from bounded
> capture. Software parameters can update this field while capture runs.
>
> Mark the runtime stop-threshold assignments with WRITE_ONCE and the proc
> status read with READ_ONCE. This pairs the assignments with the driver's
> READ_ONCE policy sampling in the following patch.
>
> These annotations do not provide a transaction for the entire sw_params
> update. The driver serializes its cached policy and accounting with its
> own substream lock.
>
> Assisted-by: LLM
> Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029054354.2931-1-mynameisjune111@gmail.com?part=7
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 12/14] ALSA: pcm: add an optional reset operation under the stream lock
2026-10-29 5:43 ` [PATCH v8 12/14] ALSA: pcm: add an optional reset operation under the stream lock Joonho Lee
@ 2026-10-29 5:52 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-10-29 5:52 UTC (permalink / raw)
To: Joonho Lee; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> A driver which keeps private capture counters needs to reset those
> counters together with the PCM application's position. The ioctl RESET
> path releases the stream lock before core completes the position rebase,
> and appl_ptr is updated in a separate post_action. Period processing can
> observe the intermediate state.
>
> Add an optional reset operation whose stream lock remains held through
> the core position and appl_ptr updates. Export the common pointer reset
> as snd_pcm_lib_reset(), so drivers can reuse pointer validation, time
> accounting and XRUN handling without reacquiring the stream lock.
> Drivers without the new operation keep their existing ioctl path.
>
> A successful callback member completes its reset immediately. If a later
> linked member fails, the first error is returned and completed members
> are not rolled back. Legacy members retain the existing post_action
> [ ... ]
> a user-visible failure. The new-candidate tests validate the
> synchronization contract and recovery paths.
>
> Assisted-by: LLM
> Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029054354.2931-1-mynameisjune111@gmail.com?part=12
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 05/14] ALSA: virtio: Keep control payloads alive through DMA completion
2026-10-29 5:43 ` [PATCH v8 05/14] ALSA: virtio: Keep control payloads alive through DMA completion Joonho Lee
@ 2026-10-29 5:54 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-10-29 5:54 UTC (permalink / raw)
To: Joonho Lee, linux-sound; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ALSA: virtio: misaligned virtio_snd_ctl_value access in virtsnd_kctl_get()
--
commit b0116a304a65fa5afc94f948da091c12c54f919b
Author: Joonho Lee <mynameisjune111@gmail.com>
ALSA: virtio: Keep control payloads alive through DMA completion
This commit moves control message headers and data payloads into a
reference-counted DMA buffer owned by struct virtio_snd_msg so that caller
cleanup on timeout or interruption does not invalidate in-flight DMA. It also
aligns scatterlist regions to the greater of the DMA cacheline and
struct virtio_snd_ctl_value alignment.
> diff --git a/sound/virtio/virtio_ctl_msg.c b/sound/virtio/virtio_ctl_msg.c
> index 7114bea4033a5..2e0ff15e729da 100644
> --- a/sound/virtio/virtio_ctl_msg.c
> +++ b/sound/virtio/virtio_ctl_msg.c
[ ... ]
> @@ -85,20 +161,85 @@ void *virtsnd_ctl_msg_response(struct virtio_snd_msg *msg)
> * Return: Allocated message on success, NULL on failure.
> */
> struct virtio_snd_msg *virtsnd_ctl_msg_alloc(size_t request_size,
> - size_t response_size, gfp_t gfp)
> + size_t response_size,
> + size_t request_data_size,
> + size_t response_data_size,
> + gfp_t gfp)
> {
[ ... ]
> + cache_alignment = dma_get_cache_alignment();
> + if (cache_alignment <= 0)
> + return NULL;
> + /* DMA coherence does not remove the CPU's type alignment requirement. */
> + alignment = max_t(size_t, cache_alignment,
> + __alignof__(struct virtio_snd_ctl_value));
[ ... ]
> diff --git a/sound/virtio/virtio_kctl.c b/sound/virtio/virtio_kctl.c
> index 45f7b6a5b3080..4cf75dedbe9a3 100644
> --- a/sound/virtio/virtio_kctl.c
> +++ b/sound/virtio/virtio_kctl.c
> @@ -161,10 +163,12 @@ static int virtsnd_kctl_get(struct snd_kcontrol *kcontrol,
> unsigned int i;
> int rc;
>
> - msg = virtsnd_ctl_msg_alloc(request_size, response_size, GFP_KERNEL);
> + msg = virtsnd_ctl_msg_alloc(request_size, response_size, 0, 0,
> + GFP_KERNEL);
[Severity: Medium]
Does packing both struct virtio_snd_hdr and struct virtio_snd_ctl_value into
response_size here leave kvalue misaligned in virtsnd_kctl_get()?
In sound/virtio/virtio_ctl_msg.c, virtsnd_ctl_msg_alloc() aligns the start of
each scatterlist region (including msg->sg_response) to a multiple of
__alignof__(struct virtio_snd_ctl_value) (8 bytes) via
virtsnd_ctl_msg_sg_init().
However, in sound/virtio/virtio_kctl.c, virtsnd_kctl_get() passes
response_size = sizeof(struct virtio_snd_hdr) + sizeof(*kvalue) and
response_data_size = 0, and then computes kvalue by adding the 4-byte
sizeof(struct virtio_snd_hdr) to the 8-byte-aligned response buffer:
sound/virtio/virtio_kctl.c:virtsnd_kctl_get() {
...
kvalue = (void *)((u8 *)virtsnd_ctl_msg_response(msg) +
sizeof(struct virtio_snd_hdr));
switch (type) {
...
case VIRTIO_SND_CTL_TYPE_INTEGER64:
for (i = 0; i < count; ++i)
uvalue->value.integer64.value[i] =
le64_to_cpu(kvalue->value.integer64[i]);
break;
...
}
Because virtsnd_ctl_msg_response(msg) is 8-byte aligned and
sizeof(struct virtio_snd_hdr) is 4 bytes, kvalue is guaranteed to be at an
address congruent to 4 mod 8. When userspace reads a control element with
type VIRTIO_SND_CTL_TYPE_INTEGER64 via SNDRV_CTL_IOCTL_ELEM_READ,
le64_to_cpu(kvalue->value.integer64[i]) performs an unaligned 64-bit typed
load through kvalue.
Note that a later commit in the series, 775188574a68 ("ALSA: virtio: add
staged capture accounting and lifecycle recovery"), addresses this by passing
sizeof(*kvalue) as response_data_size and retrieving kvalue via
virtsnd_ctl_msg_response_data(msg). Should that change be folded into this
commit so that kvalue is aligned here as intended?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029054354.2931-1-mynameisjune111@gmail.com?part=5
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 06/14] ALSA: virtio: wait through signals for PCM teardown
2026-10-29 5:43 ` [PATCH v8 06/14] ALSA: virtio: wait through signals for PCM teardown Joonho Lee
@ 2026-10-29 6:00 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-10-29 6:00 UTC (permalink / raw)
To: Joonho Lee; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> A signal can end the interruptible wait while a STOP or RELEASE request is
> still pending. A later teardown may then submit another RELEASE before the
> first request completes.
>
> Keep both waits non-interruptible and use the existing message timeout.
> Other control requests remain interruptible. Device reset still cancels the
> pending request and wakes the waiter.
>
> Assisted-by: LLM
> Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029054354.2931-1-mynameisjune111@gmail.com?part=6
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 08/14] ALSA: pcm: stop paused streams before synchronizing teardown
2026-10-29 5:43 ` [PATCH v8 08/14] ALSA: pcm: stop paused streams before synchronizing teardown Joonho Lee
@ 2026-10-29 6:04 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-10-29 6:04 UTC (permalink / raw)
To: Joonho Lee, linux-sound; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ALSA: pcm: unbalanced STOP trigger from PAUSED in snd_pcm_do_stop()
--
commit 1209424a81e0273da5db98a8f56a0fd0d7015011
Author: Joonho Lee <mynameisjune111@gmail.com>
ALSA: pcm: stop paused streams before synchronizing teardown
This commit updates snd_pcm_do_stop() to issue the SNDRV_PCM_TRIGGER_STOP
trigger and set stop_operating when a substream is in SNDRV_PCM_STATE_PAUSED,
in addition to when it is running.
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index a2ea44c3c1d29..2b18c5f5801f9 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -1607,7 +1607,8 @@ static int snd_pcm_do_stop(struct snd_pcm_substream *substream,
> snd_pcm_state_t state)
> {
> if (substream->runtime->trigger_master == substream &&
> - snd_pcm_running(substream)) {
> + (snd_pcm_running(substream) ||
> + substream->runtime->state == SNDRV_PCM_STATE_PAUSED)) {
> substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_STOP);
> substream->runtime->stop_operating = true;
> }
[Severity: High]
Can calling substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_STOP)
directly while in SNDRV_PCM_STATE_PAUSED cause regressions in drivers that
expect start-class triggers (START, RESUME, PAUSE_RELEASE) and stop-class
triggers (STOP, SUSPEND, PAUSE_PUSH) to be strictly paired?
When a substream enters SNDRV_PCM_STATE_PAUSED via snd_pcm_do_pause(), the
driver has already received SNDRV_PCM_TRIGGER_PAUSE_PUSH. Reaching
snd_pcm_do_stop() while runtime->state is still SNDRV_PCM_STATE_PAUSED
delivers SNDRV_PCM_TRIGGER_STOP without an intervening
SNDRV_PCM_TRIGGER_PAUSE_RELEASE.
This path can be reached from userspace when:
1. __snd_pcm_xrun() -> snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN) runs on
a paused stream without calling snd_pcm_pause(substream, false), such as
when ops->ack() returns -EPIPE in pcm_lib_apply_appl_ptr() during write,
read, rewind, forward, or SNDRV_PCM_IOCTL_SYNC_PTR, when avail reaches
runtime->stop_threshold in snd_pcm_update_state(), or via a direct driver
snd_pcm_stop() call.
2. snd_pcm_pause(substream, false) in snd_pcm_drop() or snd_pcm_prepare()
fails (its return value is ignored), for example if the driver's
SNDRV_PCM_TRIGGER_PAUSE_RELEASE trigger returns an error or if a linked
substream is not in SNDRV_PCM_STATE_PAUSED so snd_pcm_pre_pause() returns
-EBADFD.
3. snd_pcm_drop(), snd_pcm_prepare(), or snd_pcm_stop() is called on a
non-paused substream that is linked to a paused substream.
Several in-tree drivers appear to rely on never receiving
SNDRV_PCM_TRIGGER_STOP after SNDRV_PCM_TRIGGER_PAUSE_PUSH without an
intervening release:
1. Clock enable_count underflow in sound/soc/qcom/lpass-cpu.c
(lpass_cpu_daiops_trigger()) and sound/soc/spear/spdif_in.c
(spdif_in_trigger()):
sound/soc/qcom/lpass-cpu.c:lpass_cpu_daiops_trigger() {
...
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
...
clk_disable(drvdata->mi2s_bit_clk[dai->driver->id]);
break;
}
Receiving STOP while PAUSED calls clk_disable() a second time for a single
clk_enable().
2. Runtime PM usage count underflow in sound/soc/samsung/i2s.c
(i2s_trigger()):
sound/soc/samsung/i2s.c:i2s_trigger() {
...
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
...
pm_runtime_put(dai->dev);
break;
}
Receiving STOP after PAUSE_PUSH calls pm_runtime_put(dai->dev) twice.
3. Reference and active counter underflows or premature shared hardware
shutdown in sound/soc/stm/stm32_i2s.c (stm32_i2s_trigger() decrementing
i2s->refcount), sound/soc/rockchip/rockchip_i2s_tdm.c
(rockchip_snd_txrxctrl() decrementing i2s_tdm->refcount),
sound/soc/atmel/atmel-i2s.c (atmel_i2s_trigger() decrementing
dev->clk_use_no), sound/soc/dwc/dwc-i2s.c (dw_i2s_trigger() decrementing
dev->active), sound/soc/ti/omap-mcbsp.c (omap_mcbsp_dai_trigger()
decrementing mcbsp->active), sound/soc/pxa/mmp-sspa.c
(mmp_sspa_trigger()), sound/soc/bcm/cygnus-ssp.c (cygnus_ssp_trigger()),
and sound/soc/fsl/imx-pcm-fiq.c (snd_imx_pcm_trigger() calling
disable_fiq(imx_pcm_fiq) twice):
sound/soc/stm/stm32_i2s.c:stm32_i2s_trigger() {
...
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
...
scoped_guard(spinlock, &i2s->lock_fd) {
i2s->refcount--;
}
4. SOF IPC4 pipeline state leak in sound/soc/sof/ipc4-pcm.c:
sound/soc/sof/ipc4-pcm.c:sof_ipc4_add_pipeline_to_trigger_list() {
...
case SOF_IPC4_PIPE_RESET:
/* RESET if the pipeline is neither running nor paused */
if (!spipe->started_count && !spipe->paused_count)
sof_ipc4_add_pipeline_by_priority(trigger_list, pipe_widget, pipe_priority,
true);
break;
case SOF_IPC4_PIPE_PAUSED:
case SOF_IPC4_PIPE_EOS:
if (spipe->paused_count == (spipe->started_count - 1))
sof_ipc4_add_pipeline_by_priority(trigger_list, pipe_widget, pipe_priority,
true);
break;
}
When START is followed by PAUSE_PUSH, spipe->started_count is 1 and
spipe->paused_count is 1. If SNDRV_PCM_TRIGGER_STOP is then issued while
PAUSED, sof_ipc4_pcm_trigger() requests SOF_IPC4_PIPE_PAUSED, the check
spipe->paused_count == (spipe->started_count - 1) evaluates to false, and
neither started_count nor paused_count is decremented (and
sof_ipc4_update_pipeline_state() only decrements paused_count on
SNDRV_PCM_TRIGGER_PAUSE_RELEASE). Subsequent sof_ipc4_pcm_hw_free() then
skips SOF_IPC4_PIPE_RESET because started_count and paused_count remain
non-zero.
5. Stuck hardware pause bits in sound/pci/cmipci.c (snd_cmipci_pcm_trigger()),
sound/pci/oxygen/oxygen_pcm.c (oxygen_trigger()), sound/pci/als300.c
(snd_als300_trigger()), sound/pci/ens1370.c (snd_ensoniq_trigger()), and
sound/pci/ice1712/ice1712.c (snd_ice1712_pcm_trigger()):
sound/pci/cmipci.c:snd_cmipci_pcm_trigger() {
...
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
case SNDRV_PCM_TRIGGER_SUSPEND:
cm->ctrl |= pause;
snd_cmipci_write(cm, CM_REG_FUNCTRL0, cm->ctrl);
break;
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
case SNDRV_PCM_TRIGGER_RESUME:
cm->ctrl &= ~pause;
snd_cmipci_write(cm, CM_REG_FUNCTRL0, cm->ctrl);
break;
}
In these drivers, the pause bit is only cleared on PAUSE_RELEASE or RESUME,
not by SNDRV_PCM_TRIGGER_STOP or prepare().
6. Duplicate VIRTIO_SND_R_PCM_STOP in sound/virtio/virtio_pcm_ops.c
(virtsnd_pcm_trigger()):
sound/virtio/virtio_pcm_ops.c:virtsnd_pcm_trigger() {
...
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
scoped_guard(spinlock_irqsave, &vss->lock) {
if (command != SNDRV_PCM_TRIGGER_PAUSE_PUSH)
vss->stopped = true;
vss->xfer_enabled = false;
}
msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_STOP,
GFP_KERNEL);
if (!msg)
return -ENOMEM;
return virtsnd_ctl_msg_send_sync(snd, msg);
}
Because SNDRV_PCM_TRIGGER_PAUSE_PUSH already sends VIRTIO_SND_R_PCM_STOP
to the device, triggering SNDRV_PCM_TRIGGER_STOP from
SNDRV_PCM_STATE_PAUSED sends a second VIRTIO_SND_R_PCM_STOP while the
device stream is already stopped.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029054354.2931-1-mynameisjune111@gmail.com?part=8
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 13/14] ALSA: virtio: synchronize staged capture reset with PCM positions
2026-10-29 5:43 ` [PATCH v8 13/14] ALSA: virtio: synchronize staged capture reset with PCM positions Joonho Lee
@ 2026-10-29 6:05 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-10-29 6:05 UTC (permalink / raw)
To: Joonho Lee, linux-sound; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ALSA: virtio: unupdated capture_notified_periods in virtsnd_pcm_reset()
- [High] ALSA: virtio: false XRUN and skipped rebase in virtsnd_pcm_reset() race
--
commit 4e51f7611d17678ff733d2492023b2e0abcb06d3
Author: Joonho Lee <mynameisjune111@gmail.com>
ALSA: virtio: synchronize staged capture reset with PCM positions
This commit introduces a custom PCM reset operation for staged capture
streams to synchronize private copied and acknowledged byte counters with
ALSA core pointer resets. When resetting a running stream, it calls
snd_pcm_lib_reset(), advances capture_acked_total to
capture_pointer_seen_total, and attempts to refill the RX queue.
> diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
> index 66560d82b7f8e..ba14f395eef8a 100644
> --- a/sound/virtio/virtio_pcm_ops.c
> +++ b/sound/virtio/virtio_pcm_ops.c
[ ... ]
> @@ -1591,6 +1598,58 @@ static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream)
> return virtsnd_pcm_sync_stop_locked(substream);
> }
>
> +/* The core holds the stream lock through the matching appl_ptr reset. */
> +static int virtsnd_pcm_reset(struct snd_pcm_substream *substream)
> +{
> + struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
> + struct snd_pcm_runtime *runtime = substream->runtime;
> + struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss);
> + size_t position;
> + size_t period;
> + int rc;
> +
> + /* Keep ALSA's pointer validation, time accounting and XRUN processing. */
> + rc = snd_pcm_lib_reset(substream);
> + if (rc || !vss->capture_staging)
> + return rc;
> +
> + guard(spinlock_irqsave)(&queue->lock);
> + guard(spinlock)(&vss->lock);
> + if (vss->resetting)
> + return -ECANCELED;
> + /* The common reset has already handled a pointer-reported XRUN. */
> + if (vss->xfer_xrun)
> + return 0;
[Severity: High]
Can an RX completion arriving between snd_pcm_lib_reset() and acquiring
vss->lock trigger a false XRUN and skip the private counter rebase?
During snd_pcm_do_reset() -> virtsnd_pcm_reset(), snd_pcm_lib_reset() calls
virtsnd_pcm_cp_pointer(), which records vss->capture_pointer_seen_total and
releases vss->lock while vss->capture_ready still holds the pre-reset
unacknowledged byte count:
sound/virtio/virtio_pcm_ops.c:virtsnd_pcm_cp_pointer() {
...
} else {
vss->capture_pointer_seen_total = total;
hw_ptr = virtsnd_pcm_ring_offset(total, vss->buffer_bytes);
}
...
}
In managed mode (!vss->capture_free_running), if an in-flight RX completion
arrives before virtsnd_pcm_reset() acquires queue->lock and vss->lock,
virtsnd_pcm_capture_complete() checks the incoming bytes against the stale
pre-reset vss->capture_ready:
sound/virtio/virtio_pcm_msg.c:virtsnd_pcm_capture_complete() {
...
else if (!free_running &&
audio_bytes >= vss->buffer_bytes - vss->capture_ready)
invalid_reason = "ring_capacity";
else if (!free_running &&
check_add_overflow(vss->capture_ready, audio_bytes,
&next_ready))
invalid_reason = "ready_overflow";
else if (!free_running && next_ready >= vss->buffer_bytes)
invalid_reason = "ring_full";
...
}
When the pre-reset unacknowledged bytes plus audio_bytes reach
vss->buffer_bytes, virtsnd_pcm_capture_complete() rejects the completion,
drops the captured audio, and sets vss->xfer_xrun = true.
When virtsnd_pcm_reset() then acquires the locks, it sees vss->xfer_xrun and
returns 0 without updating vss->capture_acked_total, vss->capture_ready, or
vss->pcm_indirect.appl_ptr, even though snd_pcm_lib_reset() did not observe
the XRUN and snd_pcm_do_reset() proceeds to rebase runtime->status->hw_ptr
and runtime->control->appl_ptr.
> + if (snd_pcm_running(substream)) {
> + /* Keep completions arriving after the core's pointer observation. */
> + vss->capture_acked_total = vss->capture_pointer_seen_total;
> + vss->capture_ready = vss->capture_free_running ? 0 :
> + vss->capture_copied_total - vss->capture_acked_total;
> + } else {
> + position = frames_to_bytes(runtime, runtime->status->hw_ptr);
> + vss->hw_ptr = position;
> + period = snd_pcm_lib_period_bytes(substream);
> + vss->capture_copied_total = position;
> + vss->capture_acked_total = position;
> + vss->capture_pointer_seen_total = position;
> + vss->capture_ready = 0;
> + vss->capture_notified_periods = period ? position / period : 0;
> + }
[Severity: High]
Should the running-stream branch in virtsnd_pcm_reset() also advance
vss->capture_notified_periods to match vss->capture_pointer_seen_total?
If a completion in virtsnd_pcm_capture_complete() crosses a period boundary
and schedules vss->elapsed_period, and userspace invokes
SNDRV_PCM_IOCTL_RESET before virtsnd_pcm_period_elapsed() acquires the PCM
stream lock, snd_pcm_lib_reset() -> snd_pcm_update_hw_ptr() ->
virtsnd_pcm_cp_pointer() observes the new vss->capture_copied_total and
updates vss->capture_pointer_seen_total.
In snd_pcm_do_reset(), the core then folds that position into
runtime->hw_ptr_interrupt:
sound/core/pcm_native.c:snd_pcm_do_reset() {
...
runtime->hw_ptr_base = 0;
runtime->hw_ptr_interrupt = runtime->status->hw_ptr -
runtime->status->hw_ptr % runtime->period_size;
runtime->silence_start = runtime->status->hw_ptr;
runtime->silence_filled = 0;
runtime->control->appl_ptr = runtime->status->hw_ptr;
...
}
Because the running branch in virtsnd_pcm_reset() leaves
vss->capture_notified_periods at its pre-reset value, the pending
virtsnd_pcm_period_elapsed() worker later sees period_floor >
vss->capture_notified_periods for a period boundary already accounted for in
runtime->hw_ptr_interrupt:
sound/virtio/virtio_pcm.c:virtsnd_pcm_period_elapsed() {
...
period_floor = div64_ul(vss->capture_copied_total, period_bytes);
if (period_floor > vss->capture_notified_periods) {
/* Reserve crossed boundaries before the callback. */
vss->capture_notified_periods = period_floor;
notify = true;
}
...
}
This triggers a duplicate snd_pcm_period_elapsed_under_stream_lock() ->
snd_pcm_update_hw_ptr0(substream, 1) call where delta > new_hw_ptr:
sound/core/pcm_lib.c:snd_pcm_update_hw_ptr0() {
...
if (in_interrupt) {
/* we know that one period was processed */
/* delta = "expected next hw_ptr" for in_interrupt != 0 */
delta = runtime->hw_ptr_interrupt + runtime->period_size;
if (delta > new_hw_ptr) {
/* check for double acknowledged interrupts */
hdelta = curr_jiffies - runtime->hw_ptr_jiffies;
if (hdelta > runtime->hw_ptr_buffer_jiffies/2 + 1) {
hw_base += runtime->buffer_size;
...
}
If the jiffies delta exceeds half the buffer duration, snd_pcm_update_hw_ptr0()
treats the duplicate notification as a missed full-buffer wrap-around and
advances runtime->status->hw_ptr by a full buffer_size of uncaptured frames.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029054354.2931-1-mynameisjune111@gmail.com?part=13
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 14/14] ALSA: virtio: synchronize indirect capture positions on reset
2026-10-29 5:43 ` [PATCH v8 14/14] ALSA: virtio: synchronize indirect capture positions on reset Joonho Lee
@ 2026-10-29 6:06 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-10-29 6:06 UTC (permalink / raw)
To: Joonho Lee, linux-sound; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ALSA: virtio: uncompleted hw_ptr rebase on virtsnd_pcm_indirect_reset() error
--
commit 0cc03a4b001753550dc69b32471d30cec1719208
Author: Joonho Lee <mynameisjune111@gmail.com>
ALSA: virtio: synchronize indirect capture positions on reset
This commit adds a dedicated reset operation for indirect capture streams
to synchronize private indirect capture accounting with the core PCM
pointer rebase on reset. It discards unread data, returns the freed RX
buffer space to the virtqueue, and updates the indirect application
pointer to match the rebased hardware pointer.
> diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
> index ba14f395eef8a..e4a7a4da4d3a8 100644
> --- a/sound/virtio/virtio_pcm_ops.c
> +++ b/sound/virtio/virtio_pcm_ops.c
[ ... ]
> @@ -1598,6 +1602,54 @@ static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream)
> return virtsnd_pcm_sync_stop_locked(substream);
> }
>
> +/* The PCM lock covers both indirect accounting and core completion. */
> +static int virtsnd_pcm_indirect_reset(struct snd_pcm_substream *substream)
> +{
> + struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
> + struct snd_pcm_runtime *runtime = substream->runtime;
> + struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss);
> + struct virtsnd_pcm_transfer transfer = {};
> + int rc;
> +
> + rc = snd_pcm_lib_reset(substream);
> + if (rc)
> + return rc;
> + if (!snd_pcm_running(substream) &&
> + runtime->state != SNDRV_PCM_STATE_PAUSED)
> + return 0;
> +
> + guard(spinlock_irqsave)(&queue->lock);
> + guard(spinlock)(&vss->lock);
> + if (vss->resetting)
> + return -ECANCELED;
> + if (!vss->prepared || vss->stopped)
> + return -EBADFD;
> +
> + /* Pausing does not move the device's fixed DMA buffer positions. */
> + if (runtime->state == SNDRV_PCM_STATE_PAUSED)
> + runtime->status->hw_ptr =
> + bytes_to_frames(runtime, vss->pcm_indirect.sw_io);
> +
> + transfer.indirect = vss->pcm_indirect;
> + transfer.indirect.sw_ready = 0;
> + /* Consume no application delta while returning discarded RX space. */
> + transfer.indirect.appl_ptr = runtime->control->appl_ptr;
> + rc = snd_pcm_indirect_capture_transfer(substream, &transfer.indirect,
> + virtsnd_pcm_trans_copy);
> + if (!rc)
> + rc = transfer.error;
> + if (rc) {
> + virtsnd_pcm_transfer_failed(vss);
> + return rc;
> + }
[Severity: High]
What happens to the PCM ring buffer accounting if virtsnd_pcm_indirect_reset()
returns an error after snd_pcm_lib_reset() has already rebased
runtime->status->hw_ptr?
On an indirect capture substream in SNDRV_PCM_STATE_RUNNING or
SNDRV_PCM_STATE_PAUSED that has wrapped past runtime->buffer_size (so
runtime->hw_ptr_base >= runtime->buffer_size and runtime->control->appl_ptr >=
runtime->buffer_size), virtsnd_pcm_indirect_reset() first calls
snd_pcm_lib_reset() (and in the paused state sets runtime->status->hw_ptr from
vss->pcm_indirect.sw_io), which rebases runtime->status->hw_ptr into
[0, runtime->buffer_size).
If virtsnd_pcm_indirect_reset() then returns -ECANCELED, -EBADFD, or
transfer.error, snd_pcm_do_reset() in sound/core/pcm_native.c aborts early
without completing the rest of the rebase:
sound/core/pcm_native.c:snd_pcm_do_reset() {
...
err = substream->ops->reset(substream);
if (err < 0)
return err;
runtime->hw_ptr_base = 0;
runtime->hw_ptr_interrupt = runtime->status->hw_ptr -
runtime->status->hw_ptr % runtime->period_size;
runtime->silence_start = runtime->status->hw_ptr;
runtime->silence_filled = 0;
runtime->control->appl_ptr = runtime->status->hw_ptr;
...
}
This leaves runtime->status->hw_ptr rebased below runtime->control->appl_ptr
while runtime->hw_ptr_base and runtime->control->appl_ptr remain at their
pre-reset values, and runtime->state stays in SNDRV_PCM_STATE_RUNNING or
SNDRV_PCM_STATE_PAUSED. Although virtsnd_pcm_transfer_failed() sets
runtime->stop_operating = true on transfer errors, stop_operating is only
acted on by snd_pcm_sync_stop() and is not checked by pcm_accessible_state()
or __snd_pcm_lib_xfer().
In include/sound/pcm.h:snd_pcm_capture_avail():
snd_pcm_sframes_t avail = runtime->status->hw_ptr - runtime->control->appl_ptr;
if (avail < 0)
avail += runtime->boundary;
return avail;
avail underflows and wraps by runtime->boundary. Wouldn't a subsequent read
via sound/core/pcm_lib.c:__snd_pcm_lib_xfer() see this wrapped avail value and
copy stale DMA ring buffer frames to userspace via writer() before
pcm_accessible_state() or pcm_lib_apply_appl_ptr() stops the transfer?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029054354.2931-1-mynameisjune111@gmail.com?part=14
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 01/14] ALSA: virtio: Propagate PCM send failures through ACK
2026-10-29 5:43 ` [PATCH v8 01/14] ALSA: virtio: Propagate PCM send failures through ACK Joonho Lee
@ 2026-10-29 6:06 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-10-29 6:06 UTC (permalink / raw)
To: Joonho Lee; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> virtsnd_pcm_msg_send() can fail while queueing a transfer. Restore the
> failed message's length and notify the device about earlier messages
> queued by the same call, so they do not remain stranded.
>
> Propagate that failure through the indirect PCM transfer callback instead
> of discarding it. Commit the updated indirect state only after successful
> submission and stop further transfers when a send fails. Return an XRUN
> error to ALSA so the stream can recover.
>
> Keep the polling feature path unchanged.
>
> Fixes: fe981e67568c ("ALSA: virtio: use ack callback")
> Assisted-by: LLM
> Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029054354.2931-1-mynameisjune111@gmail.com?part=1
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 04/14] ALSA: virtio: Keep PCM DMA buffers alive through device ownership
2026-10-29 5:43 ` [PATCH v8 04/14] ALSA: virtio: Keep PCM DMA buffers alive through device ownership Joonho Lee
@ 2026-10-29 6:07 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-10-29 6:07 UTC (permalink / raw)
To: Joonho Lee, linux-sound; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ALSA: virtio: out-of-bounds vss->msgs[] access when buffer_bytes < dma_bytes
--
commit 5684205c20865cb426276aaf2c41df707838c019
Author: Joonho Lee <mynameisjune111@gmail.com>
ALSA: virtio: Keep PCM DMA buffers alive through device ownership
This commit retains references to pages backing ALSA vmalloc buffers (or uses
a driver-owned DMA buffer on NOMMU systems) until queued virtio-snd I/O
messages are freed. It also adds buffer validation during message allocation
and clamps device-reported capture completion lengths to the message capacity.
> diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c
> index 762480363b861..b7f2334bb221c 100644
> --- a/sound/virtio/virtio_pcm_msg.c
> +++ b/sound/virtio/virtio_pcm_msg.c
[ ... ]
> @@ -132,36 +211,99 @@ static void virtsnd_pcm_sg_from(struct scatterlist *sgs, int nsgs, u8 *data,
> int virtsnd_pcm_msg_alloc(struct virtio_pcm_substream *vss,
> unsigned int periods, unsigned int period_bytes)
> {
> - struct snd_pcm_runtime *runtime = vss->substream->runtime;
> + struct snd_pcm_runtime *runtime;
> + size_t buffer_bytes;
> unsigned int i;
> + int rc;
>
> - vss->msgs = kzalloc_objs(*vss->msgs, periods);
> - if (!vss->msgs)
> + if (vss->msgs || vss->nmsgs || vss->dma_pages || vss->ndma_pages)
> + return -EBUSY;
> +#ifndef CONFIG_MMU
> + if (vss->owned_dma_area || vss->owned_dma_bytes)
> + return -EBUSY;
> +#endif
> + if (!periods || !period_bytes)
> + return -EINVAL;
> +
> + if (check_mul_overflow((size_t)periods, (size_t)period_bytes,
> + &buffer_bytes))
> + return -EOVERFLOW;
> +
> + if (!vss->substream || !vss->substream->runtime)
> + return -EINVAL;
> + runtime = vss->substream->runtime;
> + if (!runtime->dma_area || buffer_bytes > runtime->dma_bytes)
> + return -EINVAL;
[Severity: High]
Can this validation allow buffer_bytes < runtime->dma_bytes to pass and lead
to an out-of-bounds read on vss->msgs[] in virtsnd_pcm_msg_send()?
virtsnd_pcm_open() constrains SNDRV_PCM_HW_PARAM_PERIODS to an integer, and
ALSA core constrains SNDRV_PCM_HW_PARAM_PERIOD_SIZE and
SNDRV_PCM_HW_PARAM_BUFFER_BYTES to integers, but SNDRV_PCM_HW_PARAM_PERIOD_BYTES
is not constrained to an integer. When the device supports the 4-bit
SNDRV_PCM_FORMAT_IMA_ADPCM format (VIRTIO_SND_PCM_FMT_IMA_ADPCM), a 1-channel
stream (frame_bits == 4) configured with an odd period_size (for example, 2001
frames) and an even number of periods (for example, 2, giving buffer_size ==
4002 frames) refines SNDRV_PCM_HW_PARAM_BUFFER_BYTES to the exact integer 2001
(so runtime->dma_bytes is 2001), while SNDRV_PCM_HW_PARAM_PERIOD_BYTES refines
to the non-empty open interval (1000, 1001). Both params_period_bytes() and
snd_pcm_lib_period_bytes() truncate that period byte size to 1000.
In virtsnd_pcm_msg_alloc(), buffer_bytes is computed as periods * period_bytes
(2000), which passes buffer_bytes > runtime->dma_bytes and allocates only
vss->nmsgs = 2 messages (along with 2000 bytes of retained pages or
owned_dma_area).
Later, virtsnd_pcm_prepare() sets vss->buffer_bytes and the pcm_indirect
buffer sizes from snd_pcm_lib_buffer_bytes(substream) (2001):
sound/virtio/virtio_pcm_ops.c:virtsnd_pcm_prepare() {
...
vss->buffer_bytes = snd_pcm_lib_buffer_bytes(substream);
vss->hw_ptr = 0;
...
vss->pcm_indirect.sw_buffer_size =
vss->pcm_indirect.hw_buffer_size =
snd_pcm_lib_buffer_bytes(substream);
}
When capture starts via virtsnd_pcm_trigger() ->
virtsnd_pcm_msg_send(vss, 0, vss->buffer_bytes), or playback transfers the
final byte of the buffer via virtsnd_pcm_pb_ack() -> virtsnd_pcm_msg_send(),
virtsnd_pcm_msg_send() computes end = (offset + bytes - 1) / period_bytes =
2000 / 1000 = 2 and indexes vss->msgs[2] past the end of the 2-element
vss->msgs[] array:
sound/virtio/virtio_pcm_msg.c:virtsnd_pcm_msg_send() {
...
start = offset / period_bytes;
end = (offset + bytes - 1) / period_bytes;
for (i = start; i <= end; i++) {
struct virtio_pcm_msg *msg = vss->msgs[i];
...
}
Should virtsnd_pcm_msg_alloc() check buffer_bytes != runtime->dma_bytes (or
constrain SNDRV_PCM_HW_PARAM_PERIOD_BYTES to an integer in virtsnd_pcm_open())
so that periods * period_bytes always covers the full PCM buffer?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029054354.2931-1-mynameisjune111@gmail.com?part=4
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 02/14] ALSA: virtio: Recover PCM state after a failed start
2026-10-29 5:43 ` [PATCH v8 02/14] ALSA: virtio: Recover PCM state after a failed start Joonho Lee
@ 2026-10-29 6:10 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-10-29 6:10 UTC (permalink / raw)
To: Joonho Lee, linux-sound; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ALSA: virtio: virtsnd_pcm_sync_stop() resends PCM_RELEASE after drain failure
--
commit 8526e884d4c74a3678dceb6e148567c0ca936c95
Author: Joonho Lee <mynameisjune111@gmail.com>
ALSA: virtio: Recover PCM state after a failed start
This patch recovers PCM substream state when a start trigger fails by marking
the stream stopped, disabling transfers, arming stop_operating, and protecting
accesses to the stopped flag with vss->lock.
> diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
> index a40ea394bf174..970499fe419b4 100644
> --- a/sound/virtio/virtio_pcm_ops.c
> +++ b/sound/virtio/virtio_pcm_ops.c
[ ... ]
> @@ -115,7 +124,8 @@ static int virtsnd_pcm_open(struct snd_pcm_substream *substream)
> snd_pcm_hw_constraint_integer(substream->runtime,
> SNDRV_PCM_HW_PARAM_PERIODS);
>
> - vss->stopped = !!virtsnd_pcm_msg_pending_num(vss);
> + scoped_guard(spinlock_irqsave, &vss->lock)
> + vss->stopped |= !!vss->msg_count;
> vss->suspended = false;
[ ... ]
> @@ -393,21 +434,31 @@ static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream)
> struct virtio_snd *snd = vss->snd;
> struct virtio_snd_msg *msg;
> unsigned int js = msecs_to_jiffies(virtsnd_msg_timeout_ms);
> + bool need_release;
> int rc;
>
> + scoped_guard(spinlock_irqsave, &vss->lock) {
> + vss->xfer_enabled = false;
> + need_release = vss->stopped || vss->msg_count;
> + if (need_release)
> + vss->stopped = true;
> + }
> cancel_work_sync(&vss->elapsed_period);
> -
> - if (!vss->stopped)
> + if (!need_release)
> return 0;
>
> msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_RELEASE,
> GFP_KERNEL);
> - if (!msg)
> + if (!msg) {
> + substream->runtime->stop_operating = true;
> return -ENOMEM;
> + }
>
> rc = virtsnd_ctl_msg_send_sync(snd, msg);
> - if (rc)
> + if (rc) {
> + substream->runtime->stop_operating = true;
> return rc;
> + }
>
> /*
> * The spec states that upon receipt of the RELEASE command "the device
> @@ -421,11 +472,13 @@ static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream)
> if (rc <= 0) {
> dev_warn(&snd->vdev->dev, "SID %u: failed to flush I/O queue\n",
> vss->sid);
> + substream->runtime->stop_operating = true;
>
> return !rc ? -ETIMEDOUT : rc;
> }
>
> - vss->stopped = false;
> + scoped_guard(spinlock_irqsave, &vss->lock)
> + vss->stopped = false;
>
> return 0;
> }
[Severity: Medium]
Can virtsnd_pcm_sync_stop() send a duplicate VIRTIO_SND_R_PCM_RELEASE command
if the post-release queue drain wait is interrupted or times out?
In virtsnd_pcm_sync_stop(), after virtsnd_ctl_msg_send_sync() succeeds for
VIRTIO_SND_R_PCM_RELEASE, wait_event_interruptible_timeout() waits for
in-flight I/O messages to drain. If a signal is pending or the wait times out
before the last message completes, virtsnd_pcm_sync_stop() sets
substream->runtime->stop_operating = true and returns an error while leaving
vss->stopped = true.
Once the device finishes returning the remaining I/O messages and
vss->msg_count reaches 0, vss->stopped remains true because a single boolean
tracks both whether VIRTIO_SND_R_PCM_RELEASE still needs to be sent and
whether pending I/O messages have finished draining.
Because virtsnd_pcm_open() now uses vss->stopped |= !!vss->msg_count, and
virtsnd_pcm_hw_params() and virtsnd_pcm_prepare() reject the substream with
-EBADFD while vss->stopped is true, the next virtsnd_pcm_sync_stop() call
(during close, hw_free, prepare, hw_params, or reopen via virtsnd_pcm_open())
sees need_release = vss->stopped || vss->msg_count as true and sends a second
VIRTIO_SND_R_PCM_RELEASE command for a stream that was already released. If
the device rejects the duplicate release command, virtsnd_pcm_sync_stop()
fails again and leaves vss->stopped = true, wedging the substream.
Note that a later commit in the series ("ALSA: virtio: add staged capture
accounting and lifecycle recovery") changes the drain wait in
virtsnd_pcm_sync_stop_locked() to wait_event_timeout() and tracks
VIRTSND_PCM_PAUSE_RESTART_RELEASE_SENT in the opt-in pause-restart state
machine, but the default virtsnd_pcm_sync_stop_locked() path still leaves
vss->stopped = true (and vss->prepared = true) if the drain wait times out
after VIRTIO_SND_R_PCM_RELEASE succeeded.
Should the driver track whether VIRTIO_SND_R_PCM_RELEASE has already succeeded
separately from whether vss->msg_count has drained to 0, so a subsequent
virtsnd_pcm_sync_stop() only waits for remaining messages to drain instead of
resending VIRTIO_SND_R_PCM_RELEASE?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029054354.2931-1-mynameisjune111@gmail.com?part=2
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 03/14] ALSA: virtio: Retain queued messages through device reset
2026-10-29 5:43 ` [PATCH v8 03/14] ALSA: virtio: Retain queued messages through device reset Joonho Lee
@ 2026-10-29 6:14 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-10-29 6:14 UTC (permalink / raw)
To: Joonho Lee; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> Keep the control queue available through snd_card_free() so close-time
> STOP and RELEASE commands can still be submitted. Block new control
> requests and mark pending messages canceled before reset, then release
> their queue references only after reset and queue deletion.
>
> For PCM queues, detach each remaining virtqueue buffer after the device
> has stopped DMA and retire its message without reporting a normal
> completion. Detach all queues before deleting them so outstanding DMA
> mappings are unmapped and no token remains for a late device completion.
>
> Fixes: 9d45e514da88 ("ALSA: virtio: handling control messages")
> Assisted-by: LLM
> Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029054354.2931-1-mynameisjune111@gmail.com?part=3
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 09/14] ALSA: virtio: add staged capture accounting and lifecycle recovery
2026-10-29 5:43 ` [PATCH v8 09/14] ALSA: virtio: add staged capture accounting and lifecycle recovery Joonho Lee
@ 2026-10-29 6:23 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-10-29 6:23 UTC (permalink / raw)
To: Joonho Lee, linux-sound; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] ALSA: virtio: virtsnd_pcm_prepare_locked() fails after playback pre-buffering
- [High] ALSA: virtio: virtsnd_pcm_trigger() sends PCM_STOP when stream is not running
- [High] ALSA: virtio: virtsnd_pcm_start_failed() marks unprepared stream stopped
- [Medium] ALSA: virtio: missed period wakeup in virtsnd_pcm_period_elapsed()
- [Medium] ALSA: virtio: unreleased device stream on failed re-prepare in virtsnd_pcm_prepare_locked()
- [Medium] ALSA: virtio: reset_epoch TOCTOU in virtsnd_pcm_send_sync() helpers
- [Low] ALSA: virtio: virtsnd_pcm_trigger() mixes goto cleanup with guard() helpers
--
commit 775188574a684cd4f7321eccd1403d5cf535ff0a
Author: Joonho Lee <mynameisjune111@gmail.com>
ALSA: virtio: add staged capture accounting and lifecycle recovery
This commit introduces staged capture with separate produced, observed, and
consumed positions using request-owned buffers, along with a free-running
capture policy for boundary stop thresholds. It also adds a lifecycle mutex,
reset epoch tracking, and an opt-in pause-restart state machine to handle
control transitions and device reset recovery.
> diff --git a/sound/virtio/virtio_pcm.c b/sound/virtio/virtio_pcm.c
> index be3893de40a55..ecd017edba72c 100644
> --- a/sound/virtio/virtio_pcm.c
> +++ b/sound/virtio/virtio_pcm.c
> @@ -310,8 +310,98 @@ static void virtsnd_pcm_period_elapsed(struct work_struct *work)
> {
> struct virtio_pcm_substream *vss =
> container_of(work, struct virtio_pcm_substream, elapsed_period);
> + struct snd_pcm_substream *substream = vss->substream;
> + struct snd_pcm_runtime *runtime;
> + size_t period_bytes;
> + u64 period_floor;
> + u64 snapshot;
> + bool staged_capture;
> + bool paused_xrun = false;
> + bool notify = false;
> + bool requeue = false;
[ ... ]
> + scoped_guard(spinlock_irqsave, &vss->lock) {
> + runtime = substream->runtime;
> + if (!vss->runtime_active ||
> + (!vss->xfer_enabled && !vss->xfer_xrun) || !runtime)
> + return;
> +
> + period_bytes = snd_pcm_lib_period_bytes(substream);
> + if (vss->xfer_xrun) {
> + notify = true;
> + } else if (!period_bytes || period_bytes >= vss->buffer_bytes ||
> + !virtsnd_pcm_capture_counters_valid(vss) ||
> + vss->capture_notified_periods >
> + div64_ul(vss->capture_copied_total, period_bytes)) {
> + vss->xfer_xrun = true;
> + notify = true;
> + } else {
> + period_floor = div64_ul(vss->capture_copied_total, period_bytes);
> + if (period_floor > vss->capture_notified_periods) {
> + /* Reserve crossed boundaries before the callback. */
> + vss->capture_notified_periods = period_floor;
> + notify = true;
> + }
> + }
> +
> + if (notify) {
> + snapshot = vss->capture_copied_total;
> + vss->capture_callback_snapshot = snapshot;
> + vss->capture_callback_snapshot_active = true;
> + }
> + }
[Severity: Medium]
When virtsnd_capture_staging and virtsnd_capture_allow_short are enabled, can
gating period notifications strictly on integer multiples of period_bytes in
virtsnd_pcm_period_elapsed() miss a period wakeup and trigger a ring_capacity
xrun?
When virtsnd_pcm_period_elapsed() sets vss->capture_callback_snapshot to
vss->capture_copied_total (or when virtsnd_pcm_cp_pointer() is called
directly), any sub-period tail is exposed to the reader and acknowledged via
virtsnd_pcm_cp_ack() -> virtsnd_pcm_capture_staged_ack().
If a subsequent short completion in virtsnd_pcm_capture_complete() crosses the
next integer multiple of period_bytes while total unacknowledged data
(vss->capture_copied_total - vss->capture_acked_total) is still less than one
period, virtsnd_pcm_period_elapsed() advances vss->capture_notified_periods to
period_floor and calls snd_pcm_period_elapsed_under_stream_lock(), which leaves
the reader sleeping because available frames are below avail_min.
If another short completion then arrives within that same period_floor
interval and brings unacknowledged bytes in vss->capture_ready to at least
period_bytes without crossing the next integer period boundary,
period_floor > vss->capture_notified_periods is false and notify remains
false. On a two-period buffer, leaving at least period_bytes unacknowledged in
vss->capture_ready causes the next full-period completion in
virtsnd_pcm_capture_complete() to fail:
else if (!free_running &&
audio_bytes >= vss->buffer_bytes - vss->capture_ready)
invalid_reason = "ring_capacity";
Should virtsnd_pcm_period_elapsed() also notify when unacknowledged bytes
reach period_bytes even if period_floor has not advanced?
> diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
> index f74bf365065b6..66560d82b7f8e 100644
> --- a/sound/virtio/virtio_pcm_ops.c
> +++ b/sound/virtio/virtio_pcm_ops.c
[ ... ]
> @@ -97,14 +128,238 @@ static const struct virtsnd_a2v_rate g_a2v_rate_map[] = {
> };
>
> static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream);
> +static int virtsnd_pcm_sync_stop_locked(struct snd_pcm_substream *substream);
> +static int virtsnd_pcm_pause_restart_sync_stop(struct snd_pcm_substream *substream);
>
> -static void virtsnd_pcm_start_failed(struct virtio_pcm_substream *vss)
> +static void virtsnd_pcm_start_failed(struct virtio_pcm_substream *vss, u64 epoch)
> {
> + bool notify;
> +
> scoped_guard(spinlock_irqsave, &vss->lock) {
> + if (vss->resetting || vss->reset_epoch != epoch)
> + return;
> vss->xfer_enabled = false;
> vss->stopped = true;
> + notify = vss->runtime_active && vss->capture_staging &&
> + vss->direction == SNDRV_PCM_STREAM_CAPTURE;
> + if (notify)
> + vss->xfer_xrun = true;
> }
> vss->substream->runtime->stop_operating = true;
> + if (notify)
> + virtsnd_pcm_schedule_period_elapsed(vss);
> +}
[Severity: High]
Does unconditionally setting vss->stopped = true in virtsnd_pcm_start_failed()
cause virtsnd_pcm_sync_stop_locked() to send an invalid release command when a
stream is not prepared on the device?
After a device reset and restore (virtsnd_reset_vqs() -> virtsnd_restore()),
or after a failed re-prepare in virtsnd_pcm_prepare_locked(), vss->prepared is
false, vss->stopped is false, and vss->msg_count is 0, while the substream
runtime state can still be paused or prepared.
If userspace then drops, prepares, or closes a paused substream (calling
snd_pcm_pause() -> virtsnd_pcm_trigger()), or starts a prepared substream,
virtsnd_pcm_trigger() sees !vss->prepared and jumps to start_failed:
if (vss->stopped || !vss->prepared || vss->xfer_xrun) {
rc = vss->xfer_xrun ? -EPIPE : -EBADFD;
goto start_failed;
}
virtsnd_pcm_start_failed() then sets vss->stopped = true and
runtime->stop_operating = true even though the device stream was never
prepared in the current epoch and has no queued messages. When
virtsnd_pcm_sync_stop_locked() subsequently runs, need_release evaluates to
true because vss->stopped is set, so it sends a release control message for an
unprepared stream. If the device rejects that release request,
virtsnd_pcm_sync_stop_locked() returns an error with vss->stopped still true,
wedging subsequent prepare, hw_params, and open calls.
Should virtsnd_pcm_start_failed() only set vss->stopped and
runtime->stop_operating when vss->prepared || vss->msg_count is true?
[ ... ]
> +static int virtsnd_pcm_send_sync(struct virtio_pcm_substream *vss,
> + struct virtio_snd_msg *msg)
> +{
> + u64 epoch;
> + int rc;
> +
> + scoped_guard(spinlock_irqsave, &vss->lock) {
> + if (vss->resetting) {
> + virtsnd_ctl_msg_unref(msg);
> + return -ECANCELED;
> + }
> + epoch = vss->reset_epoch;
> + }
> + rc = virtsnd_ctl_msg_send_sync(vss->snd, msg);
> + scoped_guard(spinlock_irqsave, &vss->lock) {
> + if (epoch != vss->reset_epoch || vss->resetting)
> + return -ECANCELED;
> + }
> + return rc;
> +}
> +
> +static int
> +virtsnd_pcm_pause_restart_send(struct virtio_pcm_substream *vss,
> + struct virtio_snd_msg *msg,
> + unsigned long allowed_states,
> + enum virtio_pcm_pause_restart_state submitted_state,
> + enum virtio_pcm_pause_restart_state success_state)
> +{
> + enum virtio_pcm_pause_restart_state state;
> + bool uncertain;
> + u64 epoch;
> + int rc;
> +
> + if (!virtsnd_pcm_pause_restart_enabled(vss))
> + return virtsnd_pcm_send_sync(vss, msg);
> +
> + scoped_guard(spinlock_irqsave, &vss->lock) {
> + epoch = vss->reset_epoch;
> + state = vss->pause_restart_state;
> + if (vss->resetting) {
> + rc = -ECANCELED;
> + } else if (state == VIRTSND_PCM_PAUSE_RESTART_RESET_REQUIRED) {
> + rc = -EIO;
> + } else if (virtsnd_pcm_pause_restart_transitioning(state)) {
> + vss->pause_restart_state =
> + VIRTSND_PCM_PAUSE_RESTART_RESET_REQUIRED;
> + rc = -EIO;
> + } else if (!(allowed_states & BIT(state))) {
> + rc = -EBADFD;
> + } else {
> + vss->pause_restart_state = submitted_state;
> + rc = 0;
> + }
> + }
[Severity: Medium]
Can re-sampling vss->reset_epoch inside virtsnd_pcm_send_sync() and
virtsnd_pcm_pause_restart_send() allow a control request from before a device
reset to be sent in the new epoch?
Callers such as virtsnd_pcm_trigger(), virtsnd_pcm_prepare_locked(),
virtsnd_pcm_sync_stop_locked(), virtsnd_pcm_pause_release_flush(),
virtsnd_pcm_pause_restart_start(), virtsnd_pcm_pause_restart_pause(), and
virtsnd_pcm_pause_restart_sync_stop() check substream state (and in the start
paths, queue virtqueue buffers) under vss->lock while recording
epoch = vss->reset_epoch, then drop vss->lock to call cancel_work_sync() or
virtsnd_pcm_ctl_msg_alloc() before calling virtsnd_pcm_send_sync() or
virtsnd_pcm_pause_restart_send().
If virtsnd_reset_vqs() and virtsnd_restore() both complete during that
unlocked window, vss->resetting is false again and vss->reset_epoch has been
incremented. Because virtsnd_pcm_send_sync() and
virtsnd_pcm_pause_restart_send() overwrite their local epoch variable with the
new vss->reset_epoch instead of checking the caller's epoch, they can send a
stale start, stop, release, or prepare command to the freshly restored device,
or fail the state check against the reset pause_restart_state with -EBADFD
instead of -ECANCELED.
Should virtsnd_pcm_send_sync() and virtsnd_pcm_pause_restart_send() take the
caller's expected epoch parameter and return -ECANCELED if vss->reset_epoch no
longer matches?
[ ... ]
> @@ -277,27 +587,90 @@ static int virtsnd_pcm_hw_params(struct snd_pcm_substream *substream,
[ ... ]
> static int virtsnd_pcm_prepare_locked(struct snd_pcm_substream *substream)
> {
> struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
> struct virtio_device *vdev = vss->snd->vdev;
> - struct virtio_snd_msg *msg;
> + bool pause_restart = virtsnd_pcm_pause_restart_enabled(vss);
> + bool free_running;
> + int rc;
> bool stopped;
> + bool suspended;
> + u64 epoch;
> +
> + scoped_guard(spinlock_irqsave, &vss->lock) {
> + if (vss->resetting)
> + return -ECANCELED;
> + stopped = vss->stopped;
> + }
> + if (stopped) {\n+ rc = virtsnd_pcm_sync_stop_locked(substream);
> + if (rc)
> + return rc;
> + }
> + scoped_guard(spinlock_irqsave, &vss->lock) {
> + vss->prepared = false;
> + epoch = vss->reset_epoch;
> + suspended = vss->suspended;
> + }
> +
> + if (pause_restart) {
> + enum virtio_pcm_pause_restart_state state;
> +
> + scoped_guard(spinlock_irqsave, &vss->lock)
> + state = vss->pause_restart_state;
> + if (state == VIRTSND_PCM_PAUSE_RESTART_RESET_REQUIRED ||
> + virtsnd_pcm_pause_restart_transitioning(state)) {
> + virtsnd_pcm_pause_restart_fail(vss, true, epoch);
> + return -EIO;
> + }
> + }
>
> scoped_guard(spinlock_irqsave, &vss->lock)
> stopped = vss->stopped;
> @@ -308,44 +681,320 @@ static int virtsnd_pcm_prepare(struct snd_pcm_substream *substream)
> return -EBADFD;
> }
>
> - if (!vss->suspended) {
> + if (!suspended) {
> if (virtsnd_pcm_msg_pending_num(vss)) {
> dev_err(&vdev->dev, "SID %u: invalid I/O queue state\n",
> vss->sid);
> return -EBADFD;
> }
>
> - vss->buffer_bytes = snd_pcm_lib_buffer_bytes(substream);
> - vss->hw_ptr = 0;
[Severity: High]
What happens in virtsnd_pcm_prepare_locked() if a playback stream queues
at least one period while in the prepared state before reaching
runtime->start_threshold, and userspace then calls snd_pcm_drop() followed by
snd_pcm_prepare() (or calls snd_pcm_prepare() directly)?
During playback pre-buffering, pcm_lib_apply_appl_ptr() ->
virtsnd_pcm_pb_ack() -> snd_pcm_indirect_playback_transfer() ->
virtsnd_pcm_trans_copy() -> virtsnd_pcm_msg_send() submits full periods to the
tx virtqueue and increments vss->msg_count while the stream is still in the
prepared state and vss->stopped is false. Because snd_pcm_do_stop() only
invokes trigger(stop) when the stream is running or paused, vss->stopped
remains false.
When virtsnd_pcm_prepare_locked() runs (and similarly in
virtsnd_pcm_sync_stop()), it only checks if (stopped) before calling
virtsnd_pcm_sync_stop_locked(substream), skipping the release flush even
though vss->msg_count > 0. It then clears vss->prepared = false and fails the
virtsnd_pcm_msg_pending_num(vss) check with -EBADFD, leaving pending messages
in the queue with vss->stopped == false and vss->prepared == false so
subsequent prepare or write calls cannot recover until hw_params or hw_free.
Should virtsnd_pcm_prepare_locked() and virtsnd_pcm_sync_stop() also run
virtsnd_pcm_sync_stop_locked() when vss->msg_count > 0?
> } else {
> struct snd_pcm_runtime *runtime = substream->runtime;
> unsigned int buffer_bytes = snd_pcm_lib_buffer_bytes(substream);
> unsigned int period_bytes = snd_pcm_lib_period_bytes(substream);
> - int rc;
>
> rc = virtsnd_pcm_dev_set_params(vss, buffer_bytes, period_bytes,
> runtime->channels,
> runtime->format, runtime->rate);
> - if (rc)
> + if (rc) {
> + if (pause_restart && rc != -ECANCELED)
> + virtsnd_pcm_pause_restart_fail(vss, false, epoch);
> return rc;
> + }
> }
>
> - vss->xfer_xrun = false;
> - vss->suspended = false;
> - vss->msg_count = 0;
> - virtsnd_pcm_msg_reset(vss);
> + scoped_guard(spinlock_irqsave, &vss->lock) {
> + if (epoch != vss->reset_epoch || vss->resetting)
> + return -ECANCELED;
> + vss->prepared = false;
> + vss->buffer_bytes = snd_pcm_lib_buffer_bytes(substream);
> + vss->hw_ptr = 0;
> + if (!pause_restart)
> + vss->xfer_xrun = false;
> + vss->suspended = false;
> + vss->msg_count = 0;
> + virtsnd_pcm_msg_reset(vss);
> + memset(&vss->pcm_indirect, 0, sizeof(vss->pcm_indirect));
> + vss->pcm_indirect.sw_buffer_size = vss->buffer_bytes;
> + vss->pcm_indirect.hw_buffer_size = vss->buffer_bytes;
> + if (vss->capture_staging) {
> + vss->capture_boundary = substream->runtime->boundary;
> + rc = virtsnd_pcm_capture_refresh_mode_locked(vss,
> + &free_running);
> + if (rc)
> + return rc;
> + }
> + }
>
> - memset(&vss->pcm_indirect, 0, sizeof(vss->pcm_indirect));
> - vss->pcm_indirect.sw_buffer_size =
> - vss->pcm_indirect.hw_buffer_size =
> - snd_pcm_lib_buffer_bytes(substream);
> + rc = virtsnd_pcm_dev_prepare(vss);
> + scoped_guard(spinlock_irqsave, &vss->lock) {
> + if (epoch != vss->reset_epoch || vss->resetting)
> + return -ECANCELED;
> + vss->prepared = !rc;
> + }
[Severity: Medium]
In virtsnd_pcm_prepare_locked(), if the substream was already prepared on the
device (vss->prepared was true, vss->stopped was false) and userspace calls
prepare again without pause_restart enabled, what happens if
virtsnd_pcm_dev_prepare() fails (or if virtsnd_pcm_dev_set_params() or
virtsnd_pcm_capture_refresh_mode_locked() fails earlier)?
Because vss->prepared is cleared to false at the start of
virtsnd_pcm_prepare_locked() without recording that the device stream was
already prepared, a failure when !pause_restart leaves both vss->prepared and
vss->stopped false. When virtsnd_pcm_hw_free() later runs on close:
scoped_guard(spinlock_irqsave, &vss->lock) {
vss->stopped |= vss->prepared;
vss->prepared = false;
vss->runtime_active = false;
vss->xfer_enabled = false;
}
rc = virtsnd_pcm_sync_stop_locked(substream);
virtsnd_pcm_sync_stop_locked() sees vss->prepared, vss->stopped, and
vss->msg_count all false/zero and skips sending the release command to the
device. Should a failed re-prepare preserve the need to release a previously
prepared device stream (or release it before re-preparing)?
[ ... ]
> @@ -372,38 +1042,51 @@ static int virtsnd_pcm_trigger(struct snd_pcm_substream *substream, int command)
[ ... ]
> case SNDRV_PCM_TRIGGER_SUSPEND:
> - vss->suspended = true;
> + scoped_guard(spinlock_irqsave, &vss->lock)
> + vss->suspended = true;
> fallthrough;
> case SNDRV_PCM_TRIGGER_STOP:
> case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> scoped_guard(spinlock_irqsave, &vss->lock) {
> + if (!vss->prepared)
> + return 0;
> if (command != SNDRV_PCM_TRIGGER_PAUSE_PUSH)
> vss->stopped = true;
> vss->xfer_enabled = false;
> }
>
> msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_STOP,
> GFP_KERNEL);
> if (!msg)
> return -ENOMEM;
>
> - return virtsnd_ctl_msg_send_sync(snd, msg);
> + return virtsnd_pcm_send_sync(vss, msg);
[Severity: High]
When virtsnd_pcm_pause_restart_enabled(vss) is false, can the stop and
pause_push cases in virtsnd_pcm_trigger() send a stop control request when the
device stream is not running?
Unlike virtsnd_pcm_pause_restart_stop(), which returns 0 when the state is not
running, this path only checks if (!vss->prepared). That appears to allow
sending an invalid or duplicate stop request in two situations:
1. When a running stream is paused via pause_push, virtsnd_pcm_trigger() sends
a stop request to the device while leaving vss->prepared = true and setting
vss->xfer_enabled = false. If that paused stream is then stopped (for example
via snd_pcm_drop(), close, or paused xrun handling in
virtsnd_pcm_period_elapsed() -> snd_pcm_stop()), snd_pcm_do_stop() invokes
virtsnd_pcm_trigger() with the stop command while vss->prepared is still true,
sending a second stop request to a stream that is already stopped on the
device.
2. If start or pause_release fails in virtsnd_pcm_trigger() on an unlinked
stream, virtsnd_pcm_start_failed() leaves vss->prepared = true. In
snd_pcm_action_single(), the error path calls ops->undo_action()
(snd_pcm_undo_start() or snd_pcm_undo_pause()), which invokes
virtsnd_pcm_trigger() with stop or pause_push and sends a stop request even
though start failed.
Also, if pause_push fails in virtsnd_pcm_ctl_msg_alloc() or
virtsnd_pcm_send_sync(), vss->xfer_enabled has already been cleared to false
without marking the stream stopped, and snd_pcm_action_single() will call
snd_pcm_undo_pause() -> virtsnd_pcm_trigger(pause_release) on the still-running
device stream. Should the non-pause-restart stop/pause path check whether the
stream was actually running before sending the stop request, and call
virtsnd_pcm_start_failed() if sending stop for pause_push fails?
> default:
> return -EINVAL;
> }
> +
> +start_failed:
> + virtsnd_pcm_start_failed(vss, epoch);
> + return rc;
> +}
[Severity: Low]
In virtsnd_pcm_trigger(), the start and pause_release case uses
scoped_guard(spinlock_irqsave, &queue->lock) and guard(spinlock)(&vss->lock)
and jumps out of the guarded scope via goto start_failed. Since
include/linux/cleanup.h discourages mixing goto labels with guard() and
scoped_guard() in the same function, could virtsnd_pcm_trigger() break out of
the scoped_guard block and handle the error without a goto label?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029054354.2931-1-mynameisjune111@gmail.com?part=9
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2026-10-29 6:23 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-29 5:43 [PATCH v8 00/14] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
2026-10-29 5:43 ` [PATCH v8 01/14] ALSA: virtio: Propagate PCM send failures through ACK Joonho Lee
2026-10-29 6:06 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 02/14] ALSA: virtio: Recover PCM state after a failed start Joonho Lee
2026-10-29 6:10 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 03/14] ALSA: virtio: Retain queued messages through device reset Joonho Lee
2026-10-29 6:14 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 04/14] ALSA: virtio: Keep PCM DMA buffers alive through device ownership Joonho Lee
2026-10-29 6:07 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 05/14] ALSA: virtio: Keep control payloads alive through DMA completion Joonho Lee
2026-10-29 5:54 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 06/14] ALSA: virtio: wait through signals for PCM teardown Joonho Lee
2026-10-29 6:00 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 07/14] ALSA: pcm: mark stop-threshold accesses for capture policy sampling Joonho Lee
2026-10-29 5:48 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 08/14] ALSA: pcm: stop paused streams before synchronizing teardown Joonho Lee
2026-10-29 6:04 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 09/14] ALSA: virtio: add staged capture accounting and lifecycle recovery Joonho Lee
2026-10-29 6:23 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 10/14] ALSA: virtio: prioritize staged capture period work Joonho Lee
2026-10-29 5:48 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 11/14] docs: describe virtio-sound capture options Joonho Lee
2026-10-29 5:47 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 12/14] ALSA: pcm: add an optional reset operation under the stream lock Joonho Lee
2026-10-29 5:52 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 13/14] ALSA: virtio: synchronize staged capture reset with PCM positions Joonho Lee
2026-10-29 6:05 ` sashiko-bot
2026-10-29 5:43 ` [PATCH v8 14/14] ALSA: virtio: synchronize indirect capture positions on reset Joonho Lee
2026-10-29 6:06 ` sashiko-bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.