[PATCH v1 4/5] ACPI: processor: Rescan "dead" SMT siblings during initialization

Rafael J. Wysocki posted 1 patch 6 months, 2 weeks ago
drivers/acpi/internal.h         |    6 ++++++
drivers/acpi/processor_driver.c |    3 +++
drivers/acpi/processor_idle.c   |    8 ++++++++
3 files changed, 17 insertions(+)
[PATCH v1 4/5] ACPI: processor: Rescan "dead" SMT siblings during initialization
Posted by Rafael J. Wysocki 6 months, 2 weeks ago
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Make acpi_processor_driver_init() call arch_cpu_rescan_dead_smt_siblings(),
via a new wrapper function called acpi_idle_rescan_dead_smt_siblings(),
after successfully initializing the driver, to allow the "dead" SMT
siblings to go into deep idle states, which is necessary for the
processor to be able to reach deep package C-states (like PC10) going
forward, so that power can be reduced sufficiently in suspend-to-idle,
among other things.

However, do it only if the ACPI idle driver is the current cpuidle
driver (otherwise it is assumed that another cpuidle driver will take
care of this) and avoid doing it on architectures other than x86.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/internal.h         |    6 ++++++
 drivers/acpi/processor_driver.c |    3 +++
 drivers/acpi/processor_idle.c   |    8 ++++++++
 3 files changed, 17 insertions(+)

--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -175,6 +175,12 @@
 static inline void acpi_early_processor_control_setup(void) {}
 #endif
 
+#ifdef CONFIG_ACPI_PROCESSOR_CSTATE
+void acpi_idle_rescan_dead_smt_siblings(void);
+#else
+static inline void acpi_idle_rescan_dead_smt_siblings(void) {}
+#endif
+
 /* --------------------------------------------------------------------------
                                   Embedded Controller
    -------------------------------------------------------------------------- */
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -279,6 +279,9 @@
 	 * after acpi_cppc_processor_probe() has been called for all online CPUs
 	 */
 	acpi_processor_init_invariance_cppc();
+
+	acpi_idle_rescan_dead_smt_siblings();
+
 	return 0;
 err:
 	driver_unregister(&acpi_processor_driver);
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -24,6 +24,8 @@
 #include <acpi/processor.h>
 #include <linux/context_tracking.h>
 
+#include "internal.h"
+
 /*
  * Include the apic definitions for x86 to have the APIC timer related defines
  * available also for UP (on SMP it gets magically included via linux/smp.h).
@@ -55,6 +57,12 @@
 };
 
 #ifdef CONFIG_ACPI_PROCESSOR_CSTATE
+void acpi_idle_rescan_dead_smt_siblings(void)
+{
+	if (cpuidle_get_driver() == &acpi_idle_driver)
+		arch_cpu_rescan_dead_smt_siblings();
+}
+
 static
 DEFINE_PER_CPU(struct acpi_processor_cx * [CPUIDLE_STATE_MAX], acpi_cstate);
Re: [PATCH v1 4/5] ACPI: processor: Rescan "dead" SMT siblings during initialization
Posted by Dave Hansen 6 months, 2 weeks ago
On 6/5/25 08:07, Rafael J. Wysocki wrote:
>  #ifdef CONFIG_ACPI_PROCESSOR_CSTATE
> +void acpi_idle_rescan_dead_smt_siblings(void)
> +{
> +	if (cpuidle_get_driver() == &acpi_idle_driver)
> +		arch_cpu_rescan_dead_smt_siblings();
> +}

My only thought in reading this is that maybe cpuidle_register_driver()
would be a better spot to force the arch_cpu_rescan_dead_smt_siblings().
That way, each driver would not have to do the rescan.

But that's just a little nit at worst, otherwise the series looks good
to me. Thanks for chasing this down.

For the x86 bits:

Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
Re: [PATCH v1 4/5] ACPI: processor: Rescan "dead" SMT siblings during initialization
Posted by Rafael J. Wysocki 6 months, 2 weeks ago
On Thu, Jun 5, 2025 at 6:14 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 6/5/25 08:07, Rafael J. Wysocki wrote:
> >  #ifdef CONFIG_ACPI_PROCESSOR_CSTATE
> > +void acpi_idle_rescan_dead_smt_siblings(void)
> > +{
> > +     if (cpuidle_get_driver() == &acpi_idle_driver)
> > +             arch_cpu_rescan_dead_smt_siblings();
> > +}
>
> My only thought in reading this is that maybe cpuidle_register_driver()
> would be a better spot to force the arch_cpu_rescan_dead_smt_siblings().
> That way, each driver would not have to do the rescan.

Unfortunately, this wouldn't work in the current arrangement of things
because cpuidle_register_driver() can be called in a CPU online path.

It should be possible to make this work in the future, but first things first.

> But that's just a little nit at worst, otherwise the series looks good
> to me. Thanks for chasing this down.
>
> For the x86 bits:
>
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>

Thank you!