linux-kernel - Re: [PATCH v4 3/4] i2c: imx: use readb_relaxed and writeb

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: <Zv1FuHlpeayZq-Zv@eichest-laptop>
Date: Wed, 2 Oct 2024 15:08:08 +0200
From: Stefan Eichenberger <eichest@...il.com>
To: Arnd Bergmann <arnd@...db.de>
Cc: o.rempel@...gutronix.de,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Andi Shyti <andi.shyti@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>, Frank Li <Frank.Li@....com>,
	linux-i2c@...r.kernel.org, imx@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Francesco Dolcini <francesco.dolcini@...adex.com>,
	Stefan Eichenberger <stefan.eichenberger@...adex.com>
Subject: Re: [PATCH v4 3/4] i2c: imx: use readb_relaxed and writeb_relaxed

On Wed, Oct 02, 2024 at 11:51:22AM +0000, Arnd Bergmann wrote:
> On Wed, Oct 2, 2024, at 11:19, Stefan Eichenberger wrote:
> > From: Stefan Eichenberger <stefan.eichenberger@...adex.com>
> >
> > Use the relaxed version of readb and writeb to reduce overhead. It is
> > safe to use the relaxed version because we either do not rely on dma
> > completion, or we use a dma callback to ensure that the dma transfer is
> > complete before we continue.
> 
> I would still consider this a bug in general, you should
> never default to the unsafe variants.
> 
> If there is a codepath that needs the barrierless version,
> please add imx_i2c_write_reg_relaxed()/imx_i2c_read_reg_relaxed()
> helpers that use those only in the places where it makes
> a measurable difference, with a comment that explains
> the usage.

I added the patch because of the following dicussion:
https://lore.kernel.org/linux-i2c/ZpVWXlR6j2i0ZtVQ@lizhi-Precision-Tower-5810/

I can't determine if the relaxed version improves performance. The
'normal' version worked well for our use case too. Therefore, dropping
the change would be acceptable for us. Another potential solution could
be to use the relaxed version only inside the ISR. Would that be an
acceptable solution? What is your impression, Frank Li
<Frank.Li@....com>?

Regards,
Stefan