diff mbox

[Xen-devel] xen: arm: correct guest PSCI handling on 64-bit hypervisor.

Message ID 1389720774-27931-1-git-send-email-ian.campbell@citrix.com
State Accepted
Commit f26d849dff24120e6ad633db94abfbc6e6572503
Headers show

Commit Message

Ian Campbell Jan. 14, 2014, 5:32 p.m. UTC 
Using ->rN truncates the 64-bit registers to 32-bits, which on X-gene chops
off the top bit of the entry address for PSCI_UP.

Follow the pattern established in do_trap_hypercall.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---

Release argument: Only supporting single vcpu guests on arm64 would be
unfortunate. There is no risk to arm32 since the ifdef ensures the code
remains the same.
---
 xen/arch/arm/traps.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Comments

Julien Grall Jan. 14, 2014, 6:57 p.m. UTC | #1 
On 01/14/2014 05:32 PM, Ian Campbell wrote:
> Using ->rN truncates the 64-bit registers to 32-bits, which on X-gene chops
> off the top bit of the entry address for PSCI_UP.
> 
> Follow the pattern established in do_trap_hypercall.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Acked-by: Julien Grall <julien.grall@linaro.org>

> ---
> 
> Release argument: Only supporting single vcpu guests on arm64 would be
> unfortunate. There is no risk to arm32 since the ifdef ensures the code
> remains the same.
> ---
>  xen/arch/arm/traps.c |   17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index fdf9440..62c9df2 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1065,23 +1065,34 @@ 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
> +#else
> +#define PSCI_OP_REG(r) (r)->r0
> +#define PSCI_RESULT_REG(r) (r)->r0
> +#define PSCI_ARGS(r) (r)->r1, (r)->r2
> +#endif
> +
>  static void do_trap_psci(struct cpu_user_regs *regs)
>  {
>      arm_psci_fn_t psci_call = NULL;
>  
> -    if ( regs->r0 >= ARRAY_SIZE(arm_psci_table) )
> +    if ( PSCI_OP_REG(regs) >= ARRAY_SIZE(arm_psci_table) )
>      {
>          domain_crash_synchronous();
>          return;
>      }
>  
> -    psci_call = arm_psci_table[regs->r0].fn;
> +    psci_call = arm_psci_table[PSCI_OP_REG(regs)].fn;
>      if ( psci_call == NULL )
>      {
>          domain_crash_synchronous();
>          return;
>      }
> -    regs->r0 = psci_call(regs->r1, regs->r2);
> +
> +    PSCI_RESULT_REG(regs) = psci_call(PSCI_ARGS(regs));
>  }
>  
>  #ifdef CONFIG_ARM_64
>
Ian Campbell Jan. 17, 2014, 10:25 a.m. UTC | #2 
On Tue, 2014-01-14 at 18:57 +0000, Julien Grall wrote:
> On 01/14/2014 05:32 PM, Ian Campbell wrote:
> > Using ->rN truncates the 64-bit registers to 32-bits, which on X-gene chops
> > off the top bit of the entry address for PSCI_UP.
> > 
> > Follow the pattern established in do_trap_hypercall.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Acked-by: Julien Grall <julien.grall@linaro.org>

Applied thanks.

> > ---
> > 
> > Release argument: Only supporting single vcpu guests on arm64 would be
> > unfortunate. There is no risk to arm32 since the ifdef ensures the code
> > remains the same.
> > ---
> >  xen/arch/arm/traps.c |   17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > index fdf9440..62c9df2 100644
> > --- a/xen/arch/arm/traps.c
> > +++ b/xen/arch/arm/traps.c
> > @@ -1065,23 +1065,34 @@ 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
> > +#else
> > +#define PSCI_OP_REG(r) (r)->r0
> > +#define PSCI_RESULT_REG(r) (r)->r0
> > +#define PSCI_ARGS(r) (r)->r1, (r)->r2
> > +#endif
> > +
> >  static void do_trap_psci(struct cpu_user_regs *regs)
> >  {
> >      arm_psci_fn_t psci_call = NULL;
> >  
> > -    if ( regs->r0 >= ARRAY_SIZE(arm_psci_table) )
> > +    if ( PSCI_OP_REG(regs) >= ARRAY_SIZE(arm_psci_table) )
> >      {
> >          domain_crash_synchronous();
> >          return;
> >      }
> >  
> > -    psci_call = arm_psci_table[regs->r0].fn;
> > +    psci_call = arm_psci_table[PSCI_OP_REG(regs)].fn;
> >      if ( psci_call == NULL )
> >      {
> >          domain_crash_synchronous();
> >          return;
> >      }
> > -    regs->r0 = psci_call(regs->r1, regs->r2);
> > +
> > +    PSCI_RESULT_REG(regs) = psci_call(PSCI_ARGS(regs));
> >  }
> >  
> >  #ifdef CONFIG_ARM_64
> > 
> 
>
diff mbox

Patch

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index fdf9440..62c9df2 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1065,23 +1065,34 @@  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
+#else
+#define PSCI_OP_REG(r) (r)->r0
+#define PSCI_RESULT_REG(r) (r)->r0
+#define PSCI_ARGS(r) (r)->r1, (r)->r2
+#endif
+
 static void do_trap_psci(struct cpu_user_regs *regs)
 {
     arm_psci_fn_t psci_call = NULL;
 
-    if ( regs->r0 >= ARRAY_SIZE(arm_psci_table) )
+    if ( PSCI_OP_REG(regs) >= ARRAY_SIZE(arm_psci_table) )
     {
         domain_crash_synchronous();
         return;
     }
 
-    psci_call = arm_psci_table[regs->r0].fn;
+    psci_call = arm_psci_table[PSCI_OP_REG(regs)].fn;
     if ( psci_call == NULL )
     {
         domain_crash_synchronous();
         return;
     }
-    regs->r0 = psci_call(regs->r1, regs->r2);
+
+    PSCI_RESULT_REG(regs) = psci_call(PSCI_ARGS(regs));
 }
 
 #ifdef CONFIG_ARM_64