[PATCH v7 0/5] x86/pvh: fix unbootable VMs again (PVH + KASAN)

[PATCH v7 0/5] x86/pvh: fix unbootable VMs again (PVH + KASAN)

[Mauricio Faria de Oliveira]

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 v7:
- Patch 2 (added):
  - Address pre-existing issues in 'asm' (Borislav Petkov, Sashiko).
- Link to v6: https://lore.kernel.org/r/20260701-pvh-kasan-inline-v6-0-ba99045dfa9f@igalia.com

Changes in v6:
- Patch 1:
  - Explain the return value difference between __inline_memcmp() and memcmp().
- Patch 2 (added):
  - Group __inline string functions in <asm/shared/string.h>.
- Link to v5: https://lore.kernel.org/r/20260630-pvh-kasan-inline-v5-0-52afc979be81@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 (5):
      x86/asm, x86/boot: expose inline memcmp
      x86/asm: add volatile, clobbers and zero-length check in inline memcmp
      x86/asm: group inline string functions
      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 | 50 ++++++++++++++++++++++++++++++++++++
 arch/x86/include/asm/string.h        | 21 +--------------
 arch/x86/platform/pvh/enlighten.c    |  3 ++-
 5 files changed, 56 insertions(+), 26 deletions(-)
---
base-commit: 3fe08b9796f36ef437ab9328e7dd1e5ff2d66603
change-id: 20260422-pvh-kasan-inline-6efac77f1b27

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

[PATCH v7 1/5] 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'.

Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>

---

Thanks to David Laight for noticing the return value difference between
inline and regular memcmp().
---
 arch/x86/boot/string.c               |  6 ++----
 arch/x86/include/asm/shared/string.h | 20 ++++++++++++++++++++
 2 files changed, 22 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..02b92927553f7b8e1c87e6122bbaa70439e57ea7
--- /dev/null
+++ b/arch/x86/include/asm/shared/string.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_SHARED_STRING_H
+#define _ASM_X86_SHARED_STRING_H
+
+/*
+ * This inline memcmp() returns 0 (equal) or 1 (not equal).
+ * The regular memcmp() returns <0 (less than), 0 (equal), or >0 (greater than)
+ * to indicate ordering as well.
+ */
+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 */

-- 
2.47.3

[PATCH v7 2/5] x86/asm: add volatile, clobbers and zero-length check in inline memcmp

[Mauricio Faria de Oliveira]

Add the volatile qualifier and clobbers parameter to prevent bugs with
instruction reordering and optimization.

Also check the zero-length case, as the 'repe' prefix does not run the
'cmpsb' instruction if the 'count' register is zero, which doesn't set
the condition-code/zero flag, so the result is based on a stale flag.

Those are pre-existing issues found by Sashiko.

Link: https://sashiko.dev/#/patchset/20260701-pvh-kasan-inline-v6-0-ba99045dfa9f%40igalia.com
Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
---
 arch/x86/include/asm/shared/string.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
index 02b92927553f7b8e1c87e6122bbaa70439e57ea7..166274e44f3cb49e3dccab3cdac281d67aef5d44 100644
--- a/arch/x86/include/asm/shared/string.h
+++ b/arch/x86/include/asm/shared/string.h
@@ -11,8 +11,12 @@ static __always_inline int __inline_memcmp(const void *s1, const void *s2, size_
 {
 	bool diff;
 
-	asm("repe cmpsb"
-	    : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
+	if (len == 0)
+		return 0;
+
+	asm volatile("repe cmpsb"
+		     : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len)
+		     : : "cc", "memory");
 
 	return diff;
 }

-- 
2.47.3

[PATCH v7 3/5] x86/asm: group inline string functions

[Mauricio Faria de Oliveira]

Group the __inline string functions in the same header.

Use <asm/shared/string.h> since __inline_memcmp() must remain there for use
by arch/x86/boot/string.c.

Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
---
 arch/x86/include/asm/shared/string.h | 26 ++++++++++++++++++++++++++
 arch/x86/include/asm/string.h        | 21 +--------------------
 2 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
index 166274e44f3cb49e3dccab3cdac281d67aef5d44..9714fbda8b734e20686bfb88a6ca857bfee59e1a 100644
--- a/arch/x86/include/asm/shared/string.h
+++ b/arch/x86/include/asm/shared/string.h
@@ -2,6 +2,32 @@
 #ifndef _ASM_X86_SHARED_STRING_H
 #define _ASM_X86_SHARED_STRING_H
 
+/*
+ * The __inline string functions are grouped in this file for consistency and
+ * for use by arch/x86/boot code due to limitations on including asm/string.h.
+ */
+
+static __always_inline void *__inline_memcpy(void *to, const void *from, size_t len)
+{
+	void *ret = to;
+
+	asm volatile("rep movsb"
+		     : "+D" (to), "+S" (from), "+c" (len)
+		     : : "memory");
+	return ret;
+}
+
+static __always_inline void *__inline_memset(void *s, int v, size_t n)
+{
+	void *ret = s;
+
+	asm volatile("rep stosb"
+		     : "+D" (s), "+c" (n)
+		     : "a" ((uint8_t)v)
+		     : "memory");
+	return ret;
+}
+
 /*
  * This inline memcmp() returns 0 (equal) or 1 (not equal).
  * The regular memcmp() returns <0 (less than), 0 (equal), or >0 (greater than)
diff --git a/arch/x86/include/asm/string.h b/arch/x86/include/asm/string.h
index 9cb5aae7fba9ffcf0f5af8f939d30467750ccaa9..dbf59f0d4cca71e2ddce0d8764aeec8782236669 100644
--- a/arch/x86/include/asm/string.h
+++ b/arch/x86/include/asm/string.h
@@ -8,25 +8,6 @@
 # include <asm/string_64.h>
 #endif
 
-static __always_inline void *__inline_memcpy(void *to, const void *from, size_t len)
-{
-	void *ret = to;
-
-	asm volatile("rep movsb"
-		     : "+D" (to), "+S" (from), "+c" (len)
-		     : : "memory");
-	return ret;
-}
-
-static __always_inline void *__inline_memset(void *s, int v, size_t n)
-{
-	void *ret = s;
-
-	asm volatile("rep stosb"
-		     : "+D" (s), "+c" (n)
-		     : "a" ((uint8_t)v)
-		     : "memory");
-	return ret;
-}
+#include <asm/shared/string.h>
 
 #endif /* _ASM_X86_STRING_H */

-- 
2.47.3

[PATCH v7 4/5] 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 v7 5/5] 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 v7 2/5] x86/asm: add volatile, clobbers and zero-length check in inline memcmp

