Re: [PATCH 1/2] ACPI: processor: Do not use C3 w/o ARB_DIS=1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 20, 2022 at 3:44 PM Ville Syrjala
<ville.syrjala@xxxxxxxxxxxxxxx> wrote:
>
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx
> Cc: Woody Suwalski <wsuwalski@xxxxxxxxx>
> Cc: Mario Limonciello <mario.limonciello@xxxxxxx>
> Cc: Richard Gong <richard.gong@xxxxxxx>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> Fixes: d6b88ce2eb9d ("ACPI: processor idle: Allow playing dead in C3 state")
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
>  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



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux