From 6458446ead3d9ad5690fc22138344728847857f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Peixoto?= Date: Mon, 21 Oct 2024 14:55:12 +0100 Subject: [PATCH 1/2] fix(plat/qemu): psci powerdown and standby states MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: João Peixoto --- src/platform/qemu-aarch64-virt/inc/plat/psci.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/platform/qemu-aarch64-virt/inc/plat/psci.h b/src/platform/qemu-aarch64-virt/inc/plat/psci.h index 9f777b4fb..c1c908269 100644 --- a/src/platform/qemu-aarch64-virt/inc/plat/psci.h +++ b/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__ From 8adff2474cd9cabb4b61c1bf40482538571af909 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Peixoto?= Date: Fri, 1 Nov 2024 17:17:41 +0000 Subject: [PATCH 2/2] feat(core/cpu): decouple CPU standby from CPU power down MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decoupled the CPU idle state (now called standby) from the CPU power down state. This change allows future implementations requiring CPU standby — where the core may be needed shortly after an event — to avoid transitioning to a deeper power down state. This commit also removes the `PSCI_WAKEUP_IDLE` (or standby in this case) from `psci_wake_handlers` since the standby mode does not use the `entry_point_address` and `context_id` fields and, therefore, does not require a custom wake-up handler (e.g., psci_boot_entry). Signed-off-by: João Peixoto --- src/arch/armv8/armv8-a/cpu.c | 12 ++++++++++-- src/arch/armv8/armv8-a/psci.c | 10 ++-------- src/arch/armv8/armv8-r/cpu.c | 7 ++++++- src/arch/armv8/armv8-r/psci.c | 4 +--- src/arch/armv8/cpu.c | 25 ++++++++++++++++++++----- src/arch/armv8/inc/arch/cpu.h | 3 ++- src/arch/armv8/inc/arch/psci.h | 9 ++------- src/arch/armv8/psci.c | 4 ++-- src/arch/armv8/vm.c | 11 +---------- src/arch/riscv/cpu.c | 14 +++++++++++--- src/arch/riscv/vm.c | 8 ++------ src/core/cpu.c | 34 +++++++++++++++++++++++++++------- src/core/inc/cpu.h | 9 ++++++--- src/core/inc/vm.h | 2 +- src/core/vm.c | 10 +++++++--- src/core/vmm.c | 2 +- 16 files changed, 101 insertions(+), 63 deletions(-) diff --git a/src/arch/armv8/armv8-a/cpu.c b/src/arch/armv8/armv8-a/cpu.c index 227a8f7be..96c00d27f 100644 --- a/src/arch/armv8/armv8-a/cpu.c +++ b/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); } diff --git a/src/arch/armv8/armv8-a/psci.c b/src/arch/armv8/armv8-a/psci.c index b934f8f5c..6aa5cc7a6 100644 --- a/src/arch/armv8/armv8-a/psci.c +++ b/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); diff --git a/src/arch/armv8/armv8-r/cpu.c b/src/arch/armv8/armv8-r/cpu.c index 9a4320180..43a39b27b 100644 --- a/src/arch/armv8/armv8-r/cpu.c +++ b/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"); } diff --git a/src/arch/armv8/armv8-r/psci.c b/src/arch/armv8/armv8-r/psci.c index e4b1c64f4..dafc055f2 100644 --- a/src/arch/armv8/armv8-r/psci.c +++ b/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(); } diff --git a/src/arch/armv8/cpu.c b/src/arch/armv8/cpu.c index 7ab9d5b18..af13ffdab 100644 --- a/src/arch/armv8/cpu.c +++ b/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"); } diff --git a/src/arch/armv8/inc/arch/cpu.h b/src/arch/armv8/inc/arch/cpu.h index 3c8040ca0..23f51c4b0 100644 --- a/src/arch/armv8/inc/arch/cpu.h +++ b/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; diff --git a/src/arch/armv8/inc/arch/psci.h b/src/arch/armv8/inc/arch/psci.h index 29e5ad1d6..684aca47c 100644 --- a/src/arch/armv8/inc/arch/psci.h +++ b/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); diff --git a/src/arch/armv8/psci.c b/src/arch/armv8/psci.c index e326086ab..4b733e2f9 100644 --- a/src/arch/armv8/psci.c +++ b/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; diff --git a/src/arch/armv8/vm.c b/src/arch/armv8/vm.c index 84cd2f5e9..8ec0786e3 100644 --- a/src/arch/armv8/vm.c +++ b/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(); - } -} diff --git a/src/arch/riscv/cpu.c b/src/arch/riscv/cpu.c index 75e8dd60d..089deb227 100644 --- a/src/arch/riscv/cpu.c +++ b/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"); } diff --git a/src/arch/riscv/vm.c b/src/arch/riscv/vm.c index e3123f0c3..1bf84c8c3 100644 --- a/src/arch/riscv/vm.c +++ b/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; } diff --git a/src/core/cpu.c b/src/core/cpu.c index 29c25fb5e..a4c7a50be 100644 --- a/src/core/cpu.c +++ b/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(); } } diff --git a/src/core/inc/cpu.h b/src/core/inc/cpu.h index 70c915336..c4572b74e 100644 --- a/src/core/inc/cpu.h +++ b/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) diff --git a/src/core/inc/vm.h b/src/core/inc/vm.h index fc0ef565c..dd8dcaa20 100644 --- a/src/core/inc/vm.h +++ b/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__ */ diff --git a/src/core/vm.c b/src/core/vm.c index e9441413a..553aa211c 100644 --- a/src/core/vm.c +++ b/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(); } } diff --git a/src/core/vmm.c b/src/core/vmm.c index 41257fbc5..327378893 100644 --- a/src/core/vmm.c +++ b/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(); } }