[Borislav Petkov]

hpa in To:.

On Tue, Jul 21, 2026 at 12:56:43PM -0300, Mauricio Faria de Oliveira wrote:
> Add the volatile qualifier and clobbers parameter to prevent bugs with
> instruction reordering and optimization.
> 
> Also check the zero-length case, as the 'repe' prefix does not run the
> 'cmpsb' instruction if the 'count' register is zero, which doesn't set

Please use capital letters for insns: REPE, CMPSB and you don't need to put
words in '' - it reads fine without them.

> the condition-code/zero flag, so the result is based on a stale flag.
> 
> Those are pre-existing issues found by Sashiko.
> 
> Link: https://sashiko.dev/#/patchset/20260701-pvh-kasan-inline-v6-0-ba99045dfa9f%40igalia.com
> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
> ---
>  arch/x86/include/asm/shared/string.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
> index 02b92927553f7b8e1c87e6122bbaa70439e57ea7..166274e44f3cb49e3dccab3cdac281d67aef5d44 100644
> --- a/arch/x86/include/asm/shared/string.h
> +++ b/arch/x86/include/asm/shared/string.h
> @@ -11,8 +11,12 @@ static __always_inline int __inline_memcmp(const void *s1, const void *s2, size_
>  {
>  	bool diff;
>  
> -	asm("repe cmpsb"
> -	    : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
> +	if (len == 0)
> +		return 0;
> +
> +	asm volatile("repe cmpsb"
> +		     : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len)
> +		     : : "cc", "memory");
>  
>  	return diff;
>  }

So this is a fix which should probably go to stable, I think. Going back into
git history, it points to

  62bd0337d0c4 ("Top header file for new x86 setup code")

from 2007. And we have carried it this way through the years and who knows
what hit this or not. So please make this the first patch in your set - you
can even send it separately so that I can get route it through stable.

Then you can base the rest ontop.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v7 2/5] x86/asm: add volatile, clobbers and zero-length check in inline memcmp

[H. Peter Anvin]

