diff mbox series

[3/7] intel_idle: Add a sanity check in the new state_update_enter_method function

Message ID 20230601182801.2622044-4-arjan@linux.intel.com (mailing list archive)
State Changes Requested, archived
Headers show
Series [1/7] intel_idle: refactor state->enter manipulation into its own function | expand

Commit Message

Arjan van de Ven June 1, 2023, 6:27 p.m. UTC 
From: Arjan van de Ven <arjan.van.de.ven@intel.com>

The state_update_enter_method function updates a state's enter function pointer,
but does so assuming that the current function is "intel_idle" or "intel_idle_irq".

In the code currently that's basically the case, but soon this will change.
Add a sanity check early in the function to make the assumption explicit,
and return early if the precondition is not met.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 drivers/idle/intel_idle.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Rafael J. Wysocki June 4, 2023, 3:43 p.m. UTC | #1 
On Thu, Jun 1, 2023 at 8:28 PM <arjan@linux.intel.com> wrote:
>
> From: Arjan van de Ven <arjan.van.de.ven@intel.com>
>
> The state_update_enter_method function updates a state's enter function pointer,
> but does so assuming that the current function is "intel_idle" or "intel_idle_irq".
>
> In the code currently that's basically the case, but soon this will change.
> Add a sanity check early in the function to make the assumption explicit,
> and return early if the precondition is not met.
>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> ---
>  drivers/idle/intel_idle.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 256c2d42e350..8415965372c7 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -1841,6 +1841,14 @@ static bool __init intel_idle_verify_cstate(unsigned int mwait_hint)
>
>  static void state_update_enter_method(struct cpuidle_state *state, int cstate)
>  {
> +       /*
> +        * The updates below are only valid if state->enter is actually the
> +        * 'intel_idle' or 'intel_idle_irq' functions; for all other cases
> +        * we just bow out early.
> +        */
> +       if (state->enter != intel_idle && state->enter != intel_idle_irq )
> +               return;

Instead of doing this, I would add a check against intel_idle_hlt_irq
in patch [6/7] and extend it to cover intel_idle_hlt in patch [6/7],
because these are the cases to skip here.

> +
>         if (state->flags & CPUIDLE_FLAG_INIT_XSTATE) {
>                 /*
>                  * Combining with XSTATE with IBRS or IRQ_ENABLE flags
> --
> 2.40.1
>
diff mbox series

Patch

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 256c2d42e350..8415965372c7 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1841,6 +1841,14 @@  static bool __init intel_idle_verify_cstate(unsigned int mwait_hint)
 
 static void state_update_enter_method(struct cpuidle_state *state, int cstate)
 {
+	/*
+	 * The updates below are only valid if state->enter is actually the
+	 * 'intel_idle' or 'intel_idle_irq' functions; for all other cases
+	 * we just bow out early.
+	 */
+	if (state->enter != intel_idle && state->enter != intel_idle_irq )
+		return;
+
 	if (state->flags & CPUIDLE_FLAG_INIT_XSTATE) {
 		/*
 		 * Combining with XSTATE with IBRS or IRQ_ENABLE flags