Re: [PATCH RFC] ACPI: processor: idle: Do not propagate acpi_processor_ffh_lpi_probe() -ENODEV

["lihuisong (C)" <lihuisong@huawei.com>]

On 4/14/2026 8:25 PM, Sudeep Holla wrote:
> On Tue, Apr 14, 2026 at 07:31:29PM +0800, lihuisong (C) wrote:
>> On 4/14/2026 6:21 PM, Breno Leitao wrote:
>>> Hello Huisong,
>>>
>>> On Tue, Apr 14, 2026 at 05:43:51PM +0800, lihuisong (C) wrote:
>>>> But it is a real issue. Thanks for your report.
>>>> I think the best way to fix your issue is that remove this verification in
>>>> psci_acpi_cpu_init_idle().
>>>> Because it is legal for platform to report one LPI state.
>>>> This function just needs to verify the LPI states which are FFH.
>>> Thank you for the prompt feedback.
>>>
>>> Would this approach work?
>>>
>>> commit 6c9d52840a4f778cc989838ba76ee51416e85de3
>>> Author: Breno Leitao <leitao@debian.org>
>>> Date:   Tue Apr 14 03:16:08 2026 -0700
>>>
>>>       ACPI: processor: idle: Allow platforms with only one LPI state
>>>       psci_acpi_cpu_init_idle() rejects platforms where power.count - 1 <= 0
>>>       by returning -ENODEV. However, having a single LPI state (WFI) is a
>>>       valid configuration. The function's purpose is to verify FFH idle states,
>>>       and when count is zero, there are simply no FFH states to validate —
>>>       this is not an error.
>>>       On NVIDIA Grace (aarch64) systems with PSCIv1.1, power.count is 1 for
>>>       all 72 CPUs, so the probe fails with -ENODEV. After commit cac173bea57d
>>>       ("ACPI: processor: idle: Rework the handling of
>>>       acpi_processor_ffh_lpi_probe()"), this failure propagates up and prevents
>>>       cpuidle registration entirely.
>>>       Change the check from (count <= 0) to (count < 0) so that platforms
>>>       with only WFI are accepted. The for loop naturally handles count == 0
>>>       by not iterating.
>>>       Fixes: cac173bea57d ("ACPI: processor: idle: Rework the handling of acpi_processor_ffh_lpi_probe()")
>>>       Signed-off-by: Breno Leitao <leitao@debian.org>
>>>
>>> diff --git a/drivers/acpi/arm64/cpuidle.c b/drivers/acpi/arm64/cpuidle.c
>>> index 801f9c4501425..7791b751042ce 100644
>>> --- a/drivers/acpi/arm64/cpuidle.c
>>> +++ b/drivers/acpi/arm64/cpuidle.c
>>> @@ -31,7 +31,7 @@ static int psci_acpi_cpu_init_idle(unsigned int cpu)
>>>    		return -EOPNOTSUPP;
>>>    	count = pr->power.count - 1;
>>> -	if (count <= 0)
>>> +	if (count < 0)
>>>    		return -ENODEV;
>>>    	for (i = 0; i < count; i++) {
>> This count already verified in acpi_processor_get_lpi_info.
>>
>> I suggest modifing it as below:
>>
>> -->
>>
>> git diff
>> diff --git a/drivers/acpi/arm64/cpuidle.c b/drivers/acpi/arm64/cpuidle.c
>> index 801f9c450142..c68a5db8ebba 100644
>> --- a/drivers/acpi/arm64/cpuidle.c
>> +++ b/drivers/acpi/arm64/cpuidle.c
>> @@ -16,7 +16,7 @@
>>
>>   static int psci_acpi_cpu_init_idle(unsigned int cpu)
>>   {
>> -       int i, count;
>> +       int i;
>>          struct acpi_lpi_state *lpi;
>>          struct acpi_processor *pr = per_cpu(processors, cpu);
>>
>> @@ -30,14 +30,10 @@ static int psci_acpi_cpu_init_idle(unsigned int cpu)
>>          if (!psci_ops.cpu_suspend)
>>                  return -EOPNOTSUPP;
>>
>> -       count = pr->power.count - 1;
>> -       if (count <= 0)
>> -               return -ENODEV;
>> -
> It was intentionally designed this way, as there is little value in defining
> only WFI in the _LPI tables. In the absence of a cpuidle driver/LPI entry,
> arch_cpu_idle() is invoked, which is sufficient and avoids unnecessary
> complexity, only to ultimately execute wfi() anyway.
Yeah, it's correct. The code flow will be more simple and high-efficiency.
This looks good to me.


But cpuidle sysfs under per CPU is created when firmware just reports 
WFI state before
my commit cac173bea57d ("ACPI: processor: idle: Rework the handling of 
acpi_processor_ffh_lpi_probe()").
However, these platforms will no longer be created now and some 
statistics for state0 are also missing.
This change in behavor is visiable to user space.I'm not sure if it is 
acceptable.
What do you think, Rafael?
> So while I understand that the kernel did not report an error previously, that
> does not mean the _LPI table is merely moot on this platform when it contains
> only a WFI state.
>
>