xen: arm: Spin-up cpu instead PSCI CPU OFF

Commit Message

Dmytro Semenets June 22, 2022, 7:24 a.m. UTC

From: Dmytro Semenets <[e-mail]>

Use spin-up cpu with disabled interrupts instead PSCI CPU OFF
halt and reboot procedures. Some platforms can't stop CPU via PSCI
because Thrusted OS can't migrate execution to other CPU.

Signed-off-by: Dmytro Semenets <[e-mail]>
---
 xen/arch/arm/shutdown.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Julien Grall June 22, 2022, 6:19 p.m. UTC | #1

Hi Dmytro,

Title: It seems to suggest we are completely removing PSCI CPU off. I 
would suggest to rename to:

xen/arm: Don't use stop_cpu() in halt_this_cpu()

On 22/06/2022 08:24, [e-mail] wrote:
> From: Dmytro Semenets <[e-mail]>
> 
> Use spin-up cpu with disabled interrupts instead PSCI CPU OFF
> halt and reboot procedures. Some platforms can't stop CPU via PSCI
> because Thrusted OS can't migrate execution to other CPU.

There are some information missing:
  - What's the problem if we don't do that (i.e. Xen will panic())
  - Reference to the spec
  - Why this is fine to not use PSCI off

I would suggest the following commit message:

"
When shutting down (or rebooting) the platform, Xen will call stop_cpu() 
on all the CPUs but one. The last CPU will then request the system to 
shutdown/restart.

On platform using PSCI, stop_cpu() will call PSCI CPU off. Per the spec 
(section 5.5.2 DEN0022D.b), the call could return DENIED if the Trusted 
OS is resident on the CPU that is about to be turned off.

As Xen doesn't migrate off the trusted OS (which BTW may not be 
migratable), it would be possible to hit the panic().

In the ideal situation, Xen should migrate the trusted OS or make sure 
the CPU off is not called. However, when shutting down (or rebooting) 
the platform, it is pointless to try to turn off all the CPUs (per 
section 5.10.2, it is only required to put the core in a known state).

So solve the problem by open-coding stop_cpu() in halt_this_cpu() and 
not call PSCI CPU off.
"

I will give an opportunity for you, Bertrand and Stefano to answer 
before committing it.

Cheers,

Patch

diff --git a/xen/arch/arm/shutdown.c b/xen/arch/arm/shutdown.c
index 3dc6819d56..a9aea19e8e 100644
--- a/xen/arch/arm/shutdown.c
+++ b/xen/arch/arm/shutdown.c
@@ -8,7 +8,12 @@ 
 
 static void noreturn halt_this_cpu(void *arg)
 {
-    stop_cpu();
+    local_irq_disable();
+    /* Make sure the write happens before we sleep forever */
+    dsb(sy);
+    isb();
+    while ( 1 )
+        wfi();
 }
 
 void machine_halt(void)