[PATCH v7] dmaengine: qcom: bam_dma: Fix command element mask field for BAM v1.6.0+

[PATCH v7] dmaengine: qcom: bam_dma: Fix command element mask field for BAM v1.6.0+

[Varadarajan Narayanan]

From: Md Sadre Alam <md.alam@oss.qualcomm.com>

BAM version 1.6.0 and later changed the behavior of the mask field in
command elements for read operations.

In older BAM versions, or prior implementation assumptions, the mask
field was effectively ignored for read commands. However, starting from
BAM v1.6.0, the mask field for read commands is repurposed to carry the
upper 4 bits of the destination address, enabling support for 36-bit
addressing. For write commands, the mask field continues to function as
a traditional write mask.

The current driver sets mask = 0xffffffff for all command elements.
While this works for write operations, it breaks read operations on
BAM v1.6.0+ hardware. In such cases, the hardware interprets the upper
address bits as 0xf, resulting in an invalid destination address
(0xf_xxxxxxxx instead of 0x0_xxxxxxxx).

This leads to failures such as NAND enumeration issues observed on
platforms like IPQ5424.

Fix this by assigning the mask field based on command type:
  - For read commands: set mask = 0 (upper address bits = 0)
  - For write commands: retain mask = 0xffffffff

Also update the bam_cmd_element structure documentation to reflect the
dual purpose of the mask field across BAM versions.

This ensures correct behavior on BAM v1.6.0+ while maintaining backward
compatibility with older hardware.

Fixes: dfebb055f73a2 ("dmaengine: qcom: bam_dma: wrapper functions for command descriptor")
Tested-by: Lakshmi Sowjanya D <lakshmi.d@oss.qualcomm.com>
Signed-off-by: Md Sadre Alam <md.alam@oss.qualcomm.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Cc: stable@vger.kernel.org
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
Change in [v7] -
	- Remove blank line after 'Fixes' tag
	- Add 'Cc: stable@vger.kernel.org'
	- Add R-b Dmitry Baryshkov
	- No code changes

Change in [v6] - https://lore.kernel.org/linux-arm-msm/20260611045757.2841252-1-varadarajan.narayanan@oss.qualcomm.com/
	- Commit message updated, no code changes
	- Pick R-b Frank Li (given in v4)
	- Change 'Lakshmi Sowjanya D' e-mail id to oss.qualcomm.com instead of quicinc.com

Change in [v5] - https://lore.kernel.org/linux-arm-msm/20260514-bam-fix-v5-1-58f6edb34969@oss.qualcomm.com/#t
	- Split the driver change into a separate patch
	- Update commit log with 'Fixes' tag

Change in [v4] - https://lore.kernel.org/linux-arm-msm/20260206100202.413834-2-quic_mdalam@quicinc.com/

* No change

Change in [v3]

* Added Tested-by tag

Change in [v2]

* No change

Change in [v1]

* Updated bam_prep_ce_le32() to set the mask field conditionally based on
  command type

* Enhanced kernel-doc comments to clarify mask behavior for BAM v1.6.0+
---
 include/linux/dma/qcom_bam_dma.h | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/include/linux/dma/qcom_bam_dma.h b/include/linux/dma/qcom_bam_dma.h
index 68fc0e643b1b..d9d07a9ab313 100644
--- a/include/linux/dma/qcom_bam_dma.h
+++ b/include/linux/dma/qcom_bam_dma.h
@@ -13,9 +13,12 @@
  * supported by BAM DMA Engine.
  *
  * @cmd_and_addr - upper 8 bits command and lower 24 bits register address.
- * @data - for write command: content to be written into peripheral register.
- *	   for read command: dest addr to write peripheral register value.
- * @mask - register mask.
+ * @data - For write command: content to be written into peripheral register.
+ *	   For read command: lower 32 bits of destination address.
+ * @mask - For write command: register write mask.
+ *	   For read command on BAM v1.6.0+: upper 4 bits of destination address.
+ *	   For read command on BAM < v1.6.0: ignored by hardware.
+ *	   Setting to 0 ensures 32-bit addressing compatibility.
  * @reserved - for future usage.
  *
  */
