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

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

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.

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.

> 
>>  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