linux-kernel - Re: [PATCH 1/3] ACPI: processor: idle: Relocate and verify acpi_processor_ffh_lpi

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAJZ5v0gq=Cw49fEozs+9VT=UtgmzHgBCRgW38w956cEzb-1vow@mail.gmail.com>
Date: Fri, 16 Jan 2026 13:22:13 +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 1/3] ACPI: processor: idle: Relocate and verify acpi_processor_ffh_lpi_probe

On Fri, Jan 16, 2026 at 7:38 AM lihuisong (C) <lihuisong@...wei.com> wrote:
>
>
> On 1/15/2026 9:06 PM, Rafael J. Wysocki wrote:
> > On Thu, Jan 15, 2026 at 1:09 PM lihuisong (C) <lihuisong@...wei.com> wrote:
> >> Hi Rafael,
> >>
> >> On 1/15/2026 1:27 AM, Rafael J. Wysocki wrote:
> >>> On Tue, Nov 25, 2025 at 7:52 AM Huisong Li <lihuisong@...wei.com> wrote:
> >>>> The platform used LPI need check if the LPI support and the entry
> >>>> method is valid by the acpi_processor_ffh_lpi_probe(). But the return
> >>>> of acpi_processor_ffh_lpi_probe() in acpi_processor_setup_cpuidle_dev()
> >>>> isn't verified by any caller.
> >>>>
> >>>> What's more, acpi_processor_get_power_info() is a more logical place for
> >>>> verifying the validity of FFH LPI than acpi_processor_setup_cpuidle_dev().
> >>>> So move acpi_processor_ffh_lpi_probe() from the latter to the former and
> >>>> verify its return.
> >>>>
> >>>> Signed-off-by: Huisong Li <lihuisong@...wei.com>
> >>>> ---
> >>>>    drivers/acpi/processor_idle.c | 10 ++++++++--
> >>>>    1 file changed, 8 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> >>>> index 5f86297c8b23..cdf86874a87a 100644
> >>>> --- a/drivers/acpi/processor_idle.c
> >>>> +++ b/drivers/acpi/processor_idle.c
> >>>> @@ -1252,7 +1252,7 @@ static int acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
> >>>>
> >>>>           dev->cpu = pr->id;
> >>>>           if (pr->flags.has_lpi)
> >>>> -               return acpi_processor_ffh_lpi_probe(pr->id);
> >>>> +               return 0;
> >>>>
> >>>>           acpi_processor_setup_cpuidle_cx(pr, dev);
> >>>>           return 0;
> >>>> @@ -1264,7 +1264,13 @@ static int acpi_processor_get_power_info(struct acpi_processor *pr)
> >>>>
> >>>>           ret = acpi_processor_get_lpi_info(pr);
> >>>>           if (ret)
> >>>> -               ret = acpi_processor_get_cstate_info(pr);
> >>>> +               return acpi_processor_get_cstate_info(pr);
> >>>> +
> >>>> +       if (pr->flags.has_lpi) {
> >>>> +               ret = acpi_processor_ffh_lpi_probe(pr->id);
> >>>> +               if (ret)
> >>>> +                       pr_err("Processor FFH LPI state is invalid.\n");
> >>>> +       }
> >>>>
> >>>>           return ret;
> >>>>    }
> >>>> --
> >>> Please reorder this behind the next patch in the series.
> >> Patch 2/3 depends on this patch.
> >> So I don't know how to reorder this patch.
> > I should have been more precise, sorry.
> >
> > Please first convert acpi_processor_setup_cpuidle_dev() to a void
> > function and then make the changes from this patch on top of that.
> The acpi_processor_ffh_lpi_probe may return an error.
> And acpi_processor_setup_cpuidle_dev can pass the error code to its
> caller(Although the caller ignored it currently).

If all of its callers ignore its return value, it can and arguably
should be a void function.

> It may be inapproprate to convert acpi_processor_setup_cpuidle_dev() to
> a void function directly if we doesn't move acpi_processor_ffh_lpi_probe
> out first.
> So I first relocate the position of acpi_processor_ffh_lpi_probe. Then
> changing it to a void function would be more logical.
>
> Or we need to drop the return value of acpi_processor_ffh_lpi_probe and
> convert acpi_processor_setup_cpuidle_dev to a void function, like:
> -->
>
> -static int acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
> -                                           struct cpuidle_device *dev)
> +static void acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
> +                                            struct cpuidle_device *dev)
>   {
>         if (!pr->flags.power_setup_done || !pr->flags.power || !dev)
> -               return -EINVAL;
> +               return;
>
>         dev->cpu = pr->id;
>         if (pr->flags.has_lpi) {
> -               return acpi_processor_ffh_lpi_probe();
> +               acpi_processor_ffh_lpi_probe();
> +               return;
>          }
>
>         acpi_processor_setup_cpuidle_cx(pr, dev);
> -       return 0;
>   }
>
> What do you think now?

Well, please see above.