[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH RFC] xen/arm : emulation of arm's psci v0.2 standard



On Thu, 19 Jun 2014, Parth Dixit wrote:
> From: parthd <parth.dixit@xxxxxxxxxx>
> 
> 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.
> 
> Signed-off-by: Parth Dixit <parth.dixit@xxxxxxxxxx>
> ---
>  xen/arch/arm/domain_build.c     |  5 ++-
>  xen/arch/arm/traps.c            | 56 ++++++++++++++++++++++--
>  xen/arch/arm/vpsci.c            | 75 ++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/processor.h |  6 +++
>  xen/include/asm-arm/psci.h      | 18 ++++++++
>  xen/include/public/arch-arm.h   | 95 
> +++++++++++++++++++++++++++++++++++++++--
>  6 files changed, 246 insertions(+), 9 deletions(-)
> 
> 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;
>  

Even though you are adding the psci-0.2 compatible string, I don't see
the new PSCI_0_2_FN_* function numbers being exposed to the guest yet.


> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 03a3da6..dc4ff56 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1046,6 +1046,15 @@ typedef struct {
>  static arm_psci_t arm_psci_table[] = {
>      PSCI(cpu_off, 1),
>      PSCI(cpu_on, 2),
> +    PSCI(0_2_psci_version,1),
> +    PSCI(0_2_cpu_suspend,2),
> +    PSCI(0_2_affinity_info,2),
> +    PSCI(0_2_migrate,1),
> +    PSCI(0_2_migrate_info_type,0),
> +    PSCI(0_2_migrate_info_up_cpu,0),
> +    PSCI(0_2_system_off,0),
> +    PSCI(0_2_system_reset,0),
> +
>  };
>  
>  #ifndef NDEBUG
> @@ -1092,14 +1101,53 @@ static void do_debug_trap(struct cpu_user_regs *regs, 
> unsigned int code)
>  static void do_trap_psci(struct cpu_user_regs *regs)
>  {
>      arm_psci_fn_t psci_call = NULL;
> +    int fn_index = -1;
>  
> -    if ( PSCI_OP_REG(regs) >= ARRAY_SIZE(arm_psci_table) )
> +    switch ( PSCI_OP_REG(regs) )
>      {
> -        domain_crash_synchronous();
> -        return;
> +        case PSCI_0_2_FN_PSCI_VERSION:
> +            fn_index = PSCI_0_2_psci_version;
> +            break;
> +        case PSCI_0_2_FN_CPU_SUSPEND:
> +        case PSCI_0_2_FN64_CPU_SUSPEND:
> +            fn_index = PSCI_0_2_cpu_suspend;
> +            break;
> +        case PSCI_cpu_off:
> +        case PSCI_0_2_FN_CPU_OFF:
> +            fn_index = PSCI_cpu_off;
> +            break;
> +        case PSCI_cpu_on:
> +        case PSCI_0_2_FN_CPU_ON:
> +        case PSCI_0_2_FN64_CPU_ON:
> +            fn_index = PSCI_cpu_on;
> +            break;
> +        case PSCI_0_2_FN_AFFINITY_INFO:
> +        case PSCI_0_2_FN64_AFFINITY_INFO:
> +            fn_index = PSCI_0_2_affinity_info;
> +            break;
> +        case PSCI_0_2_FN_MIGRATE:
> +        case PSCI_0_2_FN64_MIGRATE:
> +            fn_index = PSCI_0_2_migrate;
> +            break;
> +        case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
> +            fn_index = 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:
> +            fn_index = PSCI_0_2_migrate_info_up_cpu;
> +            break;
> +        case PSCI_0_2_FN_SYSTEM_OFF:
> +            fn_index = PSCI_0_2_system_off;
> +            break;
> +        case PSCI_0_2_FN_SYSTEM_RESET:
> +            fn_index = PSCI_0_2_system_reset;
> +            break;
> +        default:
> +            domain_crash_synchronous();
> +                 return;
>      }
>  
> -    psci_call = arm_psci_table[PSCI_OP_REG(regs)].fn;
> +    psci_call = arm_psci_table[fn_index].fn;
>      if ( psci_call == NULL )
>      {
>          domain_crash_synchronous();
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 1ceb8cb..c1243d4 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -83,6 +83,81 @@ int do_psci_cpu_off(uint32_t power_state)
>      return PSCI_SUCCESS;
>  }

You haven't modified do_psci_cpu_on, but the current implementation
takes a 32bit vcpu id instead of a register_t to specify the target cpu.
Also do_psci_cpu_on doesn't currently take a context_id as parameter.


> +int do_psci_0_2_psci_version(uint32_t vcpuid)
> +{
> +    return XEN_PSCI_V_0_2;
> +}
> +
> +int do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point)
> +{
> +    struct cpu_user_regs *regs = guest_cpu_user_regs();
> +    regs->cpsr &= ~PSR_IRQ_MASK;
> +    vcpu_block();
> +    return PSCI_RET_SUCCESS;
> +}
> +
> +int do_psci_0_2_affinity_info(uint32_t target_affinity, uint32_t 
> lowest_affinity_level)
> +{
> +    unsigned long target_affinity_mask;
> +    unsigned int mpidr;
> +    struct vcpu *v = current;
> +
> +    switch ( lowest_affinity_level )
> +    {
> +    case MPIDR_AFF0_SHIFT:
> +    case MPIDR_AFF1_SHIFT:
> +    case MPIDR_AFF2_SHIFT:
> +        target_affinity_mask
> +            = ( MPIDR_HWID_MASK & AFFINITY_MASK( lowest_affinity_level ) );
> +        break;
> +    case MPIDR_AFF3_SHIFT:
> +        target_affinity_mask
> +            = ( MPIDR_HWID_MASK & AFFINITY_MASK(lowest_affinity_level+1) );
> +        break;
> +    default:
> +        return PSCI_RET_INVALID_PARAMS;
> +    }
> +
> +    target_affinity &= target_affinity_mask;
> +    mpidr = v->arch.vmpidr;
> +    if ( ( mpidr & target_affinity_mask ) == target_affinity )
> +        return PSCI_0_2_AFFINITY_LEVEL_ON;
> +    else
> +        return PSCI_0_2_AFFINITY_LEVEL_OFF;
> +
> +}
> +
> +int do_psci_0_2_migrate(uint32_t vcpuid)
> +{
> +    return PSCI_RET_NOT_SUPPORTED;
> +}
> +
> +
> +int do_psci_0_2_migrate_info_type(void)
> +{
> +    return PSCI_0_2_TOS_MP;
> +}
> +
> +
> +int do_psci_0_2_migrate_info_up_cpu(void)
> +{
> +    return PSCI_RET_NOT_SUPPORTED;
> +}
> +
> +
> +void do_psci_0_2_system_off( void )

code style


> +{
> +    struct domain *d = current->domain;
> +    domain_shutdown(d,0);
> +}
> +
> +
> +void do_psci_0_2_system_reset(void)
> +{
> +    struct domain *d = current->domain;
> +    domain_shutdown(d,1);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index 9267c1b..20f02d5 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -13,9 +13,15 @@
>  #define _MPIDR_SMP          (31)
>  #define MPIDR_SMP           (_AC(1,U) << _MPIDR_SMP)
>  #define MPIDR_AFF0_SHIFT    (0)
> +#define MPIDR_AFF1_SHIFT    (1)
> +#define MPIDR_AFF2_SHIFT    (2)
> +#define MPIDR_AFF3_SHIFT    (3)
>  #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_AFF_BIT_SHIFT (8)
> +#define AFFINITY_MASK(level) (_AC(0xff,U) << ((level)*MPIDR_AFF_BIT_SHIFT)) 
> +
>  
>  /* 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..dbaa885 100644
> --- a/xen/include/asm-arm/psci.h
> +++ b/xen/include/asm-arm/psci.h
> @@ -18,6 +18,24 @@ 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);
>  
> +/* PSCI 0.2 functions to handle guest PSCI requests */
> +int do_psci_0_2_psci_version(uint32_t vcpuid);
> +int do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point);
> +int do_psci_0_2_cpu_off(uint32_t power_state);
> +int do_psci_0_2_cpu_on(uint32_t vcpuid, register_t entry_point);
> +int do_psci_0_2_affinity_info(uint32_t target_affinity, uint32_t 
> lowest_affinity_level);
> +int do_psci_0_2_migrate(uint32_t vcpuid);
> +int do_psci_0_2_migrate_info_type(void);
> +int 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);
> +int do_psci_0_2_fn64_cpu_suspend(uint32_t power_state, uint32_t entry_point);
> +int do_psci_0_2_fn64_cpu_on(uint32_t vcpuid, register_t entry_point);
> +int do_psci_0_2_fn64_affinity_info(uint32_t target_affinity, uint32_t 
> lowest_affinity_level);
> +int do_psci_0_2_fn64_migrate(uint32_t vcpuid);
> +int do_psci_0_2_fn64_migrate_info_up_cpu(void);
> +
> +
>  #endif /* __ASM_PSCI_H__ */
>  
>  /*
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 7496556..1663a3e 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -380,11 +380,98 @@ typedef uint64_t xen_callback_t;
>  #define GUEST_TIMER_PHYS_NS_PPI 30
>  #define GUEST_EVTCHN_PPI        31
>  
> +/* PSCI version */
> +#define XEN_PSCI_V_0_1 1
> +#define XEN_PSCI_V_0_2 2
> +
>  /* PSCI functions */
> -#define PSCI_cpu_suspend 0
> -#define PSCI_cpu_off     1
> -#define PSCI_cpu_on      2
> -#define PSCI_migrate     3
> +#define PSCI_migrate     17
> +#define PSCI_cpu_suspend 18
> +#define PSCI_cpu_off     0
> +#define PSCI_cpu_on      1

