Hi Val,
On Thu Apr 23, 2026 at 6:41 AM CEST, Val Packett wrote:
> The response sent by the firmware when requesting a clock vote (opcode
> AFE_CMD_RSP_REMOTE_LPASS_CORE_HW_VOTE_REQUEST) does not actually have
> the same opcode + status payload as APR_BASIC_RSP_RESULT. Rather, it
> returns one single u32 which is the client_handle that must be used in
> future unvote requests for the same clock.
>
> As a result of this type confusion, the status returned by the callback
> to q6afe_vote_lpass_core_hw was actually an out-of-bounds read. It was
> only interpreted as success (0) most of the time due to luck, but there
> are reports of random ("more likely on cold boot", "depending on hacks
> made in other drivers") errors such as:
>
> [ 20.961100] qcom-q6afe aprsvc:service:4:4: AFE failed to vote (3)
> [ 20.961131] Failed to prepare clk 'core': -110
>
> Fix by correctly interpreting the response as a single u32, and actually
> store it as the client_handle to ensure unvote would work correctly.
>
> Link: https://lore.kernel.org/all/5976946.DvuYhMxLoT@antlia/
> Signed-off-by: Val Packett <[email protected]>
> ---
>
> Found by reading and comparing with downstream:
> https://github.com/YumeMichi/kernel_xiaomi_pipa/blob/27190355fb31284988ddf505cb7cfba5128104cf/techpack/audio/dsp/q6afe.c#L1261-L1263
>
> What's really bizzare though is that some of the logs go:
>
> [ 10.827469] qcom-q6afe aprsvc:service:4:4: cmd = 0x100f4 returned error
> = 0x16
> [ 10.827512] qcom-q6afe aprsvc:service:4:4: Unknown cmd 0x100f4
> [ 14.052896] qcom-q6afe aprsvc:service:4:4: AFE failed to vote (3)
>
> ..where the "returned error =" message is something that can only be
> printed by the callback if it goes into the **other** switch() branch,
> i.e. when hdr->opcode == APR_BASIC_RSP_RESULT. How reading out-of-bounds
> only in the AFE_CMD_RSP_REMOTE_LPASS_CORE_HW_VOTE_REQUEST branch would
> cause that to happen (even on a subsequent vote after the first one to
> perform the read) is beyond me.
>
> Still, the person that reported this heisenbug told me that this has
> actually completely fixed it.
This seems conceptually similar to what I needed to do for SM6350/SM7225
microphone (wip) - it's not necessary for just speaker btw:
https://github.com/z3ntu/linux/commit/107bf0c39e40a207036e963f878f39467f978393
There I'm storing this handle per 'block' and not just one "vote_res",
essentially copying how downstream Linux does it. Your solution
definitely has less lines of diff, but I can't judge whether it's enough
to store it like that :)
Thanks for looking into this though!
>
> ---
> sound/soc/qcom/qdsp6/q6afe.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
> index 40237267fda0..28b5b6b91897 100644
> --- a/sound/soc/qcom/qdsp6/q6afe.c
> +++ b/sound/soc/qcom/qdsp6/q6afe.c
> @@ -379,6 +379,7 @@ struct q6afe {
> struct q6core_svc_api_info ainfo;
> struct mutex lock;
> struct aprv2_ibasic_rsp_result_t result;
> + uint32_t vote_result;
> wait_queue_head_t wait;
> struct list_head port_list;
> spinlock_t port_list_lock;
> @@ -968,13 +969,14 @@ static int q6afe_callback(struct apr_device *adev,
> const struct apr_resp_pkt *da
> const struct aprv2_ibasic_rsp_result_t *res;
> const struct apr_hdr *hdr = &data->hdr;
> struct q6afe_port *port;
> + uint32_t *vote_res;
>
> if (!data->payload_size)
> return 0;
>
> - res = data->payload;
> switch (hdr->opcode) {
> case APR_BASIC_RSP_RESULT: {
> + res = data->payload;
> if (res->status) {
> dev_err(afe->dev, "cmd = 0x%x returned error = 0x%x\n",
> res->opcode, res->status);
> @@ -1001,8 +1003,10 @@ static int q6afe_callback(struct apr_device *adev,
> const struct apr_resp_pkt *da
> }
> break;
> case AFE_CMD_RSP_REMOTE_LPASS_CORE_HW_VOTE_REQUEST:
> + vote_res = data->payload;
> afe->result.opcode = hdr->opcode;
> - afe->result.status = res->status;
> + afe->result.status = 0;
> + afe->vote_result = *vote_res;
> wake_up(&afe->wait);
> break;
> default:
> @@ -1899,6 +1903,8 @@ int q6afe_vote_lpass_core_hw(struct device *dev,
> uint32_t hw_block_id,
> AFE_CMD_RSP_REMOTE_LPASS_CORE_HW_VOTE_REQUEST);
> if (ret)
> dev_err(afe->dev, "AFE failed to vote (%d)\n", hw_block_id);
> + else
> + *client_handle = afe->vote_result;
>
> return ret;
> }
