Hi Adam,
On Wed, 2024-11-13 at 07:06 +0000, Yannic Moog wrote:
> On Mon, 2024-11-11 at 04:49 -0600, Adam Ford wrote:
> > On Mon, Nov 11, 2024 at 1:48 AM Yannic Moog <y.m...@phytec.de> wrote:
> > >
> > > Hi,
> > >
> > > On Fri, 2024-11-08 at 10:05 -0800, Tim Harvey wrote:
> > > > On Fri, Nov 8, 2024 at 5:49 AM Adam Ford <aford...@gmail.com> wrote:
> > > > >
> > > > > On Thu, Nov 7, 2024 at 2:42 AM Yannic Moog <y.m...@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.
> > > > > >
> > > > > > Signed-off-by: Yannic Moog <y.m...@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/mach-imx/imx8m/Kconfig | 8 ++++++++
> > > > > > 4 files changed, 56 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/arch/arm/dts/imx8mm-u-boot.dtsi
> > > > > > b/arch/arm/dts/imx8mm-u-boot.dtsi
> > > > > > index d31bc822532..ecc2319279e 100644
> > > > > > --- a/arch/arm/dts/imx8mm-u-boot.dtsi
> > > > > > +++ b/arch/arm/dts/imx8mm-u-boot.dtsi
> > > > > > @@ -164,6 +164,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;
> > > > > > + };
> > > > > > + };
> > > > > > +
> > > > > > binman_fip: fip {
> > > > > > arch = "arm64";
> > > > > > compression =
> > > > > > "none";
> > > > > > @@ -192,7 +207,7 @@
> > > > > > fdt = "fdt-SEQ";
> > > > > > firmware = "uboot";
> > > > > > #ifndef CONFIG_ARMV8_PSCI
> > > > > > - loadables = "atf";
> > > > > > + loadables = "atf",
> > > > > > "tee";
> > > > > > #endif
> > > > > > };
> > > > > > };
> > > > > > diff --git a/arch/arm/dts/imx8mn-u-boot.dtsi
> > > > > > b/arch/arm/dts/imx8mn-u-boot.dtsi
> > > > > > index 6875c6d44ff..f9108cb75c7 100644
> > > > > > --- a/arch/arm/dts/imx8mn-u-boot.dtsi
> > > > > > +++ b/arch/arm/dts/imx8mn-u-boot.dtsi
> > > > > > @@ -235,6 +235,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;
> > > > > > + };
> > > > > > + };
> > > > > > +
> > > > > > binman_fip: fip {
> > > > > > arch = "arm64";
> > > > > > compression =
> > > > > > "none";
> > > > > > @@ -263,7 +278,7 @@
> > > > > > fdt = "fdt-SEQ";
> > > > > > firmware = "uboot";
> > > > > > #ifndef CONFIG_ARMV8_PSCI
> > > > > > - loadables = "atf";
> > > > > > + loadables = "atf",
> > > > > > "tee";
> > > > > > #endif
> > > > > > };
> > > > > > };
> > > > > > diff --git a/arch/arm/dts/imx8mp-u-boot.dtsi
> > > > > > b/arch/arm/dts/imx8mp-u-boot.dtsi
> > > > > > index 56749ccacd2..9ede98a11e4 100644
> > > > > > --- a/arch/arm/dts/imx8mp-u-boot.dtsi
> > > > > > +++ b/arch/arm/dts/imx8mp-u-boot.dtsi
> > > > > > @@ -185,6 +185,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-SEQ {
> > > > > > description =
> > > > > > "NAME";
> > > > > > type = "flat_dt";
> > > > > > @@ -204,7 +219,7 @@
> > > > > > fdt = "fdt-SEQ";
> > > > > > firmware = "uboot";
> > > > > > #ifndef CONFIG_ARMV8_PSCI
> > > > > > - loadables = "atf";
> > > > > > + loadables = "atf",
> > > > > > "tee";
> > > > > > #endif
> > > > > > };
> > > > > > };
> > > > > > diff --git a/arch/arm/mach-imx/imx8m/Kconfig
> > > > > > b/arch/arm/mach-imx/imx8m/Kconfig
> > > > > > index b254a50b091..14f14db6a35 100644
> > > > > > --- a/arch/arm/mach-imx/imx8m/Kconfig
> > > > > > +++ b/arch/arm/mach-imx/imx8m/Kconfig
> > > > > > @@ -37,6 +37,14 @@ 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 0x56000000
> > > > > > + help
> > > > > > + The load and entry address for the OPTEE image. This
> > > > > > value defaults to
> > > > > > + 0x56000000 if not provided in the board defconfig file.
> > > > >
> > > > > I think this would need to vary based on the platform. I think the
> > > > > addresses may be different depending on Mini, Nano and Plus and also
> > > > > dependent on the available SDRAM.
> > > >
> > > > Indeed the address differs for platform (as the DRAM base address
> > > > does) and is also based on the DRAM size of the board. Here are my
> > > > notes for IMX8M:
> > > >
> > > > The address is the link/load address for TEE that should match
> > > > BL32_BASE for the ATF (for imx8m) and CFG_TZDRAM_START for OPTEE. The
> > > > BL32_BASE is the same for imx8mm/n/p but it does depend on the size of
> > > > DRAM. The address should be the top 32MiB of DRAM but because U-Boot
> > > > is loading tee.bin from a FIT image into DRAM it needs to be a 32bit
> > > > address and thus needs to be adjusted down if you are on a board with
> > > > 4GiB:
> > > > 1GiB DRAM use 0x7e000000
> > > > 2GiB DRAM use 0xbe000000
> > > > 3GiB or larger use 0xfe000000
> > >
> > > I agree the addresses may be different. To my knowledge you can take
> > > whatever (32bit) address
> > > you
> > > want within addressable RAM as load address. I assume you are looking for
> > > sensible defaults
> > > here.
> > > It appears to me the challenge is agreeing on a sensible value.
> > > Do you have any suggestions?
> >
> > On my Mini with 2GB of RAM, there are several definitions:
> > PHYS_SDRAM 0x40000000
> > PHYS_SDRAM_SIZE 0x80000000 /* 2GB DDR */
> >
> > What we if did a calculation like:
> > PHYS_SDRAM + MIN(PHYS_SDRAM_SIZE, 0xc0000000) - 0x2000000
> >
> > With 2 GB of RAM, it would be 0xbe000000
> >
> > My Nano Board has the following:
> >
> > PHYS_SDRAM 0x40000000
> > PHYS_SDRAM_SIZE 0x40000000
> >
> > That calculation should return 7E000000.
>
> Math looks good to me, however I don't understand where we would put this in
> the source code.
> How can we set this as default while having access to the macros?
As I don't think it is possible to implement the mechanic as you suggested, I
would like to keep it
simple.
trusted-firmware-a also has default for BL32_BASE. I would like to set the
OPTEE_LOAD_ADDR as they
are set in tf-a. This has the advantage that for users who don't change the
default, they can rely
on it being consistent across projects.
Yannic
>
> Yannic
>
> >
> > I am not sure if we can do MIN and MAX functions in the precompiler,
> > but it would then cap the value to 0xfe000000
> >
> > adam
> >
> > >
> > > Yannic
> > >
> > > >
> > > > Best Regards,
> > > >
> > > > Tim
> > >
>
> _______________________________________________
> upstream mailing list
> upstr...@lists.phytec.de
> http://lists.phytec.de/cgi-bin/mailman/listinfo/upstream
