| Message ID | 20241129182232.14987-3-patryk.wlazlyn@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SRF: Fix offline CPU preventing pc6 entry | expand |
On Fri, Nov 29, 2024 at 07:22:30PM +0100, Patryk Wlazlyn wrote: > Recent Intel platforms will depend on the idle driver to pass the > correct hint for playing dead via mwait_play_dead_with_hint(). Expand > the existing enter_dead interface with handling for FFH states and pass > the MWAIT hint to the mwait_play_dead code. > > Signed-off-by: Patryk Wlazlyn <patryk.wlazlyn@linux.intel.com> > Suggested-by: Gautham R. Shenoy <gautham.shenoy@amd.com> Looks good to me. Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> -- Thanks and Regards gautham. > --- > arch/x86/kernel/acpi/cstate.c | 10 ++++++++++ > drivers/acpi/processor_idle.c | 2 ++ > include/acpi/processor.h | 5 +++++ > 3 files changed, 17 insertions(+) > > diff --git a/arch/x86/kernel/acpi/cstate.c b/arch/x86/kernel/acpi/cstate.c > index f3ffd0a3a012..8d7b8b02ddb9 100644 > --- a/arch/x86/kernel/acpi/cstate.c > +++ b/arch/x86/kernel/acpi/cstate.c > @@ -15,6 +15,7 @@ > #include <acpi/processor.h> > #include <asm/mwait.h> > #include <asm/special_insns.h> > +#include <asm/smp.h> > > /* > * Initialize bm_flags based on the CPU cache properties > @@ -204,6 +205,15 @@ int acpi_processor_ffh_cstate_probe(unsigned int cpu, > } > EXPORT_SYMBOL_GPL(acpi_processor_ffh_cstate_probe); > > +void acpi_processor_ffh_play_dead(struct acpi_processor_cx *cx) > +{ > + unsigned int cpu = smp_processor_id(); > + struct cstate_entry *percpu_entry; > + > + percpu_entry = per_cpu_ptr(cpu_cstate_entry, cpu); > + mwait_play_dead(percpu_entry->states[cx->index].eax); > +} > + > void __cpuidle acpi_processor_ffh_cstate_enter(struct acpi_processor_cx *cx) > { > unsigned int cpu = smp_processor_id(); > diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c > index ce728cf7e301..83213fa47c1b 100644 > --- a/drivers/acpi/processor_idle.c > +++ b/drivers/acpi/processor_idle.c > @@ -590,6 +590,8 @@ static void acpi_idle_play_dead(struct cpuidle_device *dev, int index) > raw_safe_halt(); > else if (cx->entry_method == ACPI_CSTATE_SYSTEMIO) { > io_idle(cx->address); > + } else if (cx->entry_method == ACPI_CSTATE_FFH) { > + acpi_processor_ffh_play_dead(cx); > } else > return; > } > diff --git a/include/acpi/processor.h b/include/acpi/processor.h > index a17e97e634a6..63a37e72b721 100644 > --- a/include/acpi/processor.h > +++ b/include/acpi/processor.h > @@ -280,6 +280,7 @@ int acpi_processor_ffh_cstate_probe(unsigned int cpu, > struct acpi_processor_cx *cx, > struct acpi_power_register *reg); > void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx *cstate); > +void acpi_processor_ffh_play_dead(struct acpi_processor_cx *cx); > #else > static inline void acpi_processor_power_init_bm_check(struct > acpi_processor_flags > @@ -300,6 +301,10 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx > { > return; > } > +static inline void acpi_processor_ffh_play_dead(struct acpi_processor_cx *cx) > +{ > + return; > +} > #endif > > static inline int call_on_cpu(int cpu, long (*fn)(void *), void *arg, > -- > 2.47.1 >
On Fri, Nov 29, 2024 at 7:22 PM Patryk Wlazlyn <patryk.wlazlyn@linux.intel.com> wrote: > > Recent Intel platforms will depend on the idle driver to pass the > correct hint for playing dead via mwait_play_dead_with_hint(). Expand > the existing enter_dead interface with handling for FFH states and pass > the MWAIT hint to the mwait_play_dead code. First, the changelog doesn't match the code changes because you've decided to use mwait_play_dead() without changing its name. Second, "recent" is something that has happened already, so I would say "new" or "upcoming" instead. Overall, my changelog for this patch would be something like this: "The ACPI processor_idle driver's implementation of the :enter_dead() callback, acpi_idle_play_dead(), is missing the handling of FFH (Fixed Functional Hardware) idle states that on x86 platforms are entered by executing the MWAIT instruction. If such idle states are listed by _CST for the given CPU, the idle driver should be able to use the MWAIT hints corresponding to them in order to put the CPU into an appropriate idle state when it is going offline, so update acpi_idle_play_dead() to take the FFH idle states into account. Going forward this will be depended on by Intel platforms if the (otherwise default) intel_idle driver is disabled." The code changes look good to me. > Signed-off-by: Patryk Wlazlyn <patryk.wlazlyn@linux.intel.com> > Suggested-by: Gautham R. Shenoy <gautham.shenoy@amd.com> > --- > arch/x86/kernel/acpi/cstate.c | 10 ++++++++++ > drivers/acpi/processor_idle.c | 2 ++ > include/acpi/processor.h | 5 +++++ > 3 files changed, 17 insertions(+) > > diff --git a/arch/x86/kernel/acpi/cstate.c b/arch/x86/kernel/acpi/cstate.c > index f3ffd0a3a012..8d7b8b02ddb9 100644 > --- a/arch/x86/kernel/acpi/cstate.c > +++ b/arch/x86/kernel/acpi/cstate.c > @@ -15,6 +15,7 @@ > #include <acpi/processor.h> > #include <asm/mwait.h> > #include <asm/special_insns.h> > +#include <asm/smp.h> > > /* > * Initialize bm_flags based on the CPU cache properties > @@ -204,6 +205,15 @@ int acpi_processor_ffh_cstate_probe(unsigned int cpu, > } > EXPORT_SYMBOL_GPL(acpi_processor_ffh_cstate_probe); > > +void acpi_processor_ffh_play_dead(struct acpi_processor_cx *cx) > +{ > + unsigned int cpu = smp_processor_id(); > + struct cstate_entry *percpu_entry; > + > + percpu_entry = per_cpu_ptr(cpu_cstate_entry, cpu); > + mwait_play_dead(percpu_entry->states[cx->index].eax); > +} > + > void __cpuidle acpi_processor_ffh_cstate_enter(struct acpi_processor_cx *cx) > { > unsigned int cpu = smp_processor_id(); > diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c > index ce728cf7e301..83213fa47c1b 100644 > --- a/drivers/acpi/processor_idle.c > +++ b/drivers/acpi/processor_idle.c > @@ -590,6 +590,8 @@ static void acpi_idle_play_dead(struct cpuidle_device *dev, int index) > raw_safe_halt(); > else if (cx->entry_method == ACPI_CSTATE_SYSTEMIO) { > io_idle(cx->address); > + } else if (cx->entry_method == ACPI_CSTATE_FFH) { > + acpi_processor_ffh_play_dead(cx); > } else > return; > } > diff --git a/include/acpi/processor.h b/include/acpi/processor.h > index a17e97e634a6..63a37e72b721 100644 > --- a/include/acpi/processor.h > +++ b/include/acpi/processor.h > @@ -280,6 +280,7 @@ int acpi_processor_ffh_cstate_probe(unsigned int cpu, > struct acpi_processor_cx *cx, > struct acpi_power_register *reg); > void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx *cstate); > +void acpi_processor_ffh_play_dead(struct acpi_processor_cx *cx); > #else > static inline void acpi_processor_power_init_bm_check(struct > acpi_processor_flags > @@ -300,6 +301,10 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx > { > return; > } > +static inline void acpi_processor_ffh_play_dead(struct acpi_processor_cx *cx) > +{ > + return; > +} > #endif > > static inline int call_on_cpu(int cpu, long (*fn)(void *), void *arg, > --
diff --git a/arch/x86/kernel/acpi/cstate.c b/arch/x86/kernel/acpi/cstate.c index f3ffd0a3a012..8d7b8b02ddb9 100644 --- a/arch/x86/kernel/acpi/cstate.c +++ b/arch/x86/kernel/acpi/cstate.c @@ -15,6 +15,7 @@ #include <acpi/processor.h> #include <asm/mwait.h> #include <asm/special_insns.h> +#include <asm/smp.h> /* * Initialize bm_flags based on the CPU cache properties @@ -204,6 +205,15 @@ int acpi_processor_ffh_cstate_probe(unsigned int cpu, } EXPORT_SYMBOL_GPL(acpi_processor_ffh_cstate_probe); +void acpi_processor_ffh_play_dead(struct acpi_processor_cx *cx) +{ + unsigned int cpu = smp_processor_id(); + struct cstate_entry *percpu_entry; + + percpu_entry = per_cpu_ptr(cpu_cstate_entry, cpu); + mwait_play_dead(percpu_entry->states[cx->index].eax); +} + void __cpuidle acpi_processor_ffh_cstate_enter(struct acpi_processor_cx *cx) { unsigned int cpu = smp_processor_id(); diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index ce728cf7e301..83213fa47c1b 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -590,6 +590,8 @@ static void acpi_idle_play_dead(struct cpuidle_device *dev, int index) raw_safe_halt(); else if (cx->entry_method == ACPI_CSTATE_SYSTEMIO) { io_idle(cx->address); + } else if (cx->entry_method == ACPI_CSTATE_FFH) { + acpi_processor_ffh_play_dead(cx); } else return; } diff --git a/include/acpi/processor.h b/include/acpi/processor.h index a17e97e634a6..63a37e72b721 100644 --- a/include/acpi/processor.h +++ b/include/acpi/processor.h @@ -280,6 +280,7 @@ int acpi_processor_ffh_cstate_probe(unsigned int cpu, struct acpi_processor_cx *cx, struct acpi_power_register *reg); void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx *cstate); +void acpi_processor_ffh_play_dead(struct acpi_processor_cx *cx); #else static inline void acpi_processor_power_init_bm_check(struct acpi_processor_flags @@ -300,6 +301,10 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx { return; } +static inline void acpi_processor_ffh_play_dead(struct acpi_processor_cx *cx) +{ + return; +} #endif static inline int call_on_cpu(int cpu, long (*fn)(void *), void *arg,
Recent Intel platforms will depend on the idle driver to pass the correct hint for playing dead via mwait_play_dead_with_hint(). Expand the existing enter_dead interface with handling for FFH states and pass the MWAIT hint to the mwait_play_dead code. Signed-off-by: Patryk Wlazlyn <patryk.wlazlyn@linux.intel.com> Suggested-by: Gautham R. Shenoy <gautham.shenoy@amd.com> --- arch/x86/kernel/acpi/cstate.c | 10 ++++++++++ drivers/acpi/processor_idle.c | 2 ++ include/acpi/processor.h | 5 +++++ 3 files changed, 17 insertions(+)