PSCI: fix some PSCI issues and decouple CPU standby from CPU power down

src/arch/armv8/armv8-a/cpu.c

@@ -27,9 +27,17 @@ void cpu_arch_profile_init(cpuid_t cpuid, paddr_t load_addr)
     }
 }
 
-void cpu_arch_profile_idle()
+void cpu_arch_profile_standby()
 {
-    int64_t err = psci_power_down(PSCI_WAKEUP_IDLE);
+    int32_t err = psci_standby();
+    if (err) {
+        ERROR("PSCI cpu%d standby failed with error %ld", cpu()->id, err);
+    }
+}
+
+void cpu_arch_profile_powerdown()
+{
+    int32_t err = psci_power_down();
     if (err) {
         ERROR("PSCI cpu%d power down failed with error %ld", cpu()->id, err);
     }

src/arch/armv8/armv8-a/psci.c

@@ -56,17 +56,11 @@ static void psci_wake_from_powerdown(void)
     vcpu_run(cpu()->vcpu);
 }
 
-static void psci_wake_from_idle(void)
-{
-    cpu_idle_wakeup();
-}
-
 void psci_wake_from_off(void);
 
 void (*psci_wake_handlers[PSCI_WAKEUP_NUM])(void) = {
     [PSCI_WAKEUP_CPU_OFF] = psci_wake_from_off,
     [PSCI_WAKEUP_POWERDOWN] = psci_wake_from_powerdown,
-    [PSCI_WAKEUP_IDLE] = psci_wake_from_idle,
 };
 
 void psci_wake(uint32_t handler_id)
@@ -98,7 +92,7 @@ int32_t psci_standby()
     return psci_cpu_suspend(pwr_state_aux, 0, 0);
 }
 
-int32_t psci_power_down(enum wakeup_reason reason)
+int32_t psci_power_down()
 {
     /* We've observed that some platforms behave unexpectedly when performing
      * power down. In these cases, after powerdown, the CPU cores are not awaken
@@ -111,7 +105,7 @@ int32_t psci_power_down(enum wakeup_reason reason)
 
     uint32_t pwr_state_aux = PSCI_POWER_STATE_LVL_0 | PSCI_STATE_TYPE_POWERDOWN;
 
-    psci_save_state(reason);
+    psci_save_state(PSCI_WAKEUP_POWERDOWN);
     paddr_t cntxt_paddr;
     paddr_t psci_wakeup_addr;
     mem_translate(&cpu()->as, (vaddr_t)&cpu()->arch.profile.psci_off_state, &cntxt_paddr);

src/arch/armv8/armv8-r/cpu.c

@@ -12,7 +12,12 @@ void cpu_arch_profile_init(cpuid_t cpuid, paddr_t load_addr)
     UNUSED_ARG(load_addr);
 }
 
-void cpu_arch_profile_idle()
+void cpu_arch_profile_standby()
+{
+    __asm__ volatile("wfi");
+}
+
+void cpu_arch_profile_powerdown()
 {
     __asm__ volatile("wfi");
 }

src/arch/armv8/armv8-r/psci.c

@@ -18,9 +18,7 @@ int32_t psci_standby()
     return PSCI_E_SUCCESS;
 }
 
-int32_t psci_power_down(enum wakeup_reason reason)
+int32_t psci_power_down()
 {
-    UNUSED_ARG(reason);
-
     return psci_standby();
 }

src/arch/armv8/cpu.c

@@ -22,17 +22,32 @@ unsigned long cpu_id_to_mpidr(cpuid_t id)
     return platform_arch_cpuid_to_mpidr(&platform, id);
 }
 
-void cpu_arch_idle()
+void cpu_arch_standby()
 {
-    cpu_arch_profile_idle();
+    cpu_arch_profile_standby();
 
     /*
      * In case the profile implementation does not jump to a predefined wake-up point and just
-     * returns from the profile, manually rewind stack and jump to idle wake up. Therefore, we
+     * returns from the profile, manually rewind stack and jump to standby wake up. Therefore, we
      * should not return after this point.
      */
     __asm__ volatile("mov sp, %0\n\r"
-                     "b cpu_idle_wakeup\n\r" ::"r"(&cpu()->stack[STACK_SIZE]));
+                     "b cpu_standby_wakeup\n\r" ::"r"(&cpu()->stack[STACK_SIZE]));
 
-    ERROR("returned from idle wake up");
+    ERROR("returned from standby wake up");
+}
+
+void cpu_arch_powerdown()
+{
+    cpu_arch_profile_powerdown();
+
+    /*
+     * In case the profile implementation does not jump to a predefined wake-up point and just
+     * returns from the profile, manually rewind stack and jump to powerdown wake up. Therefore, we
+     * should not return after this point.
+     */
+    __asm__ volatile("mov sp, %0\n\r"
+                     "b cpu_powerdown_wakeup\n\r" ::"r"(&cpu()->stack[STACK_SIZE]));
+
+    ERROR("returned from powerdown wake up");
 }

src/arch/armv8/inc/arch/cpu.h

@@ -18,7 +18,8 @@ struct cpu_arch {
 
 unsigned long cpu_id_to_mpidr(cpuid_t id);
 void cpu_arch_profile_init(cpuid_t cpuid, paddr_t load_addr);
-void cpu_arch_profile_idle(void);
+void cpu_arch_profile_standby(void);
+void cpu_arch_profile_powerdown(void);
 
 extern cpuid_t CPU_MASTER;
 

src/arch/armv8/inc/arch/psci.h

@@ -90,12 +90,7 @@ struct psci_off_state {
     struct gicc_state gicc_state __attribute__((aligned(8)));
 } __attribute__((packed, aligned(8)));
 
-enum wakeup_reason {
-    PSCI_WAKEUP_CPU_OFF,
-    PSCI_WAKEUP_POWERDOWN,
-    PSCI_WAKEUP_IDLE,
-    PSCI_WAKEUP_NUM
-};
+enum wakeup_reason { PSCI_WAKEUP_CPU_OFF, PSCI_WAKEUP_POWERDOWN, PSCI_WAKEUP_NUM };
 
 /* --------------------------------
         SMC Trapping
@@ -104,7 +99,7 @@ enum wakeup_reason {
 int32_t psci_smc_handler(uint32_t smc_fid, unsigned long x1, unsigned long x2, unsigned long x3);
 
 int32_t psci_standby(void);
-int32_t psci_power_down(enum wakeup_reason reason);
+int32_t psci_power_down(void);
 void psci_wake_from_off(void);
 void psci_wake(uint32_t handler_id);
 

src/arch/armv8/psci.c

@@ -66,7 +66,7 @@ static int32_t psci_cpu_suspend_handler(uint32_t power_state, unsigned long entr
         cpu()->vcpu->arch.psci_ctx.entrypoint = entrypoint;
         cpu()->vcpu->arch.psci_ctx.context_id = context_id;
         spin_unlock(&cpu()->vcpu->arch.psci_ctx.lock);
-        ret = psci_power_down(PSCI_WAKEUP_POWERDOWN);
+        ret = psci_power_down();
     } else {
         // PSCI_STATE_TYPE_STANDBY:
         ret = psci_standby();
@@ -86,7 +86,7 @@ static int32_t psci_cpu_off_handler(void)
     cpu()->vcpu->arch.psci_ctx.state = OFF;
     spin_unlock(&cpu()->vcpu->arch.psci_ctx.lock);
 
-    cpu_idle();
+    cpu_powerdown();
 
     spin_lock(&cpu()->vcpu->arch.psci_ctx.lock);
     cpu()->vcpu->arch.psci_ctx.state = ON;

src/arch/armv8/vm.c

@@ -80,16 +80,7 @@ void vcpu_arch_reset(struct vcpu* vcpu, vaddr_t entry)
      */
 }
 
-static inline bool vcpu_psci_state_on(struct vcpu* vcpu)
+bool vcpu_arch_is_on(struct vcpu* vcpu)
 {
     return vcpu->arch.psci_ctx.state == ON;
 }
-
-void vcpu_arch_run(struct vcpu* vcpu)
-{
-    if (vcpu_psci_state_on(vcpu)) {
-        vcpu_arch_entry();
-    } else {
-        cpu_idle();
-    }
-}

src/arch/riscv/cpu.c

@@ -27,13 +27,21 @@ void cpu_arch_init(cpuid_t cpuid, paddr_t load_addr)
     }
 }
 
-void cpu_arch_idle(void)
+void cpu_arch_standby(void)
 {
     struct sbiret ret = sbi_hart_suspend(SBI_HSM_SUSPEND_RET_DEFAULT, 0, 0);
     if (ret.error < 0) {
         ERROR("failed to suspend hart %d", cpu()->id);
     }
     __asm__ volatile("mv sp, %0\n\r"
-                     "j cpu_idle_wakeup\n\r" ::"r"(&cpu()->stack[STACK_SIZE]));
-    ERROR("returned from idle wake up");
+                     "j cpu_standby_wakeup\n\r" ::"r"(&cpu()->stack[STACK_SIZE]));
+    ERROR("returned from standby wake up");
+}
+
+void cpu_arch_powerdown(void)
+{
+    __asm__ volatile("wfi\n\t" ::: "memory");
+    __asm__ volatile("mv sp, %0\n\r"
+                     "j cpu_powerdown_wakeup\n\r" ::"r"(&cpu()->stack[STACK_SIZE]));
+    ERROR("returned from powerdown wake up");
 }

src/arch/riscv/vm.c

@@ -90,11 +90,7 @@ void vcpu_writepc(struct vcpu* vcpu, unsigned long pc)
     vcpu->regs.sepc = pc;
 }
 
-void vcpu_arch_run(struct vcpu* vcpu)
+bool vcpu_arch_is_on(struct vcpu* vcpu)
 {
-    if (vcpu->arch.sbi_ctx.state == STARTED) {
-        vcpu_arch_entry();
-    } else {
-        cpu_idle();
-    }
+    return vcpu->arch.sbi_ctx.state == STARTED;
 }

src/core/cpu.c

@@ -89,18 +89,29 @@ void cpu_msg_handler(void)
     cpu()->handling_msgs = false;
 }
 
-void cpu_idle(void)
+void cpu_standby(void)
 {
-    cpu_arch_idle();
+    cpu_arch_standby();
 
     /**
-     * Should not return here. cpu should "wake up" from idle in cpu_idle_wakeup with a rewinded
-     * stack.
+     * Should not return here. cpu should "wake up" from standby in cpu_standby_wakeup with a
+     * rewinded stack.
      */
-    ERROR("Spurious idle wake up");
+    ERROR("Spurious standby wake up");
 }
 
-void cpu_idle_wakeup(void)
+void cpu_powerdown(void)
+{
+    cpu_arch_powerdown();
+
+    /**
+     * Should not return here. cpu should "wake up" from powerdown in cpu_powerdown_wakeup with a
+     * rewinded stack.
+     */
+    ERROR("Spurious powerdown wake up");
+}
+
+void cpu_standby_wakeup(void)
 {
     if (interrupts_check(interrupts_ipi_id)) {
         interrupts_clear(interrupts_ipi_id);
@@ -110,6 +121,15 @@ void cpu_idle_wakeup(void)
     if (cpu()->vcpu != NULL) {
         vcpu_run(cpu()->vcpu);
     } else {
-        cpu_idle();
+        cpu_standby();
+    }
+}
+
+void cpu_powerdown_wakeup(void)
+{
+    if (cpu()->vcpu != NULL) {
+        vcpu_run(cpu()->vcpu);
+    } else {
+        cpu_powerdown();
     }
 }

src/core/inc/cpu.h

@@ -67,11 +67,14 @@ void cpu_send_msg(cpuid_t cpu, struct cpu_msg* msg);
 bool cpu_get_msg(struct cpu_msg* msg);
 void cpu_msg_handler(void);
 void cpu_msg_set_handler(cpuid_t id, cpu_msg_handler_t handler);
-void cpu_idle(void);
-void cpu_idle_wakeup(void);
+void cpu_standby(void);
+void cpu_powerdown(void);
+void cpu_standby_wakeup(void);
+void cpu_powerdown_wakeup(void);
 
 void cpu_arch_init(cpuid_t cpu_id, paddr_t load_addr);
-void cpu_arch_idle(void);
+void cpu_arch_standby(void);
+void cpu_arch_powerdown(void);
 
 extern struct cpuif cpu_interfaces[];
 static inline struct cpuif* cpu_if(cpuid_t cpu_id)

src/core/inc/vm.h

@@ -174,7 +174,7 @@ unsigned long vcpu_readreg(struct vcpu* vcpu, unsigned long reg);
 void vcpu_writereg(struct vcpu* vcpu, unsigned long reg, unsigned long val);
 unsigned long vcpu_readpc(struct vcpu* vcpu);
 void vcpu_writepc(struct vcpu* vcpu, unsigned long pc);
-void vcpu_arch_run(struct vcpu* vcpu);
 void vcpu_arch_reset(struct vcpu* vcpu, vaddr_t entry);
+bool vcpu_arch_is_on(struct vcpu* vcpu);
 
 #endif /* __VM_H__ */

src/core/vm.c

@@ -350,9 +350,13 @@ __attribute__((weak)) cpumap_t vm_translate_to_vcpu_mask(struct vm* vm, cpumap_t
 
 void vcpu_run(struct vcpu* vcpu)
 {
-    if (vcpu->active == false) {
-        cpu_idle();
+    if (vcpu_arch_is_on(vcpu)) {
+        if (vcpu->active) {
+            vcpu_arch_entry();
+        } else {
+            cpu_standby();
+        }
     } else {
-        vcpu_arch_run(vcpu);
+        cpu_powerdown();
     }
 }

src/core/vmm.c

@@ -140,6 +140,6 @@ void vmm_init()
         cpu_sync_barrier(&vm->sync);
         vcpu_run(cpu()->vcpu);
     } else {
-        cpu_idle();
+        cpu_powerdown();
     }
 }

src/platform/qemu-aarch64-virt/inc/plat/psci.h

@@ -9,8 +9,9 @@
 #define PSCI_POWER_STATE_LVL_0    0x0000000
 #define PSCI_POWER_STATE_LVL_1    0x1000000
 #define PSCI_POWER_STATE_LVL_2    0x2000000
-#define PSCI_STATE_TYPE_STANDBY   0x00000
+#define PSCI_STATE_TYPE_STANDBY   0x1
 #define PSCI_STATE_TYPE_BIT       (1UL << 16)
-#define PSCI_STATE_TYPE_POWERDOWN (0x1)
+#define PSCI_STATE_ID_RETENTION   0x2
+#define PSCI_STATE_TYPE_POWERDOWN PSCI_STATE_TYPE_BIT | PSCI_STATE_ID_RETENTION
 
 #endif // __PLAT_PSCI_H__