[PATCH] cpuidle: ladder: Fix state index when only one idle state is registered

[PATCH] cpuidle: ladder: Fix state index when only one idle state is registered

[Aboorva Devarajan]

On certain platforms (PowerNV systems without a power-mgt DT node),
cpuidle may register only a single idle state. In cases where that
single state is a polling state (state 0), the ladder governor may
incorrectly treat state 1 as the first usable state and pass an
out-of-bounds index. This can lead to a NULL enter callback being
invoked, ultimately resulting in a system crash.

[   13.342636] cpuidle-powernv : Only Snooze is available
[   13.351854] Faulting instruction address: 0x00000000
[   13.376489] NIP [0000000000000000] 0x0
[   13.378351] LR  [c000000001e01974] cpuidle_enter_state+0x2c4/0x668

Fix this by determining the first non-polling state index based on
the number of registered states, and by returning state 0 when only
one state is registered.

Fixes: dc2251bf98c6 ("cpuidle: Eliminate the CPUIDLE_DRIVER_STATE_START symbol")
Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
---
 drivers/cpuidle/governors/ladder.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
index 6617eb494a11..294a688ed0bb 100644
--- a/drivers/cpuidle/governors/ladder.c
+++ b/drivers/cpuidle/governors/ladder.c
@@ -42,6 +42,21 @@ struct ladder_device {
 
 static DEFINE_PER_CPU(struct ladder_device, ladder_devices);
 
+/**
+ * ladder_get_first_idx - get the first non-polling state index
+ * @drv: cpuidle driver
+ *
+ * Returns the index of the first non-polling state, or 0 if state 0 is not
+ * polling or if there's only one state available.
+ */
+static inline int ladder_get_first_idx(struct cpuidle_driver *drv)
+{
+	if (drv->state_count > 1 &&
+	    drv->states[0].flags & CPUIDLE_FLAG_POLLING)
+		return 1;
+	return 0;
+}
+
 /**
  * ladder_do_selection - prepares private data for a state change
  * @dev: the CPU
@@ -70,16 +85,17 @@ static int ladder_select_state(struct cpuidle_driver *drv,
 	struct ladder_device *ldev = this_cpu_ptr(&ladder_devices);
 	struct ladder_device_state *last_state;
 	int last_idx = dev->last_state_idx;
-	int first_idx = drv->states[0].flags & CPUIDLE_FLAG_POLLING ? 1 : 0;
+	int first_idx;
 	s64 latency_req = cpuidle_governor_latency_req(dev->cpu);
 	s64 last_residency;
 
-	/* Special case when user has set very strict latency requirement */
-	if (unlikely(latency_req == 0)) {
+	/* Special case when there's only one state or strict latency requirement */
+	if (unlikely(drv->state_count <= 1 || latency_req == 0)) {
 		ladder_do_selection(dev, ldev, last_idx, 0);
 		return 0;
 	}
 
+	first_idx = ladder_get_first_idx(drv);
 	last_state = &ldev->states[last_idx];
 
 	last_residency = dev->last_residency_ns - drv->states[last_idx].exit_latency_ns;
@@ -134,7 +150,7 @@ static int ladder_enable_device(struct cpuidle_driver *drv,
 				struct cpuidle_device *dev)
 {
 	int i;
-	int first_idx = drv->states[0].flags & CPUIDLE_FLAG_POLLING ? 1 : 0;
+	int first_idx = ladder_get_first_idx(drv);
 	struct ladder_device *ldev = &per_cpu(ladder_devices, dev->cpu);
 	struct ladder_device_state *lstate;
 	struct cpuidle_state *state;
-- 
2.52.0

Re: [PATCH] cpuidle: ladder: Fix state index when only one idle state is registered

[Christian Loehle]

On 2/11/26 05:35, Aboorva Devarajan wrote:
> On certain platforms (PowerNV systems without a power-mgt DT node),
> cpuidle may register only a single idle state. In cases where that
> single state is a polling state (state 0), the ladder governor may
> incorrectly treat state 1 as the first usable state and pass an
> out-of-bounds index. This can lead to a NULL enter callback being
> invoked, ultimately resulting in a system crash.
> 
> [   13.342636] cpuidle-powernv : Only Snooze is available
> [   13.351854] Faulting instruction address: 0x00000000
> [   13.376489] NIP [0000000000000000] 0x0
> [   13.378351] LR  [c000000001e01974] cpuidle_enter_state+0x2c4/0x668
> 
> Fix this by determining the first non-polling state index based on
> the number of registered states, and by returning state 0 when only
> one state is registered.
> 
> Fixes: dc2251bf98c6 ("cpuidle: Eliminate the CPUIDLE_DRIVER_STATE_START symbol")
> Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>

Agreed that the current behavior is a bug, but is there really much value
in using a cpuidle governor with just a polling state?
It's dead code and trivial to bail out of in cpuidle, right?

> ---
>  drivers/cpuidle/governors/ladder.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
> index 6617eb494a11..294a688ed0bb 100644
> --- a/drivers/cpuidle/governors/ladder.c
> +++ b/drivers/cpuidle/governors/ladder.c
> @@ -42,6 +42,21 @@ struct ladder_device {
>  
>  static DEFINE_PER_CPU(struct ladder_device, ladder_devices);
>  
> +/**
> + * ladder_get_first_idx - get the first non-polling state index
> + * @drv: cpuidle driver
> + *
> + * Returns the index of the first non-polling state, or 0 if state 0 is not
> + * polling or if there's only one state available.
> + */
> +static inline int ladder_get_first_idx(struct cpuidle_driver *drv)
> +{
> +	if (drv->state_count > 1 &&
> +	    drv->states[0].flags & CPUIDLE_FLAG_POLLING)
> +		return 1;
> +	return 0;
> +}
> +
>  /**
>   * ladder_do_selection - prepares private data for a state change
>   * @dev: the CPU
> @@ -70,16 +85,17 @@ static int ladder_select_state(struct cpuidle_driver *drv,
>  	struct ladder_device *ldev = this_cpu_ptr(&ladder_devices);
>  	struct ladder_device_state *last_state;
>  	int last_idx = dev->last_state_idx;
> -	int first_idx = drv->states[0].flags & CPUIDLE_FLAG_POLLING ? 1 : 0;
> +	int first_idx;
>  	s64 latency_req = cpuidle_governor_latency_req(dev->cpu);
>  	s64 last_residency;
>  
> -	/* Special case when user has set very strict latency requirement */
> -	if (unlikely(latency_req == 0)) {
> +	/* Special case when there's only one state or strict latency requirement */
> +	if (unlikely(drv->state_count <= 1 || latency_req == 0)) {
>  		ladder_do_selection(dev, ldev, last_idx, 0);
>  		return 0;
>  	}
>  
> +	first_idx = ladder_get_first_idx(drv);
>  	last_state = &ldev->states[last_idx];
>  
>  	last_residency = dev->last_residency_ns - drv->states[last_idx].exit_latency_ns;
> @@ -134,7 +150,7 @@ static int ladder_enable_device(struct cpuidle_driver *drv,
>  				struct cpuidle_device *dev)
>  {
>  	int i;
> -	int first_idx = drv->states[0].flags & CPUIDLE_FLAG_POLLING ? 1 : 0;
> +	int first_idx = ladder_get_first_idx(drv);
>  	struct ladder_device *ldev = &per_cpu(ladder_devices, dev->cpu);
>  	struct ladder_device_state *lstate;
>  	struct cpuidle_state *state;

Re: [PATCH] cpuidle: ladder: Fix state index when only one idle state is registered

[Aboorva Devarajan]

On Wed, 2026-02-11 at 15:00 +0000, Christian Loehle wrote:
> On 2/11/26 05:35, Aboorva Devarajan wrote:
> > On certain platforms (PowerNV systems without a power-mgt DT node),
> > cpuidle may register only a single idle state. In cases where that
> > single state is a polling state (state 0), the ladder governor may
> > incorrectly treat state 1 as the first usable state and pass an
> > out-of-bounds index. This can lead to a NULL enter callback being
> > invoked, ultimately resulting in a system crash.
> > 
> > [   13.342636] cpuidle-powernv : Only Snooze is available
> > [   13.351854] Faulting instruction address: 0x00000000
> > [   13.376489] NIP [0000000000000000] 0x0
> > [   13.378351] LR  [c000000001e01974] cpuidle_enter_state+0x2c4/0x668
> > 
> > Fix this by determining the first non-polling state index based on
> > the number of registered states, and by returning state 0 when only
> > one state is registered.
> > 
> > Fixes: dc2251bf98c6 ("cpuidle: Eliminate the CPUIDLE_DRIVER_STATE_START symbol")
> > Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
> 
> Agreed that the current behavior is a bug, but is there really much value
> in using a cpuidle governor with just a polling state?
> It's dead code and trivial to bail out of in cpuidle, right?
> 

Hi Christian,

Thanks for the review.

Other governors (teo, menu) already handle this single-state scenario
correctly. Fixing ladder's first_idx calculation seemed like the most
targeted fix, however since ladder is not widely used this is likely
to go unnoticed, it only popped up during testing with a missing
power-mgt device tree node.

yes, adding a bail-out in the core cpuidle_select() is also trivial and
would benefit all governors uniformly. Setting stop_tick to false keeps
the tick running, which is correct for a single state configuration.

Please let me know if you'd prefer this approach instead.

---

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index c7876e9e024f..ea082419f7db 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -359,6 +359,16 @@ noinstr int cpuidle_enter_state(struct
cpuidle_device *dev,
 int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device
*dev,
                   bool *stop_tick)
 {
+       /*
+        * If there is only a single idle state (or none), there is
nothing
+        * meaningful for the governor to choose. Skip the governor and
+        * always use state 0 with the tick running.
+        */
+       if (unlikely(drv->state_count <= 1)) {
+               *stop_tick = false;
+               return 0;
+       }
+
        return cpuidle_curr_governor->select(drv, dev, stop_tick);
 }

---


Regards,
Aboorva


> > ---
> >  drivers/cpuidle/governors/ladder.c | 24 ++++++++++++++++++++----
> >  1 file changed, 20 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
> > index 6617eb494a11..294a688ed0bb 100644
> > --- a/drivers/cpuidle/governors/ladder.c
> > +++ b/drivers/cpuidle/governors/ladder.c
> > @@ -42,6 +42,21 @@ struct ladder_device {
> >  
> >  static DEFINE_PER_CPU(struct ladder_device, ladder_devices);
> >  
> > +/**
> > + * ladder_get_first_idx - get the first non-polling state index
> > + * @drv: cpuidle driver
> > + *
> > + * Returns the index of the first non-polling state, or 0 if state 0 is not
> > + * polling or if there's only one state available.
> > + */
> > +static inline int ladder_get_first_idx(struct cpuidle_driver *drv)
> > +{
> > +	if (drv->state_count > 1 &&
> > +	    drv->states[0].flags & CPUIDLE_FLAG_POLLING)
> > +		return 1;
> > +	return 0;
> > +}
> > +
> >  /**
> >   * ladder_do_selection - prepares private data for a state change
> >   * @dev: the CPU
> > @@ -70,16 +85,17 @@ static int ladder_select_state(struct cpuidle_driver *drv,
> >  	struct ladder_device *ldev = this_cpu_ptr(&ladder_devices);
> >  	struct ladder_device_state *last_state;
> >  	int last_idx = dev->last_state_idx;
> > -	int first_idx = drv->states[0].flags & CPUIDLE_FLAG_POLLING ? 1 : 0;
> > +	int first_idx;
> >  	s64 latency_req = cpuidle_governor_latency_req(dev->cpu);
> >  	s64 last_residency;
> >  
> > -	/* Special case when user has set very strict latency requirement */
> > -	if (unlikely(latency_req == 0)) {
> > +	/* Special case when there's only one state or strict latency requirement */
> > +	if (unlikely(drv->state_count <= 1 || latency_req == 0)) {
> >  		ladder_do_selection(dev, ldev, last_idx, 0);
> >  		return 0;
> >  	}
> >  
> > +	first_idx = ladder_get_first_idx(drv);
> >  	last_state = &ldev->states[last_idx];
> >  
> >  	last_residency = dev->last_residency_ns - drv->states[last_idx].exit_latency_ns;
> > @@ -134,7 +150,7 @@ static int ladder_enable_device(struct cpuidle_driver *drv,
> >  				struct cpuidle_device *dev)
> >  {
> >  	int i;
> > -	int first_idx = drv->states[0].flags & CPUIDLE_FLAG_POLLING ? 1 : 0;
> > +	int first_idx = ladder_get_first_idx(drv);
> >  	struct ladder_device *ldev = &per_cpu(ladder_devices, dev->cpu);
> >  	struct ladder_device_state *lstate;
> >  	struct cpuidle_state *state;

Re: [PATCH] cpuidle: ladder: Fix state index when only one idle state is registered

[Christian Loehle]

On 2/13/26 08:29, Aboorva Devarajan wrote:
> On Wed, 2026-02-11 at 15:00 +0000, Christian Loehle wrote:
>> On 2/11/26 05:35, Aboorva Devarajan wrote:
>>> On certain platforms (PowerNV systems without a power-mgt DT node),
>>> cpuidle may register only a single idle state. In cases where that
>>> single state is a polling state (state 0), the ladder governor may
>>> incorrectly treat state 1 as the first usable state and pass an
>>> out-of-bounds index. This can lead to a NULL enter callback being
>>> invoked, ultimately resulting in a system crash.
>>>
>>> [   13.342636] cpuidle-powernv : Only Snooze is available
>>> [   13.351854] Faulting instruction address: 0x00000000
>>> [   13.376489] NIP [0000000000000000] 0x0
>>> [   13.378351] LR  [c000000001e01974] cpuidle_enter_state+0x2c4/0x668
>>>
>>> Fix this by determining the first non-polling state index based on
>>> the number of registered states, and by returning state 0 when only
>>> one state is registered.
>>>
>>> Fixes: dc2251bf98c6 ("cpuidle: Eliminate the CPUIDLE_DRIVER_STATE_START symbol")
>>> Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
>>
>> Agreed that the current behavior is a bug, but is there really much value
>> in using a cpuidle governor with just a polling state?
>> It's dead code and trivial to bail out of in cpuidle, right?
>>
> 
> Hi Christian,
> 
> Thanks for the review.
> 
> Other governors (teo, menu) already handle this single-state scenario
> correctly. Fixing ladder's first_idx calculation seemed like the most
> targeted fix, however since ladder is not widely used this is likely
> to go unnoticed, it only popped up during testing with a missing
> power-mgt device tree node.
> 
> yes, adding a bail-out in the core cpuidle_select() is also trivial and
> would benefit all governors uniformly. Setting stop_tick to false keeps
> the tick running, which is correct for a single state configuration.
> 
> Please let me know if you'd prefer this approach instead.
> 
> ---
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index c7876e9e024f..ea082419f7db 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -359,6 +359,16 @@ noinstr int cpuidle_enter_state(struct
> cpuidle_device *dev,
>  int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device
> *dev,
>                    bool *stop_tick)
>  {
> +       /*
> +        * If there is only a single idle state (or none), there is
> nothing
> +        * meaningful for the governor to choose. Skip the governor and
> +        * always use state 0 with the tick running.
> +        */
> +       if (unlikely(drv->state_count <= 1)) {

I think the unlikely isn't helping here, this just let the branch predictor
handle this as it won't change anyway.

> +               *stop_tick = false;
> +               return 0;
> +       }
> +
>         return cpuidle_curr_governor->select(drv, dev, stop_tick);
>  }
> 

I prefer this, additionally of course:

-------8<-------

diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index 64d6f7a1c776..fdfa5d7e10a6 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -271,7 +271,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
                data->bucket = BUCKETS - 1;
        }
 
-       if (unlikely(drv->state_count <= 1 || latency_req == 0) ||
+       if (unlikely(latency_req == 0) ||
            ((data->next_timer_ns < drv->states[1].target_residency_ns ||
              latency_req < drv->states[1].exit_latency_ns) &&
             !dev->states_usage[0].disable)) {
diff --git a/drivers/cpuidle/governors/teo.c b/drivers/cpuidle/governors/teo.c
index 81ac5fd58a1c..9b5b8c617806 100644
--- a/drivers/cpuidle/governors/teo.c
+++ b/drivers/cpuidle/governors/teo.c
@@ -317,12 +317,6 @@ static int teo_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
         */
        cpu_data->sleep_length_ns = KTIME_MAX;
 
-       /* Check if there is any choice in the first place. */
-       if (drv->state_count < 2) {
-               idx = 0;
-               goto out_tick;
-       }
-
        if (!dev->states_usage[0].disable)
                idx = 0;
 

Re: [PATCH] cpuidle: ladder: Fix state index when only one idle state is registered

[Christian Loehle]

[-- Attachment #1: Type: text/plain, Size: 3245 bytes --]

On 2/13/26 13:44, Christian Loehle wrote:
> On 2/13/26 08:29, Aboorva Devarajan wrote:
>> On Wed, 2026-02-11 at 15:00 +0000, Christian Loehle wrote:
>>> On 2/11/26 05:35, Aboorva Devarajan wrote:
>>>> On certain platforms (PowerNV systems without a power-mgt DT node),
>>>> cpuidle may register only a single idle state. In cases where that
>>>> single state is a polling state (state 0), the ladder governor may
>>>> incorrectly treat state 1 as the first usable state and pass an
>>>> out-of-bounds index. This can lead to a NULL enter callback being
>>>> invoked, ultimately resulting in a system crash.
>>>>
>>>> [   13.342636] cpuidle-powernv : Only Snooze is available
>>>> [   13.351854] Faulting instruction address: 0x00000000
>>>> [   13.376489] NIP [0000000000000000] 0x0
>>>> [   13.378351] LR  [c000000001e01974] cpuidle_enter_state+0x2c4/0x668
>>>>
>>>> Fix this by determining the first non-polling state index based on
>>>> the number of registered states, and by returning state 0 when only
>>>> one state is registered.
>>>>
>>>> Fixes: dc2251bf98c6 ("cpuidle: Eliminate the CPUIDLE_DRIVER_STATE_START symbol")
>>>> Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
>>>
>>> Agreed that the current behavior is a bug, but is there really much value
>>> in using a cpuidle governor with just a polling state?
>>> It's dead code and trivial to bail out of in cpuidle, right?
>>>
>>
>> Hi Christian,
>>
>> Thanks for the review.
>>
>> Other governors (teo, menu) already handle this single-state scenario
>> correctly. Fixing ladder's first_idx calculation seemed like the most
>> targeted fix, however since ladder is not widely used this is likely
>> to go unnoticed, it only popped up during testing with a missing
>> power-mgt device tree node.
>>
>> yes, adding a bail-out in the core cpuidle_select() is also trivial and
>> would benefit all governors uniformly. Setting stop_tick to false keeps
>> the tick running, which is correct for a single state configuration.
>>
>> Please let me know if you'd prefer this approach instead.
>>
>> ---
>>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index c7876e9e024f..ea082419f7db 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -359,6 +359,16 @@ noinstr int cpuidle_enter_state(struct
>> cpuidle_device *dev,
>>  int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device
>> *dev,
>>                    bool *stop_tick)
>>  {
>> +       /*
>> +        * If there is only a single idle state (or none), there is
>> nothing
>> +        * meaningful for the governor to choose. Skip the governor and
>> +        * always use state 0 with the tick running.
>> +        */
>> +       if (unlikely(drv->state_count <= 1)) {
> 
> I think the unlikely isn't helping here, this just let the branch predictor
> handle this as it won't change anyway.
> 
>> +               *stop_tick = false;
>> +               return 0;
>> +       }
>> +
>>         return cpuidle_curr_governor->select(drv, dev, stop_tick);
>>  }
>>
> 
> I prefer this, additionally of course:

I've attached them as patches with a sign-off, feel free to pick them up as a series
or if you provide your signoff I can do that as well.

[-- Attachment #2: 0002-cpuidle-teo-Remove-single-state-handling.patch --]
[-- Type: text/x-patch, Size: 1073 bytes --]

From 56ba6cbec9aca645a51744261ecf0276072e001d Mon Sep 17 00:00:00 2001
From: Christian Loehle <christian.loehle@arm.com>
Date: Mon, 16 Feb 2026 11:02:55 +0000
Subject: [PATCH 2/2] cpuidle: teo: Remove single state handling

cpuidle systems where the governor has no choice because there's only
a single idle state are now handled by cpuidle core and bypass the
governor, so remove the related handling.

Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 drivers/cpuidle/governors/teo.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/cpuidle/governors/teo.c b/drivers/cpuidle/governors/teo.c
index 81ac5fd58a1c..9b5b8c617806 100644
--- a/drivers/cpuidle/governors/teo.c
+++ b/drivers/cpuidle/governors/teo.c
@@ -317,12 +317,6 @@ static int teo_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
 	 */
 	cpu_data->sleep_length_ns = KTIME_MAX;
 
-	/* Check if there is any choice in the first place. */
-	if (drv->state_count < 2) {
-		idx = 0;
-		goto out_tick;
-	}
-
 	if (!dev->states_usage[0].disable)
 		idx = 0;
 
-- 
2.34.1


[-- Attachment #3: 0001-cpuidle-menu-Remove-single-state-handling.patch --]
[-- Type: text/x-patch, Size: 1167 bytes --]

From 1046bac618c05262d139071e55f4248d3121310d Mon Sep 17 00:00:00 2001
From: Christian Loehle <christian.loehle@arm.com>
Date: Mon, 16 Feb 2026 11:01:02 +0000
Subject: [PATCH 1/2] cpuidle: menu: Remove single state handling

cpuidle systems where the governor has no choice because there's only
a single idle state are now handled by cpuidle core and bypass the
governor, so remove the related handling.

Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 drivers/cpuidle/governors/menu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index 64d6f7a1c776..fdfa5d7e10a6 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -271,7 +271,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
 		data->bucket = BUCKETS - 1;
 	}
 
-	if (unlikely(drv->state_count <= 1 || latency_req == 0) ||
+	if (unlikely(latency_req == 0) ||
 	    ((data->next_timer_ns < drv->states[1].target_residency_ns ||
 	      latency_req < drv->states[1].exit_latency_ns) &&
 	     !dev->states_usage[0].disable)) {
-- 
2.34.1

Re: [PATCH] cpuidle: ladder: Fix state index when only one idle state is registered

[Aboorva Devarajan]

On Mon, 2026-02-16 at 11:05 +0000, Christian Loehle wrote:
> On 2/13/26 13:44, Christian Loehle wrote:
> > On 2/13/26 08:29, Aboorva Devarajan wrote:
> > > On Wed, 2026-02-11 at 15:00 +0000, Christian Loehle wrote:
> > > > On 2/11/26 05:35, Aboorva Devarajan wrote:
> > > > > On certain platforms (PowerNV systems without a power-mgt DT node),
> > > > > cpuidle may register only a single idle state. In cases where that
> > > > > single state is a polling state (state 0), the ladder governor may
> > > > > incorrectly treat state 1 as the first usable state and pass an
> > > > > out-of-bounds index. This can lead to a NULL enter callback being
> > > > > invoked, ultimately resulting in a system crash.
> > > > > 
> > > > > [   13.342636] cpuidle-powernv : Only Snooze is available
> > > > > [   13.351854] Faulting instruction address: 0x00000000
> > > > > [   13.376489] NIP [0000000000000000] 0x0
> > > > > [   13.378351] LR  [c000000001e01974] cpuidle_enter_state+0x2c4/0x668
> > > > > 
> > > > > Fix this by determining the first non-polling state index based on
> > > > > the number of registered states, and by returning state 0 when only
> > > > > one state is registered.
> > > > > 
> > > > > Fixes: dc2251bf98c6 ("cpuidle: Eliminate the CPUIDLE_DRIVER_STATE_START symbol")
> > > > > Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
> > > > 
> > > > Agreed that the current behavior is a bug, but is there really much value
> > > > in using a cpuidle governor with just a polling state?
> > > > It's dead code and trivial to bail out of in cpuidle, right?
> > > > 
> > > 
> > > Hi Christian,
> > > 
> > > Thanks for the review.
> > > 
> > > Other governors (teo, menu) already handle this single-state scenario
> > > correctly. Fixing ladder's first_idx calculation seemed like the most
> > > targeted fix, however since ladder is not widely used this is likely
> > > to go unnoticed, it only popped up during testing with a missing
> > > power-mgt device tree node.
> > > 
> > > yes, adding a bail-out in the core cpuidle_select() is also trivial and
> > > would benefit all governors uniformly. Setting stop_tick to false keeps
> > > the tick running, which is correct for a single state configuration.
> > > 
> > > Please let me know if you'd prefer this approach instead.
> > > 
> > > ---
> > > 
> > > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> > > index c7876e9e024f..ea082419f7db 100644
> > > --- a/drivers/cpuidle/cpuidle.c
> > > +++ b/drivers/cpuidle/cpuidle.c
> > > @@ -359,6 +359,16 @@ noinstr int cpuidle_enter_state(struct
> > > cpuidle_device *dev,
> > >  int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device
> > > *dev,
> > >                    bool *stop_tick)
> > >  {
> > > +       /*
> > > +        * If there is only a single idle state (or none), there is
> > > nothing
> > > +        * meaningful for the governor to choose. Skip the governor and
> > > +        * always use state 0 with the tick running.
> > > +        */
> > > +       if (unlikely(drv->state_count <= 1)) {
> > 
> > I think the unlikely isn't helping here, this just let the branch predictor
> > handle this as it won't change anyway.
> > 
> > > +               *stop_tick = false;
> > > +               return 0;
> > > +       }
> > > +
> > >         return cpuidle_curr_governor->select(drv, dev, stop_tick);
> > >  }
> > > 
> > 
> > I prefer this, additionally of course:
> 
> I've attached them as patches with a sign-off, feel free to pick them up as a series
> or if you provide your signoff I can do that as well.

Hi Christian,

Thanks, I've included your patches and sent them as a series:
https://lore.kernel.org/all/20260216185005.1131593-1-aboorvad@linux.ibm.com/

Regards,
Aboorva