diff mbox series

[2/2] arm/psci: Add PSCI_CPU_OFF testscase to arm/psci testsuite

Message ID 20221205121053.125964-3-Nikita.Venkatesh@arm.com (mailing list archive)
State New, archived
Headers show
Series [1/2] arm/psci: Test that CPU 1 has been successfully brought online | expand

Commit Message

Nikita Venkatesh Dec. 5, 2022, 12:10 p.m. UTC 
The test uses the following method.

The primary CPU brings up all the secondary CPUs, which are held in a wait
loop. Once the primary releases the CPUs, each of the secondary CPUs
proceed to issue PSCI_CPU_OFF. This is indicated by a cpumask and also
the status of the call is updated by the secondary CPU in cpu_off_done[].

The primary CPU waits for all the secondary CPUs to update the cpumask
and then proceeds to check for the status of the individual CPU CPU_OFF
request. There is a chance that some CPUs might fail at the CPU_OFF
request and come back and update the status once the primary CPU has
finished the scan. There is no fool proof method to handle this. As of
now, we add a 1sec delay between the cpumask check and the scan for the
status.

Signed-off-by: Nikita Venkatesh <Nikita.Venkatesh@arm.com>
---
 arm/psci.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 70 insertions(+), 6 deletions(-)

Comments

Alexandru Elisei Dec. 9, 2022, 4 p.m. UTC | #1 
Hi,

On Mon, Dec 05, 2022 at 12:10:53PM +0000, Nikita Venkatesh wrote:
> The test uses the following method.
> 
> The primary CPU brings up all the secondary CPUs, which are held in a wait
> loop. Once the primary releases the CPUs, each of the secondary CPUs
> proceed to issue PSCI_CPU_OFF. This is indicated by a cpumask and also
> the status of the call is updated by the secondary CPU in cpu_off_done[].
> 
> The primary CPU waits for all the secondary CPUs to update the cpumask
> and then proceeds to check for the status of the individual CPU CPU_OFF
> request. There is a chance that some CPUs might fail at the CPU_OFF
> request and come back and update the status once the primary CPU has
> finished the scan. There is no fool proof method to handle this. As of
> now, we add a 1sec delay between the cpumask check and the scan for the
> status.
> 
> Signed-off-by: Nikita Venkatesh <Nikita.Venkatesh@arm.com>
> ---
>  arm/psci.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 70 insertions(+), 6 deletions(-)
> 
> diff --git a/arm/psci.c b/arm/psci.c
> index 0b9834c..9a49347 100644
> --- a/arm/psci.c
> +++ b/arm/psci.c
> @@ -12,6 +12,9 @@
>  #include <asm/processor.h>
>  #include <asm/smp.h>
>  #include <asm/psci.h>
> +#include <asm/delay.h>
> +
> +#define CPU_OFF_TEST_WAIT_TIME 1000
>  
>  static bool invalid_function_exception;
>  
> @@ -69,8 +72,10 @@ static bool psci_affinity_info_off(void)
>  }
>  
>  static int cpu_on_ret[NR_CPUS];
> -static cpumask_t cpu_on_ready, cpu_on_done;
> +static bool cpu_off_success[NR_CPUS];
> +static cpumask_t cpu_on_ready, cpu_on_done, cpu_off_done;
>  static volatile int cpu_on_start;
> +static volatile int cpu_off_start;
>  
>  extern void secondary_entry(void);
>  static void cpu_on_wake_target(void)
> @@ -92,6 +97,20 @@ static void cpu_on_target(void)
>  	cpumask_set_cpu(cpu, &cpu_on_done);
>  }
>  
> +static void cpu_off_secondary_entry(void *data)
> +{
> +       int cpu = smp_processor_id();
> +
> +       while (!cpu_off_start)
> +               cpu_relax();
> +       /* On to the CPU off test */
> +       cpu_off_success[cpu] = true;
> +       cpumask_set_cpu(cpu, &cpu_off_done);
> +       cpu_psci_cpu_die();
> +       /* The CPU shouldn't execute the next steps. */
> +       cpu_off_success[cpu] = false;
> +}
> +
>  static bool psci_cpu_on_test(void)
>  {
>  	bool failed = false;
> @@ -142,7 +161,48 @@ static bool psci_cpu_on_test(void)
>  	return !failed;
>  }
>  
> -int main(void)
> +static void secondary_entry_stub (void *unused)
> +{
> +}
> +
> +static bool psci_cpu_off_test(void)
> +{
> +       bool failed = false;
> +       int cpu;
> +
> +       for_each_present_cpu(cpu) {
> +               if (cpu == 0)
> +                       continue;
> +               on_cpu_async(cpu, cpu_off_secondary_entry, NULL);
> +       }
> +
> +       cpumask_set_cpu(0, &cpu_off_done);
> +
> +       report_info("starting CPU_OFF test...");
> +
> +       /* Release the CPUs */
> +       cpu_off_start = 1;
> +
> +       /* Wait until all are done */
> +       while (!cpumask_full(&cpu_off_done))
> +               cpu_relax();
> +
> +       /* Allow all the other CPUs to complete the operation */
> +       mdelay(CPU_OFF_TEST_WAIT_TIME);
> +
> +       for_each_present_cpu(cpu) {
> +               if (cpu == 0)
> +                       continue;
> +
> +               if (!cpu_off_success[cpu]) {
> +                       report_info("CPU%d could not be turned off", cpu);
> +                       failed = true;
> +               }
> +       }
> +       return !failed;
> +}
> +
> +int main(int argc, char **argv)
>  {
>  	int ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
>  
> @@ -155,14 +215,18 @@ int main(void)
>  
>  	report_info("PSCI version %d.%d", PSCI_VERSION_MAJOR(ver),
>  					  PSCI_VERSION_MINOR(ver));
> +
>  	report(psci_invalid_function(), "invalid-function");
> -	report(psci_affinity_info_on(), "affinity-info-on");
> -	report(psci_affinity_info_off(), "affinity-info-off");
> +        report(psci_affinity_info_on(), "affinity-info-on");
> +        report(psci_affinity_info_off(), "affinity-info-off");
>  
> -	if (ERRATA(6c7a5dce22b3))
> +        if (ERRATA(6c7a5dce22b3)){
>  		report(psci_cpu_on_test(), "cpu-on");
> -	else
> +	} else {
>  		report_skip("Skipping unsafe cpu-on test. Set ERRATA_6c7a5dce22b3=y to enable.");
> +		on_cpus(secondary_entry_stub, NULL);

I think I was wrong when I proposed this, on_cpu_async() will call
__smp_boot_secondary() if the target CPU is no online. So no need to use
on_cpus() to make sure secondaries are online before the CPU_OFF test.

Also, it kind of looks to me like there's a problem with the indentation, you
could fix it for the next iteration.

Thanks,
Alex

> +	}
> +	report(psci_cpu_off_test(), "cpu-off");
>  
>  done:
>  #if 0
> -- 
> 2.25.1
>
diff mbox series

Patch

diff --git a/arm/psci.c b/arm/psci.c
index 0b9834c..9a49347 100644
--- a/arm/psci.c
+++ b/arm/psci.c
@@ -12,6 +12,9 @@ 
 #include <asm/processor.h>
 #include <asm/smp.h>
 #include <asm/psci.h>
+#include <asm/delay.h>
+
+#define CPU_OFF_TEST_WAIT_TIME 1000
 
 static bool invalid_function_exception;
 
@@ -69,8 +72,10 @@  static bool psci_affinity_info_off(void)
 }
 
 static int cpu_on_ret[NR_CPUS];
