Re: [Xen-devel] [PATCH 1/2] arm: implement hypercall continuations

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 25 Jul 2012, Ian Campbell wrote:
> On Mon, 2012-07-23 at 14:32 +0100, Ian Campbell wrote:
> > @@ -482,8 +494,15 @@ static void do_trap_hypercall(struct
> > cpu_user_regs *regs, unsigned long iss)
> >      regs->r0 = call(regs->r0, regs->r1, regs->r2, regs->r3,
> > regs->r4);
> >  
> >  #ifndef NDEBUG
> > -    /* clobber registers */
> > -    regs->r1 = regs->r2 = regs->r3 = regs->r4 = regs->r12 =
> > 0xDEADBEEF;
> > +    /* clobber non-argument registers */
> > +    switch ( arm_hypercall_table[regs->r12].nr_args ) {
> > +    case 1: regs->r1 = 0xDEADBEEF;
> > +    case 2: regs->r2 = 0xDEADBEEF;
> > +    case 3: regs->r3 = 0xDEADBEEF;
> > +    case 4: regs->r4 = 0xDEADBEEF;
> > +            break;
> > +    default: BUG();
> > +    }
> >  #endif
> >  } 
> 
> This actually inadvertently changes the behaviour to only clobber the
> arguments which the particular hypercall does not uses, which is
> contrary to the comment in arch-arm.h, which says hypercalls always
> clobber all of those registers.
> 
> However having spoken to Stefano IRL we think that actually better to
> only clobber the ones which are actually used (unless PC has changed, so
> continations work). i.e. to change the ABI such that:

I confirm :)


> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 6e0fe47..a0593e2 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -42,8 +42,9 @@
>   *
>   * The return value is in r0.
>   *
> - * The hypercall will always clobber r0, r1, r2, r3, r4 and r12,
> - * regardless of how many arguments the particular hypercall takes.
> + * The hypercall will clobber the argument registers, except r0 which
> + * is the return value, i.e. a 2 argument hypercall will clobber r1
> + * and a 4 argument hypercall will clobber r1, r2 and r3.
>   *
>   */
> 
> I'll do this and make the corresponding implementation change and respin
> this change on top.
> 
> 

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