[PATCH RESEND v5 0/3] x86/pvh: fix unbootable VMs again (PVH + KASAN)

[PATCH RESEND v5 0/3] x86/pvh: fix unbootable VMs again (PVH + KASAN)

[Mauricio Faria de Oliveira]

[RESEND v5 (2026-06-01): address feedback from Borislav Petkov in Patch 1.]

The issue of unbootable VMs with CONFIG_PVH due to CONFIG_KASAN is back.

Booting directly from vmlinux (instead of bzImage) now fails with gcc-14/15
(but works with gcc-12/13) if CONFIG_KASAN_GENERIC is set, on Ubuntu 25.10.

The PVH code is required/supposed not to use the KASAN memory access check
in the kernel entry point as KASAN has not yet been setup, or an exception
is hit and the boot fails.

This was previously described and addressed with __builtin_mem{cmp,set}():
- commit 661362e3dcab ("xen, pvh: fix unbootable VMs (PVH + KASAN - AMD_MEM_ENCRYPT)")
- commit 416a33c9afce ("x86/cpu: fix unbootable VMs by inlining memcmp() in hypervisor_cpuid_base()")
- commit fbe5a6dfe492 ("xen, pvh: fix unbootable VMs by inlining memset() in xen_prepare_pvh()")

However, even with __builtin the compiler may decide to use the out of line
function instead of the inline implementation. So, that does not really fix
the issue unconditionally; see details below.

In order to address this, it's required to switch to inline implementations
that do not depend on the compiler.

There's such a memset() in <asm/string.h> and memcmp() in 'boot/string.c'.
Use them instead of builtins in PVH entry.

Testing:

- Booting from vmlinux (fixed) and bzImage (still works) using
  allnoconfig + CONFIG_PVH + CONFIG_KASAN with gcc-12/13/14/15.

- Building with CONFIG_KEXEC_FILE, CONFIG_CFI and !CONFIG_KASAN with LLVM 20
  (check for a build error not caught previously).

Details/Debugging:

- Only CONFIG_PVH (works):

  make allnoconfig
  ./scripts/config \
    -e 64BIT -e HYPERVISOR_GUEST -e PVH \
    -e SERIAL_8250 -e SERIAL_8250_CONSOLE
  make olddefconfig
  make -j$(nproc) vmlinux

  qemu-system-x86_64 \
    -accel kvm -nodefaults -nographic -serial stdio \
    -kernel vmlinux -append 'console=ttyS0'
  ...
  SeaBIOS (version ...)
  Booting from ROM...
  Linux version ...
  ...
  <Ctrl-C>

- With CONFIG_KASAN (fails)

  ./scripts/config -e KASAN
  make olddefconfig
  make -j$(nproc) vmlinux

  qemu-system-x86_64 \
    -accel kvm -nodefaults -nographic -serial stdio \
    -kernel vmlinux -append 'console=ttyS0'
  ...
  SeaBIOS (version ...)
  Booting from ROM...
  <QEMU reboot loop, flashing the text above>

