linux-kernel - Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers

Open Source and information security mailing list archives

Message-ID: <aNVbHeky1X58EYT1@lizhi-Precision-Tower-5810>
Date: Thu, 25 Sep 2025 11:09:17 -0400
From: Frank Li <Frank.li@....com>
To: Arnd Bergmann <arnd@...nel.org>
Cc: Alexandre Belloni <alexandre.belloni@...tlin.com>,
	Jorge Marques <jorge.marques@...log.com>,
	Wolfram Sang <wsa+renesas@...g-engineering.com>,
	Arnd Bergmann <arnd@...db.de>,
	Manikanta Guntupalli <manikanta.guntupalli@....com>,
	linux-i3c@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers

On Wed, Sep 24, 2025 at 10:18:33PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.de>
>
> Short MMIO transfers that are not a multiple of four bytes in size need
> a special case for the final bytes, however the existing implementation
> is not endian-safe and introduces an incorrect byteswap on big-endian
> kernels.
>
> This usually does not cause problems because most systems are
> little-endian and most transfers are multiple of four bytes long, but
> still needs to be fixed to avoid the extra byteswap.
>
> Change the special case for both i3c_writel_fifo() and i3c_readl_fifo()
> to use non-byteswapping writesl() and readsl() with a single element
> instead of the byteswapping writel()/readl() that are meant for individual
> MMIO registers. As data is copied between a FIFO and a memory buffer,
> the writesl()/readsl() loops are typically based on __raw_readl()/
> __raw_writel(), resulting in the order of bytes in the FIFO to match
> the order in the buffer, regardless of the CPU endianess.
>
> The earlier versions in the dw-i3c and i3c-master-cdns had a correct
> implementation, but the generic version that was recently added broke it.
>
> Fixes: 733b439375b4 ("i3c: master: Add inline i3c_readl_fifo() and i3c_writel_fifo()")
> Cc: Manikanta Guntupalli <manikanta.guntupalli@....com>
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
> This was a recent regression, the version in 6.16 still works,
> but 6.17-rc is broken.
>
> v2 changes:
>  - add code comments
>  - write correct data buffer
> ---
>  drivers/i3c/internals.h | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
> index 0d857cc68cc5..79ceaa5f5afd 100644
> --- a/drivers/i3c/internals.h
> +++ b/drivers/i3c/internals.h
> @@ -38,7 +38,11 @@ static inline void i3c_writel_fifo(void __iomem *addr, const void *buf,
>  		u32 tmp = 0;
>
>  		memcpy(&tmp, buf + (nbytes & ~3), nbytes & 3);
> -		writel(tmp, addr);
> +		/*
> +		 * writesl() instead of writel() to keep FIFO
> +		 * byteorder on big-endian targets
> +		 */

endian look like how CPU decode byte order to MSB to LSB.
targets FIFO define look like

BIT 31..24   23..16     15..8     7..0
    B3        B2         B1       B0

regardless CPU is big endian or little endian system, data in memory should
be

0x000 B0
0x004 B1
0x008 B2
0x00c B3

I think your sentence in commit message is better

/* writesl() instead of writel() to keep FIFO byte orderer to match the order
in the buffer regardless of the CPU endianess.
*/

Frank
> +		writesl(addr, &tmp, 1);
>  	}
>  }
>
> @@ -55,7 +59,11 @@ static inline void i3c_readl_fifo(const void __iomem *addr, void *buf,
>  	if (nbytes & 3) {
>  		u32 tmp;
>
> -		tmp = readl(addr);
> +		/*
> +		 * readsl() instead of readl() to keep FIFO
> +		 * byteorder on big-endian targets
> +		 */
> +		readsl(addr, &tmp, 1);
>  		memcpy(buf + (nbytes & ~3), &tmp, nbytes & 3);
>  	}
>  }
> --
> 2.39.5
>
>
> --
> linux-i3c mailing list
> linux-i3c@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives