Re: [PATCHv2 1/4] dma: fsl_raid: convert descriptor stores to big-endian

[sashiko-bot@kernel.org]

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

New issues:
- [High] Bitwise OR operation on a __be32 field using a host-endian constant introduces data corruption.

Pre-existing issues:
- [High] Incomplete endianness conversion for command descriptor blocks leaves the driver broken on little-endian platforms.
--

--- Patch [1]: [PATCHv2 1/4] dma: fsl_raid: convert descriptor stores to big-endian ---
commit 4b4dfb0a314d465959f6268650dc8a30d7c69bd2
Author: Rosen Penev <rosenp@gmail.com>

dma: fsl_raid: convert descriptor stores to big-endian

The descriptor structs (fsl_re_cmpnd_frame / fsl_re_hw_desc) are
in-memory but their fields are __be32, because the structures are handed
to the device as big-endian. The driver stored CPU-endian u32 values
into them directly, which is both wrong (the engine would see
byte-swapped lengths/addresses) and flagged by sparse as a base-type
mismatch. Wrap those stores in cpu_to_be32().

Fixes: https://lore.kernel.org/oe-kbuild-all/202008111749.yy85rFMD%25lkp@intel.com/

> diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
> index 99945845d8b5e..2778d07a05e86 100644
> --- a/drivers/dma/fsl_raid.c
> +++ b/drivers/dma/fsl_raid.c
> @@ -242,9 +242,9 @@ static void fill_cfd_frame(struct fsl_re_cmpnd_frame *cf, u8 index,
>  	u32 efrl = length & FSL_RE_CF_LENGTH_MASK;
>  
>  	efrl |= final << FSL_RE_CF_FINAL_SHIFT;
> -	cf[index].efrl32 = efrl;
> -	cf[index].addr_high = upper_32_bits(addr);
> -	cf[index].addr_low = lower_32_bits(addr);
> +	cf[index].efrl32 = cpu_to_be32(efrl);

[Severity: High]
Since this function now writes a big-endian value to efrl32, doesn't this
cause a regression later when a host-endian bitwise OR is applied?

In fsl_re_prep_dma_genq():

	/* Setting the final bit in the last source buffer frame in CFD */
	cf[i - 1].efrl32 |= 1 << FSL_RE_CF_FINAL_SHIFT;

Since efrl32 now contains a byte-swapped value on little-endian architectures,
won't this host-endian bitwise OR place the final flag in the wrong byte and
corrupt the lower bits of the length field? 

The same pattern also occurs in fsl_re_prep_dma_pq().

[Severity: High]
This is a pre-existing issue, but while you are fixing endianness, should the
cdb32 field in the command descriptor blocks also be converted?

For example, in fsl_re_prep_dma_genq():

	xor = desc->cdb_addr;
	xor->cdb32 = cdb;

And similarly in fsl_re_prep_dma_pq() and fsl_re_prep_dma_memcpy(), a
host-endian u32 is assigned directly to the __be32 cdb32 field. Doesn't this
leave the driver broken on little-endian platforms?

> +	cf[index].addr_high = cpu_to_be32(upper_32_bits(addr));
> +	cf[index].addr_low = cpu_to_be32(lower_32_bits(addr));
>  }

[ ... ]

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