Re: [Xen-devel] [PATCH v3 01/17] xen/arm: vpsci: Add support for PSCI 1.1

[Julien Grall]

Hi Stefano,

On 21/02/2018 00:37, Stefano Stabellini wrote:

On Thu, 15 Feb 2018, Julien Grall wrote:

At the moment, Xen provides virtual PSCI interface compliant with 0.1
and 0.2. Since them, the specification has been updated and the latest
version is 1.1 (see ARM DEN 0022D).

>From an implementation point of view, only PSCI_FEATURES is mandatory.
The rest is optional and can be left unimplemented for now.

At the same time, the compatible for PSCI node have been updated to
expose "arm,psci-1.0".

Signed-off-by: Julien Grall 
Acked-by: Wei Liu 
Reviewed-by: Volodymyr Babchuk 
Cc: Ian Jackson 
Cc: mirela.simono...@aggios.com


This patch doesn't apply cleanly to staging. Am I missing a
prerequisite?


I messed up my git-format command and forgot to include "xen/arm: psci: 
Rework the PSCI definitions". It was included in v2.


I will resend the series with the ack/reviewed-by collected.

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 01/17] xen/arm: vpsci: Add support for PSCI 1.1

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall wrote:
> At the moment, Xen provides virtual PSCI interface compliant with 0.1
> and 0.2. Since them, the specification has been updated and the latest
> version is 1.1 (see ARM DEN 0022D).
> 
> >From an implementation point of view, only PSCI_FEATURES is mandatory.
> The rest is optional and can be left unimplemented for now.
> 
> At the same time, the compatible for PSCI node have been updated to
> expose "arm,psci-1.0".
> 
> Signed-off-by: Julien Grall 
> Acked-by: Wei Liu 
> Reviewed-by: Volodymyr Babchuk 
> Cc: Ian Jackson 
> Cc: mirela.simono...@aggios.com

This patch doesn't apply cleanly to staging. Am I missing a
prerequisite?


