From mboxrd@z Thu Jan 1 00:00:00 1970 From: Parth Dixit Subject: Re: [PATCH v3 2/2] xen/arm : emulation of arm's PSCI v0.2 standard Date: Thu, 10 Jul 2014 16:04:37 +0530 Message-ID: References: <1404390158-21542-1-git-send-email-parth.dixit@linaro.org> <1404390158-21542-2-git-send-email-parth.dixit@linaro.org> <1404912270.16789.52.camel@kazak.uk.xensource.com> <1404988104.32404.23.camel@hastur.hellion.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1404988104.32404.23.camel@hastur.hellion.org.uk> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Stefano Stabellini , tim@xen.org, Christoffer Dall , Julien Grall , xen-devel List-Id: xen-devel@lists.xenproject.org Hi, On 10 July 2014 15:58, Ian Campbell wrote: > On Thu, 2014-07-10 at 15:44 +0530, Parth Dixit wrote: >> Hi, >> >> On 9 July 2014 18:54, Ian Campbell 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