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

[Usama Arif <usama.arif@linux.dev>]

On Thu, 12 Nov 2026 09:37:29 +0000 David Laight <david.laight.linux@gmail.com> wrote:

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

Hopefully the version in [1] is acceptable!
 
> 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.
> 

I think it might be a good idea to start with just a constant and if
someone can comeup with a usecase that needs it patched, we can then
look at this? Basically prefer to keep it usecase driven.

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

Replacing the caller's CALL with REP MOVSB could remove actual call
overhead. The C wrapper I tried patches a CALL inside __memcpy() and
adds a helper call and return. The attached tail dispatcher handles the
existing public ABI and leaves caller-side patching for a follow-up.

The scalar helper currently uses RDX and R8-R11 as scratch. Your private
void interface would need an implementation that preserves those
registers, or register saves. RCX already holds the count, and the
scalar helper preserves RAX and the callee-saved registers.

There are return-value users: memdup_buf() in
lib/crypto/tests/test-utils.h returns memcpy()'s pointer. The normal
entry must still return the original destination in RAX and support
compiler-generated and address-taken calls, as you point out. The
current revision preserves that ABI and its aliases.


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

I agree that CPU model and alignment can inform a better policy.

I think to do this, we need access to many different CPUs to test.
I am confident that the 2K cmp will not regress other CPUs and is a good
start. Would it make sense to go ahead with the apprach in the current
patch and develop later on it if someone comes up with a usecase?
 
> 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
> 

[1] https://lore.kernel.org/all/20261113141137.1113517-1-usama.arif@linux.dev/