linux-kernel - Re: [PATCH] Add io_sync stubs to generic IO memcpy/memset

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d7779dbd-2e09-4cd3-b27c-fc285596b995@outer-limits.org>
Date: Mon, 27 Jan 2025 15:11:25 +0100
From: Julian Vetter <julian@...er-limits.org>
To: Arnd Bergmann <arnd@...db.de>, Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Add io_sync stubs to generic IO memcpy/memset

On 1/27/25 11:27, Arnd Bergmann wrote:
> On Mon, Jan 27, 2025, at 11:04, Julian Vetter wrote:
>> The recently added IO memcpy and memset functions lack support for
>> barriers or other sync functions before and/or after the transaction. To
>> convert more architectures to use the generic IO memcpy and memset
>> functions, add empty __pre_io_sync and __post_io_sync defines that can
>> be overwritten by individual architectures if needed.
>>
>> Signed-off-by: Julian Vetter <julian@...er-limits.org>
>> ---
>>   lib/iomem_copy.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/lib/iomem_copy.c b/lib/iomem_copy.c
>> index dec7eaea60e0..2e81182dd4d3 100644
>> --- a/lib/iomem_copy.c
>> +++ b/lib/iomem_copy.c
>> @@ -9,6 +9,14 @@
>>   #include <linux/types.h>
>>   #include <linux/unaligned.h>
>>
>> +#ifndef __pre_io_sync
>> +#define __pre_io_sync
>> +#endif
>> +
>> +#ifndef __post_io_sync
>> +#define __post_io_sync
>> +#endif
> 
> I think we should define what these barriers are supposed to
> do exactly, and how they relate to the __io_br/__io_ar/__io_bw/__io_aw
> ones include/asm-generic/io.h.
> 

Thank you for your quick reply. You're right, I was just going with the
naming used in the powerpc arch which has an io_sync define. I'm now
wondering if we can't simply use the read{l,q}/write{l,q} functions
(instead of the __raw_xxx version), there are already calls to __io_br
before and__io_ar after each read (and write). But this might have
performance implications on some architectures, depending what it
resolves to.
Otherwise I propose renaming the __pre_io_sync and __post_io_sync into a
single __io_mbr which is called before and after each loop. Looking at 
PowerPC and SuperH, both of them could be consolidated into the generic 
IO memcpy code when adding this. What do you think?
The existing ones, especially __io_br unfortunately don't resolve to the 
right define on these architectures. The __io_ar and __io_br resolve to 
the right mb() on SuperH and PowerPC as well, but this would again have 
implications on other architectures.
Thank you!

Julian

> Depending on what the barriers are meant to do, we probably
> want to either use the existing ones directly or use a similar
> naming scheme.
> 
>       Arnd