[PATCH v1 2/9] ACPI: processor: idle: Return failure if entry method is not buffer or integer type

Huisong Li posted 9 patches 4 months, 1 week ago
There is a newer version of this series
[PATCH v1 2/9] ACPI: processor: idle: Return failure if entry method is not buffer or integer type
Posted by Huisong Li 4 months, 1 week ago
According to ACPI spec, entry method in LPI sub-package must be buffer
or integer. However, acpi_processor_evaluate_lpi() regeards it as success
and treat it as an effective LPI state. This is unreasonable and needs to
return failure to prevent other problems from occurring.

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 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 5acf12a0441f..681587f2614b 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -958,7 +958,9 @@ static int acpi_processor_evaluate_lpi(acpi_handle handle,
 			lpi_state->entry_method = ACPI_CSTATE_INTEGER;
 			lpi_state->address = obj->integer.value;
 		} else {
-			continue;
+			pr_err("Entry method in LPI sub-package must be buffer or integer.\n");
+			ret = -EINVAL;
+			goto end;
 		}
 
 		/* elements[7,8] skipped for now i.e. Residency/Usage counter*/
-- 
2.33.0
Re: [PATCH v1 2/9] ACPI: processor: idle: Return failure if entry method is not buffer or integer type
Posted by Rafael J. Wysocki 3 months, 3 weeks ago
On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
>
> According to ACPI spec, entry method in LPI sub-package must be buffer
> or integer. However, acpi_processor_evaluate_lpi() regeards it as success
> and treat it as an effective LPI state.

Is that the case?  AFAICS, it just gets to the next state in this case
and what's wrong with that?

> This is unreasonable and needs to
> return failure to prevent other problems from occurring.
>
> 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 | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 5acf12a0441f..681587f2614b 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -958,7 +958,9 @@ static int acpi_processor_evaluate_lpi(acpi_handle handle,
>                         lpi_state->entry_method = ACPI_CSTATE_INTEGER;
>                         lpi_state->address = obj->integer.value;
>                 } else {
> -                       continue;
> +                       pr_err("Entry method in LPI sub-package must be buffer or integer.\n");
> +                       ret = -EINVAL;
> +                       goto end;
>                 }
>
>                 /* elements[7,8] skipped for now i.e. Residency/Usage counter*/
> --
Re: [PATCH v1 2/9] ACPI: processor: idle: Return failure if entry method is not buffer or integer type
Posted by lihuisong (C) 3 months, 2 weeks ago
在 2025/10/22 3:34, Rafael J. Wysocki 写道:
> On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
>> According to ACPI spec, entry method in LPI sub-package must be buffer
>> or integer. However, acpi_processor_evaluate_lpi() regeards it as success
>> and treat it as an effective LPI state.
> Is that the case?  AFAICS, it just gets to the next state in this case
> and what's wrong with that?
The flatten_lpi_states() would consider the state with illegal entry 
method sub-package as a valid one
if the flag of this state is enabled(ACPI_LPI_STATE_FLAGS_ENABLED is set).
And then cpuidle governor would use it because the caller of 
acpi_processor_ffh_lpi_probe() also don't see the return value.
>
>> This is unreasonable and needs to
>> return failure to prevent other problems from occurring.
>>
>> 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 | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>> index 5acf12a0441f..681587f2614b 100644
>> --- a/drivers/acpi/processor_idle.c
>> +++ b/drivers/acpi/processor_idle.c
>> @@ -958,7 +958,9 @@ static int acpi_processor_evaluate_lpi(acpi_handle handle,
>>                          lpi_state->entry_method = ACPI_CSTATE_INTEGER;
>>                          lpi_state->address = obj->integer.value;
>>                  } else {
>> -                       continue;
>> +                       pr_err("Entry method in LPI sub-package must be buffer or integer.\n");
>> +                       ret = -EINVAL;
>> +                       goto end;
>>                  }
>>
>>                  /* elements[7,8] skipped for now i.e. Residency/Usage counter*/
>> --
Re: [PATCH v1 2/9] ACPI: processor: idle: Return failure if entry method is not buffer or integer type
Posted by Rafael J. Wysocki 3 months, 2 weeks ago
On Thu, Oct 23, 2025 at 11:25 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>
>
> 在 2025/10/22 3:34, Rafael J. Wysocki 写道:
> > On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
> >> According to ACPI spec, entry method in LPI sub-package must be buffer
> >> or integer. However, acpi_processor_evaluate_lpi() regeards it as success
> >> and treat it as an effective LPI state.
> > Is that the case?  AFAICS, it just gets to the next state in this case
> > and what's wrong with that?
> The flatten_lpi_states() would consider the state with illegal entry
> method sub-package as a valid one
> if the flag of this state is enabled(ACPI_LPI_STATE_FLAGS_ENABLED is set).
> And then cpuidle governor would use it because the caller of
> acpi_processor_ffh_lpi_probe() also don't see the return value.

