|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v8] xen/arm : emulation of arm's PSCI v0.2 standard
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@xxxxxxxxxx>
> ---
> 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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |