They're not used by any other translation unit, so shouldn't live in
asm/processor.h, which is included almost everywhere.

Our new toolchain baseline knows the MONITOR/MWAIT instructions, so use them
directly rather than using raw hex.

Change the hint/extention parameters from long to int.  They're specified to
remain 32bit operands even 64-bit mode.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/acpi/cpu_idle.c         | 21 +++++++++++++++++----
 xen/arch/x86/include/asm/processor.h | 17 -----------------
 2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index 1dbf15b01ed7..40af42a18fb8 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -60,6 +60,19 @@
 
 /*#define DEBUG_PM_CX*/
 
+static always_inline void monitor(
+    const void *addr, unsigned int ecx, unsigned int edx)
+{
+    asm volatile ( "monitor"
+                   :: "a" (addr), "c" (ecx), "d" (edx) );
+}
+
+static always_inline void mwait(unsigned int eax, unsigned int ecx)
+{
+    asm volatile ( "mwait"
+                   :: "a" (eax), "c" (ecx) );
+}
+
 #define GET_HW_RES_IN_NS(msr, val) \
     do { rdmsrl(msr, val); val = tsc_ticks2ns(val); } while( 0 )
 #define GET_MC6_RES(val)  GET_HW_RES_IN_NS(0x664, val)
@@ -470,7 +483,7 @@ void mwait_idle_with_hints(unsigned int eax, unsigned int ecx)
         mb();
     }
 
-    __monitor(monitor_addr, 0, 0);
+    monitor(monitor_addr, 0, 0);
     smp_mb();
 
     /*
@@ -484,7 +497,7 @@ void mwait_idle_with_hints(unsigned int eax, unsigned int ecx)
         cpumask_set_cpu(cpu, &cpuidle_mwait_flags);
 
         spec_ctrl_enter_idle(info);
-        __mwait(eax, ecx);
+        mwait(eax, ecx);
         spec_ctrl_exit_idle(info);
 
         cpumask_clear_cpu(cpu, &cpuidle_mwait_flags);
@@ -915,9 +928,9 @@ void cf_check acpi_dead_idle(void)
              */
             mb();
             clflush(mwait_ptr);
-            __monitor(mwait_ptr, 0, 0);
+            monitor(mwait_ptr, 0, 0);
             mb();
-            __mwait(cx->address, 0);
+            mwait(cx->address, 0);
         }
     }
     else if ( (current_cpu_data.x86_vendor &
diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h
index 96b9bf5f5edb..04824c3633cf 100644
--- a/xen/arch/x86/include/asm/processor.h
+++ b/xen/arch/x86/include/asm/processor.h
@@ -315,23 +315,6 @@ static always_inline void set_in_cr4 (unsigned long mask)
         cr4_pv32_mask |= (mask & XEN_CR4_PV32_BITS);
 }
 
-static always_inline void __monitor(const void *eax, unsigned long ecx,
-                                    unsigned long edx)
-{
-    /* "monitor %eax,%ecx,%edx;" */
-    asm volatile (
-        ".byte 0x0f,0x01,0xc8;"
-        : : "a" (eax), "c" (ecx), "d"(edx) );
-}
-
-static always_inline void __mwait(unsigned long eax, unsigned long ecx)
-{
-    /* "mwait %eax,%ecx;" */
-    asm volatile (
-        ".byte 0x0f,0x01,0xc9;"
-        : : "a" (eax), "c" (ecx) );
-}
-
 #define IOBMP_BYTES             8192
 #define IOBMP_INVALID_OFFSET    0x8000
 
-- 
2.39.5

On 24.06.2025 18:39, Andrew Cooper wrote:
> They're not used by any other translation unit, so shouldn't live in
> asm/processor.h, which is included almost everywhere.
> 
> Our new toolchain baseline knows the MONITOR/MWAIT instructions, so use them
> directly rather than using raw hex.
> 
> Change the hint/extention parameters from long to int.  They're specified to
> remain 32bit operands even 64-bit mode.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

We may need to undo this though if we ever meant to use mwait elsewhere, like
for spinlocks. Many years ago that idea was entertained some, but it may well
be that it was given up altogether by now.

Jan

On 25/06/2025 10:19 am, Jan Beulich wrote:
> On 24.06.2025 18:39, Andrew Cooper wrote:
>> They're not used by any other translation unit, so shouldn't live in
>> asm/processor.h, which is included almost everywhere.
>>
>> Our new toolchain baseline knows the MONITOR/MWAIT instructions, so use them
>> directly rather than using raw hex.
>>
>> Change the hint/extention parameters from long to int.  They're specified to
>> remain 32bit operands even 64-bit mode.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

> We may need to undo this though if we ever meant to use mwait elsewhere, like
> for spinlocks. Many years ago that idea was entertained some, but it may well
> be that it was given up altogether by now.

If they need to move back into a header, that's fine, but it will be
something new.

That said, I'm not sure how useful it would be to mwait on a spinlock. 
You'd need the spinlock in its own cacheline or you'll get false wakeups
triggered by CPU which has the has the lock writing adjacent to the
lock.  Transitions in and out of idle like this are almost certainly
more expensive than a pause loop.

~Andrew