So the problem appears to be that lpi_state increments in every step
of the loop, but it should only increment if the given state is valid.
Re: [PATCH v1 2/9] ACPI: processor: idle: Return failure if entry method is not buffer or integer type
Posted by lihuisong (C) 3 months, 2 weeks ago
在 2025/10/23 18:07, Rafael J. Wysocki 写道:
> On Thu, Oct 23, 2025 at 11:25 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>>
>> 在 2025/10/22 3:34, Rafael J. Wysocki 写道:
>>> On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
>>>> According to ACPI spec, entry method in LPI sub-package must be buffer
>>>> or integer. However, acpi_processor_evaluate_lpi() regeards it as success
>>>> and treat it as an effective LPI state.
>>> Is that the case?  AFAICS, it just gets to the next state in this case
>>> and what's wrong with that?
>> The flatten_lpi_states() would consider the state with illegal entry
>> method sub-package as a valid one
>> if the flag of this state is enabled(ACPI_LPI_STATE_FLAGS_ENABLED is set).
>> And then cpuidle governor would use it because the caller of
>> acpi_processor_ffh_lpi_probe() also don't see the return value.
> So the problem appears to be that lpi_state increments in every step
> of the loop, but it should only increment if the given state is valid.
Yes,
So set the flag of the state with illegal entry method sub-package to 
zero so that this invalid LPI state will be skiped in 
flatten_lpi_states(), ok?
Re: [PATCH v1 2/9] ACPI: processor: idle: Return failure if entry method is not buffer or integer type
Posted by Rafael J. Wysocki 3 months, 2 weeks ago
On Fri, Oct 24, 2025 at 11:25 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>
>
> 在 2025/10/23 18:07, Rafael J. Wysocki 写道:
> > On Thu, Oct 23, 2025 at 11:25 AM lihuisong (C) <lihuisong@huawei.com> wrote:
> >>
> >> 在 2025/10/22 3:34, Rafael J. Wysocki 写道:
> >>> On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
> >>>> According to ACPI spec, entry method in LPI sub-package must be buffer
> >>>> or integer. However, acpi_processor_evaluate_lpi() regeards it as success
> >>>> and treat it as an effective LPI state.
> >>> Is that the case?  AFAICS, it just gets to the next state in this case
> >>> and what's wrong with that?
> >> The flatten_lpi_states() would consider the state with illegal entry
> >> method sub-package as a valid one
> >> if the flag of this state is enabled(ACPI_LPI_STATE_FLAGS_ENABLED is set).
> >> And then cpuidle governor would use it because the caller of
> >> acpi_processor_ffh_lpi_probe() also don't see the return value.
> > So the problem appears to be that lpi_state increments in every step
> > of the loop, but it should only increment if the given state is valid.
> Yes,
> So set the flag of the state with illegal entry method sub-package to
> zero so that this invalid LPI state will be skiped in
> flatten_lpi_states(), ok?

Sounds reasonable.