| Message ID | 46324bcf24e72c919e444502ee684ab587e6ee34.1458420388.git.jcd@tribudubois.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19 March 2016 at 20:59, Jean-Christophe Dubois <jcd@tribudubois.net> wrote: > Split ARM on/off function from PSCI support code. > > This will allow to reuse these functions in other code. > > Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net> > --- > > Changes since V1: > * Not present on V1 > > Changes since V2: > * Not present on V2 > > Changes since V3: > * Move to a more generic/usefull API > * Manage CPU level/mode change at startup > * Allow PSCI to cope with EL2/HYP level. > * Keep distinct errors for different causes. > > target-arm/Makefile.objs | 1 + > target-arm/arm-powerctl.c | 224 ++++++++++++++++++++++++++++++++++++++++++++++ > target-arm/arm-powerctl.h | 44 +++++++++ > target-arm/psci.c | 69 ++------------ > 4 files changed, 275 insertions(+), 63 deletions(-) > create mode 100644 target-arm/arm-powerctl.c > create mode 100644 target-arm/arm-powerctl.h > > diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs > index a80eb39..60fd1dd 100644 > --- a/target-arm/Makefile.objs > +++ b/target-arm/Makefile.objs > @@ -9,3 +9,4 @@ obj-y += neon_helper.o iwmmxt_helper.o > obj-y += gdbstub.o > obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o helper-a64.o gdbstub64.o > obj-y += crypto_helper.o > +obj-y += arm-powerctl.o > diff --git a/target-arm/arm-powerctl.c b/target-arm/arm-powerctl.c > new file mode 100644 > index 0000000..8ebb3d8 > --- /dev/null > +++ b/target-arm/arm-powerctl.c > @@ -0,0 +1,224 @@ > +/* > + * QEMU support -- ARM Power Control specific functions. > + * > + * Copyright (c) 2016 Jean-Christophe Dubois > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#include "qemu/osdep.h" > +#include <cpu.h> > +#include <cpu-qom.h> > +#include "internals.h" > +#include "arm-powerctl.h" > + > +#ifndef DEBUG_ARM_POWERCTL > +#define DEBUG_ARM_POWERCTL 0 > +#endif > + > +#define DPRINTF(fmt, args...) \ > + do { \ > + if (DEBUG_ARM_POWERCTL) { \ > + fprintf(stderr, "[ARM]%s: " fmt , __func__, ##args); \ > + } \ > + } while (0) > + > +CPUState *arm_get_cpu_by_id(uint64_t id) > +{ > + CPUState *cpu; > + > + DPRINTF("cpu %" PRId64 "\n", id); > + > + CPU_FOREACH(cpu) { > + ARMCPU *armcpu = ARM_CPU(cpu); > + > + if (armcpu->mp_affinity == id) { > + return cpu; > + } > + } > + > + qemu_log_mask(LOG_GUEST_ERROR, > + "[ARM]%s: Requesting unknown CPU %" PRId64 "\n", > + __func__, id); > + > + return NULL; > +} > + > +int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id, > + int target_el, bool target_aa64) > +{ > + CPUState *target_cpu_state; > + ARMCPU *target_cpu; > + CPUClass *target_cpu_class; > + > + DPRINTF("cpu %" PRId64 " (EL %d, %s) @ 0x%" PRIx64 " with R0 = 0x%" PRIx64 "\n", > + cpuid, target_el, target_aa64 ? "aarch64" : "aarch32", entry, > + context_id); > + > + /* requested EL level need to be above 0 */ > + assert(target_el >= 1 && target_el <= 3); > + > + /* change to the cpu we are powering up */ > + target_cpu_state = arm_get_cpu_by_id(cpuid); > + if (!target_cpu_state) { > + /* The cpu was not found */ > + return QEMU_ARM_POWERCTL_INVALID_PARAM; > + } > + target_cpu = ARM_CPU(target_cpu_state); > + if (!target_cpu->powered_off) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "[ARM]%s: CPU %" PRId64 " is already running\n", > + __func__, cpuid); > + return QEMU_ARM_POWERCTL_ALREADY_ON; > + } > + target_cpu_class = CPU_GET_CLASS(target_cpu); > + > + /* Initialize the cpu we are turning on */ > + cpu_reset(target_cpu_state); > + target_cpu->powered_off = false; > + target_cpu_state->halted = 0; > + > + /* > + * The newly brought CPU is requested to enter the exception level > + * "target_el" and be in the requested mode (aarch64 ou aarch32). > + */ > + > + /* > + * Check that the CPU is supporting the requested level > + */ > + switch (target_el) { > + case 3: > + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)) { > + if (is_a64(&target_cpu->env)) { Why are we looking at the is_a64() state of the CPU as it comes out of reset? What we care about is target_aa64 here (if we are putting the CPU into a 64-bit state at a lower exception level then we must ensure the state is consistent with all the higher levels being 64-bit). > + /* Lower EL3 exception is aarch64 */ > + target_cpu->env.cp15.scr_el3 |= SCR_RW; > + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL2)) { > + /* Lower EL2 exception is aarch64 */ > + target_cpu->env.cp15.hcr_el2 |= HCR_RW; If you're starting in EL3 you don't need to mess with any of these registers, because they control register width for EL2 and EL1. You can let the guest do that itself. > + } > + target_cpu->env.pstate = PSTATE_MODE_EL3h; > + } else { > + switch_mode(&target_cpu->env, ARM_CPU_MODE_MON); switch_mode() is an internal function (and does some register bank copying we don't need); use cpsr_write(&target_cpu->env, ARM_CPU_MODE_MON, CPSR_M, CPSRWriteRaw); > + } > + /* Processor is in secure mode */ > + target_cpu->env.cp15.scr_el3 &= ~SCR_NS; > + } else { > + return QEMU_ARM_POWERCTL_INVALID_PARAM; > + } > + break; > + case 2: > + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL2)) { > + if (is_a64(&target_cpu->env)) { > + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)) { > + /* Lower EL3 exception is aarch64 */ > + target_cpu->env.cp15.scr_el3 |= SCR_RW; > + } > + /* Lower EL2 exception is aarch64 */ > + target_cpu->env.cp15.hcr_el2 |= HCR_RW; Again, to go into EL2 you don't need to worry about setting HCR_EL2.RW. > + target_cpu->env.pstate = PSTATE_MODE_EL2h; > + } else { > + switch_mode(&target_cpu->env, ARM_CPU_MODE_HYP); > + } > + /* Processor is not in secure mode */ > + target_cpu->env.cp15.scr_el3 |= SCR_NS; > + } else { > + return QEMU_ARM_POWERCTL_INVALID_PARAM; > + } > + break; > + case 1: > + default: > + if (is_a64(&target_cpu->env)) { > + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)) { > + /* Lower EL3 exception is aarch64 */ > + target_cpu->env.cp15.scr_el3 |= SCR_RW; > + } > + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL2)) { > + /* Lower EL2 exception is aarch64 */ > + target_cpu->env.cp15.hcr_el2 |= HCR_RW; > + } > + target_cpu->env.pstate = PSTATE_MODE_EL1h; > + } else { > + switch_mode(&target_cpu->env, ARM_CPU_MODE_SVC); > + } > + /* Processor is not in secure mode */ > + target_cpu->env.cp15.scr_el3 |= SCR_NS; > + break; > + } > + > + /* We assert if the request cannot be fulfilled */ > + assert(target_aa64 == arm_el_is_aa64(&target_cpu->env, target_el)); So, when can this happen, or is it genuinely "can't happen" ? > + > + /* We check if the started CPU is now in the correct level */ > + assert(target_el == arm_current_el(&target_cpu->env)); > + > + if (target_aa64) { > + /* Thumb32 is not supported on AARCH64 */ > + if (entry & 1) { > + return QEMU_ARM_POWERCTL_INVALID_PARAM; > + } If you're going to check the PC alignment for 64-bit then you might as well check that it's 4-aligned. > + target_cpu->env.xregs[0] = context_id; > + } else { > + target_cpu->env.regs[0] = context_id; > + target_cpu->env.thumb = entry & 1; > + } > + > + /* Start the new CPU at the requested address */ > + target_cpu_class->set_pc(target_cpu_state, entry); > + > + /* We are good to go */ > + return QEMU_ARM_POWERCTL_RET_SUCCESS; > +} > + > +void arm_set_cpu_off(uint64_t cpuid) > +{ > + CPUState *target_cpu_state; > + ARMCPU *target_cpu; > + > + DPRINTF("cpu %" PRId64 "\n", cpuid); > + > + /* change to the cpu we are powering up */ > + target_cpu_state = arm_get_cpu_by_id(cpuid); > + if (!target_cpu_state) { > + return; > + } > + target_cpu = ARM_CPU(target_cpu_state); > + if (target_cpu->powered_off) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "[ARM]%s: CPU %" PRId64 " is already off\n", > + __func__, cpuid); > + return; > + } > + > + target_cpu->powered_off = true; > + target_cpu_state->halted = 1; > + target_cpu_state->exception_index = EXCP_HLT; > + cpu_loop_exit(target_cpu_state); > +} > + > +int arm_reset_cpu(uint64_t cpuid) > +{ > + CPUState *target_cpu_state; > + ARMCPU *target_cpu; > + > + DPRINTF("cpu %" PRId64 "\n", cpuid); > + > + /* change to the cpu we are resetting */ > + target_cpu_state = arm_get_cpu_by_id(cpuid); > + if (!target_cpu_state) { > + return QEMU_ARM_POWERCTL_INVALID_PARAM; > + } > + target_cpu = ARM_CPU(target_cpu_state); > + if (target_cpu->powered_off) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "[ARM]%s: CPU %" PRId64 " is off\n", > + __func__, cpuid); > + return -1; > + } > + > + /* Reset the cpu */ > + cpu_reset(target_cpu_state); > + > + return QEMU_ARM_POWERCTL_RET_SUCCESS; > +} > diff --git a/target-arm/arm-powerctl.h b/target-arm/arm-powerctl.h > new file mode 100644 > index 0000000..2195483 > --- /dev/null > +++ b/target-arm/arm-powerctl.h > @@ -0,0 +1,44 @@ > +/* > + * QEMU support -- ARM Power Control specific functions. > + * > + * Copyright (c) 2016 Jean-Christophe Dubois > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#ifndef QEMU_ARM_POWERCTL_H > +#define QEMU_ARM_POWERCTL_H > + > +#include "kvm-consts.h" > + > +#define QEMU_ARM_POWERCTL_RET_SUCCESS QEMU_PSCI_RET_SUCCESS > +#define QEMU_ARM_POWERCTL_INVALID_PARAM QEMU_PSCI_RET_INVALID_PARAMS > +#define QEMU_ARM_POWERCTL_ALREADY_ON QEMU_PSCI_RET_ALREADY_ON > + > +/* > + * Retreive a CPUState object from its CPU ID > + */ > +CPUState *arm_get_cpu_by_id(uint64_t id); New global functions should all have properly formatted doc comments. > + > +/* > + * Start a give CPU. The CPU will start running at address "entry" with > + * "context_id" in r0/x0. The CPU should start at exception level "target_el" > + * and in mode aa64 or aa32 depending on "target_aa64" > + */ > +int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id, > + int target_el, bool target_aa64); What's the return value? Do we start the CPU secure or nonsecure? In AArch32, in which mode do we start? We need to either document these things or let the caller specify. (For PSCI we want to always start in non-secure, and we want to start in the same execution state as the calling cpu, which I take to include the mode. We won't ever try to start a cpu in EL3.) > + > +/* > + * Stop/Power off a given CPU > + */ > + > +void arm_set_cpu_off(uint64_t cpuid); > + > +/* > + * Reset a given CPU > + */ > +int arm_reset_cpu(uint64_t cpuid); > + > +#endif > diff --git a/target-arm/psci.c b/target-arm/psci.c > index c55487f..f1c8eb2 100644 > --- a/target-arm/psci.c > +++ b/target-arm/psci.c > @@ -22,6 +22,7 @@ > #include <kvm-consts.h> > #include <sysemu/sysemu.h> > #include "internals.h" > +#include "arm-powerctl.h" > > bool arm_is_psci_call(ARMCPU *cpu, int excp_type) > { > @@ -73,21 +74,6 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type) > } > } > > -static CPUState *get_cpu_by_id(uint64_t id) > -{ > - CPUState *cpu; > - > - CPU_FOREACH(cpu) { > - ARMCPU *armcpu = ARM_CPU(cpu); > - > - if (armcpu->mp_affinity == id) { > - return cpu; > - } > - } > - > - return NULL; > -} > - > void arm_handle_psci_call(ARMCPU *cpu) > { > /* > @@ -98,7 +84,6 @@ void arm_handle_psci_call(ARMCPU *cpu) > * Additional information about the calling convention used is available in > * the document 'SMC Calling Convention' (ARM DEN 0028) > */ > - CPUState *cs = CPU(cpu); > CPUARMState *env = &cpu->env; > uint64_t param[4]; > uint64_t context_id, mpidr; > @@ -123,7 +108,6 @@ void arm_handle_psci_call(ARMCPU *cpu) > switch (param[0]) { > CPUState *target_cpu_state; > ARMCPU *target_cpu; > - CPUClass *target_cpu_class; > > case QEMU_PSCI_0_2_FN_PSCI_VERSION: > ret = QEMU_PSCI_0_2_RET_VERSION_0_2; > @@ -137,7 +121,7 @@ void arm_handle_psci_call(ARMCPU *cpu) > > switch (param[2]) { > case 0: > - target_cpu_state = get_cpu_by_id(mpidr); > + target_cpu_state = arm_get_cpu_by_id(mpidr); > if (!target_cpu_state) { > ret = QEMU_PSCI_RET_INVALID_PARAMS; > break; > @@ -167,52 +151,14 @@ void arm_handle_psci_call(ARMCPU *cpu) > mpidr = param[1]; > entry = param[2]; > context_id = param[3]; > - > - /* change to the cpu we are powering up */ > - target_cpu_state = get_cpu_by_id(mpidr); > - if (!target_cpu_state) { > - ret = QEMU_PSCI_RET_INVALID_PARAMS; > - break; > - } > - target_cpu = ARM_CPU(target_cpu_state); > - if (!target_cpu->powered_off) { > - ret = QEMU_PSCI_RET_ALREADY_ON; > - break; > - } > - target_cpu_class = CPU_GET_CLASS(target_cpu); > - > - /* Initialize the cpu we are turning on */ > - cpu_reset(target_cpu_state); > - target_cpu->powered_off = false; > - target_cpu_state->halted = 0; > - > /* > * The PSCI spec mandates that newly brought up CPUs enter the > * exception level of the caller in the same execution mode as > * the caller, with context_id in x0/r0, respectively. > - * > - * For now, it is sufficient to assert() that CPUs come out of > - * reset in the same mode as the calling CPU, since we only > - * implement EL1, which means that > - * (a) there is no EL2 for the calling CPU to trap into to change > - * its state > - * (b) the newly brought up CPU enters EL1 immediately after coming > - * out of reset in the default state > */ > - assert(is_a64(env) == is_a64(&target_cpu->env)); > - if (is_a64(env)) { > - if (entry & 1) { > - ret = QEMU_PSCI_RET_INVALID_PARAMS; > - break; > - } > - target_cpu->env.xregs[0] = context_id; > - } else { > - target_cpu->env.regs[0] = context_id; > - target_cpu->env.thumb = entry & 1; > - } > - target_cpu_class->set_pc(target_cpu_state, entry); > - > - ret = 0; > + ret = arm_set_cpu_on(mpidr, entry, context_id, > + arm_current_el(&ARM_CPU(current_cpu)->env), > + is_a64(&ARM_CPU(current_cpu)->env)); You already have the current CPU's env in the 'env' variable; you don't need to go via current_cpu for this. > break; > case QEMU_PSCI_0_1_FN_CPU_OFF: > case QEMU_PSCI_0_2_FN_CPU_OFF: > @@ -250,9 +196,6 @@ err: > return; > > cpu_off: > - cpu->powered_off = true; > - cs->halted = 1; > - cs->exception_index = EXCP_HLT; > - cpu_loop_exit(cs); > + arm_set_cpu_off(cpu->mp_affinity); > /* notreached */ > } > -- > 2.5.0 thanks -- PMM
Le 23/03/2016 16:24, Peter Maydell a écrit : > On 19 March 2016 at 20:59, Jean-Christophe Dubois <jcd@tribudubois.net> wrote: >> Split ARM on/off function from PSCI support code. >> >> This will allow to reuse these functions in other code. >> >> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net> >> --- >> >> Changes since V1: >> * Not present on V1 >> >> Changes since V2: >> * Not present on V2 >> >> Changes since V3: >> * Move to a more generic/usefull API >> * Manage CPU level/mode change at startup >> * Allow PSCI to cope with EL2/HYP level. >> * Keep distinct errors for different causes. >> >> target-arm/Makefile.objs | 1 + >> target-arm/arm-powerctl.c | 224 ++++++++++++++++++++++++++++++++++++++++++++++ >> target-arm/arm-powerctl.h | 44 +++++++++ >> target-arm/psci.c | 69 ++------------ >> 4 files changed, 275 insertions(+), 63 deletions(-) >> create mode 100644 target-arm/arm-powerctl.c >> create mode 100644 target-arm/arm-powerctl.h >> >> diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs >> index a80eb39..60fd1dd 100644 >> --- a/target-arm/Makefile.objs >> +++ b/target-arm/Makefile.objs >> @@ -9,3 +9,4 @@ obj-y += neon_helper.o iwmmxt_helper.o >> obj-y += gdbstub.o >> obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o helper-a64.o gdbstub64.o >> obj-y += crypto_helper.o >> +obj-y += arm-powerctl.o >> diff --git a/target-arm/arm-powerctl.c b/target-arm/arm-powerctl.c >> new file mode 100644 >> index 0000000..8ebb3d8 >> --- /dev/null >> +++ b/target-arm/arm-powerctl.c >> @@ -0,0 +1,224 @@ >> +/* >> + * QEMU support -- ARM Power Control specific functions. >> + * >> + * Copyright (c) 2016 Jean-Christophe Dubois >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + * >> + */ >> + >> +#include "qemu/osdep.h" >> +#include <cpu.h> >> +#include <cpu-qom.h> >> +#include "internals.h" >> +#include "arm-powerctl.h" >> + >> +#ifndef DEBUG_ARM_POWERCTL >> +#define DEBUG_ARM_POWERCTL 0 >> +#endif >> + >> +#define DPRINTF(fmt, args...) \ >> + do { \ >> + if (DEBUG_ARM_POWERCTL) { \ >> + fprintf(stderr, "[ARM]%s: " fmt , __func__, ##args); \ >> + } \ >> + } while (0) >> + >> +CPUState *arm_get_cpu_by_id(uint64_t id) >> +{ >> + CPUState *cpu; >> + >> + DPRINTF("cpu %" PRId64 "\n", id); >> + >> + CPU_FOREACH(cpu) { >> + ARMCPU *armcpu = ARM_CPU(cpu); >> + >> + if (armcpu->mp_affinity == id) { >> + return cpu; >> + } >> + } >> + >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "[ARM]%s: Requesting unknown CPU %" PRId64 "\n", >> + __func__, id); >> + >> + return NULL; >> +} >> + >> +int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id, >> + int target_el, bool target_aa64) >> +{ >> + CPUState *target_cpu_state; >> + ARMCPU *target_cpu; >> + CPUClass *target_cpu_class; >> + >> + DPRINTF("cpu %" PRId64 " (EL %d, %s) @ 0x%" PRIx64 " with R0 = 0x%" PRIx64 "\n", >> + cpuid, target_el, target_aa64 ? "aarch64" : "aarch32", entry, >> + context_id); >> + >> + /* requested EL level need to be above 0 */ >> + assert(target_el >= 1 && target_el <= 3); >> + >> + /* change to the cpu we are powering up */ >> + target_cpu_state = arm_get_cpu_by_id(cpuid); >> + if (!target_cpu_state) { >> + /* The cpu was not found */ >> + return QEMU_ARM_POWERCTL_INVALID_PARAM; >> + } >> + target_cpu = ARM_CPU(target_cpu_state); >> + if (!target_cpu->powered_off) { >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "[ARM]%s: CPU %" PRId64 " is already running\n", >> + __func__, cpuid); >> + return QEMU_ARM_POWERCTL_ALREADY_ON; >> + } >> + target_cpu_class = CPU_GET_CLASS(target_cpu); >> + >> + /* Initialize the cpu we are turning on */ >> + cpu_reset(target_cpu_state); >> + target_cpu->powered_off = false; >> + target_cpu_state->halted = 0; >> + >> + /* >> + * The newly brought CPU is requested to enter the exception level >> + * "target_el" and be in the requested mode (aarch64 ou aarch32). >> + */ >> + >> + /* >> + * Check that the CPU is supporting the requested level >> + */ >> + switch (target_el) { >> + case 3: >> + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)) { >> + if (is_a64(&target_cpu->env)) { > Why are we looking at the is_a64() state of the CPU as it comes > out of reset? What we care about is target_aa64 here (if we > are putting the CPU into a 64-bit state at a lower exception > level then we must ensure the state is consistent with all > the higher levels being 64-bit). Because we are trying to switch the CPU to the desired level and we don't do it the same for aarch64 and aarch32. This is not about current mode but about the real architecture. Other functions like arm_is_secure() or arm_current_el() are doing the same. Even cpu_reset() is doing the same ... > >> + /* Lower EL3 exception is aarch64 */ >> + target_cpu->env.cp15.scr_el3 |= SCR_RW; >> + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL2)) { >> + /* Lower EL2 exception is aarch64 */ >> + target_cpu->env.cp15.hcr_el2 |= HCR_RW; > If you're starting in EL3 you don't need to mess with any of > these registers, because they control register width for > EL2 and EL1. You can let the guest do that itself. OK > >> + } >> + target_cpu->env.pstate = PSTATE_MODE_EL3h; >> + } else { >> + switch_mode(&target_cpu->env, ARM_CPU_MODE_MON); > switch_mode() is an internal function (and does some register > bank copying we don't need); use > cpsr_write(&target_cpu->env, ARM_CPU_MODE_MON, CPSR_M, CPSRWriteRaw); OK > >> + } >> + /* Processor is in secure mode */ >> + target_cpu->env.cp15.scr_el3 &= ~SCR_NS; >> + } else { >> + return QEMU_ARM_POWERCTL_INVALID_PARAM; >> + } >> + break; >> + case 2: >> + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL2)) { >> + if (is_a64(&target_cpu->env)) { >> + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)) { >> + /* Lower EL3 exception is aarch64 */ >> + target_cpu->env.cp15.scr_el3 |= SCR_RW; >> + } >> + /* Lower EL2 exception is aarch64 */ >> + target_cpu->env.cp15.hcr_el2 |= HCR_RW; > Again, to go into EL2 you don't need to worry about setting HCR_EL2.RW. OK > >> + target_cpu->env.pstate = PSTATE_MODE_EL2h; >> + } else { >> + switch_mode(&target_cpu->env, ARM_CPU_MODE_HYP); >> + } >> + /* Processor is not in secure mode */ >> + target_cpu->env.cp15.scr_el3 |= SCR_NS; >> + } else { >> + return QEMU_ARM_POWERCTL_INVALID_PARAM; >> + } >> + break; >> + case 1: >> + default: >> + if (is_a64(&target_cpu->env)) { >> + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)) { >> + /* Lower EL3 exception is aarch64 */ >> + target_cpu->env.cp15.scr_el3 |= SCR_RW; >> + } >> + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL2)) { >> + /* Lower EL2 exception is aarch64 */ >> + target_cpu->env.cp15.hcr_el2 |= HCR_RW; >> + } >> + target_cpu->env.pstate = PSTATE_MODE_EL1h; >> + } else { >> + switch_mode(&target_cpu->env, ARM_CPU_MODE_SVC); >> + } >> + /* Processor is not in secure mode */ >> + target_cpu->env.cp15.scr_el3 |= SCR_NS; >> + break; >> + } >> + >> + /* We assert if the request cannot be fulfilled */ >> + assert(target_aa64 == arm_el_is_aa64(&target_cpu->env, target_el)); > So, when can this happen, or is it genuinely "can't happen" ? Well, for now I don't force the mode of the desired level. So if we are asked to go EL1 in arch32 but by default the CORE foes EL1 in arch64 this will fail. > >> + >> + /* We check if the started CPU is now in the correct level */ >> + assert(target_el == arm_current_el(&target_cpu->env)); >> + >> + if (target_aa64) { >> + /* Thumb32 is not supported on AARCH64 */ >> + if (entry & 1) { >> + return QEMU_ARM_POWERCTL_INVALID_PARAM; >> + } > If you're going to check the PC alignment for 64-bit then you might > as well check that it's 4-aligned. OK, I'll change that. > >> + target_cpu->env.xregs[0] = context_id; >> + } else { >> + target_cpu->env.regs[0] = context_id; >> + target_cpu->env.thumb = entry & 1; >> + } >> + >> + /* Start the new CPU at the requested address */ >> + target_cpu_class->set_pc(target_cpu_state, entry); >> + >> + /* We are good to go */ >> + return QEMU_ARM_POWERCTL_RET_SUCCESS; >> +} >> + >> +void arm_set_cpu_off(uint64_t cpuid) >> +{ >> + CPUState *target_cpu_state; >> + ARMCPU *target_cpu; >> + >> + DPRINTF("cpu %" PRId64 "\n", cpuid); >> + >> + /* change to the cpu we are powering up */ >> + target_cpu_state = arm_get_cpu_by_id(cpuid); >> + if (!target_cpu_state) { >> + return; >> + } >> + target_cpu = ARM_CPU(target_cpu_state); >> + if (target_cpu->powered_off) { >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "[ARM]%s: CPU %" PRId64 " is already off\n", >> + __func__, cpuid); >> + return; >> + } >> + >> + target_cpu->powered_off = true; >> + target_cpu_state->halted = 1; >> + target_cpu_state->exception_index = EXCP_HLT; >> + cpu_loop_exit(target_cpu_state); >> +} >> + >> +int arm_reset_cpu(uint64_t cpuid) >> +{ >> + CPUState *target_cpu_state; >> + ARMCPU *target_cpu; >> + >> + DPRINTF("cpu %" PRId64 "\n", cpuid); >> + >> + /* change to the cpu we are resetting */ >> + target_cpu_state = arm_get_cpu_by_id(cpuid); >> + if (!target_cpu_state) { >> + return QEMU_ARM_POWERCTL_INVALID_PARAM; >> + } >> + target_cpu = ARM_CPU(target_cpu_state); >> + if (target_cpu->powered_off) { >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "[ARM]%s: CPU %" PRId64 " is off\n", >> + __func__, cpuid); >> + return -1; >> + } >> + >> + /* Reset the cpu */ >> + cpu_reset(target_cpu_state); >> + >> + return QEMU_ARM_POWERCTL_RET_SUCCESS; >> +} >> diff --git a/target-arm/arm-powerctl.h b/target-arm/arm-powerctl.h >> new file mode 100644 >> index 0000000..2195483 >> --- /dev/null >> +++ b/target-arm/arm-powerctl.h >> @@ -0,0 +1,44 @@ >> +/* >> + * QEMU support -- ARM Power Control specific functions. >> + * >> + * Copyright (c) 2016 Jean-Christophe Dubois >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + * >> + */ >> + >> +#ifndef QEMU_ARM_POWERCTL_H >> +#define QEMU_ARM_POWERCTL_H >> + >> +#include "kvm-consts.h" >> + >> +#define QEMU_ARM_POWERCTL_RET_SUCCESS QEMU_PSCI_RET_SUCCESS >> +#define QEMU_ARM_POWERCTL_INVALID_PARAM QEMU_PSCI_RET_INVALID_PARAMS >> +#define QEMU_ARM_POWERCTL_ALREADY_ON QEMU_PSCI_RET_ALREADY_ON >> + >> +/* >> + * Retreive a CPUState object from its CPU ID >> + */ >> +CPUState *arm_get_cpu_by_id(uint64_t id); > New global functions should all have properly formatted doc comments. I'll wok on it. Do you have a good citizen to point me to as a reference? > >> + >> +/* >> + * Start a give CPU. The CPU will start running at address "entry" with >> + * "context_id" in r0/x0. The CPU should start at exception level "target_el" >> + * and in mode aa64 or aa32 depending on "target_aa64" >> + */ >> +int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id, >> + int target_el, bool target_aa64); > What's the return value? Do we start the CPU secure or nonsecure? > In AArch32, in which mode do we start? We need to either document > these things or let the caller specify. The idea is to start the CPU in the requested level/mode (or fail if it is not possible). I'll document return value/error. > > (For PSCI we want to always start in non-secure, and we want to start > in the same execution state as the calling cpu, which I take to include > the mode. We won't ever try to start a cpu in EL3.) The PSCI call will take care of the requested level/mode. This is a more generic function that can be use for other purpose than PSCI. > >> + >> +/* >> + * Stop/Power off a given CPU >> + */ >> + >> +void arm_set_cpu_off(uint64_t cpuid); >> + >> +/* >> + * Reset a given CPU >> + */ >> +int arm_reset_cpu(uint64_t cpuid); >> + >> +#endif >> diff --git a/target-arm/psci.c b/target-arm/psci.c >> index c55487f..f1c8eb2 100644 >> --- a/target-arm/psci.c >> +++ b/target-arm/psci.c >> @@ -22,6 +22,7 @@ >> #include <kvm-consts.h> >> #include <sysemu/sysemu.h> >> #include "internals.h" >> +#include "arm-powerctl.h" >> >> bool arm_is_psci_call(ARMCPU *cpu, int excp_type) >> { >> @@ -73,21 +74,6 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type) >> } >> } >> >> -static CPUState *get_cpu_by_id(uint64_t id) >> -{ >> - CPUState *cpu; >> - >> - CPU_FOREACH(cpu) { >> - ARMCPU *armcpu = ARM_CPU(cpu); >> - >> - if (armcpu->mp_affinity == id) { >> - return cpu; >> - } >> - } >> - >> - return NULL; >> -} >> - >> void arm_handle_psci_call(ARMCPU *cpu) >> { >> /* >> @@ -98,7 +84,6 @@ void arm_handle_psci_call(ARMCPU *cpu) >> * Additional information about the calling convention used is available in >> * the document 'SMC Calling Convention' (ARM DEN 0028) >> */ >> - CPUState *cs = CPU(cpu); >> CPUARMState *env = &cpu->env; >> uint64_t param[4]; >> uint64_t context_id, mpidr; >> @@ -123,7 +108,6 @@ void arm_handle_psci_call(ARMCPU *cpu) >> switch (param[0]) { >> CPUState *target_cpu_state; >> ARMCPU *target_cpu; >> - CPUClass *target_cpu_class; >> >> case QEMU_PSCI_0_2_FN_PSCI_VERSION: >> ret = QEMU_PSCI_0_2_RET_VERSION_0_2; >> @@ -137,7 +121,7 @@ void arm_handle_psci_call(ARMCPU *cpu) >> >> switch (param[2]) { >> case 0: >> - target_cpu_state = get_cpu_by_id(mpidr); >> + target_cpu_state = arm_get_cpu_by_id(mpidr); >> if (!target_cpu_state) { >> ret = QEMU_PSCI_RET_INVALID_PARAMS; >> break; >> @@ -167,52 +151,14 @@ void arm_handle_psci_call(ARMCPU *cpu) >> mpidr = param[1]; >> entry = param[2]; >> context_id = param[3]; >> - >> - /* change to the cpu we are powering up */ >> - target_cpu_state = get_cpu_by_id(mpidr); >> - if (!target_cpu_state) { >> - ret = QEMU_PSCI_RET_INVALID_PARAMS; >> - break; >> - } >> - target_cpu = ARM_CPU(target_cpu_state); >> - if (!target_cpu->powered_off) { >> - ret = QEMU_PSCI_RET_ALREADY_ON; >> - break; >> - } >> - target_cpu_class = CPU_GET_CLASS(target_cpu); >> - >> - /* Initialize the cpu we are turning on */ >> - cpu_reset(target_cpu_state); >> - target_cpu->powered_off = false; >> - target_cpu_state->halted = 0; >> - >> /* >> * The PSCI spec mandates that newly brought up CPUs enter the >> * exception level of the caller in the same execution mode as >> * the caller, with context_id in x0/r0, respectively. >> - * >> - * For now, it is sufficient to assert() that CPUs come out of >> - * reset in the same mode as the calling CPU, since we only >> - * implement EL1, which means that >> - * (a) there is no EL2 for the calling CPU to trap into to change >> - * its state >> - * (b) the newly brought up CPU enters EL1 immediately after coming >> - * out of reset in the default state >> */ >> - assert(is_a64(env) == is_a64(&target_cpu->env)); >> - if (is_a64(env)) { >> - if (entry & 1) { >> - ret = QEMU_PSCI_RET_INVALID_PARAMS; >> - break; >> - } >> - target_cpu->env.xregs[0] = context_id; >> - } else { >> - target_cpu->env.regs[0] = context_id; >> - target_cpu->env.thumb = entry & 1; >> - } >> - target_cpu_class->set_pc(target_cpu_state, entry); >> - >> - ret = 0; >> + ret = arm_set_cpu_on(mpidr, entry, context_id, >> + arm_current_el(&ARM_CPU(current_cpu)->env), >> + is_a64(&ARM_CPU(current_cpu)->env)); > You already have the current CPU's env in the 'env' variable; > you don't need to go via current_cpu for this. OK > >> break; >> case QEMU_PSCI_0_1_FN_CPU_OFF: >> case QEMU_PSCI_0_2_FN_CPU_OFF: >> @@ -250,9 +196,6 @@ err: >> return; >> >> cpu_off: >> - cpu->powered_off = true; >> - cs->halted = 1; >> - cs->exception_index = EXCP_HLT; >> - cpu_loop_exit(cs); >> + arm_set_cpu_off(cpu->mp_affinity); >> /* notreached */ >> } >> -- >> 2.5.0 > thanks > -- PMM >
On 24 March 2016 at 07:10, Jean-Christophe DUBOIS <jcd@tribudubois.net> wrote: > Le 23/03/2016 16:24, Peter Maydell a écrit : >> >> On 19 March 2016 at 20:59, Jean-Christophe Dubois <jcd@tribudubois.net> >> wrote: >>> + >>> + /* >>> + * The newly brought CPU is requested to enter the exception level >>> + * "target_el" and be in the requested mode (aarch64 ou aarch32). >>> + */ >>> + >>> + /* >>> + * Check that the CPU is supporting the requested level >>> + */ >>> + switch (target_el) { >>> + case 3: >>> + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)) { >>> + if (is_a64(&target_cpu->env)) { >> >> Why are we looking at the is_a64() state of the CPU as it comes >> out of reset? What we care about is target_aa64 here (if we >> are putting the CPU into a 64-bit state at a lower exception >> level then we must ensure the state is consistent with all >> the higher levels being 64-bit). > > Because we are trying to switch the CPU to the desired level and we don't do > it the same for aarch64 and aarch32. This is not about current mode but > about the real architecture. Other functions like arm_is_secure() or > arm_current_el() are doing the same. Even cpu_reset() is doing the same ... No, this doesn't seem right to me. You're trying to set the SCR_EL3.RW and HCR_EL2.RW bits, and the condition for setting them is "are we trying to transition *to* an EL which is 64 bits". Whether the current EL is 64 bits or not is not relevant (and we know that it must be 64 bits because 64-bit capable CPUs always reset into 64-bits, assuming the caller didn't buggily ask us for a 64-bit state on a 32-bit CPU). arm_is_secure() and arm_current_el() are functions which are asking questions about the current state of a CPU, so obviously they look at the current state of the CPU. >>> + /* We assert if the request cannot be fulfilled */ >>> + assert(target_aa64 == arm_el_is_aa64(&target_cpu->env, target_el)); >> >> So, when can this happen, or is it genuinely "can't happen" ? > > > Well, for now I don't force the mode of the desired level. So if we are > asked to go EL1 in arch32 but by default the CORE foes EL1 in arch64 this > will fail. Oh, I see. That needs at least a TODO comment, because it's a missing feature that we'll need to implement at some point. (The current PSCI code doesn't do it, but the conditions it documents for why it can't do it aren't true any more -- we do now implement more than just EL1.) It might be better to check this early and return failure to the caller, since the guest can provoke it. >> New global functions should all have properly formatted doc comments. > > > I'll wok on it. Do you have a good citizen to point me to as a reference? I usually use the extract/deposit functions in include/qemu/bitops.h as a reference. >> >>> + >>> +/* >>> + * Start a give CPU. The CPU will start running at address "entry" with >>> + * "context_id" in r0/x0. The CPU should start at exception level >>> "target_el" >>> + * and in mode aa64 or aa32 depending on "target_aa64" >>> + */ >>> +int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id, >>> + int target_el, bool target_aa64); >> >> What's the return value? Do we start the CPU secure or nonsecure? >> In AArch32, in which mode do we start? We need to either document >> these things or let the caller specify. > > The idea is to start the CPU in the requested level/mode (or fail if it is > not possible). > > I'll document return value/error. >> >> >> (For PSCI we want to always start in non-secure, and we want to start >> in the same execution state as the calling cpu, which I take to include >> the mode. We won't ever try to start a cpu in EL3.) > > > The PSCI call will take care of the requested level/mode. > > This is a more generic function that can be use for other purpose than PSCI. Yes; I was just giving you the PSCI information to save you having to go look up the spec. The generic function needs to be able to at least provide sufficient functionality to implement the PSCI requirements. thanks -- PMM
----- Le 24 Mar 16, à 11:46, Peter Maydell peter.maydell@linaro.org a écrit : > On 24 March 2016 at 07:10, Jean-Christophe DUBOIS <jcd@tribudubois.net> wrote: > > Le 23/03/2016 16:24, Peter Maydell a écrit : > >> On 19 March 2016 at 20:59, Jean-Christophe Dubois <jcd@tribudubois.net> > >> wrote: > >>> + > >>> + /* > >>> + * The newly brought CPU is requested to enter the exception level > >>> + * "target_el" and be in the requested mode (aarch64 ou aarch32). > >>> + */ > >>> + > >>> + /* > >>> + * Check that the CPU is supporting the requested level > >>> + */ > >>> + switch (target_el) { > >>> + case 3: > >>> + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)) { > >>> + if (is_a64(&target_cpu->env)) { > >> Why are we looking at the is_a64() state of the CPU as it comes > >> out of reset? What we care about is target_aa64 here (if we > >> are putting the CPU into a 64-bit state at a lower exception > >> level then we must ensure the state is consistent with all > >> the higher levels being 64-bit). > > Because we are trying to switch the CPU to the desired level and we don't do > > it the same for aarch64 and aarch32. This is not about current mode but > > about the real architecture. Other functions like arm_is_secure() or > > arm_current_el() are doing the same. Even cpu_reset() is doing the same ... > No, this doesn't seem right to me. You're trying to set > the SCR_EL3.RW and HCR_EL2.RW bits, and the condition > for setting them is "are we trying to transition *to* > an EL which is 64 bits". Whether the current EL is 64 bits > or not is not relevant (and we know that it must be 64 bits > because 64-bit capable CPUs always reset into 64-bits, > assuming the caller didn't buggily ask us for a 64-bit > state on a 32-bit CPU). You mean that env->aarch64 will change dynamically depending on the CPU mode? > arm_is_secure() and arm_current_el() are functions which > are asking questions about the current state of a CPU, > so obviously they look at the current state of the CPU. > >>> + /* We assert if the request cannot be fulfilled */ > >>> + assert(target_aa64 == arm_el_is_aa64(&target_cpu->env, target_el)); > >> So, when can this happen, or is it genuinely "can't happen" ? > > Well, for now I don't force the mode of the desired level. So if we are > > asked to go EL1 in arch32 but by default the CORE foes EL1 in arch64 this > > will fail. > Oh, I see. That needs at least a TODO comment, because it's > a missing feature that we'll need to implement at some point. > (The current PSCI code doesn't do it, but the conditions it > documents for why it can't do it aren't true any more -- we > do now implement more than just EL1.) I was wondeing if I could influence on the capability to get the requested mode by tweaking target_cpu->env.cp15.scr_el3 and target_cpu->env.cp15.hcr_el2 > It might be better to check this early and return failure to the > caller, since the guest can provoke it. > >> New global functions should all have properly formatted doc comments. > > I'll wok on it. Do you have a good citizen to point me to as a reference? > I usually use the extract/deposit functions in include/qemu/bitops.h > as a reference. > >>> + > >>> +/* > >>> + * Start a give CPU. The CPU will start running at address "entry" with > >>> + * "context_id" in r0/x0. The CPU should start at exception level > >>> "target_el" > >>> + * and in mode aa64 or aa32 depending on "target_aa64" > >>> + */ > >>> +int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id, > >>> + int target_el, bool target_aa64); > >> What's the return value? Do we start the CPU secure or nonsecure? > >> In AArch32, in which mode do we start? We need to either document > >> these things or let the caller specify. > > The idea is to start the CPU in the requested level/mode (or fail if it is > > not possible). > > I'll document return value/error. > >> (For PSCI we want to always start in non-secure, and we want to start > >> in the same execution state as the calling cpu, which I take to include > >> the mode. We won't ever try to start a cpu in EL3.) > > The PSCI call will take care of the requested level/mode. > > This is a more generic function that can be use for other purpose than PSCI. > Yes; I was just giving you the PSCI information to save you having to > go look up the spec. The generic function needs to be able to at least > provide sufficient functionality to implement the PSCI requirements. > thanks > -- PMM
On 24 March 2016 at 13:54, <jcd@tribudubois.net> wrote: > > > ----- Le 24 Mar 16, à 11:46, Peter Maydell peter.maydell@linaro.org a écrit : > >> On 24 March 2016 at 07:10, Jean-Christophe DUBOIS <jcd@tribudubois.net> wrote: >> > Le 23/03/2016 16:24, Peter Maydell a écrit : > >> >> On 19 March 2016 at 20:59, Jean-Christophe Dubois <jcd@tribudubois.net> >> >> wrote: >> >>> + >> >>> + /* >> >>> + * The newly brought CPU is requested to enter the exception level >> >>> + * "target_el" and be in the requested mode (aarch64 ou aarch32). >> >>> + */ >> >>> + >> >>> + /* >> >>> + * Check that the CPU is supporting the requested level >> >>> + */ >> >>> + switch (target_el) { >> >>> + case 3: >> >>> + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)) { >> >>> + if (is_a64(&target_cpu->env)) { > >> >> Why are we looking at the is_a64() state of the CPU as it comes >> >> out of reset? What we care about is target_aa64 here (if we >> >> are putting the CPU into a 64-bit state at a lower exception >> >> level then we must ensure the state is consistent with all >> >> the higher levels being 64-bit). > >> > Because we are trying to switch the CPU to the desired level and we don't do >> > it the same for aarch64 and aarch32. This is not about current mode but >> > about the real architecture. Other functions like arm_is_secure() or >> > arm_current_el() are doing the same. Even cpu_reset() is doing the same ... > >> No, this doesn't seem right to me. You're trying to set >> the SCR_EL3.RW and HCR_EL2.RW bits, and the condition >> for setting them is "are we trying to transition *to* >> an EL which is 64 bits". Whether the current EL is 64 bits >> or not is not relevant (and we know that it must be 64 bits >> because 64-bit capable CPUs always reset into 64-bits, >> assuming the caller didn't buggily ask us for a 64-bit >> state on a 32-bit CPU). > > You mean that env->aarch64 will change dynamically depending on > the CPU mode? Yes, env->aarch64 tells you whether the CPU is currently in AArch32 or AArch64. (You should use is_a64() rather than directly looking at it, though.) If you want to know if the CPU supports AArch64 in general, you can call arm_feature(env, ARM_FEATURE_AARCH64). >> arm_is_secure() and arm_current_el() are functions which >> are asking questions about the current state of a CPU, >> so obviously they look at the current state of the CPU. > >> >>> + /* We assert if the request cannot be fulfilled */ >> >>> + assert(target_aa64 == arm_el_is_aa64(&target_cpu->env, target_el)); > >> >> So, when can this happen, or is it genuinely "can't happen" ? > > >> > Well, for now I don't force the mode of the desired level. So if we are >> > asked to go EL1 in arch32 but by default the CORE foes EL1 in arch64 this >> > will fail. > >> Oh, I see. That needs at least a TODO comment, because it's >> a missing feature that we'll need to implement at some point. >> (The current PSCI code doesn't do it, but the conditions it >> documents for why it can't do it aren't true any more -- we >> do now implement more than just EL1.) > > I was wondeing if I could influence on the capability to get the requested mode > by tweaking target_cpu->env.cp15.scr_el3 and target_cpu->env.cp15.hcr_el2 The RW bits in SCR_EL3 and HCR_EL2 are there only to define what the register width of lower exception levels are. If a guest tries an exception return asking for a different register width to what the system register say then this is an illegal return event. So since we're manually adjusting the state of the CPU to make it look as though we're in some lower exception level, we need to adjust the RW bits to be consistent with the fact that we're now in (say) AArch64 EL1; otherwise when the guest later does an exception return it will break. If you want to actually change the current mode then it's probably sufficient to set the SCR_EL3.RW and HCR_EL2.RW bits correctly and then set env->aarch64 to 0 for aarch32. You then need to make sure you write a full correct cpsr. This is getting complicated enough I'd rather leave it to a separate patch, though. thanks -- PMM
diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs index a80eb39..60fd1dd 100644 --- a/target-arm/Makefile.objs +++ b/target-arm/Makefile.objs @@ -9,3 +9,4 @@ obj-y += neon_helper.o iwmmxt_helper.o obj-y += gdbstub.o obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o helper-a64.o gdbstub64.o obj-y += crypto_helper.o +obj-y += arm-powerctl.o diff --git a/target-arm/arm-powerctl.c b/target-arm/arm-powerctl.c new file mode 100644 index 0000000..8ebb3d8 --- /dev/null +++ b/target-arm/arm-powerctl.c @@ -0,0 +1,224 @@ +/* + * QEMU support -- ARM Power Control specific functions. + * + * Copyright (c) 2016 Jean-Christophe Dubois + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ + +#include "qemu/osdep.h" +#include <cpu.h> +#include <cpu-qom.h> +#include "internals.h" +#include "arm-powerctl.h" + +#ifndef DEBUG_ARM_POWERCTL +#define DEBUG_ARM_POWERCTL 0 +#endif + +#define DPRINTF(fmt, args...) \ + do { \ + if (DEBUG_ARM_POWERCTL) { \ + fprintf(stderr, "[ARM]%s: " fmt , __func__, ##args); \ + } \ + } while (0) + +CPUState *arm_get_cpu_by_id(uint64_t id) +{ + CPUState *cpu; + + DPRINTF("cpu %" PRId64 "\n", id); + + CPU_FOREACH(cpu) { + ARMCPU *armcpu = ARM_CPU(cpu); + + if (armcpu->mp_affinity == id) { + return cpu; + } + } + + qemu_log_mask(LOG_GUEST_ERROR, + "[ARM]%s: Requesting unknown CPU %" PRId64 "\n", + __func__, id); + + return NULL; +} + +int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id, + int target_el, bool target_aa64) +{ + CPUState *target_cpu_state; + ARMCPU *target_cpu; + CPUClass *target_cpu_class; + + DPRINTF("cpu %" PRId64 " (EL %d, %s) @ 0x%" PRIx64 " with R0 = 0x%" PRIx64 "\n", + cpuid, target_el, target_aa64 ? "aarch64" : "aarch32", entry, + context_id); + + /* requested EL level need to be above 0 */ + assert(target_el >= 1 && target_el <= 3); + + /* change to the cpu we are powering up */ + target_cpu_state = arm_get_cpu_by_id(cpuid); + if (!target_cpu_state) { + /* The cpu was not found */ + return QEMU_ARM_POWERCTL_INVALID_PARAM; + } + target_cpu = ARM_CPU(target_cpu_state); + if (!target_cpu->powered_off) { + qemu_log_mask(LOG_GUEST_ERROR, + "[ARM]%s: CPU %" PRId64 " is already running\n", + __func__, cpuid); + return QEMU_ARM_POWERCTL_ALREADY_ON; + } + target_cpu_class = CPU_GET_CLASS(target_cpu); + + /* Initialize the cpu we are turning on */ + cpu_reset(target_cpu_state); + target_cpu->powered_off = false; + target_cpu_state->halted = 0; + + /* + * The newly brought CPU is requested to enter the exception level + * "target_el" and be in the requested mode (aarch64 ou aarch32). + */ + + /* + * Check that the CPU is supporting the requested level + */ + switch (target_el) { + case 3: + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)) { + if (is_a64(&target_cpu->env)) { + /* Lower EL3 exception is aarch64 */ + target_cpu->env.cp15.scr_el3 |= SCR_RW; + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL2)) { + /* Lower EL2 exception is aarch64 */ + target_cpu->env.cp15.hcr_el2 |= HCR_RW; + } + target_cpu->env.pstate = PSTATE_MODE_EL3h; + } else { + switch_mode(&target_cpu->env, ARM_CPU_MODE_MON); + } + /* Processor is in secure mode */ + target_cpu->env.cp15.scr_el3 &= ~SCR_NS; + } else { + return QEMU_ARM_POWERCTL_INVALID_PARAM; + } + break; + case 2: + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL2)) { + if (is_a64(&target_cpu->env)) { + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)) { + /* Lower EL3 exception is aarch64 */ + target_cpu->env.cp15.scr_el3 |= SCR_RW; + } + /* Lower EL2 exception is aarch64 */ + target_cpu->env.cp15.hcr_el2 |= HCR_RW; + target_cpu->env.pstate = PSTATE_MODE_EL2h; + } else { + switch_mode(&target_cpu->env, ARM_CPU_MODE_HYP); + } + /* Processor is not in secure mode */ + target_cpu->env.cp15.scr_el3 |= SCR_NS; + } else { + return QEMU_ARM_POWERCTL_INVALID_PARAM; + } + break; + case 1: + default: + if (is_a64(&target_cpu->env)) { + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL3)) { + /* Lower EL3 exception is aarch64 */ + target_cpu->env.cp15.scr_el3 |= SCR_RW; + } + if (arm_feature(&target_cpu->env, ARM_FEATURE_EL2)) { + /* Lower EL2 exception is aarch64 */ + target_cpu->env.cp15.hcr_el2 |= HCR_RW; + } + target_cpu->env.pstate = PSTATE_MODE_EL1h; + } else { + switch_mode(&target_cpu->env, ARM_CPU_MODE_SVC); + } + /* Processor is not in secure mode */ + target_cpu->env.cp15.scr_el3 |= SCR_NS; + break; + } + + /* We assert if the request cannot be fulfilled */ + assert(target_aa64 == arm_el_is_aa64(&target_cpu->env, target_el)); + + /* We check if the started CPU is now in the correct level */ + assert(target_el == arm_current_el(&target_cpu->env)); + + if (target_aa64) { + /* Thumb32 is not supported on AARCH64 */ + if (entry & 1) { + return QEMU_ARM_POWERCTL_INVALID_PARAM; + } + target_cpu->env.xregs[0] = context_id; + } else { + target_cpu->env.regs[0] = context_id; + target_cpu->env.thumb = entry & 1; + } + + /* Start the new CPU at the requested address */ + target_cpu_class->set_pc(target_cpu_state, entry); + + /* We are good to go */ + return QEMU_ARM_POWERCTL_RET_SUCCESS; +} + +void arm_set_cpu_off(uint64_t cpuid) +{ + CPUState *target_cpu_state; + ARMCPU *target_cpu; + + DPRINTF("cpu %" PRId64 "\n", cpuid); + + /* change to the cpu we are powering up */ + target_cpu_state = arm_get_cpu_by_id(cpuid); + if (!target_cpu_state) { + return; + } + target_cpu = ARM_CPU(target_cpu_state); + if (target_cpu->powered_off) { + qemu_log_mask(LOG_GUEST_ERROR, + "[ARM]%s: CPU %" PRId64 " is already off\n", + __func__, cpuid); + return; + } + + target_cpu->powered_off = true; + target_cpu_state->halted = 1; + target_cpu_state->exception_index = EXCP_HLT; + cpu_loop_exit(target_cpu_state); +} + +int arm_reset_cpu(uint64_t cpuid) +{ + CPUState *target_cpu_state; + ARMCPU *target_cpu; + + DPRINTF("cpu %" PRId64 "\n", cpuid); + + /* change to the cpu we are resetting */ + target_cpu_state = arm_get_cpu_by_id(cpuid); + if (!target_cpu_state) { + return QEMU_ARM_POWERCTL_INVALID_PARAM; + } + target_cpu = ARM_CPU(target_cpu_state); + if (target_cpu->powered_off) { + qemu_log_mask(LOG_GUEST_ERROR, + "[ARM]%s: CPU %" PRId64 " is off\n", + __func__, cpuid); + return -1; + } + + /* Reset the cpu */ + cpu_reset(target_cpu_state); + + return QEMU_ARM_POWERCTL_RET_SUCCESS; +} diff --git a/target-arm/arm-powerctl.h b/target-arm/arm-powerctl.h new file mode 100644 index 0000000..2195483 --- /dev/null +++ b/target-arm/arm-powerctl.h @@ -0,0 +1,44 @@ +/* + * QEMU support -- ARM Power Control specific functions. + * + * Copyright (c) 2016 Jean-Christophe Dubois + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ + +#ifndef QEMU_ARM_POWERCTL_H +#define QEMU_ARM_POWERCTL_H + +#include "kvm-consts.h" + +#define QEMU_ARM_POWERCTL_RET_SUCCESS QEMU_PSCI_RET_SUCCESS +#define QEMU_ARM_POWERCTL_INVALID_PARAM QEMU_PSCI_RET_INVALID_PARAMS +#define QEMU_ARM_POWERCTL_ALREADY_ON QEMU_PSCI_RET_ALREADY_ON + +/* + * Retreive a CPUState object from its CPU ID + */ +CPUState *arm_get_cpu_by_id(uint64_t id); + +/* + * Start a give CPU. The CPU will start running at address "entry" with + * "context_id" in r0/x0. The CPU should start at exception level "target_el" + * and in mode aa64 or aa32 depending on "target_aa64" + */ +int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id, + int target_el, bool target_aa64); + +/* + * Stop/Power off a given CPU + */ + +void arm_set_cpu_off(uint64_t cpuid); + +/* + * Reset a given CPU + */ +int arm_reset_cpu(uint64_t cpuid); + +#endif diff --git a/target-arm/psci.c b/target-arm/psci.c index c55487f..f1c8eb2 100644 --- a/target-arm/psci.c +++ b/target-arm/psci.c @@ -22,6 +22,7 @@ #include <kvm-consts.h> #include <sysemu/sysemu.h> #include "internals.h" +#include "arm-powerctl.h" bool arm_is_psci_call(ARMCPU *cpu, int excp_type) { @@ -73,21 +74,6 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type) } } -static CPUState *get_cpu_by_id(uint64_t id) -{ - CPUState *cpu; - - CPU_FOREACH(cpu) { - ARMCPU *armcpu = ARM_CPU(cpu); - - if (armcpu->mp_affinity == id) { - return cpu; - } - } - - return NULL; -} - void arm_handle_psci_call(ARMCPU *cpu) { /* @@ -98,7 +84,6 @@ void arm_handle_psci_call(ARMCPU *cpu) * Additional information about the calling convention used is available in * the document 'SMC Calling Convention' (ARM DEN 0028) */ - CPUState *cs = CPU(cpu); CPUARMState *env = &cpu->env; uint64_t param[4]; uint64_t context_id, mpidr; @@ -123,7 +108,6 @@ void arm_handle_psci_call(ARMCPU *cpu) switch (param[0]) { CPUState *target_cpu_state; ARMCPU *target_cpu; - CPUClass *target_cpu_class; case QEMU_PSCI_0_2_FN_PSCI_VERSION: ret = QEMU_PSCI_0_2_RET_VERSION_0_2; @@ -137,7 +121,7 @@ void arm_handle_psci_call(ARMCPU *cpu) switch (param[2]) { case 0: - target_cpu_state = get_cpu_by_id(mpidr); + target_cpu_state = arm_get_cpu_by_id(mpidr); if (!target_cpu_state) { ret = QEMU_PSCI_RET_INVALID_PARAMS; break; @@ -167,52 +151,14 @@ void arm_handle_psci_call(ARMCPU *cpu) mpidr = param[1]; entry = param[2]; context_id = param[3]; - - /* change to the cpu we are powering up */ - target_cpu_state = get_cpu_by_id(mpidr); - if (!target_cpu_state) { - ret = QEMU_PSCI_RET_INVALID_PARAMS; - break; - } - target_cpu = ARM_CPU(target_cpu_state); - if (!target_cpu->powered_off) { - ret = QEMU_PSCI_RET_ALREADY_ON; - break; - } - target_cpu_class = CPU_GET_CLASS(target_cpu); - - /* Initialize the cpu we are turning on */ - cpu_reset(target_cpu_state); - target_cpu->powered_off = false; - target_cpu_state->halted = 0; - /* * The PSCI spec mandates that newly brought up CPUs enter the * exception level of the caller in the same execution mode as * the caller, with context_id in x0/r0, respectively. - * - * For now, it is sufficient to assert() that CPUs come out of - * reset in the same mode as the calling CPU, since we only - * implement EL1, which means that - * (a) there is no EL2 for the calling CPU to trap into to change - * its state - * (b) the newly brought up CPU enters EL1 immediately after coming - * out of reset in the default state */ - assert(is_a64(env) == is_a64(&target_cpu->env)); - if (is_a64(env)) { - if (entry & 1) { - ret = QEMU_PSCI_RET_INVALID_PARAMS; - break; - } - target_cpu->env.xregs[0] = context_id; - } else { - target_cpu->env.regs[0] = context_id; - target_cpu->env.thumb = entry & 1; - } - target_cpu_class->set_pc(target_cpu_state, entry); - - ret = 0; + ret = arm_set_cpu_on(mpidr, entry, context_id, + arm_current_el(&ARM_CPU(current_cpu)->env), + is_a64(&ARM_CPU(current_cpu)->env)); break; case QEMU_PSCI_0_1_FN_CPU_OFF: case QEMU_PSCI_0_2_FN_CPU_OFF: @@ -250,9 +196,6 @@ err: return; cpu_off: - cpu->powered_off = true; - cs->halted = 1; - cs->exception_index = EXCP_HLT; - cpu_loop_exit(cs); + arm_set_cpu_off(cpu->mp_affinity); /* notreached */ }
Split ARM on/off function from PSCI support code. This will allow to reuse these functions in other code. Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net> --- Changes since V1: * Not present on V1 Changes since V2: * Not present on V2 Changes since V3: * Move to a more generic/usefull API * Manage CPU level/mode change at startup * Allow PSCI to cope with EL2/HYP level. * Keep distinct errors for different causes. target-arm/Makefile.objs | 1 + target-arm/arm-powerctl.c | 224 ++++++++++++++++++++++++++++++++++++++++++++++ target-arm/arm-powerctl.h | 44 +++++++++ target-arm/psci.c | 69 ++------------ 4 files changed, 275 insertions(+), 63 deletions(-) create mode 100644 target-arm/arm-powerctl.c create mode 100644 target-arm/arm-powerctl.h