From 39a74fdedd1c1461d6fb6d330b5266886513c98f Mon Sep 17 00:00:00 2001 From: Shaohua Li Date: Tue, 10 Jan 2012 15:48:19 -0800 Subject: intel_idle: fix API misuse smp_call_function() only lets all other CPUs execute a specific function, while we expect all CPUs do in intel_idle. Without the fix, we could have one cpu which has auto_demotion enabled or has no broadcast timer setup. Usually we don't see impact because auto demotion just harms power and the intel_idle init is called in CPU 0, where boradcast timer delivers interrupt, but this still could be a problem. Cc: stable@vger.kernel.org Signed-off-by: Shaohua Li Signed-off-by: Andrew Morton Signed-off-by: Len Brown --- drivers/idle/intel_idle.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/idle') diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 5d2f8e1..af0b4a5 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -394,7 +394,7 @@ static int intel_idle_probe(void) if (boot_cpu_has(X86_FEATURE_ARAT)) /* Always Reliable APIC Timer */ lapic_timer_reliable_states = LAPIC_TIMER_ALWAYS_RELIABLE; else { - smp_call_function(__setup_broadcast_timer, (void *)true, 1); + on_each_cpu(__setup_broadcast_timer, (void *)true, 1); register_cpu_notifier(&setup_broadcast_notifier); } @@ -471,7 +471,7 @@ static int intel_idle_cpuidle_driver_init(void) } if (auto_demotion_disable_flags) - smp_call_function(auto_demotion_disable, NULL, 1); + on_each_cpu(auto_demotion_disable, NULL, 1); return 0; } @@ -568,7 +568,7 @@ static void __exit intel_idle_exit(void) cpuidle_unregister_driver(&intel_idle_driver); if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE) { - smp_call_function(__setup_broadcast_timer, (void *)false, 1); + on_each_cpu(__setup_broadcast_timer, (void *)false, 1); unregister_cpu_notifier(&setup_broadcast_notifier); } -- cgit v1.1 From 63ff07beaebef2a82da41cf04053ae88b579226d Mon Sep 17 00:00:00 2001 From: Yanmin Zhang Date: Tue, 10 Jan 2012 15:48:21 -0800 Subject: intel_idle: remove redundant local_irq_disable() call irq disabling happens earlier in process_32.c:cpu_idle. Basically, cpuidle_state->enter is called, cpu irq is disabled. cpuidle_state->enter would turn on irq when exiting. intel_idle doesn't follow this assumption. Although it doesn't cause real issue, it misleads developers. Remove the call to local_irq_disable() at entry. [akpm@linux-foundation.org: add comment] Signed-off-by: Mingming Zhang Signed-off-by: Andrew Morton Signed-off-by: Len Brown --- drivers/idle/intel_idle.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/idle') diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index af0b4a5..920c61c 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -232,6 +232,7 @@ static int get_driver_data(int cstate) * @drv: cpuidle driver * @index: index of cpuidle state * + * Must be called under local_irq_disable(). */ static int intel_idle(struct cpuidle_device *dev, struct cpuidle_driver *drv, int index) @@ -247,8 +248,6 @@ static int intel_idle(struct cpuidle_device *dev, cstate = (((eax) >> MWAIT_SUBSTATE_SIZE) & MWAIT_CSTATE_MASK) + 1; - local_irq_disable(); - /* * leave_mm() to avoid costly and often unnecessary wakeups * for flushing the user TLB's associated with the active mm. -- cgit v1.1 From 95e3ec11491d0cbce9fcdf1cc17a527c114c7dcf Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 15 Dec 2011 13:03:14 +0000 Subject: intel_idle: Fix a cast to pointer from integer of different size warning in intel_idle Fix the following warning: drivers/idle/intel_idle.c: In function 'intel_idle_cpuidle_devices_init': drivers/idle/intel_idle.c:518:5: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] By making get_driver_data() return a long instead of an int. Signed-off-by: David Howells Signed-off-by: Len Brown --- drivers/idle/intel_idle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/idle') diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 920c61c..18f9ad8 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -197,7 +197,7 @@ static struct cpuidle_state atom_cstates[MWAIT_MAX_NUM_CSTATES] = { .enter = &intel_idle }, }; -static int get_driver_data(int cstate) +static long get_driver_data(int cstate) { int driver_data; switch (cstate) { -- cgit v1.1 From 5c2a9f06a9cd7194f884cdc88144866235dec07d Mon Sep 17 00:00:00 2001 From: Thomas Renninger Date: Sun, 4 Dec 2011 22:17:29 +0100 Subject: intel idle: Make idle driver more robust kvm -cpu host passes the original cpuid info to the guest. Latest kvm version seem to return true for mwait_leaf cpuid function on recent Intel CPUs. But it does not return mwait C-states (mwait_substates), instead zero is returned. While real CPUs seem to always return non-zero values, the intel idle driver should not get active in kvm (mwait_substates == 0) case and bail out. Otherwise a Null pointer exception will happen later when the cpuidle subsystem tries to get active: [0.984807] BUG: unable to handle kernel NULL pointer dereference at (null) [0.984807] IP: [<(null)>] (null) ... [0.984807][] ? cpuidle_idle_call+0xb4/0x340 [0.984807][] ? __atomic_notifier_call_chain+0x4c/0x70 [0.984807][] ? cpu_idle+0x78/0xd0 Reference: https://bugzilla.novell.com/show_bug.cgi?id=726296 Cc: stable@vger.kernel.org Signed-off-by: Thomas Renninger CC: Bruno Friedmann Signed-off-by: Len Brown --- drivers/idle/intel_idle.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/idle') diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 18f9ad8..91aff18 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -347,7 +347,8 @@ static int intel_idle_probe(void) cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &mwait_substates); if (!(ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED) || - !(ecx & CPUID5_ECX_INTERRUPT_BREAK)) + !(ecx & CPUID5_ECX_INTERRUPT_BREAK) || + !mwait_substates) return -ENODEV; pr_debug(PREFIX "MWAIT substates: 0x%x\n", mwait_substates); -- cgit v1.1 From 65b7f839ceecc0a36c7969c0c9151d5748cd4242 Mon Sep 17 00:00:00 2001 From: Thomas Renninger Date: Tue, 17 Jan 2012 22:40:08 +0100 Subject: intel_idle: Split up and provide per CPU initialization func Function split up, should have no functional change. Provides entry point for physically hotplugged CPUs to initialize and activate cpuidle. Signed-off-by: Thomas Renninger CC: Deepthi Dharwar CC: Shaohua Li CC: Andrew Morton Signed-off-by: Len Brown --- drivers/idle/intel_idle.c | 82 ++++++++++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 40 deletions(-) (limited to 'drivers/idle') diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 5d2f8e1..ef0c04d 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -478,64 +478,60 @@ static int intel_idle_cpuidle_driver_init(void) /* - * intel_idle_cpuidle_devices_init() + * intel_idle_cpu_init() * allocate, initialize, register cpuidle_devices + * @cpu: cpu/core to initialize */ -static int intel_idle_cpuidle_devices_init(void) +int intel_idle_cpu_init(int cpu) { - int i, cstate; + int cstate; struct cpuidle_device *dev; - intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device); - if (intel_idle_cpuidle_devices == NULL) - return -ENOMEM; - - for_each_online_cpu(i) { - dev = per_cpu_ptr(intel_idle_cpuidle_devices, i); + dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu); - dev->state_count = 1; + dev->state_count = 1; - for (cstate = 1; cstate < MWAIT_MAX_NUM_CSTATES; ++cstate) { - int num_substates; + for (cstate = 1; cstate < MWAIT_MAX_NUM_CSTATES; ++cstate) { + int num_substates; - if (cstate > max_cstate) { - printk(PREFIX "max_cstate %d reached\n", - max_cstate); - break; - } + if (cstate > max_cstate) { + printk(PREFIX "max_cstate %d reached\n", + max_cstate); + break; + } - /* does the state exist in CPUID.MWAIT? */ - num_substates = (mwait_substates >> ((cstate) * 4)) - & MWAIT_SUBSTATE_MASK; - if (num_substates == 0) - continue; - /* is the state not enabled? */ - if (cpuidle_state_table[cstate].enter == NULL) { - continue; - } + /* does the state exist in CPUID.MWAIT? */ + num_substates = (mwait_substates >> ((cstate) * 4)) + & MWAIT_SUBSTATE_MASK; + if (num_substates == 0) + continue; + /* is the state not enabled? */ + if (cpuidle_state_table[cstate].enter == NULL) + continue; - dev->states_usage[dev->state_count].driver_data = - (void *)get_driver_data(cstate); + dev->states_usage[dev->state_count].driver_data = + (void *)get_driver_data(cstate); dev->state_count += 1; } + dev->cpu = cpu; - dev->cpu = i; - if (cpuidle_register_device(dev)) { - pr_debug(PREFIX "cpuidle_register_device %d failed!\n", - i); - intel_idle_cpuidle_devices_uninit(); - return -EIO; - } + if (cpuidle_register_device(dev)) { + pr_debug(PREFIX "cpuidle_register_device %d failed!\n", cpu); + intel_idle_cpuidle_devices_uninit(); + return -EIO; } + if (auto_demotion_disable_flags) + smp_call_function_single(cpu, auto_demotion_disable, NULL, 1); + return 0; } static int __init intel_idle_init(void) { - int retval; + int retval, i; /* Do not load intel_idle at all for now if idle= is passed */ if (boot_option_idle_override != IDLE_NO_OVERRIDE) @@ -553,10 +549,16 @@ static int __init intel_idle_init(void) return retval; } - retval = intel_idle_cpuidle_devices_init(); - if (retval) { - cpuidle_unregister_driver(&intel_idle_driver); - return retval; + intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device); + if (intel_idle_cpuidle_devices == NULL) + return -ENOMEM; + + for_each_online_cpu(i) { + retval = intel_idle_cpu_init(i); + if (retval) { + cpuidle_unregister_driver(&intel_idle_driver); + return retval; + } } return 0; -- cgit v1.1