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

[Usama Arif]

On Tue, 10 Nov 2026 17:06:55 +0000 David Laight <david.laight.linux@xxxxxxxxx> wrote:

> On Tue, 10 Nov 2026 16:25:54 +0000
> Kiryl Shutsemau <kas@xxxxxxxxxx> wrote:
> 
> > On Wed, Nov 04, 2026 at 02:28:06PM -0800, Usama Arif wrote:
> > > +#ifdef CONFIG_MITIGATION_ITS
> > > +	/*
> > > +	 * Returns are patched before alternatives. Align the replacement
> > > +	 * source so its return is in the first half of a cache line and ITS
> > > +	 * retains its thunk. The copied return can then execute in either
> > > +	 * half of a cache line.
> > > +	 */
> > > +	.pushsection .altinstr_replacement, "ax"
> > > +	.balign 64
> > > +	.popsection
> > > +#endif
> > > +	ALTERNATIVE __stringify(cmpq $0x20, %rcx; jb .Lhandle_tail), \
> > > +		    __stringify(rep movsb; RET), X86_FEATURE_FSRM  
> > 
> > Does this mean any RET in a replacement needs the same alignment? Would
> > anything catch it if the next one doesn't have it?
> 
> My suspicion is that ideally 'yes'.
> But someone said (in this thread) that it is worse for RET in the first
> half a cache line - so aligning the replacement that way is better.)

Yes, ITS requires mitigation for a return in the first half of a
cache line. Aligning the old replacement there kept its thunk when
copied. The next version will keep RET in ordinary text, so ITS uses its
execution address and the replacement alignment is unnecessary.

> 
> > Linus' layout keeps the RET out of the replacement, with no padding:
> > 
> > 	movq %rdi, %rax
> > 	movq %rdx, %rcx
> > 	ALTERNATIVE "jmp memcpy_orig", "rep movsb", X86_FEATURE_FSRM
> > 	RET
> > 
> > Non-FSRM takes the same jump it takes today. Does that avoid the ITS
> > problem? Why not this layout?
> 
> It adds a jump to the non-FSRM path.
> Cold cache that is likely to be measurable.

The upstream code already jumps to memcpy_orig(); I have the next version
retaining that jump. Preparing RCX at the public entry adds a MOV on the
scalar path. The return stays in ordinary text so ITS uses its execution
address. I will post the next version today so that its easier to have
a discussion

> (But not if RET ends up being anything other than 0xc3 -  that will
> just kill everything.)
> 
> > 
> > > +	/*
> > > +	 * 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  
> > 
> > The RCX/RDX swap is most of the diff, and all it saves is one MOV on
> > copies of 2K and up.
> 
> Register-register moves don't cost much (they can even be done my the
> register renaming logic rather than the alu - but still have to be
> fetched and decoded), but needing them is a bit of 'lack of attention
> detail' when a simple backtrack can get the value where it is needed.
> gcc is very bad at it, a compiler really ought to do a lot better.
> 
> David
> 
> > 
> > What about something like this? Untested.
> > 
> > diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
> > --- a/arch/x86/lib/memcpy_64.S
> > +++ b/arch/x86/lib/memcpy_64.S
> > @@ -31,11 +31,9 @@
> >   * 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,11 +43,11 @@ SYM_PIC_ALIAS(memcpy)
> >  EXPORT_SYMBOL(memcpy)
> > 
> >  SYM_FUNC_START_LOCAL(memcpy_orig)
> > -	movq %rdi, %rax
> > -
> >  	cmpq $0x20, %rdx
> >  	jb .Lhandle_tail
> > 
> > +	ALTERNATIVE "", __stringify(shrq $11, %rcx; jnz .Lerms), X86_FEATURE_ERMS
> > +
> >  	/*
> >  	 * We check whether memory false dependence could occur,
> >  	 * then jump to corresponding copy mode.
> > @@ -169,5 +167,10 @@ SYM_FUNC_START_LOCAL(memcpy_orig)
> > 
> >  .Lend:
> >  	RET
> > +
> > +.Lerms:
> > +	movq %rdx, %rcx
> > +	rep movsb
> > +	RET
> >  SYM_FUNC_END(memcpy_orig)
> > 
> > If you keep the swap, please make it a separate patch with no functional
> > change.
> > 
> 
>