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

[David Laight]

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

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