drivers: psci: PSCI checker module

On arm and arm64, PSCI is one of the possible firmware interfaces used for power management. This includes both turning CPUs on and off, and suspending them (entering idle states).

This patch adds a PSCI checker module that enables basic testing of PSCI operations during startup. There are two main tests: CPU hotplugging and suspending.

In the hotplug tests, the hotplug API is used to turn off and on again all CPUs in the system, and then all CPUs in each cluster, checking the consistency of the return codes.

In the suspend tests, a high-priority thread is created on each core and uses low-level cpuidle functionalities to enter suspend, in all the possible states and multiple times. This should allow a maximum number of CPUs to enter the same sleep state at the same or slightly different time.

Quoted 12 lines · Kevin Brodsky

On arm and arm64, PSCI is one of the possible firmware interfaces used for power management. This includes both turning CPUs on and off, and suspending them (entering idle states).

This patch adds a PSCI checker module that enables basic testing of PSCI operations during startup. There are two main tests: CPU hotplugging and suspending.

In the hotplug tests, the hotplug API is used to turn off and on again all CPUs in the system, and then all CPUs in each cluster, checking the consistency of the return codes.

Quoted 5 lines

In the suspend tests, a high-priority thread is created on each core and uses low-level cpuidle functionalities to enter suspend, in all the possible states and multiple times. This should allow a maximum number of CPUs to enter the same sleep state at the same or slightly different time.

Since you're doing it multiple times, it might be useful to track the actual time it takes to suspend and wakeup, and possibly even warn if the time is longer than the target_residency. This could catch poorly configured idle states.

On arm and arm64, PSCI is one of the possible firmware interfaces used for power management. This includes both turning CPUs on and off, and suspending them (entering idle states).

This patch adds a PSCI checker module that enables basic testing of PSCI operations during startup. There are two main tests: CPU hotplugging and suspending.

In the hotplug tests, the hotplug API is used to turn off and on again all CPUs in the system, and then all CPUs in each cluster, checking the consistency of the return codes.

In the suspend tests, a high-priority thread is created on each core and uses low-level cpuidle functionalities to enter suspend, in all the possible states and multiple times. This should allow a maximum number of CPUs to enter the same sleep state at the same or slightly different time.

In essence, the suspend tests use a principle similar to that of the intel_powerclamp driver (drivers/thermal/intel_powerclamp.c), but the threads are only kept for the duration of the test (they are already gone when userspace is started).

While in theory power management PSCI functions (CPU_{ON,OFF,SUSPEND}) could be directly called, this proved too difficult as it would imply the duplication of all the logic used by the kernel to allow for a clean shutdown/bringup/suspend of the CPU (the deepest sleep states implying potentially the shutdown of the CPU).

Note that this file cannot be compiled as a loadable module, since it uses a number of non-exported identifiers (essentially for PSCI-specific checks and direct use of cpuidle) and relies on the absence of userspace to avoid races when calling hotplug and cpuidle functions.

Cc Thomas Gleixner <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc "Rafael J. Wysocki" <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Mark Rutland <[email protected]>

Signed-off-by Kevin Brodsky <[email protected]>

---
Changelog v1..v2:
* Do not count tick_broadcast_enter() failures as errors, as they may be unavoidable. When it happens, fall back to WFI.
* Do not count unexpected sleep states as errors (currently, the only case is when falling back to WFI). Instead, report the number of times it happens before the suspend thread exits.
* Use usecs_to_jiffies() to compute the suspend timeout. The previous version resulted in a zero timeout if the target residency was shorter than a jiffy.
* Various cleanup.

Thanks to Lorenzo for his help with improving this patch!

Kevin

drivers/firmware/psci_checker.c new file +44 -0

+
+static int suspend_tests(void)
+{
+	int i, cpu, err = 0;
+	struct task_struct **threads;
+	int nb_threads = 0;
+
+	threads = kmalloc_array(nb_available_cpus, sizeof(*threads),
+				GFP_KERNEL);
+	if (!threads)
+		return -ENOMEM;
+
+	for_each_online_cpu(cpu) {
+		struct task_struct *thread;
+		/* Check that cpuidle is available on that CPU. */
+		struct cpuidle_device *dev = per_cpu(cpuidle_devices, cpu);
+		struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
+
+		if (cpuidle_not_available(drv, dev)) {
+			pr_warn("cpuidle not available on CPU %d, ignoring\n",
+				cpu);
+			continue;
+		}
+
+		thread = kthread_create_on_cpu(suspend_stress_thread,
+					       (void *)(long)cpu, cpu,
+					       "psci_suspend_stress");
+		if (IS_ERR(thread))
+			pr_err("Failed to create kthread on CPU %d\n", cpu);
+		else
+			threads[nb_threads++] = thread;
+	}
+	if (nb_threads < 1) {
+		kfree(threads);
+		return -ENODEV;
+	}
+
+	atomic_set(&nb_active_threads, nb_threads);
+
+	/*
+	 * Stop cpuidle to prevent the idle tasks from entering a deep sleep
+	 * mode, as it might interfere with the suspend threads on other CPUs.
+	 * This does not prevent the suspend threads from using cpuidle (only
+	 * the idle tasks check this status).

Hi Kevin,

Quoted 58 lines · Kevin Brodsky

On arm and arm64, PSCI is one of the possible firmware interfaces used for power management. This includes both turning CPUs on and off, and suspending them (entering idle states).

This patch adds a PSCI checker module that enables basic testing of PSCI operations during startup. There are two main tests: CPU hotplugging and suspending.

In the hotplug tests, the hotplug API is used to turn off and on again all CPUs in the system, and then all CPUs in each cluster, checking the consistency of the return codes.

In the suspend tests, a high-priority thread is created on each core and uses low-level cpuidle functionalities to enter suspend, in all the possible states and multiple times. This should allow a maximum number of CPUs to enter the same sleep state at the same or slightly different time.

In essence, the suspend tests use a principle similar to that of the intel_powerclamp driver (drivers/thermal/intel_powerclamp.c), but the threads are only kept for the duration of the test (they are already gone when userspace is started).

While in theory power management PSCI functions (CPU_{ON,OFF,SUSPEND}) could be directly called, this proved too difficult as it would imply the duplication of all the logic used by the kernel to allow for a clean shutdown/bringup/suspend of the CPU (the deepest sleep states implying potentially the shutdown of the CPU).

Note that this file cannot be compiled as a loadable module, since it uses a number of non-exported identifiers (essentially for PSCI-specific checks and direct use of cpuidle) and relies on the absence of userspace to avoid races when calling hotplug and cpuidle functions.

Cc Thomas Gleixner <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc "Rafael J. Wysocki" <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Mark Rutland <[email protected]>

Signed-off-by Kevin Brodsky <[email protected]>

---
Changelog v1..v2:
* Do not count tick_broadcast_enter() failures as errors, as they may be unavoidable. When it happens, fall back to WFI.
* Do not count unexpected sleep states as errors (currently, the only case is when falling back to WFI). Instead, report the number of times it happens before the suspend thread exits.
* Use usecs_to_jiffies() to compute the suspend timeout. The previous version resulted in a zero timeout if the target residency was shorter than a jiffy.
* Various cleanup.

Thanks to Lorenzo for his help with improving this patch!

Kevin

[...]

Quoted 44 lines

drivers/firmware/psci_checker.c new file +44 -0

+
+static int suspend_tests(void)
+{
+    int i, cpu, err = 0;
+    struct task_struct **threads;
+    int nb_threads = 0;
+
+    threads = kmalloc_array(nb_available_cpus, sizeof(*threads),
+                            GFP_KERNEL);
+    if (!threads)
+            return -ENOMEM;
+
+    for_each_online_cpu(cpu) {
+            struct task_struct *thread;
+            /* Check that cpuidle is available on that CPU. */
+            struct cpuidle_device *dev = per_cpu(cpuidle_devices, cpu);
+            struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
+
+            if (cpuidle_not_available(drv, dev)) {
+                    pr_warn("cpuidle not available on CPU %d, ignoring\n",
+                            cpu);
+                    continue;
+            }
+
+            thread = kthread_create_on_cpu(suspend_stress_thread,
+                                           (void *)(long)cpu, cpu,
+                                           "psci_suspend_stress");
+            if (IS_ERR(thread))
+                    pr_err("Failed to create kthread on CPU %d\n", cpu);
+            else
+                    threads[nb_threads++] = thread;
+    }
+    if (nb_threads < 1) {
+            kfree(threads);
+            return -ENODEV;
+    }
+
+    atomic_set(&nb_active_threads, nb_threads);
+
+    /*
+     * Stop cpuidle to prevent the idle tasks from entering a deep sleep
+     * mode, as it might interfere with the suspend threads on other CPUs.
+     * This does not prevent the suspend threads from using cpuidle (only
+     * the idle tasks check this status).

drivers/firmware/psci_checker.c line 419

Quoted 10 lines

drivers/firmware/psci_checker.c new file +10 -0

+     */
+    cpuidle_pause();
+
+    /*
+     * Unpark the suspend threads. To avoid the main thread being preempted
+     * before all the threads have been unparked, the suspend threads will
+     * wait for the completion of suspend_threads_started.
+     */
+    for (i = 0; i < nb_threads; ++i)
+            kthread_unpark(threads[i]);