Interestingly this is going to change the existing function numbers.
Linux reads them from device tree so that should be OK. It is definitely
worth noting that you are making this change in the commit message.


> +#define PSCI_0_2_psci_version             2
> +#define PSCI_0_2_cpu_suspend              3
> +#define PSCI_0_2_affinity_info            4
> +#define PSCI_0_2_migrate                  5
> +#define PSCI_0_2_migrate_info_type        6
> +#define PSCI_0_2_migrate_info_up_cpu      7
> +#define PSCI_0_2_system_off               8
> +#define PSCI_0_2_system_reset             9
> +#define PSCI_0_2_cpu_on                   10
> +#define PSCI_0_2_cpu_off                  11
> +#define PSCI_0_2_fn64_cpu_suspend         12
> +#define PSCI_0_2_fn64_cpu_on              13
> +#define PSCI_0_2_fn64_affinity_info       14
> +#define PSCI_0_2_fn64_migrate             15
> +#define PSCI_0_2_fn64_migrate_info_up_cpu 16

These are just internal between traps.c and vpsci.c, right?
You should write that down.


> +/* 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 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)
> +#define PSCI_0_2_POWER_STATE_AFFL_SHIFT              24
> +#define PSCI_0_2_POWER_STATE_AFFL_MASK               \
> +                             (0x3 << PSCI_0_2_POWER_STATE_AFFL_SHIFT)
> +
> +/* 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                      0
> +#define PSCI_0_2_TOS_UP_NO_MIGRATE           1
> +#define PSCI_0_2_TOS_MP                              2
> +
> +/* PSCI version decoding (independent of PSCI version) */
> +#define PSCI_VERSION_MAJOR_SHIFT             16
> +#define PSCI_VERSION_MINOR_MASK                      \
> +             ((1U << PSCI_VERSION_MAJOR_SHIFT) - 1)
> +#define PSCI_VERSION_MAJOR_MASK                      ~PSCI_VERSION_MINOR_MASK
> +#define PSCI_VERSION_MAJOR(ver)                      \
> +             (((ver) & PSCI_VERSION_MAJOR_MASK) >> PSCI_VERSION_MAJOR_SHIFT)
> +#define PSCI_VERSION_MINOR(ver)                      \
> +             ((ver) & PSCI_VERSION_MINOR_MASK)
> +
> +/* PSCI return values (inclusive of all PSCI versions) */
> +#define PSCI_RET_SUCCESS                      0
> +#define PSCI_RET_NOT_SUPPORTED                       -1
> +#define PSCI_RET_INVALID_PARAMS                      -2
> +#define PSCI_RET_DENIED                              -3
> +#define PSCI_RET_ALREADY_ON                  -4
> +#define PSCI_RET_ON_PENDING                  -5
> +#define PSCI_RET_INTERNAL_FAILURE            -6
> +#define PSCI_RET_NOT_PRESENT                 -7
> +#define PSCI_RET_DISABLED                    -8

Be careful: at the moment the functions in vpsci.c return the error
codes specified in xen/include/asm-arm/psci.h.
We should define the PSCI return codes only in one place.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.