From f187851b9b4a76952b1158b86434563dd2031103 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Fri, 1 Sep 2017 14:29:56 +1000 Subject: [PATCH 1/8] cpuidle: fix broadcast control when broadcast can not be entered When failing to enter broadcast timer mode for an idle state that requires it, a new state is selected that does not require broadcast, but the broadcast variable remains set. This causes tick_broadcast_exit to be called despite not having entered broadcast mode. This causes the WARN_ON_ONCE(!irqs_disabled()) to trigger in some cases. It does not appear to cause problems for code today, but seems to violate the interface so should be fixed. Signed-off-by: Nicholas Piggin Reviewed-by: Thomas Gleixner Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index 484cc8909d5c..ed4df58a855e 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -208,6 +208,7 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv, return -EBUSY; } target_state = &drv->states[index]; + broadcast = false; } /* Take note of the planned idle state. */ From 0563bb7ba67eec7a87a9ccc04b80bb59de26c319 Mon Sep 17 00:00:00 2001 From: Jason Baron Date: Fri, 6 Oct 2017 13:19:45 -0400 Subject: [PATCH 2/8] intel_idle: replace conditionals with static_cpu_has(X86_FEATURE_ARAT) If the 'arat' cpu flag is set, then the conditionals in intel_idle() that guard calling tick_broadcast_enter()/exit() will never be true. Use static_cpu_has(X86_FEATURE_ARAT) to create a fast path to replace the conditional. Signed-off-by: Jason Baron Acked-by: Jacob Pan Signed-off-by: Rafael J. Wysocki --- drivers/idle/intel_idle.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 5dc7ea4b6bc4..5db5e3176f6a 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -913,8 +913,7 @@ static __cpuidle int intel_idle(struct cpuidle_device *dev, struct cpuidle_state *state = &drv->states[index]; unsigned long eax = flg2MWAIT(state->flags); unsigned int cstate; - - cstate = (((eax) >> MWAIT_SUBSTATE_SIZE) & MWAIT_CSTATE_MASK) + 1; + bool uninitialized_var(tick); /* * NB: if CPUIDLE_FLAG_TLB_FLUSHED is set, this idle transition @@ -923,12 +922,19 @@ static __cpuidle int intel_idle(struct cpuidle_device *dev, * useful with this knowledge. */ - if (!(lapic_timer_reliable_states & (1 << (cstate)))) - tick_broadcast_enter(); + if (!static_cpu_has(X86_FEATURE_ARAT)) { + cstate = (((eax) >> MWAIT_SUBSTATE_SIZE) & + MWAIT_CSTATE_MASK) + 1; + tick = false; + if (!(lapic_timer_reliable_states & (1 << (cstate)))) { + tick = true; + tick_broadcast_enter(); + } + } mwait_idle_with_hints(eax, ecx); - if (!(lapic_timer_reliable_states & (1 << (cstate)))) + if (!static_cpu_has(X86_FEATURE_ARAT) && tick) tick_broadcast_exit(); return index; From 0f87855d969a87f02048ff5ced7503465d5ab2f1 Mon Sep 17 00:00:00 2001 From: Leo Yan Date: Tue, 10 Oct 2017 13:47:55 +0800 Subject: [PATCH 3/8] ARM: cpuidle: Correct driver unregistration if init fails If cpuidle init fails, the code misses to unregister the driver for current CPU. Furthermore, we also need to rollback to cancel all previous CPUs registration; but the code retrieves driver handler by using function cpuidle_get_driver(), this function returns back current CPU driver handler but not previous CPU's handler, which leads to the failure handling code cannot unregister previous CPUs driver. This commit fixes two mentioned issues, it adds error handling path 'goto out_unregister_drv' for current CPU driver unregistration; and it is to replace cpuidle_get_driver() with cpuidle_get_cpu_driver(), the later function can retrieve driver handler for previous CPUs according to the CPU device handler so can unregister the driver properly. This patch also adds extra error handling paths 'goto out_kfree_dev' and 'goto out_kfree_drv' and adjusts the freeing sentences for previous CPUs; so make the code more readable for freeing 'dev' and 'drv' structures. Suggested-by: Daniel Lezcano Signed-off-by: Leo Yan Fixes: d50a7d8acd78 (ARM: cpuidle: Support asymmetric idle definition) Acked-by: Daniel Lezcano Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle-arm.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c index 52a75053ee03..f47c54546752 100644 --- a/drivers/cpuidle/cpuidle-arm.c +++ b/drivers/cpuidle/cpuidle-arm.c @@ -104,13 +104,13 @@ static int __init arm_idle_init(void) ret = dt_init_idle_driver(drv, arm_idle_state_match, 1); if (ret <= 0) { ret = ret ? : -ENODEV; - goto init_fail; + goto out_kfree_drv; } ret = cpuidle_register_driver(drv); if (ret) { pr_err("Failed to register cpuidle driver\n"); - goto init_fail; + goto out_kfree_drv; } /* @@ -128,14 +128,14 @@ static int __init arm_idle_init(void) if (ret) { pr_err("CPU %d failed to init idle CPU ops\n", cpu); - goto out_fail; + goto out_unregister_drv; } dev = kzalloc(sizeof(*dev), GFP_KERNEL); if (!dev) { pr_err("Failed to allocate cpuidle device\n"); ret = -ENOMEM; - goto out_fail; + goto out_unregister_drv; } dev->cpu = cpu; @@ -143,21 +143,25 @@ static int __init arm_idle_init(void) if (ret) { pr_err("Failed to register cpuidle device for CPU %d\n", cpu); - kfree(dev); - goto out_fail; + goto out_kfree_dev; } } return 0; -init_fail: + +out_kfree_dev: + kfree(dev); +out_unregister_drv: + cpuidle_unregister_driver(drv); +out_kfree_drv: kfree(drv); out_fail: while (--cpu >= 0) { dev = per_cpu(cpuidle_devices, cpu); + drv = cpuidle_get_cpu_driver(dev); cpuidle_unregister_device(dev); - kfree(dev); - drv = cpuidle_get_driver(); cpuidle_unregister_driver(drv); + kfree(dev); kfree(drv); } From 7943bfaeb6bbbf595df4bd4087f5b890761c4898 Mon Sep 17 00:00:00 2001 From: Leo Yan Date: Tue, 10 Oct 2017 13:47:56 +0800 Subject: [PATCH 4/8] ARM: cpuidle: Refactor rollback operations if init fails If init fails, we need execute two levels rollback operations: the first level is for the failed CPU rollback operations, the second level is to iterate all succeeded CPUs to cancel their registration; currently the code uses one function to finish these two levels rollback operations. This commit is to refactor rollback operations, so it adds a new function arm_idle_init_cpu() to encapsulate one specified CPU driver registration and rollback the first level operations; and use function arm_idle_init() to iterate all CPUs and finish the second level's rollback operations. Suggested-by: Daniel Lezcano Signed-off-by: Leo Yan Acked-by: Daniel Lezcano Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle-arm.c | 131 +++++++++++++++++++--------------- 1 file changed, 75 insertions(+), 56 deletions(-) diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c index f47c54546752..ddee1b601b89 100644 --- a/drivers/cpuidle/cpuidle-arm.c +++ b/drivers/cpuidle/cpuidle-arm.c @@ -72,79 +72,74 @@ static const struct of_device_id arm_idle_state_match[] __initconst = { }; /* - * arm_idle_init + * arm_idle_init_cpu * * Registers the arm specific cpuidle driver with the cpuidle * framework. It relies on core code to parse the idle states * and initialize them using driver data structures accordingly. */ -static int __init arm_idle_init(void) +static int __init arm_idle_init_cpu(int cpu) { - int cpu, ret; + int ret; struct cpuidle_driver *drv; struct cpuidle_device *dev; - for_each_possible_cpu(cpu) { + drv = kmemdup(&arm_idle_driver, sizeof(*drv), GFP_KERNEL); + if (!drv) + return -ENOMEM; - drv = kmemdup(&arm_idle_driver, sizeof(*drv), GFP_KERNEL); - if (!drv) { - ret = -ENOMEM; - goto out_fail; - } + drv->cpumask = (struct cpumask *)cpumask_of(cpu); - drv->cpumask = (struct cpumask *)cpumask_of(cpu); + /* + * Initialize idle states data, starting at index 1. This + * driver is DT only, if no DT idle states are detected (ret + * == 0) let the driver initialization fail accordingly since + * there is no reason to initialize the idle driver if only + * wfi is supported. + */ + ret = dt_init_idle_driver(drv, arm_idle_state_match, 1); + if (ret <= 0) { + ret = ret ? : -ENODEV; + goto out_kfree_drv; + } - /* - * Initialize idle states data, starting at index 1. This - * driver is DT only, if no DT idle states are detected (ret - * == 0) let the driver initialization fail accordingly since - * there is no reason to initialize the idle driver if only - * wfi is supported. - */ - ret = dt_init_idle_driver(drv, arm_idle_state_match, 1); - if (ret <= 0) { - ret = ret ? : -ENODEV; - goto out_kfree_drv; - } + ret = cpuidle_register_driver(drv); + if (ret) { + pr_err("Failed to register cpuidle driver\n"); + goto out_kfree_drv; + } - ret = cpuidle_register_driver(drv); - if (ret) { - pr_err("Failed to register cpuidle driver\n"); - goto out_kfree_drv; - } + /* + * Call arch CPU operations in order to initialize + * idle states suspend back-end specific data + */ + ret = arm_cpuidle_init(cpu); - /* - * Call arch CPU operations in order to initialize - * idle states suspend back-end specific data - */ - ret = arm_cpuidle_init(cpu); + /* + * Skip the cpuidle device initialization if the reported + * failure is a HW misconfiguration/breakage (-ENXIO). + */ + if (ret == -ENXIO) + return 0; - /* - * Skip the cpuidle device initialization if the reported - * failure is a HW misconfiguration/breakage (-ENXIO). - */ - if (ret == -ENXIO) - continue; + if (ret) { + pr_err("CPU %d failed to init idle CPU ops\n", cpu); + goto out_unregister_drv; + } - if (ret) { - pr_err("CPU %d failed to init idle CPU ops\n", cpu); - goto out_unregister_drv; - } + dev = kzalloc(sizeof(*dev), GFP_KERNEL); + if (!dev) { + pr_err("Failed to allocate cpuidle device\n"); + ret = -ENOMEM; + goto out_unregister_drv; + } + dev->cpu = cpu; - dev = kzalloc(sizeof(*dev), GFP_KERNEL); - if (!dev) { - pr_err("Failed to allocate cpuidle device\n"); - ret = -ENOMEM; - goto out_unregister_drv; - } - dev->cpu = cpu; - - ret = cpuidle_register_device(dev); - if (ret) { - pr_err("Failed to register cpuidle device for CPU %d\n", - cpu); - goto out_kfree_dev; - } + ret = cpuidle_register_device(dev); + if (ret) { + pr_err("Failed to register cpuidle device for CPU %d\n", + cpu); + goto out_kfree_dev; } return 0; @@ -155,6 +150,30 @@ out_unregister_drv: cpuidle_unregister_driver(drv); out_kfree_drv: kfree(drv); + return ret; +} + +/* + * arm_idle_init - Initializes arm cpuidle driver + * + * Initializes arm cpuidle driver for all CPUs, if any CPU fails + * to register cpuidle driver then rollback to cancel all CPUs + * registeration. + */ +static int __init arm_idle_init(void) +{ + int cpu, ret; + struct cpuidle_driver *drv; + struct cpuidle_device *dev; + + for_each_possible_cpu(cpu) { + ret = arm_idle_init_cpu(cpu); + if (ret) + goto out_fail; + } + + return 0; + out_fail: while (--cpu >= 0) { dev = per_cpu(cpuidle_devices, cpu); From c523c68da2117a3f9f777110839b1cf7ed7221be Mon Sep 17 00:00:00 2001 From: Ramesh Thomas Date: Thu, 26 Oct 2017 19:01:34 -0700 Subject: [PATCH 5/8] cpuidle: ladder: Add per CPU PM QoS resume latency support Individual CPUs may have special requirements to not enter deep idle states. For example, a CPU running real time applications would not want to enter deep idle states to avoid latency impacts. At the same time other CPUs that do not have such a requirement could allow deep idle states to save power. This was already implemented in the menu governor. Implementing similar changes in the ladder governor which gets selected when CONFIG_NO_HZ and CONFIG_NO_HZ_IDLE are not set. Refer following commits for the menu governor changes. Signed-off-by: Ramesh Thomas Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/governors/ladder.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c index ce1a2ffffb2a..1ad8745fd6d6 100644 --- a/drivers/cpuidle/governors/ladder.c +++ b/drivers/cpuidle/governors/ladder.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -67,10 +68,16 @@ static int ladder_select_state(struct cpuidle_driver *drv, struct cpuidle_device *dev) { struct ladder_device *ldev = this_cpu_ptr(&ladder_devices); + struct device *device = get_cpu_device(dev->cpu); struct ladder_device_state *last_state; int last_residency, last_idx = ldev->last_state_idx; int first_idx = drv->states[0].flags & CPUIDLE_FLAG_POLLING ? 1 : 0; int latency_req = pm_qos_request(PM_QOS_CPU_DMA_LATENCY); + int resume_latency = dev_pm_qos_raw_read_value(device); + + if (resume_latency < latency_req && + resume_latency != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) + latency_req = resume_latency; /* Special case when user has set very strict latency requirement */ if (unlikely(latency_req == 0)) { From e7b06a09e7d87ec0d6d8b17eec50fbb93667eee1 Mon Sep 17 00:00:00 2001 From: Gaurav Jindal Date: Fri, 1 Sep 2017 20:37:26 +0530 Subject: [PATCH 6/8] cpuidle: Clean up cpuidle_enable_device() error handling a bit Do not fetch per CPU drv if cpuidle_curr_governor is NULL to avoid useless per CPU processing. Signed-off-by: Gaurav Jindal [ rjw: Subject & changelog ] Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index ed4df58a855e..27f9648b61c2 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -388,9 +388,12 @@ int cpuidle_enable_device(struct cpuidle_device *dev) if (dev->enabled) return 0; + if (!cpuidle_curr_governor) + return -EIO; + drv = cpuidle_get_cpu_driver(dev); - if (!drv || !cpuidle_curr_governor) + if (!drv) return -EIO; if (!dev->registered) From 3fc74bd8a723c91a5b4627079c511fcaf3c75017 Mon Sep 17 00:00:00 2001 From: Gaurav Jindal Date: Sat, 2 Sep 2017 00:56:38 +0530 Subject: [PATCH 7/8] cpuidle: Avoid assignment in if () argument Clean up cpuidle_enable_device() to avoid doing an assignment in an expression evaluated as an argument of if (), which also makes the code in question more readable. Signed-off-by: Gaurav Jindal [ rjw: Subject & changelog ] Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index 27f9648b61c2..68a16827f45f 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -403,9 +403,11 @@ int cpuidle_enable_device(struct cpuidle_device *dev) if (ret) return ret; - if (cpuidle_curr_governor->enable && - (ret = cpuidle_curr_governor->enable(drv, dev))) - goto fail_sysfs; + if (cpuidle_curr_governor->enable) { + ret = cpuidle_curr_governor->enable(drv, dev); + if (ret) + goto fail_sysfs; + } smp_wmb(); From a4c447533a18ee86e07232d6344ba12b1f9c5077 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Thu, 9 Nov 2017 02:19:39 -0500 Subject: [PATCH 8/8] intel_idle: Graceful probe failure when MWAIT is disabled When MWAIT is disabled, intel_idle refuses to probe. But it may mis-lead the user by blaming this on the model number: intel_idle: does not run on family 6 modesl 79 So defer the check for MWAIT until after the model# white-list check succeeds, and if the MWAIT check fails, tell the user how to fix it: intel_idle: Please enable MWAIT in BIOS SETUP Signed-off-by: Len Brown Signed-off-by: Rafael J. Wysocki --- drivers/idle/intel_idle.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 5db5e3176f6a..9c93abdf635f 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -1066,7 +1066,7 @@ static const struct idle_cpu idle_cpu_dnv = { }; #define ICPU(model, cpu) \ - { X86_VENDOR_INTEL, 6, model, X86_FEATURE_MWAIT, (unsigned long)&cpu } + { X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned long)&cpu } static const struct x86_cpu_id intel_idle_ids[] __initconst = { ICPU(INTEL_FAM6_NEHALEM_EP, idle_cpu_nehalem), @@ -1130,6 +1130,11 @@ static int __init intel_idle_probe(void) return -ENODEV; } + if (!boot_cpu_has(X86_FEATURE_MWAIT)) { + pr_debug("Please enable MWAIT in BIOS SETUP\n"); + return -ENODEV; + } + if (boot_cpu_data.cpuid_level < CPUID_MWAIT_LEAF) return -ENODEV;