mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] ALSA: xen-front: Fix event channel stream lifetime bugs
@ 2026-05-26 12:48 Cássio Gabriel
  2026-05-26 12:48 ` [PATCH 1/2] ALSA: xen-front: Reset event channel state on stream clear Cássio Gabriel
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Cássio Gabriel @ 2026-05-26 12:48 UTC (permalink / raw)
  To: Takashi Iwai, Oleksandr Andrushchenko, Jaroslav Kysela
  Cc: xen-devel, linux-sound, linux-kernel, Cássio Gabriel

Fix two related event-channel issues in the Xen ALSA frontend.

The first patch resets the event channel's expected incoming event
id when a stream is cleared, and drops stale queued events from
the previous stream instance.

The second patch keeps the request channel connected from .open(),
where it is needed for hw-rule queries and stream open requests,
but delays the event channel until after a successful .prepare().
This prevents current-position events from reaching the ALSA position
handler before runtime buffer and period geometry are valid.

Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
---
Cássio Gabriel (2):
      ALSA: xen-front: Reset event channel state on stream clear
      ALSA: xen-front: Connect event channel after stream prepare

 sound/xen/xen_snd_front_alsa.c    | 17 ++++++++++++-----
 sound/xen/xen_snd_front_evtchnl.c | 28 +++++++++++++++++++---------
 sound/xen/xen_snd_front_evtchnl.h |  6 ++++--
 3 files changed, 35 insertions(+), 16 deletions(-)
---
base-commit: 31d40472d5699a6139fe3a2ad6558645a99b7422
change-id: 20260423-alsa-xen-event-channel-fixes-6e4b40e649f7

Best regards,
--  
Cássio Gabriel <cassiogabrielcontato@gmail.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] ALSA: xen-front: Reset event channel state on stream clear
  2026-05-26 12:48 [PATCH 0/2] ALSA: xen-front: Fix event channel stream lifetime bugs Cássio Gabriel
@ 2026-05-26 12:48 ` Cássio Gabriel
  2026-05-26 12:48 ` [PATCH 2/2] ALSA: xen-front: Connect event channel after stream prepare Cássio Gabriel
  2026-05-27  5:29 ` [PATCH 0/2] ALSA: xen-front: Fix event channel stream lifetime bugs Takashi Iwai
  2 siblings, 0 replies; 4+ messages in thread
From: Cássio Gabriel @ 2026-05-26 12:48 UTC (permalink / raw)
  To: Takashi Iwai, Oleksandr Andrushchenko, Jaroslav Kysela
  Cc: xen-devel, linux-sound, linux-kernel, Cássio Gabriel

xen_snd_front_evtchnl_pair_clear() resets evt_next_id for both
channels. That is correct for the request channel, where evt_next_id is
used to allocate the next request id. It is wrong for the event channel:
incoming events are validated against evt_id, and evt_id is incremented
by evtchnl_interrupt_evt().

This leaves the expected event id from the previous stream instance. A
backend that restarts event ids for a reopened stream can then have valid
current-position events dropped until the stale frontend id catches up.

Reset evt_id for the event channel. Also advance the event-page consumer
to the current producer while clearing the stream, so obsolete events
queued for the previous stream instance are not delivered to the next
ALSA runtime.

Fixes: 1cee559351a7 ("ALSA: xen-front: Implement ALSA virtual sound driver")
Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
---
 sound/xen/xen_snd_front_evtchnl.c | 8 ++++++--
 sound/xen/xen_snd_front_evtchnl.h | 4 ++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/sound/xen/xen_snd_front_evtchnl.c b/sound/xen/xen_snd_front_evtchnl.c
