| Message ID | 20241108122909.763663-4-patryk.wlazlyn@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | SRF: Fix offline CPU preventing pc6 entry | expand |
On 11/8/24 04:29, Patryk Wlazlyn wrote: > Intel's Sierra Forest report two C6 substates in cpuid leaf 5: > C6S (hint 0x22) > C6SP (hint 0x23) > > Hints 0x20 and 0x21 are skipped entirely, causing the generic > implementation in mwait_play_dead() to compute the wrong hint, when > looking for the deepest cstate. As a result, package with an offlined > CPU can never reach PC6. This series has said multiple times how the old algorithm is wrong. But it never actually _fixed_ the bad algorithm, only worked around it. Does mwait_play_dead() itself need to get fixed? > Define the enter_dead() handler for SRF. This effectively gets the mwait hints from ______ instead of using the calculation in mwait_play_dead(). > diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c > index 9aab7abc2ae9..bd67959e5e8b 100644 > --- a/drivers/idle/intel_idle.c > +++ b/drivers/idle/intel_idle.c > @@ -56,6 +56,7 @@ > #include <asm/mwait.h> > #include <asm/spec-ctrl.h> > #include <asm/fpu/api.h> > +#include <asm/smp.h> > > #define INTEL_IDLE_VERSION "0.5.1" > > @@ -221,6 +222,17 @@ static __cpuidle int intel_idle_s2idle(struct cpuidle_device *dev, > return 0; > } > > +static __cpuidle int intel_idle_enter_dead(struct cpuidle_device *dev, > + int index) > +{ > + struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev); > + struct cpuidle_state *state = &drv->states[index]; > + unsigned long eax = flg2MWAIT(state->flags); > + > + /* Retruns only in case of an error. */ ^ returns? > + return mwait_play_dead_with_hint(eax); > +} > + > /* > * States are indexed by the cstate number, > * which is also the index into the MWAIT hint array. > @@ -1303,6 +1315,7 @@ static struct cpuidle_state srf_cstates[] __initdata = { > .exit_latency = 1, > .target_residency = 1, > .enter = &intel_idle, > + .enter_dead = &intel_idle_enter_dead, > .enter_s2idle = intel_idle_s2idle, }, > { > .name = "C1E", > @@ -1311,6 +1324,7 @@ static struct cpuidle_state srf_cstates[] __initdata = { > .exit_latency = 2, > .target_residency = 10, > .enter = &intel_idle, > + .enter_dead = &intel_idle_enter_dead, > .enter_s2idle = intel_idle_s2idle, }, > { > .name = "C6S", > @@ -1319,6 +1333,7 @@ static struct cpuidle_state srf_cstates[] __initdata = { > .exit_latency = 270, > .target_residency = 700, > .enter = &intel_idle, > + .enter_dead = &intel_idle_enter_dead, > .enter_s2idle = intel_idle_s2idle, }, > { > .name = "C6SP", > @@ -1327,6 +1342,7 @@ static struct cpuidle_state srf_cstates[] __initdata = { > .exit_latency = 310, > .target_residency = 900, > .enter = &intel_idle, > + .enter_dead = &intel_idle_enter_dead, > .enter_s2idle = intel_idle_s2idle, }, > { > .enter = NULL }
Hi Patryk,
kernel test robot noticed the following build warnings:
[auto build test WARNING on acpi/next]
[also build test WARNING on tip/master tip/x86/core linus/master v6.12-rc6 next-20241108]
[cannot apply to tip/auto-latest]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Patryk-Wlazlyn/x86-smp-Allow-calling-mwait_play_dead-with-arbitrary-hint/20241108-203137
base: https://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git next
patch link: https://lore.kernel.org/r/20241108122909.763663-4-patryk.wlazlyn%40linux.intel.com
patch subject: [PATCH v3 3/3] intel_idle: Provide enter_dead() handler for SRF
config: x86_64-randconfig-161-20241109 (https://download.01.org/0day-ci/archive/20241109/202411090651.FNnbtLe2-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241109/202411090651.FNnbtLe2-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411090651.FNnbtLe2-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> vmlinux.o: warning: objtool: intel_idle_enter_dead+0x9: call to cpuidle_get_cpu_driver() leaves .noinstr.text section
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 9aab7abc2ae9..bd67959e5e8b 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -56,6 +56,7 @@ #include <asm/mwait.h> #include <asm/spec-ctrl.h> #include <asm/fpu/api.h> +#include <asm/smp.h> #define INTEL_IDLE_VERSION "0.5.1" @@ -221,6 +222,17 @@ static __cpuidle int intel_idle_s2idle(struct cpuidle_device *dev, return 0; } +static __cpuidle int intel_idle_enter_dead(struct cpuidle_device *dev, + int index) +{ + struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev); + struct cpuidle_state *state = &drv->states[index]; + unsigned long eax = flg2MWAIT(state->flags); + + /* Retruns only in case of an error. */ + return mwait_play_dead_with_hint(eax); +} + /* * States are indexed by the cstate number, * which is also the index into the MWAIT hint array. @@ -1303,6 +1315,7 @@ static struct cpuidle_state srf_cstates[] __initdata = { .exit_latency = 1, .target_residency = 1, .enter = &intel_idle, + .enter_dead = &intel_idle_enter_dead, .enter_s2idle = intel_idle_s2idle, }, { .name = "C1E", @@ -1311,6 +1324,7 @@ static struct cpuidle_state srf_cstates[] __initdata = { .exit_latency = 2, .target_residency = 10, .enter = &intel_idle, + .enter_dead = &intel_idle_enter_dead, .enter_s2idle = intel_idle_s2idle, }, { .name = "C6S", @@ -1319,6 +1333,7 @@ static struct cpuidle_state srf_cstates[] __initdata = { .exit_latency = 270, .target_residency = 700, .enter = &intel_idle, + .enter_dead = &intel_idle_enter_dead, .enter_s2idle = intel_idle_s2idle, }, { .name = "C6SP", @@ -1327,6 +1342,7 @@ static struct cpuidle_state srf_cstates[] __initdata = { .exit_latency = 310, .target_residency = 900, .enter = &intel_idle, + .enter_dead = &intel_idle_enter_dead, .enter_s2idle = intel_idle_s2idle, }, { .enter = NULL }
Intel's Sierra Forest report two C6 substates in cpuid leaf 5: C6S (hint 0x22) C6SP (hint 0x23) Hints 0x20 and 0x21 are skipped entirely, causing the generic implementation in mwait_play_dead() to compute the wrong hint, when looking for the deepest cstate. As a result, package with an offlined CPU can never reach PC6. Define the enter_dead() handler for SRF. Signed-off-by: Patryk Wlazlyn <patryk.wlazlyn@linux.intel.com> --- drivers/idle/intel_idle.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)