linux-kernel - Re: [PATCH v4 10/13] x86/mm: Simplify clear_page

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: <b53d59de-1aba-41f0-a908-e574f3db5958@intel.com>
Date: Mon, 16 Jun 2025 07:35:48 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: Ankur Arora <ankur.a.arora@...cle.com>, linux-kernel@...r.kernel.org,
 linux-mm@...ck.org, x86@...nel.org
Cc: akpm@...ux-foundation.org, bp@...en8.de, dave.hansen@...ux.intel.com,
 hpa@...or.com, mingo@...hat.com, mjguzik@...il.com, luto@...nel.org,
 peterz@...radead.org, acme@...nel.org, namhyung@...nel.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 v4 10/13] x86/mm: Simplify clear_page_*

On 6/15/25 22:22, Ankur Arora wrote:
> clear_page_rep() and clear_page_erms() are wrappers around "REP; STOS"
> variations. Inlining gets rid of the costly call/ret (for cases with
> speculative execution related mitigations.)

Could you elaborate a bit on which "speculative execution related
mitigations" are so costly with these direct calls?


> -	kmsan_unpoison_memory(page, PAGE_SIZE);
> -	alternative_call_2(clear_page_orig,
> -			   clear_page_rep, X86_FEATURE_REP_GOOD,
> -			   clear_page_erms, X86_FEATURE_ERMS,
> -			   "=D" (page),
> -			   "D" (page),
> -			   "cc", "memory", "rax", "rcx");

I've got to say, I don't dislike the old code. It's utterly clear from
that code what's going on. It's arguable that it's not clear that the
rep/erms variants are just using stosb vs. stosq, but the high level
concept of "use a feature flag to switch between three implementations
of clear page" is crystal clear.

> +	kmsan_unpoison_memory(page, len);
> +	asm volatile(ALTERNATIVE_2("call memzero_page_aligned_unrolled",
> +				   "shrq $3, %%rcx; rep stosq", X86_FEATURE_REP_GOOD,
> +				   "rep stosb", X86_FEATURE_ERMS)
> +			: "+c" (len), "+D" (page), ASM_CALL_CONSTRAINT
> +			: "a" (0)
> +			: "cc", "memory");
>  }

This is substantially less clear. It also doesn't even add comments to
make up for the decreased clarity.

>  void copy_page(void *to, void *from);
> diff --git a/arch/x86/lib/clear_page_64.S b/arch/x86/lib/clear_page_64.S
> index a508e4a8c66a..27debe0c018c 100644
> --- a/arch/x86/lib/clear_page_64.S
> +++ b/arch/x86/lib/clear_page_64.S
> @@ -6,30 +6,15 @@
>  #include <asm/asm.h>
>  
>  /*
> - * Most CPUs support enhanced REP MOVSB/STOSB instructions. It is
> - * recommended to use this when possible and we do use them by default.
> - * If enhanced REP MOVSB/STOSB is not available, try to use fast string.
> - * Otherwise, use original.
> + * Zero page aligned region.
> + * %rdi	- dest
> + * %rcx	- length
>   */

That comment was pretty useful, IMNHO.

How about we add something like this above it? I think it explains the
whole landscape, including the fact that X86_FEATURE_REP_GOOD is
synthetic and X86_FEATURE_ERMS is not:

Switch between three implementation of page clearing based on CPU
capabilities:

 1. memzero_page_aligned_unrolled(): the oldest, slowest and universally
    supported method. Uses a for loop (in assembly) to write a 64-byte
    cacheline on each loop. Each loop iteration writes to memory using
    8x 8-byte MOV instructions.
 2. "rep stosq": Really old CPUs had crummy REP implementations.
    Vendor CPU setup code sets 'REP_GOOD' on CPUs where REP can be
    trusted. The instruction writes 8 bytes per REP iteration but CPUs
    internally batch these together and do larger writes.
 3. "rep stosb": CPUs that enumerate 'ERMS' have an improved STOS
    implementation that is less picky about alignment and where STOSB
    (1 byte at a time) is actually faster than STOSQ (8 bytes at a
    time).