On July 22, 2026 10:03:34 AM PDT, Borislav Petkov <bp@alien8.de> wrote:
>hpa in To:.
>
>On Tue, Jul 21, 2026 at 12:56:43PM -0300, Mauricio Faria de Oliveira wrote:
>> Add the volatile qualifier and clobbers parameter to prevent bugs with
>> instruction reordering and optimization.
>> 
>> Also check the zero-length case, as the 'repe' prefix does not run the
>> 'cmpsb' instruction if the 'count' register is zero, which doesn't set
>
>Please use capital letters for insns: REPE, CMPSB and you don't need to put
>words in '' - it reads fine without them.
>
>> the condition-code/zero flag, so the result is based on a stale flag.
>> 
>> Those are pre-existing issues found by Sashiko.
>> 
>> Link: https://sashiko.dev/#/patchset/20260701-pvh-kasan-inline-v6-0-ba99045dfa9f%40igalia.com
>> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
>> ---
>>  arch/x86/include/asm/shared/string.h | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
>> index 02b92927553f7b8e1c87e6122bbaa70439e57ea7..166274e44f3cb49e3dccab3cdac281d67aef5d44 100644
>> --- a/arch/x86/include/asm/shared/string.h
>> +++ b/arch/x86/include/asm/shared/string.h
>> @@ -11,8 +11,12 @@ static __always_inline int __inline_memcmp(const void *s1, const void *s2, size_
>>  {
>>  	bool diff;
>>  
>> -	asm("repe cmpsb"
>> -	    : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
>> +	if (len == 0)
>> +		return 0;
>> +
>> +	asm volatile("repe cmpsb"
>> +		     : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len)
>> +		     : : "cc", "memory");
>>  
>>  	return diff;
>>  }
>
>So this is a fix which should probably go to stable, I think. Going back into
>git history, it points to
>
>  62bd0337d0c4 ("Top header file for new x86 setup code")
>
>from 2007. And we have carried it this way through the years and who knows
>what hit this or not. So please make this the first patch in your set - you
>can even send it separately so that I can get route it through stable.
>
>Then you can base the rest ontop.
>
>Thx.
>

Also, this is silly. Instead of adding a whole separate test, just do "test %3,%3" before the repe to set ZF and let the REPE skip.

Re: [PATCH v7 2/5] x86/asm: add volatile, clobbers and zero-length check in inline memcmp

[H. Peter Anvin]

On July 22, 2026 10:03:34 AM PDT, Borislav Petkov <bp@alien8.de> wrote:
>hpa in To:.
>
>On Tue, Jul 21, 2026 at 12:56:43PM -0300, Mauricio Faria de Oliveira wrote:
>> Add the volatile qualifier and clobbers parameter to prevent bugs with
>> instruction reordering and optimization.
>> 
>> Also check the zero-length case, as the 'repe' prefix does not run the
>> 'cmpsb' instruction if the 'count' register is zero, which doesn't set
>
>Please use capital letters for insns: REPE, CMPSB and you don't need to put
>words in '' - it reads fine without them.
>
>> the condition-code/zero flag, so the result is based on a stale flag.
>> 
>> Those are pre-existing issues found by Sashiko.
>> 
>> Link: https://sashiko.dev/#/patchset/20260701-pvh-kasan-inline-v6-0-ba99045dfa9f%40igalia.com
>> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
>> ---
>>  arch/x86/include/asm/shared/string.h | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
>> index 02b92927553f7b8e1c87e6122bbaa70439e57ea7..166274e44f3cb49e3dccab3cdac281d67aef5d44 100644
>> --- a/arch/x86/include/asm/shared/string.h
>> +++ b/arch/x86/include/asm/shared/string.h
>> @@ -11,8 +11,12 @@ static __always_inline int __inline_memcmp(const void *s1, const void *s2, size_
>>  {
>>  	bool diff;
>>  
>> -	asm("repe cmpsb"
>> -	    : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
>> +	if (len == 0)
>> +		return 0;
>> +
>> +	asm volatile("repe cmpsb"
>> +		     : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len)
>> +		     : : "cc", "memory");
>>  
>>  	return diff;
>>  }
>
>So this is a fix which should probably go to stable, I think. Going back into
>git history, it points to
>
>  62bd0337d0c4 ("Top header file for new x86 setup code")
>
>from 2007. And we have carried it this way through the years and who knows
>what hit this or not. So please make this the first patch in your set - you
>can even send it separately so that I can get route it through stable.
>
>Then you can base the rest ontop.
>
>Thx.
>

I suspect that the zero-length case simply can't happen in the real code, but who knows; the use of the code has expanded over the years.

Re: [PATCH v7 2/5] x86/asm: add volatile, clobbers and zero-length check in inline memcmp

[Jan Beulich]

On 22.07.2026 20:45, H. Peter Anvin wrote:
> On July 22, 2026 10:03:34 AM PDT, Borislav Petkov <bp@alien8.de> wrote:
>> hpa in To:.
>>
>> On Tue, Jul 21, 2026 at 12:56:43PM -0300, Mauricio Faria de Oliveira wrote:
>>> Add the volatile qualifier and clobbers parameter to prevent bugs with
>>> instruction reordering and optimization.
>>>
>>> Also check the zero-length case, as the 'repe' prefix does not run the
>>> 'cmpsb' instruction if the 'count' register is zero, which doesn't set
>>
>> Please use capital letters for insns: REPE, CMPSB and you don't need to put
>> words in '' - it reads fine without them.
>>
>>> the condition-code/zero flag, so the result is based on a stale flag.
>>>
>>> Those are pre-existing issues found by Sashiko.
>>>
>>> Link: https://sashiko.dev/#/patchset/20260701-pvh-kasan-inline-v6-0-ba99045dfa9f%40igalia.com
>>> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
>>> ---
>>>  arch/x86/include/asm/shared/string.h | 8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
>>> index 02b92927553f7b8e1c87e6122bbaa70439e57ea7..166274e44f3cb49e3dccab3cdac281d67aef5d44 100644
>>> --- a/arch/x86/include/asm/shared/string.h
>>> +++ b/arch/x86/include/asm/shared/string.h
>>> @@ -11,8 +11,12 @@ static __always_inline int __inline_memcmp(const void *s1, const void *s2, size_
>>>  {
>>>  	bool diff;
>>>  
>>> -	asm("repe cmpsb"
>>> -	    : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
>>> +	if (len == 0)
>>> +		return 0;
>>> +
>>> +	asm volatile("repe cmpsb"
>>> +		     : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len)
>>> +		     : : "cc", "memory");
>>>  
>>>  	return diff;
>>>  }
>>
>> So this is a fix which should probably go to stable, I think. Going back into
>> git history, it points to
>>
>>  62bd0337d0c4 ("Top header file for new x86 setup code")
>>
>>from 2007. And we have carried it this way through the years and who knows
>> what hit this or not. So please make this the first patch in your set - you
>> can even send it separately so that I can get route it through stable.
>>
>> Then you can base the rest ontop.
>>
>> Thx.
>>
> 
> Also, this is silly. Instead of adding a whole separate test, just do "test %3,%3" before the repe to set ZF and let the REPE skip.

Besides this, isn't the function effectively returning bool wrong anyway? This
way you can use it for equal / not-equal comparisons, but not for sorting and
alike.

Jan

Re: [PATCH v7 2/5] x86/asm: add volatile, clobbers and zero-length check in inline memcmp

[Brian Gerst]

On Wed, Jul 22, 2026 at 2:53 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> On July 22, 2026 10:03:34 AM PDT, Borislav Petkov <bp@alien8.de> wrote:
> >hpa in To:.
> >
> >On Tue, Jul 21, 2026 at 12:56:43PM -0300, Mauricio Faria de Oliveira wrote:
> >> Add the volatile qualifier and clobbers parameter to prevent bugs with
> >> instruction reordering and optimization.
> >>
> >> Also check the zero-length case, as the 'repe' prefix does not run the
> >> 'cmpsb' instruction if the 'count' register is zero, which doesn't set
> >
> >Please use capital letters for insns: REPE, CMPSB and you don't need to put
> >words in '' - it reads fine without them.
> >
> >> the condition-code/zero flag, so the result is based on a stale flag.
> >>
> >> Those are pre-existing issues found by Sashiko.
> >>
> >> Link: https://sashiko.dev/#/patchset/20260701-pvh-kasan-inline-v6-0-ba99045dfa9f%40igalia.com
> >> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
> >> ---
> >>  arch/x86/include/asm/shared/string.h | 8 ++++++--
> >>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
> >> index 02b92927553f7b8e1c87e6122bbaa70439e57ea7..166274e44f3cb49e3dccab3cdac281d67aef5d44 100644
> >> --- a/arch/x86/include/asm/shared/string.h
> >> +++ b/arch/x86/include/asm/shared/string.h
> >> @@ -11,8 +11,12 @@ static __always_inline int __inline_memcmp(const void *s1, const void *s2, size_
> >>  {
> >>      bool diff;
> >>
> >> -    asm("repe cmpsb"
> >> -        : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
> >> +    if (len == 0)
> >> +            return 0;
> >> +
> >> +    asm volatile("repe cmpsb"
> >> +                 : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len)
> >> +                 : : "cc", "memory");
> >>
> >>      return diff;
> >>  }
> >
> >So this is a fix which should probably go to stable, I think. Going back into
> >git history, it points to
> >
> >  62bd0337d0c4 ("Top header file for new x86 setup code")
> >
> >from 2007. And we have carried it this way through the years and who knows
> >what hit this or not. So please make this the first patch in your set - you
> >can even send it separately so that I can get route it through stable.
> >
> >Then you can base the rest ontop.
> >
> >Thx.
> >
>
> Also, this is silly. Instead of adding a whole separate test, just do "test %3,%3" before the repe to set ZF and let the REPE skip.

REPE does not check ZF before the first iteration.

From the Intel SDM:
"When the REPE/REPZ and REPNE/REPNZ prefixes are used, the ZF flag
does not require initialization because both
the CMPS and SCAS instructions affect the ZF flag according to the
results of the comparisons they make."

Re: [PATCH v7 3/5] x86/asm: group inline string functions

[Borislav Petkov]

On Tue, Jul 21, 2026 at 12:56:44PM -0300, Mauricio Faria de Oliveira wrote:
> Group the __inline string functions in the same header.
> 
> Use <asm/shared/string.h> since __inline_memcmp() must remain there for use
> by arch/x86/boot/string.c.

"No functional changes."

> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
> ---
>  arch/x86/include/asm/shared/string.h | 26 ++++++++++++++++++++++++++
>  arch/x86/include/asm/string.h        | 21 +--------------------
>  2 files changed, 27 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
> index 166274e44f3cb49e3dccab3cdac281d67aef5d44..9714fbda8b734e20686bfb88a6ca857bfee59e1a 100644
> --- a/arch/x86/include/asm/shared/string.h
> +++ b/arch/x86/include/asm/shared/string.h
> @@ -2,6 +2,32 @@
>  #ifndef _ASM_X86_SHARED_STRING_H
>  #define _ASM_X86_SHARED_STRING_H
>  
> +/*
> + * The __inline string functions are grouped in this file for consistency and
> + * for use by arch/x86/boot code due to limitations on including asm/string.h.

What limitations?

Either explain them or don't bother adding a comment. asm/shared/ is generally
an understood concept if you grep the tree and you don't really need a comment
here.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v7 2/5] x86/asm: add volatile, clobbers and zero-length check in inline memcmp

[H. Peter Anvin]

On 2026-07-22 23:59, Jan Beulich wrote:
>>
>> Also, this is silly. Instead of adding a whole separate test, just do "test %3,%3" before the repe to set ZF and let the REPE skip.
> 
> Besides this, isn't the function effectively returning bool wrong anyway? This
> way you can use it for equal / not-equal comparisons, but not for sorting and
> alike.
> 
There is no use case in the early code for sorting, and it seems rather broken
to burden the code with that.

That being said it probably should return bool explicitly (it makes no sense
for the prototype to be different than the internal variable.)

We could call it memneq() if someone really, really cares, I guess.

The early code is very size-sensitive, so I'm really not fond of the idea of
burdening it further. Perhaps something like:

A memory clobber is ugly here since no memory is actually modified, although
it probably doesn't affect code; "cc" is completely redundant with condition
code output operand.

static __always_inline bool
__inline_memcmp(const void *s1, const void *s2, size_t len)
{
	bool diff;

	if (__builtin_constant_p(len == 0)) {
		if (!len)
			return false;
		asm volatile("repe cmpsb"
				: "=@ccnz" (diff),
				  "+D" (s1), "+S" (s2), "+c" (len)
				: : "memory");
	} else {
		/* Clear ZF beforehand in case len == 0 */
		asm volatile("test %3,%3; repe cmpsb"
				: "=@ccnz" (diff),
				  "+D" (s1), "+S" (s2), "+c" (len)
				: : "memory");
	}
	return diff;
}

Re: [PATCH v7 2/5] x86/asm: add volatile, clobbers and zero-length check in inline memcmp

[H. Peter Anvin]

On 2026-07-23 12:37, Brian Gerst wrote:
> On Wed, Jul 22, 2026 at 2:53 PM H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> On July 22, 2026 10:03:34 AM PDT, Borislav Petkov <bp@alien8.de> wrote:
>>> hpa in To:.
>>>
>>> On Tue, Jul 21, 2026 at 12:56:43PM -0300, Mauricio Faria de Oliveira wrote:
>>>> Add the volatile qualifier and clobbers parameter to prevent bugs with
>>>> instruction reordering and optimization.
>>>>
>>>> Also check the zero-length case, as the 'repe' prefix does not run the
>>>> 'cmpsb' instruction if the 'count' register is zero, which doesn't set
>>>
>>> Please use capital letters for insns: REPE, CMPSB and you don't need to put
>>> words in '' - it reads fine without them.
>>>
>>>> the condition-code/zero flag, so the result is based on a stale flag.
>>>>
>>>> Those are pre-existing issues found by Sashiko.
>>>>
>>>> Link: https://sashiko.dev/#/patchset/20260701-pvh-kasan-inline-v6-0-ba99045dfa9f%40igalia.com
>>>> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
>>>> ---
>>>>  arch/x86/include/asm/shared/string.h | 8 ++++++--
>>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
>>>> index 02b92927553f7b8e1c87e6122bbaa70439e57ea7..166274e44f3cb49e3dccab3cdac281d67aef5d44 100644
>>>> --- a/arch/x86/include/asm/shared/string.h
>>>> +++ b/arch/x86/include/asm/shared/string.h
>>>> @@ -11,8 +11,12 @@ static __always_inline int __inline_memcmp(const void *s1, const void *s2, size_
>>>>  {
>>>>      bool diff;
>>>>
>>>> -    asm("repe cmpsb"
>>>> -        : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
>>>> +    if (len == 0)
>>>> +            return 0;
>>>> +
>>>> +    asm volatile("repe cmpsb"
>>>> +                 : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len)
>>>> +                 : : "cc", "memory");
>>>>
>>>>      return diff;
>>>>  }
>>>
>>> So this is a fix which should probably go to stable, I think. Going back into
>>> git history, it points to
>>>
>>>  62bd0337d0c4 ("Top header file for new x86 setup code")
>>>
>> >from 2007. And we have carried it this way through the years and who knows
>>> what hit this or not. So please make this the first patch in your set - you
>>> can even send it separately so that I can get route it through stable.
>>>
>>> Then you can base the rest ontop.
>>>
>>> Thx.
>>>
>>
>> Also, this is silly. Instead of adding a whole separate test, just do "test %3,%3" before the repe to set ZF and let the REPE skip.
> 
> REPE does not check ZF before the first iteration.
> 
> From the Intel SDM:
> "When the REPE/REPZ and REPNE/REPNZ prefixes are used, the ZF flag
> does not require initialization because both
> the CMPS and SCAS instructions affect the ZF flag according to the
> results of the comparisons they make."
> 

The point was to ensure we have a valid result in ZF (which should be ZF=1)
even if len = 0.

	-hpa

Re: [PATCH v7 2/5] x86/asm: add volatile, clobbers and zero-length check in inline memcmp

[Mauricio Faria de Oliveira]

On 2026-07-22 14:03, Borislav Petkov wrote:
> hpa in To:.
> 
> On Tue, Jul 21, 2026 at 12:56:43PM -0300, Mauricio Faria de Oliveira wrote:
>> Add the volatile qualifier and clobbers parameter to prevent bugs with
>> instruction reordering and optimization.
>> 
>> Also check the zero-length case, as the 'repe' prefix does not run the
>> 'cmpsb' instruction if the 'count' register is zero, which doesn't set
> 
> Please use capital letters for insns: REPE, CMPSB and you don't need to put
> words in '' - it reads fine without them.

Ok.

> 
>> the condition-code/zero flag, so the result is based on a stale flag.
>> 
>> Those are pre-existing issues found by Sashiko.
>> 
>> Link: https://sashiko.dev/#/patchset/20260701-pvh-kasan-inline-v6-0-ba99045dfa9f%40igalia.com
>> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
>> ---
>>  arch/x86/include/asm/shared/string.h | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
>> index 02b92927553f7b8e1c87e6122bbaa70439e57ea7..166274e44f3cb49e3dccab3cdac281d67aef5d44 100644
>> --- a/arch/x86/include/asm/shared/string.h
>> +++ b/arch/x86/include/asm/shared/string.h
>> @@ -11,8 +11,12 @@ static __always_inline int __inline_memcmp(const void *s1, const void *s2, size_
>>  {
>>  	bool diff;
>>  
>> -	asm("repe cmpsb"
>> -	    : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
>> +	if (len == 0)
>> +		return 0;
>> +
>> +	asm volatile("repe cmpsb"
>> +		     : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len)
>> +		     : : "cc", "memory");
>>  
>>  	return diff;
>>  }
> 
> So this is a fix which should probably go to stable, I think. Going back into
> git history, it points to
> 
>   62bd0337d0c4 ("Top header file for new x86 setup code")
> 
> from 2007. And we have carried it this way through the years and who knows
> what hit this or not. So please make this the first patch in your set - you
> can even send it separately so that I can get route it through stable.

Sure:
https://lore.kernel.org/all/20260723-x86-memcmp-asm-v2-1-d93ecb43797f@igalia.com/

> Then you can base the rest ontop.

Done:
https://lore.kernel.org/all/20260723-pvh-kasan-inline-v8-0-c1f62c156f52@igalia.com/

Thanks,

> 
> Thx.

-- 
Mauricio

Re: [PATCH v7 2/5] x86/asm: add volatile, clobbers and zero-length check in inline memcmp

[Mauricio Faria de Oliveira]

On 2026-07-22 15:45, H. Peter Anvin wrote:
> On July 22, 2026 10:03:34 AM PDT, Borislav Petkov <bp@alien8.de> wrote:
>>hpa in To:.
>>
>>On Tue, Jul 21, 2026 at 12:56:43PM -0300, Mauricio Faria de Oliveira wrote:
>>> Add the volatile qualifier and clobbers parameter to prevent bugs with
>>> instruction reordering and optimization.
>>> 
>>> Also check the zero-length case, as the 'repe' prefix does not run the
>>> 'cmpsb' instruction if the 'count' register is zero, which doesn't set
>>
>>Please use capital letters for insns: REPE, CMPSB and you don't need to put
>>words in '' - it reads fine without them.
>>
>>> the condition-code/zero flag, so the result is based on a stale flag.
>>> 
>>> Those are pre-existing issues found by Sashiko.
>>> 
>>> Link: https://sashiko.dev/#/patchset/20260701-pvh-kasan-inline-v6-0-ba99045dfa9f%40igalia.com
>>> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
>>> ---
>>>  arch/x86/include/asm/shared/string.h | 8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
>>> index 02b92927553f7b8e1c87e6122bbaa70439e57ea7..166274e44f3cb49e3dccab3cdac281d67aef5d44 100644
>>> --- a/arch/x86/include/asm/shared/string.h
>>> +++ b/arch/x86/include/asm/shared/string.h
>>> @@ -11,8 +11,12 @@ static __always_inline int __inline_memcmp(const void *s1, const void *s2, size_
>>>  {
>>>  	bool diff;
>>>  
>>> -	asm("repe cmpsb"
>>> -	    : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
>>> +	if (len == 0)
>>> +		return 0;
>>> +
>>> +	asm volatile("repe cmpsb"
>>> +		     : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len)
>>> +		     : : "cc", "memory");
>>>  
>>>  	return diff;
>>>  }
>>
>>So this is a fix which should probably go to stable, I think. Going back into
>>git history, it points to
>>
>>  62bd0337d0c4 ("Top header file for new x86 setup code")
>>
>>from 2007. And we have carried it this way through the years and who knows
>>what hit this or not. So please make this the first patch in your set - you
>>can even send it separately so that I can get route it through stable.
>>
>>Then you can base the rest ontop.
>>
>>Thx.
>>
> 
> Also, this is silly. Instead of adding a whole separate test, just do "test %3,%3" before the repe to set ZF and let the REPE skip.

This is elegant; done. Thanks!

-- 
Mauricio

Re: [PATCH v7 2/5] x86/asm: add volatile, clobbers and zero-length check in inline memcmp

[Mauricio Faria de Oliveira]

On 2026-07-22 15:48, H. Peter Anvin wrote:
> On July 22, 2026 10:03:34 AM PDT, Borislav Petkov <bp@alien8.de> wrote:
>>hpa in To:.
>>
>>On Tue, Jul 21, 2026 at 12:56:43PM -0300, Mauricio Faria de Oliveira wrote:
>>> Add the volatile qualifier and clobbers parameter to prevent bugs with
>>> instruction reordering and optimization.
>>> 
>>> Also check the zero-length case, as the 'repe' prefix does not run the
>>> 'cmpsb' instruction if the 'count' register is zero, which doesn't set
>>
>>Please use capital letters for insns: REPE, CMPSB and you don't need to put
>>words in '' - it reads fine without them.
>>
>>> the condition-code/zero flag, so the result is based on a stale flag.
>>> 
>>> Those are pre-existing issues found by Sashiko.
>>> 
>>> Link: https://sashiko.dev/#/patchset/20260701-pvh-kasan-inline-v6-0-ba99045dfa9f%40igalia.com
>>> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
>>> ---
>>>  arch/x86/include/asm/shared/string.h | 8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
>>> index 02b92927553f7b8e1c87e6122bbaa70439e57ea7..166274e44f3cb49e3dccab3cdac281d67aef5d44 100644
>>> --- a/arch/x86/include/asm/shared/string.h
>>> +++ b/arch/x86/include/asm/shared/string.h
>>> @@ -11,8 +11,12 @@ static __always_inline int __inline_memcmp(const void *s1, const void *s2, size_
>>>  {
>>>  	bool diff;
>>>  
>>> -	asm("repe cmpsb"
>>> -	    : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
>>> +	if (len == 0)
>>> +		return 0;
>>> +
>>> +	asm volatile("repe cmpsb"
>>> +		     : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len)
>>> +		     : : "cc", "memory");
>>>  
>>>  	return diff;
>>>  }
>>
>>So this is a fix which should probably go to stable, I think. Going back into
>>git history, it points to
>>
>>  62bd0337d0c4 ("Top header file for new x86 setup code")
>>
>>from 2007. And we have carried it this way through the years and who knows
>>what hit this or not. So please make this the first patch in your set - you
>>can even send it separately so that I can get route it through stable.
>>
>>Then you can base the rest ontop.
>>
>>Thx.
>>
> 
> I suspect that the zero-length case simply can't happen in the real code, but who knows; the use of the code has expanded over the years.

Agreed, and its use may expand more in the future, being in a header
(e.g., this very series :)

Thanks,

-- 
Mauricio

Re: [PATCH v7 2/5] x86/asm: add volatile, clobbers and zero-length check in inline memcmp

[Mauricio Faria de Oliveira]

On 2026-07-23 20:12, H. Peter Anvin wrote:
> On 2026-07-22 23:59, Jan Beulich wrote:
>>> 
>>> Also, this is silly. Instead of adding a whole separate test, just do "test %3,%3" before the repe to set ZF and let the REPE skip.
>> 
>> Besides this, isn't the function effectively returning bool wrong anyway? This
>> way you can use it for equal / not-equal comparisons, but not for sorting and
>> alike.
>> 
> There is no use case in the early code for sorting, and it seems rather broken
> to burden the code with that.

If this is merged, other users not in early code might eventually
appear, which might introduce the use case for sorting. Even though that
is unlikely, as such users could probably use regular memcmp() instead,
consider that just in case, so I can ask for your input/advice on this:

> That being said it probably should return bool explicitly (it makes no sense
> for the prototype to be different than the internal variable.)
> 
> We could call it memneq() if someone really, really cares, I guess.
> 
> The early code is very size-sensitive, so I'm really not fond of the idea of
> burdening it further. Perhaps something like:

Do you think this implementation (returns -1/0/+1) is reasonable,
size-wise, for early code?

I considered submitting it eventually, once the return value difference
to regular memcmp() was called out [0], as an improvement, if there's
agreement this would be a good idea considering the scenario above.

static __always_inline int __inline_memcmp(const void *s1, const void
*s2, size_t len)
{
        int above, below;

        asm volatile("test %2, %2\n\t"
                     "repe cmpsb"
                     : "+S" (s1), "+D" (s2), "+c" (len),
                       "=@cca" (above), "=@ccb" (below)
                     : : "memory");

        return above - below;
}

@ arch/x86/boot/string.o
00000028 <memcmp>:
  28:   66 57                   push   %di
  2a:   66 56                   push   %si
  2c:   66 89 c6                mov    %ax,%si
  2f:   66 89 d7                mov    %dx,%di

  32:   66 85 c9                test   %cx,%cx
  35:   f3 a6                   repz cmpsb %es:(%edi),%ds:(%esi)
  37:   0f 97 c0                seta   %al
  3a:   66 0f b6 c0             movzbw %al,%ax
  3e:   0f 92 c2                setb   %dl
  41:   66 0f b6 d2             movzbw %dl,%dx
  45:   66 29 d0                sub    %dx,%ax

  48:   66 5e                   pop    %si
  4a:   66 5f                   pop    %di
  4c:   66 c3                   retw

> A memory clobber is ugly here since no memory is actually modified, although

I'm definitely not an expert, but IIUIC, this memory clobber is for
reads, not writes? Say, a caller/optimized code that writes to the
buffer(s) prior to __inline_memcmp() and data might still reside in
registers; even if theoretical/unlikely.

As in [1]:

  The "memory" clobber tells the compiler that the assembly code
performs memory reads or writes [...] (for example, accessing the memory
pointed to [...]). To ensure memory contains correct values, GCC may
need to flush specific register values to memory before executing the
asm.

> it probably doesn't affect code; "cc" is completely redundant with condition
> code output operand.

Thanks for explaining. I'll remove that in the next version.

[0]
https://lore.kernel.org/all/324ef97b16f52e0ccc72f6381d1b5dd2@igalia.com/
[1]
https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Clobbers-and-Scratch-Registers-1

cheers,

> static __always_inline bool
> __inline_memcmp(const void *s1, const void *s2, size_t len)
> {
> 	bool diff;
> 
> 	if (__builtin_constant_p(len == 0)) {
> 		if (!len)
> 			return false;
> 		asm volatile("repe cmpsb"
> 				: "=@ccnz" (diff),
> 				  "+D" (s1), "+S" (s2), "+c" (len)
> 				: : "memory");
> 	} else {
> 		/* Clear ZF beforehand in case len == 0 */
> 		asm volatile("test %3,%3; repe cmpsb"
> 				: "=@ccnz" (diff),
> 				  "+D" (s1), "+S" (s2), "+c" (len)
> 				: : "memory");
> 	}
> 	return diff;
> }


-- 
Mauricio

Re: [PATCH v7 2/5] x86/asm: add volatile, clobbers and zero-length check in inline memcmp

[Brian Gerst]

On Thu, Jul 23, 2026 at 7:13 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> On 2026-07-23 12:37, Brian Gerst wrote:
> > On Wed, Jul 22, 2026 at 2:53 PM H. Peter Anvin <hpa@zytor.com> wrote:
> >>
> >> On July 22, 2026 10:03:34 AM PDT, Borislav Petkov <bp@alien8.de> wrote:
> >>> hpa in To:.
> >>>
> >>> On Tue, Jul 21, 2026 at 12:56:43PM -0300, Mauricio Faria de Oliveira wrote:
> >>>> Add the volatile qualifier and clobbers parameter to prevent bugs with
> >>>> instruction reordering and optimization.
> >>>>
> >>>> Also check the zero-length case, as the 'repe' prefix does not run the
> >>>> 'cmpsb' instruction if the 'count' register is zero, which doesn't set
> >>>
> >>> Please use capital letters for insns: REPE, CMPSB and you don't need to put
> >>> words in '' - it reads fine without them.
> >>>
> >>>> the condition-code/zero flag, so the result is based on a stale flag.
> >>>>
> >>>> Those are pre-existing issues found by Sashiko.
> >>>>
> >>>> Link: https://sashiko.dev/#/patchset/20260701-pvh-kasan-inline-v6-0-ba99045dfa9f%40igalia.com
> >>>> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
> >>>> ---
> >>>>  arch/x86/include/asm/shared/string.h | 8 ++++++--
> >>>>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
> >>>> index 02b92927553f7b8e1c87e6122bbaa70439e57ea7..166274e44f3cb49e3dccab3cdac281d67aef5d44 100644
> >>>> --- a/arch/x86/include/asm/shared/string.h
> >>>> +++ b/arch/x86/include/asm/shared/string.h
> >>>> @@ -11,8 +11,12 @@ static __always_inline int __inline_memcmp(const void *s1, const void *s2, size_
> >>>>  {
> >>>>      bool diff;
> >>>>
> >>>> -    asm("repe cmpsb"
> >>>> -        : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
> >>>> +    if (len == 0)
> >>>> +            return 0;
> >>>> +
> >>>> +    asm volatile("repe cmpsb"
> >>>> +                 : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len)
> >>>> +                 : : "cc", "memory");
> >>>>
> >>>>      return diff;
> >>>>  }
> >>>
> >>> So this is a fix which should probably go to stable, I think. Going back into
> >>> git history, it points to
> >>>
> >>>  62bd0337d0c4 ("Top header file for new x86 setup code")
> >>>
> >> >from 2007. And we have carried it this way through the years and who knows
> >>> what hit this or not. So please make this the first patch in your set - you
> >>> can even send it separately so that I can get route it through stable.
> >>>
> >>> Then you can base the rest ontop.
> >>>
> >>> Thx.
> >>>
> >>
> >> Also, this is silly. Instead of adding a whole separate test, just do "test %3,%3" before the repe to set ZF and let the REPE skip.
> >
> > REPE does not check ZF before the first iteration.
> >
> > From the Intel SDM:
> > "When the REPE/REPZ and REPNE/REPNZ prefixes are used, the ZF flag
> > does not require initialization because both
> > the CMPS and SCAS instructions affect the ZF flag according to the
> > results of the comparisons they make."
> >
>
> The point was to ensure we have a valid result in ZF (which should be ZF=1)
> even if len = 0.

I had originally thought that the test for RCX==0 was also at the end
of the loop, after the decrement.  But a closer look at the
pseudo-code in the SDM does show that the test is at the start of the
loop.  Starting with RCX==0 does indeed skip the comparison and does
not touch ZF.  Your suggestion is good, but should have a comment
explaining why it's there to someone reading the code in the future.

Re: [PATCH v7 3/5] x86/asm: group inline string functions

[Mauricio Faria de Oliveira]

On 2026-07-23 18:59, Borislav Petkov wrote:
> On Tue, Jul 21, 2026 at 12:56:44PM -0300, Mauricio Faria de Oliveira wrote:
>> Group the __inline string functions in the same header.
>> 
>> Use <asm/shared/string.h> since __inline_memcmp() must remain there for use
>> by arch/x86/boot/string.c.
> 
> "No functional changes."

Ok, added.

> 
>> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
>> ---
>>  arch/x86/include/asm/shared/string.h | 26 ++++++++++++++++++++++++++
>>  arch/x86/include/asm/string.h        | 21 +--------------------
>>  2 files changed, 27 insertions(+), 20 deletions(-)
>> 
>> diff --git a/arch/x86/include/asm/shared/string.h b/arch/x86/include/asm/shared/string.h
>> index 166274e44f3cb49e3dccab3cdac281d67aef5d44..9714fbda8b734e20686bfb88a6ca857bfee59e1a 100644
>> --- a/arch/x86/include/asm/shared/string.h
>> +++ b/arch/x86/include/asm/shared/string.h
>> @@ -2,6 +2,32 @@
>>  #ifndef _ASM_X86_SHARED_STRING_H
>>  #define _ASM_X86_SHARED_STRING_H
>>  
>> +/*
>> + * The __inline string functions are grouped in this file for consistency and
>> + * for use by arch/x86/boot code due to limitations on including asm/string.h.
> 
> What limitations?
> 
> Either explain them or don't bother adding a comment. asm/shared/ is generally
> an understood concept if you grep the tree and you don't really need a comment
> here.

Ok, removed.

Thanks,

-- 
Mauricio