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

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

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


> 
>> +	 */
>> +	.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)
>