* [PATCH v13 00/12] ALSA: virtio: staged capture recovery and PCM RESET synchronization
@ 2026-11-03 8:08 Joonho Lee
2026-11-03 8:08 ` [PATCH v13 01/12] ALSA: virtio: Propagate PCM send failures through ACK Joonho Lee
` (11 more replies)
0 siblings, 12 replies; 25+ messages in thread
From: Joonho Lee @ 2026-11-03 8:08 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc
Hi Takashi,
This revision addresses the v12 review and Randy's formatting comments.
It replaces v12 and includes the unmerged prerequisites. Apply patches
1-12 to sound.git for-next at
a4311e9447bbb219044db38897f95be112f46a8a.
Targeting 7.5 or later still works for me.
Previous series:
https://lore.kernel.org/linux-sound/20261103000403.67106-1-mynameisjune111@gmail.com/T/
Randy's review:
https://lore.kernel.org/linux-sound/dbf3b343-0af1-4297-b78a-bed68865d59f@infradead.org/
ACK now checks the PCM state before changing transfer state. A raw
SYNC_PTR racing with DROP previously changed SETUP to XRUN and re-armed
stop_operating. The correction preserves SETUP and rejects that ACK with
-EBADFD. Event callback admission is protected by the event queue lock,
so a callback pending at disable cannot dispatch into freed ALSA objects.
Enable and pending-event draining use the same lock.
The prepare gate is cleared on failure and device reset. A successful
hw_params clears the suspend flag, avoiding a second SET_PARAMS during
prepare. Staged pointer queries reject unavailable epochs while retaining
the released paused compatibility case. Playback pointer and ACK paths
honor latched device XRUNs, and TX completion preserves the latch. The
pause compatibility START path reports a latched XRUN as -EPIPE.
Staged ACK retains an unconsumed sub-byte frame for the next complete
byte, including appl_ptr boundary wrap. Capture RESET completes the
private cursor rebase on error paths, retaining the XRUN latch and
notification. Synchronous teardown owns a balanced work disable until
the next successful prepare, preventing RX completions from requeuing
period work after cancellation. The last completion always wakes the
release waiter.
Playback RESET changes behavior when TX requests remain device-owned:
it returns -EBUSY before changing the pointers or queue accounting.
With no outstanding TX, it rebases every indirect offset and clears
partial message fills together. DROP/PREPARE retires a busy generation.
This affects playback callers with capture_staging disabled as well.
I would appreciate review of this busy-TX RESET policy.
The refined partial-period test demonstrated why the accounting matters:
a 64-frame partial write before RESET caused a later 960-frame write to
submit a 1024-frame period. The correction waits for the remaining 64
frames. Linked RESET retains first-error and partial-completion behavior;
both member orders and group prepare recovery were tested.
Full-ring capture overwrite protection remains enabled. Delaying RX
observation can overwrite the ring before drain. The candidate still
returns -EPIPE in that case and returns the correct 64-frame tail for a
normal drain. The post-cancellation worker issue is handled separately
through work lifetime management.
The suspend-flag findings share one correction. The playback state
finding overlaps the ACK/DROP correction; its remaining XRUN handling
is included here. Capture compatibility options stay opt-in and default
to false. Every patch includes a diffstat, and the virtio_snd parameter
entries follow virtio_mmio in the documentation.
The series layout remains twelve patches: request and DMA lifetime in
1-6, core stop-threshold access in 7, partial-progress notification in 8,
the optional PCM reset operation in 9, capture/lifecycle/RESET handling
in 10, work priority in 11 and parameter documentation in 12. The core
source is unchanged from v12.
Validation selected 19 ARM64 QEMU runs with KASAN/lockdep and separate
KCSAN_STRICT tests, including expected-failing original comparisons.
The correction passed native and pause-compatibility lifecycle tests,
prepare recovery, reconfiguration, XRUN delivery, repeated teardown and
both linked RESET orders. The staged 4-bit ACK and pointer/error-reset
checks call the real driver functions with isolated kernel fixtures.
They do not establish negotiated hardware IMA_ADPCM capture support.
The suspend test models the flag before real hw_params calls.
Uninstrumented ordinary and staged capture each passed 70-second RW and
MMAP recordings with RESET, pause/resume and reopen. All 14 segments,
14,208,000 mono S16 frames at 48 kHz, matched independent sequence
hashes. These results cover ARM64 QEMU TCG with two vCPUs and 512 MiB.
Physical VZ/MOTU, host sleep/resume and broader format, channel and
architecture coverage remain pending. No general high-load scheduling
improvement is claimed.
LLM assistance was used for analysis, implementation, fixtures and this
explanation. The patches carry Assisted-by: LLM.
Thanks,
Joonho
base-commit: a4311e9447bbb219044db38897f95be112f46a8a
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v13 01/12] ALSA: virtio: Propagate PCM send failures through ACK
2026-11-03 8:08 [PATCH v13 00/12] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
@ 2026-11-03 8:08 ` Joonho Lee
2026-11-03 8:33 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 02/12] ALSA: virtio: Recover PCM state after a failed start Joonho Lee
` (10 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Joonho Lee @ 2026-11-03 8:08 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")
Reject ACK from invalid PCM states under the stream lock before modifying
private transfer state. A concurrent DROP must keep SETUP and must not
re-arm stop_operating through the ACK error path.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
sound/virtio/virtio_pcm_msg.c | 12 ++++
sound/virtio/virtio_pcm_ops.c | 116 ++++++++++++++++++++++++++++++++++++-----
2 files changed, 114 insertions(+), 14 deletions(-)
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
@@ -260,6 +260,18 @@
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
--- a/sound/virtio/virtio_pcm_ops.c
+++ b/sound/virtio/virtio_pcm_ops.c
@@ -466,36 +466,124 @@
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);
-
- virtsnd_pcm_msg_send(vss, rec->sw_data, bytes);
+ struct virtsnd_pcm_transfer *transfer =
+ container_of(rec, struct virtsnd_pcm_transfer, indirect);
+
+ if (transfer->error)
+ return;
+
+ 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;
+}
+
+/* The caller holds the PCM lock, also used by DROP and state transitions. */
+static int virtsnd_pcm_ack_state(struct snd_pcm_substream *substream)
+{
+ switch (substream->runtime->state) {
+ case SNDRV_PCM_STATE_XRUN:
+ return -EPIPE;
+ case SNDRV_PCM_STATE_PREPARED:
+ case SNDRV_PCM_STATE_RUNNING:
+ case SNDRV_PCM_STATE_PAUSED:
+ case SNDRV_PCM_STATE_DRAINING:
+ return 0;
+ default:
+ return -EBADFD;
+ }
}
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);
-
- guard(spinlock_irqsave)(&queue->lock);
- guard(spinlock)(&vss->lock);
-
- return snd_pcm_indirect_playback_transfer(substream, &vss->pcm_indirect,
- virtsnd_pcm_trans_copy);
+ struct virtsnd_pcm_transfer transfer = {};
+ int rc;
+
+ rc = virtsnd_pcm_ack_state(substream);
+ if (rc)
+ return rc;
+
+ 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 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);
-
- guard(spinlock_irqsave)(&queue->lock);
- guard(spinlock)(&vss->lock);
-
- return snd_pcm_indirect_capture_transfer(substream, &vss->pcm_indirect,
- virtsnd_pcm_trans_copy);
+ struct virtsnd_pcm_transfer transfer = {};
+ int rc;
+
+ rc = virtsnd_pcm_ack_state(substream);
+ if (rc)
+ return rc;
+
+ 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 0;
}
/* PCM substream operators map. */
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v13 02/12] ALSA: virtio: Recover PCM state after a failed start
2026-11-03 8:08 [PATCH v13 00/12] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
2026-11-03 8:08 ` [PATCH v13 01/12] ALSA: virtio: Propagate PCM send failures through ACK Joonho Lee
@ 2026-11-03 8:08 ` Joonho Lee
2026-11-03 8:55 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 03/12] ALSA: virtio: Retain queued messages through device reset Joonho Lee
` (9 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Joonho Lee @ 2026-11-03 8:08 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.
Record the RELEASE obligation after successful PREPARE and clear it as
soon as RELEASE succeeds. If the subsequent RX wait fails, retry that
wait without submitting RELEASE again.
Fixes: da76e9f3e43a ("ALSA: virtio: PCM substream operators")
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
sound/virtio/virtio_pcm.h | 6 ++
sound/virtio/virtio_pcm_ops.c | 106 ++++++++++++++++++++++++++++++++---------
2 files changed, 88 insertions(+), 24 deletions(-)
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
@@ -26,13 +26,13 @@
* @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.
@@ -56,6 +56,8 @@
bool xfer_enabled;
bool xfer_xrun;
bool stopped;
+ /* Successful PREPARE still needs RELEASE; protected by lock. */
+ bool release_required;
bool suspended;
struct virtio_pcm_msg **msgs;
unsigned int nmsgs;
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
@@ -96,6 +96,15 @@
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 @@
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,11 +227,23 @@
{
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;
}
@@ -273,6 +295,17 @@
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
struct virtio_device *vdev = vss->snd->vdev;
struct virtio_snd_msg *msg;
+ bool stopped;
+ int rc;
+
+ 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)) {
@@ -311,7 +344,12 @@
if (!msg)
return -ENOMEM;
- return virtsnd_ctl_msg_send_sync(vss->snd, msg);
+ rc = virtsnd_ctl_msg_send_sync(vss->snd, msg);
+ if (!rc) {
+ guard(spinlock_irqsave)(&vss->lock);
+ vss->release_required = true;
+ }
+ return rc;
}
/**
@@ -338,31 +376,40 @@
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 +440,34 @@
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->release_required;
+ if (need_release)
+ vss->stopped = true;
+ }
cancel_work_sync(&vss->elapsed_period);
- if (!vss->stopped)
- return 0;
-
- msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_RELEASE,
- GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
-
- rc = virtsnd_ctl_msg_send_sync(snd, msg);
- if (rc)
- return rc;
+ if (need_release) {
+ msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_RELEASE,
+ GFP_KERNEL);
+ if (!msg) {
+ substream->runtime->stop_operating = true;
+ return -ENOMEM;
+ }
+
+ rc = virtsnd_ctl_msg_send_sync(snd, msg);
+ if (rc) {
+ substream->runtime->stop_operating = true;
+ return rc;
+ }
+
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ vss->release_required = false;
+ }
/*
* The spec states that upon receipt of the RELEASE command "the device
@@ -421,11 +481,13 @@
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 [flat|nested] 25+ messages in thread
* [PATCH v13 03/12] ALSA: virtio: Retain queued messages through device reset
2026-11-03 8:08 [PATCH v13 00/12] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
2026-11-03 8:08 ` [PATCH v13 01/12] ALSA: virtio: Propagate PCM send failures through ACK Joonho Lee
2026-11-03 8:08 ` [PATCH v13 02/12] ALSA: virtio: Recover PCM state after a failed start Joonho Lee
@ 2026-11-03 8:08 ` Joonho Lee
2026-11-03 9:25 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 04/12] ALSA: virtio: Keep PCM DMA buffers alive through device ownership Joonho Lee
` (8 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Joonho Lee @ 2026-11-03 8:08 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.
Clear each queue wrapper inside the same critical section that detaches
its buffers. Check the callback's queue identity under the wrapper lock
for PCM, control and event queues. This makes queue retirement explicit
through teardown and restore. A forced callback test passed on both
versions; the proposed detached-ring IRQ failure was not reproduced on
the tested PCI transport.
Fixes: 9d45e514da88 ("ALSA: virtio: handling control messages")
Protect event callback admission with the event queue lock. Disabling
events waits for an active dispatch, and later callbacks see a closed
gate before ALSA objects are freed. Enable and pending-event draining
share the same lock.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
sound/virtio/virtio_card.c | 93 ++++++++++++++++++++++++++++++++---------
sound/virtio/virtio_card.h | 4 ++
sound/virtio/virtio_ctl_msg.c | 64 +++++++++++++++++++++++++---
sound/virtio/virtio_ctl_msg.h | 4 ++
sound/virtio/virtio_pcm.h | 2 +
sound/virtio/virtio_pcm_msg.c | 42 +++++++++++++++++--
6 files changed, 177 insertions(+), 32 deletions(-)
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
@@ -70,23 +70,13 @@
}
}
-/**
- * virtsnd_event_notify_cb() - Dispatch all reported events from the event queue.
- * @vqueue: Underlying event virtqueue.
- *
- * This callback function is called upon a vring interrupt request from the
- * device.
- *
- * Context: Interrupt context.
- */
-static void virtsnd_event_notify_cb(struct virtqueue *vqueue)
-{
- struct virtio_snd *snd = vqueue->vdev->priv;
- struct virtio_snd_queue *queue = virtsnd_event_queue(snd);
+static void virtsnd_event_drain_locked(struct virtio_snd *snd,
+ struct virtqueue *vqueue)
+{
struct virtio_snd_event *event;
u32 length;
- guard(spinlock_irqsave)(&queue->lock);
+ lockdep_assert_held(&virtsnd_event_queue(snd)->lock);
do {
virtqueue_disable_cb(vqueue);
while ((event = virtqueue_get_buf(vqueue, &length))) {
@@ -94,6 +84,26 @@
virtsnd_event_send(vqueue, event, true, GFP_ATOMIC);
}
} while (!virtqueue_enable_cb(vqueue));
+}
+
+/**
+ * virtsnd_event_notify_cb() - Dispatch all reported events from the event queue.
+ * @vqueue: Underlying event virtqueue.
+ *
+ * This callback function is called upon a vring interrupt request from the
+ * device.
+ *
+ * Context: Interrupt context.
+ */
+static void virtsnd_event_notify_cb(struct virtqueue *vqueue)
+{
+ struct virtio_snd *snd = vqueue->vdev->priv;
+ struct virtio_snd_queue *queue = virtsnd_event_queue(snd);
+
+ guard(spinlock_irqsave)(&queue->lock);
+ if (queue->vqueue != vqueue || !snd->event_vq_enabled)
+ return;
+ virtsnd_event_drain_locked(snd, vqueue);
}
/**
@@ -158,8 +168,12 @@
{
struct virtio_snd_queue *queue = virtsnd_event_queue(snd);
+ guard(spinlock_irqsave)(&queue->lock);
+ if (!queue->vqueue)
+ return;
+ snd->event_vq_enabled = true;
if (!virtqueue_enable_cb(queue->vqueue))
- virtsnd_event_notify_cb(queue->vqueue);
+ virtsnd_event_drain_locked(snd, queue->vqueue);
}
/**
@@ -174,8 +188,9 @@
struct virtio_snd_event *event;
u32 length;
+ guard(spinlock_irqsave)(&queue->lock);
+ snd->event_vq_enabled = false;
if (queue->vqueue) {
- guard(spinlock_irqsave)(&queue->lock);
virtqueue_disable_cb(queue->vqueue);
while ((event = virtqueue_get_buf(queue->vqueue, &length)))
virtsnd_event_dispatch(snd, event);
@@ -338,6 +353,40 @@
}
/**
+ * 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);
+ if (i == VIRTIO_SND_VQ_EVENT)
+ snd->event_vq_enabled = false;
+ /* IRQ callbacks must never see a detached ring as live. */
+ queue->vqueue = NULL;
+ spin_unlock_irqrestore(&queue->lock, flags);
+ }
+
+ vdev->config->del_vqs(vdev);
+}
+
+/**
* virtsnd_remove() - Remove VirtIO and ALSA devices.
* @vdev: VirtIO parent device.
*
@@ -349,13 +398,14 @@
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 +433,8 @@
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 +462,7 @@
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
--- a/sound/virtio/virtio_card.h
+++ b/sound/virtio/virtio_card.h
@@ -47,6 +47,8 @@
* @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_vq_enabled: Event callbacks may dispatch, protected by the event queue lock.
* @event_msgs: Device events.
* @pcm_list: VirtIO PCM device list.
* @jacks: VirtIO jacks.
@@ -64,6 +66,8 @@
struct virtio_snd_queue queues[VIRTIO_SND_VQ_MAX];
struct snd_card *card;
struct list_head ctl_msgs;
+ bool ctl_msgs_blocked;
+ bool event_vq_enabled;
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
--- 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 list_head list;
struct completion notify;
refcount_t ref_count;
+ bool cancelled;
};
/**
@@ -100,7 +102,7 @@
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 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 @@
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);
}
}
@@ -168,9 +176,6 @@
goto on_exit;
}
-
- if (notify)
- virtqueue_notify(queue->vqueue);
if (nowait)
goto on_exit;
@@ -184,6 +189,11 @@
rc = -ETIMEDOUT;
}
+ goto on_exit;
+ }
+
+ if (READ_ONCE(msg->cancelled)) {
+ rc = -ENODEV;
goto on_exit;
}
@@ -227,9 +237,33 @@
* 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);
@@ -244,6 +278,20 @@
}
/**
+ * 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.
* @command: Control request code (VIRTIO_SND_R_XXX_INFO).
@@ -295,6 +343,8 @@
u32 length;
guard(spinlock_irqsave)(&queue->lock);
+ if (queue->vqueue != vqueue)
+ return;
do {
virtqueue_disable_cb(vqueue);
while ((msg = virtqueue_get_buf(vqueue, &length)))
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
@@ -68,6 +68,10 @@
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
--- a/sound/virtio/virtio_pcm.h
+++ b/sound/virtio/virtio_pcm.h
@@ -122,6 +122,8 @@
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
--- a/sound/virtio/virtio_pcm_msg.c
+++ b/sound/virtio/virtio_pcm_msg.c
@@ -305,6 +305,35 @@
{
guard(spinlock_irqsave)(&vss->lock);
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);
}
/**
@@ -367,16 +396,21 @@
/**
* virtsnd_pcm_notify_cb() - Process all completed I/O messages.
- * @queue: Underlying tx/rx virtqueue.
+ * @queue: Underlying tx/rx queue wrapper.
+ * @vqueue: Queue that raised this callback.
*
* Context: Interrupt context. Takes and releases the tx/rx queue spinlock.
*/
-static inline void virtsnd_pcm_notify_cb(struct virtio_snd_queue *queue)
+static inline void
+virtsnd_pcm_notify_cb(struct virtio_snd_queue *queue, struct virtqueue *vqueue)
{
struct virtio_pcm_msg *msg;
u32 written_bytes;
guard(spinlock_irqsave)(&queue->lock);
+ /* Reset can detach the queue before the transport drains its IRQs. */
+ if (queue->vqueue != vqueue)
+ return;
do {
virtqueue_disable_cb(queue->vqueue);
while ((msg = virtqueue_get_buf(queue->vqueue, &written_bytes)))
@@ -394,7 +428,7 @@
{
struct virtio_snd *snd = vqueue->vdev->priv;
- virtsnd_pcm_notify_cb(virtsnd_tx_queue(snd));
+ virtsnd_pcm_notify_cb(virtsnd_tx_queue(snd), vqueue);
}
/**
@@ -407,7 +441,7 @@
{
struct virtio_snd *snd = vqueue->vdev->priv;
- virtsnd_pcm_notify_cb(virtsnd_rx_queue(snd));
+ virtsnd_pcm_notify_cb(virtsnd_rx_queue(snd), vqueue);
}
/**
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v13 04/12] ALSA: virtio: Keep PCM DMA buffers alive through device ownership
2026-11-03 8:08 [PATCH v13 00/12] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (2 preceding siblings ...)
2026-11-03 8:08 ` [PATCH v13 03/12] ALSA: virtio: Retain queued messages through device reset Joonho Lee
@ 2026-11-03 8:08 ` Joonho Lee
2026-11-03 9:49 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 05/12] ALSA: virtio: Keep control payloads alive through DMA completion Joonho Lee
` (7 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Joonho Lee @ 2026-11-03 8:08 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.
Require whole-byte periods whose total matches the negotiated buffer.
Check offset, length and the final message index before indexing the
message array, including empty transfers and arithmetic wraparound.
Seed the ordinary capture ring once per prepare. Native STOP preserves
queued requests and unread samples, so PAUSE_RELEASE must retain their
ownership. Re-enqueuing the entire ring adds another period to pending
message lengths and exposes unread DMA slots to the device. Return
consumed space through the existing indirect ACK path.
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>
---
sound/virtio/virtio_pcm.h | 13 ++
sound/virtio/virtio_pcm_msg.c | 262 +++++++++++++++++++++++++++++++++++++----
sound/virtio/virtio_pcm_ops.c | 19 +++
3 files changed, 270 insertions(+), 24 deletions(-)
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
@@ -13,6 +13,7 @@
struct virtio_pcm;
struct virtio_pcm_msg;
+struct page;
/**
* struct virtio_pcm_substream - VirtIO PCM substream.
@@ -29,6 +30,7 @@
* substream operators, including @stopped.
* @buffer_bytes: Current buffer size in bytes.
* @hw_ptr: Substream hardware pointer value in bytes [0 ... buffer_bytes).
+ * @capture_rx_seeded: Ordinary capture ring was queued since prepare.
* @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
@@ -38,6 +40,10 @@
* @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 {
@@ -53,6 +59,7 @@
spinlock_t lock;
size_t buffer_bytes;
size_t hw_ptr;
+ bool capture_rx_seeded;
bool xfer_enabled;
bool xfer_xrun;
bool stopped;
@@ -62,6 +69,12 @@
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
--- 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_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 @@
};
/**
- * 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,14 +53,18 @@
*/
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)
@@ -75,11 +85,80 @@
return num;
}
-/**
- * virtsnd_pcm_sg_from() - Build sg-list from vmalloc'ed buffer.
+#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_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 @@
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;
+
+ 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)
- return -ENOMEM;
+ 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 @@
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;
@@ -199,6 +348,7 @@
guard(spinlock_irqsave)(&vss->lock);
for (i = 0; vss->msgs && i < vss->nmsgs; ++i)
vss->msgs[i]->length = 0;
+ vss->capture_rx_seeded = false;
}
/**
@@ -228,8 +378,15 @@
bool notify = false;
int rc;
+ if (!bytes)
+ return 0;
+ if (!period_bytes || !vss->msgs || offset >= vss->buffer_bytes ||
+ bytes > vss->buffer_bytes - offset)
+ return -EINVAL;
start = offset / period_bytes;
end = (offset + bytes - 1) / period_bytes;
+ if (end >= vss->nmsgs)
+ return -EINVAL;
for (i = start; i <= end; i++) {
struct virtio_pcm_msg *msg = vss->msgs[i];
@@ -248,6 +405,33 @@
if (msg->length == period_bytes) {
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,
@@ -363,15 +547,48 @@
* 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 +604,6 @@
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
--- 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:
*
@@ -228,8 +230,19 @@
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
struct virtio_device *vdev = vss->snd->vdev;
unsigned int msg_count;
+ size_t period_buffer_bytes;
bool stopped;
int rc;
+
+ /* Fixed descriptors require whole-byte periods covering the buffer. */
+ if (check_mul_overflow((size_t)params_periods(hw_params),
+ (size_t)params_period_bytes(hw_params),
+ &period_buffer_bytes) ||
+ period_buffer_bytes != params_buffer_bytes(hw_params) ||
+ (params_period_size(hw_params) *
+ (u64)snd_pcm_format_physical_width(params_format(hw_params)) *
+ params_channels(hw_params)) % 8)
+ return -EINVAL;
scoped_guard(spinlock_irqsave, &vss->lock) {
msg_count = vss->msg_count;
@@ -380,8 +393,12 @@
substream->runtime->stop_operating = true;
return -EBADFD;
}
- if (vss->direction == SNDRV_PCM_STREAM_CAPTURE)
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
+ !vss->capture_rx_seeded) {
rc = virtsnd_pcm_msg_send(vss, 0, vss->buffer_bytes);
+ if (!rc)
+ vss->capture_rx_seeded = true;
+ }
if (rc) {
vss->stopped = true;
substream->runtime->stop_operating = true;
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v13 05/12] ALSA: virtio: Keep control payloads alive through DMA completion
2026-11-03 8:08 [PATCH v13 00/12] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (3 preceding siblings ...)
2026-11-03 8:08 ` [PATCH v13 04/12] ALSA: virtio: Keep PCM DMA buffers alive through device ownership Joonho Lee
@ 2026-11-03 8:08 ` Joonho Lee
2026-11-03 9:56 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 06/12] ALSA: virtio: wait through signals for PCM teardown Joonho Lee
` (6 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Joonho Lee @ 2026-11-03 8:08 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.
Keep response values in a separately allocated aligned response buffer
so INTEGER64 control accesses are aligned from this patch onward.
Fixes: 9d45e514da88 ("ALSA: virtio: handling control messages")
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
sound/virtio/virtio_ctl_msg.c | 289 +++++++++++++++++++++++++++++++----------
sound/virtio/virtio_ctl_msg.h | 24 ++-
sound/virtio/virtio_kctl.c | 121 +++++++++--------
sound/virtio/virtio_pcm_msg.c | 2
4 files changed, 300 insertions(+), 136 deletions(-)
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
@@ -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_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);
+ }
}
/**
@@ -71,12 +86,73 @@
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 @@
* 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;
-
- if (!request_size || !response_size)
+ 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 || request_size > UINT_MAX ||
+ response_size > UINT_MAX || request_data_size > UINT_MAX ||
+ response_data_size > UINT_MAX)
return NULL;
-
- msg = kzalloc(sizeof(*msg) + request_size + response_size, 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));
+
+ /* 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), 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 @@
* 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 @@
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 @@
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);
-
- list_add_tail(&msg->list, &snd->ctl_msgs);
- if (notify)
- virtqueue_notify(queue->vqueue);
- }
- }
+ if (msg->response_data_size)
+ psgs[nouts + nins++] = &msg->sg_response_data;
+
+ 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 @@
* 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 @@
{
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 @@
query->count = cpu_to_le32(count);
query->size = cpu_to_le32(size);
- sg_init_one(&sg, info, count * size);
-
- return virtsnd_ctl_msg_send(snd, msg, NULL, &sg, false);
+ 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 rc;
}
/**
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
@@ -21,20 +21,28 @@
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 @@
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_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
--- 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 <linux/limits.h>
+#include <linux/overflow.h>
+#include <linux/virtio_config.h>
#include <sound/control.h>
-#include <linux/virtio_config.h>
#include "virtio_card.h"
@@ -157,14 +159,16 @@
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, GFP_KERNEL);
+ msg = virtsnd_ctl_msg_alloc(request_size, response_size, 0,
+ sizeof(*kvalue), GFP_KERNEL);
if (!msg)
return -ENOMEM;
+ /* Response lifetime through ALSA value copyout. */
virtsnd_ctl_msg_ref(msg);
hdr = virtsnd_ctl_msg_request(msg);
@@ -175,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:
@@ -233,7 +236,8 @@
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,22 +292,27 @@
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 +321,16 @@
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;
- }
-
- break;
+ /* 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);
+
+ return rc;
case SNDRV_CTL_TLV_OP_WRITE:
case SNDRV_CTL_TLV_OP_CMD:
if (op_flag == SNDRV_CTL_TLV_OP_WRITE)
@@ -327,28 +339,16 @@
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;
- }
- kfree(tlv);
- return rc;
-
-on_msg_unref:
- virtsnd_ctl_msg_unref(msg);
- kfree(tlv);
-
- return rc;
+ return virtsnd_ctl_msg_send(snd, msg, false);
+ }
+
+ return -EINVAL;
}
/**
@@ -369,27 +369,36 @@
struct virtio_snd_msg *msg;
struct virtio_snd_ctl_hdr *hdr;
unsigned int n = le32_to_cpu(kinfo->value.enumerated.items);
- struct scatterlist sg;
-
- msg = virtsnd_ctl_msg_alloc(sizeof(*hdr),
- sizeof(struct virtio_snd_hdr), GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
+ size_t items_size;
+ int rc;
+
+ 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);
- return -ENOMEM;
- }
-
- sg_init_one(&sg, kctl->items, n * sizeof(*kctl->items));
+ if (!kctl->items)
+ return -ENOMEM;
+
+ 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
--- a/sound/virtio/virtio_pcm_msg.c
+++ b/sound/virtio/virtio_pcm_msg.c
@@ -684,7 +684,7 @@
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 [flat|nested] 25+ messages in thread
* [PATCH v13 06/12] ALSA: virtio: wait through signals for PCM teardown
2026-11-03 8:08 [PATCH v13 00/12] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (4 preceding siblings ...)
2026-11-03 8:08 ` [PATCH v13 05/12] ALSA: virtio: Keep control payloads alive through DMA completion Joonho Lee
@ 2026-11-03 8:08 ` Joonho Lee
2026-11-03 10:15 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 07/12] ALSA: pcm: mark stop-threshold accesses for capture policy sampling Joonho Lee
` (5 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Joonho Lee @ 2026-11-03 8:08 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>
---
sound/virtio/virtio_ctl_msg.c | 15 ++++++++++++++-
sound/virtio/virtio_ctl_msg.h | 3 +++
2 files changed, 17 insertions(+), 1 deletion(-)
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
@@ -314,7 +314,20 @@
*/
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
--- a/sound/virtio/virtio_ctl_msg.h
+++ b/sound/virtio/virtio_ctl_msg.h
@@ -47,6 +47,9 @@
* 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 [flat|nested] 25+ messages in thread
* [PATCH v13 07/12] ALSA: pcm: mark stop-threshold accesses for capture policy sampling
2026-11-03 8:08 [PATCH v13 00/12] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (5 preceding siblings ...)
2026-11-03 8:08 ` [PATCH v13 06/12] ALSA: virtio: wait through signals for PCM teardown Joonho Lee
@ 2026-11-03 8:08 ` Joonho Lee
2026-11-03 10:17 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 08/12] ALSA: pcm: report partial progress under the stream lock Joonho Lee
` (4 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Joonho Lee @ 2026-11-03 8:08 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 patch 9.
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>
---
sound/core/oss/pcm_oss.c | 2 +-
sound/core/pcm.c | 3 ++-
sound/core/pcm_native.c | 5 +++--
3 files changed, 6 insertions(+), 4 deletions(-)
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] 25+ messages in thread
* [PATCH v13 08/12] ALSA: pcm: report partial progress under the stream lock
2026-11-03 8:08 [PATCH v13 00/12] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (6 preceding siblings ...)
2026-11-03 8:08 ` [PATCH v13 07/12] ALSA: pcm: mark stop-threshold accesses for capture policy sampling Joonho Lee
@ 2026-11-03 8:08 ` Joonho Lee
2026-11-03 10:20 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 09/12] ALSA: pcm: add an optional reset operation " Joonho Lee
` (3 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Joonho Lee @ 2026-11-03 8:08 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
Staged capture can make readable data available before a complete period
has elapsed. Skipping notification leaves a blocked reader asleep, while
using the period-interrupt path for this event can infer a false ring wrap
when the notification is delayed.
Add snd_pcm_progress_under_stream_lock() to update the hardware pointer
through the non-interrupt path and notify readers and fasync listeners.
It runs under the caller's PCM stream lock and does not tick the PCM timer.
The driver must release its pointer and trigger locks before calling it.
Existing period notification callers keep their current behavior.
Atomic and nonatomic synthetic PCM tests reproduced the missing wakeup
and false-wrap alternatives. The new helper delivered 64 final frames and
reported four available frames after delayed partial progress without XRUN.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
include/sound/pcm.h | 1 +
sound/core/pcm_lib.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -1182,6 +1182,7 @@
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);
+void snd_pcm_progress_under_stream_lock(struct snd_pcm_substream *substream);
void snd_pcm_period_elapsed(struct snd_pcm_substream *substream);
snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
void *buf, bool interleaved,
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
@@ -1919,6 +1919,25 @@
EXPORT_SYMBOL(snd_pcm_period_elapsed_under_stream_lock);
/**
+ * snd_pcm_progress_under_stream_lock - Report progress without a period IRQ
+ * @substream: PCM substream
+ *
+ * Context: PCM stream lock held, with no driver pointer or trigger locks held.
+ * Updates the pointer and wakes readers for partial-period transfers. Unlike
+ * a period notification, this does not imply an elapsed period or tick the PCM
+ * timer. The same pointer, trigger and timestamp callbacks may be invoked.
+ */
+void snd_pcm_progress_under_stream_lock(struct snd_pcm_substream *substream)
+{
+ if (PCM_RUNTIME_CHECK(substream) || !snd_pcm_running(substream))
+ return;
+ snd_pcm_update_hw_ptr(substream);
+ snd_kill_fasync(substream->runtime->fasync, SIGIO, POLL_IN);
+}
+EXPORT_SYMBOL_GPL(snd_pcm_progress_under_stream_lock);
+
+
+/**
* snd_pcm_period_elapsed() - update the status of runtime for the next period by acquiring lock of
* PCM substream.
* @substream: the instance of PCM substream.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v13 09/12] ALSA: pcm: add an optional reset operation under the stream lock
2026-11-03 8:08 [PATCH v13 00/12] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (7 preceding siblings ...)
2026-11-03 8:08 ` [PATCH v13 08/12] ALSA: pcm: report partial progress under the stream lock Joonho Lee
@ 2026-11-03 8:08 ` Joonho Lee
2026-11-03 10:27 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 10/12] ALSA: virtio: add staged capture and serialize PCM lifecycle accounting Joonho Lee
` (2 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Joonho Lee @ 2026-11-03 8:08 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
A driver may need to synchronize private capture accounting with the
common RESET pointer observation and the core's position rebase.
Add an optional reset operation called under the member's stream lock.
Expose snd_pcm_lib_reset() so the callback can retain common pointer and
time handling. On zero return the core completes that member's rebase
before unlocking it. Drivers without the callback retain the ioctl path.
The operation is also used after a successful prepare.
Zero permits the position rebase; it does not promise XRUN recovery.
A negative return skips the core rebase, so a callback returning an error
must leave coherent pointer state even after partial work.
Linked RESET and prepare stop at the first error without rollback.
Earlier callback members may already have completed. Legacy members keep
the existing post_action policy. Applications must recover the group
after partial failure before relying on shared positions.
This adds an internal kernel operation and no userspace ioctl or layout.
Out-of-tree modules using snd_pcm_ops need to be rebuilt.
Complete appl_ptr rebasing once in the callback path. A later linked
member can sleep in prepare while the earlier member accepts SYNC_PTR;
a subsequent unlocked post_prepare assignment would overwrite that
acknowledgment. Keep explicit RESET playback silence processing and
leave prepare-time filling to the normal START path, preserving samples
written and rewound after prepare.
Atomic and nonatomic synthetic-driver tests exercised both callback and
legacy members, mixed linked groups and later-member failures. The
callback retained an acknowledged appl_ptr of 64, and finite-silence
playback kept the written prefix while clearing the rewound tail.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
include/sound/pcm.h | 21 +++++++++++++++++++++
sound/core/pcm_lib.c | 29 ++++++++++++++++++++++++-----
sound/core/pcm_native.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
3 files changed, 85 insertions(+), 10 deletions(-)
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,26 @@
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 to let the core complete the position rebase. As with
+ * the legacy RESET ioctl, zero does not guarantee recovery from XRUN;
+ * callers must still handle the stream state and subsequent I/O errors.
+ * On a negative return the core does not finish the rebase, so the
+ * callback must leave coherent pointer state even after partial work.
+ *
+ * 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 +1199,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
@@ -1984,11 +1984,35 @@
}
}
-static int snd_pcm_do_reset(struct snd_pcm_substream *substream,
- snd_pcm_state_t state)
+static int snd_pcm_reset_substream(struct snd_pcm_substream *substream,
+ bool for_prepare)
{
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 (!for_prepare && 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);
@@ -2000,10 +2024,19 @@
return 0;
}
+static int snd_pcm_do_reset(struct snd_pcm_substream *substream,
+ snd_pcm_state_t state)
+{
+ return snd_pcm_reset_substream(substream, false);
+}
+
static void snd_pcm_post_reset(struct snd_pcm_substream *substream,
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 &&
@@ -2053,14 +2086,16 @@
if (err < 0)
return err;
pcm_qos_req_update(substream, period_to_usecs(substream->runtime));
- return snd_pcm_do_reset(substream, state);
+ return snd_pcm_reset_substream(substream, true);
}
static void snd_pcm_post_prepare(struct snd_pcm_substream *substream,
snd_pcm_state_t state)
{
struct snd_pcm_runtime *runtime = substream->runtime;
- runtime->control->appl_ptr = runtime->status->hw_ptr;
+ /* The reset callback path already completed this rebase under the lock. */
+ if (!substream->ops->reset)
+ runtime->control->appl_ptr = runtime->status->hw_ptr;
snd_pcm_set_state(substream, SNDRV_PCM_STATE_PREPARED);
}
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v13 10/12] ALSA: virtio: add staged capture and serialize PCM lifecycle accounting
2026-11-03 8:08 [PATCH v13 00/12] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (8 preceding siblings ...)
2026-11-03 8:08 ` [PATCH v13 09/12] ALSA: pcm: add an optional reset operation " Joonho Lee
@ 2026-11-03 8:08 ` Joonho Lee
2026-11-03 11:05 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 11/12] ALSA: virtio: prioritize staged capture period work Joonho Lee
2026-11-03 8:08 ` [PATCH v13 12/12] docs: describe virtio-sound capture options Joonho Lee
11 siblings, 1 reply; 25+ messages in thread
From: Joonho Lee @ 2026-11-03 8:08 UTC (permalink / raw)
To: linux-sound
Cc: tiwai, perex, anton.yakovlev, mst, virtualization, linux-kernel,
corbet, skhan, rdunlap, linux-doc, Joonho Lee
Use request-owned RX storage and separate produced, observed and consumed
positions for shared capture clients. Serialize lifecycle operations with
lifecycle_lock and ACK accounting with the PCM lock.
Close the prepare gate only while prepare and its core pointer reset are
in progress. Failed prepare clears that transient gate while preserving
the error latch. Successful hw_params clears a stale suspend flag so
prepare does not repeat SET_PARAMS. Staged pointer queries reject an
unprepared, preparing or suspended epoch, preserving the released paused
compatibility case.
Keep fractional-byte capture ACK progress until the next complete byte,
including boundary wrap. Playback retains device XRUN errors through TX
completion and rejects them in both pointer and ACK callbacks. The pause
compatibility START path returns -EPIPE for a latched XRUN.
RESET of playback with device-owned TX requests returns -EBUSY before
changing either pointer. The RESET callback holds the PCM lock and does
not acquire lifecycle_lock to retire requests. With no outstanding TX,
rebase all indirect offsets and clear partial message fills together.
Applications can use DROP/PREPARE to retire a busy epoch before RESET.
This return-value change needs maintainer review.
Complete capture's private cursor rebase on RESET error paths while
retaining the XRUN latch and notification. This permits the core's
matching appl_ptr update without claiming stream recovery.
Synchronous teardown owns a balanced disable_work_sync()/enable_work()
pair until the next successful prepare. Late RX completion cannot queue
period work after teardown cancellation. Always wake the release waiter
when the pending message count reaches zero. Keep full-ring overwrite
protection; discarding post-STOP payloads can expose a stale capture tail.
The capture compatibility options remain opt-in and disabled by default.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
sound/virtio/virtio_card.c | 34 +
sound/virtio/virtio_ctl_msg.c | 58 +
sound/virtio/virtio_ctl_msg.h | 9
sound/virtio/virtio_pcm.c | 125 +++
sound/virtio/virtio_pcm.h | 143 +++
sound/virtio/virtio_pcm_msg.c | 488 ++++++++++--
sound/virtio/virtio_pcm_ops.c | 1681 ++++++++++++++++++++++++++++++++++++++---
7 files changed, 2321 insertions(+), 217 deletions(-)
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
@@ -363,6 +363,23 @@
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->preparing = false;
+ vss->device_running = false;
+ vss->release_required = false;
+ vss->control_uncertain = false;
+ vss->capture_reset_observing = false;
+ vss->xfer_enabled = false;
+ vss->suspended = true;
+ }
virtio_reset_device(vdev);
/* Deleting a virtqueue does not unmap its outstanding descriptors. */
@@ -382,8 +399,16 @@
queue->vqueue = NULL;
spin_unlock_irqrestore(&queue->lock, flags);
}
-
vdev->config->del_vqs(vdev);
+ 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;
+ }
}
/**
@@ -456,6 +481,7 @@
{
struct virtio_snd *snd = vdev->priv;
int rc;
+ unsigned int i;
rc = virtsnd_find_vqs(snd);
if (rc)
@@ -464,6 +490,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,9 @@
* @list: Pending message list entry.
* @notify: Request completed notification.
* @ref_count: Reference count used to manage a message lifetime.
+ * @pcm: Optional PCM generation owner, valid until control queue teardown.
+ * @pcm_epoch: Operation generation captured before request allocation.
+ * @submitted: Request was handed to the device.
* @cancelled: Request canceled during device teardown.
*/
struct virtio_snd_msg {
@@ -37,7 +40,49 @@
struct completion notify;
refcount_t ref_count;
bool cancelled;
+ bool submitted;
+ struct virtio_pcm_substream *pcm;
+ u64 pcm_epoch;
};
+
+/* Bind an operation's original generation before publishing the request. */
+void virtsnd_ctl_msg_bind_pcm(struct virtio_snd_msg *msg,
+ struct virtio_pcm_substream *vss, u64 epoch)
+{
+ msg->pcm = vss;
+ msg->pcm_epoch = epoch;
+}
+
+u64 virtsnd_ctl_msg_pcm_epoch(struct virtio_snd_msg *msg)
+{
+ return msg->pcm_epoch;
+}
+
+static int virtsnd_ctl_msg_enqueue(struct virtio_snd *snd,
+ struct virtio_snd_msg *msg,
+ struct scatterlist **sgs,
+ unsigned int nouts, unsigned int nins)
+{
+ struct virtqueue *vq = virtsnd_control_queue(snd)->vqueue;
+
+ if (!msg->pcm)
+ return virtqueue_add_sgs(vq, sgs, nouts, nins, msg, GFP_ATOMIC);
+
+ /* Control queue -> PCM lock serializes enqueue against device reset. */
+ guard(spinlock)(&msg->pcm->lock);
+ if (msg->pcm->resetting || msg->pcm->reset_epoch != msg->pcm_epoch)
+ return -ECANCELED;
+ return virtqueue_add_sgs(vq, sgs, nouts, nins, msg, GFP_ATOMIC);
+}
+
+/* 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.
@@ -293,9 +338,9 @@
if (snd->ctl_msgs_blocked)
rc = -ENODEV;
else
- rc = virtqueue_add_sgs(queue->vqueue, psgs, nouts, nins, msg,
- GFP_ATOMIC);
+ rc = virtsnd_ctl_msg_enqueue(snd, msg, psgs, nouts, nins);
if (!rc) {
+ msg->submitted = true;
notify = virtqueue_kick_prepare(queue->vqueue);
list_add_tail(&msg->list, &snd->ctl_msgs);
@@ -315,11 +360,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,13 @@
struct virtio_snd;
struct virtio_snd_msg;
+struct virtio_pcm_substream;
+
+void virtsnd_ctl_msg_bind_pcm(struct virtio_snd_msg *msg,
+ struct virtio_pcm_substream *vss, u64 epoch);
+u64 virtsnd_ctl_msg_pcm_epoch(struct virtio_snd_msg *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 +54,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_pcm.c b/sound/virtio/virtio_pcm.c
--- a/sound/virtio/virtio_pcm.c
+++ b/sound/virtio/virtio_pcm.c
@@ -310,8 +310,119 @@
{
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 partial = false;
+ bool requeue = false;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ staged_capture = vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
+ vss->capture_staging;
+ }
+
+ /* 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->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;
+ }
+
+ if (!staged_capture) {
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE) {
+ guard(spinlock_irqsave)(&vss->lock);
+ period_bytes = snd_pcm_lib_period_bytes(substream);
+ period_floor = period_bytes ?
+ div64_ul(vss->capture_copied_total, period_bytes) : 0;
+ partial = vss->xfer_xrun ||
+ period_floor <= vss->capture_notified_periods;
+ if (!partial)
+ vss->capture_notified_periods = period_floor;
+ }
+ /* Resume recovery can wake readers without a new period IRQ. */
+ if (partial)
+ snd_pcm_progress_under_stream_lock(substream);
+ else
+ snd_pcm_period_elapsed_under_stream_lock(substream);
+ 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;
+ } else if (vss->capture_copied_total >
+ vss->capture_pointer_seen_total) {
+ notify = true;
+ partial = 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. */
+ if (partial)
+ snd_pcm_progress_under_stream_lock(substream);
+ else
+ 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 &&
+ vss->capture_copied_total > vss->capture_pointer_seen_total)
+ requeue = true;
+ }
+ }
+
+ if (requeue)
+ virtsnd_pcm_schedule_period_elapsed(vss);
}
/**
@@ -351,8 +462,10 @@
vss->snd = snd;
vss->sid = i;
INIT_WORK(&vss->elapsed_period, virtsnd_pcm_period_elapsed);
+ disable_work(&vss->elapsed_period);
init_waitqueue_head(&vss->msg_empty);
spin_lock_init(&vss->lock);
+ mutex_init(&vss->lifecycle_lock);
}
info = kzalloc_objs(*info, snd->nsubstreams);
@@ -486,7 +599,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 +630,12 @@
break;
case VIRTIO_SND_EVT_PCM_XRUN:
scoped_guard(spinlock, &vss->lock) {
- if (vss->xfer_enabled)
+ if (vss->xfer_enabled ||
+ (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
+ vss->prepared && !vss->stopped)) {
vss->xfer_xrun = true;
+ 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,17 +45,45 @@
* @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.
+ * @period_work_quiesced: Teardown owns a work disable, protected by @lifecycle_lock.
+ * @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_rx_seeded: Ordinary capture ring was queued since prepare.
+ * @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.
+ * @preparing: ACK gate held through prepare's core pointer reset; protected by
+ * the PCM stream lock and @lock.
+ * @device_running: Native START succeeded without a subsequent successful STOP.
+ * @release_required: Successful PREPARE still needs a successful RELEASE.
+ * @control_uncertain: Native control result requires device reset before reuse.
+ * @capture_reset_observing: Reclaim staged samples during RESET pointer capture.
+ * @indirect_reset_observing: Suppress ordinary ACK refill during RESET pointer
+ * capture; protected by the PCM stream 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.
@@ -57,15 +104,39 @@
struct snd_pcm_hardware hw;
struct work_struct elapsed_period;
spinlock_t lock;
+ struct mutex lifecycle_lock;
+ bool period_work_quiesced;
+ 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_rx_seeded;
+ bool capture_free_running;
+ unsigned int capture_empty;
bool xfer_enabled;
bool xfer_xrun;
bool stopped;
- /* Successful PREPARE still needs RELEASE; protected by lock. */
+ bool prepared;
+ bool preparing;
+ /* Native control state, protected by lock. */
+ bool device_running;
bool release_required;
+ bool control_uncertain;
+ bool capture_reset_observing;
+ bool indirect_reset_observing;
bool suspended;
+ enum virtio_pcm_pause_restart_state pause_restart_state;
struct virtio_pcm_msg **msgs;
unsigned int nmsgs;
unsigned int msg_count;
@@ -78,6 +149,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;
+}
+
+/* Keep non-staged PCM notifications on schedule_work()'s normal queue. */
+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.
@@ -108,6 +237,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);
@@ -126,11 +260,16 @@
struct virtio_snd_msg *
virtsnd_pcm_ctl_msg_alloc(struct virtio_pcm_substream *vss,
- unsigned int command, gfp_t gfp);
+ unsigned int command, gfp_t gfp, u64 epoch);
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,12 @@
* virtio-snd: Virtio sound device
* Copyright (C) 2021 OpenSynergy GmbH
*/
+#include <linux/compiler.h>
+#include <linux/gcd.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 +19,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 +233,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 +243,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 +267,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 +350,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 +379,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;
vss->capture_rx_seeded = false;
}
@@ -378,6 +429,8 @@
bool notify = false;
int rc;
+ if (!vqueue || !vss->prepared || vss->suspended)
+ return -EBADFD;
if (!bytes)
return 0;
if (!period_bytes || !vss->msgs || offset >= vss->buffer_bytes ||
@@ -403,8 +456,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) {
@@ -478,6 +531,293 @@
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 (msg->substream->capture_staging && 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--;
+ /* Native pause stops refills but must retain requests already in flight. */
+ if (vss->runtime_active && vss->prepared && !vss->stopped &&
+ !vss->xfer_xrun) {
+ struct snd_pcm_runtime *runtime = vss->substream->runtime;
+ size_t frame_bytes = runtime ?
+ runtime->frame_bits / gcd(runtime->frame_bits, 8) : 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);
+ }
+ } else if (!vss->msg_count) {
+ wake_up_all(&vss->msg_empty);
+ }
+ if (got_audio && !vss->xfer_xrun)
+ virtsnd_pcm_schedule_period_elapsed(vss);
+}
+
/**
* virtsnd_pcm_msg_pending_num() - Returns the number of pending I/O messages.
* @vss: VirtIO substream.
@@ -529,9 +869,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.
@@ -540,19 +880,52 @@
size_t written_bytes)
{
struct virtio_pcm_substream *vss = msg->substream;
-
- /*
- * hw_ptr always indicates the buffer position of the first I/O message
- * in the virtqueue. Therefore, on each completion of an I/O message,
- * the hw_ptr value is unconditionally advanced.
- */
+ size_t completed_bytes;
+ u32 latency_bytes;
+
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
+ vss->capture_staging) {
+ virtsnd_pcm_capture_complete(msg, written_bytes);
+ return;
+ }
+
+ /* Only completed TX requests and valid RX audio advance the pointer. */
guard(spinlock)(&vss->lock);
+ /* Empty STOP returns retire ownership without inventing captured data. */
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
+ (!vss->prepared ||
+ (vss->stopped && written_bytes <= sizeof(*msg->status)))) {
+ msg->length = 0;
+ vss->msg_count--;
+ if (!vss->msg_count)
+ wake_up_all(&vss->msg_empty);
+ return;
+ }
+ completed_bytes = msg->length;
+ latency_bytes = le32_to_cpu(msg->status->latency_bytes);
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE) {
+ struct snd_pcm_runtime *runtime = vss->substream->runtime;
+ size_t frame_bytes = runtime->frame_bits / gcd(runtime->frame_bits, 8);
+
+ if (virtsnd_pcm_capture_status(msg, written_bytes, frame_bytes,
+ &completed_bytes, &latency_bytes) ||
+ !completed_bytes) {
+ vss->xfer_xrun = true;
+ msg->length = 0;
+ vss->msg_count--;
+ if (!vss->msg_count)
+ wake_up_all(&vss->msg_empty);
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ return;
+ }
+ }
#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 ||
@@ -568,26 +941,16 @@
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. 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)) {
- vss->hw_ptr += msg->length;
- } else {
- size_t data_bytes = min_t(size_t,
- written_bytes - sizeof(msg->status),
- msg->length);
-
- vss->hw_ptr += data_bytes;
+ vss->hw_ptr += completed_bytes;
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE) {
+ /* A full unseen ring cannot be represented by the modulo pointer. */
+ if (check_add_overflow(vss->capture_copied_total,
+ (u64)completed_bytes,
+ &vss->capture_copied_total) ||
+ vss->capture_copied_total < vss->capture_pointer_seen_total ||
+ vss->capture_copied_total - vss->capture_pointer_seen_total >=
+ vss->buffer_bytes)
+ vss->xfer_xrun = true;
}
if (vss->hw_ptr >= vss->buffer_bytes)
@@ -595,19 +958,18 @@
msg->length = 0;
- vss->xfer_xrun = false;
+ /* Keep latched errors until prepare establishes a fresh epoch. */
vss->msg_count--;
- if (vss->xfer_enabled) {
+ if (vss->xfer_enabled ||
+ (vss->direction == SNDRV_PCM_STREAM_CAPTURE && vss->xfer_xrun)) {
struct snd_pcm_runtime *runtime = vss->substream->runtime;
- runtime->delay =
- bytes_to_frames(runtime,
- le32_to_cpu(msg->status.latency_bytes));
- schedule_work(&vss->elapsed_period);
- } else if (!vss->msg_count) {
+ runtime->delay = bytes_to_frames(runtime, latency_bytes);
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ }
+ if (!vss->msg_count)
wake_up_all(&vss->msg_empty);
- }
}
/**
@@ -664,6 +1026,7 @@
* virtsnd_pcm_ctl_msg_alloc() - Allocate and initialize the PCM device control
* message for the specified substream.
* @vss: VirtIO PCM substream.
+ * @epoch: Operation generation captured before allocation.
* @command: Control request code (VIRTIO_SND_R_PCM_XXX).
* @gfp: Kernel flags for memory allocation.
*
@@ -672,7 +1035,7 @@
*/
struct virtio_snd_msg *
virtsnd_pcm_ctl_msg_alloc(struct virtio_pcm_substream *vss,
- unsigned int command, gfp_t gfp)
+ unsigned int command, gfp_t gfp, u64 epoch)
{
size_t request_size = sizeof(struct virtio_snd_pcm_hdr);
size_t response_size = sizeof(struct virtio_snd_hdr);
@@ -690,6 +1053,7 @@
hdr->hdr.code = cpu_to_le32(command);
hdr->stream_id = cpu_to_le32(vss->sid);
+ virtsnd_ctl_msg_bind_pcm(msg, vss, epoch);
}
return msg;
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,43 @@
* 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"
+
+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);
+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 +131,242 @@
};
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 = vss->release_required || vss->msg_count;
+ notify = vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
+ (!vss->capture_staging || vss->runtime_active);
+ 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 = virtsnd_ctl_msg_pcm_epoch(msg);
+ bool uncertain;
+ int rc;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting || vss->reset_epoch != epoch ||
+ vss->control_uncertain) {
+ rc = vss->control_uncertain ? -EIO : -ECANCELED;
+ 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 (epoch != vss->reset_epoch || vss->resetting)
+ return -ECANCELED;
+ if (rc && uncertain)
+ vss->control_uncertain = true;
+ }
+ 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 = virtsnd_ctl_msg_pcm_epoch(msg);
+ int rc;
+
+ if (!virtsnd_pcm_pause_restart_enabled(vss))
+ return virtsnd_pcm_send_sync(vss, msg);
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ state = vss->pause_restart_state;
+ if (vss->resetting || vss->reset_epoch != epoch) {
+ 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, u64 epoch)
+{
+ struct virtio_snd_msg *msg;
+
+ msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_PREPARE,
+ GFP_KERNEL, epoch);
+ 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, u64 epoch)
+{
+ struct virtio_snd_msg *msg;
+
+ msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_PREPARE,
+ GFP_KERNEL, epoch);
+ 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, u64 epoch)
+{
+ struct virtio_snd_msg *msg;
+ int rc;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ }
+ msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_RELEASE,
+ GFP_KERNEL, epoch);
+ 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 +381,42 @@
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;
+ }
+ }
+
+ /* 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);
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)
+ enable_work(&vss->elapsed_period);
+ return rc;
}
/**
@@ -147,6 +428,15 @@
*/
static int virtsnd_pcm_close(struct snd_pcm_substream *substream)
{
+ struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ vss->runtime_active = false;
+ vss->xfer_enabled = false;
+ }
+ /* Block late producers as well as drain already queued notifications. */
+ disable_work_sync(&vss->elapsed_period);
+
return 0;
}
@@ -159,6 +449,7 @@
* @channels: Selected number of channels.
* @format: Selected sample format (SNDRV_PCM_FORMAT_XXX).
* @rate: Selected frame rate.
+ * @epoch: Operation generation captured before preparing the request.
*
* Context: Any context that permits to sleep.
* Return: 0 on success, -errno on failure.
@@ -168,7 +459,7 @@
unsigned int period_bytes,
unsigned int channels,
snd_pcm_format_t format,
- unsigned int rate)
+ unsigned int rate, u64 epoch)
{
struct virtio_snd_msg *msg;
struct virtio_snd_pcm_set_params *request;
@@ -194,7 +485,7 @@
return -EINVAL;
msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_SET_PARAMS,
- GFP_KERNEL);
+ GFP_KERNEL, epoch);
if (!msg)
return -ENOMEM;
@@ -213,7 +504,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);
}
/**
@@ -232,8 +530,10 @@
unsigned int msg_count;
size_t period_buffer_bytes;
bool stopped;
+ u64 epoch;
int rc;
+ guard(mutex)(&vss->lifecycle_lock);
/* Fixed descriptors require whole-byte periods covering the buffer. */
if (check_mul_overflow((size_t)params_periods(hw_params),
(size_t)params_period_bytes(hw_params),
@@ -243,6 +543,25 @@
(u64)snd_pcm_format_physical_width(params_format(hw_params)) *
params_channels(hw_params)) % 8)
return -EINVAL;
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ epoch = vss->reset_epoch;
+ /* 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) {
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ 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;
@@ -260,11 +579,16 @@
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),
params_channels(hw_params),
params_format(hw_params),
- params_rate(hw_params));
+ params_rate(hw_params), epoch);
if (rc)
return rc;
@@ -274,8 +598,18 @@
*/
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) {
+ guard(spinlock_irqsave)(&vss->lock);
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ vss->suspended = false;
+ if (vss->capture_staging)
+ vss->runtime_active = true;
+ }
+
+ return rc;
}
/**
@@ -288,28 +622,95 @@
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);
+ cancel_work_sync(&vss->elapsed_period);
+
+ 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;
- int rc;
+ bool suspended;
+ u64 epoch;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting)
+ return -ECANCELED;
+ epoch = vss->reset_epoch;
+ stopped = vss->stopped || vss->release_required || vss->msg_count;
+ }
+ if (stopped) {
+ rc = virtsnd_pcm_sync_stop_locked(substream);
+ if (rc)
+ return rc;
+ }
+ scoped_guard(pcm_stream_lock_irqsave, substream) {
+ guard(spinlock_irqsave)(&vss->lock);
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ vss->prepared = false;
+ vss->preparing = true;
+ 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;
@@ -320,48 +721,386 @@
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)
+ runtime->format, runtime->rate, epoch);
+ 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);
+ }
+ }
+
+ rc = virtsnd_pcm_dev_prepare(vss, epoch);
+ if (rc) {
+ if (pause_restart && rc != -ECANCELED)
+ virtsnd_pcm_pause_restart_fail(vss, false, epoch);
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->reset_epoch == epoch && !vss->resetting) {
+ vss->xfer_xrun = true;
+ vss->stopped = true;
+ }
+ }
+ return rc;
+ }
+
+ scoped_guard(pcm_stream_lock_irqsave, substream) {
+ guard(spinlock_irqsave)(&vss->lock);
+ if (epoch != vss->reset_epoch || vss->resetting)
+ return -ECANCELED;
+ vss->release_required = true;
+ vss->buffer_bytes = snd_pcm_lib_buffer_bytes(substream);
+ vss->hw_ptr = 0;
+ 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) {
+ vss->xfer_xrun = true;
+ vss->stopped = true;
+ return rc;
+ }
+ }
+ vss->prepared = true;
+ 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);
+
+ int rc;
+
+ guard(mutex)(&vss->lifecycle_lock);
+ rc = virtsnd_pcm_prepare_locked(substream);
+ if (rc) {
+ guard(pcm_stream_lock_irqsave)(substream);
+ guard(spinlock_irqsave)(&vss->lock);
+ vss->preparing = false;
+ } else if (vss->period_work_quiesced) {
+ enable_work(&vss->elapsed_period);
+ vss->period_work_quiesced = false;
+ }
+ return rc;
+}
+
+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;
+ bool xrun;
+ 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;
+ xrun = vss->xfer_xrun;
+ reset_required =
+ state == VIRTSND_PCM_PAUSE_RESTART_RESET_REQUIRED ||
+ virtsnd_pcm_pause_restart_transitioning(state);
+ invalid_state =
+ (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 (xrun)
+ return -EPIPE;
+ if (invalid_state) {
+ virtsnd_pcm_pause_restart_fail(vss, false, epoch);
+ return -EBADFD;
+ }
+
+ if (resume) {
+ rc = virtsnd_pcm_pause_prepare(vss, epoch);
+ 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->xfer_xrun) {
+ rc = -EPIPE;
+ } else if (vss->pause_restart_state !=
+ VIRTSND_PCM_PAUSE_RESTART_PREPARED ||
+ vss->msg_count || !vss->runtime_active) {
+ 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, epoch);
+ if (!msg) {
+ virtsnd_pcm_pause_restart_failed(vss, epoch);
+ return -ENOMEM;
+ }
+
+ 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, epoch);
+ 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), epoch);
+ 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, epoch);
+ 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;
+}
+
+static int virtsnd_pcm_stop_device(struct virtio_pcm_substream *vss, u64 epoch)
+{
+ struct virtio_snd_msg *msg;
+ int rc;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ if (vss->control_uncertain)
+ return -EIO;
+ if (!vss->device_running)
+ return 0;
+ }
+ msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_STOP,
+ GFP_KERNEL, epoch);
if (!msg)
return -ENOMEM;
-
- rc = virtsnd_ctl_msg_send_sync(vss->snd, msg);
- if (!rc) {
- guard(spinlock_irqsave)(&vss->lock);
- vss->release_required = true;
- }
+ rc = virtsnd_pcm_send_sync(vss, msg);
+ if (rc)
+ return rc;
+ guard(spinlock_irqsave)(&vss->lock);
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ vss->device_running = false;
+ return 0;
+}
+
+static int virtsnd_pcm_start_queue(struct virtio_pcm_substream *vss, u64 epoch)
+{
+ struct virtio_snd_queue *queue = virtsnd_pcm_queue(vss);
+ int rc = 0;
+
+ guard(spinlock_irqsave)(&queue->lock);
+ guard(spinlock)(&vss->lock);
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ if (vss->stopped || !vss->prepared || vss->xfer_xrun)
+ return vss->xfer_xrun ? -EPIPE : -EBADFD;
+ if (vss->control_uncertain)
+ return -EIO;
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE) {
+ if (vss->capture_staging) {
+ rc = vss->runtime_active ? virtsnd_pcm_capture_send(vss) : -EBADFD;
+ } else if (!vss->capture_rx_seeded) {
+ rc = virtsnd_pcm_msg_send(vss, 0, vss->buffer_bytes);
+ if (!rc)
+ vss->capture_rx_seeded = true;
+ }
+ }
+ if (!rc)
+ vss->xfer_enabled = true;
return rc;
}
@@ -377,72 +1116,236 @@
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;
+ bool running;
+ 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:
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
- queue = virtsnd_pcm_queue(vss);
-
- 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 &&
- !vss->capture_rx_seeded) {
- rc = virtsnd_pcm_msg_send(vss, 0, vss->buffer_bytes);
- if (!rc)
- vss->capture_rx_seeded = true;
- }
- if (rc) {
- vss->stopped = true;
- substream->runtime->stop_operating = true;
- return rc;
- }
- vss->xfer_enabled = true;
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ running = vss->device_running;
+ /* Preserve queued requests and unread RX data across native pause. */
+ rc = virtsnd_pcm_start_queue(vss, epoch);
+ if (rc) {
+ virtsnd_pcm_start_failed(vss, epoch);
+ return rc;
+ }
+
+ if (running) {
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE)
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ return 0;
}
msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_START,
- GFP_KERNEL);
+ GFP_KERNEL, epoch);
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) {
+ guard(spinlock_irqsave)(&vss->lock);
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
+ vss->device_running = true;
+ }
+ if (!rc && vss->direction == SNDRV_PCM_STREAM_CAPTURE)
+ 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;
- }
-
- 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);
+ if (!vss->device_running)
+ return 0;
+ }
+
+ rc = virtsnd_pcm_stop_device(vss, epoch);
+ if (rc && command != SNDRV_PCM_TRIGGER_PAUSE_PUSH) {
+ guard(spinlock_irqsave)(&vss->lock);
+
+ if (vss->reset_epoch == epoch && !vss->resetting) {
+ vss->xfer_xrun = true;
+ substream->runtime->stop_operating = true;
+ if (vss->direction == SNDRV_PCM_STREAM_CAPTURE)
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ }
+ }
+ if (rc && command == SNDRV_PCM_TRIGGER_PAUSE_PUSH) {
+ if (virtsnd_pcm_start_queue(vss, epoch)) {
+ guard(spinlock_irqsave)(&vss->lock);
+ if (vss->reset_epoch == epoch && !vss->resetting) {
+ vss->xfer_xrun = true;
+ vss->stopped = true;
+ substream->runtime->stop_operating = true;
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ }
+ } else if (vss->direction == SNDRV_PCM_STREAM_CAPTURE) {
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ }
+ }
+ return rc;
default:
return -EINVAL;
}
+
+}
+
+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, epoch);
+ 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), epoch);
+ 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
@@ -451,7 +1354,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;
@@ -459,31 +1362,54 @@
unsigned int js = msecs_to_jiffies(virtsnd_msg_timeout_ms);
bool need_release;
int rc;
-
- scoped_guard(spinlock_irqsave, &vss->lock) {
+ u64 epoch;
+
+ lockdep_assert_held(&vss->lifecycle_lock);
+ /* Keep late RX completions from requeuing work after cancellation. */
+ if (!vss->period_work_quiesced) {
+ disable_work_sync(&vss->elapsed_period);
+ vss->period_work_quiesced = true;
+ }
+ 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->release_required;
if (need_release)
vss->stopped = true;
}
- cancel_work_sync(&vss->elapsed_period);
+ /* A rejected STOP leaves the device running and must precede RELEASE. */
+ rc = virtsnd_pcm_stop_device(vss, epoch);
+ if (rc) {
+ substream->runtime->stop_operating = true;
+ return rc;
+ }
if (need_release) {
msg = virtsnd_pcm_ctl_msg_alloc(vss, VIRTIO_SND_R_PCM_RELEASE,
- GFP_KERNEL);
+ GFP_KERNEL, epoch);
if (!msg) {
substream->runtime->stop_operating = true;
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;
}
- scoped_guard(spinlock_irqsave, &vss->lock)
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->resetting || vss->reset_epoch != epoch)
+ return -ECANCELED;
vss->release_required = false;
+ vss->device_running = false;
+ vss->prepared = false;
+ }
}
/*
@@ -492,7 +1418,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) {
@@ -503,8 +1429,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;
}
@@ -521,10 +1451,16 @@
virtsnd_pcm_pb_pointer(struct snd_pcm_substream *substream)
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+ size_t hw_ptr;
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (!vss->prepared || vss->preparing || vss->xfer_xrun)
+ return SNDRV_PCM_POS_XRUN;
+ hw_ptr = vss->hw_ptr;
+ }
return snd_pcm_indirect_playback_pointer(substream,
- &vss->pcm_indirect,
- vss->hw_ptr);
+ &vss->pcm_indirect, hw_ptr);
}
/**
@@ -539,10 +1475,69 @@
virtsnd_pcm_cp_pointer(struct snd_pcm_substream *substream)
{
struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ size_t hw_ptr = 0;
+ u64 total;
+ bool free_running;
+ bool xrun;
+
+ if (vss->capture_staging) {
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (!vss->runtime_active || vss->preparing || vss->suspended ||
+ (!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 SNDRV_PCM_POS_XRUN;
+ 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;
+ if (vss->capture_reset_observing) {
+ size_t period = snd_pcm_lib_period_bytes(substream);
+
+ vss->capture_acked_total = total;
+ vss->capture_ready = 0;
+ vss->capture_notified_periods =
+ period ? div64_ul(total, period) : 0;
+ }
+ 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);
+ }
+
+ scoped_guard(spinlock_irqsave, &vss->lock) {
+ if (vss->xfer_xrun || !vss->prepared || vss->preparing)
+ return SNDRV_PCM_POS_XRUN;
+ vss->capture_pointer_seen_total = vss->capture_copied_total;
+ hw_ptr = vss->hw_ptr;
+ }
return snd_pcm_indirect_capture_pointer(substream,
- &vss->pcm_indirect,
- vss->hw_ptr);
+ &vss->pcm_indirect, hw_ptr);
}
struct virtsnd_pcm_transfer {
@@ -557,7 +1552,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);
@@ -569,6 +1566,47 @@
vss->xfer_enabled = false;
vss->stopped = true;
vss->substream->runtime->stop_operating = true;
+}
+
+static int virtsnd_pcm_pb_ack_locked(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;
+
+ scoped_guard(spinlock_irqsave, &queue->lock) {
+ guard(spinlock)(&vss->lock);
+ if (vss->preparing)
+ return -EBADFD;
+ if (vss->xfer_xrun)
+ return -EPIPE;
+ if (!vss->prepared)
+ return -EBADFD;
+ 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 0;
}
/* The caller holds the PCM lock, also used by DROP and state transitions. */
@@ -587,26 +1625,152 @@
}
}
+/* The PCM lock serializes ACK with prepare's accounting reset. */
static int virtsnd_pcm_pb_ack(struct snd_pcm_substream *substream)
+{
+ struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+ int rc;
+
+ if (vss->indirect_reset_observing)
+ return 0;
+ rc = virtsnd_pcm_ack_state(substream);
+ if (rc)
+ return rc;
+ return virtsnd_pcm_pb_ack_locked(substream);
+}
+
+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 || vss->preparing || !runtime || !runtime->control ||
+ !runtime->dma_area || vss->suspended)
+ return -EBADFD;
+ if (vss->xfer_xrun)
+ return -EPIPE;
+ 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->stopped && 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 -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;
+ /* Keep an unconsumed sub-byte frame for the next acknowledgment. */
+ vss->pcm_indirect.appl_ptr += bytes_to_frames(runtime, bytes);
+ if (vss->pcm_indirect.appl_ptr >= runtime->boundary)
+ vss->pcm_indirect.appl_ptr -= runtime->boundary;
+ }
+
+ /* 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_locked(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;
- rc = virtsnd_pcm_ack_state(substream);
- if (rc)
- return 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->preparing)
+ return -EBADFD;
+ if (vss->xfer_xrun)
+ return -EPIPE;
+ if (!vss->prepared)
+ return -EBADFD;
transfer.indirect = vss->pcm_indirect;
- if (vss->stopped) {
+ if (vss->xfer_xrun ||
+ (vss->stopped && runtime->state != SNDRV_PCM_STATE_DRAINING)) {
rc = -EPIPE;
} else {
- rc = snd_pcm_indirect_playback_transfer(substream,
- &transfer.indirect,
- virtsnd_pcm_trans_copy);
+ rc = snd_pcm_indirect_capture_transfer(substream,
+ &transfer.indirect,
+ virtsnd_pcm_trans_copy);
}
if (!rc)
rc = transfer.error;
@@ -617,51 +1781,254 @@
}
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 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;
-
- rc = virtsnd_pcm_ack_state(substream);
- if (rc)
- return rc;
-
- 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 0;
+}
+
+/* Every linked member's prepare closes the ACK gate under its PCM lock. */
+static int virtsnd_pcm_cp_ack(struct snd_pcm_substream *substream)
+{
+ struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
+ int rc;
+
+ /* RESET observes the pointer before returning discarded RX space. */
+ if (vss->indirect_reset_observing)
+ return 0;
+ rc = virtsnd_pcm_ack_state(substream);
+ if (rc)
+ return rc;
+ return virtsnd_pcm_cp_ack_locked(substream);
+}
+
+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(pcm_stream_lock_irqsave, substream) {
+ snd_pcm_state_t state = substream->runtime->state;
+
+ /*
+ * Trigger holds the PCM lock through its control requests. Wait
+ * for it, then leave a replacement active generation alone.
+ * lifecycle_lock keeps prepare from replacing an inactive one
+ * until sync_stop_locked has finished its teardown.
+ */
+ if (state == SNDRV_PCM_STATE_RUNNING ||
+ state == SNDRV_PCM_STATE_PAUSED ||
+ state == SNDRV_PCM_STATE_PREPARED ||
+ (state == SNDRV_PCM_STATE_DRAINING &&
+ substream->stream == SNDRV_PCM_STREAM_PLAYBACK))
+ return 0;
+ guard(spinlock_irqsave)(&vss->lock);
+ if (!vss->stopped)
+ return 0;
+ }
+ return virtsnd_pcm_sync_stop_locked(substream);
+}
+
+static void virtsnd_pcm_capture_reset_error(struct virtio_pcm_substream *vss,
+ struct snd_pcm_runtime *runtime)
+{
+ size_t position = frames_to_bytes(runtime, runtime->status->hw_ptr);
+ size_t period = snd_pcm_lib_period_bytes(vss->substream);
+
+ vss->preparing = false;
+ vss->pcm_indirect.appl_ptr = runtime->status->hw_ptr;
+ vss->pcm_indirect.sw_ready = 0;
+ if (vss->capture_staging) {
+ vss->hw_ptr = position;
+ 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_active = false;
+ }
+ if (vss->xfer_xrun) {
+ virtsnd_pcm_transfer_failed(vss);
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ }
+}
+
+/*
+ * The PCM lock covers indirect accounting and core completion. Once lib_reset
+ * changes hw_ptr, complete the core rebase even if refilling fails. The latched
+ * XRUN rejects subsequent pointer/ACK operations; the worker publishes XRUN to
+ * paused streams too. RESET success therefore does not imply stream recovery.
+ */
+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 = {};
+ snd_pcm_uframes_t paused_ptr = 0;
+ bool paused = runtime->state == SNDRV_PCM_STATE_PAUSED;
+ int rc;
+
+ /* Only PCM-locked callbacks access this observation flag. */
+ vss->indirect_reset_observing = true;
+ rc = snd_pcm_lib_reset(substream);
+ vss->indirect_reset_observing = false;
+ if (rc)
+ return rc;
+ guard(spinlock_irqsave)(&queue->lock);
+ guard(spinlock)(&vss->lock);
+ vss->preparing = false;
+ if (vss->resetting || vss->xfer_xrun) {
+ virtsnd_pcm_capture_reset_error(vss, runtime);
+ return 0;
+ }
+ if (!snd_pcm_running(substream) && !paused) {
+ vss->pcm_indirect.appl_ptr = runtime->status->hw_ptr;
+ vss->pcm_indirect.sw_ready = 0;
+ return 0;
+ }
+ if (!vss->prepared || vss->stopped) {
+ virtsnd_pcm_transfer_failed(vss);
+ vss->xfer_xrun = true;
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ return 0;
+ }
+
+ transfer.indirect = vss->pcm_indirect;
+ if (paused) {
+ /* Observe and reclaim paused completions in one queue transaction. */
+ vss->capture_pointer_seen_total = vss->capture_copied_total;
+ vss->indirect_reset_observing = true;
+ paused_ptr = snd_pcm_indirect_capture_pointer(substream,
+ &transfer.indirect, vss->hw_ptr);
+ vss->indirect_reset_observing = false;
+ if (paused_ptr == SNDRV_PCM_POS_XRUN) {
+ virtsnd_pcm_transfer_failed(vss);
+ vss->xfer_xrun = true;
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ return 0;
+ }
+ runtime->status->hw_ptr = paused_ptr;
+ }
+ 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);
+ vss->xfer_xrun = true;
+ virtsnd_pcm_schedule_period_elapsed(vss);
+ return 0;
+ }
+
+ /* 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;
+ {
+ size_t period = snd_pcm_lib_period_bytes(substream);
+
+ vss->capture_notified_periods = period ?
+ div64_ul(vss->capture_pointer_seen_total, period) : 0;
+ }
+ return 0;
+}
+
+/* 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;
+
+ /* Reclaim RX space atomically with the pointer observation. */
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ vss->capture_reset_observing = true;
+ rc = snd_pcm_lib_reset(substream);
+ scoped_guard(spinlock_irqsave, &vss->lock)
+ vss->capture_reset_observing = false;
+ if (rc || !vss->capture_staging)
+ return rc;
+
+ guard(spinlock_irqsave)(&queue->lock);
+ guard(spinlock)(&vss->lock);
+ vss->preparing = false;
+ if (vss->resetting || vss->xfer_xrun) {
+ virtsnd_pcm_capture_reset_error(vss, runtime);
+ 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;
+}
+
+/* RESET can discard partial fills only after the transport returns ownership. */
+static int virtsnd_pcm_playback_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;
+ int rc;
+
+ /* The PCM lock excludes new ACK submissions through the entire reset. */
+ scoped_guard(spinlock_irqsave, &queue->lock) {
+ guard(spinlock)(&vss->lock);
+ if (vss->msg_count)
+ return -EBUSY;
+ }
+ vss->indirect_reset_observing = true;
+ rc = snd_pcm_lib_reset(substream);
+ vss->indirect_reset_observing = false;
+ if (rc)
+ return rc;
+ guard(spinlock_irqsave)(&queue->lock);
+ guard(spinlock)(&vss->lock);
+ position = frames_to_bytes(runtime, runtime->status->hw_ptr) %
+ vss->buffer_bytes;
+ vss->preparing = false;
+ vss->hw_ptr = position;
+ vss->pcm_indirect.hw_io = position;
+ vss->pcm_indirect.hw_data = position;
+ vss->pcm_indirect.sw_io = position;
+ vss->pcm_indirect.sw_data = position;
+ vss->pcm_indirect.hw_ready = 0;
+ vss->pcm_indirect.sw_ready = 0;
+ vss->pcm_indirect.appl_ptr = runtime->status->hw_ptr;
+ virtsnd_pcm_msg_reset(vss);
return 0;
}
@@ -678,6 +2045,7 @@
.sync_stop = virtsnd_pcm_sync_stop,
.pointer = virtsnd_pcm_pb_pointer,
.ack = virtsnd_pcm_pb_ack,
+ .reset = virtsnd_pcm_playback_reset,
},
{
.open = virtsnd_pcm_open,
@@ -692,3 +2060,32 @@
.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,
+};
+
+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] 25+ messages in thread
* [PATCH v13 11/12] ALSA: virtio: prioritize staged capture period work
2026-11-03 8:08 [PATCH v13 00/12] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (9 preceding siblings ...)
2026-11-03 8:08 ` [PATCH v13 10/12] ALSA: virtio: add staged capture and serialize PCM lifecycle accounting Joonho Lee
@ 2026-11-03 8:08 ` Joonho Lee
2026-11-03 11:09 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 12/12] docs: describe virtio-sound capture options Joonho Lee
11 siblings, 1 reply; 25+ messages in thread
From: Joonho Lee @ 2026-11-03 8:08 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>
---
sound/virtio/virtio_pcm.h | 4 ++++
1 file changed, 4 insertions(+)
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
@@ -169,6 +169,10 @@
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] 25+ messages in thread
* [PATCH v13 12/12] docs: describe virtio-sound capture options
2026-11-03 8:08 [PATCH v13 00/12] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
` (10 preceding siblings ...)
2026-11-03 8:08 ` [PATCH v13 11/12] ALSA: virtio: prioritize staged capture period work Joonho Lee
@ 2026-11-03 8:08 ` Joonho Lee
2026-11-03 11:27 ` sashiko-bot
11 siblings, 1 reply; 25+ messages in thread
From: Joonho Lee @ 2026-11-03 8:08 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.
Keep the virtio_snd entries after virtio_mmio in alphabetical order.
Include a diffstat in every mail patch.
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
Documentation/admin-guide/kernel-parameters.txt | 30 +++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -8344,6 +8344,36 @@
Can be used multiple times for multiple devices.
+ 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.
+
vga= [BOOT,X86-32] Select a particular video mode
See Documentation/arch/x86/boot.rst and
Documentation/admin-guide/svga.rst.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v13 01/12] ALSA: virtio: Propagate PCM send failures through ACK
2026-11-03 8:08 ` [PATCH v13 01/12] ALSA: virtio: Propagate PCM send failures through ACK Joonho Lee
@ 2026-11-03 8:33 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-11-03 8:33 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: race with sync_stop in virtsnd_pcm_pb_ack()
--
commit 75f714a08c45af9f82b5baffff5589166c1182bf
Author: Joonho Lee <mynameisjune111@gmail.com>
ALSA: virtio: Propagate PCM send failures through ACK
This commit restores message length and notifies the virtqueue on partial
send failures in virtsnd_pcm_msg_send(), and propagates those errors through
the PCM ACK callbacks as XRUNs while rejecting ACK calls from invalid PCM
states.
> diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
> index 1091100a4c982..67460bd2a61ca 100644
> --- a/sound/virtio/virtio_pcm_ops.c
> +++ b/sound/virtio/virtio_pcm_ops.c
> @@ -466,36 +466,124 @@ virtsnd_pcm_cp_pointer(struct snd_pcm_substream *substream)
[ ... ]
> +/* 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;
> +}
>
> - virtsnd_pcm_msg_send(vss, rec->sw_data, bytes);
> +/* The caller holds the PCM lock, also used by DROP and state transitions. */
> +static int virtsnd_pcm_ack_state(struct snd_pcm_substream *substream)
> +{
> + switch (substream->runtime->state) {
> + case SNDRV_PCM_STATE_XRUN:
> + return -EPIPE;
> + case SNDRV_PCM_STATE_PREPARED:
> + case SNDRV_PCM_STATE_RUNNING:
> + case SNDRV_PCM_STATE_PAUSED:
> + case SNDRV_PCM_STATE_DRAINING:
> + return 0;
> + default:
> + return -EBADFD;
> + }
> }
>
> 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);
> + rc = virtsnd_pcm_ack_state(substream);
> + if (rc)
> + return rc;
>
> - return snd_pcm_indirect_playback_transfer(substream, &vss->pcm_indirect,
> - virtsnd_pcm_trans_copy);
> + 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);
[Severity: Medium]
As reported by Sashiko on v12 [1], can virtsnd_pcm_pb_ack() and
virtsnd_pcm_cp_ack() still race with snd_pcm_sync_stop() ->
virtsnd_pcm_sync_stop() on vss->stopped and substream->runtime->stop_operating
when a stream is in SNDRV_PCM_STATE_PREPARED or SNDRV_PCM_STATE_DRAINING?
Because virtio-snd sets vpcm->pcm->nonatomic = true, snd_pcm_prepare() and
snd_pcm_hw_params() hold runtime->buffer_mutex rather than the PCM stream
lock across snd_pcm_sync_stop() and substream->ops->prepare() /
substream->ops->hw_params(). Meanwhile, a concurrent SNDRV_PCM_IOCTL_SYNC_PTR
in snd_pcm_sync_ptr() only acquires the PCM stream lock and calls
pcm_lib_apply_appl_ptr() -> virtsnd_pcm_pb_ack() / virtsnd_pcm_cp_ack().
In virtsnd_pcm_ack_state(), SNDRV_PCM_STATE_SETUP is rejected, which covers
snd_pcm_drop(), but SNDRV_PCM_STATE_PREPARED and SNDRV_PCM_STATE_DRAINING are
allowed:
1. When a capture stream with available frames is drained via
snd_pcm_drain(), snd_pcm_do_drain_init() stops the stream and places it in
SNDRV_PCM_STATE_DRAINING with vss->stopped = true and
runtime->stop_operating = true. Similarly, if snd_pcm_start() fails on a
later linked stream, snd_pcm_undo_start() stops the earlier stream while
leaving it in SNDRV_PCM_STATE_PREPARED with vss->stopped = true and
runtime->stop_operating = true. On a draining capture stream, any read or
SNDRV_PCM_IOCTL_SYNC_PTR will also see vss->stopped == true in
virtsnd_pcm_cp_ack() and fail with -EPIPE.
2. When snd_pcm_prepare() or snd_pcm_hw_params() is called from
SNDRV_PCM_STATE_PREPARED (or snd_pcm_prepare() from
SNDRV_PCM_STATE_DRAINING on capture), runtime->state is unchanged while
snd_pcm_sync_stop() clears runtime->stop_operating = false and calls
virtsnd_pcm_sync_stop():
sound/virtio/virtio_pcm_ops.c:virtsnd_pcm_sync_stop() {
if (!vss->stopped)
return 0;
...
vss->stopped = false;
}
While virtsnd_pcm_sync_stop() is sleeping in cancel_work_sync(),
VIRTIO_SND_R_PCM_RELEASE, or wait_event_interruptible_timeout(), a
concurrent SNDRV_PCM_IOCTL_SYNC_PTR passes virtsnd_pcm_ack_state(), sees
vss->stopped == true, calls virtsnd_pcm_transfer_failed(vss) to re-arm
vss->stopped = true and runtime->stop_operating = true, and returns -EPIPE
so pcm_lib_apply_appl_ptr() triggers __snd_pcm_xrun(). Conversely, if
SNDRV_PCM_IOCTL_SYNC_PTR runs right after virtsnd_pcm_sync_stop() clears
vss->stopped = false and before prepare finishes, ACK sees
vss->stopped == false and enqueues I/O messages on the released stream.
3. Later commits in this series ("ALSA: virtio: Recover PCM state after a
failed start" and "ALSA: virtio: add staged capture and serialize PCM
lifecycle accounting") add vss->lock around vss->stopped and gate ACK on
vss->prepared and vss->preparing, but virtsnd_pcm_prepare_locked() and
virtsnd_pcm_hw_params() still call virtsnd_pcm_sync_stop_locked() before
setting vss->preparing = true or clearing vss->prepared = false. While
virtsnd_pcm_sync_stop_locked() or virtsnd_pcm_pause_restart_sync_stop()
sleeps in disable_work_sync(), cancel_work_sync(),
virtsnd_pcm_stop_device(), or virtsnd_pcm_send_sync() for
VIRTIO_SND_R_PCM_RELEASE, runtime->state remains SNDRV_PCM_STATE_PREPARED,
vss->preparing is false, and vss->prepared remains true. A concurrent
SNDRV_PCM_IOCTL_SYNC_PTR can still enqueue messages during
disable_work_sync() or observe vss->stopped == true during STOP/RELEASE,
re-arm runtime->stop_operating = true via virtsnd_pcm_transfer_failed(),
and return -EPIPE to trigger __snd_pcm_xrun().
[1] https://lore.kernel.org/r/20261103000403.67106-2-mynameisjune111@gmail.com
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103080821.16983-1-mynameisjune111@gmail.com?part=1
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v13 02/12] ALSA: virtio: Recover PCM state after a failed start
2026-11-03 8:08 ` [PATCH v13 02/12] ALSA: virtio: Recover PCM state after a failed start Joonho Lee
@ 2026-11-03 8:55 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-11-03 8:55 UTC (permalink / raw)
To: Joonho Lee; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> 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.
>
> Record the RELEASE obligation after successful PREPARE and clear it as
> soon as RELEASE succeeds. If the subsequent RX wait fails, retry that
> wait without submitting RELEASE again.
>
> Fixes: da76e9f3e43a ("ALSA: virtio: PCM substream operators")
> 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/20261103080821.16983-1-mynameisjune111@gmail.com?part=2
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v13 03/12] ALSA: virtio: Retain queued messages through device reset
2026-11-03 8:08 ` [PATCH v13 03/12] ALSA: virtio: Retain queued messages through device reset Joonho Lee
@ 2026-11-03 9:25 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-11-03 9:25 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: NULL deref in virtsnd_disable_event_vq() on probe failure
--
commit dbe53b6c2e9b3ca5c54f2da85fde0a3fdaa123b3
Author: Joonho Lee <mynameisjune111@gmail.com>
ALSA: virtio: Retain queued messages through device reset
This commit retains the control queue through snd_card_free() during device
teardown, detaches remaining virtqueue buffers after device reset, and
protects virtqueue callbacks under their respective queue locks.
> diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
> index 6f35276416fed..aec4959c973ca 100644
> --- a/sound/virtio/virtio_card.c
> +++ b/sound/virtio/virtio_card.c
[ ... ]
> @@ -174,8 +188,9 @@ static void virtsnd_disable_event_vq(struct virtio_snd *snd)
> struct virtio_snd_event *event;
> u32 length;
>
> + guard(spinlock_irqsave)(&queue->lock);
> + snd->event_vq_enabled = false;
> if (queue->vqueue) {
> - guard(spinlock_irqsave)(&queue->lock);
> virtqueue_disable_cb(queue->vqueue);
> while ((event = virtqueue_get_buf(queue->vqueue, &length)))
> virtsnd_event_dispatch(snd, event);
[Severity: High]
Can virtsnd_disable_event_vq() dereference a NULL pointer when cleaning up
after a failure in virtsnd_probe()?
In virtsnd_probe(), virtsnd_find_vqs() populates the event virtqueue with
buffers while snd->event_vq_enabled is still false, and virtio_device_ready()
marks the device live before virtsnd_build_devs() creates the ALSA objects:
sound/virtio/virtio_card.c:virtsnd_probe() {
...
rc = virtsnd_find_vqs(snd);
if (rc)
goto on_exit;
virtio_device_ready(vdev);
rc = virtsnd_build_devs(snd);
if (rc)
goto on_exit;
virtsnd_enable_event_vq(snd);
on_exit:
if (rc)
virtsnd_remove(vdev);
...
}
Because VIRTIO_CONFIG_S_DRIVER_OK is set before virtsnd_build_devs(), the
device can place events into the event queue while virtsnd_build_devs() is
running. With this change, virtsnd_event_notify_cb() sees
!snd->event_vq_enabled and leaves those events in the ring.
If virtsnd_build_devs() fails before virtsnd_enable_event_vq() is reached
(for example, after virtsnd_kctl_parse_cfg() sets snd->nkctls and allocates
snd->kctls, but before virtsnd_kctl_build_devs() populates each kctl->kctl),
virtsnd_probe() calls virtsnd_remove() -> virtsnd_disable_event_vq().
Since virtsnd_disable_event_vq() unconditionally drains queue->vqueue and
calls virtsnd_event_dispatch() without checking whether snd->event_vq_enabled
was true, dispatching a pending event against partially initialized state
(such as passing &kctl->kctl->id to snd_ctl_notify() in virtsnd_kctl_event()
when kctl->kctl is NULL) will dereference a NULL pointer.
Should virtsnd_disable_event_vq() only drain and dispatch events if
snd->event_vq_enabled was previously true?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103080821.16983-1-mynameisjune111@gmail.com?part=3
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v13 04/12] ALSA: virtio: Keep PCM DMA buffers alive through device ownership
2026-11-03 8:08 ` [PATCH v13 04/12] ALSA: virtio: Keep PCM DMA buffers alive through device ownership Joonho Lee
@ 2026-11-03 9:49 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-11-03 9:49 UTC (permalink / raw)
To: Joonho Lee; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> 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.
>
> Require whole-byte periods whose total matches the negotiated buffer.
> Check offset, length and the final message index before indexing the
> message array, including empty transfers and arithmetic wraparound.
>
> Seed the ordinary capture ring once per prepare. Native STOP preserves
> queued requests and unread samples, so PAUSE_RELEASE must retain their
> ownership. Re-enqueuing the entire ring adds another period to pending
> message lengths and exposes unread DMA slots to the device. Return
> consumed space through the existing indirect ACK path.
>
> 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>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103080821.16983-1-mynameisjune111@gmail.com?part=4
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v13 05/12] ALSA: virtio: Keep control payloads alive through DMA completion
2026-11-03 8:08 ` [PATCH v13 05/12] ALSA: virtio: Keep control payloads alive through DMA completion Joonho Lee
@ 2026-11-03 9:56 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-11-03 9:56 UTC (permalink / raw)
To: Joonho Lee; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> 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.
>
> Keep response values in a separately allocated aligned response buffer
> so INTEGER64 control accesses are aligned from this patch onward.
>
> 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/20261103080821.16983-1-mynameisjune111@gmail.com?part=5
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v13 06/12] ALSA: virtio: wait through signals for PCM teardown
2026-11-03 8:08 ` [PATCH v13 06/12] ALSA: virtio: wait through signals for PCM teardown Joonho Lee
@ 2026-11-03 10:15 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-11-03 10:15 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/20261103080821.16983-1-mynameisjune111@gmail.com?part=6
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v13 07/12] ALSA: pcm: mark stop-threshold accesses for capture policy sampling
2026-11-03 8:08 ` [PATCH v13 07/12] ALSA: pcm: mark stop-threshold accesses for capture policy sampling Joonho Lee
@ 2026-11-03 10:17 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-11-03 10:17 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 patch 9.
>
> 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/20261103080821.16983-1-mynameisjune111@gmail.com?part=7
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v13 08/12] ALSA: pcm: report partial progress under the stream lock
2026-11-03 8:08 ` [PATCH v13 08/12] ALSA: pcm: report partial progress under the stream lock Joonho Lee
@ 2026-11-03 10:20 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-11-03 10:20 UTC (permalink / raw)
To: Joonho Lee; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> Staged capture can make readable data available before a complete period
> has elapsed. Skipping notification leaves a blocked reader asleep, while
> using the period-interrupt path for this event can infer a false ring wrap
> when the notification is delayed.
>
> Add snd_pcm_progress_under_stream_lock() to update the hardware pointer
> through the non-interrupt path and notify readers and fasync listeners.
> It runs under the caller's PCM stream lock and does not tick the PCM timer.
> The driver must release its pointer and trigger locks before calling it.
> Existing period notification callers keep their current behavior.
>
> Atomic and nonatomic synthetic PCM tests reproduced the missing wakeup
> and false-wrap alternatives. The new helper delivered 64 final frames and
> reported four available frames after delayed partial progress without XRUN.
>
> 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/20261103080821.16983-1-mynameisjune111@gmail.com?part=8
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v13 09/12] ALSA: pcm: add an optional reset operation under the stream lock
2026-11-03 8:08 ` [PATCH v13 09/12] ALSA: pcm: add an optional reset operation " Joonho Lee
@ 2026-11-03 10:27 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-11-03 10:27 UTC (permalink / raw)
To: Joonho Lee; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> A driver may need to synchronize private capture accounting with the
> common RESET pointer observation and the core's position rebase.
>
> Add an optional reset operation called under the member's stream lock.
> Expose snd_pcm_lib_reset() so the callback can retain common pointer and
> time handling. On zero return the core completes that member's rebase
> before unlocking it. Drivers without the callback retain the ioctl path.
> The operation is also used after a successful prepare.
>
> Zero permits the position rebase; it does not promise XRUN recovery.
> A negative return skips the core rebase, so a callback returning an error
> must leave coherent pointer state even after partial work.
>
> Linked RESET and prepare stop at the first error without rollback.
> Earlier callback members may already have completed. Legacy members keep
> [ ... ]
> callback retained an acknowledged appl_ptr of 64, and finite-silence
> playback kept the written prefix while clearing the rewound tail.
>
> 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/20261103080821.16983-1-mynameisjune111@gmail.com?part=9
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v13 10/12] ALSA: virtio: add staged capture and serialize PCM lifecycle accounting
2026-11-03 8:08 ` [PATCH v13 10/12] ALSA: virtio: add staged capture and serialize PCM lifecycle accounting Joonho Lee
@ 2026-11-03 11:05 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-11-03 11: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 9 potential issue(s) to consider:
- [Medium] ALSA: virtio: missing stop_operating arming on -EPIPE in ACK and start paths
- [Medium] ALSA: virtio: skipped NOMMU post-STOP copy in virtsnd_pcm_msg_complete()
- [Medium] ALSA: virtio: cancel_work_sync() in virtsnd_reset_vqs() stalls ctl cleanup
- [Medium] ALSA: virtio: missing stop_operating on XRUN in pause_restart trigger
- [Medium] ALSA: virtio: delayed preparing gate and stale release_required in prepare
- [Medium] ALSA: virtio: virtsnd_pcm_pause_restart_stop() skips setting suspended
- [Medium] ALSA: virtio: virtsnd_pcm_indirect_reset() skips cursor rebase on error
- [Medium] ALSA: virtio: virtsnd_pcm_capture_staged_ack() rejects sub-byte carryover
- [Medium] ALSA: virtio: unaligned byte ACK in virtsnd_pcm_capture_staged_ack()
--
commit ca5397134e4f3f33edd4ee72441d7b063f6912f2
Author: Joonho Lee <mynameisjune111@gmail.com>
ALSA: virtio: add staged capture and serialize PCM lifecycle accounting
This commit introduces request-owned staged capture buffers with separate
produced, observed, and consumed position tracking, and serializes PCM
lifecycle transitions and ACK accounting using lifecycle_lock and the PCM
stream lock. It also adds reset callbacks for playback and capture streams
and refines XRUN and teardown handling across device resets and pause
transitions.
> diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
> index aec4959c973ca..f32e9fb9e8b1d 100644
> --- a/sound/virtio/virtio_card.c
> +++ b/sound/virtio/virtio_card.c
> @@ -382,8 +399,16 @@ static void virtsnd_reset_vqs(struct virtio_snd *snd)
> queue->vqueue = NULL;
> spin_unlock_irqrestore(&queue->lock, flags);
> }
> -
> vdev->config->del_vqs(vdev);
> + for (i = 0; snd->substreams && i < snd->nsubstreams; ++i) {
> + struct virtio_pcm_substream *vss = &snd->substreams[i];
> +
> + cancel_work_sync(&vss->elapsed_period);
[Severity: Medium]
Can calling cancel_work_sync(&vss->elapsed_period) inside virtsnd_reset_vqs()
stall freeze or remove for the full virtsnd_msg_timeout_ms duration?
In virtsnd_freeze() and virtsnd_remove():
virtsnd_ctl_msg_cancel_all(snd);
virtsnd_reset_vqs(snd);
virtsnd_ctl_msg_cleanup(snd);
virtsnd_ctl_msg_cancel_all() marks pending control messages with
msg->cancelled = true, but does not complete msg->notify; waking the waiters
only happens later in virtsnd_ctl_msg_cleanup() after virtsnd_reset_vqs()
returns.
Because vpcm->pcm->nonatomic is true, virtsnd_pcm_period_elapsed() acquires
the sleeping PCM stream lock via guard(pcm_stream_lock_irqsave)(substream).
If another thread holds that PCM stream lock while waiting in
virtsnd_ctl_msg_send() on an already-submitted control message (such as in
virtsnd_pcm_trigger()), or if virtsnd_pcm_period_elapsed() itself entered
snd_pcm_stop() -> virtsnd_pcm_trigger(SNDRV_PCM_TRIGGER_STOP) ->
virtsnd_ctl_msg_send_sync() just before virtsnd_reset_vqs() set
vss->resetting = true, the device has already been reset and will never
complete that control message.
Waiting in cancel_work_sync(&vss->elapsed_period) before
virtsnd_ctl_msg_cleanup(snd) prevents msg->notify from being completed until
wait_for_completion_timeout(&msg->notify, js) expires. Should control message
waiters be woken before synchronously canceling elapsed_period work?
[ ... ]
> diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c
> index b94ab24f5ffef..d6510d813f3ca 100644
> --- a/sound/virtio/virtio_pcm_msg.c
> +++ b/sound/virtio/virtio_pcm_msg.c
> @@ -540,19 +880,52 @@ static void virtsnd_pcm_msg_complete(struct virtio_pcm_msg *msg,
> size_t written_bytes)
> {
> struct virtio_pcm_substream *vss = msg->substream;
> + size_t completed_bytes;
> + u32 latency_bytes;
>
> + if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
> + vss->capture_staging) {
> + virtsnd_pcm_capture_complete(msg, written_bytes);
> + return;
> + }
> +
> + /* Only completed TX requests and valid RX audio advance the pointer. */
> guard(spinlock)(&vss->lock);
> + /* Empty STOP returns retire ownership without inventing captured data. */
> + if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
> + (!vss->prepared ||
> + (vss->stopped && written_bytes <= sizeof(*msg->status)))) {
> + msg->length = 0;
> + vss->msg_count--;
> + if (!vss->msg_count)
> + wake_up_all(&vss->msg_empty);
> + return;
> + }
> + completed_bytes = msg->length;
> + latency_bytes = le32_to_cpu(msg->status->latency_bytes);
> + if (vss->direction == SNDRV_PCM_STREAM_CAPTURE) {
> + struct snd_pcm_runtime *runtime = vss->substream->runtime;
> + size_t frame_bytes = runtime->frame_bits / gcd(runtime->frame_bits, 8);
> +
> + if (virtsnd_pcm_capture_status(msg, written_bytes, frame_bytes,
> + &completed_bytes, &latency_bytes) ||
> + !completed_bytes) {
> + vss->xfer_xrun = true;
> + msg->length = 0;
> + vss->msg_count--;
> + if (!vss->msg_count)
> + wake_up_all(&vss->msg_empty);
> + virtsnd_pcm_schedule_period_elapsed(vss);
> + return;
> + }
> + }
> #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 ||
> @@ -568,26 +941,16 @@ static void virtsnd_pcm_msg_complete(struct virtio_pcm_msg *msg,
> vss->owned_dma_area + msg->data_offset, data_bytes);
> }
> #endif
> + vss->hw_ptr += completed_bytes;
> + if (vss->direction == SNDRV_PCM_STREAM_CAPTURE) {
> + /* A full unseen ring cannot be represented by the modulo pointer. */
> + if (check_add_overflow(vss->capture_copied_total,
> + (u64)completed_bytes,
> + &vss->capture_copied_total) ||
[Severity: Medium]
On !CONFIG_MMU systems, does checking !vss->stopped before copying from
vss->owned_dma_area leave stale data in runtime->dma_area on post-STOP
capture completions?
At the top of virtsnd_pcm_msg_complete(), post-STOP capture completions with
written_bytes > sizeof(*msg->status) are intentionally kept so that in-flight
payloads (such as during capture SNDRV_PCM_STATE_DRAINING after
virtsnd_pcm_trigger(SNDRV_PCM_TRIGGER_STOP) sets vss->stopped = true) advance
vss->hw_ptr and vss->capture_copied_total by completed_bytes.
However, the #ifndef CONFIG_MMU bounce-buffer copy block is gated on
vss->prepared && !vss->stopped. When vss->stopped is true, memcpy() into
runtime->dma_area is skipped even though vss->hw_ptr and
vss->capture_copied_total are advanced, exposing stale or zeroed buffer
contents to userspace. Should the !vss->stopped check be removed from the
!CONFIG_MMU copy condition (and data_bytes use completed_bytes)?
[ ... ]
> diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
> index f4b45bddfb09c..05b38d8a67d7a 100644
> --- a/sound/virtio/virtio_pcm_ops.c
> +++ b/sound/virtio/virtio_pcm_ops.c
> @@ -288,28 +622,95 @@ 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 stopped;
> + 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;
> + epoch = vss->reset_epoch;
> + stopped = vss->stopped || vss->release_required || vss->msg_count;
> + }
> + if (stopped) {
> + rc = virtsnd_pcm_sync_stop_locked(substream);
> + if (rc)
> + return rc;
> + }
> + scoped_guard(pcm_stream_lock_irqsave, substream) {
> + guard(spinlock_irqsave)(&vss->lock);
> + if (vss->resetting || vss->reset_epoch != epoch)
> + return -ECANCELED;
> + vss->prepared = false;
> + vss->preparing = true;
> + suspended = vss->suspended;
> + }
[Severity: Medium]
When re-preparing a substream that is already in SNDRV_PCM_STATE_PREPARED,
can a concurrent SNDRV_PCM_IOCTL_SYNC_PTR ioctl race with
virtsnd_pcm_sync_stop_locked() before vss->preparing is set?
When snd_pcm_prepare() is called on a prepared substream (where
vss->release_required is true), ALSA core holds runtime->buffer_mutex but not
the PCM stream lock, and runtime->state remains SNDRV_PCM_STATE_PREPARED
until snd_pcm_post_prepare().
In virtsnd_pcm_prepare_locked(), virtsnd_pcm_sync_stop_locked() runs before
the PCM stream lock is taken to set vss->preparing = true and
vss->prepared = false. Inside virtsnd_pcm_sync_stop_locked() (and
virtsnd_pcm_pause_restart_sync_stop()), vss->stopped is set to true while
vss->prepared remains true across the sleeping VIRTIO_SND_R_PCM_RELEASE
request:
virtsnd_pcm_prepare_locked()
virtsnd_pcm_sync_stop_locked()
vss->stopped = true;
/* vss->prepared is still true, vss->preparing is still false */
virtsnd_pcm_send_sync(vss, msg); /* sleeps */
If another thread calls snd_pcm_sync_ptr() -> pcm_lib_apply_appl_ptr() during
this window, it acquires the PCM stream lock, passes virtsnd_pcm_ack_state(),
sees vss->preparing == false, vss->prepared == true, and vss->stopped == true
in virtsnd_pcm_pb_ack_locked() or virtsnd_pcm_cp_ack_locked(), and returns
-EPIPE, causing pcm_lib_apply_appl_ptr() to call __snd_pcm_xrun(substream)
mid-prepare.
Additionally, virtsnd_pcm_prepare_locked() sets vss->release_required = true
even when pause_restart is enabled, but neither virtsnd_pcm_release_wait()
nor virtsnd_pcm_pause_restart_sync_stop() clears vss->release_required,
causing stopped to be true on every subsequent prepare. Should vss->preparing
be armed under the PCM stream lock before calling
virtsnd_pcm_sync_stop_locked(), and should vss->release_required be cleared
when pause_restart releases the stream?
[ ... ]
> +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;
> + bool xrun;
> 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;
> + xrun = vss->xfer_xrun;
[ ... ]
> + }
> + if (reset_required) {
> + virtsnd_pcm_pause_restart_fail(vss, true, epoch);
> + return -EIO;
> + }
> + if (xrun)
> + return -EPIPE;
[ ... ]
> +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;
> + }
[Severity: Medium]
In virtsnd_pcm_pause_restart_start() and virtsnd_pcm_pause_restart_pause(),
does returning -EPIPE on a latched XRUN without arming vss->stopped and
runtime->stop_operating prevent subsequent DROP or prepare teardown from
running sync_stop?
In virtsnd_pcm_pause_restart_start(), if xrun is true on entry, it returns
-EPIPE directly without calling virtsnd_pcm_pause_restart_failed(vss, epoch)
(unlike the later vss->xfer_xrun check when queueing capture buffers, and
unlike the native trigger path which calls virtsnd_pcm_start_failed()). When
SNDRV_PCM_TRIGGER_START fails with -EPIPE on a linked substream,
snd_pcm_do_start() sets runtime->state to SNDRV_PCM_STATE_XRUN and
snd_pcm_action_group() only runs undo_action on group members preceding the
failed stream, leaving vss->stopped and runtime->stop_operating false on the
failed stream so SNDRV_PCM_IOCTL_DROP will not release the prepared stream on
the device.
Similarly, if a paused stream has vss->xfer_xrun latched and snd_pcm_drop()
or snd_pcm_prepare() calls snd_pcm_pause(substream, false) under the PCM
stream lock, virtsnd_pcm_pause_restart_start() returns -EPIPE and
snd_pcm_undo_pause() -> virtsnd_pcm_pause_restart_pause() also returns -EPIPE
in the VIRTSND_PCM_PAUSE_RESTART_RELEASED branch without setting
runtime->stop_operating = true (unlike the
VIRTSND_PCM_PAUSE_RESTART_RELEASE_SENT branch just below it). Because
runtime->state remains SNDRV_PCM_STATE_PAUSED and runtime->stop_operating is
false, both snd_pcm_do_stop() and snd_pcm_sync_stop() skip teardown.
Should both of these -EPIPE paths call virtsnd_pcm_pause_restart_failed(vss,
epoch) (or arm vss->stopped and runtime->stop_operating)?
[ ... ]
> +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;
[Severity: Medium]
In virtsnd_pcm_pause_restart_stop(), if suspend is true while !vss->prepared
&& !vss->msg_count, won't returning early skip setting vss->suspended = true?
In the native trigger path in virtsnd_pcm_trigger(), SNDRV_PCM_TRIGGER_SUSPEND
sets vss->suspended = true before checking if (!vss->prepared) return 0. Here
in virtsnd_pcm_pause_restart_stop(), if (!vss->prepared && !vss->msg_count)
returns 0 before setting vss->suspended = true. If a stream in pause_restart
mode has vss->prepared cleared and messages drained (for example, while paused
in VIRTSND_PCM_PAUSE_RESTART_RELEASED or after a failed pause transition) and
then receives SNDRV_PCM_TRIGGER_SUSPEND, vss->suspended remains false, so
staged pointer/ACK callbacks will not observe the suspended state and a
subsequent prepare will skip re-sending VIRTIO_SND_R_PCM_SET_PARAMS. Should
if (suspend) vss->suspended = true precede the early return check?
[ ... ]
> -static int virtsnd_pcm_pb_ack(struct snd_pcm_substream *substream)
> +static int virtsnd_pcm_pb_ack_locked(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;
>
> - rc = virtsnd_pcm_ack_state(substream);
> - if (rc)
> - return rc;
> -
> scoped_guard(spinlock_irqsave, &queue->lock) {
> guard(spinlock)(&vss->lock);
> + if (vss->preparing)
> + return -EBADFD;
> + if (vss->xfer_xrun)
> + return -EPIPE;
> + if (!vss->prepared)
> + return -EBADFD;
> transfer.indirect = vss->pcm_indirect;
> if (vss->stopped) {
> rc = -EPIPE;
> @@ -626,21 +1609,163 @@ static int virtsnd_pcm_pb_ack(struct snd_pcm_substream *substream)
> 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;
[ ... ]
> + 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;
> + }
[Severity: Medium]
Can carrying over an unconsumed sub-byte frame in vss->pcm_indirect.appl_ptr
cause a subsequent full-buffer ACK to fail this diff > runtime->buffer_size
check with a false -EPIPE?
For a sub-byte frame format such as 1-channel SNDRV_PCM_FORMAT_IMA_ADPCM
(where frame_bits == 4), acknowledging 1 frame yields
bytes = frames_to_bytes(runtime, 1) = 0, leaving vss->pcm_indirect.appl_ptr
lagging runtime->control->appl_ptr (old_appl_ptr) by 1 frame.
If the application then advances appl_ptr by runtime->buffer_size frames in
free-running mode (which pcm_lib_apply_appl_ptr() allows because
appl_ptr - old_appl_ptr == runtime->buffer_size, and which equals
vss->buffer_bytes in bytes), diff = appl_ptr - vss->pcm_indirect.appl_ptr
becomes runtime->buffer_size + 1. Checking
(snd_pcm_uframes_t)diff > runtime->buffer_size then rejects the ACK with
-EPIPE and latches vss->xfer_xrun before frames_to_bytes(runtime, diff) is
computed.
> +
> + /* 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;
> + /* Keep an unconsumed sub-byte frame for the next acknowledgment. */
> + vss->pcm_indirect.appl_ptr += bytes_to_frames(runtime, bytes);
[Severity: Medium]
What happens here when an odd number of frames is acknowledged on a
multi-channel fractional-byte format (such as 3-, 5-, or 7-channel
SNDRV_PCM_FORMAT_IMA_ADPCM where frame_bits > 8 and frame_bits % 8 != 0)?
For example, with 3-channel IMA_ADPCM (frame_bits == 12, 2 frames == 3 bytes),
if diff == 1 frame, frames_to_bytes(runtime, 1) is 1 byte (or if diff == 3
frames, bytes is 4 bytes). That unaligned byte count is committed to
vss->capture_acked_total and subtracted from vss->capture_ready, but
bytes_to_frames(runtime, bytes) then truncates 1 byte to 0 frames (or 4 bytes
to 2 frames == 3 bytes) when advancing vss->pcm_indirect.appl_ptr.
Because vss->pcm_indirect.appl_ptr does not advance for that partial frame's
byte, the next ACK will include that frame again in diff and double-count the
byte in vss->capture_acked_total, desynchronizing vss->capture_acked_total
from vss->pcm_indirect.appl_ptr and triggering a spurious -EPIPE XRUN when
next_acked_total exceeds vss->capture_pointer_seen_total or bytes exceeds
vss->capture_ready. Should bytes be rounded down to a whole number of frames
(for instance, frames_to_bytes(runtime, bytes_to_frames(runtime, bytes)))
before updating vss->capture_acked_total and vss->capture_ready?
[ ... ]
> +static int virtsnd_pcm_cp_ack_locked(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->preparing)
> + return -EBADFD;
> + if (vss->xfer_xrun)
> + return -EPIPE;
> + if (!vss->prepared)
> + return -EBADFD;
> transfer.indirect = vss->pcm_indirect;
> - if (vss->stopped) {
> + if (vss->xfer_xrun ||
> + (vss->stopped && runtime->state != SNDRV_PCM_STATE_DRAINING)) {
> rc = -EPIPE;
> } else {
> rc = snd_pcm_indirect_capture_transfer(substream,
[Severity: Medium]
When virtsnd_pcm_pb_ack_locked() or virtsnd_pcm_cp_ack_locked() returns
-EPIPE on a non-running substream (such as in SNDRV_PCM_STATE_PAUSED or
SNDRV_PCM_STATE_PREPARED), can leaving vss->stopped and
runtime->stop_operating unset cause a subsequent SNDRV_PCM_IOCTL_DROP to skip
virtsnd_pcm_sync_stop()?
In ALSA core, when pcm_lib_apply_appl_ptr() receives -EPIPE from ops->ack(),
it calls __snd_pcm_xrun(substream) -> snd_pcm_stop(substream,
SNDRV_PCM_STATE_XRUN). In snd_pcm_do_stop(), ALSA only invokes
ops->trigger(SNDRV_PCM_TRIGGER_STOP) and sets runtime->stop_operating = true
when snd_pcm_running(substream) is true. Once runtime->state becomes
SNDRV_PCM_STATE_XRUN, virtsnd_pcm_period_elapsed() also sees
runtime->state != SNDRV_PCM_STATE_PAUSED and does not call
virtsnd_pcm_capture_arm_paused_stop().
There are two places where -EPIPE is returned from ACK without arming
vss->stopped and runtime->stop_operating:
1. In both virtsnd_pcm_pb_ack_locked() and virtsnd_pcm_cp_ack_locked(), the
early check:
if (vss->xfer_xrun)
return -EPIPE;
returns before reaching virtsnd_pcm_transfer_failed(vss) (which also makes
the second vss->xfer_xrun check in virtsnd_pcm_cp_ack_locked() unreachable).
2. In virtsnd_pcm_cp_ack_locked() for staged capture, when
virtsnd_pcm_capture_staged_ack() returns -EPIPE,
virtsnd_pcm_capture_arm_paused_stop() only sets vss->stopped = true and
runtime->stop_operating = true when runtime->state == SNDRV_PCM_STATE_PAUSED,
leaving SNDRV_PCM_STATE_PREPARED (and capture SNDRV_PCM_STATE_DRAINING)
unarmed.
If the application then calls SNDRV_PCM_IOCTL_DROP, snd_pcm_drop() sees
runtime->state == SNDRV_PCM_STATE_XRUN with runtime->stop_operating == false
and transitions to SNDRV_PCM_STATE_SETUP without calling snd_pcm_sync_stop(),
leaving the device stream prepared/active and any queued descriptors
unreleased until prepare or hw_free. Should these -EPIPE paths always arm
vss->stopped and runtime->stop_operating via virtsnd_pcm_transfer_failed()?
[ ... ]
> +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 = {};
> + snd_pcm_uframes_t paused_ptr = 0;
> + bool paused = runtime->state == SNDRV_PCM_STATE_PAUSED;
> + int rc;
> +
> + /* Only PCM-locked callbacks access this observation flag. */
> + vss->indirect_reset_observing = true;
> + rc = snd_pcm_lib_reset(substream);
> + vss->indirect_reset_observing = false;
> + if (rc)
> + return rc;
> + guard(spinlock_irqsave)(&queue->lock);
> + guard(spinlock)(&vss->lock);
> + vss->preparing = false;
> + if (vss->resetting || vss->xfer_xrun) {
> + virtsnd_pcm_capture_reset_error(vss, runtime);
> + return 0;
> + }
> + if (!snd_pcm_running(substream) && !paused) {
> + vss->pcm_indirect.appl_ptr = runtime->status->hw_ptr;
> + vss->pcm_indirect.sw_ready = 0;
> + return 0;
> + }
> + if (!vss->prepared || vss->stopped) {
> + virtsnd_pcm_transfer_failed(vss);
> + vss->xfer_xrun = true;
> + virtsnd_pcm_schedule_period_elapsed(vss);
> + return 0;
> + }
> +
> + transfer.indirect = vss->pcm_indirect;
> + if (paused) {
> + /* Observe and reclaim paused completions in one queue transaction. */
> + vss->capture_pointer_seen_total = vss->capture_copied_total;
> + vss->indirect_reset_observing = true;
> + paused_ptr = snd_pcm_indirect_capture_pointer(substream,
> + &transfer.indirect, vss->hw_ptr);
> + vss->indirect_reset_observing = false;
> + if (paused_ptr == SNDRV_PCM_POS_XRUN) {
> + virtsnd_pcm_transfer_failed(vss);
> + vss->xfer_xrun = true;
> + virtsnd_pcm_schedule_period_elapsed(vss);
> + return 0;
> + }
> + runtime->status->hw_ptr = paused_ptr;
> + }
> + 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);
> + vss->xfer_xrun = true;
> + virtsnd_pcm_schedule_period_elapsed(vss);
> + return 0;
> + }
[Severity: Medium]
In virtsnd_pcm_indirect_reset(), shouldn't the three error paths after
snd_pcm_lib_reset() also rebase vss->pcm_indirect.appl_ptr and clear
vss->pcm_indirect.sw_ready?
The commit message notes:
Complete capture's private cursor rebase on RESET error paths while
retaining the XRUN latch and notification. This permits the core's
matching appl_ptr update without claiming stream recovery.
While the first error branch (vss->resetting || vss->xfer_xrun) calls
virtsnd_pcm_capture_reset_error(vss, runtime) to set
vss->pcm_indirect.appl_ptr = runtime->status->hw_ptr and
vss->pcm_indirect.sw_ready = 0, the other three error branches
(!vss->prepared || vss->stopped, paused_ptr == SNDRV_PCM_POS_XRUN, and rc != 0
from snd_pcm_indirect_capture_transfer()) return 0 to snd_pcm_reset_substream()
without updating vss->pcm_indirect.appl_ptr or vss->pcm_indirect.sw_ready,
leaving vss->pcm_indirect desynchronized from runtime->control->appl_ptr.
Should those three branches also call virtsnd_pcm_capture_reset_error(vss,
runtime) after setting vss->xfer_xrun = true?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103080821.16983-1-mynameisjune111@gmail.com?part=10
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v13 11/12] ALSA: virtio: prioritize staged capture period work
2026-11-03 8:08 ` [PATCH v13 11/12] ALSA: virtio: prioritize staged capture period work Joonho Lee
@ 2026-11-03 11:09 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-11-03 11:09 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/20261103080821.16983-1-mynameisjune111@gmail.com?part=11
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v13 12/12] docs: describe virtio-sound capture options
2026-11-03 8:08 ` [PATCH v13 12/12] docs: describe virtio-sound capture options Joonho Lee
@ 2026-11-03 11:27 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-11-03 11:27 UTC (permalink / raw)
To: Joonho Lee, linux-sound; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Medium] ALSA: virtio: missing NOMMU copy when stopped in virtsnd_pcm_msg_complete()
- [Medium] ALSA: virtio: reject short non-staged replies in virtsnd_pcm_msg_complete()
- [Low] docs: kernel-parameters: document capture_staging dependency for virtio_snd
- [Low] docs: kernel-parameters: use [HW,ALSA] instead of undefined [HW,SOUND] tag
- [Low] docs: virtio-sound: remove prompt instructions from commit description
--
commit 0d2f51e1deefbf07b12391911246c9ce81e71037
Author: Joonho Lee <mynameisjune111@gmail.com>
docs: describe virtio-sound capture options
This commit documents the five opt-in capture module parameters for
virtio_snd in Documentation/admin-guide/kernel-parameters.txt.
> docs: describe virtio-sound capture options
>
> 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.
>
> Keep the virtio_snd entries after virtio_mmio in alphabetical order.
> Include a diffstat in every mail patch.
>
> Assisted-by: LLM
> Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
[Severity: Low]
This isn't a bug, but "Include a diffstat in every mail patch." in the
commit description looks like a leftover patch-submission or prompt
instruction. Should that sentence be removed from the changelog?
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index e75344f4e0cde..de47885cd54b3 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -8344,6 +8344,36 @@ Kernel parameters
>
> Can be used multiple times for multiple devices.
>
> + virtio_snd.capture_allow_empty= [HW,SOUND]
[Severity: Low]
This isn't a bug, but the parameter restriction list at the top of
Documentation/admin-guide/kernel-parameters.txt defines ALSA:
ALSA ALSA sound support is enabled.
and does not define SOUND. Should these five virtio_snd.capture_* entries
use [HW,ALSA] instead of [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.
[Severity: Low]
In sound/virtio/virtio_pcm_msg.c and sound/virtio/virtio_pcm_ops.c, all
four of these compatibility parameters only take effect when
vss->capture_staging is enabled:
sound/virtio/virtio_pcm_msg.c:virtsnd_pcm_msg_complete() {
if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
vss->capture_staging) {
virtsnd_pcm_capture_complete(msg, written_bytes);
return;
}
}
sound/virtio/virtio_pcm_msg.c:virtsnd_pcm_capture_status() {
if (msg->substream->capture_staging && virtsnd_capture_packed_status) {
...
}
}
sound/virtio/virtio_pcm_ops.c:virtsnd_pcm_pause_restart_enabled() {
return virtsnd_capture_pause_restart && vss->capture_staging &&
vss->direction == SNDRV_PCM_STREAM_CAPTURE;
}
If a user enables virtio_snd.capture_allow_empty,
virtio_snd.capture_allow_short, virtio_snd.capture_packed_status, or
virtio_snd.capture_pause_restart while leaving virtio_snd.capture_staging
at its default value of false, the options silently have no effect.
Should the documentation for these four parameters explicitly state that
they require virtio_snd.capture_staging to be enabled?
[Severity: Medium]
Also looking at the underlying capture completion handling in
sound/virtio/virtio_pcm_msg.c:virtsnd_pcm_msg_complete(), can non-staged
capture on CONFIG_MMU=n builds advance vss->hw_ptr without copying the
captured audio payload when vss->stopped is true (such as during
SNDRV_PCM_STATE_DRAINING)?
In virtsnd_pcm_msg_complete(), post-STOP capture completions are only
discarded when written_bytes <= sizeof(*msg->status):
if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
(!vss->prepared ||
(vss->stopped && written_bytes <= sizeof(*msg->status)))) {
msg->length = 0;
vss->msg_count--;
if (!vss->msg_count)
wake_up_all(&vss->msg_empty);
return;
}
When a draining stream stops via snd_pcm_drain() -> snd_pcm_do_drain_init()
-> snd_pcm_do_stop() -> virtsnd_pcm_trigger(..., SNDRV_PCM_TRIGGER_STOP),
vss->stopped is set to true while vss->prepared remains true. If an
in-flight RX request then completes with written_bytes >
sizeof(*msg->status), the #ifndef CONFIG_MMU bounce-buffer copy is skipped
because of the !vss->stopped check:
#ifndef CONFIG_MMU
if (vss->direction == SNDRV_PCM_STREAM_CAPTURE &&
vss->prepared && !vss->stopped &&
written_bytes > sizeof(*msg->status)) {
...
memcpy(runtime->dma_area + msg->data_offset,
vss->owned_dma_area + msg->data_offset, data_bytes);
}
#endif
vss->hw_ptr += completed_bytes;
Since virtsnd_pcm_cp_pointer() and virtsnd_pcm_cp_ack_locked() allow reads
while in SNDRV_PCM_STATE_DRAINING, doesn't advancing vss->hw_ptr and
vss->capture_copied_total without copying from vss->owned_dma_area expose
stale or zeroed bytes in runtime->dma_area to userspace?
[Severity: Medium]
In the same non-staged capture path in
sound/virtio/virtio_pcm_msg.c:virtsnd_pcm_msg_complete(), what happens if
a device returns a non-empty, frame-aligned short capture reply (0 <
completed_bytes < msg->length) when virtio_snd.capture_staging is false?
In staged mode, virtsnd_pcm_capture_complete() explicitly rejects short
replies unless virtsnd_capture_allow_short is set:
} else if (audio_bytes < msg->length &&
!virtsnd_capture_allow_short) {
virtsnd_pcm_capture_xrun_locked(vss);
However, in non-staged mode, virtsnd_pcm_msg_complete() only checks
!completed_bytes after virtsnd_pcm_capture_status():
if (virtsnd_pcm_capture_status(msg, written_bytes, frame_bytes,
&completed_bytes, &latency_bytes) ||
!completed_bytes) {
vss->xfer_xrun = true;
...
return;
}
Because virtsnd_pcm_msg_alloc() permanently binds each non-staged
vss->msgs[i] to a fixed slice at runtime->dma_area + i * period_bytes,
accepting completed_bytes < msg->length advances vss->hw_ptr by
completed_bytes and resets vss->msgs[i]->length = 0 while the next
descriptor vss->msgs[i + 1] writes at (i + 1) * period_bytes. Doesn't this
desynchronize vss->hw_ptr from the fixed descriptor slices (exposing the
unwritten tail of vss->msgs[i] to userspace on subsequent completions) and
leave later descriptors with msg->length < period_bytes in
virtsnd_pcm_msg_send() so they are not re-queued?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103080821.16983-1-mynameisjune111@gmail.com?part=12
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2026-11-03 11:27 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03 8:08 [PATCH v13 00/12] ALSA: virtio: staged capture recovery and PCM RESET synchronization Joonho Lee
2026-11-03 8:08 ` [PATCH v13 01/12] ALSA: virtio: Propagate PCM send failures through ACK Joonho Lee
2026-11-03 8:33 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 02/12] ALSA: virtio: Recover PCM state after a failed start Joonho Lee
2026-11-03 8:55 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 03/12] ALSA: virtio: Retain queued messages through device reset Joonho Lee
2026-11-03 9:25 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 04/12] ALSA: virtio: Keep PCM DMA buffers alive through device ownership Joonho Lee
2026-11-03 9:49 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 05/12] ALSA: virtio: Keep control payloads alive through DMA completion Joonho Lee
2026-11-03 9:56 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 06/12] ALSA: virtio: wait through signals for PCM teardown Joonho Lee
2026-11-03 10:15 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 07/12] ALSA: pcm: mark stop-threshold accesses for capture policy sampling Joonho Lee
2026-11-03 10:17 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 08/12] ALSA: pcm: report partial progress under the stream lock Joonho Lee
2026-11-03 10:20 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 09/12] ALSA: pcm: add an optional reset operation " Joonho Lee
2026-11-03 10:27 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 10/12] ALSA: virtio: add staged capture and serialize PCM lifecycle accounting Joonho Lee
2026-11-03 11:05 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 11/12] ALSA: virtio: prioritize staged capture period work Joonho Lee
2026-11-03 11:09 ` sashiko-bot
2026-11-03 8:08 ` [PATCH v13 12/12] docs: describe virtio-sound capture options Joonho Lee
2026-11-03 11:27 ` 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.