Re: [PATCH v5 03/10] ASoC: qcom: Add QAIF hardware register map

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The QAIF_EE_*_MAP_REG macros have incorrect base addresses due to an arithmetic typo (using truncated base values like `0x1920` instead of values in the `0x19000` EE block), resulting in accesses to unmapped memory regions.
- [Low] Missing `_REG` suffix on RAW_STAT macros for WRDMA and RDDMA Error Responses.
--

commit 7629bdb576e93d9171f876d77ffa9606caac557c
Author: Harendra Gautam <harendra.gautam@oss.qualcomm.com>

ASoC: qcom: Add QAIF hardware register map

This commit adds the QAIF hardware block MMIO register offsets in a new
header file. It defines accessor macros to select the appropriate DMA
register sets based on the DAI ID for the core registers, interfaces,
and execution engine (EE) contexts.

> diff --git a/sound/soc/qcom/qaif-reg.h b/sound/soc/qcom/qaif-reg.h
> new file mode 100644
> --- /dev/null
> +++ b/sound/soc/qcom/qaif-reg.h

[ ... ]

> +#define QAIF_EE_RDDMA_ASSIGNMENT_REG(v)			(0x19148 + (0x1000 * ((v)->ee)))
> +#define QAIF_EE_WRDMA_ASSIGNMENT_REG(v)			(0x19150 + (0x1000 * ((v)->ee)))
> +#define QAIF_EE_INTF_ASSIGNMENT_REG(v)			(0x19158 + (0x1000 * ((v)->ee)))
> +#define QAIF_EE_CODEC_RDDMA_ASSIGN_REG(v)		(0x19308 + (0x1000 * ((v)->ee)))
> +#define QAIF_EE_CODEC_WRDMA_ASSIGN_REG(v)		(0x19318 + (0x1000 * ((v)->ee)))
> +#define QAIF_EE_RDDMA_MAP_REG(v)			(0x1920 + (0x1000 * ((v)->ee)))
> +#define QAIF_EE_WRDMA_MAP_REG(v)			(0x1940 + (0x1000 * ((v)->ee)))
> +#define QAIF_EE_INTF_MAP_REG(v)				(0x1960 + (0x1000 * ((v)->ee)))
> +#define QAIF_EE_CODEC_RDDMA_MAP_REG(v)			(0x1980 + (0x1000 * ((v)->ee)))
> +#define QAIF_EE_CODEC_WRDMA_MAP_REG(v)			(0x1A00 + (0x1000 * ((v)->ee)))

[Severity: Critical]
Does this arithmetic calculation result in out-of-bounds memory accesses?

If v->ee > 0 (for example, v->ee == 1), QAIF_EE_RDDMA_MAP_REG calculates to
0x1920 + 0x1000 = 0x2920. Since the stride is 0x1000 and the other EE
registers start in the 0x19000 block (e.g. 0x19148), does this mean 0x1920
is a typo for a value in the 0x19000 range?

Adding 0x1000 to the truncated base 0x1920 lands in an unmapped hole
(0x2920) completely outside the intended EE block, which could cause a
system bus fault (SError) and a kernel panic when written to during driver
initialization. This same pattern seems to affect all five MAP_REG macros
defined here.

[ ... ]

> +/* RDDMA Error Response Interrupts */
> +#define QAIF_EE_RDDMA_ERR_RSP_IRQ_EN_REG(v, i)		qaif_rddma_irq_reg_addr(v, i, 0x50)
> +#define QAIF_EE_RDDMA_ERR_RSP_IRQ_STAT_REG(v, i)	qaif_rddma_irq_reg_addr(v, i, 0x58)
> +#define QAIF_EE_RDDMA_ERR_RSP_IRQ_RAW_STAT(v, i)	qaif_rddma_irq_reg_addr(v, i, 0x60)

[Severity: Low]
This isn't a bug, but should this macro have a _REG suffix to match the
naming convention of all the other raw stat macros in this file?

[ ... ]

> +/* WRDMA Error Response Interrupts */
> +#define QAIF_EE_WRDMA_ERR_RSP_IRQ_EN_REG(v, i)		qaif_wrdma_irq_reg_addr(v, i, 0x00)
> +#define QAIF_EE_WRDMA_ERR_RSP_IRQ_STAT_REG(v, i)	qaif_wrdma_irq_reg_addr(v, i, 0x08)
> +#define QAIF_EE_WRDMA_ERR_RSP_IRQ_RAW_STAT(v, i)	qaif_wrdma_irq_reg_addr(v, i, 0x60)

[Severity: Low]
Similarly, should this macro also include the _REG suffix?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-b4-qaif-shikra-audio-v5-0-6d8ad9ba5c9f@oss.qualcomm.com?part=3