- Debugging:

  Enable debug info and rebuild.

  QEMU: enable and wait for GDB, stop rebooting, remain running.

  qemu-system-x86_64 \
    -s -S -no-reboot -no-shutdown \
    <other options>

  gdb vmlinux
  (gdb) target remote localhost:1234
  ...
  (gdb) c
  ...
  Thread 2 received signal SIGQUIT, Quit.
  ...
  (gdb) info threads
    Id   Target Id                    Frame
    1    Thread 1.1 (CPU#0 [running]) bytes_is_nonzero (
      start=0xfffffbfff031eebe <error: Cannot access memory at address 0xfffffbfff031eebe>, size=1)
      at .../linux/mm/kasan/generic.c:98
  * 2    Thread 1.2 (CPU#1 [halted ]) 0x00000000000fd0a9 in ?? ()
  ...
  (gdb) thr 1
  ...
  (gdb) bt
  #0  bytes_is_nonzero (start=0xfffffbfff031eebe <error: Cannot access memory at address 0xfffffbfff031eebe>, size=1)
      at .../linux/mm/kasan/generic.c:98
  #1  memory_is_nonzero (start=0xfffffbfff031eebe, end=0xfffffbfff031eebf) at .../linux/mm/kasan/generic.c:115
  #2  memory_is_poisoned_n (addr=0xffffffff818f75f0, size=8) at .../linux/mm/kasan/generic.c:140
  #3  memory_is_poisoned (addr=0xffffffff818f75f0, size=8) at .../linux/mm/kasan/generic.c:172
  #4  check_region_inline (addr=0xffffffff818f75f0, size=8, write=false, ret_ip=18446744071585002062)
      at .../linux/mm/kasan/generic.c:191
  #5  kasan_check_range (addr=addr@entry=0xffffffff818f75f0, size=size@entry=8, write=write@entry=false,
      ret_ip=18446744071585002062) at .../linux/mm/kasan/generic.c:200
  #6  0xffffffff813eb283 in __asan_loadN (addr=addr@entry=0xffffffff818f75f0, size=size@entry=8)
      at .../linux/mm/kasan/generic.c:278
  #7  0xffffffff815df24e in memcmp (cs=cs@entry=0xffffffff818f75f0, ct=ct@entry=0x1be2fe4, count=<optimized out>,
      count@entry=12) at .../linux/lib/string.c:683
  #8  0xffffffff81ba2323 in cpuid_base_hypervisor (sig=0xffffffff818f75f0 "XenVMMXenVMM", leaves=2)
      at .../linux/arch/x86/include/asm/cpuid/api.h:206
  #9  xen_cpuid_base () at .../linux/arch/x86/include/asm/xen/hypervisor.h:46
  #10 xen_prepare_pvh () at .../linux/arch/x86/platform/pvh/enlighten.c:119
  #11 0x0000000001ba2588 in ?? ()
  #12 0x0000000000000000 in ?? ()
  (gdb)

  Frames #7-#8 show the non-builtin memcmp() (lib/string.c) was called
  even with __builtin_memcmp() being used in cpuid_base_hypervisor().

Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
---
Changes in v5:
- Create a minimal separate header in <asm/shared/string.h> instead,
  to be used by 'boot/setup.c' and <asm/string.h> (Borislav Petkov).
- Patch 1 (in v4/v3) is no longer needed; removed.
- Patch 1 (in v5):
  - Briefly mention there are issues with <asm/string.h>.
  - Remove 'Reviewed-by: Jurgen Gross' to be conservative
    (same code change and result, but the means changed).
- Link to v4: https://lore.kernel.org/r/20260526-pvh-kasan-inline-v4-0-a310e6a25ecd@igalia.com

Changes in v4:
- Patch 1: address Juergen's feedback:
  - s/In next patch/In a future patch/.
  - Move footnote (Reasons not to include...) after "---".
- Add 'Reviewed-by: Juergen Gross' in patches 1 and 2 as well.
- Link to v3: https://lore.kernel.org/r/20260520-pvh-kasan-inline-v3-0-bede769c6ec7@igalia.com

Changes in v3:
- Create and use a separate header for inline string functions
  to fix a build error reported by kernel test robot (patch 1).
- That also removes '#ifndef _SETUP/#endif' in <asm/string.h>.
- Link to v2: https://lore.kernel.org/r/20260427-pvh-kasan-inline-v2-0-2c57b8dcff6a@igalia.com

Changes in v2:
- Add comment about the return value of __inline_memcmp() in patch 1. (v3: now 2)
- Add 'Reviewed-by: Juergen Gross' in patches 2 and 3 (v3: now 3 and 4).
- Link to v1: https://lore.kernel.org/r/20260422-pvh-kasan-inline-v1-0-7e6194344c92@igalia.com

---
Mauricio Faria de Oliveira (3):
      x86/asm, x86/boot: expose inline memcmp
      x86/cpuid: fix unbootable VMs by really inlining memcmp() in hypervisor_cpuid_base()
      x86/pvh: fix unbootable VMs by really inlining memset() in xen_prepare_pvh()

 arch/x86/boot/string.c               |  6 ++----
 arch/x86/include/asm/cpuid/api.h     |  2 +-
 arch/x86/include/asm/shared/string.h | 16 ++++++++++++++++
 arch/x86/include/asm/string.h        |  1 +
 arch/x86/platform/pvh/enlighten.c    |  3 ++-
 5 files changed, 22 insertions(+), 6 deletions(-)
---
base-commit: 7de6ae9e12207ec146f2f3f1e58d1a99317e88bc
change-id: 20260422-pvh-kasan-inline-6efac77f1b27

