Rafael J. Wysocki wrote:

On Wed, Apr 20, 2022 at 3:44 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:

...

From: Ville Syrjälä ville.syrjala@linux.intel.com

commit d6b88ce2eb9d ("ACPI: processor idle: Allow playing dead in C3 state") was supposedly just trying to enable C3 when the CPU is offlined, but it also mistakenly enabled C3 usage without setting ARB_DIS=1 in normal idle scenarios.

This results in a machine that won't boot past the point when it first enters C3. Restore the correct behaviour (either demote to C1/C2, or use C3 but also set ARB_DIS=1).

I hit this on a Fujitsu Siemens Lifebook S6010 (P3) machine.

Cc: stable@vger.kernel.org Cc: Woody Suwalski wsuwalski@gmail.com Cc: Mario Limonciello mario.limonciello@amd.com Cc: Richard Gong richard.gong@amd.com Cc: Rafael J. Wysocki rafael.j.wysocki@intel.com Fixes: d6b88ce2eb9d ("ACPI: processor idle: Allow playing dead in C3 state") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com

---

drivers/acpi/processor_idle.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 4556c86c3465..54f0a1915025 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -793,10 +793,10 @@ static int acpi_processor_setup_cstates(struct acpi_processor *pr)

             state->flags = 0;
             if (cx->type == ACPI_STATE_C1 || cx->type == ACPI_STATE_C2 ||

•               cx->type == ACPI_STATE_C3) {
  

•               cx->type == ACPI_STATE_C3)
                     state->enter_dead = acpi_idle_play_dead;
  

•           if (cx->type == ACPI_STATE_C1 || cx->type == ACPI_STATE_C2)
                     drv->safe_state_index = count;
  

•           }
             /*
              * Halt-induced C1 is not good for ->enter_s2idle, because it
              * re-enables interrupts on exit.  Moreover, C1 is generally not
  

--

Good catch, but I would prefer doing the below which should be equivalent (modulo GMail-induced white space breakage):

---

drivers/acpi/processor_idle.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/acpi/processor_idle.c

--- linux-pm.orig/drivers/acpi/processor_idle.c +++ linux-pm/drivers/acpi/processor_idle.c @@ -795,7 +795,8 @@ static int acpi_processor_setup_cstates( if (cx->type == ACPI_STATE_C1 || cx->type == ACPI_STATE_C2 || cx->type == ACPI_STATE_C3) { state->enter_dead = acpi_idle_play_dead;

•        drv->safe_state_index = count;
  

•        if (cx->type != ACPI_STATE_C3)
  

•            drv->safe_state_index = count;
      }
      /*
       * Halt-induced C1 is not good for ->enter_s2idle, because it
  

I have tested both solutions from Ville and Rafael, and both work OK on T40. Clearly, since Ville has duped the issue on a non-T40 machine, the processor_power_dmi_table[] solution is incorrect.

Rafael's solution seems simpler - so I also like it more :-)

Thanks, Woody

On Thu, Apr 21, 2022 at 3:36 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:

From: Ville Syrjälä ville.syrjala@linux.intel.com

Thanks for the updating!

I'm going to apply this for 5.18-rc along with the [2/2].

However, I'm going to change the subject of this patch to "ACPI: processor: idle: Avoid falling back to C3 type C-states".

commit d6b88ce2eb9d ("ACPI: processor idle: Allow playing dead in C3 state") was supposedly just trying to enable C3 when the CPU is offlined, but it also mistakenly enabled C3 usage without setting ARB_DIS=1 in normal idle scenarios.

And I'm going to replace the above paragraph in the changelog with:

"The "safe state" index is used by acpi_idle_enter_bm() to avoid entering a C-state that may require bus mastering to be disabled on entry in the cases when this is not going to happen. For this reason, it should not be set to point to C3 type of C-states, because they may require bus mastering to be disabled on entry in principle.

This was broken by commit d6b88ce2eb9d ("ACPI: processor idle: Allow playing dead in C3 state") which inadvertently allowed the "safe state" index to point to C3 type of C-states."

This results in a machine that won't boot past the point when it first enters C3. Restore the correct behaviour (either demote to C1/C2, or use C3 but also set ARB_DIS=1).

I hit this on a Fujitsu Siemens Lifebook S6010 (P3) machine.

Cc: stable@vger.kernel.org Cc: Woody Suwalski wsuwalski@gmail.com Cc: Mario Limonciello mario.limonciello@amd.com Cc: Richard Gong richard.gong@amd.com Cc: Rafael J. Wysocki rafael.j.wysocki@intel.com Fixes: d6b88ce2eb9d ("ACPI: processor idle: Allow playing dead in C3 state") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com

---

v2: Paint it in a different color (Rafael)

drivers/acpi/processor_idle.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 4556c86c3465..5f296e099bce 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -795,7 +795,8 @@ static int acpi_processor_setup_cstates(struct acpi_processor *pr) if (cx->type == ACPI_STATE_C1 || cx->type == ACPI_STATE_C2 || cx->type == ACPI_STATE_C3) { state->enter_dead = acpi_idle_play_dead;

•                   drv->safe_state_index = count;
  

•                   if (cx->type != ACPI_STATE_C3)
  

•                           drv->safe_state_index = count;
            }
            /*
             * Halt-induced C1 is not good for ->enter_s2idle, because it
  

-- 2.35.1