Re: [PATCH v3 2/2] xen/arm : emulation of arm's PSCI v0.2 standard

[Ian Campbell <ian.campbell@citrix.com>]

On Thu, 2014-07-10 at 15:44 +0530, Parth Dixit wrote:
> Hi,
> 
> On 9 July 2014 18:54, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Thu, 2014-07-03 at 17:52 +0530, Parth Dixit wrote:
> >>  static void do_trap_psci(struct cpu_user_regs *regs)
> >>  {
> >>      arm_psci_fn_t psci_call = NULL;
> >>
> >> -    if ( PSCI_OP_REG(regs) >= ARRAY_SIZE(arm_psci_table) )
> >> +    if ( PSCI_OP_REG(regs) < PSCI_migrate )
> >>      {
> >> -        domain_crash_synchronous();
> >> -        return;
> >> +        if ( PSCI_OP_REG(regs) >= ARRAY_SIZE(arm_psci_table) )
> >> +        {
> >> +            domain_crash_synchronous();
> >> +            return;
> >> +        }
> >> +        psci_call = arm_psci_table[PSCI_OP_REG(regs)].fn;
> >> +    }
> >> +    else
> >> +    {
> >> +        if ( ( PSCI_OP_REG(regs) & PSCI_FN_MASK ) >= ARRAY_SIZE(arm_psci_0_2_table) )
> >
> > I think you need to also check that "PSCI_OP_REG(regs) & ~PSCI_FN_MASK"
> > is either the 32-bit or 64-bit base. Otherwise I could call 0xdeadfe01
> > and it would work.
> >
> > Do you not also need to check that the guest is of the appropriate type?
> > Is an aarch64 guest allowed to call the aarch32 entry points? The spec
> > doesn't say so explicitly AFAICT.
> >
> > If it is allowed then I think you need to be truncating the 32-bit
> > arguments to 32-bits when an aarch64 guest calls the 32-bit entry
> > points.
> >
> > Hrm, checking through the spec I see now that only some of the functions
> > have both a 32- and 64-bit function id. VERSION, CPU_OFF,
> > MIGRATE_INFO_TYPE, SYSTEM_OFF and SYSTEM_RESET only have a single
> > function id (I suppose because they do not take any arguments which are
> > mode specific).
> >
> > I'm afraid that AFAICT you need to handle this distinction, which I
> > expect makes it rather hard to share the same lookup table between 32-
> > and 64-bit. A switch(PSCI_OP_REG(regs)) is looking increasingly like the
> > correct approach to me.
> 
> I am bit confused, are you saying for eg. aarch64 can call
> "psci_0_2_suspend" function with aarch32 function id (effectively 32
> bit version of function?)

SUSPEND is not a good example since it has both 32- and 64-bit fn ids.

But e.g CPU_OFF only defines a single id, 0x84000002. I think calling
c4000002 would need to be rejected.

It seems like the SMC Calling Convention doc which I referenced covers
the requirements here.

Ian.