Re: [PATCH v9 2/5] x86/asm, x86/boot: expose inline memcmp()

[David Laight]

On Tue, 8 Sep 2026 16:16:51 -0700
"H. Peter Anvin" <hpa@xxxxxxxxx> wrote:

> On 2026-09-08 12:32, Borislav Petkov wrote:
> > On Tue, Sep 08, 2026 at 03:04:53PM -0300, Mauricio Faria de Oliveira wrote:  
> >> Boris, perhaps the approach here could be changed to add an actual
> >> memcmp()-like inline implementation (e.g., as provided in a previous
> >> revision, without return value differences), or continue with the
> >> memeq()-like memcmp() from arch/x86/boot/ but rename it to memeq() ?  
> > 
> > Nah, new functionality is not needed. We can add it later when it is really needed.
> >   
> 
> I personally think it would be a good reason to explicitly rename it memeq(),
> after all it indicates what it actually *does*.
> 
> 	bool memeq(const void *m1, const void *m2, size_t n);
> 
> Note, however, that the sense of the return value is opposite -- true means
> equal, so !memcmp(...) needs to be replaced with memeq(...) and vice versa.
> 
> The other issue is when gcc/clang wants to call memcmp() out of line. Although
> a theoretical concern, there really isn't any reason not to DTRT there since
> there is only one instance in the code, ever.
> 
> Here is an out-of-line compact memcmp() which works for both 16/32 and 64 bits:
> 
> int memcmp(const void *s1, const void *s2, size_t len)
> {
> 	int rv;
> 
> 	asm volatile("xor %0,%0 ; "
> 		     "test %3, %3 ; "	/* Handle len == 0 correctly */

That comment doesn't really say what the instruction is for.
The XOR sets Z and clears C (I just checked) so it isn't needed
in order to get the correct flags.

> 		     "repe cmpsb ; "}g
> 		     "setz %b0 ; "
> 		     "sbb $0, %0"

That pair is just wrong.
Only one of C or Z can be set, so I think this is ok:
			"seta %b0 ; "	/* C == 0 && Z == 0 */
			"sbb $0, %0"

> 		: "=&r" (rv), "+D" (s1), "+S" (s2), "+c" (len)
> 		: : "cc", "memory");
> 
> 	return rv;
> }
> 
> On 64 bits it compiles to:
> 
> 0000000000000000 <memcmp>:
>    0:   48 89 d1                mov    %rdx,%rcx
>    3:   31 d2                   xor    %edx,%edx
>    5:   31 c0                   xor    %eax,%eax
>    7:   f3 a6                   repz cmpsb (%rdi),(%rsi)
>    9:   0f 97 c2                seta   %dl
>    c:   0f 92 c0                setb   %al
>    f:   29 d0                   sub    %edx,%eax

That isn't the object code from the source ...

David