Re: [Xen-devel] [PATCH v5 2/7] xen/arm: zynqmp: Forward plaform specific firmware calls

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 11 Dec 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 03/12/2018 21:03, Stefano Stabellini wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxxx>
> > 
> > From: Edgar E. Iglesias <edgar.iglesias@xxxxxxxxxx>
> > 
> > Introduce zynqmp_eemi: a function responsible for implementing access
> > controls over the firmware calls. Only calls that are allowed are
> > forwarded to the firmware.
> > 
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xxxxxxxxxx>
> > Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> > 
> > ---
> > Changes in v4:
> > - fix typo
> > - add header guard
> > - add emacs magic
> > - remove #includes that will only be used later
> > - add copyright notice to header
> > - remove SMCCC 1.1 check
> > ---
> >   xen/arch/arm/platforms/Makefile                    |  1 +
> >   xen/arch/arm/platforms/xilinx-zynqmp-eemi.c        | 34
> > ++++++++++++++++++++++
> >   xen/arch/arm/platforms/xilinx-zynqmp.c             | 11 +++++++
> >   xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 30
> > +++++++++++++++++++
> >   4 files changed, 76 insertions(+)
> >   create mode 100644 xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> >   create mode 100644 xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> > 
> > diff --git a/xen/arch/arm/platforms/Makefile
> > b/xen/arch/arm/platforms/Makefile
> > index a79bdb9..fe8e0c7 100644
> > --- a/xen/arch/arm/platforms/Makefile
> > +++ b/xen/arch/arm/platforms/Makefile
> > @@ -9,3 +9,4 @@ obj-y += sunxi.o
> >   obj-$(CONFIG_ARM_64) += thunderx.o
> >   obj-$(CONFIG_ARM_64) += xgene-storm.o
> >   obj-$(CONFIG_MPSOC_PLATFORM)  += xilinx-zynqmp.o
> > +obj-$(CONFIG_MPSOC_PLATFORM)  += xilinx-zynqmp-eemi.o
> > diff --git a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> > b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> > new file mode 100644
> > index 0000000..369bb3f
> > --- /dev/null
> > +++ b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> > @@ -0,0 +1,34 @@
> > +/*
> > + * xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> > + *
> > + * Xilinx ZynqMP EEMI API
> > + *
> > + * Copyright (c) 2018 Xilinx Inc.
> > + * Written by Edgar E. Iglesias <edgar.iglesias@xxxxxxxxxx>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms and conditions of the GNU General Public
> > + * License, version 2, as published by the Free Software Foundation.
> > + *
> > + * 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/regs.h>
> > +#include <asm/platforms/xilinx-zynqmp-eemi.h>
> > +
> > +bool zynqmp_eemi(struct cpu_user_regs *regs)
> > +{
> > +    return false;
> > +}
> > +
> > +/*
> > + * Local variables:
> > + * mode: C
> > + * c-file-style: "BSD"
> > + * c-basic-offset: 4
> > + * indent-tabs-mode: nil
> > + * End:
> > + */
> > diff --git a/xen/arch/arm/platforms/xilinx-zynqmp.c
> > b/xen/arch/arm/platforms/xilinx-zynqmp.c
> > index d8ceded..9c174d2 100644
> > --- a/xen/arch/arm/platforms/xilinx-zynqmp.c
> > +++ b/xen/arch/arm/platforms/xilinx-zynqmp.c
> > @@ -18,6 +18,8 @@
> >    */
> >     #include <asm/platform.h>
> > +#include <asm/platforms/xilinx-zynqmp-eemi.h>
> > +#include <asm/smccc.h>
> >     static const char * const zynqmp_dt_compat[] __initconst =
> >   {
> > @@ -32,8 +34,17 @@ static const struct dt_device_match
> > zynqmp_blacklist_dev[] __initconst =
> >       { /* sentinel */ },
> >   };
> >   +static bool zynqmp_smc(struct cpu_user_regs *regs)
> > +{
> > +    if ( !is_64bit_domain(current->domain) )
> 
> Please document why you only expose eemi to 64-bit domain. What if the user
> start with 32-bit Dom0?

I'll add a in-code comment saying that only 64-bit guests are supported
by the current implementation -- the 32 bit EEMI ABI is not yet covered.


> > +        return false;
> > +
> > +    return  zynqmp_eemi(regs);
> 
> double space.

I'll fix


> > +}
> > +
> >   PLATFORM_START(xilinx_zynqmp, "Xilinx ZynqMP")
> >       .compatible = zynqmp_dt_compat,
> > +    .smc = zynqmp_smc,
> >       .blacklist_dev = zynqmp_blacklist_dev,
> >   PLATFORM_END
> >   diff --git a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> > b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> > new file mode 100644
> > index 0000000..43cefb5
> > --- /dev/null
> > +++ b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> > @@ -0,0 +1,30 @@
> > +/*
> > + * Copyright (c) 2018 Xilinx Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms and conditions of the GNU General Public
> > + * License, version 2, as published by the Free Software Foundation.
> > + *
> > + * 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.
> > + */
> > +
> > +#ifndef __ASM_ARM_PLATFORMS_ZYNQMP_H
> > +#define __ASM_ASM_PLATFORMS_ZYNQMP_H
> > +
> > +#include <asm/processor.h>
> > +
> > +extern bool zynqmp_eemi(struct cpu_user_regs *regs);
> > +
> > +#endif /* __ASM_ARM_PLATFORMS_ZYNQMP_H */
> > +
> > +/*
> > + * Local variables:
> > + * mode: C
> > + * c-file-style: "BSD"
> > + * c-basic-offset: 4
> > + * indent-tabs-mode: nil
> > + * End:
> > + */
> > 
> 
> Cheers,
> 
> -- 
> Julien Grall
> 

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