Re: [Xen-devel] [PATCH 1/7] xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu

2018-02-09 Thread Volodymyr Babchuk 
Hi Julien,

On 8 February 2018 at 20:12, Julien Grall  wrote:

Currently, the behavior of do_common_cpu will slightly change depending
>>> on the PSCI version passed in parameter. Looking at the code, more the
>>> specific 0.2 behavior could move out of the function or adapted for 0.1:
>>>
>>>  - x0/r0 can be updated on PSCI 0.1 because general purpose registers
>>>  are undefined upon CPU on.
>>>  - PSCI 0.1 does not defined PSCI_ALREADY_ON. However, it would be
>>>  safer to bail out if the CPU is already on.
>>>
>>> Based on this, the parameter 'ver' is removed and do_psci_cpu_on
>>> (implementation for PSCI 0.1) is adapted to avoid returning
>>> PSCI_ALREADY_ON.
>>>
>>> Signed-off-by: Julien Grall 
>>
>> Reviewed-by: Volodymyr Babchuk 
>
>
> Thank you for the reviewed. FIY, I moved that patch towards the end of the
> series as it is not necessary for backporting. I kept your reviewed-by
> because there are no clash.
>
> I hope that is fine for you.
Yes, I'm perfectly fine with this.


-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babc...@gmail.com

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

Re: [Xen-devel] [PATCH 1/7] xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu

2018-02-08 Thread Julien Grall 



On 06/02/18 15:42, Volodymyr Babchuk wrote:

Hi Julien,


Hi Volodymyr,


On 5 February 2018 at 15:20, Julien Grall  wrote:

Currently, the behavior of do_common_cpu will slightly change depending
on the PSCI version passed in parameter. Looking at the code, more the
specific 0.2 behavior could move out of the function or adapted for 0.1:

 - x0/r0 can be updated on PSCI 0.1 because general purpose registers
 are undefined upon CPU on.
 - PSCI 0.1 does not defined PSCI_ALREADY_ON. However, it would be
 safer to bail out if the CPU is already on.

Based on this, the parameter 'ver' is removed and do_psci_cpu_on
(implementation for PSCI 0.1) is adapted to avoid returning
PSCI_ALREADY_ON.

Signed-off-by: Julien Grall 

Reviewed-by: Volodymyr Babchuk 


Thank you for the reviewed. FIY, I moved that patch towards the end of 
the series as it is not necessary for backporting. I kept your 
reviewed-by because there are no clash.


I hope that is fine for you.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 1/7] xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu

2018-02-06 Thread Volodymyr Babchuk 
Hi Julien,

On 5 February 2018 at 15:20, Julien Grall  wrote:
> Currently, the behavior of do_common_cpu will slightly change depending
> on the PSCI version passed in parameter. Looking at the code, more the
> specific 0.2 behavior could move out of the function or adapted for 0.1:
>
> - x0/r0 can be updated on PSCI 0.1 because general purpose registers
> are undefined upon CPU on.
> - PSCI 0.1 does not defined PSCI_ALREADY_ON. However, it would be
> safer to bail out if the CPU is already on.
>
> Based on this, the parameter 'ver' is removed and do_psci_cpu_on
> (implementation for PSCI 0.1) is adapted to avoid returning
> PSCI_ALREADY_ON.
>
> Signed-off-by: Julien Grall 
Reviewed-by: Volodymyr Babchuk 