> ---
> We may want to provide a way for the toolstack to specify a PSCI
> version. This could be useful if a guest is expecting a given
> version.
> 
> Changes in v3:
> - Add Wei's acked-by
> - Add Volodymyr's reviewed-by
> 
> Changes in v2:
> - Return v1.1 on GET_VERSION call as claimed by this patch
> - Order by function ID the calls in FEATURES call
> ---
>  tools/libxl/libxl_arm.c  |  3 ++-
>  xen/arch/arm/domain_build.c  |  1 +
>  xen/arch/arm/vpsci.c | 39 ++-
>  xen/include/asm-arm/perfc_defn.h |  1 +
>  xen/include/asm-arm/psci.h   |  1 +
>  xen/include/asm-arm/vpsci.h  |  2 +-
>  6 files changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 3e46554301..86f59c0d80 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -410,7 +410,8 @@ 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, 2, "arm,psci-0.2","arm,psci");
> +res = fdt_property_compat(gc, fdt, 3, "arm,psci-1.0",
> +  "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 155c952349..941688a2ce 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -637,6 +637,7 @@ static int make_psci_node(void *fdt, const struct 
> dt_device_node *parent)
>  {
>  int res;
>  const char compat[] =
> +"arm,psci-1.0""\0"
>  "arm,psci-0.2""\0"
>  "arm,psci";
>  
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 6ab8ab64d0..e82b62db1a 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -106,7 +106,11 @@ static int32_t do_psci_cpu_off(uint32_t power_state)
>  
>  static uint32_t do_psci_0_2_version(void)
>  {
> -return PSCI_VERSION(0, 2);
> +/*
> + * PSCI is backward compatible from 0.2. So we can bump the version
> + * without any issue.
> + */
> +return PSCI_VERSION(1, 1);
>  }
>  
>  static register_t do_psci_0_2_cpu_suspend(uint32_t power_state,
> @@ -191,6 +195,29 @@ static void do_psci_0_2_system_reset(void)
>  domain_shutdown(d,SHUTDOWN_reboot);
>  }
>  
> +static int32_t do_psci_1_0_features(uint32_t psci_func_id)
> +{
> +/* /!\ Ordered by function ID and not name */
> +switch ( psci_func_id )
> +{
> +case PSCI_0_2_FN32_PSCI_VERSION:
> +case PSCI_0_2_FN32_CPU_SUSPEND:
> +case PSCI_0_2_FN64_CPU_SUSPEND:
> +case PSCI_0_2_FN32_CPU_OFF:
> +case PSCI_0_2_FN32_CPU_ON:
> +case PSCI_0_2_FN64_CPU_ON:
> +case PSCI_0_2_FN32_AFFINITY_INFO:
> +case PSCI_0_2_FN64_AFFINITY_INFO:
> +case PSCI_0_2_FN32_MIGRATE_INFO_TYPE:
> +case PSCI_0_2_FN32_SYSTEM_OFF:
> +case PSCI_0_2_FN32_SYSTEM_RESET:
> +case PSCI_1_0_FN32_PSCI_FEATURES:
> +return 0;
> +default:
> +return PSCI_NOT_SUPPORTED;
> +}
> +}
> +
>  #define PSCI_SET_RESULT(reg, val) set_user_reg(reg, 0, val)
>  #define PSCI_ARG(reg, n) get_user_reg(reg, n)
>  
> @@ -304,6 +331,16 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, 
> uint32_t fid)
>  PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff));
>  return true;
>  }
> +
> +case PSCI_1_0_FN32_PSCI_FEATURES:
> +{
> +uint32_t psci_func_id = PSCI_ARG32(regs, 1);
> +
> +perfc_incr(vpsci_features);
> +PSCI_SET_RESULT(regs, do_psci_1_0_features(psci_func_id));
> +return true;
> +}
> +
>  default:
>  return false;
>  }
> diff --git a/xen/include/asm-arm/perfc_defn.h 
> b/xen/include/asm-arm/perfc_defn.h
> index a7acb7d21c..87866264ca 100644
> --- a/xen/include/asm-arm/perfc_defn.h
> +++ b/xen/include/asm-arm/perfc_defn.h
> @@ -31,6 +31,7 @@ PERFCOUNTER(vpsci_system_off,  "vpsci: system_off")
>  PERFCOUNTER(vpsci_system_reset,"vpsci: system_reset")
>  PERFCOUNTER(vpsci_cpu_suspend, "vpsci: cpu_suspend")
>  

Re: [Xen-devel] [PATCH v3 01/17] xen/arm: vpsci: Add support for PSCI 1.1

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall wrote:
> At the moment, Xen provides virtual PSCI interface compliant with 0.1
> and 0.2. Since them, the specification has been updated and the latest
> version is 1.1 (see ARM DEN 0022D).
> 
> >From an implementation point of view, only PSCI_FEATURES is mandatory.
> The rest is optional and can be left unimplemented for now.
> 
> At the same time, the compatible for PSCI node have been updated to
> expose "arm,psci-1.0".
> 
> Signed-off-by: Julien Grall 
> Acked-by: Wei Liu 
> Reviewed-by: Volodymyr Babchuk 
> Cc: Ian Jackson 
> Cc: mirela.simono...@aggios.com

Acked-by: Stefano Stabellini 


