[PATCH v3 1/6] arm: imx8m: add OP-TEE node

[Yannic Moog]

Hi Tim, Fabio,

On Thu, 2025-01-16 at 14:45 -0800, Tim Harvey wrote:
> On Fri, Jan 10, 2025 at 4:35 AM Yannic Moog <y.moog at phytec.de> wrote:
> > 
> > Add tee node in SoC u-boot device trees. Use a kconfig entry to specify
> > load and entry addresses for the op-tee image in the respective
> > defconfig.
> > Default IMX8M_OPTEE_LOAD_ADDR is supplied. To keep it simple, the same
> > addresses are used for each SoC as are defined in upstream tf-a
> > (BL32_BASE) [1].
> > 
> > [1] https://github.com/ARM-software/arm-trusted-firmware/tree/master/plat/imx/imx8m
> > 
> > Signed-off-by: Yannic Moog <y.moog at phytec.de>
> > ---
> >  arch/arm/dts/imx8mm-u-boot.dtsi | 17 ++++++++++++++++-
> >  arch/arm/dts/imx8mn-u-boot.dtsi | 17 ++++++++++++++++-
> >  arch/arm/dts/imx8mp-u-boot.dtsi | 17 ++++++++++++++++-
> >  arch/arm/dts/imx8mq-u-boot.dtsi | 17 ++++++++++++++++-
> >  arch/arm/mach-imx/imx8m/Kconfig | 11 +++++++++++
> >  5 files changed, 75 insertions(+), 4 deletions(-)
> > 
> > 

[...]

> > diff --git a/arch/arm/dts/imx8mq-u-boot.dtsi b/arch/arm/dts/imx8mq-u-boot.dtsi
> > index d7a83a78f4d..93e2ef27f7c 100644
> > --- a/arch/arm/dts/imx8mq-u-boot.dtsi
> > +++ b/arch/arm/dts/imx8mq-u-boot.dtsi
> > @@ -144,6 +144,21 @@
> >                                         };
> >  #endif
> > 
> > +                                       tee: tee {
> > +                                               description = "OP-TEE";
> > +                                               type = "tee";
> > +                                               arch = "arm64";
> > +                                               compression = "none";
> > +                                               os = "tee";
> > +                                               load = <CONFIG_IMX8M_OPTEE_LOAD_ADDR>;
> > +                                               entry = <CONFIG_IMX8M_OPTEE_LOAD_ADDR>;
> > +
> > +                                               tee-os {
> > +                                                       filename = "tee.bin";
> > +                                                       optional;
> > +                                               };
> > +                                       };
> > +
> >                                         fdt {
> >                                                 compression = "none";
> >                                                 description = "NAME";
> > @@ -164,7 +179,7 @@
> >                                                 fdt = "fdt";
> >                                                 firmware = "uboot";
> >  #ifndef CONFIG_ARMV8_PSCI
> > -                                               loadables = "atf";
> > +                                               loadables = "atf", "tee";
> >  #endif
> >                                         };
> >                                 };
> 
> Hi Yannic,
> 
> As I mentioned before, I still don't see how this builds when tee.bin
> is missing because someone isn't using tee?

Well, because tee.bin is marked optional. In my initial series, this worked as expected.
All you got was 

  BINMAN  .binman_stamp
Image 'image' is missing optional external blobs but is still functional: tee-os


Since Simon submitted [1], that behaviour changed and you got 2 messages. One error, one warning,
both contradicting each other.
I think this is a bug in binman and I highlighted this to Simon.

> If tee is not used, we should not get warnings about missing tee.bin .
> 
> I sent a patch. Please give it a try.

I see that Fabio does not want any warnings to be emitted regardless.
I would like to give some explanation why I implemented it that way and why I still think this is an
adequate solution:

The OPTEE kconfig is for U-Boot driver support. To my knowledge however, this is not needed to have
OP-TEE be packaged into the final image, thus I should not have to enable OP-TEE driver support to
be able to package optee.

Please give me your opinion on this matter.

Yannic

[1] https://lore.kernel.org/u-boot/20241106153544.1277835-1-sjg@chromium.org/

> 
> It looks like Fabio has merged this already to imx/master which now
> fails to build for imx8m*_venice boards because of it.
> 
> Fabio, did you realize that you merged this already and if so how does
> it work for you?
> 
> Best Regards,
> 
> Tim
> 
> > diff --git a/arch/arm/mach-imx/imx8m/Kconfig b/arch/arm/mach-imx/imx8m/Kconfig
> > index b254a50b091..8aeb1ea75f7 100644
> > --- a/arch/arm/mach-imx/imx8m/Kconfig
> > +++ b/arch/arm/mach-imx/imx8m/Kconfig
> > @@ -37,6 +37,17 @@ config SYS_HAS_ARMV8_SECURE_BASE
> >           If enabled, please also define the value for ARMV8_SECURE_BASE,
> >           for i.MX8M, it could be some address in OCRAM.
> > 
> > +config IMX8M_OPTEE_LOAD_ADDR
> > +       hex "Load address of OPTEE image"
> > +       default 0xbe000000 if IMX8MM
> > +       default 0xbe000000 if IMX8MN
> > +       default 0x56000000 if IMX8MP
> > +       default 0xfe000000 if IMX8MQ
> > +       help
> > +         The load and entry address for the OPTEE image. This value defaults to
> > +         the one defined in tf-a if not provided in the board defconfig file.
> > +
> > +
> >  choice
> >         prompt "NXP i.MX8M board select"
> >         optional
> > 
> > --
> > 2.43.0
> >