linux-kernel - Re: [PATCH v3 1/4] x86/clear_page: extend clear

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: <pf2p3ugs3blztd5jtxuwrg3hc3qldc4a7lfpigf24tit5noyik@67qhychq2b77>
Date: Tue, 15 Apr 2025 00:26:05 +0200
From: Mateusz Guzik <mjguzik@...il.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Ingo Molnar <mingo@...nel.org>, Ankur Arora <ankur.a.arora@...cle.com>, 
	linux-kernel@...r.kernel.org, linux-mm@...ck.org, x86@...nel.org, 
	torvalds@...ux-foundation.org, akpm@...ux-foundation.org, bp@...en8.de, 
	dave.hansen@...ux.intel.com, hpa@...or.com, mingo@...hat.com, luto@...nel.org, 
	paulmck@...nel.org, rostedt@...dmis.org, tglx@...utronix.de, willy@...radead.org, 
	jon.grimm@....com, bharata@....com, raghavendra.kt@....com, 
	boris.ostrovsky@...cle.com, konrad.wilk@...cle.com
Subject: Re: [PATCH v3 1/4] x86/clear_page: extend clear_page*() for
 multi-page clearing

On Mon, Apr 14, 2025 at 01:02:59PM +0200, Peter Zijlstra wrote:
> This symbol is written as a C function with C calling convention, even
> though it is only meant to be called from that clear_page() alternative.
> 
> If we want to go change all this, then we should go do the same we do
> for __clear_user() and write it thusly:
> 
> 	asm volatile(ALTERNATIVE("rep stosb",
> 				 "call rep_stos_alternative", ALT_NOT(X86_FEATURE_FSRS)
> 				 : "+c" (size), "+D" (addr), ASM_CALL_CONSTRAINT
> 				 : "a" (0))
> 
> And forget about all those clear_page_*() thingies.
> 

I have to disagree.

Next to nobody has FSRS, so for now one would have to expect everyone
would be punting to the routine. Did you mean ERMS as sizes are in fact
not short?

rep_stos_alternative() as implemented right now sucks in its own right
("small" areas sorted out with an 8 byte and 1 byte loops, bigger ones
unrolled 64 byte loop at a time, no rep stos{b,q} in sight). Someone(tm)
should fix it and for the sake of argument suppose it happened. That's
still some code executed to figure out how to zero and to align the buf.

Instead, I think one can start with just retiring clear_page_orig().

With that sucker out of the way, an optional quest is to figure out if
rep stosq vs rep stosb makes any difference for pages -- for all I know
rep stosq is the way. This would require testing on quite a few uarchs
and I'm not going to blame anyone for not being interested.

Let's say nobody bothered OR rep stosb provides a win. In that case this
can trivially ALTERNATIVE between rep stosb and rep stosq based on ERMS,
no func calls necessary.