Re: [Xen-devel] [RFC PATCH 16/31] arm: add SMC wrapper that is compatible with SMCCC

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 5 Dec 2017, Volodymyr Babchuk wrote:
> Hi Stefano,
> 
> On Mon, Dec 04, 2017 at 06:30:13PM -0800, Stefano Stabellini wrote:
> > On Thu, 9 Nov 2017, Oleksandr Tyshchenko wrote:
> > > From: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
> > > 
> > > Existing SMC wrapper call_smc() allows only 4 parameters and
> > > returns only one value. This is enough for existing
> > > use in PSCI code, but TEE mediator will need a call that is
> > > fully compatible with ARM SMCCC.
> > > This patch adds this call for both arm32 and arm64.
> > > 
> > > There was similar patch by Edgar E. Iglesias ([1]), but looks
> > > like it is abandoned.
> > > 
> > > [1] 
> > > https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg00636.html
> > > 
> > > CC: "Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxxx>
> > > 
> > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
> > > CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > CC: Julien Grall <julien.grall@xxxxxxxxxx>
> > > ---
> > >  xen/arch/arm/arm32/Makefile     |  1 +
> > >  xen/arch/arm/arm32/smc.S        | 32 ++++++++++++++++++++++++++++++++
> > >  xen/arch/arm/arm64/Makefile     |  1 +
> > >  xen/arch/arm/arm64/smc.S        | 29 +++++++++++++++++++++++++++++
> > >  xen/include/asm-arm/processor.h |  4 ++++
> > >  5 files changed, 67 insertions(+)
> > >  create mode 100644 xen/arch/arm/arm32/smc.S
> > >  create mode 100644 xen/arch/arm/arm64/smc.S
> > > 
> > > diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
> > > index 0ac254f..a2362f3 100644
> > > --- a/xen/arch/arm/arm32/Makefile
> > > +++ b/xen/arch/arm/arm32/Makefile
> > > @@ -8,6 +8,7 @@ obj-y += insn.o
> > >  obj-$(CONFIG_LIVEPATCH) += livepatch.o
> > >  obj-y += proc-v7.o proc-caxx.o
> > >  obj-y += smpboot.o
> > > +obj-y += smc.o
> > >  obj-y += traps.o
> > >  obj-y += vfp.o
> > >  
> > > diff --git a/xen/arch/arm/arm32/smc.S b/xen/arch/arm/arm32/smc.S
> > > new file mode 100644
> > > index 0000000..1cc9528
> > > --- /dev/null
> > > +++ b/xen/arch/arm/arm32/smc.S
> > > @@ -0,0 +1,32 @@
> > > +/*
> > > + * xen/arch/arm/arm32/smc.S
> > > + *
> > > + * Wrapper for Secure Monitors Calls
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License for more details.
> > > + */
> > > +
> > > +#include <asm/macros.h>
> > > +
> > > +/*
> > > + * void call_smccc_smc(register_t a0, register_t a1, register_t a2,
> > > + *                     register_t a3, register_t a4, register_t a5,
> > > + *                     register_t a6, register_t a7, register_t res[4])
> > > + */
> > > +ENTRY(call_smccc_smc)
> > > +        mov     r12, sp
> > > +        push    {r4-r7}
> > > +        ldm     r12, {r4-r7}
> > > +        smc     #0
> > > +        pop     {r4-r7}
> > > +        ldr     r12, [sp, #(4 * 4)]
> > 
> > I haven't run this, but shouldn't it be:
> > 
> >   ldr     r12, [sp, #20]
> > 
> > ?
> > 
> I took this code from linux (arch/arm/kernel/arm-smccc.h).
> But, why #20? There are 5 parameters on the stack: a4-a7 and res:
> a4:  [sp]
> a5:  [sp, #4]
> a6:  [sp, #8]
> a7:  [sp, #12]
> res: [sp, #16]
> 
> We need to save returnred values to res. So it looks right. Unless
> I'm terribly wrong :)

Ops, I miscounted.
When taking code from Linux, it would be nice to say where you took it
from. Also, you definitely need to add the right signed-off-by lines for
copyright reasons.

Cheers,

Stefano

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel