[PATCHv2] dmaengine: fsl_raid: switch __b32 to u32

[PATCHv2] dmaengine: fsl_raid: switch __b32 to u32

[Rosen Penev]

The register maps in fsl_raid.h annotate every field as __be32, but
they are accessed only through ioread32be()/iowrite32be(), which
already perform the byte swap and take plain u32 __iomem pointers.
The __be32 annotation therefore makes sparse reject each access:

  drivers/dma/fsl_raid.c:116:17: sparse: incorrect type in argument 1
      (different base types) ... got restricted __be32 [noderef] __iomem *

Drop __be32 from struct fsl_re_ctrl and struct fsl_re_chan_cfg; the
descriptor/CDB structures written via cpu_to_be32() keep their
annotations.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202608200637.ro1CqMxi-lkp@intel.com/
Fixes: 68b7fbc23529 ("dma: fsl_raid: keep MMIO bases as void __iomem and cast at access")
Assisted-by: opencode:deepseek-v4-flash-free
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 v2: split patch up.
 drivers/dma/fsl_raid.h | 54 +++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/dma/fsl_raid.h b/drivers/dma/fsl_raid.h
index adbfede330a7..6fde5192ab77 100644
--- a/drivers/dma/fsl_raid.h
+++ b/drivers/dma/fsl_raid.h
@@ -110,59 +110,59 @@
 
 struct fsl_re_ctrl {
 	/* General Configuration Registers */
-	__be32 global_config;	/* Global Configuration Register */
+	u32 global_config;	/* Global Configuration Register */
 	u8     rsvd1[4];
-	__be32 galois_field_config; /* Galois Field Configuration Register */
+	u32 galois_field_config; /* Galois Field Configuration Register */
 	u8     rsvd2[4];
-	__be32 jq_wrr_config;   /* WRR Configuration register */
+	u32 jq_wrr_config;	/* WRR Configuration register */
 	u8     rsvd3[4];
-	__be32 crc_config;	/* CRC Configuration register */
+	u32 crc_config;		/* CRC Configuration register */
 	u8     rsvd4[228];
-	__be32 system_reset;	/* System Reset Register */
+	u32 system_reset;	/* System Reset Register */
 	u8     rsvd5[252];
-	__be32 global_status;	/* Global Status Register */
+	u32 global_status;	/* Global Status Register */
 	u8     rsvd6[832];
-	__be32 re_liodn_base;	/* LIODN Base Register */
+	u32 re_liodn_base;	/* LIODN Base Register */
 	u8     rsvd7[1712];
-	__be32 re_version_id;	/* Version ID register of RE */
-	__be32 re_version_id_2; /* Version ID 2 register of RE */
+	u32 re_version_id;	/* Version ID register of RE */
+	u32 re_version_id_2;	/* Version ID 2 register of RE */
 	u8     rsvd8[512];
-	__be32 host_config;	/* Host I/F Configuration Register */
+	u32 host_config;	/* Host I/F Configuration Register */
 };
 
 struct fsl_re_chan_cfg {
 	/* Registers for JR interface */
-	__be32 jr_config_0;	/* Job Queue Configuration 0 Register */
-	__be32 jr_config_1;	/* Job Queue Configuration 1 Register */
-	__be32 jr_interrupt_status; /* Job Queue Interrupt Status Register */
+	u32 jr_config_0;	/* Job Queue Configuration 0 Register */
+	u32 jr_config_1;	/* Job Queue Configuration 1 Register */
+	u32 jr_interrupt_status; /* Job Queue Interrupt Status Register */
 	u8     rsvd1[4];
-	__be32 jr_command;	/* Job Queue Command Register */
+	u32 jr_command;		/* Job Queue Command Register */
 	u8     rsvd2[4];
-	__be32 jr_status;	/* Job Queue Status Register */
+	u32 jr_status;		/* Job Queue Status Register */
 	u8     rsvd3[228];
 
 	/* Input Ring */
-	__be32 inbring_base_h;	/* Inbound Ring Base Address Register - High */
-	__be32 inbring_base_l;	/* Inbound Ring Base Address Register - Low */
-	__be32 inbring_size;	/* Inbound Ring Size Register */
+	u32 inbring_base_h;	/* Inbound Ring Base Address Register - High */
+	u32 inbring_base_l;	/* Inbound Ring Base Address Register - Low */
+	u32 inbring_size;	/* Inbound Ring Size Register */
 	u8     rsvd4[4];
-	__be32 inbring_slot_avail; /* Inbound Ring Slot Available Register */
+	u32 inbring_slot_avail; /* Inbound Ring Slot Available Register */
 	u8     rsvd5[4];
-	__be32 inbring_add_job;	/* Inbound Ring Add Job Register */
+	u32 inbring_add_job;	/* Inbound Ring Add Job Register */
 	u8     rsvd6[4];
-	__be32 inbring_cnsmr_indx; /* Inbound Ring Consumer Index Register */
+	u32 inbring_cnsmr_indx; /* Inbound Ring Consumer Index Register */
 	u8     rsvd7[220];
 
 	/* Output Ring */
-	__be32 oubring_base_h;	/* Outbound Ring Base Address Register - High */
-	__be32 oubring_base_l;	/* Outbound Ring Base Address Register - Low */
-	__be32 oubring_size;	/* Outbound Ring Size Register */
+	u32 oubring_base_h;	/* Outbound Ring Base Address Register - High */
+	u32 oubring_base_l;	/* Outbound Ring Base Address Register - Low */
+	u32 oubring_size;	/* Outbound Ring Size Register */
 	u8     rsvd8[4];
-	__be32 oubring_job_rmvd; /* Outbound Ring Job Removed Register */
+	u32 oubring_job_rmvd;	/* Outbound Ring Job Removed Register */
 	u8     rsvd9[4];
-	__be32 oubring_slot_full; /* Outbound Ring Slot Full Register */
+	u32 oubring_slot_full;	/* Outbound Ring Slot Full Register */
 	u8     rsvd10[4];
-	__be32 oubring_prdcr_indx; /* Outbound Ring Producer Index */
+	u32 oubring_prdcr_indx;	/* Outbound Ring Producer Index */
 };
 
 /*
-- 
2.55.0

Re: [PATCHv2] dmaengine: fsl_raid: switch __b32 to u32

[sashiko-bot]

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

Pre-existing issues:
- [High] The commit message claims that the CDB structures keep their `__be32` annotations because they are correctly written via `cpu_to_be32()`. However, the driver directly assigns native `u32` values to the `cdb32` fields of the XOR, PQ, and MOVE structures without using `cpu_to_be32()`, violating the claim and leaving an endianness bug on little-endian architectures.
- [High] Lock held while invoking client DMA callback, causing a self-deadlock if the client submits a new transaction.
- [High] Missing `free_irq()` leads to IRQ leak and potential use-after-free.
- [High] Erroneous bitwise logic for splitting ring base addresses causes silent truncation.
--

commit 7b9c559b3810d3491f22a6353857e63e9743b1e1
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: fsl_raid: switch __b32 to u32

The register maps in fsl_raid.h annotate every field as __be32, but
they are accessed only through ioread32be()/iowrite32be(), which
already perform the byte swap and take plain u32 __iomem pointers.
The __be32 annotation therefore makes sparse reject each access:

  drivers/dma/fsl_raid.c:116:17: sparse: incorrect type in argument 1
      (different base types) ... got restricted __be32 [noderef] __iomem *

Drop __be32 from struct fsl_re_ctrl and struct fsl_re_chan_cfg; the
descriptor/CDB structures written via cpu_to_be32() keep their
annotations.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202608200637.ro1CqMxi-lkp@intel.com/
Fixes: 68b7fbc23529 ("dma: fsl_raid: keep MMIO bases as void __iomem and cast at access")
Assisted-by: opencode:deepseek-v4-flash-free
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Drop __be32 from struct fsl_re_ctrl and struct fsl_re_chan_cfg; the
> descriptor/CDB structures written via cpu_to_be32() keep their
> annotations.

[Severity: High]
This isn't a bug introduced by this patch, but the commit message claims
that the CDB structures keep their __be32 annotations because they are
correctly written via cpu_to_be32().

Is this description accurate? Looking at fsl_re_prep_dma_genq(), the driver
directly assigns native u32 values to the cdb32 field of the XOR structure
without using cpu_to_be32():

drivers/dma/fsl_raid.c:fsl_re_prep_dma_genq() {
    ...
    xor->cdb32 = cdb;
    ...
}

Similar direct assignments occur in fsl_re_prep_dma_pq() for pq->cdb32
and fsl_re_prep_dma_memcpy() for move->cdb32.

Could this lead to the DMA hardware receiving a byteswapped command
descriptor block on little-endian architectures, causing it to misinterpret
opcodes and potentially corrupt memory?


[Severity: High]
This is a pre-existing issue, but can a self-deadlock occur if the client
callback submits a new transaction?

drivers/dma/fsl_raid.c:fsl_re_tx_submit() {
    ...
    spin_lock_irqsave(&re_chan->desc_lock, flags);
    ...
}

drivers/dma/fsl_raid.c:fsl_re_dequeue() {
    ...
    fsl_re_desc_done(desc);
    ...
}

If a client submits a new DMA transaction from the callback triggered by
fsl_re_desc_done() while re_chan->desc_lock is still held, fsl_re_tx_submit()
will unconditionally attempt to acquire the exact same lock again. Does this
result in a complete system hang?


[Severity: High]
This is a pre-existing issue, but does this code leak the IRQ and potentially
cause a use-after-free?

drivers/dma/fsl_raid.c:fsl_re_chan_probe() {
    ...
    ret = request_irq(chan->irq, fsl_re_isr, 0, chan->name, chandev);
    ...
}

drivers/dma/fsl_raid.c:fsl_re_remove_chan() {
    ...
    tasklet_kill(&chan->irqtask);
    ...
}

The teardown function kills the tasklet but appears to miss the required
free_irq() call. If the module is unloaded or the device is unbound, and an
interrupt fires after the re_chan structure is freed, will the ISR
fsl_re_isr() dereference the freed pointer?


[Severity: High]
This is a pre-existing issue, but is the bitwise logic for splitting ring
base addresses correct here?

drivers/dma/fsl_raid.c:fsl_re_chan_probe() {
    ...
    out_be32(&chan->jrregs->inbring_base_h,
             chan->inb_phys_addr & FSL_RE_ADDR_BIT_MASK);
    out_be32(&chan->jrregs->oubring_base_h,
             chan->oub_phys_addr & FSL_RE_ADDR_BIT_MASK);
    out_be32(&chan->jrregs->inbring_base_l,
             chan->inb_phys_addr >> FSL_RE_ADDR_BIT_SHIFT);
    ...
}

This logic appears to use an incorrect mask and shift, truncating the high
bits and completely shifting out the low bits. Could this result in the DMA
rings being placed at the wrong physical memory locations, leading to
immediate DMA failure or memory corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910194616.34760-1-rosenp@gmail.com?part=1