Re: [PATCH v6 4/7] x86: re-work memcpy()

[Jason Andryuk <jason.andryuk@xxxxxxx>]

On 2025-06-16 09:00, Jan Beulich wrote:
> Move the function to its own assembly file. Having it in C just for the
> entire body to be an asm() isn't really helpful. Then have two flavors:
> A "basic" version using qword steps for the bulk of the operation, and an
> ERMS version for modern hardware, to be substituted in via alternatives
> patching.
>
> Alternatives patching, however, requires an extra precaution: It uses
> memcpy() itself, and hence the function may patch itself. Luckily the
> patched-in code only replaces the prolog of the original function. Make
> sure this remains this way.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> Reviewed-by: Teddy Astie <teddy.astie@xxxxxxxxxx>
> --- /dev/null
> +++ b/xen/arch/x86/memcpy.S
> @@ -0,0 +1,20 @@
> +#include <asm/asm_defns.h>
> +
> +FUNC(memcpy)
> +        mov     %rdx, %rcx
> +        mov     %rdi, %rax
> +        /*
> +         * We need to be careful here: memcpy() is involved in alternatives
> +         * patching, so the code doing the actual copying (i.e. past setting
> +         * up registers) may not be subject to patching (unless further
> +         * precautions were taken).
> +         */
I think this phrasing is a little clearer:

We need to be careful here: memcpy() is involved in alternatives 
patching.  Define the original code as only the register setup.  The 
code doing the actual copying (i.e. past setting up registers) is not 
subject to patching, which avoids it changing underneath the processor.
Your comment is okay if you prefer not to change it:

Reviewed-by: Jason Andryuk <jason.andryuk@xxxxxxx>

Regards,
Jason

> +        ALTERNATIVE "and $7, %edx; shr $3, %rcx", \
> +                    STR(rep movsb; RET), X86_FEATURE_ERMS
> +        rep movsq
> +        or      %edx, %ecx
> +        jz      1f
> +        rep movsb
> +1:
> +        RET
> +END(memcpy)