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

[Borislav Petkov <bp@xxxxxxxxx>]

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@xxxxxxxxx>
> Closes: 
> https://lore.kernel.org/oe-kbuild-all/202605140922.q7IlUv7o-lkp@xxxxxxxxx/
> Signed-off-by: Mauricio Faria de Oliveira <mfo@xxxxxxxxxx>
> Reviewed-by: Juergen Gross <jgross@xxxxxxxx>
> 
> ---
> (*) 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.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette