linux-kernel - Re: [PATCH v1 6/9] ACPI: processor: idle: Do not change power states if get power info failed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0h64UoWY=Zkpc4g2UH+ii8K6APDSV2qLp==yHuRQz+fsQ@mail.gmail.com>
Date: Sun, 26 Oct 2025 13:34:28 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: "lihuisong (C)" <lihuisong@...wei.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, lenb@...nel.org, linux-acpi@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Sudeep.Holla@....com, linuxarm@...wei.com, 
	jonathan.cameron@...wei.com, zhanjie9@...ilicon.com, zhenglifeng1@...wei.com, 
	yubowen8@...wei.com
Subject: Re: [PATCH v1 6/9] ACPI: processor: idle: Do not change power states
 if get power info failed

On Fri, Oct 24, 2025 at 11:10 AM lihuisong (C) <lihuisong@...wei.com> wrote:
>
>
> 在 2025/10/22 3:49, Rafael J. Wysocki 写道:
> > On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@...wei.com> wrote:
> >> Driver will update power states when processor power states have been
> >> changed. To prevent any other abnormal issues, here add the verification
> >> for the result of getting power information, don't change power states
> >> and one error log when get power information failed.
> > But the old states may not be usable any more in that case.
> Yes
> >
> > If you want to check the acpi_processor_get_power_info(), it should
> > disable ACPi idle entirely on failures.
>  From the modification of this patch, this cpuidle device will be
> disabled if the acpi_processor_get_power_info(）fails to get on this device.
> And the cpuidle of the device will be disabled according to the
> definition of cpuidle_not_available().
> We should not call disable_cpuidle() to disable cpuidle of all CPUs.

Since the same idle state data is used for all CPUs, I'd say cpuidle
should be disabled for all of them in that case.

Alternatively, check if it works for any of them and apply the data
from the CPU where it works to all of them.  If it doesn't work for
any of them, there's nothing to apply.

> So the modification in this patch is enough, right?
> >
> >> Fixes: f427e5f1cf75 ("ACPI / processor: Get power info before updating the C-states")
> >> Signed-off-by: Huisong Li <lihuisong@...wei.com>
> >> ---
> >>   drivers/acpi/processor_idle.c | 15 ++++++++++-----
> >>   1 file changed, 10 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> >> index b0d6b51ee363..92b231f5d514 100644
> >> --- a/drivers/acpi/processor_idle.c
> >> +++ b/drivers/acpi/processor_idle.c
> >> @@ -1315,6 +1315,7 @@ int acpi_processor_power_state_has_changed(struct acpi_processor *pr)
> >>          int cpu;
> >>          struct acpi_processor *_pr;
> >>          struct cpuidle_device *dev;
> >> +       int ret = 0;
> >>
> >>          if (disabled_by_idle_boot_param())
> >>                  return 0;
> >> @@ -1344,16 +1345,20 @@ int acpi_processor_power_state_has_changed(struct acpi_processor *pr)
> >>                  }
> >>
> >>                  /* Populate Updated C-state information */
> >> -               acpi_processor_get_power_info(pr);
> >> -               acpi_processor_setup_cpuidle_states(pr);
> >> +               ret = acpi_processor_get_power_info(pr);
> >> +               if (ret)
> >> +                       pr_err("Get processor-%u power information failed.\n",
> >> +                              pr->id);
> >> +               else
> >> +                       acpi_processor_setup_cpuidle_states(pr);
> >>
> >>                  /* Enable all cpuidle devices */
> >>                  for_each_online_cpu(cpu) {
> >>                          _pr = per_cpu(processors, cpu);
> >>                          if (!_pr || !_pr->flags.power_setup_done)
> >>                                  continue;
> >> -                       acpi_processor_get_power_info(_pr);
> >> -                       if (_pr->flags.power) {
> >> +                       ret = acpi_processor_get_power_info(_pr);
> >> +                       if (!ret && _pr->flags.power) {
> >>                                  dev = per_cpu(acpi_cpuidle_device, cpu);
> >>                                  acpi_processor_setup_cpuidle_dev(_pr, dev);
> >>                                  cpuidle_enable_device(dev);
> >> @@ -1363,7 +1368,7 @@ int acpi_processor_power_state_has_changed(struct acpi_processor *pr)
> >>                  cpus_read_unlock();
> >>          }
> >>
> >> -       return 0;
> >> +       return ret;
> >>   }
> >>
> >>   void acpi_processor_register_idle_driver(void)
> >> --
> >> 2.33.0
> >>