linux-kernel - RE: [PATCH V7 3/4] i3c: master: Add endianness support for i3c_readl_fifo() and i3c_writel

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
 <DM4PR12MB61098EA538FCB7CED2E5C47B8C1CA@DM4PR12MB6109.namprd12.prod.outlook.com>
Date: Wed, 24 Sep 2025 09:00:52 +0000
From: "Guntupalli, Manikanta" <manikanta.guntupalli@....com>
To: Arnd Bergmann <arnd@...db.de>, "git (AMD-Xilinx)" <git@....com>, "Simek,
 Michal" <michal.simek@....com>, Alexandre Belloni
	<alexandre.belloni@...tlin.com>, Frank Li <Frank.Li@....com>, Rob Herring
	<robh@...nel.org>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>, Conor Dooley
	<conor+dt@...nel.org>, Przemysław Gaj <pgaj@...ence.com>,
	Wolfram Sang <wsa+renesas@...g-engineering.com>,
	"tommaso.merciai.xr@...renesas.com" <tommaso.merciai.xr@...renesas.com>,
	"quic_msavaliy@...cinc.com" <quic_msavaliy@...cinc.com>, "S-k, Shyam-sundar"
	<Shyam-sundar.S-k@....com>, Sakari Ailus <sakari.ailus@...ux.intel.com>,
	"'billy_tsai@...eedtech.com'" <billy_tsai@...eedtech.com>, Kees Cook
	<kees@...nel.org>, "Gustavo A. R. Silva" <gustavoars@...nel.org>, Jarkko
 Nikula <jarkko.nikula@...ux.intel.com>, Jorge Marques
	<jorge.marques@...log.com>, "linux-i3c@...ts.infradead.org"
	<linux-i3c@...ts.infradead.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, Linux-Arch <linux-arch@...r.kernel.org>,
	"linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org>
CC: "Pandey, Radhey Shyam" <radhey.shyam.pandey@....com>, "Goud, Srinivas"
	<srinivas.goud@....com>, "Datta, Shubhrajyoti" <shubhrajyoti.datta@....com>,
	"manion05gk@...il.com" <manion05gk@...il.com>
Subject: RE: [PATCH V7 3/4] i3c: master: Add endianness support for
 i3c_readl_fifo() and i3c_writel_fifo()

[Public]

Hi,

> -----Original Message-----
> From: Arnd Bergmann <arnd@...db.de>
> Sent: Wednesday, September 24, 2025 12:21 AM
> To: Guntupalli, Manikanta <manikanta.guntupalli@....com>; git (AMD-Xilinx)
> <git@....com>; Simek, Michal <michal.simek@....com>; Alexandre Belloni
> <alexandre.belloni@...tlin.com>; Frank Li <Frank.Li@....com>; Rob Herring
> <robh@...nel.org>; krzk+dt@...nel.org; Conor Dooley <conor+dt@...nel.org>;
> Przemysław Gaj <pgaj@...ence.com>; Wolfram Sang <wsa+renesas@...g-
> engineering.com>; tommaso.merciai.xr@...renesas.com;
> quic_msavaliy@...cinc.com; S-k, Shyam-sundar <Shyam-sundar.S-k@....com>;
> Sakari Ailus <sakari.ailus@...ux.intel.com>; 'billy_tsai@...eedtech.com'
> <billy_tsai@...eedtech.com>; Kees Cook <kees@...nel.org>; Gustavo A. R. Silva
> <gustavoars@...nel.org>; Jarkko Nikula <jarkko.nikula@...ux.intel.com>; Jorge
> Marques <jorge.marques@...log.com>; linux-i3c@...ts.infradead.org;
> devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; Linux-Arch <linux-
> arch@...r.kernel.org>; linux-hardening@...r.kernel.org
> Cc: Pandey, Radhey Shyam <radhey.shyam.pandey@....com>; Goud, Srinivas
> <srinivas.goud@....com>; Datta, Shubhrajyoti <shubhrajyoti.datta@....com>;
> manion05gk@...il.com
> Subject: Re: [PATCH V7 3/4] i3c: master: Add endianness support for i3c_readl_fifo()
> and i3c_writel_fifo()
>
> On Tue, Sep 23, 2025, at 17:45, Manikanta Guntupalli wrote:
> >  /**
> >   * i3c_writel_fifo - Write data buffer to 32bit FIFO
> >   * @addr: FIFO Address to write to
> >   * @buf: Pointer to the data bytes to write
> >   * @nbytes: Number of bytes to write
> > + * @endian: Endianness of FIFO write
> >   */
> >  static inline void i3c_writel_fifo(void __iomem *addr, const void *buf,
> > -                              int nbytes)
> > +                              int nbytes, enum i3c_fifo_endian endian)
> >  {
> > -   writesl(addr, buf, nbytes / 4);
> > +   if (endian)
> > +           writesl_be(addr, buf, nbytes / 4);
> > +   else
> > +           writesl(addr, buf, nbytes / 4);
> > +
>
> This seems counter-intuitive: a FIFO doesn't really have an endianness, it is instead
> used to transfer a stream of bytes, so if the device has a fixed endianess, the FIFO
> still needs to be read using a plain writesl().
>
> I see that your writesl_be() has an incorrect definition, which would lead to the
> i3c_writel_fifo() function accidentally still working if both the device and CPU use
> big-endian registers:
>
> static inline void writesl_be(volatile void __iomem *addr,
>                             const void *buffer,
>                             unsigned int count)
> {
>       if (count) {
>               const u32 *buf = buffer;
>               do {
>                       __raw_writel((u32 __force)__cpu_to_be32(*buf), addr);
>                       buf++;
>               } while (--count);
>       }
> }
>
> The __cpu_to_be32() call that you add here means that the FIFO data is swapped
> on little-endian CPUs but not swapped on big-endian ones. Compare this to the
> normal writesl() function that never swaps because it writes a byte stream.
The use of __cpu_to_be32() in writesl_be() is intentional. The goal here is to ensure that the FIFO is always accessed in big-endian format, regardless of whether the CPU is little-endian or big-endian. On little-endian CPUs, this introduces the required byte swap before writing; on big-endian CPUs, the data is already in big-endian order, so no additional swap occurs.
This guarantees that the FIFO sees consistent big-endian data, matching the hardware's expectation.

>
> >     if (nbytes & 3) {
> >             u32 tmp = 0;
> >
> >             memcpy(&tmp, buf + (nbytes & ~3), nbytes & 3);
> > -           writel(tmp, addr);
> > +
> > +           if (endian)
> > +                   writel_be(tmp, addr);
> > +           else
> > +                   writel(tmp, addr);
>
> This bit however seems to fix a bug, but does so in a confusing way. The way the
> FIFO registers usually deal with excess bytes is to put them into the first bytes of the
> FIFO register, so this should just be a
>
>        writesl(addr, &tmp, 1);
>
> to write one set of four bytes into the FIFO without endian-swapping.
>
> Could it be that you are just trying to use a normal i3c adapter with little-endian
> registers on a normal big-endian machine but ran into this bug?
The intention here is to enforce big-endian ordering for the trailing bytes as well. By using __cpu_to_be32() for full words and writel_be() for the leftover word, the FIFO is always accessed in big-endian format, regardless of the CPU's native endianness. This ensures consistent and correct data ordering from the device's perspective.

Thanks,
Manikanta.