> ---
>  xen/arch/arm/vpsci.c | 28 ++--
>  1 file changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 884f0fa710..359db884f9 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -22,7 +22,7 @@
>  #include 
>
>  static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
> -   register_t context_id,int ver)
> +register_t context_id)
>  {
>  struct vcpu *v;
>  struct domain *d = current->domain;
> @@ -40,8 +40,7 @@ static int do_common_cpu_on(register_t target_cpu, 
> register_t entry_point,
>  if ( is_64bit_domain(d) && is_thumb )
>  return PSCI_INVALID_PARAMETERS;
>
> -if ( (ver == PSCI_VERSION(0, 2)) &&
> -!test_bit(_VPF_down, >pause_flags) )
> +if ( !test_bit(_VPF_down, >pause_flags) )
>  return PSCI_ALREADY_ON;
>
>  if ( (ctxt = alloc_vcpu_guest_context()) == NULL )
> @@ -55,18 +54,21 @@ static int do_common_cpu_on(register_t target_cpu, 
> register_t entry_point,
>  ctxt->ttbr0 = 0;
>  ctxt->ttbr1 = 0;
>  ctxt->ttbcr = 0; /* Defined Reset Value */
> +
> +/*
> + * x0/r0_usr are always updated because for PSCI 0.1 the general
> + * purpose registers are undefined upon CPU_on.
> + */
>  if ( is_32bit_domain(d) )
>  {
>  ctxt->user_regs.cpsr = PSR_GUEST32_INIT;
> -if ( ver == PSCI_VERSION(0, 2) )
> -ctxt->user_regs.r0_usr = context_id;
> +ctxt->user_regs.r0_usr = context_id;
>  }
>  #ifdef CONFIG_ARM_64
>  else
>  {
>  ctxt->user_regs.cpsr = PSR_GUEST64_INIT;
> -if ( ver == PSCI_VERSION(0, 2) )
> -ctxt->user_regs.x0 = context_id;
> +ctxt->user_regs.x0 = context_id;
>  }
>  #endif
>
> @@ -93,7 +95,14 @@ static int do_common_cpu_on(register_t target_cpu, 
> register_t entry_point,
>
>  static int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point)
>  {
> -return do_common_cpu_on(vcpuid, entry_point, 0 , PSCI_VERSION(0, 1));
> +int32_t ret;
> +
> +ret = do_common_cpu_on(vcpuid, entry_point, 0);
> +/*
> + * PSCI 0.1 does not define the return code PSCI_ALREADY_ON.
> + * Instead, return PSCI_INVALID_PARAMETERS.
> + */
> +return (ret == PSCI_ALREADY_ON) ? PSCI_INVALID_PARAMETERS : ret;
>  }
>
>  static int32_t do_psci_cpu_off(uint32_t power_state)
> @@ -133,8 +142,7 @@ static 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,
> -PSCI_VERSION(0, 2));
> +return do_common_cpu_on(target_cpu, entry_point, context_id);
>  }
>
>  static const unsigned long target_affinity_mask[] = {
> --
> 2.11.0
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel



-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babc...@gmail.com

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

[Xen-devel] [PATCH 1/7] xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu

2018-02-05 Thread Julien Grall 
Currently, the behavior of do_common_cpu will slightly change depending
on the PSCI version passed in parameter. Looking at the code, more the
specific 0.2 behavior could move out of the function or adapted for 0.1:

- x0/r0 can be updated on PSCI 0.1 because general purpose registers
are undefined upon CPU on.
- PSCI 0.1 does not defined PSCI_ALREADY_ON. However, it would be
safer to bail out if the CPU is already on.

Based on this, the parameter 'ver' is removed and do_psci_cpu_on
(implementation for PSCI 0.1) is adapted to avoid returning
PSCI_ALREADY_ON.

Signed-off-by: Julien Grall 
---
 xen/arch/arm/vpsci.c | 28 ++--
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index 884f0fa710..359db884f9 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -22,7 +22,7 @@
 #include 
 
 static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
-   register_t context_id,int ver)
+register_t context_id)
 {
 struct vcpu *v;
 struct domain *d = current->domain;
@@ -40,8 +40,7 @@ static int do_common_cpu_on(register_t target_cpu, register_t 
entry_point,
 if ( is_64bit_domain(d) && is_thumb )
 return PSCI_INVALID_PARAMETERS;
 
-if ( (ver == PSCI_VERSION(0, 2)) &&
-!test_bit(_VPF_down, >pause_flags) )
+if ( !test_bit(_VPF_down, >pause_flags) )
 return PSCI_ALREADY_ON;
 
 if ( (ctxt = alloc_vcpu_guest_context()) == NULL )
@@ -55,18 +54,21 @@ static int do_common_cpu_on(register_t target_cpu, 
register_t entry_point,
 ctxt->ttbr0 = 0;
 ctxt->ttbr1 = 0;
 ctxt->ttbcr = 0; /* Defined Reset Value */
+
+/*
+ * x0/r0_usr are always updated because for PSCI 0.1 the general
+ * purpose registers are undefined upon CPU_on.
+ */
 if ( is_32bit_domain(d) )
 {
 ctxt->user_regs.cpsr = PSR_GUEST32_INIT;
-if ( ver == PSCI_VERSION(0, 2) )
-ctxt->user_regs.r0_usr = context_id;
+ctxt->user_regs.r0_usr = context_id;
 }
 #ifdef CONFIG_ARM_64
 else
 {
 ctxt->user_regs.cpsr = PSR_GUEST64_INIT;
-if ( ver == PSCI_VERSION(0, 2) )
-ctxt->user_regs.x0 = context_id;
+ctxt->user_regs.x0 = context_id;
 }
 #endif
 
@@ -93,7 +95,14 @@ static int do_common_cpu_on(register_t target_cpu, 
register_t entry_point,
 
 static int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point)
 {
-return do_common_cpu_on(vcpuid, entry_point, 0 , PSCI_VERSION(0, 1));
+int32_t ret;
+
+ret = do_common_cpu_on(vcpuid, entry_point, 0);
+/*
+ * PSCI 0.1 does not define the return code PSCI_ALREADY_ON.
+ * Instead, return PSCI_INVALID_PARAMETERS.
+ */
+return (ret == PSCI_ALREADY_ON) ? PSCI_INVALID_PARAMETERS : ret;
 }
 
 static int32_t do_psci_cpu_off(uint32_t power_state)
@@ -133,8 +142,7 @@ static 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,
-PSCI_VERSION(0, 2));
+return do_common_cpu_on(target_cpu, entry_point, context_id);
 }
 
 static const unsigned long target_affinity_mask[] = {
-- 
2.11.0


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