From ecad4502d090a8630f50a88cbe072b92f3a3229e Mon Sep 17 00:00:00 2001 From: "Gautham R. Shenoy" Date: Wed, 15 Mar 2017 13:45:53 +0530 Subject: powernv-cpuidle: Validate DT property array size The various properties associated with powernv idle states such as names, flags, residency-ns, latencies-ns, psscr, psscr-mask are exposed in the device-tree as property arrays such the pointwise entries in each of these arrays correspond to the properties of the same idle state. This patch validates that the lengths of the property arrays are the same. If there is a mismatch, the patch will ensure that we bail out and not expose the platform idle states via cpuidle. Signed-off-by: Gautham R. Shenoy Reviewed-by: Shilpasri G Bhat Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle-powernv.c | 64 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 3 deletions(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c index 370593006f5f..a06df51a36c0 100644 --- a/drivers/cpuidle/cpuidle-powernv.c +++ b/drivers/cpuidle/cpuidle-powernv.c @@ -197,11 +197,25 @@ static inline void add_powernv_state(int index, const char *name, stop_psscr_table[index].mask = psscr_mask; } +/* + * Returns 0 if prop1_len == prop2_len. Else returns -1 + */ +static inline int validate_dt_prop_sizes(const char *prop1, int prop1_len, + const char *prop2, int prop2_len) +{ + if (prop1_len == prop2_len) + return 0; + + pr_warn("cpuidle-powernv: array sizes don't match for %s and %s\n", + prop1, prop2); + return -1; +} + static int powernv_add_idle_states(void) { struct device_node *power_mgt; int nr_idle_states = 1; /* Snooze */ - int dt_idle_states; + int dt_idle_states, count; u32 latency_ns[CPUIDLE_STATE_MAX]; u32 residency_ns[CPUIDLE_STATE_MAX]; u32 flags[CPUIDLE_STATE_MAX]; @@ -226,6 +240,21 @@ static int powernv_add_idle_states(void) goto out; } + count = of_property_count_u32_elems(power_mgt, + "ibm,cpu-idle-state-latencies-ns"); + + if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags", dt_idle_states, + "ibm,cpu-idle-state-latencies-ns", + count) != 0) + goto out; + + count = of_property_count_strings(power_mgt, + "ibm,cpu-idle-state-names"); + if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags", dt_idle_states, + "ibm,cpu-idle-state-names", + count) != 0) + goto out; + /* * Since snooze is used as first idle state, max idle states allowed is * CPUIDLE_STATE_MAX -1 @@ -260,6 +289,22 @@ static int powernv_add_idle_states(void) has_stop_states = (flags[0] & (OPAL_PM_STOP_INST_FAST | OPAL_PM_STOP_INST_DEEP)); if (has_stop_states) { + count = of_property_count_u64_elems(power_mgt, + "ibm,cpu-idle-state-psscr"); + if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags", + dt_idle_states, + "ibm,cpu-idle-state-psscr", + count) != 0) + goto out; + + count = of_property_count_u64_elems(power_mgt, + "ibm,cpu-idle-state-psscr-mask"); + if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags", + dt_idle_states, + "ibm,cpu-idle-state-psscr-mask", + count) != 0) + goto out; + if (of_property_read_u64_array(power_mgt, "ibm,cpu-idle-state-psscr", psscr_val, dt_idle_states)) { pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr in DT\n"); @@ -274,8 +319,21 @@ static int powernv_add_idle_states(void) } } - rc = of_property_read_u32_array(power_mgt, - "ibm,cpu-idle-state-residency-ns", residency_ns, dt_idle_states); + count = of_property_count_u32_elems(power_mgt, + "ibm,cpu-idle-state-residency-ns"); + + if (count < 0) { + rc = count; + } else if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags", + dt_idle_states, + "ibm,cpu-idle-state-residency-ns", + count) != 0) { + goto out; + } else { + rc = of_property_read_u32_array(power_mgt, + "ibm,cpu-idle-state-residency-ns", + residency_ns, dt_idle_states); + } for (i = 0; i < dt_idle_states; i++) { unsigned int exit_latency, target_residency; -- cgit From 02018b3929a23acdd452b986e7c8aeca4529d492 Mon Sep 17 00:00:00 2001 From: Marcin Nowakowski Date: Wed, 19 Apr 2017 13:20:54 +0200 Subject: cpuidle: cpuidle-cps: remove unused variable 'core' in cps_cpuidle_init has never been used and is unnecessary, so remove the dead code. Signed-off-by: Marcin Nowakowski Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle-cps.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/cpuidle-cps.c b/drivers/cpuidle/cpuidle-cps.c index 926ba9871c62..12b9145913de 100644 --- a/drivers/cpuidle/cpuidle-cps.c +++ b/drivers/cpuidle/cpuidle-cps.c @@ -118,7 +118,7 @@ static void __init cps_cpuidle_unregister(void) static int __init cps_cpuidle_init(void) { - int err, cpu, core, i; + int err, cpu, i; struct cpuidle_device *device; /* Detect supported states */ @@ -160,7 +160,6 @@ static int __init cps_cpuidle_init(void) } for_each_possible_cpu(cpu) { - core = cpu_data[cpu].core; device = &per_cpu(cpuidle_dev, cpu); device->cpu = cpu; #ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED -- cgit From 79b578111febef642143669254b243ffbcf64ea9 Mon Sep 17 00:00:00 2001 From: Anton Blanchard Date: Tue, 4 Apr 2017 07:54:12 +1000 Subject: cpuidle: powernv: Don't bounce between low and very low thread priority The core of snooze_loop() continually bounces between low and very low thread priority. Changing thread priorities is an expensive operation that can negatively impact other threads on a core. All CPUs that can run PowerNV support very low priority, so we can avoid the change completely. Signed-off-by: Anton Blanchard Reviewed-by: Vaidyanathan Srinivasan Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle-powernv.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c index a06df51a36c0..0ddf1a5bb0a9 100644 --- a/drivers/cpuidle/cpuidle-powernv.c +++ b/drivers/cpuidle/cpuidle-powernv.c @@ -57,7 +57,6 @@ static int snooze_loop(struct cpuidle_device *dev, snooze_exit_time = get_tb() + snooze_timeout; ppc64_runlatch_off(); while (!need_resched()) { - HMT_low(); HMT_very_low(); if (snooze_timeout_en && get_tb() > snooze_exit_time) break; -- cgit From 26eb48a9faf241abd60aa546e6beb896011667c1 Mon Sep 17 00:00:00 2001 From: Anton Blanchard Date: Tue, 4 Apr 2017 07:54:13 +1000 Subject: cpuidle: powernv: Don't continually set thread priority in snooze_loop() The powerpc64 kernel exception handlers have preserved thread priorities for a long time now, so there is no need to continually set it. Just set it once on entry and once exit. Signed-off-by: Anton Blanchard Reviewed-by: Vaidyanathan Srinivasan Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle-powernv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c index 0ddf1a5bb0a9..f8901671fff4 100644 --- a/drivers/cpuidle/cpuidle-powernv.c +++ b/drivers/cpuidle/cpuidle-powernv.c @@ -56,8 +56,8 @@ static int snooze_loop(struct cpuidle_device *dev, snooze_exit_time = get_tb() + snooze_timeout; ppc64_runlatch_off(); + HMT_very_low(); while (!need_resched()) { - HMT_very_low(); if (snooze_timeout_en && get_tb() > snooze_exit_time) break; } -- cgit From 0baa91cb73e296242edad89cfe3f60c59ab8a95a Mon Sep 17 00:00:00 2001 From: Anton Blanchard Date: Tue, 4 Apr 2017 07:54:14 +1000 Subject: cpuidle: powernv: Avoid a branch in the core snooze_loop() loop When in the snooze_loop() we want to take up the least amount of resources. On my version of gcc (6.3), we end up with an extra branch because it predicts snooze_timeout_en to be false, whereas it is almost always true. Use likely() to avoid the branch and be a little nicer to the other non idle threads on the core. Signed-off-by: Anton Blanchard Reviewed-by: Vaidyanathan Srinivasan Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle-powernv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c index f8901671fff4..5bb4bb303fba 100644 --- a/drivers/cpuidle/cpuidle-powernv.c +++ b/drivers/cpuidle/cpuidle-powernv.c @@ -58,7 +58,7 @@ static int snooze_loop(struct cpuidle_device *dev, ppc64_runlatch_off(); HMT_very_low(); while (!need_resched()) { - if (snooze_timeout_en && get_tb() > snooze_exit_time) + if (likely(snooze_timeout_en) && get_tb() > snooze_exit_time) break; } -- cgit From 41dc750ea67f317c0deedde713d1728425524ef2 Mon Sep 17 00:00:00 2001 From: "Li, Fei" Date: Thu, 27 Apr 2017 01:47:25 +0000 Subject: cpuidle: check dev before usage in cpuidle_use_deepest_state() In case of there is no cpuidle devices registered, dev will be null, and panic will be triggered like below; In this patch, add checking of dev before usage, like that done in cpuidle_idle_call. Panic without fix: [ 184.961328] BUG: unable to handle kernel NULL pointer dereference at (null) [ 184.961328] IP: cpuidle_use_deepest_state+0x30/0x60 ... [ 184.961328] play_idle+0x8d/0x210 [ 184.961328] ? __schedule+0x359/0x8e0 [ 184.961328] ? _raw_spin_unlock_irqrestore+0x28/0x50 [ 184.961328] ? kthread_queue_delayed_work+0x41/0x80 [ 184.961328] clamp_idle_injection_func+0x64/0x1e0 Fixes: bb8313b603eb8 (cpuidle: Allow enforcing deepest idle state selection) Signed-off-by: Li, Fei Tested-by: Shi, Feng Reviewed-by: Andy Shevchenko Cc: 4.10+ # 4.10+ Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index 548b90be7685..2706be7ed334 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -111,7 +111,8 @@ void cpuidle_use_deepest_state(bool enable) preempt_disable(); dev = cpuidle_get_device(); - dev->use_deepest_state = enable; + if (dev) + dev->use_deepest_state = enable; preempt_enable(); } -- cgit From b2cdd8e1b54849477a32d820acc2e87828a38f3d Mon Sep 17 00:00:00 2001 From: Christophe Jaillet Date: Sun, 11 Jun 2017 14:28:54 +0200 Subject: cpuidle: dt: Add missing 'of_node_put()' 'of_node_put()' should be called on pointer returned by 'of_parse_phandle()' when done. In this function this is done in all path except this 'continue', so add it. Fixes: 97735da074fd (drivers: cpuidle: Add status property to ARM idle states) Signed-off-by: Christophe Jaillet Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/dt_idle_states.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/dt_idle_states.c b/drivers/cpuidle/dt_idle_states.c index ffca4fc0061d..ae8eb0359889 100644 --- a/drivers/cpuidle/dt_idle_states.c +++ b/drivers/cpuidle/dt_idle_states.c @@ -180,8 +180,10 @@ int dt_init_idle_driver(struct cpuidle_driver *drv, if (!state_node) break; - if (!of_device_is_available(state_node)) + if (!of_device_is_available(state_node)) { + of_node_put(state_node); continue; + } if (!idle_state_valid(state_node, i, cpumask)) { pr_warn("%s idle state not valid, bailing out\n", -- cgit