On Fri, Oct 25, 2024 at 5:36 AM Yannic Moog <y.m...@phytec.de> wrote:
>
> 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.m...@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-thar...@gateworks.com/
> > > [2]
> > > https://patchwork.ozlabs.org/project/uboot/patch/zehdvr-bzm935...@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.m...@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].
>
Well that sets a decent precedent for a Kconfig solution then.
> 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?
>
I was actually hoping for an env variable approach such that if you
had OPTEE_LOAD_ADDRESS defined when you build it links tee.bin in with
that load/entry address but binman doesn't support env variables to my
knowledge and i'm not sure others like that approach.
I also do not like polluting Kconfig but I'm not sure how to do it
otherwise. It's certainly better than breaking boards :)
Based on the existing K3_OPTEE_LOAD_ADDR I would suggest going that
route and perhaps the K3_OPTEE_LOAD_ADDR can be changed to a generic
OPTEE_LOAD_ADDR which defaults to 9e800000 for that soc
Best Regards,
Tim
> 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-thar...@gateworks.com/
>