@@ -42,6 +45,10 @@ enum bam_command_type {
  * @addr: target address
  * @cmd: BAM command
  * @data: actual data for write and dest addr for read in le32
+ *
+ * For BAM v1.6.0+, the mask field behavior depends on command type:
+ * - Write commands: mask = write mask (typically 0xffffffff)
+ * - Read commands: mask = upper 4 bits of destination address (0 for 32-bit)
  */
 static inline void
 bam_prep_ce_le32(struct bam_cmd_element *bam_ce, u32 addr,
@@ -50,7 +57,11 @@ bam_prep_ce_le32(struct bam_cmd_element *bam_ce, u32 addr,
 	bam_ce->cmd_and_addr =
 		cpu_to_le32((addr & 0xffffff) | ((cmd & 0xff) << 24));
 	bam_ce->data = data;
-	bam_ce->mask = cpu_to_le32(0xffffffff);
+	if (cmd == BAM_READ_COMMAND)
+		bam_ce->mask = cpu_to_le32(0x0); /* 32-bit addressing */
+	else
+		bam_ce->mask = cpu_to_le32(0xffffffff); /* Write mask */
+	bam_ce->reserved = 0;
 }
 
 /*
@@ -60,7 +71,7 @@ bam_prep_ce_le32(struct bam_cmd_element *bam_ce, u32 addr,
  * @bam_ce: BAM command element
  * @addr: target address
  * @cmd: BAM command
- * @data: actual data for write and dest addr for read
+ * @data: actual data for write and destination address for read
  */
 static inline void
 bam_prep_ce(struct bam_cmd_element *bam_ce, u32 addr,
-- 
2.34.1

Re: [PATCH v7] dmaengine: qcom: bam_dma: Fix command element mask field for BAM v1.6.0+

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The QPIC NAND driver explicitly restricts the coherent DMA mask to 32 bits but fails to restrict the streaming DMA mask, allowing streaming mappings to return addresses >4GB which are then silently truncated to 32 bits, causing DMA memory corruption.
--

commit d15e0af034d65bcfce2ab8e3f8608f0913f11a7c
Author: Md Sadre Alam <md.alam@oss.qualcomm.com>

dmaengine: qcom: bam_dma: Fix command element mask field for BAM v1.6.0+

This commit updates the BAM DMA driver to correctly handle the command
element mask field for BAM v1.6.0 and later. For read commands, the mask is
now set to 0 to support 32-bit addressing, while write commands retain the
0xffffffff mask.

> diff --git a/include/linux/dma/qcom_bam_dma.h b/include/linux/dma/qcom_bam_dma.h
> index 68fc0e643b1b9..d9d07a9ab3132 100644
> --- a/include/linux/dma/qcom_bam_dma.h
> +++ b/include/linux/dma/qcom_bam_dma.h
[ ... ]
> @@ -50,7 +57,11 @@ bam_prep_ce_le32(struct bam_cmd_element *bam_ce, u32 addr,
>  	bam_ce->cmd_and_addr =
>  		cpu_to_le32((addr & 0xffffff) | ((cmd & 0xff) << 24));
>  	bam_ce->data = data;
> -	bam_ce->mask = cpu_to_le32(0xffffffff);
> +	if (cmd == BAM_READ_COMMAND)
> +		bam_ce->mask = cpu_to_le32(0x0); /* 32-bit addressing */

[Severity: Critical]
This is a pre-existing issue, but does hardcoding the upper address bits
to zero here cement a silent truncation bug caused by the QPIC NAND driver?

In drivers/mtd/nand/qpic_common.c:qcom_nandc_alloc(), the driver sets the
coherent DMA mask to 32 bits, but does not restrict the streaming DMA mask:

	ret = dma_set_coherent_mask(nandc->dev, DMA_BIT_MASK(32));
	if (ret) {
		dev_err(nandc->dev, "failed to set DMA mask\n");
		return ret;
	}

On modern 64-bit systems, dma_map_single() can map streaming buffers to
physical addresses above 4GB. When qcom_prep_bam_dma_desc_cmd() prepares the
BAM descriptor:

	if (read)
		bam_prep_ce(&bam_ce_buffer[i],
			    offset, BAM_READ_COMMAND,
			    reg_buf_dma_addr(nandc, (__le32 *)vaddr + i));

The potentially >4GB physical address is passed as the 'data' argument to
bam_prep_ce(). However, 'data' is strongly typed as u32, silently dropping
the upper 32 bits in C:

static inline void
bam_prep_ce(struct bam_cmd_element *bam_ce, u32 addr,
	    enum bam_command_type cmd, u32 data)

With this patch explicitly hardcoding bam_ce->mask to 0 (upper 4 bits), the
BAM hardware will unconditionally execute the DMA write at the incorrect,
truncated 32-bit physical address. Could this lead to silent memory
corruption instead of raising a fault when a buffer resides above 4GB?

Should the QPIC NAND driver be updated to use dma_set_mask_and_coherent()
to properly restrict both masks and ensure the use of DMA bounce buffers?

> +	else
> +		bam_ce->mask = cpu_to_le32(0xffffffff); /* Write mask */
> +	bam_ce->reserved = 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615060908.1263171-1-varadarajan.narayanan@oss.qualcomm.com?part=1