| Message ID | 1406790023-13687-1-git-send-email-parth.dixit@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Thu, 2014-07-31 at 12:30 +0530, Parth Dixit wrote: > + /* affinity values are ignored in this implementation as > + * at present xen does not supports affinity level greater > + * than 0, for all affinity values passed we power down/ standby > + * the current core */ > + if( power_state & PSCI_0_2_POWER_STATE_TYPE_MASK ) > + { > + if ( is_32bit_domain(v->domain) ) > + regs->r0 = context_id; > +#ifdef CONFIG_ARM_64 > + else > + regs->x0 = context_id; > +#endif > + } > + > + vcpu_block_unless_event_pending(v); > + return PSCI_SUCCESS; I'm afraid this is still wrong (well, actually it is wrong but also buggy such that it actually ends up doing the right thing for the wrong reason...). You must do one of two things. Either return to the instruction after the SMC with x0==PSCI_SUCCESS *or* jump to entry_point with x0==context id. Here you are apparently trying to implement something which is neither of these by setting x0==context_id but returning to the instruction after the SMC. It is buggy though because the "return PSCI_SUCCESS" will overwrite your x0 setting. Since Xen doesn't actually do any low power state what we actually want is to return PSCI_SUCCESS to the instruction after the smc in every case, which due to the bug in the above is actually what you have implemented. So, the entire power_state block if block is redundant. This function can just be: +register_t do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point, + register_t context_id) +{ + vcpu_block_unless_event_pending(current); + return PSCI_SUCCESS; +} Does this make sense? Ian.
On 31 July 2014 14:43, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Thu, 2014-07-31 at 12:30 +0530, Parth Dixit wrote: > >> + /* affinity values are ignored in this implementation as >> + * at present xen does not supports affinity level greater >> + * than 0, for all affinity values passed we power down/ standby >> + * the current core */ >> + if( power_state & PSCI_0_2_POWER_STATE_TYPE_MASK ) >> + { >> + if ( is_32bit_domain(v->domain) ) >> + regs->r0 = context_id; >> +#ifdef CONFIG_ARM_64 >> + else >> + regs->x0 = context_id; >> +#endif >> + } >> + >> + vcpu_block_unless_event_pending(v); >> + return PSCI_SUCCESS; > > I'm afraid this is still wrong (well, actually it is wrong but also > buggy such that it actually ends up doing the right thing for the wrong > reason...). > > You must do one of two things. Either return to the instruction after > the SMC with x0==PSCI_SUCCESS *or* jump to entry_point with x0==context > id. > > Here you are apparently trying to implement something which is neither > of these by setting x0==context_id but returning to the instruction > after the SMC. It is buggy though because the "return PSCI_SUCCESS" will > overwrite your x0 setting. > > Since Xen doesn't actually do any low power state what we actually want > is to return PSCI_SUCCESS to the instruction after the smc in every > case, which due to the bug in the above is actually what you have > implemented. > > So, the entire power_state block if block is redundant. This function > can just be: > > +register_t do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point, > + register_t context_id) > +{ > + vcpu_block_unless_event_pending(current); > + return PSCI_SUCCESS; > +} > > Does this make sense? > > Ian. > ok, but with this we are ignoring the power_state type and defaulting always to standby (not retaining context_id) which should be ok because caller requests deepest possible state that it can tolerate and it is possible to return with shallower state. Also this makes our implementation inline with kvm. I will upload it in patchset 9
On Thu, Jul 31, 2014 at 12:30:23PM +0530, Parth Dixit wrote: > Arm based virtual machines dom0/guest will request power related functionality > from xen through PSCI interface. This patch implements version 0.2 of > PSCI standard specified by arm for 64bit and 32 bit arm machines. > > - removed arm_psci_fn_t > - implemented psci_cpu_on with additional error conditions > - added switch-case in do_trap_psci function > - added PSCI v0.2 macros in psci.h > - removed tables for PSCI v0.1 and v0.2 > - implemented affinity_info > - moved do_common_cpu up in vpsci.c removed declaration > - removed PSCI_ARGS macro > - added new macro for 32 bit arguments > - added new function psci_mode_check > - modified cpu_suspend to return context_id > - added macros for checking powe_state > > Signed-off-by: Parth Dixit <parth.dixit@linaro.org> > --- > Changelog v8 > - fixed error in cpu_suspend > Changelog v7 > - removed casting from functions in do_psci_trap > - modified cpu_suspend to return success in case the call returns > > Changelog v6 > - fixed indentation > - removed casting from error codes > - modified cpu_suspend to check power_state > - ignoring context_id in case of standby > - added comments for ignoring affinity > - added macro for checking power_state > > Changelog v5 > - removed PSCI_ARGS macro > - preload error value for calls that do not return > - added new macro for 32 bit arguments > - casting return calls to appropriate types > - added new function psci_mode_check > - added error checks for PSCI v0.2 function id's > > Changelog v4 > - added target_affinity_mask[] to local variable > - removed AFF3 masking for 64 bit cpu > - added do_common_cpu_on for PSCI v0.1 and v0.2 > - moved cpu_on decision based on entry point > - removed vpsci_ver introduced in v3 > - removed psci tables > - added new macro for selecting arguments > - do_trap_psci is now switch case based instead of tables > > Changelog v3 > - moved wfi helper to seperate patch > - replaced new wfi function in traps.c > - removed defining power_state variable using value directly > - added new line between declaration > - merged PSCI v0.1 and v0.2 cpu_on function to avoid duplication > - removed spurious change > - renamed PSCI return values to reflect v0.2 moved them to top > - removed PSCI v0.1 version declaration for now, will introduce it when needed > - removed hard tabs in the code lifted from linux > - removed PSCI_0_1_MAX > - did not retained copyright header from linux as most of functions are removed > - added two function tables for PSCI v0.1 and PSCI v0.2 > - added compatibility string to libxl_arm to expose new functionality > - refactored affinity_info code > - added table for masking function > - removed definitions of unused PSCI v0.2 functions > - removed function id's of unused PSCI v0.2 functions > - renamed constant PSCI_0_2_TOS_MP ( multicore aware) as per spec > > tools/libxl/libxl_arm.c | 2 +- > xen/arch/arm/domain_build.c | 5 +- > xen/arch/arm/traps.c | 123 +++++++++++++++++++++++++---------- > xen/arch/arm/vpsci.c | 140 ++++++++++++++++++++++++++++++++++++++-- > xen/include/asm-arm/processor.h | 3 + > xen/include/asm-arm/psci.h | 83 +++++++++++++++++++++--- > 6 files changed, 308 insertions(+), 48 deletions(-) > > diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c > index 4f0f0e2..e8bcd05 100644 > --- a/tools/libxl/libxl_arm.c > +++ b/tools/libxl/libxl_arm.c > @@ -237,7 +237,7 @@ static int make_psci_node(libxl__gc *gc, void *fdt) > res = fdt_begin_node(fdt, "psci"); > if (res) return res; > > - res = fdt_property_compat(gc, fdt, 1, "arm,psci"); > + res = fdt_property_compat(gc, fdt, 2, "arm,psci-0.2","arm,psci"); > if (res) return res; > > res = fdt_property_string(fdt, "method", "hvc"); > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index c424793..ebd4170 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -388,6 +388,9 @@ static int make_hypervisor_node(struct domain *d, > static int make_psci_node(void *fdt, const struct dt_device_node *parent) > { > int res; > + const char compat[] = > + "arm,psci-0.2""\0" > + "arm,psci"; > > DPRINT("Create PSCI node\n"); > > @@ -396,7 +399,7 @@ static int make_psci_node(void *fdt, const struct dt_device_node *parent) > if ( res ) > return res; > > - res = fdt_property_string(fdt, "compatible", "arm,psci"); > + res = fdt_property(fdt, "compatible", compat, sizeof(compat)); > if ( res ) > return res; > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > index 8102540..844da9d 100644 > --- a/xen/arch/arm/traps.c > +++ b/xen/arch/arm/traps.c > @@ -1030,24 +1030,6 @@ static arm_hypercall_t arm_hypercall_table[] = { > HYPERCALL_ARM(vcpu_op, 3), > }; > > -typedef int (*arm_psci_fn_t)(uint32_t, register_t); > - > -typedef struct { > - arm_psci_fn_t fn; > - int nr_args; > -} arm_psci_t; > - > -#define PSCI(_name, _nr_args) \ > - [ PSCI_ ## _name ] = { \ > - .fn = (arm_psci_fn_t) &do_psci_ ## _name, \ > - .nr_args = _nr_args, \ > - } > - > -static arm_psci_t arm_psci_table[] = { > - PSCI(cpu_off, 1), > - PSCI(cpu_on, 2), > -}; > - > #ifndef NDEBUG > static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code) > { > @@ -1080,33 +1062,108 @@ static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code) > #endif > > #ifdef CONFIG_ARM_64 > -#define PSCI_OP_REG(r) (r)->x0 > #define PSCI_RESULT_REG(r) (r)->x0 > -#define PSCI_ARGS(r) (r)->x1, (r)->x2 > +#define PSCI_ARG(r,n) (r)->x##n > +#define PSCI_ARG32(r,n) (uint32_t)( (r)->x##n & 0x00000000FFFFFFFF ) > #else > -#define PSCI_OP_REG(r) (r)->r0 > #define PSCI_RESULT_REG(r) (r)->r0 > -#define PSCI_ARGS(r) (r)->r1, (r)->r2 > +#define PSCI_ARG(r,n) (r)->r##n > +#define PSCI_ARG32(r,n) PSCI_ARG(r,n) > #endif > > -static void do_trap_psci(struct cpu_user_regs *regs) > +/* helper function for checking arm mode 32/64 bit */ > +static inline int psci_mode_check(struct domain *d, register_t fid) > { > - arm_psci_fn_t psci_call = NULL; > + return !( is_64bit_domain(d)^( (fid & PSCI_0_2_64BIT) >> 30 ) ); > +} > > - if ( PSCI_OP_REG(regs) >= ARRAY_SIZE(arm_psci_table) ) > - { > - domain_crash_synchronous(); > - return; > - } > +static void do_trap_psci(struct cpu_user_regs *regs) > +{ > + register_t fid = PSCI_ARG(regs,0); > > - psci_call = arm_psci_table[PSCI_OP_REG(regs)].fn; > - if ( psci_call == NULL ) > + /* preloading in case psci_mode_check fails */ > + PSCI_RESULT_REG(regs) = PSCI_INVALID_PARAMETERS; > + switch( fid ) > { > + case PSCI_cpu_off: > + { > + uint32_t pstate = PSCI_ARG32(regs,1); > + PSCI_RESULT_REG(regs) = do_psci_cpu_off(pstate); > + } > + break; > + case PSCI_cpu_on: > + { > + uint32_t vcpuid = PSCI_ARG32(regs,1); > + register_t epoint = PSCI_ARG(regs,2); > + PSCI_RESULT_REG(regs) = do_psci_cpu_on(vcpuid, epoint); > + } > + break; > + case PSCI_0_2_FN_PSCI_VERSION: > + PSCI_RESULT_REG(regs) = do_psci_0_2_version(); > + break; > + case PSCI_0_2_FN_CPU_OFF: > + PSCI_RESULT_REG(regs) = do_psci_0_2_cpu_off(); > + break; > + case PSCI_0_2_FN_MIGRATE_INFO_TYPE: > + PSCI_RESULT_REG(regs) = do_psci_0_2_migrate_info_type(); > + break; > + case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU: > + case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU: > + if ( psci_mode_check(current->domain, fid) ) > + PSCI_RESULT_REG(regs) = do_psci_0_2_migrate_info_up_cpu(); > + break; > + case PSCI_0_2_FN_SYSTEM_OFF: > + do_psci_0_2_system_off(); > + PSCI_RESULT_REG(regs) = PSCI_INTERNAL_FAILURE; > + break; > + case PSCI_0_2_FN_SYSTEM_RESET: > + do_psci_0_2_system_reset(); > + PSCI_RESULT_REG(regs) = PSCI_INTERNAL_FAILURE; > + break; > + case PSCI_0_2_FN_CPU_ON: > + case PSCI_0_2_FN64_CPU_ON: > + if ( psci_mode_check(current->domain, fid) ) > + { > + register_t vcpuid = PSCI_ARG(regs,1); > + register_t epoint = PSCI_ARG(regs,2); > + register_t cid = PSCI_ARG(regs,3); > + PSCI_RESULT_REG(regs) = > + do_psci_0_2_cpu_on(vcpuid, epoint, cid); > + } > + break; > + case PSCI_0_2_FN_CPU_SUSPEND: > + case PSCI_0_2_FN64_CPU_SUSPEND: > + if ( psci_mode_check(current->domain, fid) ) > + { > + uint32_t pstate = PSCI_ARG32(regs,1); > + register_t epoint = PSCI_ARG(regs,2); > + register_t cid = PSCI_ARG(regs,3); > + PSCI_RESULT_REG(regs) = > + do_psci_0_2_cpu_suspend(pstate, epoint, cid); > + } > + break; > + case PSCI_0_2_FN_AFFINITY_INFO: > + case PSCI_0_2_FN64_AFFINITY_INFO: > + if ( psci_mode_check(current->domain, fid) ) > + { > + register_t taff = PSCI_ARG(regs,1); > + uint32_t laff = PSCI_ARG32(regs,2); > + PSCI_RESULT_REG(regs) = > + do_psci_0_2_affinity_info(taff, laff); > + } > + break; > + case PSCI_0_2_FN_MIGRATE: > + case PSCI_0_2_FN64_MIGRATE: > + if ( psci_mode_check(current->domain, fid) ) > + { > + uint32_t tcpu = PSCI_ARG32(regs,1); > + PSCI_RESULT_REG(regs) = do_psci_0_2_migrate(tcpu); > + } > + break; > + default: > domain_crash_synchronous(); > return; > } > - > - PSCI_RESULT_REG(regs) = psci_call(PSCI_ARGS(regs)); > } > > #ifdef CONFIG_ARM_64 > diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c > index 1ceb8cb..c6c1bdc 100644 > --- a/xen/arch/arm/vpsci.c > +++ b/xen/arch/arm/vpsci.c > @@ -17,24 +17,37 @@ > #include <asm/current.h> > #include <asm/gic.h> > #include <asm/psci.h> > +#include <public/sched.h> > +#include <asm-arm/event.h> > > -int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) > +static int do_common_cpu_on(register_t target_cpu, register_t entry_point, > + register_t context_id,int ver) > { > struct vcpu *v; > struct domain *d = current->domain; > struct vcpu_guest_context *ctxt; > int rc; > int is_thumb = entry_point & 1; > + register_t vcpuid; > + > + if( ver == XEN_PSCI_V_0_2 ) > + vcpuid = (target_cpu & MPIDR_HWID_MASK); > + else > + vcpuid = target_cpu; > > if ( (vcpuid < 0) || (vcpuid >= MAX_VIRT_CPUS) ) > - return PSCI_EINVAL; > + return PSCI_INVALID_PARAMETERS; > > if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL ) > - return PSCI_EINVAL; > + return PSCI_INVALID_PARAMETERS; > > /* THUMB set is not allowed with 64-bit domain */ > if ( is_64bit_domain(d) && is_thumb ) > - return PSCI_EINVAL; > + return PSCI_INVALID_PARAMETERS; > + > + if( ( ver == XEN_PSCI_V_0_2 ) && > + ( !test_bit(_VPF_down, &v->pause_flags) ) ) > + return PSCI_ALREADY_ON; > > if ( (ctxt = alloc_vcpu_guest_context()) == NULL ) > return PSCI_DENIED; > @@ -48,10 +61,18 @@ int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) > ctxt->ttbr1 = 0; > ctxt->ttbcr = 0; /* Defined Reset Value */ > if ( is_32bit_domain(d) ) > + { > ctxt->user_regs.cpsr = PSR_GUEST32_INIT; > + if( ver == XEN_PSCI_V_0_2 ) > + ctxt->user_regs.r0_usr = context_id; > + } > #ifdef CONFIG_ARM_64 > else > + { > ctxt->user_regs.cpsr = PSR_GUEST64_INIT; > + if( ver == XEN_PSCI_V_0_2 ) > + ctxt->user_regs.x0 = context_id; > + } > #endif > > /* Start the VCPU with THUMB set if it's requested by the kernel */ > @@ -75,7 +96,12 @@ int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) > return PSCI_SUCCESS; > } > > -int do_psci_cpu_off(uint32_t power_state) > +int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) > +{ > + return do_common_cpu_on(vcpuid,entry_point,0,XEN_PSCI_V_0_1); > +} > + > +int32_t do_psci_cpu_off(uint32_t power_state) > { > struct vcpu *v = current; > if ( !test_and_set_bit(_VPF_down, &v->pause_flags) ) > @@ -83,6 +109,110 @@ int do_psci_cpu_off(uint32_t power_state) > return PSCI_SUCCESS; > } > > +uint32_t do_psci_0_2_version(void) > +{ > + return XEN_PSCI_V_0_2; > +} > + > +register_t do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point, > + register_t context_id) > +{ > + struct vcpu *v = current; > + struct cpu_user_regs *regs = guest_cpu_user_regs(); > + Not sure you want to keep this comment around (as I would comment on the fact that we always treat power off requests as only performing standby as it simplifies the Xen implementation (as Ian points out in his comments), but if you do want to keep the comment block: > + /* affinity values are ignored in this implementation as Not sure of the Xen common practice here, but why not start this with upper case? > + * at present xen does not supports affinity level greater does not support affinity levels greater > + * than 0, for all affinity values passed we power down/ standby ^ extra white space missing space ^ > + * the current core */ only the current core. (?) [...] -Christoffer
On Thu, 2014-07-31 at 15:29 +0530, Parth Dixit wrote: > On 31 July 2014 14:43, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Thu, 2014-07-31 at 12:30 +0530, Parth Dixit wrote: > > > >> + /* affinity values are ignored in this implementation as > >> + * at present xen does not supports affinity level greater > >> + * than 0, for all affinity values passed we power down/ standby > >> + * the current core */ > >> + if( power_state & PSCI_0_2_POWER_STATE_TYPE_MASK ) > >> + { > >> + if ( is_32bit_domain(v->domain) ) > >> + regs->r0 = context_id; > >> +#ifdef CONFIG_ARM_64 > >> + else > >> + regs->x0 = context_id; > >> +#endif > >> + } > >> + > >> + vcpu_block_unless_event_pending(v); > >> + return PSCI_SUCCESS; > > > > I'm afraid this is still wrong (well, actually it is wrong but also > > buggy such that it actually ends up doing the right thing for the wrong > > reason...). > > > > You must do one of two things. Either return to the instruction after > > the SMC with x0==PSCI_SUCCESS *or* jump to entry_point with x0==context > > id. > > > > Here you are apparently trying to implement something which is neither > > of these by setting x0==context_id but returning to the instruction > > after the SMC. It is buggy though because the "return PSCI_SUCCESS" will > > overwrite your x0 setting. > > > > Since Xen doesn't actually do any low power state what we actually want > > is to return PSCI_SUCCESS to the instruction after the smc in every > > case, which due to the bug in the above is actually what you have > > implemented. > > > > So, the entire power_state block if block is redundant. This function > > can just be: > > > > +register_t do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point, > > + register_t context_id) > > +{ > > + vcpu_block_unless_event_pending(current); > > + return PSCI_SUCCESS; > > +} > > > > Does this make sense? > > > > Ian. > > > ok, but with this we are ignoring the power_state type and defaulting > always to standby (not retaining context_id) which should be ok > because caller requests deepest possible state that it can tolerate > and it is possible to return with shallower state. Also this makes our > implementation inline with kvm. ARM says this is OK, it is valid for a request to go to low power to not be able to do so (pending events, etc) and instead return PSCI_SUCCESS. ARM also pointed out that since we do not expose any tables with state ids (there are no DTB bindings for this, and ACPI isn't here yet) no OSPM should ever be calling us in this mode. > I will upload it in patchset 9 Thanks, Ian.
Hi, On 31 July 2014 15:44, Christoffer Dall <christoffer.dall@linaro.org> wrote: > On Thu, Jul 31, 2014 at 12:30:23PM +0530, Parth Dixit wrote: >> Arm based virtual machines dom0/guest will request power related functionality >> from xen through PSCI interface. This patch implements version 0.2 of >> PSCI standard specified by arm for 64bit and 32 bit arm machines. >> >> - removed arm_psci_fn_t >> - implemented psci_cpu_on with additional error conditions >> - added switch-case in do_trap_psci function >> - added PSCI v0.2 macros in psci.h >> - removed tables for PSCI v0.1 and v0.2 >> - implemented affinity_info >> - moved do_common_cpu up in vpsci.c removed declaration >> - removed PSCI_ARGS macro >> - added new macro for 32 bit arguments >> - added new function psci_mode_check >> - modified cpu_suspend to return context_id >> - added macros for checking powe_state >> >> Signed-off-by: Parth Dixit <parth.dixit@linaro.org> >> --- >> Changelog v8 >> - fixed error in cpu_suspend >> Changelog v7 >> - removed casting from functions in do_psci_trap >> - modified cpu_suspend to return success in case the call returns >> >> Changelog v6 >> - fixed indentation >> - removed casting from error codes >> - modified cpu_suspend to check power_state >> - ignoring context_id in case of standby >> - added comments for ignoring affinity >> - added macro for checking power_state >> >> Changelog v5 >> - removed PSCI_ARGS macro >> - preload error value for calls that do not return >> - added new macro for 32 bit arguments >> - casting return calls to appropriate types >> - added new function psci_mode_check >> - added error checks for PSCI v0.2 function id's >> >> Changelog v4 >> - added target_affinity_mask[] to local variable >> - removed AFF3 masking for 64 bit cpu >> - added do_common_cpu_on for PSCI v0.1 and v0.2 >> - moved cpu_on decision based on entry point >> - removed vpsci_ver introduced in v3 >> - removed psci tables >> - added new macro for selecting arguments >> - do_trap_psci is now switch case based instead of tables >> >> Changelog v3 >> - moved wfi helper to seperate patch >> - replaced new wfi function in traps.c >> - removed defining power_state variable using value directly >> - added new line between declaration >> - merged PSCI v0.1 and v0.2 cpu_on function to avoid duplication >> - removed spurious change >> - renamed PSCI return values to reflect v0.2 moved them to top >> - removed PSCI v0.1 version declaration for now, will introduce it when needed >> - removed hard tabs in the code lifted from linux >> - removed PSCI_0_1_MAX >> - did not retained copyright header from linux as most of functions are removed >> - added two function tables for PSCI v0.1 and PSCI v0.2 >> - added compatibility string to libxl_arm to expose new functionality >> - refactored affinity_info code >> - added table for masking function >> - removed definitions of unused PSCI v0.2 functions >> - removed function id's of unused PSCI v0.2 functions >> - renamed constant PSCI_0_2_TOS_MP ( multicore aware) as per spec >> >> tools/libxl/libxl_arm.c | 2 +- >> xen/arch/arm/domain_build.c | 5 +- >> xen/arch/arm/traps.c | 123 +++++++++++++++++++++++++---------- >> xen/arch/arm/vpsci.c | 140 ++++++++++++++++++++++++++++++++++++++-- >> xen/include/asm-arm/processor.h | 3 + >> xen/include/asm-arm/psci.h | 83 +++++++++++++++++++++--- >> 6 files changed, 308 insertions(+), 48 deletions(-) >> >> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c >> index 4f0f0e2..e8bcd05 100644 >> --- a/tools/libxl/libxl_arm.c >> +++ b/tools/libxl/libxl_arm.c >> @@ -237,7 +237,7 @@ static int make_psci_node(libxl__gc *gc, void *fdt) >> res = fdt_begin_node(fdt, "psci"); >> if (res) return res; >> >> - res = fdt_property_compat(gc, fdt, 1, "arm,psci"); >> + res = fdt_property_compat(gc, fdt, 2, "arm,psci-0.2","arm,psci"); >> if (res) return res; >> >> res = fdt_property_string(fdt, "method", "hvc"); >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index c424793..ebd4170 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -388,6 +388,9 @@ static int make_hypervisor_node(struct domain *d, >> static int make_psci_node(void *fdt, const struct dt_device_node *parent) >> { >> int res; >> + const char compat[] = >> + "arm,psci-0.2""\0" >> + "arm,psci"; >> >> DPRINT("Create PSCI node\n"); >> >> @@ -396,7 +399,7 @@ static int make_psci_node(void *fdt, const struct dt_device_node *parent) >> if ( res ) >> return res; >> >> - res = fdt_property_string(fdt, "compatible", "arm,psci"); >> + res = fdt_property(fdt, "compatible", compat, sizeof(compat)); >> if ( res ) >> return res; >> >> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c >> index 8102540..844da9d 100644 >> --- a/xen/arch/arm/traps.c >> +++ b/xen/arch/arm/traps.c >> @@ -1030,24 +1030,6 @@ static arm_hypercall_t arm_hypercall_table[] = { >> HYPERCALL_ARM(vcpu_op, 3), >> }; >> >> -typedef int (*arm_psci_fn_t)(uint32_t, register_t); >> - >> -typedef struct { >> - arm_psci_fn_t fn; >> - int nr_args; >> -} arm_psci_t; >> - >> -#define PSCI(_name, _nr_args) \ >> - [ PSCI_ ## _name ] = { \ >> - .fn = (arm_psci_fn_t) &do_psci_ ## _name, \ >> - .nr_args = _nr_args, \ >> - } >> - >> -static arm_psci_t arm_psci_table[] = { >> - PSCI(cpu_off, 1), >> - PSCI(cpu_on, 2), >> -}; >> - >> #ifndef NDEBUG >> static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code) >> { >> @@ -1080,33 +1062,108 @@ static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code) >> #endif >> >> #ifdef CONFIG_ARM_64 >> -#define PSCI_OP_REG(r) (r)->x0 >> #define PSCI_RESULT_REG(r) (r)->x0 >> -#define PSCI_ARGS(r) (r)->x1, (r)->x2 >> +#define PSCI_ARG(r,n) (r)->x##n >> +#define PSCI_ARG32(r,n) (uint32_t)( (r)->x##n & 0x00000000FFFFFFFF ) >> #else >> -#define PSCI_OP_REG(r) (r)->r0 >> #define PSCI_RESULT_REG(r) (r)->r0 >> -#define PSCI_ARGS(r) (r)->r1, (r)->r2 >> +#define PSCI_ARG(r,n) (r)->r##n >> +#define PSCI_ARG32(r,n) PSCI_ARG(r,n) >> #endif >> >> -static void do_trap_psci(struct cpu_user_regs *regs) >> +/* helper function for checking arm mode 32/64 bit */ >> +static inline int psci_mode_check(struct domain *d, register_t fid) >> { >> - arm_psci_fn_t psci_call = NULL; >> + return !( is_64bit_domain(d)^( (fid & PSCI_0_2_64BIT) >> 30 ) ); >> +} >> >> - if ( PSCI_OP_REG(regs) >= ARRAY_SIZE(arm_psci_table) ) >> - { >> - domain_crash_synchronous(); >> - return; >> - } >> +static void do_trap_psci(struct cpu_user_regs *regs) >> +{ >> + register_t fid = PSCI_ARG(regs,0); >> >> - psci_call = arm_psci_table[PSCI_OP_REG(regs)].fn; >> - if ( psci_call == NULL ) >> + /* preloading in case psci_mode_check fails */ >> + PSCI_RESULT_REG(regs) = PSCI_INVALID_PARAMETERS; >> + switch( fid ) >> { >> + case PSCI_cpu_off: >> + { >> + uint32_t pstate = PSCI_ARG32(regs,1); >> + PSCI_RESULT_REG(regs) = do_psci_cpu_off(pstate); >> + } >> + break; >> + case PSCI_cpu_on: >> + { >> + uint32_t vcpuid = PSCI_ARG32(regs,1); >> + register_t epoint = PSCI_ARG(regs,2); >> + PSCI_RESULT_REG(regs) = do_psci_cpu_on(vcpuid, epoint); >> + } >> + break; >> + case PSCI_0_2_FN_PSCI_VERSION: >> + PSCI_RESULT_REG(regs) = do_psci_0_2_version(); >> + break; >> + case PSCI_0_2_FN_CPU_OFF: >> + PSCI_RESULT_REG(regs) = do_psci_0_2_cpu_off(); >> + break; >> + case PSCI_0_2_FN_MIGRATE_INFO_TYPE: >> + PSCI_RESULT_REG(regs) = do_psci_0_2_migrate_info_type(); >> + break; >> + case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU: >> + case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU: >> + if ( psci_mode_check(current->domain, fid) ) >> + PSCI_RESULT_REG(regs) = do_psci_0_2_migrate_info_up_cpu(); >> + break; >> + case PSCI_0_2_FN_SYSTEM_OFF: >> + do_psci_0_2_system_off(); >> + PSCI_RESULT_REG(regs) = PSCI_INTERNAL_FAILURE; >> + break; >> + case PSCI_0_2_FN_SYSTEM_RESET: >> + do_psci_0_2_system_reset(); >> + PSCI_RESULT_REG(regs) = PSCI_INTERNAL_FAILURE; >> + break; >> + case PSCI_0_2_FN_CPU_ON: >> + case PSCI_0_2_FN64_CPU_ON: >> + if ( psci_mode_check(current->domain, fid) ) >> + { >> + register_t vcpuid = PSCI_ARG(regs,1); >> + register_t epoint = PSCI_ARG(regs,2); >> + register_t cid = PSCI_ARG(regs,3); >> + PSCI_RESULT_REG(regs) = >> + do_psci_0_2_cpu_on(vcpuid, epoint, cid); >> + } >> + break; >> + case PSCI_0_2_FN_CPU_SUSPEND: >> + case PSCI_0_2_FN64_CPU_SUSPEND: >> + if ( psci_mode_check(current->domain, fid) ) >> + { >> + uint32_t pstate = PSCI_ARG32(regs,1); >> + register_t epoint = PSCI_ARG(regs,2); >> + register_t cid = PSCI_ARG(regs,3); >> + PSCI_RESULT_REG(regs) = >> + do_psci_0_2_cpu_suspend(pstate, epoint, cid); >> + } >> + break; >> + case PSCI_0_2_FN_AFFINITY_INFO: >> + case PSCI_0_2_FN64_AFFINITY_INFO: >> + if ( psci_mode_check(current->domain, fid) ) >> + { >> + register_t taff = PSCI_ARG(regs,1); >> + uint32_t laff = PSCI_ARG32(regs,2); >> + PSCI_RESULT_REG(regs) = >> + do_psci_0_2_affinity_info(taff, laff); >> + } >> + break; >> + case PSCI_0_2_FN_MIGRATE: >> + case PSCI_0_2_FN64_MIGRATE: >> + if ( psci_mode_check(current->domain, fid) ) >> + { >> + uint32_t tcpu = PSCI_ARG32(regs,1); >> + PSCI_RESULT_REG(regs) = do_psci_0_2_migrate(tcpu); >> + } >> + break; >> + default: >> domain_crash_synchronous(); >> return; >> } >> - >> - PSCI_RESULT_REG(regs) = psci_call(PSCI_ARGS(regs)); >> } >> >> #ifdef CONFIG_ARM_64 >> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c >> index 1ceb8cb..c6c1bdc 100644 >> --- a/xen/arch/arm/vpsci.c >> +++ b/xen/arch/arm/vpsci.c >> @@ -17,24 +17,37 @@ >> #include <asm/current.h> >> #include <asm/gic.h> >> #include <asm/psci.h> >> +#include <public/sched.h> >> +#include <asm-arm/event.h> >> >> -int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) >> +static int do_common_cpu_on(register_t target_cpu, register_t entry_point, >> + register_t context_id,int ver) >> { >> struct vcpu *v; >> struct domain *d = current->domain; >> struct vcpu_guest_context *ctxt; >> int rc; >> int is_thumb = entry_point & 1; >> + register_t vcpuid; >> + >> + if( ver == XEN_PSCI_V_0_2 ) >> + vcpuid = (target_cpu & MPIDR_HWID_MASK); >> + else >> + vcpuid = target_cpu; >> >> if ( (vcpuid < 0) || (vcpuid >= MAX_VIRT_CPUS) ) >> - return PSCI_EINVAL; >> + return PSCI_INVALID_PARAMETERS; >> >> if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL ) >> - return PSCI_EINVAL; >> + return PSCI_INVALID_PARAMETERS; >> >> /* THUMB set is not allowed with 64-bit domain */ >> if ( is_64bit_domain(d) && is_thumb ) >> - return PSCI_EINVAL; >> + return PSCI_INVALID_PARAMETERS; >> + >> + if( ( ver == XEN_PSCI_V_0_2 ) && >> + ( !test_bit(_VPF_down, &v->pause_flags) ) ) >> + return PSCI_ALREADY_ON; >> >> if ( (ctxt = alloc_vcpu_guest_context()) == NULL ) >> return PSCI_DENIED; >> @@ -48,10 +61,18 @@ int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) >> ctxt->ttbr1 = 0; >> ctxt->ttbcr = 0; /* Defined Reset Value */ >> if ( is_32bit_domain(d) ) >> + { >> ctxt->user_regs.cpsr = PSR_GUEST32_INIT; >> + if( ver == XEN_PSCI_V_0_2 ) >> + ctxt->user_regs.r0_usr = context_id; >> + } >> #ifdef CONFIG_ARM_64 >> else >> + { >> ctxt->user_regs.cpsr = PSR_GUEST64_INIT; >> + if( ver == XEN_PSCI_V_0_2 ) >> + ctxt->user_regs.x0 = context_id; >> + } >> #endif >> >> /* Start the VCPU with THUMB set if it's requested by the kernel */ >> @@ -75,7 +96,12 @@ int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) >> return PSCI_SUCCESS; >> } >> >> -int do_psci_cpu_off(uint32_t power_state) >> +int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) >> +{ >> + return do_common_cpu_on(vcpuid,entry_point,0,XEN_PSCI_V_0_1); >> +} >> + >> +int32_t do_psci_cpu_off(uint32_t power_state) >> { >> struct vcpu *v = current; >> if ( !test_and_set_bit(_VPF_down, &v->pause_flags) ) >> @@ -83,6 +109,110 @@ int do_psci_cpu_off(uint32_t power_state) >> return PSCI_SUCCESS; >> } >> >> +uint32_t do_psci_0_2_version(void) >> +{ >> + return XEN_PSCI_V_0_2; >> +} >> + >> +register_t do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point, >> + register_t context_id) >> +{ >> + struct vcpu *v = current; >> + struct cpu_user_regs *regs = guest_cpu_user_regs(); >> + > > Not sure you want to keep this comment around (as I would comment on the > fact that we always treat power off requests as only performing standby > as it simplifies the Xen implementation (as Ian points out in his > comments), but if you do want to keep the comment block: > >> + /* affinity values are ignored in this implementation as > > Not sure of the Xen common practice here, but why not start this with > upper case? > >> + * at present xen does not supports affinity level greater > > does not support affinity levels greater > >> + * than 0, for all affinity values passed we power down/ standby > > ^ extra white space missing space ^ > >> + * the current core */ > > only the current core. (?) > [...] > > -Christoffer Sure, I will change the comment to make it clear that we are performing standby and delete the existing comment. Thanks parth
On Thu, 2014-07-31 at 12:14 +0200, Christoffer Dall wrote: > > + /* affinity values are ignored in this implementation as > > Not sure of the Xen common practice here, Generally proper SPG, if someone happens to spot it ;-) (i.e. Proper sentences, capital letters, full stops etc) Ian.
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c index 4f0f0e2..e8bcd05 100644 --- a/tools/libxl/libxl_arm.c +++ b/tools/libxl/libxl_arm.c @@ -237,7 +237,7 @@ static int make_psci_node(libxl__gc *gc, void *fdt) res = fdt_begin_node(fdt, "psci"); if (res) return res; - res = fdt_property_compat(gc, fdt, 1, "arm,psci"); + res = fdt_property_compat(gc, fdt, 2, "arm,psci-0.2","arm,psci"); if (res) return res; res = fdt_property_string(fdt, "method", "hvc"); diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index c424793..ebd4170 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -388,6 +388,9 @@ static int make_hypervisor_node(struct domain *d, static int make_psci_node(void *fdt, const struct dt_device_node *parent) { int res; + const char compat[] = + "arm,psci-0.2""\0" + "arm,psci"; DPRINT("Create PSCI node\n"); @@ -396,7 +399,7 @@ static int make_psci_node(void *fdt, const struct dt_device_node *parent) if ( res ) return res; - res = fdt_property_string(fdt, "compatible", "arm,psci"); + res = fdt_property(fdt, "compatible", compat, sizeof(compat)); if ( res ) return res; diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 8102540..844da9d 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1030,24 +1030,6 @@ static arm_hypercall_t arm_hypercall_table[] = { HYPERCALL_ARM(vcpu_op, 3), }; -typedef int (*arm_psci_fn_t)(uint32_t, register_t); - -typedef struct { - arm_psci_fn_t fn; - int nr_args; -} arm_psci_t; - -#define PSCI(_name, _nr_args) \ - [ PSCI_ ## _name ] = { \ - .fn = (arm_psci_fn_t) &do_psci_ ## _name, \ - .nr_args = _nr_args, \ - } - -static arm_psci_t arm_psci_table[] = { - PSCI(cpu_off, 1), - PSCI(cpu_on, 2), -}; - #ifndef NDEBUG static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code) { @@ -1080,33 +1062,108 @@ static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code) #endif #ifdef CONFIG_ARM_64 -#define PSCI_OP_REG(r) (r)->x0 #define PSCI_RESULT_REG(r) (r)->x0 -#define PSCI_ARGS(r) (r)->x1, (r)->x2 +#define PSCI_ARG(r,n) (r)->x##n +#define PSCI_ARG32(r,n) (uint32_t)( (r)->x##n & 0x00000000FFFFFFFF ) #else -#define PSCI_OP_REG(r) (r)->r0 #define PSCI_RESULT_REG(r) (r)->r0 -#define PSCI_ARGS(r) (r)->r1, (r)->r2 +#define PSCI_ARG(r,n) (r)->r##n +#define PSCI_ARG32(r,n) PSCI_ARG(r,n) #endif -static void do_trap_psci(struct cpu_user_regs *regs) +/* helper function for checking arm mode 32/64 bit */ +static inline int psci_mode_check(struct domain *d, register_t fid) { - arm_psci_fn_t psci_call = NULL; + return !( is_64bit_domain(d)^( (fid & PSCI_0_2_64BIT) >> 30 ) ); +} - if ( PSCI_OP_REG(regs) >= ARRAY_SIZE(arm_psci_table) ) - { - domain_crash_synchronous(); - return; - } +static void do_trap_psci(struct cpu_user_regs *regs) +{ + register_t fid = PSCI_ARG(regs,0); - psci_call = arm_psci_table[PSCI_OP_REG(regs)].fn; - if ( psci_call == NULL ) + /* preloading in case psci_mode_check fails */ + PSCI_RESULT_REG(regs) = PSCI_INVALID_PARAMETERS; + switch( fid ) { + case PSCI_cpu_off: + { + uint32_t pstate = PSCI_ARG32(regs,1); + PSCI_RESULT_REG(regs) = do_psci_cpu_off(pstate); + } + break; + case PSCI_cpu_on: + { + uint32_t vcpuid = PSCI_ARG32(regs,1); + register_t epoint = PSCI_ARG(regs,2); + PSCI_RESULT_REG(regs) = do_psci_cpu_on(vcpuid, epoint); + } + break; + case PSCI_0_2_FN_PSCI_VERSION: + PSCI_RESULT_REG(regs) = do_psci_0_2_version(); + break; + case PSCI_0_2_FN_CPU_OFF: + PSCI_RESULT_REG(regs) = do_psci_0_2_cpu_off(); + break; + case PSCI_0_2_FN_MIGRATE_INFO_TYPE: + PSCI_RESULT_REG(regs) = do_psci_0_2_migrate_info_type(); + break; + case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU: + case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU: + if ( psci_mode_check(current->domain, fid) ) + PSCI_RESULT_REG(regs) = do_psci_0_2_migrate_info_up_cpu(); + break; + case PSCI_0_2_FN_SYSTEM_OFF: + do_psci_0_2_system_off(); + PSCI_RESULT_REG(regs) = PSCI_INTERNAL_FAILURE; + break; + case PSCI_0_2_FN_SYSTEM_RESET: + do_psci_0_2_system_reset(); + PSCI_RESULT_REG(regs) = PSCI_INTERNAL_FAILURE; + break; + case PSCI_0_2_FN_CPU_ON: + case PSCI_0_2_FN64_CPU_ON: + if ( psci_mode_check(current->domain, fid) ) + { + register_t vcpuid = PSCI_ARG(regs,1); + register_t epoint = PSCI_ARG(regs,2); + register_t cid = PSCI_ARG(regs,3); + PSCI_RESULT_REG(regs) = + do_psci_0_2_cpu_on(vcpuid, epoint, cid); + } + break; + case PSCI_0_2_FN_CPU_SUSPEND: + case PSCI_0_2_FN64_CPU_SUSPEND: + if ( psci_mode_check(current->domain, fid) ) + { + uint32_t pstate = PSCI_ARG32(regs,1); + register_t epoint = PSCI_ARG(regs,2); + register_t cid = PSCI_ARG(regs,3); + PSCI_RESULT_REG(regs) = + do_psci_0_2_cpu_suspend(pstate, epoint, cid); + } + break; + case PSCI_0_2_FN_AFFINITY_INFO: + case PSCI_0_2_FN64_AFFINITY_INFO: + if ( psci_mode_check(current->domain, fid) ) + { + register_t taff = PSCI_ARG(regs,1); + uint32_t laff = PSCI_ARG32(regs,2); + PSCI_RESULT_REG(regs) = + do_psci_0_2_affinity_info(taff, laff); + } + break; + case PSCI_0_2_FN_MIGRATE: + case PSCI_0_2_FN64_MIGRATE: + if ( psci_mode_check(current->domain, fid) ) + { + uint32_t tcpu = PSCI_ARG32(regs,1); + PSCI_RESULT_REG(regs) = do_psci_0_2_migrate(tcpu); + } + break; + default: domain_crash_synchronous(); return; } - - PSCI_RESULT_REG(regs) = psci_call(PSCI_ARGS(regs)); } #ifdef CONFIG_ARM_64 diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c index 1ceb8cb..c6c1bdc 100644 --- a/xen/arch/arm/vpsci.c +++ b/xen/arch/arm/vpsci.c @@ -17,24 +17,37 @@ #include <asm/current.h> #include <asm/gic.h> #include <asm/psci.h> +#include <public/sched.h> +#include <asm-arm/event.h> -int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) +static int do_common_cpu_on(register_t target_cpu, register_t entry_point, + register_t context_id,int ver) { struct vcpu *v; struct domain *d = current->domain; struct vcpu_guest_context *ctxt; int rc; int is_thumb = entry_point & 1; + register_t vcpuid; + + if( ver == XEN_PSCI_V_0_2 ) + vcpuid = (target_cpu & MPIDR_HWID_MASK); + else + vcpuid = target_cpu; if ( (vcpuid < 0) || (vcpuid >= MAX_VIRT_CPUS) ) - return PSCI_EINVAL; + return PSCI_INVALID_PARAMETERS; if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL ) - return PSCI_EINVAL; + return PSCI_INVALID_PARAMETERS; /* THUMB set is not allowed with 64-bit domain */ if ( is_64bit_domain(d) && is_thumb ) - return PSCI_EINVAL; + return PSCI_INVALID_PARAMETERS; + + if( ( ver == XEN_PSCI_V_0_2 ) && + ( !test_bit(_VPF_down, &v->pause_flags) ) ) + return PSCI_ALREADY_ON; if ( (ctxt = alloc_vcpu_guest_context()) == NULL ) return PSCI_DENIED; @@ -48,10 +61,18 @@ int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) ctxt->ttbr1 = 0; ctxt->ttbcr = 0; /* Defined Reset Value */ if ( is_32bit_domain(d) ) + { ctxt->user_regs.cpsr = PSR_GUEST32_INIT; + if( ver == XEN_PSCI_V_0_2 ) + ctxt->user_regs.r0_usr = context_id; + } #ifdef CONFIG_ARM_64 else + { ctxt->user_regs.cpsr = PSR_GUEST64_INIT; + if( ver == XEN_PSCI_V_0_2 ) + ctxt->user_regs.x0 = context_id; + } #endif /* Start the VCPU with THUMB set if it's requested by the kernel */ @@ -75,7 +96,12 @@ int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) return PSCI_SUCCESS; } -int do_psci_cpu_off(uint32_t power_state) +int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) +{ + return do_common_cpu_on(vcpuid,entry_point,0,XEN_PSCI_V_0_1); +} + +int32_t do_psci_cpu_off(uint32_t power_state) { struct vcpu *v = current; if ( !test_and_set_bit(_VPF_down, &v->pause_flags) ) @@ -83,6 +109,110 @@ int do_psci_cpu_off(uint32_t power_state) return PSCI_SUCCESS; } +uint32_t do_psci_0_2_version(void) +{ + return XEN_PSCI_V_0_2; +} + +register_t do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point, + register_t context_id) +{ + struct vcpu *v = current; + struct cpu_user_regs *regs = guest_cpu_user_regs(); + + /* affinity values are ignored in this implementation as + * at present xen does not supports affinity level greater + * than 0, for all affinity values passed we power down/ standby + * the current core */ + if( power_state & PSCI_0_2_POWER_STATE_TYPE_MASK ) + { + if ( is_32bit_domain(v->domain) ) + regs->r0 = context_id; +#ifdef CONFIG_ARM_64 + else + regs->x0 = context_id; +#endif + } + + vcpu_block_unless_event_pending(v); + return PSCI_SUCCESS; +} + +int32_t do_psci_0_2_cpu_off(void) +{ + return do_psci_cpu_off(0); +} + +int32_t do_psci_0_2_cpu_on(register_t target_cpu, register_t entry_point, + register_t context_id) +{ + return do_common_cpu_on(target_cpu,entry_point,context_id,XEN_PSCI_V_0_2); +} + +static const unsigned long target_affinity_mask[] = { + ( MPIDR_HWID_MASK & AFFINITY_MASK( 0 ) ), + ( MPIDR_HWID_MASK & AFFINITY_MASK( 1 ) ), + ( MPIDR_HWID_MASK & AFFINITY_MASK( 2 ) ) +#ifdef CONFIG_ARM_64 + ,( MPIDR_HWID_MASK & AFFINITY_MASK( 3 ) ) +#endif +}; + +int32_t do_psci_0_2_affinity_info(register_t target_affinity, + uint32_t lowest_affinity_level) +{ + struct domain *d = current->domain; + struct vcpu *v; + uint32_t vcpuid; + unsigned long tmask; + + if ( lowest_affinity_level < ARRAY_SIZE(target_affinity_mask) ) + { + tmask = target_affinity_mask[lowest_affinity_level]; + target_affinity &= tmask; + } + else + return PSCI_INVALID_PARAMETERS; + + for ( vcpuid = 0; vcpuid < d->max_vcpus; vcpuid++ ) + { + v = d->vcpu[vcpuid]; + + if ( ( ( v->arch.vmpidr & tmask ) == target_affinity ) + && ( !test_bit(_VPF_down, &v->pause_flags) ) ) + return PSCI_0_2_AFFINITY_LEVEL_ON; + } + + return PSCI_0_2_AFFINITY_LEVEL_OFF; +} + +int32_t do_psci_0_2_migrate(uint32_t target_cpu) +{ + return PSCI_NOT_SUPPORTED; +} + +uint32_t do_psci_0_2_migrate_info_type(void) +{ + return PSCI_0_2_TOS_MP_OR_NOT_PRESENT; +} + +register_t do_psci_0_2_migrate_info_up_cpu(void) +{ + return PSCI_NOT_SUPPORTED; +} + +void do_psci_0_2_system_off( void ) +{ + struct domain *d = current->domain; + domain_shutdown(d,SHUTDOWN_poweroff); +} + +void do_psci_0_2_system_reset(void) +{ + struct domain *d = current->domain; + domain_shutdown(d,SHUTDOWN_reboot); +} + /* * Local variables: * mode: C diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h index 9267c1b..8a237c4 100644 --- a/xen/include/asm-arm/processor.h +++ b/xen/include/asm-arm/processor.h @@ -16,6 +16,9 @@ #define MPIDR_AFF0_MASK (_AC(0xff,U) << MPIDR_AFF0_SHIFT) #define MPIDR_HWID_MASK _AC(0xffffff,U) #define MPIDR_INVALID (~MPIDR_HWID_MASK) +#define MPIDR_LEVEL_BITS (8) +#define AFFINITY_MASK(level) ~((_AC(0x1,U) << ((level) * MPIDR_LEVEL_BITS)) - 1) + /* TTBCR Translation Table Base Control Register */ #define TTBCR_EAE _AC(0x80000000,U) diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h index 189964b..9777c03 100644 --- a/xen/include/asm-arm/psci.h +++ b/xen/include/asm-arm/psci.h @@ -1,10 +1,16 @@ #ifndef __ASM_PSCI_H__ #define __ASM_PSCI_H__ -#define PSCI_SUCCESS 0 -#define PSCI_ENOSYS -1 -#define PSCI_EINVAL -2 -#define PSCI_DENIED -3 +/* PSCI return values (inclusive of all PSCI versions) */ +#define PSCI_SUCCESS 0 +#define PSCI_NOT_SUPPORTED -1 +#define PSCI_INVALID_PARAMETERS -2 +#define PSCI_DENIED -3 +#define PSCI_ALREADY_ON -4 +#define PSCI_ON_PENDING -5 +#define PSCI_INTERNAL_FAILURE -6 +#define PSCI_NOT_PRESENT -7 +#define PSCI_DISABLED -8 /* availability of PSCI on the host for SMP bringup */ extern bool_t psci_available; @@ -13,10 +19,71 @@ int psci_init(void); int call_psci_cpu_on(int cpu); /* functions to handle guest PSCI requests */ -int do_psci_cpu_on(uint32_t vcpuid, register_t entry_point); -int do_psci_cpu_off(uint32_t power_state); -int do_psci_cpu_suspend(uint32_t power_state, register_t entry_point); -int do_psci_migrate(uint32_t vcpuid); +int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point); +int32_t do_psci_cpu_off(uint32_t power_state); +int32_t do_psci_cpu_suspend(uint32_t power_state, register_t entry_point); +int32_t do_psci_migrate(uint32_t vcpuid); + +/* PSCI 0.2 functions to handle guest PSCI requests */ +uint32_t do_psci_0_2_version(void); +register_t do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point, + register_t context_id); +int32_t do_psci_0_2_cpu_off(void); +int32_t do_psci_0_2_cpu_on(register_t target_cpu, register_t entry_point, + register_t context_id); +int32_t do_psci_0_2_affinity_info(register_t target_affinity, + uint32_t lowest_affinity_level); +int32_t do_psci_0_2_migrate(uint32_t target_cpu); +uint32_t do_psci_0_2_migrate_info_type(void); +register_t do_psci_0_2_migrate_info_up_cpu(void); +void do_psci_0_2_system_off(void); +void do_psci_0_2_system_reset(void); + +/* PSCI version */ +#define XEN_PSCI_V_0_1 1 +#define XEN_PSCI_V_0_2 2 + +/* PSCI v0.2 interface */ +#define PSCI_0_2_FN_BASE 0x84000000 +#define PSCI_0_2_FN(n) (PSCI_0_2_FN_BASE + (n)) +#define PSCI_0_2_64BIT 0x40000000 +#define PSCI_0_2_FN64_BASE \ + (PSCI_0_2_FN_BASE + PSCI_0_2_64BIT) +#define PSCI_0_2_FN64(n) (PSCI_0_2_FN64_BASE + (n)) + +#define PSCI_0_2_FN_PSCI_VERSION PSCI_0_2_FN(0) +#define PSCI_0_2_FN_CPU_SUSPEND PSCI_0_2_FN(1) +#define PSCI_0_2_FN_CPU_OFF PSCI_0_2_FN(2) +#define PSCI_0_2_FN_CPU_ON PSCI_0_2_FN(3) +#define PSCI_0_2_FN_AFFINITY_INFO PSCI_0_2_FN(4) +#define PSCI_0_2_FN_MIGRATE PSCI_0_2_FN(5) +#define PSCI_0_2_FN_MIGRATE_INFO_TYPE PSCI_0_2_FN(6) +#define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU PSCI_0_2_FN(7) +#define PSCI_0_2_FN_SYSTEM_OFF PSCI_0_2_FN(8) +#define PSCI_0_2_FN_SYSTEM_RESET PSCI_0_2_FN(9) + +#define PSCI_0_2_FN64_CPU_SUSPEND PSCI_0_2_FN64(1) +#define PSCI_0_2_FN64_CPU_ON PSCI_0_2_FN64(3) +#define PSCI_0_2_FN64_AFFINITY_INFO PSCI_0_2_FN64(4) +#define PSCI_0_2_FN64_MIGRATE PSCI_0_2_FN64(5) +#define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU PSCI_0_2_FN64(7) + +/* PSCI v0.2 affinity level state returned by AFFINITY_INFO */ +#define PSCI_0_2_AFFINITY_LEVEL_ON 0 +#define PSCI_0_2_AFFINITY_LEVEL_OFF 1 +#define PSCI_0_2_AFFINITY_LEVEL_ON_PENDING 2 + +/* PSCI v0.2 multicore support in Trusted OS returned by MIGRATE_INFO_TYPE */ +#define PSCI_0_2_TOS_UP_MIGRATE_CAPABLE 0 +#define PSCI_0_2_TOS_UP_NOT_MIGRATE_CAPABLE 1 +#define PSCI_0_2_TOS_MP_OR_NOT_PRESENT 2 + +/* PSCI v0.2 power state encoding for CPU_SUSPEND function */ +#define PSCI_0_2_POWER_STATE_ID_MASK 0xffff +#define PSCI_0_2_POWER_STATE_ID_SHIFT 0 +#define PSCI_0_2_POWER_STATE_TYPE_SHIFT 16 +#define PSCI_0_2_POWER_STATE_TYPE_MASK \ + (0x1 << PSCI_0_2_POWER_STATE_TYPE_SHIFT) #endif /* __ASM_PSCI_H__ */
Arm based virtual machines dom0/guest will request power related functionality from xen through PSCI interface. This patch implements version 0.2 of PSCI standard specified by arm for 64bit and 32 bit arm machines. - removed arm_psci_fn_t - implemented psci_cpu_on with additional error conditions - added switch-case in do_trap_psci function - added PSCI v0.2 macros in psci.h - removed tables for PSCI v0.1 and v0.2 - implemented affinity_info - moved do_common_cpu up in vpsci.c removed declaration - removed PSCI_ARGS macro - added new macro for 32 bit arguments - added new function psci_mode_check - modified cpu_suspend to return context_id - added macros for checking powe_state Signed-off-by: Parth Dixit <parth.dixit@linaro.org> --- Changelog v8 - fixed error in cpu_suspend Changelog v7 - removed casting from functions in do_psci_trap - modified cpu_suspend to return success in case the call returns Changelog v6 - fixed indentation - removed casting from error codes - modified cpu_suspend to check power_state - ignoring context_id in case of standby - added comments for ignoring affinity - added macro for checking power_state Changelog v5 - removed PSCI_ARGS macro - preload error value for calls that do not return - added new macro for 32 bit arguments - casting return calls to appropriate types - added new function psci_mode_check - added error checks for PSCI v0.2 function id's Changelog v4 - added target_affinity_mask[] to local variable - removed AFF3 masking for 64 bit cpu - added do_common_cpu_on for PSCI v0.1 and v0.2 - moved cpu_on decision based on entry point - removed vpsci_ver introduced in v3 - removed psci tables - added new macro for selecting arguments - do_trap_psci is now switch case based instead of tables Changelog v3 - moved wfi helper to seperate patch - replaced new wfi function in traps.c - removed defining power_state variable using value directly - added new line between declaration - merged PSCI v0.1 and v0.2 cpu_on function to avoid duplication - removed spurious change - renamed PSCI return values to reflect v0.2 moved them to top - removed PSCI v0.1 version declaration for now, will introduce it when needed - removed hard tabs in the code lifted from linux - removed PSCI_0_1_MAX - did not retained copyright header from linux as most of functions are removed - added two function tables for PSCI v0.1 and PSCI v0.2 - added compatibility string to libxl_arm to expose new functionality - refactored affinity_info code - added table for masking function - removed definitions of unused PSCI v0.2 functions - removed function id's of unused PSCI v0.2 functions - renamed constant PSCI_0_2_TOS_MP ( multicore aware) as per spec tools/libxl/libxl_arm.c | 2 +- xen/arch/arm/domain_build.c | 5 +- xen/arch/arm/traps.c | 123 +++++++++++++++++++++++++---------- xen/arch/arm/vpsci.c | 140 ++++++++++++++++++++++++++++++++++++++-- xen/include/asm-arm/processor.h | 3 + xen/include/asm-arm/psci.h | 83 +++++++++++++++++++++--- 6 files changed, 308 insertions(+), 48 deletions(-)