index bc03f71bf16e..09e4c1d05636 100644
--- a/sound/xen/xen_snd_front_evtchnl.c
+++ b/sound/xen/xen_snd_front_evtchnl.c
@@ -456,7 +456,11 @@ void xen_snd_front_evtchnl_pair_clear(struct xen_snd_front_evtchnl_pair *evt_pai
 	}
 
 	scoped_guard(mutex, &evt_pair->evt.ring_io_lock) {
-		evt_pair->evt.evt_next_id = 0;
+		evt_pair->evt.evt_id = 0;
+		/* Drop obsolete events queued for the previous stream instance. */
+		evt_pair->evt.u.evt.page->in_cons =
+			evt_pair->evt.u.evt.page->in_prod;
+		/* Ensure the consumer index is visible before stream reuse. */
+		virt_wmb();
 	}
 }
-
diff --git a/sound/xen/xen_snd_front_evtchnl.h b/sound/xen/xen_snd_front_evtchnl.h
index 3675fba70564..8400261ac466 100644
--- a/sound/xen/xen_snd_front_evtchnl.h
+++ b/sound/xen/xen_snd_front_evtchnl.h
@@ -37,9 +37,9 @@ struct xen_snd_front_evtchnl {
 	/* State of the event channel. */
 	enum xen_snd_front_evtchnl_state state;
 	enum xen_snd_front_evtchnl_type type;
-	/* Either response id or incoming event id. */
+	/* Current response id or next expected incoming event id. */
 	u16 evt_id;
-	/* Next request id or next expected event id. */
+	/* Next request id. */
 	u16 evt_next_id;
 	/* Shared ring access lock. */
 	struct mutex ring_io_lock;

-- 
2.54.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] ALSA: xen-front: Connect event channel after stream prepare
  2026-05-26 12:48 [PATCH 0/2] ALSA: xen-front: Fix event channel stream lifetime bugs Cássio Gabriel
  2026-05-26 12:48 ` [PATCH 1/2] ALSA: xen-front: Reset event channel state on stream clear Cássio Gabriel
@ 2026-05-26 12:48 ` Cássio Gabriel
  2026-05-27  5:29 ` [PATCH 0/2] ALSA: xen-front: Fix event channel stream lifetime bugs Takashi Iwai
  2 siblings, 0 replies; 4+ messages in thread
From: Cássio Gabriel @ 2026-05-26 12:48 UTC (permalink / raw)
  To: Takashi Iwai, Oleksandr Andrushchenko, Jaroslav Kysela
  Cc: xen-devel, linux-sound, linux-kernel, Cássio Gabriel

The request channel must be connected from ALSA .open(), because hw-rule
queries and the stream open request use it. The event channel is
different: XENSND_EVT_CUR_POS handling uses ALSA runtime buffer and
period geometry, and the corresponding Xen stream parameters are not
submitted to the backend until .prepare() sends XENSND_OP_OPEN.

Currently .open() connects both channels. A backend current-position
event, or a stale event queued for an earlier stream instance, can
therefore reach xen_snd_front_alsa_handle_cur_pos() before
runtime->buffer_size and runtime->period_size are valid.

Add a per-channel connection helper, connect only the request channel in
.open(), connect the event channel after a successful stream prepare,
and disconnect it before stream close/free. Re-check the event-channel
state after taking ring_io_lock so disconnecting the event channel
synchronizes against a threaded IRQ that passed the initial lockless
state test. Keep defensive runtime geometry checks in the position
handler.

Fixes: 1cee559351a7 ("ALSA: xen-front: Implement ALSA virtual sound driver")
Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
---
 sound/xen/xen_snd_front_alsa.c    | 17 ++++++++++++-----
 sound/xen/xen_snd_front_evtchnl.c | 20 +++++++++++++-------
 sound/xen/xen_snd_front_evtchnl.h |  2 ++
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/sound/xen/xen_snd_front_alsa.c b/sound/xen/xen_snd_front_alsa.c
index dc626480123a..a6dd196f73d6 100644
--- a/sound/xen/xen_snd_front_alsa.c
+++ b/sound/xen/xen_snd_front_alsa.c
@@ -378,7 +378,7 @@ static int alsa_open(struct snd_pcm_substream *substream)
 
 	stream_clear(stream);
 
-	xen_snd_front_evtchnl_pair_set_connected(stream->evt_pair, true);
+	xen_snd_front_evtchnl_set_connected(&stream->evt_pair->req, true);
 
 	ret = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_FORMAT,
 				  alsa_hw_rule, stream,
@@ -498,6 +498,8 @@ static int alsa_hw_free(struct snd_pcm_substream *substream)
 	struct xen_snd_front_pcm_stream_info *stream = stream_get(substream);
 	int ret;
 
+	xen_snd_front_evtchnl_set_connected(&stream->evt_pair->evt, false);
+
 	ret = xen_snd_front_stream_close(&stream->evt_pair->req);
 	stream_free(stream);
 	return ret;
@@ -532,6 +534,7 @@ static int alsa_prepare(struct snd_pcm_substream *substream)
 			return ret;
 
 		stream->is_open = true;
+		xen_snd_front_evtchnl_set_connected(&stream->evt_pair->evt, true);
 	}
 
 	return 0;
@@ -571,20 +574,24 @@ void xen_snd_front_alsa_handle_cur_pos(struct xen_snd_front_evtchnl *evtchnl,
 {
 	struct snd_pcm_substream *substream = evtchnl->u.evt.substream;
 	struct xen_snd_front_pcm_stream_info *stream = stream_get(substream);
+	struct snd_pcm_runtime *runtime = substream->runtime;
 	snd_pcm_uframes_t delta, new_hw_ptr, cur_frame;
 
-	cur_frame = bytes_to_frames(substream->runtime, pos_bytes);
+	if (!runtime->buffer_size || !runtime->period_size)
+		return;
+
+	cur_frame = bytes_to_frames(runtime, pos_bytes);
 
 	delta = cur_frame - stream->be_cur_frame;
 	stream->be_cur_frame = cur_frame;
 
 	new_hw_ptr = (snd_pcm_uframes_t)atomic_read(&stream->hw_ptr);
-	new_hw_ptr = (new_hw_ptr + delta) % substream->runtime->buffer_size;
+	new_hw_ptr = (new_hw_ptr + delta) % runtime->buffer_size;
 	atomic_set(&stream->hw_ptr, (int)new_hw_ptr);
 
 	stream->out_frames += delta;
-	if (stream->out_frames > substream->runtime->period_size) {
-		stream->out_frames %= substream->runtime->period_size;
+	if (stream->out_frames > runtime->period_size) {
+		stream->out_frames %= runtime->period_size;
 		snd_pcm_period_elapsed(substream);
 	}
 }
diff --git a/sound/xen/xen_snd_front_evtchnl.c b/sound/xen/xen_snd_front_evtchnl.c
index 09e4c1d05636..17a30452c0cc 100644
--- a/sound/xen/xen_snd_front_evtchnl.c
+++ b/sound/xen/xen_snd_front_evtchnl.c
@@ -94,6 +94,9 @@ static irqreturn_t evtchnl_interrupt_evt(int irq, void *dev_id)
 
 	guard(mutex)(&channel->ring_io_lock);
 
+	if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
+		return IRQ_HANDLED;
+
 	prod = page->in_prod;
 	/* Ensure we see ring contents up to prod. */
 	virt_rmb();
@@ -430,8 +433,8 @@ int xen_snd_front_evtchnl_publish_all(struct xen_snd_front_info *front_info)
 	return ret;
 }
 
-void xen_snd_front_evtchnl_pair_set_connected(struct xen_snd_front_evtchnl_pair *evt_pair,
-					      bool is_connected)
+void xen_snd_front_evtchnl_set_connected(struct xen_snd_front_evtchnl *channel,
+					 bool is_connected)
 {
 	enum xen_snd_front_evtchnl_state state;
 
@@ -440,13 +443,16 @@ void xen_snd_front_evtchnl_pair_set_connected(struct xen_snd_front_evtchnl_pair
 	else
 		state = EVTCHNL_STATE_DISCONNECTED;
 
-	scoped_guard(mutex, &evt_pair->req.ring_io_lock) {
-		evt_pair->req.state = state;
+	scoped_guard(mutex, &channel->ring_io_lock) {
+		channel->state = state;
 	}
+}
 
-	scoped_guard(mutex, &evt_pair->evt.ring_io_lock) {
-		evt_pair->evt.state = state;
-	}
+void xen_snd_front_evtchnl_pair_set_connected(struct xen_snd_front_evtchnl_pair *evt_pair,
+					      bool is_connected)
+{
+	xen_snd_front_evtchnl_set_connected(&evt_pair->req, is_connected);
+	xen_snd_front_evtchnl_set_connected(&evt_pair->evt, is_connected);
 }
 
 void xen_snd_front_evtchnl_pair_clear(struct xen_snd_front_evtchnl_pair *evt_pair)
diff --git a/sound/xen/xen_snd_front_evtchnl.h b/sound/xen/xen_snd_front_evtchnl.h
index 8400261ac466..f6ebdb09c029 100644
--- a/sound/xen/xen_snd_front_evtchnl.h
+++ b/sound/xen/xen_snd_front_evtchnl.h
@@ -77,6 +77,8 @@ void xen_snd_front_evtchnl_free_all(struct xen_snd_front_info *front_info);
 int xen_snd_front_evtchnl_publish_all(struct xen_snd_front_info *front_info);
 
 void xen_snd_front_evtchnl_flush(struct xen_snd_front_evtchnl *evtchnl);
+void xen_snd_front_evtchnl_set_connected(struct xen_snd_front_evtchnl *channel,
+					 bool is_connected);
 
 void xen_snd_front_evtchnl_pair_set_connected(struct xen_snd_front_evtchnl_pair *evt_pair,
 					      bool is_connected);

-- 
2.54.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 0/2] ALSA: xen-front: Fix event channel stream lifetime bugs
  2026-05-26 12:48 [PATCH 0/2] ALSA: xen-front: Fix event channel stream lifetime bugs Cássio Gabriel
  2026-05-26 12:48 ` [PATCH 1/2] ALSA: xen-front: Reset event channel state on stream clear Cássio Gabriel
  2026-05-26 12:48 ` [PATCH 2/2] ALSA: xen-front: Connect event channel after stream prepare Cássio Gabriel
@ 2026-05-27  5:29 ` Takashi Iwai
  2 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2026-05-27  5:29 UTC (permalink / raw)
  To: Cássio Gabriel
  Cc: Takashi Iwai, Oleksandr Andrushchenko, Jaroslav Kysela,
	xen-devel, linux-sound, linux-kernel

On Tue, 26 May 2026 14:48:25 +0200,
Cássio Gabriel wrote:
> 
> Fix two related event-channel issues in the Xen ALSA frontend.
> 
> The first patch resets the event channel's expected incoming event
> id when a stream is cleared, and drops stale queued events from
> the previous stream instance.
> 
> The second patch keeps the request channel connected from .open(),
> where it is needed for hw-rule queries and stream open requests,
> but delays the event channel until after a successful .prepare().
> This prevents current-position events from reaching the ALSA position
> handler before runtime buffer and period geometry are valid.
> 
> Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
> ---
> Cássio Gabriel (2):
>       ALSA: xen-front: Reset event channel state on stream clear
>       ALSA: xen-front: Connect event channel after stream prepare

Applied both patches now to for-next branch.  Thanks.


Takashi

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-27  5:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-26 12:48 [PATCH 0/2] ALSA: xen-front: Fix event channel stream lifetime bugs Cássio Gabriel
2026-05-26 12:48 ` [PATCH 1/2] ALSA: xen-front: Reset event channel state on stream clear Cássio Gabriel
2026-05-26 12:48 ` [PATCH 2/2] ALSA: xen-front: Connect event channel after stream prepare Cássio Gabriel
2026-05-27  5:29 ` [PATCH 0/2] ALSA: xen-front: Fix event channel stream lifetime bugs Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®