Re: [PATCH RESEND v5 1/3] x86/asm, x86/boot: expose inline memcmp

[Mauricio Faria de Oliveira <mfo@igalia.com>]

On 2026-07-01 16:57, Mauricio Faria de Oliveira wrote:
> Thanks for looking at this again.
> 
> On 2026-07-01 15:40, Borislav Petkov wrote:
>> On Tue, Jun 30, 2026 at 02:21:46PM -0300, Mauricio Faria de Oliveira wrote:
>>> Move the inline memcmp function currently only available in 'boot/string.c'
>>> into the shared string function header <asm/shared/string.h> to be reused.
>>> 
>>> This is not done through <asm/string.h> to avoid pulling unnecessary code
>>> in 'boot/string.c' that causes build errors in 'boot/compressed/string.c'
>>> and 'purgatory/purgatory.ro'.
>>> 
>>> Note that the inline memcmp() returns 0/1, not -1/0/1 as regular memcmp()
>>> (reported by David Laight <david.laight.linux@gmail.com>).
>> 
>> This belongs ...
>> 
>> 
>>> 
>>> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
>>> ---
>> 
>> ... here, if anywhere.
> 
> I'm not sure, as Reported-by and Suggested-by mean something different.
>

Oops, I missed the triple dash in your comment.

That would discard the credit attribution when applied, AFAIK. So,
still:
 
> Maybe a simpler statement would do? e.g., '(Thanks to David Laight for
> noticing the return value difference between inline and regular
> memcmp())'. That gives credit while avoiding confusion with tags.

Thanks,

> 
>> 
>>>  arch/x86/boot/string.c               |  6 ++----
>>>  arch/x86/include/asm/shared/string.h | 16 ++++++++++++++++
>>>  arch/x86/include/asm/string.h        |  1 +
>>>  3 files changed, 19 insertions(+), 4 deletions(-)
>> 
>> Also, PeterZ has a point - we should group all __inline_ functions into
>> shared/string.h and say why that is the case at the top of the header.
> 
> Ok.
> 
>>> diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
>>> index ac0f900ebc47efa81c92e1bb2010ea41677899c4..be454a6864225f3a972c3e81826b77ed4e8a57fe 100644
>>> --- a/arch/x86/boot/string.c
>>> +++ b/arch/x86/boot/string.c
>>> @@ -15,6 +15,7 @@
>>>  #include <linux/errno.h>
>>>  #include <linux/limits.h>
>>>  #include <asm/asm.h>
>>> +#include <asm/shared/string.h>
>>>  #include "ctype.h"
>>>  #include "string.h"
>>>  
>>> @@ -31,10 +32,7 @@
>>>  
>>>  int memcmp(const void *s1, const void *s2, size_t len)
>>>  {
>>> -	bool diff;
>>> -	asm("repe cmpsb"
>>> -	    : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
>>> -	return diff;
>>> +	return __inline_memcmp(s1, s2, len);
>>>  }
>>>  
>>>  /*
>>> diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
>>> new file mode 100644
>>> index 0000000000000000000000000000000000000000..422952152f533ad75b98f3873297b39c4f5e2477
>>> --- /dev/null
>>> +++ b/arch/x86/include/asm/shared/string.h
>>> @@ -0,0 +1,16 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +#ifndef _ASM_X86_SHARED_STRING_H
>>> +#define _ASM_X86_SHARED_STRING_H
>>> +
>>> +/* Note: this memcmp() returns 0/1, not -1/0/1 as regular memcmp(). */
>> 
>> Please document it properly - what does 0 mean as a return value and what 1.
> 
> Ok.
> 
> cheers,
> 
>> 
>> Thx.

-- 
Mauricio