intel_idle: refactor and split out model specific fixups into its own function
PATCH 1/4 intel_idle: refactor and split out model specific fixups into its own function
From: Arjan van de Ven <[email protected]>
As this list has been growing over time, lets split it into its own function. That also makes it possible to move other hw workarounds into this function over time.
--- drivers/idle/intel_idle.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/idle/intel_idle.c +8 -3
@@ -1955,10 +1955,8 @@ static bool should_verify_mwait(struct cpuidle_state *state)return true; } -static void __init intel_idle_init_cstates_icpu (struct cpuidle_driver *drv) +static void __init model_specific_fixups (void ) { -int cstate; -switch (boot_cpu_data.x86_model) {case INTEL_FAM6_IVYBRIDGE_X :ivt_idle_state_table_update ();
@@ -1983,6 +1981,13 @@ static void __init intel_idle_init_cstates_icpu(struct cpuidle_driver *drv)adl_idle_state_table_update ();break ; } +} + +static void __init intel_idle_init_cstates_icpu (struct cpuidle_driver *drv) +{ +int cstate; + +model_specific_fixups ();for (cstate =0 ; cstate <CPUIDLE_STATE_MAX ; ++cstate) {struct cpuidle_state *state;
Signature · 1 line
2.41.0
PATCH 2/4 intel_idle: move one more model specific workaround into the newly created seperate function
From: Arjan van de Ven <[email protected]>
byt_auto_demotion_disable_flag (for baytrail) handling can now be moved into the newly created model_specific_fixups() function.
This frees up intel_idle_init_cstates_icpu() from containing model specific information direclty
--- drivers/idle/intel_idle.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/idle/intel_idle.c +5 -4
@@ -1981,6 +1981,11 @@ static void __init model_specific_fixups(void)adl_idle_state_table_update ();break ; } + +if (icpu->byt_auto_demotion_disable_flag) { +wrmsrl (MSR_CC6_DEMOTION_POLICY_CONFIG ,0 ); +wrmsrl (MSR_MC6_DEMOTION_POLICY_CONFIG ,0 ); + } }static void __init intel_idle_init_cstates_icpu (struct cpuidle_driver *drv)
@@ -2030,10 +2035,6 @@ static void __init intel_idle_init_cstates_icpu(struct cpuidle_driver *drv) drv->state_count++; } -if (icpu->byt_auto_demotion_disable_flag) { -wrmsrl (MSR_CC6_DEMOTION_POLICY_CONFIG ,0 ); -wrmsrl (MSR_MC6_DEMOTION_POLICY_CONFIG ,0 ); - } }/**
Signature · 1 line
2.41.0
PATCH 3/4 intel_idle: add VM guest (non)handling to model_specific_fixups
In reply to [email protected]From: Arjan van de Ven <[email protected]>
The model specific fixups don't make sense in a VM guest; all the MSR accesses at best will be fiction, and at worse cause logspam
--- drivers/idle/intel_idle.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/idle/intel_idle.c +4 -0
@@ -1957,6 +1957,10 @@ static bool should_verify_mwait(struct cpuidle_state *state)static void __init model_specific_fixups (void ) { +/* when running in a hypervisor, none of these fixups will work */ +if (boot_cpu_has (X86_FEATURE_HYPERVISOR )) +return ; +switch (boot_cpu_data.x86_model) {case INTEL_FAM6_IVYBRIDGE_X :ivt_idle_state_table_update ();
Signature · 1 line
2.41.0
PATCH 4/4 intel_idle: remove intel_idle_hlt_irq_on
In reply to [email protected]From: Arjan van de Ven <[email protected]>
PeterZ pointed out that the whole function is redundant as for hlt, it's always "irq on"
--- drivers/idle/intel_idle.c | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-)
drivers/idle/intel_idle.c +1 -21
@@ -224,17 +224,6 @@ static __cpuidle int intel_idle_hlt(struct cpuidle_device *dev,return __intel_idle_hlt (dev, drv, index); } -static __cpuidle int intel_idle_hlt_irq_on(struct cpuidle_device *dev, -struct cpuidle_driver *drv,int index) -{ -int ret; - -raw_local_irq_enable (); - ret =__intel_idle_hlt (dev, drv, index); -raw_local_irq_disable (); - -return ret; -}/** * intel_idle_s2idle - Ask the processor to enter the given idle state.
@@ -1897,15 +1886,8 @@ static bool __init intel_idle_verify_cstate(unsigned int mwait_hint)static void state_update_enter_method (struct cpuidle_state *state,int cstate) { -if (state->enter == intel_idle_hlt) { -if (force_irq_on) { -pr_info ("forced intel_idle_irq for state %d\n" , cstate); - state->enter = intel_idle_hlt_irq_on; - } +if (state->enter == intel_idle_hlt)return ; - } -if (state->enter == intel_idle_hlt_irq_on) -return ;/* no update scenarios */ if (state->flags &CPUIDLE_FLAG_INIT_XSTATE ) {/*
@@ -1949,8 +1931,6 @@ static bool should_verify_mwait(struct cpuidle_state *state) {if (state->enter == intel_idle_hlt)return false; -if (state->enter == intel_idle_hlt_irq_on) -return false;return true; }
Signature · 1 line
2.41.0
Rafael J. Wysocki
maintainerQuoted 7 lines · On Mon, Jul 17, 2023 at 5:19 PM <[email protected]> wrote:
From: Arjan van de Ven <[email protected]>
As this list has been growing over time, lets split it into its own function. That also makes it possible to move other hw workarounds into this function over time.
Signed-off-by Arjan van de Ven <[email protected]>
After a discussion with Peter I have decided to revert the original changes, sorry about that.
IMV Peter has a point regarding the lack of data supporting the claimed performance gain and without the umwait part they are rather hard to defend.
Quoted 37 lines
--- drivers/idle/intel_idle.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)drivers/idle/intel_idle.c +8
-4@@ -1955,10 +1955,8 @@ static bool should_verify_mwait(struct cpuidle_state *state)return true; } -static void __init intel_idle_init_cstates_icpu (struct cpuidle_driver *drv) +static void __init model_specific_fixups (void ) { -int cstate; -switch (boot_cpu_data.x86_model) {case INTEL_FAM6_IVYBRIDGE_X :ivt_idle_state_table_update ();@@ -1983,6 +1981,13 @@ static void __init intel_idle_init_cstates_icpu(struct cpuidle_driver *drv)adl_idle_state_table_update ();break ; } +} + +static void __init intel_idle_init_cstates_icpu (struct cpuidle_driver *drv) +{ +int cstate; + +model_specific_fixups ();for (cstate =0 ; cstate <CPUIDLE_STATE_MAX ; ++cstate) {struct cpuidle_state *state; --2.41.0