-static cpumask_t cpu_on_ready, cpu_on_done;
+static bool cpu_off_success[NR_CPUS];
+static cpumask_t cpu_on_ready, cpu_on_done, cpu_off_done;
 static volatile int cpu_on_start;
+static volatile int cpu_off_start;
 
 extern void secondary_entry(void);
 static void cpu_on_wake_target(void)
@@ -92,6 +97,20 @@  static void cpu_on_target(void)
 	cpumask_set_cpu(cpu, &cpu_on_done);
 }
 
+static void cpu_off_secondary_entry(void *data)
+{
+       int cpu = smp_processor_id();
+
+       while (!cpu_off_start)
+               cpu_relax();
+       /* On to the CPU off test */
+       cpu_off_success[cpu] = true;
+       cpumask_set_cpu(cpu, &cpu_off_done);
+       cpu_psci_cpu_die();
+       /* The CPU shouldn't execute the next steps. */
+       cpu_off_success[cpu] = false;
+}
+
 static bool psci_cpu_on_test(void)
 {
 	bool failed = false;
@@ -142,7 +161,48 @@  static bool psci_cpu_on_test(void)
 	return !failed;
 }
 
-int main(void)
+static void secondary_entry_stub (void *unused)
+{
+}
+
+static bool psci_cpu_off_test(void)
+{
+       bool failed = false;
+       int cpu;
+
+       for_each_present_cpu(cpu) {
+               if (cpu == 0)
+                       continue;
+               on_cpu_async(cpu, cpu_off_secondary_entry, NULL);
+       }
+
+       cpumask_set_cpu(0, &cpu_off_done);
+
+       report_info("starting CPU_OFF test...");
+
+       /* Release the CPUs */
+       cpu_off_start = 1;
+
+       /* Wait until all are done */
+       while (!cpumask_full(&cpu_off_done))
+               cpu_relax();
+
+       /* Allow all the other CPUs to complete the operation */
+       mdelay(CPU_OFF_TEST_WAIT_TIME);
+
+       for_each_present_cpu(cpu) {
+               if (cpu == 0)
+                       continue;
+
+               if (!cpu_off_success[cpu]) {
+                       report_info("CPU%d could not be turned off", cpu);
+                       failed = true;
+               }
+       }
+       return !failed;
+}
+
+int main(int argc, char **argv)
 {
 	int ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
 
@@ -155,14 +215,18 @@  int main(void)
 
 	report_info("PSCI version %d.%d", PSCI_VERSION_MAJOR(ver),
 					  PSCI_VERSION_MINOR(ver));
+
 	report(psci_invalid_function(), "invalid-function");
-	report(psci_affinity_info_on(), "affinity-info-on");
-	report(psci_affinity_info_off(), "affinity-info-off");
+        report(psci_affinity_info_on(), "affinity-info-on");
+        report(psci_affinity_info_off(), "affinity-info-off");
 
-	if (ERRATA(6c7a5dce22b3))
+        if (ERRATA(6c7a5dce22b3)){
 		report(psci_cpu_on_test(), "cpu-on");
-	else
+	} else {
 		report_skip("Skipping unsafe cpu-on test. Set ERRATA_6c7a5dce22b3=y to enable.");
+		on_cpus(secondary_entry_stub, NULL);
+	}
+	report(psci_cpu_off_test(), "cpu-off");
 
 done:
 #if 0