> ---
> We may want to provide a way for the toolstack to specify a PSCI
> version. This could be useful if a guest is expecting a given
> version.
> 
> Changes in v3:
> - Add Wei's acked-by
> - Add Volodymyr's reviewed-by
> 
> Changes in v2:
> - Return v1.1 on GET_VERSION call as claimed by this patch
> - Order by function ID the calls in FEATURES call
> ---
>  tools/libxl/libxl_arm.c  |  3 ++-
>  xen/arch/arm/domain_build.c  |  1 +
>  xen/arch/arm/vpsci.c | 39 ++-
>  xen/include/asm-arm/perfc_defn.h |  1 +
>  xen/include/asm-arm/psci.h   |  1 +
>  xen/include/asm-arm/vpsci.h  |  2 +-
>  6 files changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 3e46554301..86f59c0d80 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -410,7 +410,8 @@ 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, 2, "arm,psci-0.2","arm,psci");
> +res = fdt_property_compat(gc, fdt, 3, "arm,psci-1.0",
> +  "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 155c952349..941688a2ce 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -637,6 +637,7 @@ static int make_psci_node(void *fdt, const struct 
> dt_device_node *parent)
>  {
>  int res;
>  const char compat[] =
> +"arm,psci-1.0""\0"
>  "arm,psci-0.2""\0"
>  "arm,psci";
>  
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 6ab8ab64d0..e82b62db1a 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -106,7 +106,11 @@ static int32_t do_psci_cpu_off(uint32_t power_state)
>  
>  static uint32_t do_psci_0_2_version(void)
>  {
> -return PSCI_VERSION(0, 2);
> +/*
> + * PSCI is backward compatible from 0.2. So we can bump the version
> + * without any issue.
> + */
> +return PSCI_VERSION(1, 1);
>  }
>  
>  static register_t do_psci_0_2_cpu_suspend(uint32_t power_state,
> @@ -191,6 +195,29 @@ static void do_psci_0_2_system_reset(void)
>  domain_shutdown(d,SHUTDOWN_reboot);
>  }
>  
> +static int32_t do_psci_1_0_features(uint32_t psci_func_id)
> +{
> +/* /!\ Ordered by function ID and not name */
> +switch ( psci_func_id )
> +{
> +case PSCI_0_2_FN32_PSCI_VERSION:
> +case PSCI_0_2_FN32_CPU_SUSPEND:
> +case PSCI_0_2_FN64_CPU_SUSPEND:
> +case PSCI_0_2_FN32_CPU_OFF:
> +case PSCI_0_2_FN32_CPU_ON:
> +case PSCI_0_2_FN64_CPU_ON:
> +case PSCI_0_2_FN32_AFFINITY_INFO:
> +case PSCI_0_2_FN64_AFFINITY_INFO:
> +case PSCI_0_2_FN32_MIGRATE_INFO_TYPE:
> +case PSCI_0_2_FN32_SYSTEM_OFF:
> +case PSCI_0_2_FN32_SYSTEM_RESET:
> +case PSCI_1_0_FN32_PSCI_FEATURES:
> +return 0;
> +default:
> +return PSCI_NOT_SUPPORTED;
> +}
> +}
> +
>  #define PSCI_SET_RESULT(reg, val) set_user_reg(reg, 0, val)
>  #define PSCI_ARG(reg, n) get_user_reg(reg, n)
>  
> @@ -304,6 +331,16 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, 
> uint32_t fid)
>  PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff));
>  return true;
>  }
> +
> +case PSCI_1_0_FN32_PSCI_FEATURES:
> +{
> +uint32_t psci_func_id = PSCI_ARG32(regs, 1);
> +
> +perfc_incr(vpsci_features);
> +PSCI_SET_RESULT(regs, do_psci_1_0_features(psci_func_id));
> +return true;
> +}
> +
>  default:
>  return false;
>  }
> diff --git a/xen/include/asm-arm/perfc_defn.h 
> b/xen/include/asm-arm/perfc_defn.h
> index a7acb7d21c..87866264ca 100644
> --- a/xen/include/asm-arm/perfc_defn.h
> +++ b/xen/include/asm-arm/perfc_defn.h
> @@ -31,6 +31,7 @@ PERFCOUNTER(vpsci_system_off,  "vpsci: system_off")
>  PERFCOUNTER(vpsci_system_reset,"vpsci: system_reset")
>  PERFCOUNTER(vpsci_cpu_suspend, "vpsci: cpu_suspend")
>