Both ARM64 and RISCV architecture would validate Entry Method of LPI
state and SBI HSM or PSCI cpu suspend. Driver should return failure
if FFH of LPI state are not ok.
Fixes: a36a7fecfe60 ("ACPI / processor_idle: Add support for Low Power Idle(LPI) states")
Signed-off-by: Huisong Li <lihuisong@huawei.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 5684925338b3..b0d6b51ee363 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -1264,7 +1264,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;
return acpi_processor_setup_cpuidle_cx(pr, dev);
}
@@ -1275,7 +1275,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;
}
--
2.33.0On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
>
> Both ARM64 and RISCV architecture would validate Entry Method of LPI
> state and SBI HSM or PSCI cpu suspend. Driver should return failure
> if FFH of LPI state are not ok.
First of all, I cannot parse this changelog, so I don't know the
motivation for the change.
Second, if _LPI is ever used on x86, the
acpi_processor_ffh_lpi_probe() in acpi_processor_get_power_info() will
get in the way.
Why does the evaluation in acpi_processor_setup_cpuidle_dev() not work?
> Fixes: a36a7fecfe60 ("ACPI / processor_idle: Add support for Low Power Idle(LPI) states")
> Signed-off-by: Huisong Li <lihuisong@huawei.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 5684925338b3..b0d6b51ee363 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -1264,7 +1264,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;
>
> return acpi_processor_setup_cpuidle_cx(pr, dev);
> }
> @@ -1275,7 +1275,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;
> }
> --
> 2.33.0
>
在 2025/10/22 3:42, Rafael J. Wysocki 写道:
> On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
>> Both ARM64 and RISCV architecture would validate Entry Method of LPI
>> state and SBI HSM or PSCI cpu suspend. Driver should return failure
>> if FFH of LPI state are not ok.
> First of all, I cannot parse this changelog, so I don't know the
> motivation for the change.
Sorry for your confusion.
> Second, if _LPI is ever used on x86, the
> acpi_processor_ffh_lpi_probe() in acpi_processor_get_power_info() will
> get in the way.
AFAICS, it's also ok if X86 platform use LPI.
>
> Why does the evaluation in acpi_processor_setup_cpuidle_dev() not work?
The acpi_processor_ffh_lpi_probe does verify the validity of LPI for ARM
and RISCV.
But the caller of the acpi_processor_setup_cpuidle_dev()don't verify the
return value.
In addition, from the name of acpi_processor_setup_cpuidle_dev(), its
main purpose is to setup cpudile device rather than to verify LPI.
So I move it to a more prominent position and redefine the
acpi_processor_setup_cpuidle_dev to void in patch 9/9.
>
>> Fixes: a36a7fecfe60 ("ACPI / processor_idle: Add support for Low Power Idle(LPI) states")
>> Signed-off-by: Huisong Li <lihuisong@huawei.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 5684925338b3..b0d6b51ee363 100644
>> --- a/drivers/acpi/processor_idle.c
>> +++ b/drivers/acpi/processor_idle.c
>> @@ -1264,7 +1264,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;
>>
>> return acpi_processor_setup_cpuidle_cx(pr, dev);
>> }
>> @@ -1275,7 +1275,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;
>> }
>> --
>> 2.33.0
>>
On Thu, Oct 23, 2025 at 12:17 PM lihuisong (C) <lihuisong@huawei.com> wrote:
>
>
> 在 2025/10/22 3:42, Rafael J. Wysocki 写道:
> > On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
> >> Both ARM64 and RISCV architecture would validate Entry Method of LPI
> >> state and SBI HSM or PSCI cpu suspend. Driver should return failure
> >> if FFH of LPI state are not ok.
> > First of all, I cannot parse this changelog, so I don't know the
> > motivation for the change.
> Sorry for your confusion.
> > Second, if _LPI is ever used on x86, the
> > acpi_processor_ffh_lpi_probe() in acpi_processor_get_power_info() will
> > get in the way.
> AFAICS, it's also ok if X86 platform use LPI.
No, because it returns an error by default as it stands today.
> >
> > Why does the evaluation in acpi_processor_setup_cpuidle_dev() not work?
> The acpi_processor_ffh_lpi_probe does verify the validity of LPI for ARM
> and RISCV.
> But the caller of the acpi_processor_setup_cpuidle_dev()don't verify the
> return value.
> In addition, from the name of acpi_processor_setup_cpuidle_dev(), its
> main purpose is to setup cpudile device rather than to verify LPI.
That's fair enough.
Also, the list of idle states belongs to the cpuidle driver, not to a
cpuidle device.
> So I move it to a more prominent position and redefine the
> acpi_processor_setup_cpuidle_dev to void in patch 9/9.
> >
> >> Fixes: a36a7fecfe60 ("ACPI / processor_idle: Add support for Low Power Idle(LPI) states")
> >> Signed-off-by: Huisong Li <lihuisong@huawei.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 5684925338b3..b0d6b51ee363 100644
> >> --- a/drivers/acpi/processor_idle.c
> >> +++ b/drivers/acpi/processor_idle.c
> >> @@ -1264,7 +1264,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;
> >>
> >> return acpi_processor_setup_cpuidle_cx(pr, dev);
> >> }
> >> @@ -1275,7 +1275,13 @@ static int acpi_processor_get_power_info(struct acpi_processor *pr)
> >>
> >> ret = acpi_processor_get_lpi_info(pr);
> >> if (ret)
So I think it would be better to check it here, that is
if (!ret) {
ret = acpi_processor_ffh_lpi_probe(pr->id));
if (!ret)
return 0;
pr_info("CPU%d: FFH LPI state is invalid\n", pr->id);
pr->flags.has_lpi = 0;
}
return acpi_processor_get_cstate_info(pr);
And the default acpi_processor_ffh_lpi_probe() needs to be changed to return 0.
> >> - 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;
> >> }
> >> --
在 2025/10/23 18:35, Rafael J. Wysocki 写道:
> On Thu, Oct 23, 2025 at 12:17 PM lihuisong (C) <lihuisong@huawei.com> wrote:
>>
>> 在 2025/10/22 3:42, Rafael J. Wysocki 写道:
>>> On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
>>>> Both ARM64 and RISCV architecture would validate Entry Method of LPI
>>>> state and SBI HSM or PSCI cpu suspend. Driver should return failure
>>>> if FFH of LPI state are not ok.
>>> First of all, I cannot parse this changelog, so I don't know the
>>> motivation for the change.
>> Sorry for your confusion.
>>> Second, if _LPI is ever used on x86, the
>>> acpi_processor_ffh_lpi_probe() in acpi_processor_get_power_info() will
>>> get in the way.
>> AFAICS, it's also ok if X86 platform use LPI.
> No, because it returns an error by default as it stands today.
>
>>> Why does the evaluation in acpi_processor_setup_cpuidle_dev() not work?
>> The acpi_processor_ffh_lpi_probe does verify the validity of LPI for ARM
>> and RISCV.
>> But the caller of the acpi_processor_setup_cpuidle_dev()don't verify the
>> return value.
>> In addition, from the name of acpi_processor_setup_cpuidle_dev(), its
>> main purpose is to setup cpudile device rather than to verify LPI.
> That's fair enough.
>
> Also, the list of idle states belongs to the cpuidle driver, not to a
> cpuidle device.
>
>> So I move it to a more prominent position and redefine the
>> acpi_processor_setup_cpuidle_dev to void in patch 9/9.
>>>> Fixes: a36a7fecfe60 ("ACPI / processor_idle: Add support for Low Power Idle(LPI) states")
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.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 5684925338b3..b0d6b51ee363 100644
>>>> --- a/drivers/acpi/processor_idle.c
>>>> +++ b/drivers/acpi/processor_idle.c
>>>> @@ -1264,7 +1264,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;
>>>>
>>>> return acpi_processor_setup_cpuidle_cx(pr, dev);
>>>> }
>>>> @@ -1275,7 +1275,13 @@ static int acpi_processor_get_power_info(struct acpi_processor *pr)
>>>>
>>>> ret = acpi_processor_get_lpi_info(pr);
>>>> if (ret)
> So I think it would be better to check it here, that is
>
> if (!ret) {
> ret = acpi_processor_ffh_lpi_probe(pr->id));
> if (!ret)
> return 0;
>
> pr_info("CPU%d: FFH LPI state is invalid\n", pr->id);
> pr->flags.has_lpi = 0;
> }
>
> return acpi_processor_get_cstate_info(pr);
>
> And the default acpi_processor_ffh_lpi_probe() needs to be changed to return 0.
Sorry, I don't understand why pr->flags.has_lpi is true if
acpi_processor_ffh_lpi_probe() return failure.
In addition, X86 platform doesn't define acpi_processor_ffh_lpi_probe().
this function will return EOPNOTSUPP.
>
>>>> - 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;
>>>> }
>>>> --
On Fri, Oct 24, 2025 at 11:40 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>
>
> 在 2025/10/23 18:35, Rafael J. Wysocki 写道:
> > On Thu, Oct 23, 2025 at 12:17 PM lihuisong (C) <lihuisong@huawei.com> wrote:
> >>
> >> 在 2025/10/22 3:42, Rafael J. Wysocki 写道:
> >>> On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
> >>>> Both ARM64 and RISCV architecture would validate Entry Method of LPI
> >>>> state and SBI HSM or PSCI cpu suspend. Driver should return failure
> >>>> if FFH of LPI state are not ok.
> >>> First of all, I cannot parse this changelog, so I don't know the
> >>> motivation for the change.
> >> Sorry for your confusion.
> >>> Second, if _LPI is ever used on x86, the
> >>> acpi_processor_ffh_lpi_probe() in acpi_processor_get_power_info() will
> >>> get in the way.
> >> AFAICS, it's also ok if X86 platform use LPI.
> > No, because it returns an error by default as it stands today.
> >
> >>> Why does the evaluation in acpi_processor_setup_cpuidle_dev() not work?
> >> The acpi_processor_ffh_lpi_probe does verify the validity of LPI for ARM
> >> and RISCV.
> >> But the caller of the acpi_processor_setup_cpuidle_dev()don't verify the
> >> return value.
> >> In addition, from the name of acpi_processor_setup_cpuidle_dev(), its
> >> main purpose is to setup cpudile device rather than to verify LPI.
> > That's fair enough.
> >
> > Also, the list of idle states belongs to the cpuidle driver, not to a
> > cpuidle device.
> >
> >> So I move it to a more prominent position and redefine the
> >> acpi_processor_setup_cpuidle_dev to void in patch 9/9.
> >>>> Fixes: a36a7fecfe60 ("ACPI / processor_idle: Add support for Low Power Idle(LPI) states")
> >>>> Signed-off-by: Huisong Li <lihuisong@huawei.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 5684925338b3..b0d6b51ee363 100644
> >>>> --- a/drivers/acpi/processor_idle.c
> >>>> +++ b/drivers/acpi/processor_idle.c
> >>>> @@ -1264,7 +1264,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;
> >>>>
> >>>> return acpi_processor_setup_cpuidle_cx(pr, dev);
> >>>> }
> >>>> @@ -1275,7 +1275,13 @@ static int acpi_processor_get_power_info(struct acpi_processor *pr)
> >>>>
> >>>> ret = acpi_processor_get_lpi_info(pr);
> >>>> if (ret)
> > So I think it would be better to check it here, that is
> >
> > if (!ret) {
> > ret = acpi_processor_ffh_lpi_probe(pr->id));
> > if (!ret)
> > return 0;
> >
> > pr_info("CPU%d: FFH LPI state is invalid\n", pr->id);
> > pr->flags.has_lpi = 0;
> > }
> >
> > return acpi_processor_get_cstate_info(pr);
> >
> > And the default acpi_processor_ffh_lpi_probe() needs to be changed to return 0.
> Sorry, I don't understand why pr->flags.has_lpi is true if
> acpi_processor_ffh_lpi_probe() return failure.
It is set by acpi_processor_get_lpi_info() on success and
acpi_processor_ffh_lpi_probe() does not update it.
> In addition, X86 platform doesn't define acpi_processor_ffh_lpi_probe().
> this function will return EOPNOTSUPP.
Which is exactly why it is a problem. x86 has no reason to implement
it because FFH always works there.
> >
> >>>> - 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;
> >>>> }
> >>>> --
在 2025/10/26 20:40, Rafael J. Wysocki 写道:
> On Fri, Oct 24, 2025 at 11:40 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>>
>> 在 2025/10/23 18:35, Rafael J. Wysocki 写道:
>>> On Thu, Oct 23, 2025 at 12:17 PM lihuisong (C) <lihuisong@huawei.com> wrote:
>>>> 在 2025/10/22 3:42, Rafael J. Wysocki 写道:
>>>>> On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
>>>>>> Both ARM64 and RISCV architecture would validate Entry Method of LPI
>>>>>> state and SBI HSM or PSCI cpu suspend. Driver should return failure
>>>>>> if FFH of LPI state are not ok.
>>>>> First of all, I cannot parse this changelog, so I don't know the
>>>>> motivation for the change.
>>>> Sorry for your confusion.
>>>>> Second, if _LPI is ever used on x86, the
>>>>> acpi_processor_ffh_lpi_probe() in acpi_processor_get_power_info() will
>>>>> get in the way.
>>>> AFAICS, it's also ok if X86 platform use LPI.
>>> No, because it returns an error by default as it stands today.
>>>
>>>>> Why does the evaluation in acpi_processor_setup_cpuidle_dev() not work?
>>>> The acpi_processor_ffh_lpi_probe does verify the validity of LPI for ARM
>>>> and RISCV.
>>>> But the caller of the acpi_processor_setup_cpuidle_dev()don't verify the
>>>> return value.
>>>> In addition, from the name of acpi_processor_setup_cpuidle_dev(), its
>>>> main purpose is to setup cpudile device rather than to verify LPI.
>>> That's fair enough.
>>>
>>> Also, the list of idle states belongs to the cpuidle driver, not to a
>>> cpuidle device.
>>>
>>>> So I move it to a more prominent position and redefine the
>>>> acpi_processor_setup_cpuidle_dev to void in patch 9/9.
>>>>>> Fixes: a36a7fecfe60 ("ACPI / processor_idle: Add support for Low Power Idle(LPI) states")
>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.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 5684925338b3..b0d6b51ee363 100644
>>>>>> --- a/drivers/acpi/processor_idle.c
>>>>>> +++ b/drivers/acpi/processor_idle.c
>>>>>> @@ -1264,7 +1264,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;
>>>>>>
>>>>>> return acpi_processor_setup_cpuidle_cx(pr, dev);
>>>>>> }
>>>>>> @@ -1275,7 +1275,13 @@ static int acpi_processor_get_power_info(struct acpi_processor *pr)
>>>>>>
>>>>>> ret = acpi_processor_get_lpi_info(pr);
>>>>>> if (ret)
>>> So I think it would be better to check it here, that is
>>>
>>> if (!ret) {
>>> ret = acpi_processor_ffh_lpi_probe(pr->id));
>>> if (!ret)
>>> return 0;
>>>
>>> pr_info("CPU%d: FFH LPI state is invalid\n", pr->id);
>>> pr->flags.has_lpi = 0;
>>> }
>>>
>>> return acpi_processor_get_cstate_info(pr);
>>>
>>> And the default acpi_processor_ffh_lpi_probe() needs to be changed to return 0.
>> Sorry, I don't understand why pr->flags.has_lpi is true if
>> acpi_processor_ffh_lpi_probe() return failure.
> It is set by acpi_processor_get_lpi_info() on success and
> acpi_processor_ffh_lpi_probe() does not update it.
The acpi_processor_get_lpi_info() will return failure on X86 platform
because this function first call acpi_processor_ffh_lpi_probe().
And acpi_processor_ffh_lpi_probe return EOPNOTSUPP because X86 platform
doesn't implement it.
So I think pr->flags.has_lpi is false on X86 plaform.
>
>> In addition, X86 platform doesn't define acpi_processor_ffh_lpi_probe().
>> this function will return EOPNOTSUPP.
> Which is exactly why it is a problem. x86 has no reason to implement
> it because FFH always works there.
Sorry, I still don't understand why X86 has no reason to implement it.
I simply think that X86 doesn't need it.
AFAICS, the platform doesn't need to get LPI info if this platform
doesn't implement acpi_processor_ffh_lpi_probe().
>
>>>>>> - 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;
>>>>>> }
>>>>>> --
On Mon, Oct 27, 2025 at 2:43 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>
>
> 在 2025/10/26 20:40, Rafael J. Wysocki 写道:
> > On Fri, Oct 24, 2025 at 11:40 AM lihuisong (C) <lihuisong@huawei.com> wrote:
> >>
> >> 在 2025/10/23 18:35, Rafael J. Wysocki 写道:
> >>> On Thu, Oct 23, 2025 at 12:17 PM lihuisong (C) <lihuisong@huawei.com> wrote:
> >>>> 在 2025/10/22 3:42, Rafael J. Wysocki 写道:
> >>>>> On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
> >>>>>> Both ARM64 and RISCV architecture would validate Entry Method of LPI
> >>>>>> state and SBI HSM or PSCI cpu suspend. Driver should return failure
> >>>>>> if FFH of LPI state are not ok.
> >>>>> First of all, I cannot parse this changelog, so I don't know the
> >>>>> motivation for the change.
> >>>> Sorry for your confusion.
> >>>>> Second, if _LPI is ever used on x86, the
> >>>>> acpi_processor_ffh_lpi_probe() in acpi_processor_get_power_info() will
> >>>>> get in the way.
> >>>> AFAICS, it's also ok if X86 platform use LPI.
> >>> No, because it returns an error by default as it stands today.
> >>>
> >>>>> Why does the evaluation in acpi_processor_setup_cpuidle_dev() not work?
> >>>> The acpi_processor_ffh_lpi_probe does verify the validity of LPI for ARM
> >>>> and RISCV.
> >>>> But the caller of the acpi_processor_setup_cpuidle_dev()don't verify the
> >>>> return value.
> >>>> In addition, from the name of acpi_processor_setup_cpuidle_dev(), its
> >>>> main purpose is to setup cpudile device rather than to verify LPI.
> >>> That's fair enough.
> >>>
> >>> Also, the list of idle states belongs to the cpuidle driver, not to a
> >>> cpuidle device.
> >>>
> >>>> So I move it to a more prominent position and redefine the
> >>>> acpi_processor_setup_cpuidle_dev to void in patch 9/9.
> >>>>>> Fixes: a36a7fecfe60 ("ACPI / processor_idle: Add support for Low Power Idle(LPI) states")
> >>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.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 5684925338b3..b0d6b51ee363 100644
> >>>>>> --- a/drivers/acpi/processor_idle.c
> >>>>>> +++ b/drivers/acpi/processor_idle.c
> >>>>>> @@ -1264,7 +1264,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;
> >>>>>>
> >>>>>> return acpi_processor_setup_cpuidle_cx(pr, dev);
> >>>>>> }
> >>>>>> @@ -1275,7 +1275,13 @@ static int acpi_processor_get_power_info(struct acpi_processor *pr)
> >>>>>>
> >>>>>> ret = acpi_processor_get_lpi_info(pr);
> >>>>>> if (ret)
> >>> So I think it would be better to check it here, that is
> >>>
> >>> if (!ret) {
> >>> ret = acpi_processor_ffh_lpi_probe(pr->id));
> >>> if (!ret)
> >>> return 0;
> >>>
> >>> pr_info("CPU%d: FFH LPI state is invalid\n", pr->id);
> >>> pr->flags.has_lpi = 0;
> >>> }
> >>>
> >>> return acpi_processor_get_cstate_info(pr);
> >>>
> >>> And the default acpi_processor_ffh_lpi_probe() needs to be changed to return 0.
> >> Sorry, I don't understand why pr->flags.has_lpi is true if
> >> acpi_processor_ffh_lpi_probe() return failure.
> > It is set by acpi_processor_get_lpi_info() on success and
> > acpi_processor_ffh_lpi_probe() does not update it.
> The acpi_processor_get_lpi_info() will return failure on X86 platform
> because this function first call acpi_processor_ffh_lpi_probe().
> And acpi_processor_ffh_lpi_probe return EOPNOTSUPP because X86 platform
> doesn't implement it.
> So I think pr->flags.has_lpi is false on X86 plaform.
On x86 it is 0, but what if acpi_processor_ffh_lpi_probe() fails on arm64, say?
> >
> >> In addition, X86 platform doesn't define acpi_processor_ffh_lpi_probe().
> >> this function will return EOPNOTSUPP.
> > Which is exactly why it is a problem. x86 has no reason to implement
> > it because FFH always works there.
> Sorry, I still don't understand why X86 has no reason to implement it.
> I simply think that X86 doesn't need it.
> AFAICS, the platform doesn't need to get LPI info if this platform
> doesn't implement acpi_processor_ffh_lpi_probe().
Well, that's what is implemented in the current code, but it will need
to be changed if x86 is ever added and I'd rather avoid cleanups
making it harder to change.
在 2025/10/27 20:28, Rafael J. Wysocki 写道:
> On Mon, Oct 27, 2025 at 2:43 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>>
>> 在 2025/10/26 20:40, Rafael J. Wysocki 写道:
>>> On Fri, Oct 24, 2025 at 11:40 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>>>> 在 2025/10/23 18:35, Rafael J. Wysocki 写道:
>>>>> On Thu, Oct 23, 2025 at 12:17 PM lihuisong (C) <lihuisong@huawei.com> wrote:
>>>>>> 在 2025/10/22 3:42, Rafael J. Wysocki 写道:
>>>>>>> On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
>>>>>>>> Both ARM64 and RISCV architecture would validate Entry Method of LPI
>>>>>>>> state and SBI HSM or PSCI cpu suspend. Driver should return failure
>>>>>>>> if FFH of LPI state are not ok.
>>>>>>> First of all, I cannot parse this changelog, so I don't know the
>>>>>>> motivation for the change.
>>>>>> Sorry for your confusion.
>>>>>>> Second, if _LPI is ever used on x86, the
>>>>>>> acpi_processor_ffh_lpi_probe() in acpi_processor_get_power_info() will
>>>>>>> get in the way.
>>>>>> AFAICS, it's also ok if X86 platform use LPI.
>>>>> No, because it returns an error by default as it stands today.
>>>>>
>>>>>>> Why does the evaluation in acpi_processor_setup_cpuidle_dev() not work?
>>>>>> The acpi_processor_ffh_lpi_probe does verify the validity of LPI for ARM
>>>>>> and RISCV.
>>>>>> But the caller of the acpi_processor_setup_cpuidle_dev()don't verify the
>>>>>> return value.
>>>>>> In addition, from the name of acpi_processor_setup_cpuidle_dev(), its
>>>>>> main purpose is to setup cpudile device rather than to verify LPI.
>>>>> That's fair enough.
>>>>>
>>>>> Also, the list of idle states belongs to the cpuidle driver, not to a
>>>>> cpuidle device.
>>>>>
>>>>>> So I move it to a more prominent position and redefine the
>>>>>> acpi_processor_setup_cpuidle_dev to void in patch 9/9.
>>>>>>>> Fixes: a36a7fecfe60 ("ACPI / processor_idle: Add support for Low Power Idle(LPI) states")
>>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.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 5684925338b3..b0d6b51ee363 100644
>>>>>>>> --- a/drivers/acpi/processor_idle.c
>>>>>>>> +++ b/drivers/acpi/processor_idle.c
>>>>>>>> @@ -1264,7 +1264,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;
>>>>>>>>
>>>>>>>> return acpi_processor_setup_cpuidle_cx(pr, dev);
>>>>>>>> }
>>>>>>>> @@ -1275,7 +1275,13 @@ static int acpi_processor_get_power_info(struct acpi_processor *pr)
>>>>>>>>
>>>>>>>> ret = acpi_processor_get_lpi_info(pr);
>>>>>>>> if (ret)
>>>>> So I think it would be better to check it here, that is
>>>>>
>>>>> if (!ret) {
>>>>> ret = acpi_processor_ffh_lpi_probe(pr->id));
>>>>> if (!ret)
>>>>> return 0;
>>>>>
>>>>> pr_info("CPU%d: FFH LPI state is invalid\n", pr->id);
>>>>> pr->flags.has_lpi = 0;
>>>>> }
>>>>>
>>>>> return acpi_processor_get_cstate_info(pr);
>>>>>
>>>>> And the default acpi_processor_ffh_lpi_probe() needs to be changed to return 0.
>>>> Sorry, I don't understand why pr->flags.has_lpi is true if
>>>> acpi_processor_ffh_lpi_probe() return failure.
>>> It is set by acpi_processor_get_lpi_info() on success and
>>> acpi_processor_ffh_lpi_probe() does not update it.
>> The acpi_processor_get_lpi_info() will return failure on X86 platform
>> because this function first call acpi_processor_ffh_lpi_probe().
>> And acpi_processor_ffh_lpi_probe return EOPNOTSUPP because X86 platform
>> doesn't implement it.
>> So I think pr->flags.has_lpi is false on X86 plaform.
> On x86 it is 0, but what if acpi_processor_ffh_lpi_probe() fails on arm64, say?
Arm64 supports the acpi_processor_ffh_lpi_probe().
So pr->flags.has_lpi is 1 on success.
>>>> In addition, X86 platform doesn't define acpi_processor_ffh_lpi_probe().
>>>> this function will return EOPNOTSUPP.
>>> Which is exactly why it is a problem. x86 has no reason to implement
>>> it because FFH always works there.
>> Sorry, I still don't understand why X86 has no reason to implement it.
>> I simply think that X86 doesn't need it.
>> AFAICS, the platform doesn't need to get LPI info if this platform
>> doesn't implement acpi_processor_ffh_lpi_probe().
> Well, that's what is implemented in the current code, but it will need
> to be changed if x86 is ever added and I'd rather avoid cleanups
> making it harder to change.
What you mean is that X86 use LPI?
If X86 also define acpi_processor_ffh_lpi_probe and use LPI, this patch
is also good to it.
On Tue, Oct 28, 2025 at 1:45 PM lihuisong (C) <lihuisong@huawei.com> wrote:
>
>
> 在 2025/10/27 20:28, Rafael J. Wysocki 写道:
> > On Mon, Oct 27, 2025 at 2:43 AM lihuisong (C) <lihuisong@huawei.com> wrote:
> >>
> >> 在 2025/10/26 20:40, Rafael J. Wysocki 写道:
> >>> On Fri, Oct 24, 2025 at 11:40 AM lihuisong (C) <lihuisong@huawei.com> wrote:
> >>>> 在 2025/10/23 18:35, Rafael J. Wysocki 写道:
> >>>>> On Thu, Oct 23, 2025 at 12:17 PM lihuisong (C) <lihuisong@huawei.com> wrote:
> >>>>>> 在 2025/10/22 3:42, Rafael J. Wysocki 写道:
> >>>>>>> On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
> >>>>>>>> Both ARM64 and RISCV architecture would validate Entry Method of LPI
> >>>>>>>> state and SBI HSM or PSCI cpu suspend. Driver should return failure
> >>>>>>>> if FFH of LPI state are not ok.
> >>>>>>> First of all, I cannot parse this changelog, so I don't know the
> >>>>>>> motivation for the change.
> >>>>>> Sorry for your confusion.
> >>>>>>> Second, if _LPI is ever used on x86, the
> >>>>>>> acpi_processor_ffh_lpi_probe() in acpi_processor_get_power_info() will
> >>>>>>> get in the way.
> >>>>>> AFAICS, it's also ok if X86 platform use LPI.
> >>>>> No, because it returns an error by default as it stands today.
> >>>>>
> >>>>>>> Why does the evaluation in acpi_processor_setup_cpuidle_dev() not work?
> >>>>>> The acpi_processor_ffh_lpi_probe does verify the validity of LPI for ARM
> >>>>>> and RISCV.
> >>>>>> But the caller of the acpi_processor_setup_cpuidle_dev()don't verify the
> >>>>>> return value.
> >>>>>> In addition, from the name of acpi_processor_setup_cpuidle_dev(), its
> >>>>>> main purpose is to setup cpudile device rather than to verify LPI.
> >>>>> That's fair enough.
> >>>>>
> >>>>> Also, the list of idle states belongs to the cpuidle driver, not to a
> >>>>> cpuidle device.
> >>>>>
> >>>>>> So I move it to a more prominent position and redefine the
> >>>>>> acpi_processor_setup_cpuidle_dev to void in patch 9/9.
> >>>>>>>> Fixes: a36a7fecfe60 ("ACPI / processor_idle: Add support for Low Power Idle(LPI) states")
> >>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.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 5684925338b3..b0d6b51ee363 100644
> >>>>>>>> --- a/drivers/acpi/processor_idle.c
> >>>>>>>> +++ b/drivers/acpi/processor_idle.c
> >>>>>>>> @@ -1264,7 +1264,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;
> >>>>>>>>
> >>>>>>>> return acpi_processor_setup_cpuidle_cx(pr, dev);
> >>>>>>>> }
> >>>>>>>> @@ -1275,7 +1275,13 @@ static int acpi_processor_get_power_info(struct acpi_processor *pr)
> >>>>>>>>
> >>>>>>>> ret = acpi_processor_get_lpi_info(pr);
> >>>>>>>> if (ret)
> >>>>> So I think it would be better to check it here, that is
> >>>>>
> >>>>> if (!ret) {
> >>>>> ret = acpi_processor_ffh_lpi_probe(pr->id));
> >>>>> if (!ret)
> >>>>> return 0;
> >>>>>
> >>>>> pr_info("CPU%d: FFH LPI state is invalid\n", pr->id);
> >>>>> pr->flags.has_lpi = 0;
> >>>>> }
> >>>>>
> >>>>> return acpi_processor_get_cstate_info(pr);
> >>>>>
> >>>>> And the default acpi_processor_ffh_lpi_probe() needs to be changed to return 0.
> >>>> Sorry, I don't understand why pr->flags.has_lpi is true if
> >>>> acpi_processor_ffh_lpi_probe() return failure.
> >>> It is set by acpi_processor_get_lpi_info() on success and
> >>> acpi_processor_ffh_lpi_probe() does not update it.
> >> The acpi_processor_get_lpi_info() will return failure on X86 platform
> >> because this function first call acpi_processor_ffh_lpi_probe().
> >> And acpi_processor_ffh_lpi_probe return EOPNOTSUPP because X86 platform
> >> doesn't implement it.
> >> So I think pr->flags.has_lpi is false on X86 plaform.
> > On x86 it is 0, but what if acpi_processor_ffh_lpi_probe() fails on arm64, say?
> Arm64 supports the acpi_processor_ffh_lpi_probe().
> So pr->flags.has_lpi is 1 on success.
> >>>> In addition, X86 platform doesn't define acpi_processor_ffh_lpi_probe().
> >>>> this function will return EOPNOTSUPP.
> >>> Which is exactly why it is a problem. x86 has no reason to implement
> >>> it because FFH always works there.
> >> Sorry, I still don't understand why X86 has no reason to implement it.
> >> I simply think that X86 doesn't need it.
> >> AFAICS, the platform doesn't need to get LPI info if this platform
> >> doesn't implement acpi_processor_ffh_lpi_probe().
>
> > Well, that's what is implemented in the current code, but it will need
> > to be changed if x86 is ever added and I'd rather avoid cleanups
> > making it harder to change.
>
> What you mean is that X86 use LPI?
In the future, x86 may want to use _LPI, that's all.
> If X86 also define acpi_processor_ffh_lpi_probe and use LPI, this patch
> is also good to it.
Well, fair enough.
© 2016 - 2026 Red Hat, Inc.