Re: [Xen-devel] [PATCH v3 07/11] arm: use r12 to pass the hypercall number

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-03-02 at 14:27 +0000, Stefano Stabellini wrote:
> Use r12 to pass the hypercall number and r0-r4 for the hypercall
> arguments.
> 
> Use the ISS to pass an hypervisor specific tag.
> 
> Remove passing unused registers to arm_hypercall_table: we don't have 6
> arguments hypercalls and we never use 64 bit values as hypercall
> arguments, 64 bit values are only contained within structs passed as
> arguments.
> 
> 
> Changes in v3:
> 
> - move XEN_HYPERCALL_TAG to a public header;
> 
> - clobber register in the debug build;
> 
> - document calling convention;
> 
> - check if arm_hypercall_table[regs->r12] != NULL.
> 
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> ---
>  xen/arch/arm/traps.c          |   31 ++++++++++++++++++++-----------
>  xen/include/public/arch-arm.h |   21 +++++++++++++++++++++
>  2 files changed, 41 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 395d0af..750f85a 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -367,7 +367,6 @@ unsigned long do_arch_0(unsigned int cmd, unsigned long 
> long value)
>  }
>  
>  typedef unsigned long arm_hypercall_t(
> -    unsigned int, unsigned int, unsigned int, unsigned int, unsigned int,
>      unsigned int, unsigned int, unsigned int, unsigned int, unsigned int);
>  
>  #define HYPERCALL(x)                                        \
> @@ -407,18 +406,28 @@ static void do_debug_trap(struct cpu_user_regs *regs, 
> unsigned int code)
>  
>  static void do_trap_hypercall(struct cpu_user_regs *regs, unsigned long iss)
>  {
> +    arm_hypercall_t *call = NULL;
>      local_irq_enable();
>  
> -    regs->r0 = arm_hypercall_table[iss](regs->r0,
> -                             regs->r1,
> -                             regs->r2,
> -                             regs->r3,
> -                             regs->r4,
> -                             regs->r5,
> -                             regs->r6,
> -                             regs->r7,
> -                             regs->r8,
> -                             regs->r9);
> +    if ( iss != XEN_HYPERCALL_TAG  ) {
> +        printk("%s %d: received an alien hypercall iss=%lx\n", __func__ ,
> +                __LINE__ , iss);
> +        regs->r0 = -EINVAL;
> +        return;
> +    }
> +
> +    call = arm_hypercall_table[regs->r12];
> +    if ( call == NULL ) {

Xen coding style calls for { on a new line (twice).

> +        regs->r0 = -ENOSYS;
> +        return;
> +    }
> +
> +    regs->r0 = call(regs->r0, regs->r1, regs->r2, regs->r3, regs->r4);
> +
> +#ifdef DEBUG

Xen actually uses the confusing double negative "#ifndef NDEBUG"...

Since this is an ABI change I want to get it out of the way so I'll
correct both of those as I commit rather than waiting for a resend.

> +    /* clobber registers */
> +    regs->r1 = regs->r2 = regs->r3 = regs->r4 = regs->r12 = 0xDEADBEEF;
> +#endif
>  }
>  
>  static void do_cp15_32(struct cpu_user_regs *regs,
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index e3d5c08..4f16bde 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -27,6 +27,27 @@
>  #ifndef __XEN_PUBLIC_ARCH_ARM_H__
>  #define __XEN_PUBLIC_ARCH_ARM_H__
>  
> +/* hypercall calling convention
> + * ----------------------------
> + *
> + * An hypercall is issued using the ARM HVC instruction.
> + *
> + * The hypercall arguments go on the registers, the first argument on
> + * r0, the second argument on r1, the third on r2, and so on.

While I'm here I'll also clarify that we only go to five arguments and
below that we clobber those registers regardless of the # of arguments
the specific hypercall has.

> + *
> + * The hypercall number is passed on r12.
> + *
> + * The HVC ISS must contain a Xen specific TAG: XEN_HYPERCALL_TAG.
> + *
> + * The return value is passed on r0.
> + *
> + * The hypercall will clobber r0, r1, r2, r3, r4 and r12.
> + *
> + */
> +
> +#define XEN_HYPERCALL_TAG   0XEA1
> +
> +
>  #ifndef __ASSEMBLY__
>  #define ___DEFINE_XEN_GUEST_HANDLE(name, type) \
>      typedef struct { type *p; } __guest_handle_ ## name



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