[PATCH] ACPI: arm64: cpuidle: Tolerate platforms with no deep PSCI idle states

[PATCH] ACPI: arm64: cpuidle: Tolerate platforms with no deep PSCI idle states

[Breno Leitao]

Commit cac173bea57d ("ACPI: processor: idle: Rework the handling of
acpi_processor_ffh_lpi_probe()") moved the acpi_processor_ffh_lpi_probe()
call from acpi_processor_setup_cpuidle_dev(), where its return value was
ignored, to acpi_processor_get_power_info(), where it is now treated as
a hard failure. As a result, platforms where psci_acpi_cpu_init_idle()
returned -ENODEV stopped registering any cpuidle states, forcing CPUs to
busy-poll when idle.

On NVIDIA Grace (aarch64) systems with PSCIv1.1, pr->power.count is 1
(only WFI, no deep PSCI states beyond it), so the previous
"count = pr->power.count - 1; if (count <= 0) return -ENODEV;" check
returned -ENODEV for all 72 CPUs and disabled cpuidle entirely.

The lpi_states count is already validated in acpi_processor_get_lpi_info(),
so the check here is redundant. Simplify the loop to iterate over
lpi_states[1..power.count). When only WFI is present, the loop body
simply does not execute and the function returns 0, which is the correct
outcome: there is nothing to validate for FFH and no error to report.

Suggested-by: Huisong Li <lihuisong@huawei.com>
Cc: stable@vger.kernel.org
Fixes: cac173bea57d ("ACPI: processor: idle: Rework the handling of acpi_processor_ffh_lpi_probe()")
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/acpi/arm64/cpuidle.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/arm64/cpuidle.c b/drivers/acpi/arm64/cpuidle.c
index 801f9c4501425..c68a5db8ebba8 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;
-
-	for (i = 0; i < count; i++) {
+	for (i = 1; i < pr->power.count; i++) {
 		u32 state;
 
-		lpi = &pr->power.lpi_states[i + 1];
+		lpi = &pr->power.lpi_states[i];
 		/*
 		 * Only bits[31:0] represent a PSCI power_state while
 		 * bits[63:32] must be 0x0 as per ARM ACPI FFH Specification

---
base-commit: 1c7cc4904160c6fc6377564140062d68a3dc93a0
change-id: 20260413-ffh-93f68b2f46a3

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH] ACPI: arm64: cpuidle: Tolerate platforms with no deep PSCI idle states

[Sudeep Holla]

On Mon, Apr 20, 2026 at 02:27:13AM -0700, Breno Leitao wrote:
> Commit cac173bea57d ("ACPI: processor: idle: Rework the handling of
> acpi_processor_ffh_lpi_probe()") moved the acpi_processor_ffh_lpi_probe()
> call from acpi_processor_setup_cpuidle_dev(), where its return value was
> ignored, to acpi_processor_get_power_info(), where it is now treated as
> a hard failure. As a result, platforms where psci_acpi_cpu_init_idle()
> returned -ENODEV stopped registering any cpuidle states, forcing CPUs to
> busy-poll when idle.
> 
> On NVIDIA Grace (aarch64) systems with PSCIv1.1, pr->power.count is 1
> (only WFI, no deep PSCI states beyond it), so the previous
> "count = pr->power.count - 1; if (count <= 0) return -ENODEV;" check
> returned -ENODEV for all 72 CPUs and disabled cpuidle entirely.
> 
> The lpi_states count is already validated in acpi_processor_get_lpi_info(),
> so the check here is redundant. Simplify the loop to iterate over
> lpi_states[1..power.count). When only WFI is present, the loop body
> simply does not execute and the function returns 0, which is the correct
> outcome: there is nothing to validate for FFH and no error to report.
> 
> Suggested-by: Huisong Li <lihuisong@huawei.com>
> Cc: stable@vger.kernel.org
> Fixes: cac173bea57d ("ACPI: processor: idle: Rework the handling of acpi_processor_ffh_lpi_probe()")
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  drivers/acpi/arm64/cpuidle.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/acpi/arm64/cpuidle.c b/drivers/acpi/arm64/cpuidle.c
> index 801f9c4501425..c68a5db8ebba8 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;
> -

Does it make sense to retain this check like
  if (pr->power.count < 1)
  	return -EINVAL;

Though I see the assignment to pr->power.count in drivers/acpi/processor_idle.c
is through unsigned int. So I am fine even without the above check.

Reviewed-by: Sudeep Holla <sudeep.holla@kernel.org>

-- 
Regards,
Sudeep

Re: [PATCH] ACPI: arm64: cpuidle: Tolerate platforms with no deep PSCI idle states

[Breno Leitao]

On Mon, Apr 20, 2026 at 04:12:38PM +0100, Sudeep Holla wrote:
> On Mon, Apr 20, 2026 at 02:27:13AM -0700, Breno Leitao wrote:
> > -	count = pr->power.count - 1;
> > -	if (count <= 0)
> > -		return -ENODEV;
> > -
>
> Does it make sense to retain this check like
>   if (pr->power.count < 1)
>   	return -EINVAL;
>
> Though I see the assignment to pr->power.count in drivers/acpi/processor_idle.c
> is through unsigned int. So I am fine even without the above check.

I don't think the check is necessary. When count is 0 or 1, the loop
for (i = 1; i < pr->power.count; i++) body won't execute, and the
function will return 0.

This seems like the correct behavior — if there are no FFH PSCI states
to validate, there's nothing that should fail.

Additionally, returning -ENODEV would trigger the "Invalid FFH LPI data"
error message, which would be misleading since the LPI data isn't
invalid, it's just not present.

That said, please take this with a grain of salt since I'm not deeply
familiar with _LPI states and their expected behavior.

Thanks for the review,
--breno

Re: [PATCH] ACPI: arm64: cpuidle: Tolerate platforms with no deep PSCI idle states

[Sudeep Holla]

On Tue, Apr 21, 2026 at 02:51:42AM -0700, Breno Leitao wrote:
> On Mon, Apr 20, 2026 at 04:12:38PM +0100, Sudeep Holla wrote:
> > On Mon, Apr 20, 2026 at 02:27:13AM -0700, Breno Leitao wrote:
> > > -	count = pr->power.count - 1;
> > > -	if (count <= 0)
> > > -		return -ENODEV;
> > > -
> >
> > Does it make sense to retain this check like
> >   if (pr->power.count < 1)
> >   	return -EINVAL;
> >
> > Though I see the assignment to pr->power.count in drivers/acpi/processor_idle.c
> > is through unsigned int. So I am fine even without the above check.
> 
> I don't think the check is necessary. When count is 0 or 1, the loop
> for (i = 1; i < pr->power.count; i++) body won't execute, and the
> function will return 0.
>

Yes but the point is to handle invalid pr->power.count(0 or less) which
is not possible here though it is signed it because it is assigned from
an unsigned int during initialisation.

> This seems like the correct behavior — if there are no FFH PSCI states
> to validate, there's nothing that should fail.
> 

Agreed, but I was thinking of error in parsing _LPI being propogated here
but again that's not happening here.

> Additionally, returning -ENODEV would trigger the "Invalid FFH LPI data"
> error message, which would be misleading since the LPI data isn't
> invalid, it's just not present.
> 

The point was to throw that error if _LPI parsing fails.

> That said, please take this with a grain of salt since I'm not deeply
> familiar with _LPI states and their expected behavior.
> 

No worries, I agree the check I asked for is not needed.

-- 
Regards,
Sudeep