Just a heads up: this doesn't work anymore, since a65d4096 (kthread/smpboot: do not park in kthread_create_on_cpu()), in 4.9-rc1. I think that the unpark call could be replaced by wake_up_process. The comment of kthread_create_on_cpu is now misleading.

Thanks,
Jean-Philippe

Thanks for the heads-up! I'll rebase on 4.9-rc1 and see what needs to be done.

On 20/10/16 14:38, Kevin Brodsky wrote:

[...]

Quoted 1 line

Quoted 3 lines

Thanks for the heads-up! I'll rebase on 4.9-rc1 and see what needs to be done.

Just be aware that v4.9-rc1 doesn't have commit 9cfb38a7ba5a ("sched/fair: Fix sched domains NULL dereference in select_idle_sibling()") which fixes the cpuhotplug issue you would observe.

Quoted 9 lines · Sudeep Holla

On 20/10/16 14:38, Kevin Brodsky wrote:

[...]

Quoted 4 lines

Thanks for the heads-up! I'll rebase on 4.9-rc1 and see what needs to be done.

Quoted 4 lines

Just be aware that v4.9-rc1 doesn't have commit 9cfb38a7ba5a ("sched/fair: Fix sched domains NULL dereference in select_idle_sibling()") which fixes the cpuhotplug issue you would observe.

Good to know. I saw that issue during resume from s2ram on r8a7795/Salvator-X once, but couldn't reproduce it.

Quoted 17 lines · Geert Uytterhoeven

Quoted 13 lines · Sudeep Holla

On 20/10/16 14:38, Kevin Brodsky wrote:

[...]

Quoted 4 lines

Thanks for the heads-up! I'll rebase on 4.9-rc1 and see what needs to be done.

Just be aware that v4.9-rc1 doesn't have commit 9cfb38a7ba5a ("sched/fair: Fix sched domains NULL dereference in select_idle_sibling()") which fixes the cpuhotplug issue you would observe.

Quoted 4 lines

Good to know. I saw that issue during resume from s2ram on r8a7795/Salvator-X
once, but couldn't reproduce it.

Did you try v4.9-rc1 itself ? The above commit is present post -rc1 and must be in -rc2. I can consistently see the crash without the commit.