public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Yannic Moog <Y.Moog@phytec.de>
To: "aford173@gmail.com" <aford173@gmail.com>
Cc: "manut@mecka.net" <manut@mecka.net>,
	"uboot-imx@nxp.com" <uboot-imx@nxp.com>,
	"trini@konsulko.com" <trini@konsulko.com>,
	"festevam@gmail.com" <festevam@gmail.com>,
	Yashwanth Varakala <Y.Varakala@phytec.de>,
	"sbabic@denx.de" <sbabic@denx.de>,
	Benjamin Hahn <B.Hahn@phytec.de>,
	"tharvey@gateworks.com" <tharvey@gateworks.com>,
	"PHYTEC Upstream" <upstream@lists.phytec.de>,
	"u-boot@lists.denx.de" <u-boot@lists.denx.de>,
	Teresa Remmet <T.Remmet@phytec.de>
Subject: Re: [PATCH v2 1/6] arm: imx8m: add OP-TEE node
Date: Wed, 13 Nov 2024 07:06:39 +0000	[thread overview]
Message-ID: <b58f480b70df2d4a35352df135c5f6cd39f4701a.camel@phytec.de> (raw)
In-Reply-To: <CAHCN7xJOsasYAaE+h0+CpqE=GLU4sR6Y_ekFEX6AaYKzO52OYg@mail.gmail.com>

On Mon, 2024-11-11 at 04:49 -0600, Adam Ford wrote:
> On Mon, Nov 11, 2024 at 1:48 AM Yannic Moog <Y.Moog@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 <aford173@gmail.com> wrote:
> > > > 
> > > > On Thu, Nov 7, 2024 at 2:42 AM Yannic Moog <y.moog@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.moog@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?

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
> > 


  reply	other threads:[~2024-11-13  7:06 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-07  8:23 [PATCH v2 0/6] imx8(m): add optee node to binman FIT image Yannic Moog
2024-11-07  8:23 ` [PATCH v2 1/6] arm: imx8m: add OP-TEE node Yannic Moog
2024-11-07 17:42   ` Tim Harvey
2024-11-08 12:44     ` Yannic Moog
2024-11-08 13:49   ` Adam Ford
2024-11-08 18:05     ` Tim Harvey
2024-11-11  7:48       ` Yannic Moog
2024-11-11 10:49         ` Adam Ford
2024-11-13  7:06           ` Yannic Moog [this message]
2024-12-06 13:17             ` [Upstream] " Yannic Moog
2024-11-07  8:23 ` [PATCH v2 2/6] phycore-imx8mp_defconfig: add optee load address Yannic Moog
2024-11-07  8:52   ` [Upstream] " Wadim Egorov
2024-11-08 12:55     ` Yannic Moog
2024-11-08 13:12       ` Wadim Egorov
2024-11-07  8:23 ` [PATCH v2 3/6] imx8mm-phygate-tauri-l_defconfig: " Yannic Moog
2024-11-07  8:23 ` [PATCH v2 4/6] phycore-imx8mm_defconfig: " Yannic Moog
2024-11-07  8:23 ` [PATCH v2 5/6] doc: phytec: imx8mp: add OP-TEE documentation Yannic Moog
2024-11-07  9:07   ` [Upstream] " Wadim Egorov
2024-11-08 12:59     ` Yannic Moog
2024-11-08 13:05       ` Wadim Egorov
2024-11-07  8:23 ` [PATCH v2 6/6] doc: phytec: imx8mm: add OP-TEE integration instructions Yannic Moog

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b58f480b70df2d4a35352df135c5f6cd39f4701a.camel@phytec.de \
    --to=y.moog@phytec.de \
    --cc=B.Hahn@phytec.de \
    --cc=T.Remmet@phytec.de \
    --cc=Y.Varakala@phytec.de \
    --cc=aford173@gmail.com \
    --cc=festevam@gmail.com \
    --cc=manut@mecka.net \
    --cc=sbabic@denx.de \
    --cc=tharvey@gateworks.com \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    --cc=uboot-imx@nxp.com \
    --cc=upstream@lists.phytec.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox