[RFC PATCH 4/4] ASoC: qcom: q6apm: recover PCM from failed data receipts

[Zhang Jiaxi]

From: Jiaxi Zhang <z1529105815@xxxxxxxxxxx>

graph_callback indexes period buffers using unchecked DSP tokens and
publishes hw_ptr before checking the returned address. A valid DSP data
failure also reaches the normal period callback. The PCM consumer then
advances ALSA progress and may queue another capture read.

Validate the complete V2 payload, period index, both address halves and
submitted memory-map handle before publishing a result or event. Opt the
PCM client into a host data-error event: a matching nonzero primary DSP
status latches a PCM error without advancing hw_ptr or sending a normal
completion. Stop a running PCM as XRUN, suppress later period callbacks
and capture requeues, and make ack, start and pointer observe the latch.
A normal prepare clears the latch and can recover the stream.

Capture reads are submitted during prepare, before ALSA is RUNNING. Keep
an early error latched and fail prepare/start rather than taking the PCM
stream lock in that state. A nonatomic backend START may be awaiting a
GPR reply while owning that lock. The pointer's XRUN result also exposes
an error arriving between the final prepare check and START. Use
READ_ONCE/WRITE_ONCE for the new flags shared with the GPR RX callback.

Clients which do not opt in, including compressed audio, retain the
existing valid-receipt status handling. This patch does not claim
compressed DSP-error recovery. Malformed or identity-mismatched receipts
cannot be attributed to a live stream and do not generate an error.

No new deferred worker or GPR lifetime contract is introduced. Existing
late-callback, recycled-token, port teardown and DSP restart limitations
remain; the current mapping/period identity is not a generation tag.
RD offset/data_size and metadata-status semantics are also unchanged.

This draft targets sound.git for-next
62d9f9ffdfd44e88010412bf8f23732f0a93a9be. A focused test extracts the
actual callback and PCM prepare/ack/trigger/pointer consumer functions
and models ALSA/transport APIs. It demonstrates the baseline false
progress and the bounds-only stall, then checks matched WR/RD errors,
untrusted receipts, early errors and normal prepare recovery. It passes
with AddressSanitizer and UndefinedBehaviorSanitizer; three compile
warnings concern the harness READ_ONCE mock on existing packed pointer
fields. The expanded patch has not been hardware-tested or
lifetime-stress-tested. Target object compilation is recorded separately.

Signed-off-by: Jiaxi Zhang <z1529105815@xxxxxxxxxxx>

---
diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c
--- a/sound/soc/qcom/qdsp6/q6apm.c
+++ b/sound/soc/qcom/qdsp6/q6apm.c
@@ -570,9 +570,7 @@
 	struct device *dev = graph->dev;
 	uint32_t client_event;
 	phys_addr_t phys;
-	int token;
-
-	result = data->payload;
+	u32 token;
 
 	switch (hdr->opcode) {
 	case APM_EVENT_MODULE_TO_CLIENT:
@@ -588,6 +586,10 @@
 	case DATA_CMD_RSP_WR_SH_MEM_EP_DATA_BUFFER_DONE_V2:
 		if (!graph->ar_graph)
 			break;
+		if (data->payload_size < (int)sizeof(*done) || !data->payload) {
+			dev_err(dev, "WR BUFF Invalid payload size %d\n", data->payload_size);
+			break;
+		}
 		client_event = APM_CLIENT_EVENT_DATA_WRITE_DONE;
 		mutex_lock(&graph->lock);
 		token = hdr->token & APM_WRITE_TOKEN_MASK;
@@ -597,14 +599,33 @@
 			mutex_unlock(&graph->lock);
 			break;
 		}
+		if (token >= graph->rx_data.num_periods) {
+			mutex_unlock(&graph->lock);
+			dev_err(dev, "WR BUFF Invalid token %u\n", token);
+			break;
+		}
 		phys = graph->rx_data.buf[token].phys;
 		mutex_unlock(&graph->lock);
-		/* token numbering starts at 0 */
-		atomic_set(&graph->rx_data.hw_ptr, token + 1);
 		if (lower_32_bits(phys) == done->buf_addr_lsw &&
 		    upper_32_bits(phys) == done->buf_addr_msw) {
+			if (done->mem_map_handle != graph->info->mem_map_handle) {
+				dev_err(dev, "WR BUFF Unexpected handle %08x expected %08x\n",
+					done->mem_map_handle, graph->info->mem_map_handle);
+				break;
+			}
 			graph->result.opcode = hdr->opcode;
 			graph->result.status = done->status;
+			if (done->status) {
+				dev_err(dev, "WR BUFF DSP status %08x metadata %08x token %08x\n",
+					done->status, done->md_status, hdr->token);
+				if (READ_ONCE(graph->pcm_data_error)) {
+					if (graph->cb)
+						graph->cb(APM_CLIENT_EVENT_DATA_ERROR, hdr->token,
+							  data->payload, graph->priv);
+					break;
+				}
+			}
+			atomic_set(&graph->rx_data.hw_ptr, token + 1);
 			if (graph->cb)
 				graph->cb(client_event, hdr->token, data->payload, graph->priv);
 		} else {
@@ -616,6 +637,10 @@
 	case DATA_CMD_RSP_RD_SH_MEM_EP_DATA_BUFFER_V2:
 		if (!graph->ar_graph)
 			break;
+		if (data->payload_size < (int)sizeof(*rd_done) || !data->payload) {
+			dev_err(dev, "RD BUFF Invalid payload size %d\n", data->payload_size);
+			break;
+		}
 		client_event = APM_CLIENT_EVENT_DATA_READ_DONE;
 		mutex_lock(&graph->lock);
 		rd_done = data->payload;
@@ -623,15 +648,34 @@
 			mutex_unlock(&graph->lock);
 			break;
 		}
+		if (hdr->token >= graph->tx_data.num_periods) {
+			mutex_unlock(&graph->lock);
+			dev_err(dev, "RD BUFF Invalid token %u\n", hdr->token);
+			break;
+		}
 		phys = graph->tx_data.buf[hdr->token].phys;
 		mutex_unlock(&graph->lock);
