Re: [PATCH v3 2/2] x86/lib: Use REP MOVSB for large memcpy() on ERMS CPUs without FSRM

[Usama Arif]

On Wed, 11 Nov 2026 15:46:08 -0800 Borislav Petkov <bp@xxxxxxxxx> wrote:

> On Wed, Nov 11, 2026 at 05:20:53AM -0800, Usama Arif wrote:
> > diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
> > index 7bb146c811029..5320862e0274e 100644
> > --- a/arch/x86/lib/memcpy_64.S
> > +++ b/arch/x86/lib/memcpy_64.S
> > @@ -27,15 +27,13 @@
> >   *
> >   * The 'rep movsb' itself is small enough to replace the call, but the
> >   * two register moves blow up the code. And one of them is "needed"
> > - * only for the return value that is the same as the source input,
> > + * only for the return value that is the same as the destination input,
> >   * which the compiler could/should do much better anyway.
> >   */
> >  SYM_TYPED_FUNC_START(__memcpy)
> > -	ALTERNATIVE "jmp memcpy_orig", "", X86_FEATURE_FSRM
> > -
> >  	movq %rdi, %rax
> >  	movq %rdx, %rcx
> > -	rep movsb
> > +	ALTERNATIVE "jmp memcpy_orig", "rep movsb", X86_FEATURE_FSRM
> >  	RET
> >  SYM_FUNC_END(__memcpy)
> >  EXPORT_SYMBOL(__memcpy)
> > @@ -45,12 +43,16 @@ SYM_PIC_ALIAS(memcpy)
> >  EXPORT_SYMBOL(memcpy)
> >  
> >  SYM_FUNC_START_LOCAL(memcpy_orig)
> > -	movq %rdi, %rax
> > -	movq %rdx, %rcx
> > -
> >  	cmpq $0x20, %rcx
> >  	jb .Lhandle_tail
> >  
> > +	/*
> > +	 * RCX holds the count for REP MOVSB and scalar copying; RDX is scratch.
> > +	 * Shift RDX to check length >= 2048 without destroying the REP count.
> > +	 * The 10-byte alternative fits a NOP without ERMS.
> > +	 */
> > +	ALTERNATIVE "", __stringify(shrq $11, %rdx; jnz .Lerms), X86_FEATURE_ERMS
> 
> Sorry, this is adding more JMPs in "conditional" alternatives code and thus
> turns this into a bigger and unmaintainable mess than it already is.
>

How about something like what I have added at the end, would that be more
acceptable in terms of maintainability?
 
> The 2K limit looks like something conjured out of a thin air as it seems it
> work for your particular case. I'm sure we will have to whack it in the future
> because someone else will complain of the opposite.
> 

As mentioned in the commit message as well as [1], 2K came from experiments.

Changing the cmp immediate from 2048 to 32 bytes made aligned warmed
32/64/128/256-byte copies 101.2%/69.5%/53.5%/16.5% slower.

This is usecase driven, as Linus said in [2]: "let's see if somebody finds a
case where the kernel memory copies also care".

If someone finds a case where less than 2K is used and it doesnt cause a
regression for CPUs, they can try lowering it.

> This needs to be done with alternative_call*, even at the price of some
> duplicated code but without turning what already is a mess, into a bigger one.
> 

I tried alternative_call_2() from a C entry with separate helpers. It
adds a helper CALL/RET to every path, including FSRM. The attached
version uses tail jumps, so each helper returns directly to the original
caller and FSRM keeps REP inline. I am hoping this is acceptable?

> Or a static key or even with stop_machine_nmi() (currently in review) which
> patches the kernel even later, when it is convenient to do so.
> 
> Whatever it takes to keep this code readable and sane.
> 
> But not this.
> 
> Sorry.
> 

No worries! Hopefully the version below is more maintainable and acceptable
for everyone?

Thanks for the review!
Usama

[1] https://lore.kernel.org/all/20261017192727.2760919-1-usama.arif@xxxxxxxxx/
[2] https://git.kernel.org/linus/47ee3f1dd93bcbe031539b1ecdaafb44b661c772

diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
index 7bb146c811029..8a398edf6e160 100644
--- a/arch/x86/lib/memcpy_64.S
+++ b/arch/x86/lib/memcpy_64.S
@@ -27,15 +27,15 @@
  *
  * The 'rep movsb' itself is small enough to replace the call, but the
  * two register moves blow up the code. And one of them is "needed"
- * only for the return value that is the same as the source input,
+ * only for the return value that is the same as the destination input,
  * which the compiler could/should do much better anyway.
  */
 SYM_TYPED_FUNC_START(__memcpy)
-       ALTERNATIVE "jmp memcpy_orig", "", X86_FEATURE_FSRM
-
        movq %rdi, %rax
        movq %rdx, %rcx
-       rep movsb
+       /* FSRM takes precedence. Keep RET outside replacements for ITS. */
+       ALTERNATIVE_2 "jmp memcpy_orig", "jmp memcpy_erms", \
+                     X86_FEATURE_ERMS, "rep movsb", X86_FEATURE_FSRM
        RET
 SYM_FUNC_END(__memcpy)
 EXPORT_SYMBOL(__memcpy)
@@ -44,10 +44,15 @@ SYM_FUNC_ALIAS_MEMFUNC(memcpy, __memcpy)
 SYM_PIC_ALIAS(memcpy)
 EXPORT_SYMBOL(memcpy)

-SYM_FUNC_START_LOCAL(memcpy_orig)
-       movq %rdi, %rax
-       movq %rdx, %rcx
+/* The entry prepares RAX as the return value and RCX as the count. */
+SYM_FUNC_START_LOCAL(memcpy_erms)
+       cmpq $2048, %rcx
+       jb memcpy_orig
+       rep movsb
+       RET
+SYM_FUNC_END(memcpy_erms)

+SYM_FUNC_START_LOCAL(memcpy_orig)
        cmpq $0x20, %rcx
        jb .Lhandle_tail