[PATCH -tip] x86/idle: Work around LLVM assembler bug with MONITOR and MWAIT insn

[PATCH -tip] x86/idle: Work around LLVM assembler bug with MONITOR and MWAIT insn

[Uros Bizjak]

LLVM assembler is not able to assemble correct forms of MONITOR
and MWAIT instructions with explicit operands:

  error: invalid operand for instruction
          monitor %rax,%ecx,%edx
                       ^~~~

Use instruction mnemonics with implicit operands when LLVM assembler
is detected to work around this issue.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Fixes: cd3b85b27542 ("x86/idle: Use MONITOR and MWAIT mnemonics in <asm/mwait.h>")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202504030802.2lEVBSpN-lkp@intel.com/
---
 arch/x86/include/asm/mwait.h | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/mwait.h b/arch/x86/include/asm/mwait.h
index 5141d2acc0ef..d956d1be4873 100644
--- a/arch/x86/include/asm/mwait.h
+++ b/arch/x86/include/asm/mwait.h
@@ -25,9 +25,18 @@
 #define TPAUSE_C01_STATE		1
 #define TPAUSE_C02_STATE		0
 
+#ifdef CONFIG_LD_IS_LLD
+# define ASM_MONITOR	"monitor"
+# define ASM_MWAIT	"mwait"
+#else
+# define ASM_MONITOR	"monitor %[eax], %[ecx], %[edx]"
+# define ASM_MWAIT	"mwait %[eax], %[ecx]"
+#endif
+
 static __always_inline void __monitor(const void *eax, u32 ecx, u32 edx)
 {
-	asm volatile("monitor %0, %1, %2" :: "a" (eax), "c" (ecx), "d" (edx));
+	asm volatile(ASM_MONITOR
+		     :: [eax] "a" (eax), [ecx] "c" (ecx), [edx] "d" (edx));
 }
 
 static __always_inline void __monitorx(const void *eax, u32 ecx, u32 edx)
@@ -41,7 +50,7 @@ static __always_inline void __mwait(u32 eax, u32 ecx)
 {
 	mds_idle_clear_cpu_buffers();
 
-	asm volatile("mwait %0, %1" :: "a" (eax), "c" (ecx));
+	asm volatile(ASM_MWAIT :: [eax] "a" (eax), [ecx] "c" (ecx));
 }
 
 /*
@@ -92,7 +101,8 @@ static __always_inline void __sti_mwait(u32 eax, u32 ecx)
 {
 	mds_idle_clear_cpu_buffers();
 
-	asm volatile("sti; mwait %0, %1" :: "a" (eax), "c" (ecx));
+	asm volatile("sti\n\t"
+		     ASM_MWAIT :: [eax] "a" (eax), [ecx] "c" (ecx));
 }
 
 /*
-- 
2.49.0

Re: [PATCH -tip] x86/idle: Work around LLVM assembler bug with MONITOR and MWAIT insn

[Borislav Petkov]

On Thu, Apr 03, 2025 at 09:04:37AM +0200, Uros Bizjak wrote:
> LLVM assembler is not able to assemble correct forms of MONITOR
> and MWAIT instructions with explicit operands:
> 
>   error: invalid operand for instruction
>           monitor %rax,%ecx,%edx
>                        ^~~~
> 
> Use instruction mnemonics with implicit operands when LLVM assembler
> is detected to work around this issue.
> 
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Fixes: cd3b85b27542 ("x86/idle: Use MONITOR and MWAIT mnemonics in <asm/mwait.h>")

No, you should whack that one - the toolchains are clearly not ready yet...

> +#ifdef CONFIG_LD_IS_LLD
> +# define ASM_MONITOR	"monitor"
> +# define ASM_MWAIT	"mwait"
> +#else
> +# define ASM_MONITOR	"monitor %[eax], %[ecx], %[edx]"
> +# define ASM_MWAIT	"mwait %[eax], %[ecx]"
> +#endif

... instead of piling more ifdeffery ontop and making the code uglier than
before.

Go fix the LLVM assembler to deal with explicit operands or GCC and LLVM
should agree on common syntax they both assemble or whatever and the kernel
should do *one* thing exactly and not carry toolchain-specific ifdeffery at
every spot.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH -tip] x86/idle: Work around LLVM assembler bug with MONITOR and MWAIT insn

[Uros Bizjak]

On Thu, Apr 3, 2025 at 10:25 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Apr 03, 2025 at 09:04:37AM +0200, Uros Bizjak wrote:
> > LLVM assembler is not able to assemble correct forms of MONITOR
> > and MWAIT instructions with explicit operands:
> >
> >   error: invalid operand for instruction
> >           monitor %rax,%ecx,%edx
> >                        ^~~~
> >
> > Use instruction mnemonics with implicit operands when LLVM assembler
> > is detected to work around this issue.
> >
> > Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: Dave Hansen <dave.hansen@linux.intel.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Fixes: cd3b85b27542 ("x86/idle: Use MONITOR and MWAIT mnemonics in <asm/mwait.h>")
>
> No, you should whack that one - the toolchains are clearly not ready yet...

The least common denominator is an insn with implicit operands. I'll
post V2 that fixes it that way.

Uros.

Re: [PATCH -tip] x86/idle: Work around LLVM assembler bug with MONITOR and MWAIT insn

[Borislav Petkov]

On Thu, Apr 03, 2025 at 11:06:02AM +0200, Uros Bizjak wrote:
> The least common denominator is an insn with implicit operands. I'll
> post V2 that fixes it that way.

I guess.

You don't need to post a patch ontop - your previous one should be removed and
the whole effort can start all over again.

Maybe Ingo would let the test bots chew the patches a bit first before
applying them so quickly so that we can avoid this churn too.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH -tip] x86/idle: Work around LLVM assembler bug with MONITOR and MWAIT insn

[Uros Bizjak]

On Thu, Apr 3, 2025 at 11:22 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Apr 03, 2025 at 11:06:02AM +0200, Uros Bizjak wrote:
> > The least common denominator is an insn with implicit operands. I'll
> > post V2 that fixes it that way.
>
> I guess.
>
> You don't need to post a patch ontop - your previous one should be removed and
> the whole effort can start all over again.

IMO, in this case the fixup also documents the LLVM bug, so perhaps
the fixup on top is desirable for documentation purposes.

BTW: The test with CC=clang make argument didn't expose this issue.

Uros.

Re: [PATCH -tip] x86/idle: Work around LLVM assembler bug with MONITOR and MWAIT insn

[Borislav Petkov]

On Thu, Apr 03, 2025 at 11:27:02AM +0200, Uros Bizjak wrote:
> IMO, in this case the fixup also documents the LLVM bug, so perhaps
> the fixup on top is desirable for documentation purposes.

No.

1. You put a comment over the function so that someone sees it and *actually*
   fixes it in the compilers

2. Explain in the commit message of the correct patches *why* it cannot be
   done like you were trying initially and leave a lore link into them so that
   people can find the old discussion.

Lemme zap those patches so that they can be done properly.

Thx.

-- 
Regards/Gruss,
    Boris.

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