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

[David Laight <david.laight.linux@gmail.com>]

On Wed, 11 Nov 2026 15:46:08 -0800
Borislav Petkov <bp@alien8.de> 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.
> 
> 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.
> 
> 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.

Multiple functions seems like a good idea.

In some cases you can patch the constant in the instruction.
That is safe here even if a multi-byte constant crosses a cache line
because it is only an optimisation.

Patching the call sites would certainly be cleaner.
More interesting would be having a function with a non-standard calling
convention (%rdi, %rsi, %rcx, everything else preserved, void result)
so that the 5 byte call can be patched with "rep movsb; nop3".
I can't see anywhere that uses the result.
Maybe start with "rep movsb; nop3" and patch with the call?
You'd still need a normal memcpy() for the calls gcc makes and if the
address is taken.

A special function to do the patching would also make sense.
All Intel non-atom cpu since sandy bridge run 'rep movsb' as fast
as 'rep movsq' - so you only need to worry about short copies.
Whereas some of the older Zen cpu run misaligned copies slowly.
So you could pick/patch the most appropriate code using more complex
logic than just whether one of the X86_FEATURE_xxx bits is set.

The same calling convention might make sense for memmove().
Although it probably wants a scratch register for the (dst - src < len)
check (it might want dst in the scratch register!).

David

> 
> 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.
>