[PATCH RFC 0/6] imx8(m): add optee node to binman FIT image

[Yannic Moog]

Hey Tim

On Thu, 2024-10-24 at 08:37 -0700, Tim Harvey wrote:
> On Thu, Oct 24, 2024 at 12:04 AM Yannic Moog <y.moog at phytec.de>
> wrote:
> > 
> > There have been attempts to get op-tee node integrated upstream in
> > the
> > past [1][2]. The challenge is on how to handle the load and entry
> > addresses where the op-tee image should be loaded to.
> > Different SoC families and architectures have different RAM base
> > addresses. Further the final addresses can vary from board to board
> > (e.g. depending on populated RAM size).
> > As a result, I propose to solve the issue via device tree where the
> > optee node itself is defined the same as in previous solutions.
> > The difference in this approach is to omit the 'load' and 'entry'
> > properties from the node definition in the <soc>-u-boot.dtsi.
> > These must be defined in the <board>-u-boot.dtsi.
> > 
> > This solution avoids the route via kconfig while still allowing
> > each
> > board to individually define the op-tee load address.
> > This is possible as tee-os node has the 'optional' property.
> > There is no need to keep parts in #ifdefs when no optee integration
> > is
> > desired or possible
> > 
> > I included usage for PHYTEC boards for examples (with
> > documentation).
> > 
> > There is one caveat however:
> > The series in its current form would break all other imx8m{m,n,p}
> > boards. This is due to the fact that they do not define entry and
> > load
> > addr for the tee entry.
> > During runtime, spl_load_simple_fit fails for the OP-TEE node (no
> > load
> > addr).
> > 
> > [...]
> > Can't load tee: No load address and no buffer
> > spl_load_simple_fit: can't load image loadables index 1 (ret = -
> > 105)
> > mmc_load_image_raw_sector: mmc block read error
> > SPL: failed to boot from all boot devices
> > 
> > [3] suggests (Any absent entries are dropped immediately) that
> > using this proposed solution should work and is allowed. However
> > [4] on
> > the other hand clearly states that the entry does not get removed
> > (which
> > is the case), rather its data set to 0.
> > 
> > To me the question now is on how to move forward.
> > - Is my proposed solution undesired in the sense that I try to use
> > a
> >   mechanism that current U-Boot/binman is not designed for?
> > - Should load_simple_fit check and discard optional entries right
> > away
> >   before doing any other processing?
> > - Should binman be changed so that optional entries do in fact get
> >   removed instead of being zeroed?
> > 
> > I am asking for help regarding the questions above since that is
> > not my
> > area of expertise.
> > 
> > [1]
> > https://patchwork.ozlabs.org/project/uboot/patch/20230622173006.3921891-1-tharvey@gateworks.com/
> > [2]
> > https://patchwork.ozlabs.org/project/uboot/patch/ZeHDVr-Bzm935-5N@mecka.net/
> > [3]
> > https://docs.u-boot.org/en/latest/develop/package/binman.html#optional-entries
> > [4]
> > https://docs.u-boot.org/en/latest/develop/package/binman.html#image-description-format
> > see optional: description in [4]
> > 
> > ---
> > Yannic Moog (6):
> >       arm: dts: imx8m: add fit optee node
> >       arm: dts: imx8mp-phyboard-pollux: add optee load address
> >       arm: dts: imx8mm-phygate-tauri-l: add optee load address
> >       arm: dts: imx8mm-phyboard-polis: add optee load address
> >       doc: phytec: imx8mp: add OP-TEE documentation
> >       doc: phytec: imx8mm: add OP-TEE integration instructions
> > 
> >  arch/arm/dts/imx8mm-phyboard-polis-rdk-u-boot.dtsi |  5 +++++
> >  arch/arm/dts/imx8mm-phygate-tauri-l-u-boot.dtsi    |  5 +++++
> >  arch/arm/dts/imx8mm-u-boot.dtsi                    | 15
> > ++++++++++++-
> >  arch/arm/dts/imx8mn-u-boot.dtsi                    | 15
> > ++++++++++++-
> >  .../arm/dts/imx8mp-phyboard-pollux-rdk-u-boot.dtsi |  5 +++++
> >  arch/arm/dts/imx8mp-u-boot.dtsi                    | 15
> > ++++++++++++-
> >  doc/board/phytec/imx8mm-phygate-tauri-l.rst        | 26
> > +++++++++++++++++++++-
> >  doc/board/phytec/phycore-imx8mm.rst                | 25
> > ++++++++++++++++++++-
> >  doc/board/phytec/phycore-imx8mp.rst                | 26
> > +++++++++++++++++++++-
> >  9 files changed, 131 insertions(+), 6 deletions(-)
> > ---
> > base-commit: fd3f2e3f0edc1f87be4c5a39a0c81037d551c069
> > change-id: 20240903-phytec_imx8m_optee-8674ef012a36
> > 
> > Best regards,
> > --
> > Yannic Moog <y.moog at phytec.de>
> > 
> 
> Hi Yannic,
> 
> Thanks for looking at this - there is a need for a solution for
> easier
> integration of a TEE.
> 
> What is wrong with adding the address via Kconfig? My attempt [1]
> failed due to something in CI that I just didn't know how to fix or
> have the time to look into.

Tbf there is nothing wrong with that approach. TI k3 use such a
solution already [1].

I find it pollutes kconfig unnecessarily, but it was my backup solution
in case this one does not work or is not well received. Do you prefer
the kconfig route?

Yannic

[1]
https://github.com/u-boot/u-boot/blob/08ae12be8509daf3d1c5a148b8a50c0ffb6457c2/arch/arm/mach-k3/Kconfig#L135

> 
> Best Regards,
> 
> Tim
> [1]
> https://patchwork.ozlabs.org/project/uboot/patch/20230622173006.3921891-1-tharvey@gateworks.com/