Re: [PATCH v4 1/4] x86/asm: move inline string functions to <asm/string_inline.h>

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

On 2026-05-30 20:48, Borislav Petkov wrote:
> On Tue, May 26, 2026 at 11:52:33AM -0300, Mauricio Faria de Oliveira wrote:
>> In a future patch, 'boot/string.c' will include inline string functions.
>> 
>> Using the header <asm/string.h> is problematic for a couple of reasons
>> (i.e., build errors), which can be addressed, but introduce unnecessary
>> complexity and regression risk (beyond these _found_ couple of reasons).
>> 
>> Using a new header <asm/string_inline.h> is simpler and transparent to
>> existing users of <asm/string.h>, with less changes to 'boot/string.c'
>> and its users (eg 'boot/compressed/string.c' and 'purgatory/purgatory.ro'),
>> which minimize regression risk.
>> 
>> No functional change intended.
>> 
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202605140922.q7IlUv7o-lkp@intel.com/
>> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
>> Reviewed-by: Juergen Gross <jgross@suse.com>
>> 
>> ---
>> (*) Reasons not to include <asm/string.h> in 'boot/string.c':
>> 
>> 1) 'boot/string.c' is built for 16-bit/real mode thus some type and word
>> size errors happen when <asm/string.h> include, e.g., <asm/string_64.h>.
>> 
>> This can be addressed with '#ifndef _SETUP' (defined by 'boot/Makefile').
>> 
>> 2) 'boot/string.c' is included by 'boot/compressed/string.c' which is
>> the source of 'purgatory/string.o', linked by 'purgatory/purgatory.ro'
>> (CONFIG_KEXEC_FILE).
>> 
>> In 64BIT, <asm/string.h> includes <asm/string_64.h>, which references
>> __memset() and __memmove() with KCFI_REFERENCE(), ie, __ADDRESSABLE();
>> however, 'purgatory/purgatory.ro' is not linked with implementations.
>> 
>> So, CONFIG_KEXEC_FILE and CONFIG_CFI without CONFIG_KASAN hit errors:
>> 
>>   >> ld.lld: error: undefined symbol: __memset
>>      >>> referenced by string.c
>>      >>>               arch/x86/purgatory/purgatory.ro:\
>>                        (__UNIQUE_ID_addressable___memset_0)
>>   --
>>   >> ld.lld: error: undefined symbol: __memmove
>>      >>> referenced by string.c
>>      >>>               arch/x86/purgatory/purgatory.ro:\
>>                        (__UNIQUE_ID_addressable___memmove_1)
>> 
>> (Note: this is not hit with CONFIG_KASAN because 'boot/compressed/string.c'
>> adds aliases __memset()/__memmove() to memset()/memmove() in that case.)
>> 
>> This can be addressed with 'CFLAGS_string.o := -D__DISABLE_EXPORTS' so to
>> disable KCFI_REFERENCE() in 'purgatory/Makefile' (it removes CC_FLAGS_CFI
>> anyway).
>> 
>> ...
>> 
>> However, since a change in this series would need more changes to address
>> errors it causes, it is reasonable to change the series not to cause them,
>> by using a separate header with _just_ inline string functions.
> 
> This is very long-winded and it meanders across things. Write it more
> disciplined, please, and formulate it such that you're writing the commit
> message of a standalone patch. It should have merit on its own and not talk
> about future patches and so on.
> 
> And yes, the intent to have a separate header which doesn't pull in
> nasty deps between decompressor and kernel proper, is ok.
> 
> For that, we have arch/x86/include/asm/shared/ which contains functionality
> shared between the two objects so I think you should move it there. It'll also
> make it a "clean" header which contains solely this stuff and doesn't pull in
> any other shit.

Ack; thanks for the feedback and pointers, I'll take a look.

> 
> Thx.

-- 
Mauricio