-		/* token numbering starts at 0 */
-		atomic_set(&graph->tx_data.hw_ptr, hdr->token + 1);
 
 		if (upper_32_bits(phys) == rd_done->buf_addr_msw &&
 		    lower_32_bits(phys) == rd_done->buf_addr_lsw) {
+			if (rd_done->mem_map_handle != graph->info->mem_map_handle) {
+				dev_err(dev, "RD BUFF Unexpected handle %08x expected %08x\n",
+					rd_done->mem_map_handle, graph->info->mem_map_handle);
+				break;
+			}
 			graph->result.opcode = hdr->opcode;
 			graph->result.status = rd_done->status;
+			if (rd_done->status) {
+				dev_err(dev, "RD BUFF DSP status %08x metadata %08x token %08x\n",
+					rd_done->status, rd_done->md_status, hdr->token);
+				if (READ_ONCE(graph->pcm_data_error)) {
+					if (graph->cb)
+						graph->cb(APM_CLIENT_EVENT_DATA_ERROR, hdr->token,
+							  data->payload, graph->priv);
+					break;
+				}
+			}
+			atomic_set(&graph->tx_data.hw_ptr, hdr->token + 1);
 			if (graph->cb)
 				graph->cb(client_event, hdr->token, data->payload, graph->priv);
 		} else {
@@ -645,6 +689,7 @@
 			graph->cb(client_event, hdr->token, data->payload, graph->priv);
 		break;
 	case GPR_BASIC_RSP_RESULT:
+		result = data->payload;
 		switch (result->opcode) {
 		case APM_CMD_SHARED_MEM_MAP_REGIONS:
 		case DATA_CMD_WR_SH_MEM_EP_MEDIA_FORMAT:
diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -96,6 +96,7 @@
 	struct q6apm_graph *graph;
 	spinlock_t lock;
 	bool notify_on_drain;
+	bool data_error;
 };
 
 struct q6apm_dai_data {
@@ -240,6 +241,7 @@
 {
 	struct q6apm_dai_rtd *prtd = priv;
 	struct snd_pcm_substream *substream = prtd->substream;
+	snd_pcm_state_t state;
 
 	switch (opcode) {
 	case APM_CLIENT_EVENT_WATERMARK_EVENT:
@@ -249,14 +251,29 @@
 		prtd->state = Q6APM_STREAM_STOPPED;
 		break;
 	case APM_CLIENT_EVENT_DATA_WRITE_DONE:
+		if (READ_ONCE(prtd->data_error))
+			break;
 		snd_pcm_period_elapsed(substream);
 
 		break;
 	case APM_CLIENT_EVENT_DATA_READ_DONE:
+		if (READ_ONCE(prtd->data_error))
+			break;
 		snd_pcm_period_elapsed(substream);
 		if (prtd->state == Q6APM_STREAM_RUNNING)
 			q6apm_read(prtd->graph);
 
+		break;
+	case APM_CLIENT_EVENT_DATA_ERROR:
+		WRITE_ONCE(prtd->data_error, true);
+		WRITE_ONCE(prtd->state, Q6APM_STREAM_STOPPED);
+		state = READ_ONCE(substream->runtime->state);
+		if (state == SNDRV_PCM_STATE_RUNNING ||
+		    (state == SNDRV_PCM_STATE_DRAINING &&
+		     substream->stream == SNDRV_PCM_STREAM_PLAYBACK))
+			snd_pcm_stop_xrun(substream);
+		else
+			wake_up(&substream->runtime->sleep);
 		break;
 	default:
 		break;
@@ -353,6 +370,7 @@
 
 	prtd->last_pos_index = 0;
 	prtd->pcm_count = snd_pcm_lib_period_bytes(substream);
+	WRITE_ONCE(prtd->data_error, false);
 	if (q6apm_is_graph_in_push_pull_mode(prtd->graph)) {
 		if (prtd->pcm_size != prtd->push_pull_size) {
 			ret = q6apm_push_pull_config(prtd->graph, prtd->phys, prtd->pos_phys,
@@ -414,6 +432,9 @@
 		}
 	}
 
+	if (READ_ONCE(prtd->data_error))
+		return -EPIPE;
+
 	/* Now that graph as been prepared and started update the internal state accordingly */
 	prtd->state = Q6APM_STREAM_RUNNING;
 
@@ -425,6 +446,9 @@
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct q6apm_dai_rtd *prtd = runtime->private_data;
 	int i, ret = 0, avail_periods;
+
+	if (READ_ONCE(prtd->data_error))
+		return -EPIPE;
 
 	if (q6apm_is_graph_in_push_pull_mode(prtd->graph))
 		return 0;
@@ -455,6 +479,8 @@
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		if (READ_ONCE(prtd->data_error))
+			return -EPIPE;
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 		/* TODO support be handled via SoftPause Module */
@@ -504,6 +530,7 @@
 		ret = PTR_ERR(prtd->graph);
 		goto err;
 	}
+	WRITE_ONCE(prtd->graph->pcm_data_error, true);
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		runtime->hw = q6apm_dai_hardware_playback;
@@ -593,6 +620,9 @@
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct q6apm_dai_rtd *prtd = runtime->private_data;
 	snd_pcm_uframes_t ptr;
+
+	if (READ_ONCE(prtd->data_error))
+		return SNDRV_PCM_POS_XRUN;
 
 	if (q6apm_is_graph_in_push_pull_mode(prtd->graph)) {
 		int retries = 10;
diff --git a/sound/soc/qcom/qdsp6/q6apm.h b/sound/soc/qcom/qdsp6/q6apm.h
--- a/sound/soc/qcom/qdsp6/q6apm.h
+++ b/sound/soc/qcom/qdsp6/q6apm.h
@@ -42,6 +42,7 @@
 #define APM_CLIENT_EVENT_DATA_WRITE_DONE	0x1009
 #define APM_CLIENT_EVENT_DATA_READ_DONE		0x100a
 #define APM_CLIENT_EVENT_WATERMARK_EVENT	0x100b
+#define APM_CLIENT_EVENT_DATA_ERROR		0x100c
 #define APM_WRITE_TOKEN_MASK                   GENMASK(15, 0)
 #define APM_WRITE_TOKEN_LEN_MASK               GENMASK(31, 16)
 #define APM_WRITE_TOKEN_LEN_SHIFT              16
@@ -98,6 +99,7 @@
 struct q6apm_graph {
 	void *priv;
 	q6apm_cb cb;
+	bool pcm_data_error;
 	uint32_t id;
 	uint32_t shm_iid;
 	struct device *dev;