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

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

On Tue, 3 Nov 2026 14:14:50 +0000
Usama Arif <usama.arif@linux.dev> wrote:

> On 03/11/2026 09:32, David Laight wrote:
> > On Mon, 2 Nov 2026 23:03:39 +0000
> > Usama Arif <usama.arif@linux.dev> wrote:
> >   
> >> On 22/10/2026 10:49, David Laight wrote:  
> >>> On Wed, 21 Oct 2026 08:18:25 -1000
> >>> Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >>>     
> >>>> On Tue, 20 Oct 2026 at 23:55, Kiryl Shutsemau <kas@kernel.org> wrote:    
> >>>>>
> >>>>> Moving MOVQ inside ALTERNATIVE_2 is ugly...      
> >>>>
> >>>> Yeah, maybe it would make sense to make that function be
> >>>>
> >>>>         movq %rdi, %rax
> >>>>         movq %rdx, %rcx
> >>>>         ALTERNATIVE "jmp memcpy_orig", "rep movsb", X86_FEATURE_FSRM
> >>>>         RET
> >>>>
> >>>> instead? No nop instructions at all, and both alternatives are two bytes.
> >>>>
> >>>> Then do any ERMS logic at the top of memcpy_orig, which now has the
> >>>> registers ready for string instructions anyway.    
> >>>
> >>> Is it possibly to just patch "rep movsb; RET" over whatever gets generated
> >>> for memcpy_orig (after the two register moves) ?
> >>> Then there are neither nops nor a jmp.
> >>>
> >>> David
> >>>     
> >> Hi,
> >>
> >> Sorry for the late reply. I had taken time off for the past couple of weeks
> >> and didnt have access to my work station.
> >>
> >> I implemented Davids suggestion and I think it looks good. The scalar body is
> >> now part of __memcpy(), and FSRM replaces the existing 32-byte check with
> >> "rep movsb; RET". Non-FSRM calls fall straight into scalar code, and FSRM
> >> returns before any padding.
> >>
> >> I have retained the 2048-byte ERMS cutoff, since selecting REP from 32 bytes
> >> regressed short copies in the Cooper Lake tests.
> >>
> >> I asked LLM to review it and it indicated a problem with ITS: returns are
> >> patched before alternatives are copied, so the return decision uses the
> >> replacement's source address. I align replacement storage outside the
> >> copied range so ITS retains the return thunk and relocation preserves its
> >> target. This adds no executed padding.
> >>
> >> Any thoughts on below? I will send it as a proper patch for review
> >> if it looks acceptable.
> >>
> >>
> >> From 9baffefb71c7223702c7bcd96d43b9a2828f3939 Mon Sep 17 00:00:00 2001
> >> From: Usama Arif <usama.arif@linux.dev>
> >> Date: Thu, 1 Oct 2026 14:15:12 -0700
> >> Subject: [PATCH] x86/lib: Use REP MOVSB for large memcpy() on ERMS CPUs
> >>  without FSRM
> >>
> >> Commit 68674f94ffc9 ("x86: don't use REP_GOOD or ERMS for small
> >> memory copies") left ERMS CPUs without FSRM, including Cooper Lake and
> >> Skylake, using scalar code for all out-of-line copies. Btrfs/zstd input
> >> staging and output flushing accounted for about 34% of memcpy_orig()'s
> >> sampled weight in Meta's x86 fleet profile.
> >>
> >> Use REP MOVSB from 2048 bytes on ERMS CPUs without FSRM, keeping scalar
> >> copies below that threshold. FSRM continues to use REP for all sizes.
> >>
> >> Merge scalar code into __memcpy() and overlay its existing 32-byte check
> >> with REP MOVSB and RET on FSRM, removing the non-FSRM entry jump. Copies
> >> below 32 bytes bypass the ERMS gate. SHR11/JNZ fits one ten-byte NOP
> >> without ERMS; the REP arm reloads the original count from RDX.
> >>
> >> With ITS configured, align replacement storage before its start label so
> >> the raw FSRM return retains its thunk before relocation. The alignment
> >> padding is outside the copied range.
> >>
> >> On Cooper Lake (boost disabled, capped at 1.401 GHz), plain-RET userspace
> >> replicas with aligned warmed buffers and entry offset zero modulo 64
> >> measured 4 KiB copies at 371.7 ns originally and 123.4 ns with this patch:
> >> a 66.8% paired reduction across five process runs. A separate sweep using
> >> REP from 32 bytes increased 32/64/128/256-byte times by
> >> 114.9%/54.4%/66.1%/23.2%, respectively.
> >>
> >> Small-copy costs remain: a warmed aligned zero-byte copy at entry offset
> >> 48 modulo 64 took a paired 0.75 ns more (+7.9%) than the original, in all
> >> five runs. These microbenchmarks do not establish a kernel workload gain
> >> or the best threshold across CPUs.
> >>
> >> Direct copy checks and memcpy KUnit passed with all four ERMS/FSRM
> >> combinations and with ITS forced on; the copied return target was verified.
> >>
> >> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> >> ---
> >>  arch/x86/lib/memcpy_64.S | 42 ++++++++++++++++++++++++++--------------
> >>  1 file changed, 27 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
> >> index 12a23fa7c44c5..407b226d4de2f 100644
> >> --- a/arch/x86/lib/memcpy_64.S
> >> +++ b/arch/x86/lib/memcpy_64.S
> >> @@ -31,24 +31,27 @@
> >>   * 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
> >> -	RET
> >> -SYM_FUNC_END(__memcpy)
> >> -EXPORT_SYMBOL(__memcpy)
> >> -
> >> -SYM_FUNC_ALIAS_MEMFUNC(memcpy, __memcpy)
> >> -SYM_PIC_ALIAS(memcpy)
> >> -EXPORT_SYMBOL(memcpy)
> >>  
> >> -SYM_FUNC_START_LOCAL(memcpy_orig)
> >> -	movq %rdi, %rax
> >> +#ifdef CONFIG_MITIGATION_ITS
> >> +	/*
> >> +	 * Returns are patched before alternatives. Keep the replacement
> >> +	 * return in the first half of a cache line so ITS retains its thunk
> >> +	 * when the instruction is copied to its final address.  
> > 
> > I'm not 100% sure what that is about, but does it mean that the cache line
> > alignment of the code here also matters?  
> 
> Not the alignment of __memcpy() itself, only that of the replacement in
> .altinstr_replacement.
> 
> On ITS affected CPUs, a RET in the lower 32 bytes of a 64-byte cache
> line needs mitigation, so apply_returns() leaves such sites as a jump to
> its_return_thunk, whose own RET is in the upper half, and turns the rest
> into a plain RET. It runs before apply_alternatives(), so for the RET in
> the FSRM replacement it decides from the replacement's address, not from
> where the code is copied to. A plain RET could then end up in the lower
> half of a line in __memcpy().

So the RET in the replacement is in the first half a cache line- so becomes a ret.
But, unless __memcpy() is aligned, the relocated RET might be in the second
half so would need mitigating - but you copy in a mitigation-free ret.

If that is the only mitigation then wouldn't aligning memcpy mean you could
just use a ret (possibly followed by an 'int 3' - or whatever is the 'flavour
of the month' instruction for stopping SLS).

David

> 
> 
> >   
> >> +	 */
> >> +	.pushsection .altinstr_replacement, "ax"
> >> +	.balign 64
> >> +	.popsection
> >> +#endif
> >> +	ALTERNATIVE __stringify(cmpq $0x20, %rdx; jb .Lhandle_tail), \
> >> +		    __stringify(rep movsb; RET), X86_FEATURE_FSRM
> >>  
> >> -	cmpq $0x20, %rdx
> >> -	jb .Lhandle_tail
> >> +	/*
> >> +	 * Prepared RCX is scratch for checking length >= 2048.
> >> +	 * The 10-byte alternative fits a NOP without ERMS.
> >> +	 */
> >> +	ALTERNATIVE "", __stringify(shrq $11, %rcx; jnz .Lerms), X86_FEATURE_ERMS  
> > 
> > Can you use %rdx here, preserving %rcx for the 'rep movsb'?
> > You might need to swap cx<=>dx in the rest of the function.
> > (I'm sure we could use #defines to give names to registers....)  
> 
> 
> Yeah thats a good idea! It does make the code change a lot bigger, but its
> easy to review as its just swapping rdx to rcx. If there arent any major objections
> to this version, I will send it as an official patch tomorrow with the swap you
> suggested plus a bit more cleanup on the commit message and comments.
> 
> 
> Thanks for the review!
> Usama
> 
> > 
> > David
> >   
> >>  	/*
> >>  	 * We check whether memory false dependence could occur,
> >> @@ -169,5 +172,14 @@ SYM_FUNC_START_LOCAL(memcpy_orig)
> >>  
> >>  .Lend:
> >>  	RET
> >> -SYM_FUNC_END(memcpy_orig)
> >>  
> >> +.Lerms:
> >> +	movq %rdx, %rcx
> >> +	rep movsb
> >> +	RET
> >> +SYM_FUNC_END(__memcpy)
> >> +EXPORT_SYMBOL(__memcpy)
> >> +
> >> +SYM_FUNC_ALIAS_MEMFUNC(memcpy, __memcpy)
> >> +SYM_PIC_ALIAS(memcpy)
> >> +EXPORT_SYMBOL(memcpy)  
> >   
>