Re: [PATCH v3 1/2] x86/mwait: Add support for idle via umwait

From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Artem Bityutskiy <dedekind1@gmail.com>, x86@kernel.org
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Linux PM Mailing List <linux-pm@vger.kernel.org>,
	Arjan van de Ven <arjan@linux.intel.com>
Subject: Re: [PATCH v3 1/2] x86/mwait: Add support for idle via umwait
Date: Thu, 29 Jun 2023 21:03:14 +0200	[thread overview]
Message-ID: <CAJZ5v0jiuenEnLjeZg+rksLT5D1f0-0Xad98rumR+vJReCXx7Q@mail.gmail.com> (raw)
In-Reply-To: <20230610183518.4061159-2-dedekind1@gmail.com>

On Sat, Jun 10, 2023 at 8:35 PM Artem Bityutskiy <dedekind1@gmail.com> wrote:
>
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>
> On Intel platforms, C-states are requested using the 'monitor/mwait'
> instructions pair, as implemented in 'mwait_idle_with_hints()'. This
> mechanism allows for entering C1 and deeper C-states.
>
> Sapphire Rapids Xeon supports new idle states - C0.1 and C0.2 (later C0.x).
> These idle states have lower latency comparing to C1, and can be requested
> with either 'tpause' and 'umwait' instructions.
>
> Linux already uses the 'tpause' instruction in delay functions like
> 'udelay()'. This patch adds 'umwait' and 'umonitor' instructions support.
>
> 'umwait' and 'tpause' instructions are very similar - both send the CPU to
> C0.x and have the same break out rules. But unlike 'tpause', 'umwait' works
> together with 'umonitor' and exits the C0.x when the monitored memory
> address is modified (similar idea as with 'monitor/mwait').
>
> This patch implements the 'umwait_idle()' function, which works very
> similarly to existing 'mwait_idle_with_hints()', but requests C0.x. The
> intention is to use it from the 'intel_idle' driver.
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

x86 folks, any comments on this?

Barring any concerns, I would like to queue it up for 6.6 when the
merge is over.

> ---
>  arch/x86/include/asm/mwait.h | 65 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>
> diff --git a/arch/x86/include/asm/mwait.h b/arch/x86/include/asm/mwait.h
> index 778df05f8539..681c281eeaa7 100644
> --- a/arch/x86/include/asm/mwait.h
> +++ b/arch/x86/include/asm/mwait.h
> @@ -141,4 +141,69 @@ static inline void __tpause(u32 ecx, u32 edx, u32 eax)
>         #endif
>  }
>
> +#ifdef CONFIG_X86_64
> +/*
> + * Monitor a memory address at 'rcx' using the 'umonitor' instruction.
> + */
> +static inline void __umonitor(const void *rcx)
> +{
> +       /* "umonitor %rcx" */
> +#ifdef CONFIG_AS_TPAUSE
> +       asm volatile("umonitor %%rcx\n"
> +                    :
> +                    : "c"(rcx));
> +#else
> +       asm volatile(".byte 0xf3, 0x0f, 0xae, 0xf1\t\n"
> +                    :
> +                    : "c"(rcx));
> +#endif
> +}
> +
> +/*
> + * Same as '__tpause()', but uses the 'umwait' instruction. It is very
> + * similar to 'tpause', but also breaks out if the data at the address
> + * monitored with 'umonitor' is modified.
> + */
> +static inline void __umwait(u32 ecx, u32 edx, u32 eax)
> +{
> +       /* "umwait %ecx, %edx, %eax;" */
> +#ifdef CONFIG_AS_TPAUSE
> +       asm volatile("umwait %%ecx\n"
> +                    :
> +                    : "c"(ecx), "d"(edx), "a"(eax));
> +#else
> +       asm volatile(".byte 0xf2, 0x0f, 0xae, 0xf1\t\n"
> +                    :
> +                    : "c"(ecx), "d"(edx), "a"(eax));
> +#endif
> +}
> +
> +/*
> + * Enter C0.1 or C0.2 state and stay there until an event happens (an interrupt
> + * or the 'need_resched()'), the explicit deadline is reached, or the implicit
> + * global limit is reached.
> + *
> + * The deadline is the absolute TSC value to exit the idle state at. If it
> + * exceeds the global limit in the 'IA32_UMWAIT_CONTROL' register, the global
> + * limit prevails, and the idle state is exited earlier than the deadline.
> + */
> +static inline void umwait_idle(u64 deadline, u32 state)
> +{
> +       if (!current_set_polling_and_test()) {
> +               u32 eax, edx;
> +
> +               eax = lower_32_bits(deadline);
> +               edx = upper_32_bits(deadline);
> +
> +               __umonitor(&current_thread_info()->flags);
> +               if (!need_resched())
> +                       __umwait(state, edx, eax);
> +       }
> +       current_clr_polling();
> +}
> +#else
> +#define umwait_idle(deadline, state) \
> +               WARN_ONCE(1, "umwait CPU instruction is not supported")
> +#endif /* CONFIG_X86_64 */
> +
>  #endif /* _ASM_X86_MWAIT_H */
> --
> 2.40.1
>