Best regards,
-- 
Mauricio Faria de Oliveira <mfo@igalia.com>

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

[Mauricio Faria de Oliveira]

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

Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
---
 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(-)

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(). */
+static __always_inline int __inline_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;
+}
+
+#endif /* _ASM_X86_SHARED_STRING_H */
diff --git a/arch/x86/include/asm/string.h b/arch/x86/include/asm/string.h
index 9cb5aae7fba9ffcf0f5af8f939d30467750ccaa9..f0f4fd8227bf992e78c69209efb31f0a9a0cc3b1 100644
--- a/arch/x86/include/asm/string.h
+++ b/arch/x86/include/asm/string.h
@@ -7,6 +7,7 @@
 #else
 # include <asm/string_64.h>
 #endif
+#include <asm/shared/string.h>
 
 static __always_inline void *__inline_memcpy(void *to, const void *from, size_t len)
 {

-- 
2.47.3

[PATCH RESEND v5 2/3] x86/cpuid: fix unbootable VMs by really inlining memcmp() in hypervisor_cpuid_base()

[Mauricio Faria de Oliveira]

Even with __builtin the compiler may decide to use the out of line function
instead of the inline implementation.

The existing code is broken with gcc-14/15 but not gcc-12/13 (Ubuntu 25.10)
and vmlinux no longer boots with CONFIG_PVH if CONFIG_KASAN_GENERIC is set.

For testing purposes, if the size argument is reduced from 12 to 8 then the
compiler decides to use the inline implementation; that shows results vary.

Switch the builtin to the inline implementation to address it.

Fixes: 416a33c9afce ("x86/cpu: fix unbootable VMs by inlining memcmp() in hypervisor_cpuid_base()")
Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/include/asm/cpuid/api.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpuid/api.h b/arch/x86/include/asm/cpuid/api.h
index 82eddfa2347b32b76c2ea9b85f005ca5416ac71f..2d9f3d4d63de6e721f275d9e80d372edbdfedf30 100644
--- a/arch/x86/include/asm/cpuid/api.h
+++ b/arch/x86/include/asm/cpuid/api.h
@@ -204,7 +204,7 @@ static inline u32 cpuid_base_hypervisor(const char *sig, u32 leaves)
 		 * from PVH early boot code before instrumentation is set up
 		 * and memcmp() itself may be instrumented.
 		 */
-		if (!__builtin_memcmp(sig, signature, 12) &&
+		if (!__inline_memcmp(sig, signature, 12) &&
 		    (leaves == 0 || ((eax - base) >= leaves)))
 			return base;
 	}

-- 
2.47.3

[PATCH RESEND v5 3/3] x86/pvh: fix unbootable VMs by really inlining memset() in xen_prepare_pvh()

[Mauricio Faria de Oliveira]

Even with __builtin the compiler may decide to use the out of line function
instead of the inline implementation.

This particular one (still) generated the inline implementation as expected
(at least in these compiler versions) but this is not guaranteed to remain.

Switch the builtin to the inline implementation to address it.

Fixes: fbe5a6dfe492 ("xen, pvh: fix unbootable VMs by inlining memset() in xen_prepare_pvh()")
Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/platform/pvh/enlighten.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/platform/pvh/enlighten.c b/arch/x86/platform/pvh/enlighten.c
index f2053cbe9b0ce3d2178938269607c652ae8f528e..cb442cbd9d828619421babb281bfe9759edbca8a 100644
--- a/arch/x86/platform/pvh/enlighten.c
+++ b/arch/x86/platform/pvh/enlighten.c
@@ -8,6 +8,7 @@
 #include <asm/hypervisor.h>
 #include <asm/e820/api.h>
 #include <asm/x86_init.h>
+#include <asm/string.h>
 
 #include <asm/xen/interface.h>
 
@@ -129,7 +130,7 @@ void __init xen_prepare_pvh(void)
 	 * This must not compile to "call memset" because memset() may be
 	 * instrumented.
 	 */
-	__builtin_memset(&pvh_bootparams, 0, sizeof(pvh_bootparams));
+	__inline_memset(&pvh_bootparams, 0, sizeof(pvh_bootparams));
 
 	hypervisor_specific_init(xen_guest);
 

-- 
2.47.3

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

[Peter Zijlstra]

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>).
> 
> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
> ---
>  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(-)
> 
> 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(). */
> +static __always_inline int __inline_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;
> +}
> +
> +#endif /* _ASM_X86_SHARED_STRING_H */
> diff --git a/arch/x86/include/asm/string.h b/arch/x86/include/asm/string.h
> index 9cb5aae7fba9ffcf0f5af8f939d30467750ccaa9..f0f4fd8227bf992e78c69209efb31f0a9a0cc3b1 100644
> --- a/arch/x86/include/asm/string.h
> +++ b/arch/x86/include/asm/string.h
> @@ -7,6 +7,7 @@
>  #else
>  # include <asm/string_64.h>
>  #endif
> +#include <asm/shared/string.h>
>  
>  static __always_inline void *__inline_memcpy(void *to, const void *from, size_t len)
>  {

It seems weird to have __inline_mem{cpy,set}() in a different header than
__inline_memcmp(). I'm assuming this is because boot cannot include the
normal string thing?

Perhaps make inline_string.h or somesuch to carry all three of them and
include that from the relevant places?

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

[Mauricio Faria de Oliveira]

On 2026-07-01 04:05, Peter Zijlstra 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>).
>> 
>> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
>> ---
>>  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(-)
>> 
>> 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(). */
>> +static __always_inline int __inline_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;
>> +}
>> +
>> +#endif /* _ASM_X86_SHARED_STRING_H */
>> diff --git a/arch/x86/include/asm/string.h b/arch/x86/include/asm/string.h
>> index 9cb5aae7fba9ffcf0f5af8f939d30467750ccaa9..f0f4fd8227bf992e78c69209efb31f0a9a0cc3b1 100644
>> --- a/arch/x86/include/asm/string.h
>> +++ b/arch/x86/include/asm/string.h
>> @@ -7,6 +7,7 @@
>>  #else
>>  # include <asm/string_64.h>
>>  #endif
>> +#include <asm/shared/string.h>
>>  
>>  static __always_inline void *__inline_memcpy(void *to, const void *from, size_t len)
>>  {
> 
> It seems weird to have __inline_mem{cpy,set}() in a different header than
> __inline_memcmp(). I'm assuming this is because boot cannot include the
> normal string thing?
> 
> Perhaps make inline_string.h or somesuch to carry all three of them and
> include that from the relevant places?

Thanks for looking into this.

Yes, your question and suggestion above are essentially patch 1 in v4
[1].

Borislav Petkov requested a clean header in arch/x86/include/asm/shared/
for this (without other code), which is patch 1 in v5.

cheers,

[1]
https://lore.kernel.org/all/20260526-pvh-kasan-inline-v4-1-a310e6a25ecd@igalia.com/

-- 
Mauricio

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

[Borislav Petkov]

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.

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

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

Thx.

-- 
Regards/Gruss,
    Boris.

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

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

[Mauricio Faria de Oliveira]

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

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

[Mauricio Faria de Oliveira]

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

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

[Borislav Petkov]

On Wed, Jul 01, 2026 at 04:57:57PM -0300, Mauricio Faria de Oliveira wrote:
> 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.

The Link tag already points to the review thread where we document who
suggested what. We don't document who gave what review comments and
suggestions for the simple reason that that would be intractable and the other
simple reason is that they don't belong in the commit message.

Changelog comments belong *exactly* under the "---" line.

-- 
Regards/Gruss,
    Boris.

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

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

[Borislav Petkov]

On Wed, Jul 01, 2026 at 05:03:04PM -0300, Mauricio Faria de Oliveira wrote:
> That would discard the credit attribution when applied, AFAIK. So,

Read my reply again.

-- 
Regards/Gruss,
    Boris.

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

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

[Mauricio Faria de Oliveira]

On 2026-07-01 17:46, Borislav Petkov wrote:
> On Wed, Jul 01, 2026 at 04:57:57PM -0300, Mauricio Faria de Oliveira wrote:
>> 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.
> 
> The Link tag already points to the review thread where we document who
> suggested what. We don't document who gave what review comments and
> suggestions for the simple reason that that would be intractable and the other
> simple reason is that they don't belong in the commit message.
> 
> Changelog comments belong *exactly* under the "---" line.

Ok, I'll send v6 addressing your feedback. Thanks.

-- 
Mauricio