linux-kernel - Re: [PATCH] powerpc: Remove eieio in _memcpy

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] [day] [month] [year] [list]

Message-Id: <9fc9a994-b6cf-4ad9-9cec-6777dedd216d@app.fastmail.com>
Date: Tue, 28 Jan 2025 17:50:09 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: "Christophe Leroy" <christophe.leroy@...roup.eu>,
 "Julian Vetter" <julian@...er-limits.org>,
 "Madhavan Srinivasan" <maddy@...ux.ibm.com>,
 "Michael Ellerman" <mpe@...erman.id.au>,
 "Nicholas Piggin" <npiggin@...il.com>, "Naveen N Rao" <naveen@...nel.org>
Cc: linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] powerpc: Remove eieio in _memcpy_fromio

On Tue, Jan 28, 2025, at 16:34, Christophe Leroy wrote:
> Le 28/01/2025 à 16:24, Christophe Leroy a écrit :
>> Le 28/01/2025 à 16:07, Julian Vetter a écrit :
>>> With 'ppc' I was refering to 'include/asm-ppc/io.h'. But you're right,
>>> when going back a bit, in the 'include/asm-powerpc/io.h' there are two
>>> cases, one (eeh_memcpy_fromio) which does the the 'eieio', and a second,
>>> i.e., 'iSeries_memcpy_fromio' which does a byte-wise copy. But in the
>>> ppc code ('include/asm-ppc/io.h') there is a simple memcpy. I was
>>> referring to this one. But my description is not very clear. Sorry for 
>>> that.
>> 
>> But then is your change still valid ? Isn't there some corner case that 
>> still need it ? Is it a valid argument that because memcpy_toio() 
>> doesn't need it memcpy_fromio() doesn't need it either ?
>
> I see that _insb(), _insw_ns() and _insl_ns() also have eieio() while 
> _outsb(), _outsw_ns() and _outsl_ns() don't. Why not change those as 
> well if you think eieio() is not needed ?

I think that makes sense, even if it's beyond the scope of Julian's
work to unify the memcpy/memset I/O helpers across architectures.

I looked into the pre-2.6.12 history of arch/powerpc64 to see how
the eieio got in there originally and found that at the time the
string functions got added, this is what the readl() etc functions
did. readl() itself went through a longer set of changes to end
up with the current sync/twi/isync version, but the string functions
were never updated again during any of the later changes.

The bit that needs to be captured in the changelog here is that
on all other architectures, strcpy_fromio/strcpy_toio are written
to allow prefetching/combining/reordering, while the powerpc
version prevents this in strcpy_fromio for apparently only history
reasons.

     Arnd