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

[Parth Dixit <parth.dixit@xxxxxxxxxx>]

Hi,

On 10 July 2014 15:58, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> On Thu, 2014-07-10 at 15:44 +0530, Parth Dixit wrote:
>> Hi,
>>
>> On 9 July 2014 18:54, Ian Campbell <Ian.Campbell@xxxxxxxxxx> 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.
>
Ok, got it, will take care in patcset v4

> Ian.
>

Thanks
parth

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