diff mbox series

[1/4] intel_idle: refactor state->enter manipulation into its own function

Message ID 20230605154716.840930-2-arjan@linux.intel.com
State Accepted
Commit 4622ba923e55e12cb76081c8865b01fcb383a9d8
Headers show
Series Add support for running in VM guests to intel_idle | expand

Commit Message

Arjan van de Ven June 5, 2023, 3:47 p.m. UTC 
From: Arjan van de Ven <arjan@linux.intel.com>

Since the 6.4 kernel, the logic for updating a state's enter method
based on "environmental conditions" (command line options, cpu sidechannel
workarounds etc etc) has gotten pretty complex.
This patch refactors this into a seperate small, self contained function
(no behavior changes) for improved readability and to make future
changes to this logic easier to do and understand.

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

Comments

Zhang Rui June 7, 2023, 7:34 a.m. UTC | #1 
Just one minor comment.

On Mon, 2023-06-05 at 15:47 +0000, arjan@linux.intel.com wrote:
> 
>  
> +static void state_update_enter_method(struct cpuidle_state *state,
> int cstate)
> +{
> +       if (state->flags & CPUIDLE_FLAG_INIT_XSTATE) {
> +               /*
> +                * Combining with XSTATE with IBRS or IRQ_ENABLE
> flags
> +                * is not currently supported but this driver.

s/but/by?
seems like a typo in the original commit.

thanks,
rui
Arjan van de Ven June 7, 2023, 2:08 p.m. UTC | #2 
On 6/7/2023 12:34 AM, Zhang, Rui wrote:
> Just one minor comment.
> 
> On Mon, 2023-06-05 at 15:47 +0000, arjan@linux.intel.com wrote:
>>
>>   
>> +static void state_update_enter_method(struct cpuidle_state *state,
>> int cstate)
>> +{
>> +       if (state->flags & CPUIDLE_FLAG_INIT_XSTATE) {
>> +               /*
>> +                * Combining with XSTATE with IBRS or IRQ_ENABLE
>> flags
>> +                * is not currently supported but this driver.
> 
> s/but/by?
> seems like a typo in the original commit.

this commit is a strict move of the existing code into a function, with zero changes
I would prefer to keep it that way obviously.
If someone wants to fix up spelling in comments that should be some completely independent commit
diff mbox series

Patch

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index aa2d19db2b1d..c351b21c0875 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1839,6 +1839,32 @@  static bool __init intel_idle_verify_cstate(unsigned int mwait_hint)
 	return true;
 }
 
+static void state_update_enter_method(struct cpuidle_state *state, int cstate)
+{
+	if (state->flags & CPUIDLE_FLAG_INIT_XSTATE) {
+		/*
+		 * Combining with XSTATE with IBRS or IRQ_ENABLE flags
+		 * is not currently supported but this driver.
+		 */
+		WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IBRS);
+		WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IRQ_ENABLE);
+		state->enter = intel_idle_xstate;
+	} else if (cpu_feature_enabled(X86_FEATURE_KERNEL_IBRS) &&
+			   state->flags & CPUIDLE_FLAG_IBRS) {
+		/*
+		 * IBRS mitigation requires that C-states are entered
+		 * with interrupts disabled.
+		 */
+		WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IRQ_ENABLE);
+		state->enter = intel_idle_ibrs;
+	} else if (state->flags & CPUIDLE_FLAG_IRQ_ENABLE) {
+		state->enter = intel_idle_irq;
+	} else if (force_irq_on) {
+		pr_info("forced intel_idle_irq for state %d\n", cstate);
+		state->enter = intel_idle_irq;
+	}
+}
+
 static void __init intel_idle_init_cstates_icpu(struct cpuidle_driver *drv)
 {
 	int cstate;
@@ -1894,28 +1920,8 @@  static void __init intel_idle_init_cstates_icpu(struct cpuidle_driver *drv)
 		drv->states[drv->state_count] = cpuidle_state_table[cstate];
 		state = &drv->states[drv->state_count];
 
-		if (state->flags & CPUIDLE_FLAG_INIT_XSTATE) {
-			/*
-			 * Combining with XSTATE with IBRS or IRQ_ENABLE flags
-			 * is not currently supported but this driver.
-			 */
-			WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IBRS);
-			WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IRQ_ENABLE);
-			state->enter = intel_idle_xstate;
-		} else if (cpu_feature_enabled(X86_FEATURE_KERNEL_IBRS) &&
-			   state->flags & CPUIDLE_FLAG_IBRS) {
-			/*
-			 * IBRS mitigation requires that C-states are entered
-			 * with interrupts disabled.
-			 */
-			WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IRQ_ENABLE);
-			state->enter = intel_idle_ibrs;
-		} else if (state->flags & CPUIDLE_FLAG_IRQ_ENABLE) {
-			state->enter = intel_idle_irq;
-		} else if (force_irq_on) {
-			pr_info("forced intel_idle_irq for state %d\n", cstate);
-			state->enter = intel_idle_irq;
-		}
+		state_update_enter_method(state, cstate);
+
 
 		if ((disabled_states_mask & BIT(drv->state_count)) ||
 		    ((icpu->use_acpi || force_use_acpi) &&