public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [PATCH v2 0/6] imx8(m): add optee node to binman FIT image
@ 2024-11-07  8:23 Yannic Moog
  2024-11-07  8:23 ` [PATCH v2 1/6] arm: imx8m: add OP-TEE node Yannic Moog
                   ` (5 more replies)
  0 siblings, 6 replies; 21+ messages in thread
From: Yannic Moog @ 2024-11-07  8:23 UTC (permalink / raw)
  To: Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team, Tom Rini
  Cc: Tim Harvey, Manuel Traut, Benjamin Hahn, Teresa Remmet,
	Yashwanth Varakala, u-boot, upstream, Yannic Moog

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).
This approach follows the TI k3 kconfig solution.
To be able to define a sensible default for the new config option, put
it in the imx8m kconfig. For other imx families, the RAM start addresses
are different and thus finding a common sensible default is not
possible.
I do not have a working Nano board so I was not able to test imx8mn.

I included usage for PHYTEC boards for examples (with documentation).

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

---
Changes in v2 (from RFC):
- Use kconfig instead of trying pure dt approach.
- Add load addresses in defconfigs instead of <board>-u-boot dts.

---
Yannic Moog (6):
      arm: imx8m: add OP-TEE node
      phycore-imx8mp_defconfig: add optee load address
      imx8mm-phygate-tauri-l_defconfig: add optee load address
      phycore-imx8mm_defconfig: add optee load address
      doc: phytec: imx8mp: add OP-TEE documentation
      doc: phytec: imx8mm: add OP-TEE integration instructions

 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 ++++++++
 configs/imx8mm-phygate-tauri-l_defconfig    |  1 +
 configs/phycore-imx8mm_defconfig            |  1 +
 configs/phycore-imx8mp_defconfig            |  1 +
 doc/board/phytec/imx8mm-phygate-tauri-l.rst | 26 +++++++++++++++++++++++++-
 doc/board/phytec/phycore-imx8mm.rst         | 25 ++++++++++++++++++++++++-
 doc/board/phytec/phycore-imx8mp.rst         | 26 +++++++++++++++++++++++++-
 10 files changed, 133 insertions(+), 6 deletions(-)
---
base-commit: d88bcd6d247a2b5d1683e393d8c9dc0259cd29f0
change-id: 20240903-phytec_imx8m_optee-8674ef012a36

Best regards,
-- 
Yannic Moog <y.moog@phytec.de>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 1/6] arm: imx8m: add OP-TEE node
  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 ` Yannic Moog
  2024-11-07 17:42   ` Tim Harvey
  2024-11-08 13:49   ` Adam Ford
  2024-11-07  8:23 ` [PATCH v2 2/6] phycore-imx8mp_defconfig: add optee load address Yannic Moog
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 21+ messages in thread
From: Yannic Moog @ 2024-11-07  8:23 UTC (permalink / raw)
  To: Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team, Tom Rini
  Cc: Tim Harvey, Manuel Traut, Benjamin Hahn, Teresa Remmet,
	Yashwanth Varakala, u-boot, upstream, Yannic Moog

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.
+
+
 choice
 	prompt "NXP i.MX8M board select"
 	optional

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 2/6] phycore-imx8mp_defconfig: add optee load address
  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  8:23 ` Yannic Moog
  2024-11-07  8:52   ` [Upstream] " Wadim Egorov
  2024-11-07  8:23 ` [PATCH v2 3/6] imx8mm-phygate-tauri-l_defconfig: " Yannic Moog
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Yannic Moog @ 2024-11-07  8:23 UTC (permalink / raw)
  To: Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team, Tom Rini
  Cc: Tim Harvey, Manuel Traut, Benjamin Hahn, Teresa Remmet,
	Yashwanth Varakala, u-boot, upstream, Yannic Moog

The phyBOARD-Pollux expects 0x56000000 address to load optee.

Signed-off-by: Yannic Moog <y.moog@phytec.de>
---
 configs/phycore-imx8mp_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/phycore-imx8mp_defconfig b/configs/phycore-imx8mp_defconfig
index 1240c7fcbd0..910109afd80 100644
--- a/configs/phycore-imx8mp_defconfig
+++ b/configs/phycore-imx8mp_defconfig
@@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_LEN=0x2000000
 CONFIG_SPL_GPIO=y
 CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
+CONFIG_IMX8M_OPTEE_LOAD_ADDR=0x56000000
 CONFIG_PHYTEC_SOM_DETECTION=y
 CONFIG_SF_DEFAULT_SPEED=80000000
 CONFIG_ENV_SIZE=0x10000

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 3/6] imx8mm-phygate-tauri-l_defconfig: add optee load address
  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  8:23 ` [PATCH v2 2/6] phycore-imx8mp_defconfig: add optee load address Yannic Moog
@ 2024-11-07  8:23 ` Yannic Moog
  2024-11-07  8:23 ` [PATCH v2 4/6] phycore-imx8mm_defconfig: " Yannic Moog
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Yannic Moog @ 2024-11-07  8:23 UTC (permalink / raw)
  To: Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team, Tom Rini
  Cc: Tim Harvey, Manuel Traut, Benjamin Hahn, Teresa Remmet,
	Yashwanth Varakala, u-boot, upstream, Yannic Moog

The phyGATE-Tauri-L expects 0x56000000 address to load optee.

Signed-off-by: Yannic Moog <y.moog@phytec.de>
---
 configs/imx8mm-phygate-tauri-l_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/imx8mm-phygate-tauri-l_defconfig b/configs/imx8mm-phygate-tauri-l_defconfig
index c69fe50ec81..2e1071b4058 100644
--- a/configs/imx8mm-phygate-tauri-l_defconfig
+++ b/configs/imx8mm-phygate-tauri-l_defconfig
@@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_LEN=0x2000000
 CONFIG_SPL_GPIO=y
 CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
+CONFIG_IMX8M_OPTEE_LOAD_ADDR=0x56000000
 CONFIG_ENV_SIZE=0x10000
 CONFIG_ENV_OFFSET=0x3C0000
 CONFIG_DM_GPIO=y

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 4/6] phycore-imx8mm_defconfig: add optee load address
  2024-11-07  8:23 [PATCH v2 0/6] imx8(m): add optee node to binman FIT image Yannic Moog
                   ` (2 preceding siblings ...)
  2024-11-07  8:23 ` [PATCH v2 3/6] imx8mm-phygate-tauri-l_defconfig: " Yannic Moog
@ 2024-11-07  8:23 ` Yannic Moog
  2024-11-07  8:23 ` [PATCH v2 5/6] doc: phytec: imx8mp: add OP-TEE documentation Yannic Moog
  2024-11-07  8:23 ` [PATCH v2 6/6] doc: phytec: imx8mm: add OP-TEE integration instructions Yannic Moog
  5 siblings, 0 replies; 21+ messages in thread
From: Yannic Moog @ 2024-11-07  8:23 UTC (permalink / raw)
  To: Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team, Tom Rini
  Cc: Tim Harvey, Manuel Traut, Benjamin Hahn, Teresa Remmet,
	Yashwanth Varakala, u-boot, upstream, Yannic Moog

The phyCORE i.MX 8M Mini expects 0x56000000 address to load optee.

Signed-off-by: Yannic Moog <y.moog@phytec.de>
---
 configs/phycore-imx8mm_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/phycore-imx8mm_defconfig b/configs/phycore-imx8mm_defconfig
index 48a0c0b8b4e..240b5f75a98 100644
--- a/configs/phycore-imx8mm_defconfig
+++ b/configs/phycore-imx8mm_defconfig
@@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_LEN=0x2000000
 CONFIG_SPL_GPIO=y
 CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
+CONFIG_IMX8M_OPTEE_LOAD_ADDR=0x56000000
 CONFIG_SF_DEFAULT_SPEED=80000000
 CONFIG_ENV_SIZE=0x10000
 CONFIG_ENV_OFFSET=0x3C0000

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 5/6] doc: phytec: imx8mp: add OP-TEE documentation
  2024-11-07  8:23 [PATCH v2 0/6] imx8(m): add optee node to binman FIT image Yannic Moog
                   ` (3 preceding siblings ...)
  2024-11-07  8:23 ` [PATCH v2 4/6] phycore-imx8mm_defconfig: " Yannic Moog
@ 2024-11-07  8:23 ` Yannic Moog
  2024-11-07  9:07   ` [Upstream] " Wadim Egorov
  2024-11-07  8:23 ` [PATCH v2 6/6] doc: phytec: imx8mm: add OP-TEE integration instructions Yannic Moog
  5 siblings, 1 reply; 21+ messages in thread
From: Yannic Moog @ 2024-11-07  8:23 UTC (permalink / raw)
  To: Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team, Tom Rini
  Cc: Tim Harvey, Manuel Traut, Benjamin Hahn, Teresa Remmet,
	Yashwanth Varakala, u-boot, upstream, Yannic Moog

Add documentation for the phyBOARD-Pollux i.MX 8M Plus on OP-TEE
integration.
Also add missing '-' to TF-A build instruction while at it.

Signed-off-by: Yannic Moog <y.moog@phytec.de>
---
 doc/board/phytec/phycore-imx8mp.rst | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/doc/board/phytec/phycore-imx8mp.rst b/doc/board/phytec/phycore-imx8mp.rst
index fda751aeffb..4bcfa9f39ca 100644
--- a/doc/board/phytec/phycore-imx8mp.rst
+++ b/doc/board/phytec/phycore-imx8mp.rst
@@ -9,6 +9,7 @@ Quick Start
 -----------
 
 - Build the ARM Trusted firmware binary
+- Build the OP-TEE binary (optional)
 - Get ddr firmware
 - Build U-Boot
 - Boot
@@ -20,10 +21,32 @@ Build the ARM Trusted firmware binary
 
    $ git clone https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git
    $ cd trusted-firmware-a
-   $ export CROSS_COMPILE=aarch64-linux-gnu
+   $ export CROSS_COMPILE=aarch64-linux-gnu-
    $ export IMX_BOOT_UART_BASE=0x30860000
+   $ # with optee
+   $ make PLAT=imx8mp SPD=opteed bl31
+   $ # without optee
    $ make PLAT=imx8mp bl31
 
+Build the OP-TEE binary (optional)
+----------------------------------
+
+.. code-block:: bash
+
+   $ git clone https://github.com/OP-TEE/optee_os.git
+   $ cd optee_os
+   $ make CFG_ARM64_core=y \
+     CFG_TEE_BENCHMARK=n \
+     CROSS_COMPILE=aarch64-linux-gnu- \
+     CROSS_COMPILE_core=aarch64-linux-gnu- \
+     CROSS_COMPILE_ta_arm32=arm-linux-gnueabihf- \
+     CROSS_COMPILE_ta_arm64=aarch64-linux-gnu- \
+     O=out/arm \
+     PLATFORM=imx-mx8mpevk \
+     CFG_TZDRAM_START=0x56000000 \
+     CFG_DDR_SIZE=0x80000000 \
+     CFG_UART_BASE=UART1_BASE
+
 Get the ddr firmware
 --------------------
 
@@ -42,6 +65,7 @@ Copy binaries
 .. code-block:: bash
 
    $ cp <TF-A dir>/build/imx8mp/release/bl31.bin .
+   $ cp <OP-TEE dir>/out/arm/core/tee-raw.bin tee.bin
    $ cp firmware-imx-8.19/firmware/ddr/synopsys/lpddr4*.bin .
 
 Build U-Boot

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 6/6] doc: phytec: imx8mm: add OP-TEE integration instructions
  2024-11-07  8:23 [PATCH v2 0/6] imx8(m): add optee node to binman FIT image Yannic Moog
                   ` (4 preceding siblings ...)
  2024-11-07  8:23 ` [PATCH v2 5/6] doc: phytec: imx8mp: add OP-TEE documentation Yannic Moog
@ 2024-11-07  8:23 ` Yannic Moog
  5 siblings, 0 replies; 21+ messages in thread
From: Yannic Moog @ 2024-11-07  8:23 UTC (permalink / raw)
  To: Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team, Tom Rini
  Cc: Tim Harvey, Manuel Traut, Benjamin Hahn, Teresa Remmet,
	Yashwanth Varakala, u-boot, upstream, Yannic Moog

Add instructions on how to build and package OP-TEE for the
phycore-imx8mm based boards. The build instructions are identical for
phyGATE-Tauri-L and phyBOARD-Polis.
Also fix missig '-' for TF-A build instructions.

Signed-off-by: Yannic Moog <y.moog@phytec.de>
---
 doc/board/phytec/imx8mm-phygate-tauri-l.rst | 26 +++++++++++++++++++++++++-
 doc/board/phytec/phycore-imx8mm.rst         | 25 ++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/doc/board/phytec/imx8mm-phygate-tauri-l.rst b/doc/board/phytec/imx8mm-phygate-tauri-l.rst
index 28b614fd144..d9b5124365e 100644
--- a/doc/board/phytec/imx8mm-phygate-tauri-l.rst
+++ b/doc/board/phytec/imx8mm-phygate-tauri-l.rst
@@ -9,6 +9,7 @@ Quick Start
 -----------
 
 - Build the ARM Trusted firmware binary
+- Build the OP-TEE binary (optional)
 - Get ddr firmware
 - Build U-Boot
 - Boot
@@ -20,10 +21,32 @@ Build the ARM Trusted firmware binary
 
    $ git clone https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git
    $ cd trusted-firmware-a
-   $ export CROSS_COMPILE=aarch64-linux-gnu
+   $ export CROSS_COMPILE=aarch64-linux-gnu-
    $ export IMX_BOOT_UART_BASE=0x30880000
+   $ # with optee
+   $ make PLAT=imx8mm BL32_BASE=0x56000000 SPD=opteed bl31
+   $ # without optee
    $ make PLAT=imx8mm bl31
 
+Build the OP-TEE binary (optional)
+----------------------------------
+
+.. code-block:: bash
+
+   $ git clone https://github.com/OP-TEE/optee_os.git
+   $ cd optee_os
+   $ make CFG_ARM64_core=y \
+     CFG_TEE_BENCHMARK=n \
+     CROSS_COMPILE=aarch64-linux-gnu- \
+     CROSS_COMPILE_core=aarch64-linux-gnu- \
+     CROSS_COMPILE_ta_arm32=arm-linux-gnueabihf- \
+     CROSS_COMPILE_ta_arm64=aarch64-linux-gnu- \
+     O=out/arm \
+     PLATFORM=imx-mx8mmevk \
+     CFG_TZDRAM_START=0x56000000 \
+     CFG_DDR_SIZE=0x80000000 \
+     CFG_UART_BASE=UART3_BASE
+
 Get the ddr firmware
 --------------------
 
@@ -42,6 +65,7 @@ Copy binaries
 .. code-block:: bash
 
    $ cp <TF-A dir>/build/imx8mm/release/bl31.bin .
+   $ cp <OP-TEE dir>/out/arm/core/tee-raw.bin tee.bin
    $ cp firmware-imx-8.23/firmware/ddr/synopsys/lpddr4*.bin .
 
 Build U-Boot
diff --git a/doc/board/phytec/phycore-imx8mm.rst b/doc/board/phytec/phycore-imx8mm.rst
index e9dc2259907..5a4e8a669c6 100644
--- a/doc/board/phytec/phycore-imx8mm.rst
+++ b/doc/board/phytec/phycore-imx8mm.rst
@@ -9,6 +9,7 @@ Quick Start
 -----------
 
 - Build the ARM Trusted firmware binary
+- Build the OP-TEE binary (optional)
 - Get ddr firmware
 - Build U-Boot
 - Boot
@@ -20,10 +21,32 @@ Build the ARM Trusted firmware binary
 
    $ git clone https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git
    $ cd trusted-firmware-a
-   $ export CROSS_COMPILE=aarch64-linux-gnu
+   $ export CROSS_COMPILE=aarch64-linux-gnu-
    $ export IMX_BOOT_UART_BASE=0x30880000
+   $ # with optee
+   $ make PLAT=imx8mm BL32_BASE=0x56000000 SPD=opteed bl31
+   $ # without optee
    $ make PLAT=imx8mm bl31
 
+Build the OP-TEE binary (optional)
+----------------------------------
+
+.. code-block:: bash
+
+   $ git clone https://github.com/OP-TEE/optee_os.git
+   $ cd optee_os
+   $ make CFG_ARM64_core=y \
+     CFG_TEE_BENCHMARK=n \
+     CROSS_COMPILE=aarch64-linux-gnu- \
+     CROSS_COMPILE_core=aarch64-linux-gnu- \
+     CROSS_COMPILE_ta_arm32=arm-linux-gnueabihf- \
+     CROSS_COMPILE_ta_arm64=aarch64-linux-gnu- \
+     O=out/arm \
+     PLATFORM=imx-mx8mmevk \
+     CFG_TZDRAM_START=0x56000000 \
+     CFG_DDR_SIZE=0x80000000 \
+     CFG_UART_BASE=UART3_BASE
+
 Get the ddr firmware
 --------------------
 

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [Upstream] [PATCH v2 2/6] phycore-imx8mp_defconfig: add optee load address
  2024-11-07  8:23 ` [PATCH v2 2/6] phycore-imx8mp_defconfig: add optee load address Yannic Moog
@ 2024-11-07  8:52   ` Wadim Egorov
  2024-11-08 12:55     ` Yannic Moog
  0 siblings, 1 reply; 21+ messages in thread
From: Wadim Egorov @ 2024-11-07  8:52 UTC (permalink / raw)
  To: Yannic Moog, Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team,
	Tom Rini
  Cc: Manuel Traut, Tim Harvey, upstream, u-boot, Yashwanth Varakala

Hi Yannic,

Am 07.11.24 um 09:23 schrieb Yannic Moog:
> The phyBOARD-Pollux expects 0x56000000 address to load optee.

You are already defining a default for IMX8M_OPTEE_LOAD_ADDR in the 
mach-imx Kconf. You can drop the patches touching the defconfigs.

Regards,
Wadim

> 
> Signed-off-by: Yannic Moog <y.moog@phytec.de>
> ---
>   configs/phycore-imx8mp_defconfig | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/configs/phycore-imx8mp_defconfig b/configs/phycore-imx8mp_defconfig
> index 1240c7fcbd0..910109afd80 100644
> --- a/configs/phycore-imx8mp_defconfig
> +++ b/configs/phycore-imx8mp_defconfig
> @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_LEN=0x2000000
>   CONFIG_SPL_GPIO=y
>   CONFIG_SPL_LIBCOMMON_SUPPORT=y
>   CONFIG_SPL_LIBGENERIC_SUPPORT=y
> +CONFIG_IMX8M_OPTEE_LOAD_ADDR=0x56000000
>   CONFIG_PHYTEC_SOM_DETECTION=y
>   CONFIG_SF_DEFAULT_SPEED=80000000
>   CONFIG_ENV_SIZE=0x10000
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Upstream] [PATCH v2 5/6] doc: phytec: imx8mp: add OP-TEE documentation
  2024-11-07  8:23 ` [PATCH v2 5/6] doc: phytec: imx8mp: add OP-TEE documentation Yannic Moog
@ 2024-11-07  9:07   ` Wadim Egorov
  2024-11-08 12:59     ` Yannic Moog
  0 siblings, 1 reply; 21+ messages in thread
From: Wadim Egorov @ 2024-11-07  9:07 UTC (permalink / raw)
  To: Yannic Moog, Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team,
	Tom Rini
  Cc: Manuel Traut, Tim Harvey, upstream, u-boot, Yashwanth Varakala



Am 07.11.24 um 09:23 schrieb Yannic Moog:
> Add documentation for the phyBOARD-Pollux i.MX 8M Plus on OP-TEE
> integration.
> Also add missing '-' to TF-A build instruction while at it.


Would it make sense to move these instructions into a more general 
section, so they could be reused by other imx based boards as well?

Seems you missed to update imx8mm-phygate-tauri-l.rst

> 
> Signed-off-by: Yannic Moog <y.moog@phytec.de>
> ---
>   doc/board/phytec/phycore-imx8mp.rst | 26 +++++++++++++++++++++++++-
>   1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/board/phytec/phycore-imx8mp.rst b/doc/board/phytec/phycore-imx8mp.rst
> index fda751aeffb..4bcfa9f39ca 100644
> --- a/doc/board/phytec/phycore-imx8mp.rst
> +++ b/doc/board/phytec/phycore-imx8mp.rst
> @@ -9,6 +9,7 @@ Quick Start
>   -----------
>   
>   - Build the ARM Trusted firmware binary
> +- Build the OP-TEE binary (optional)
>   - Get ddr firmware
>   - Build U-Boot
>   - Boot
> @@ -20,10 +21,32 @@ Build the ARM Trusted firmware binary
>   
>      $ git clone https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git
>      $ cd trusted-firmware-a
> -   $ export CROSS_COMPILE=aarch64-linux-gnu
> +   $ export CROSS_COMPILE=aarch64-linux-gnu-
>      $ export IMX_BOOT_UART_BASE=0x30860000
> +   $ # with optee
> +   $ make PLAT=imx8mp SPD=opteed bl31
> +   $ # without optee
>      $ make PLAT=imx8mp bl31
>   
> +Build the OP-TEE binary (optional)
> +----------------------------------
> +
> +.. code-block:: bash
> +
> +   $ git clone https://github.com/OP-TEE/optee_os.git
> +   $ cd optee_os
> +   $ make CFG_ARM64_core=y \
> +     CFG_TEE_BENCHMARK=n \
> +     CROSS_COMPILE=aarch64-linux-gnu- \
> +     CROSS_COMPILE_core=aarch64-linux-gnu- \
> +     CROSS_COMPILE_ta_arm32=arm-linux-gnueabihf- \
> +     CROSS_COMPILE_ta_arm64=aarch64-linux-gnu- \
> +     O=out/arm \
> +     PLATFORM=imx-mx8mpevk \
> +     CFG_TZDRAM_START=0x56000000 \
> +     CFG_DDR_SIZE=0x80000000 \
> +     CFG_UART_BASE=UART1_BASE
> +
>   Get the ddr firmware
>   --------------------
>   
> @@ -42,6 +65,7 @@ Copy binaries
>   .. code-block:: bash
>   
>      $ cp <TF-A dir>/build/imx8mp/release/bl31.bin .
> +   $ cp <OP-TEE dir>/out/arm/core/tee-raw.bin tee.bin
>      $ cp firmware-imx-8.19/firmware/ddr/synopsys/lpddr4*.bin .
>   
>   Build U-Boot
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] arm: imx8m: add OP-TEE node
  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
  1 sibling, 1 reply; 21+ messages in thread
From: Tim Harvey @ 2024-11-07 17:42 UTC (permalink / raw)
  To: Yannic Moog
  Cc: Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team, Tom Rini,
	Manuel Traut, Benjamin Hahn, Teresa Remmet, Yashwanth Varakala,
	u-boot, upstream

On Thu, Nov 7, 2024 at 12:24 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;
> +                                               };
> +                                       };
> +

Hi Yannic,

Thanks for working on this.

Don't you need to protect this block with an #ifdef
CONFIG_IMX8M_OPTEE_LOAD_ADDR? I would expect binman to fail if it
doesn't find tee.bin and has an empty load/entry address.

Also, when I attempted this patch some time ago it was accepted but
failed CI which is something I never had time to figure out and
address. The failure had something to do with adding the Kconfig and
you should be able to find the discussion with a pointer to the
failure in your references above.

Did you make sure CI passes?

Best Regards,

Tim

>                                         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.
> +
> +
>  choice
>         prompt "NXP i.MX8M board select"
>         optional
>
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] arm: imx8m: add OP-TEE node
  2024-11-07 17:42   ` Tim Harvey
@ 2024-11-08 12:44     ` Yannic Moog
  0 siblings, 0 replies; 21+ messages in thread
From: Yannic Moog @ 2024-11-08 12:44 UTC (permalink / raw)
  To: tharvey@gateworks.com
  Cc: manut@mecka.net, uboot-imx@nxp.com, trini@konsulko.com,
	festevam@gmail.com, Yashwanth Varakala, sbabic@denx.de,
	Benjamin Hahn, PHYTEC Upstream, u-boot@lists.denx.de,
	Teresa Remmet

Hi Tim,

On Thu, 2024-11-07 at 09:42 -0800, Tim Harvey wrote:
> On Thu, Nov 7, 2024 at 12:24 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;
> > +                                               };
> > +                                       };
> > +
> 
> Hi Yannic,
> 
> Thanks for working on this.
> 
> Don't you need to protect this block with an #ifdef
> CONFIG_IMX8M_OPTEE_LOAD_ADDR? I would expect binman to fail if it
> doesn't find tee.bin and has an empty load/entry address.

I do not need to do that because the config is defined for all imx8m boards. If I didn't define a
default, the kconfig mechanism prompts for a value for the load addr.

> 
> Also, when I attempted this patch some time ago it was accepted but
> failed CI which is something I never had time to figure out and
> address. 
> The failure had something to do with adding the Kconfig and
> you should be able to find the discussion with a pointer to the
> failure in your references above.

I took a look at your patch before working on this series and to the best of my knowledge the
challenge was that you defined the kconfig for all boards and failed for other architectures or no
architecture in that particular case afaik. The solution is to define a default value, but obviously
it should at least be a valid address.
To illustrate, some of our (Phytec imx8m) boards have 1GiB RAM (up to 0x80000000) and for e.g. imx93
RAM starts at 0x90000000, the only good option is to put it in the imx8m Kconfig.

> 
> Did you make sure CI passes?

I did test the failed CI that Stefano highlighted at the time and that passes. For others I don't
know.

Yannic
> 
> Best Regards,
> 
> Tim
> 
> >                                         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.
> > +
> > +
> >  choice
> >         prompt "NXP i.MX8M board select"
> >         optional
> > 
> > --
> > 2.43.0
> > 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Upstream] [PATCH v2 2/6] phycore-imx8mp_defconfig: add optee load address
  2024-11-07  8:52   ` [Upstream] " Wadim Egorov
@ 2024-11-08 12:55     ` Yannic Moog
  2024-11-08 13:12       ` Wadim Egorov
  0 siblings, 1 reply; 21+ messages in thread
From: Yannic Moog @ 2024-11-08 12:55 UTC (permalink / raw)
  To: Wadim Egorov
  Cc: uboot-imx@nxp.com, manut@mecka.net, trini@konsulko.com,
	festevam@gmail.com, Yashwanth Varakala, PHYTEC Upstream,
	sbabic@denx.de, tharvey@gateworks.com, u-boot@lists.denx.de

Hi Wadim,

On Thu, 2024-11-07 at 09:52 +0100, Wadim Egorov wrote:
> Hi Yannic,
> 
> Am 07.11.24 um 09:23 schrieb Yannic Moog:
> > The phyBOARD-Pollux expects 0x56000000 address to load optee.
> 
> You are already defining a default for IMX8M_OPTEE_LOAD_ADDR in the 
> mach-imx Kconf. You can drop the patches touching the defconfigs.

I would like to keep these patches as the default value is "only" there to get a functioning build. 
To me it would seem there has been no thought put in to what the actual correct value would be if
left as default. Does that make sense?

Yannic

> 
> Regards,
> Wadim
> 
> > 
> > Signed-off-by: Yannic Moog <y.moog@phytec.de>
> > ---
> >   configs/phycore-imx8mp_defconfig | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/configs/phycore-imx8mp_defconfig b/configs/phycore-imx8mp_defconfig
> > index 1240c7fcbd0..910109afd80 100644
> > --- a/configs/phycore-imx8mp_defconfig
> > +++ b/configs/phycore-imx8mp_defconfig
> > @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_LEN=0x2000000
> >   CONFIG_SPL_GPIO=y
> >   CONFIG_SPL_LIBCOMMON_SUPPORT=y
> >   CONFIG_SPL_LIBGENERIC_SUPPORT=y
> > +CONFIG_IMX8M_OPTEE_LOAD_ADDR=0x56000000
> >   CONFIG_PHYTEC_SOM_DETECTION=y
> >   CONFIG_SF_DEFAULT_SPEED=80000000
> >   CONFIG_ENV_SIZE=0x10000
> > 
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Upstream] [PATCH v2 5/6] doc: phytec: imx8mp: add OP-TEE documentation
  2024-11-07  9:07   ` [Upstream] " Wadim Egorov
@ 2024-11-08 12:59     ` Yannic Moog
  2024-11-08 13:05       ` Wadim Egorov
  0 siblings, 1 reply; 21+ messages in thread
From: Yannic Moog @ 2024-11-08 12:59 UTC (permalink / raw)
  To: festevam@gmail.com, Wadim Egorov, uboot-imx@nxp.com,
	sbabic@denx.de, trini@konsulko.com
  Cc: PHYTEC Upstream, u-boot@lists.denx.de, tharvey@gateworks.com,
	manut@mecka.net, Yashwanth Varakala

On Thu, 2024-11-07 at 10:07 +0100, Wadim Egorov wrote:
> 
> 
> Am 07.11.24 um 09:23 schrieb Yannic Moog:
> > Add documentation for the phyBOARD-Pollux i.MX 8M Plus on OP-TEE
> > integration.
> > Also add missing '-' to TF-A build instruction while at it.
> 
> 
> Would it make sense to move these instructions into a more general 
> section, so they could be reused by other imx based boards as well?

I have not put much thought into it. At first glance though, when reading the documentation for a
board I don't want to be redirected to other pages. So using an include comes to mind. Is that what
you had in mind as well?
 
> 
> Seems you missed to update imx8mm-phygate-tauri-l.rst

Should be in the (next) patch for the imx8mm, as Tauri is based on phycore-imx8mm. Let me know if
you meant something else.

Yannic

> 
> > 
> > Signed-off-by: Yannic Moog <y.moog@phytec.de>
> > ---
> >   doc/board/phytec/phycore-imx8mp.rst | 26 +++++++++++++++++++++++++-
> >   1 file changed, 25 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/board/phytec/phycore-imx8mp.rst b/doc/board/phytec/phycore-imx8mp.rst
> > index fda751aeffb..4bcfa9f39ca 100644
> > --- a/doc/board/phytec/phycore-imx8mp.rst
> > +++ b/doc/board/phytec/phycore-imx8mp.rst
> > @@ -9,6 +9,7 @@ Quick Start
> >   -----------
> >   
> >   - Build the ARM Trusted firmware binary
> > +- Build the OP-TEE binary (optional)
> >   - Get ddr firmware
> >   - Build U-Boot
> >   - Boot
> > @@ -20,10 +21,32 @@ Build the ARM Trusted firmware binary
> >   
> >      $ git clone https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git
> >      $ cd trusted-firmware-a
> > -   $ export CROSS_COMPILE=aarch64-linux-gnu
> > +   $ export CROSS_COMPILE=aarch64-linux-gnu-
> >      $ export IMX_BOOT_UART_BASE=0x30860000
> > +   $ # with optee
> > +   $ make PLAT=imx8mp SPD=opteed bl31
> > +   $ # without optee
> >      $ make PLAT=imx8mp bl31
> >   
> > +Build the OP-TEE binary (optional)
> > +----------------------------------
> > +
> > +.. code-block:: bash
> > +
> > +   $ git clone https://github.com/OP-TEE/optee_os.git
> > +   $ cd optee_os
> > +   $ make CFG_ARM64_core=y \
> > +     CFG_TEE_BENCHMARK=n \
> > +     CROSS_COMPILE=aarch64-linux-gnu- \
> > +     CROSS_COMPILE_core=aarch64-linux-gnu- \
> > +     CROSS_COMPILE_ta_arm32=arm-linux-gnueabihf- \
> > +     CROSS_COMPILE_ta_arm64=aarch64-linux-gnu- \
> > +     O=out/arm \
> > +     PLATFORM=imx-mx8mpevk \
> > +     CFG_TZDRAM_START=0x56000000 \
> > +     CFG_DDR_SIZE=0x80000000 \
> > +     CFG_UART_BASE=UART1_BASE
> > +
> >   Get the ddr firmware
> >   --------------------
> >   
> > @@ -42,6 +65,7 @@ Copy binaries
> >   .. code-block:: bash
> >   
> >      $ cp <TF-A dir>/build/imx8mp/release/bl31.bin .
> > +   $ cp <OP-TEE dir>/out/arm/core/tee-raw.bin tee.bin
> >      $ cp firmware-imx-8.19/firmware/ddr/synopsys/lpddr4*.bin .
> >   
> >   Build U-Boot
> > 
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Upstream] [PATCH v2 5/6] doc: phytec: imx8mp: add OP-TEE documentation
  2024-11-08 12:59     ` Yannic Moog
@ 2024-11-08 13:05       ` Wadim Egorov
  0 siblings, 0 replies; 21+ messages in thread
From: Wadim Egorov @ 2024-11-08 13:05 UTC (permalink / raw)
  To: Yannic Moog, festevam@gmail.com, uboot-imx@nxp.com,
	sbabic@denx.de, trini@konsulko.com
  Cc: PHYTEC Upstream, u-boot@lists.denx.de, tharvey@gateworks.com,
	manut@mecka.net, Yashwanth Varakala



Am 08.11.24 um 13:59 schrieb Yannic Moog:
> On Thu, 2024-11-07 at 10:07 +0100, Wadim Egorov wrote:
>>
>>
>> Am 07.11.24 um 09:23 schrieb Yannic Moog:
>>> Add documentation for the phyBOARD-Pollux i.MX 8M Plus on OP-TEE
>>> integration.
>>> Also add missing '-' to TF-A build instruction while at it.
>>
>>
>> Would it make sense to move these instructions into a more general
>> section, so they could be reused by other imx based boards as well?
> 
> I have not put much thought into it. At first glance though, when reading the documentation for a
> board I don't want to be redirected to other pages. So using an include comes to mind. Is that what
> you had in mind as well?

Yes, I was thinking about using includes to reduce doc duplication.

>   
>>
>> Seems you missed to update imx8mm-phygate-tauri-l.rst
> 
> Should be in the (next) patch for the imx8mm, as Tauri is based on phycore-imx8mm. Let me know if
> you meant something else.

Ah! I missed that you change both files in the next patch.

> 
> Yannic
> 
>>
>>>
>>> Signed-off-by: Yannic Moog <y.moog@phytec.de>
>>> ---
>>>    doc/board/phytec/phycore-imx8mp.rst | 26 +++++++++++++++++++++++++-
>>>    1 file changed, 25 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/doc/board/phytec/phycore-imx8mp.rst b/doc/board/phytec/phycore-imx8mp.rst
>>> index fda751aeffb..4bcfa9f39ca 100644
>>> --- a/doc/board/phytec/phycore-imx8mp.rst
>>> +++ b/doc/board/phytec/phycore-imx8mp.rst
>>> @@ -9,6 +9,7 @@ Quick Start
>>>    -----------
>>>    
>>>    - Build the ARM Trusted firmware binary
>>> +- Build the OP-TEE binary (optional)
>>>    - Get ddr firmware
>>>    - Build U-Boot
>>>    - Boot
>>> @@ -20,10 +21,32 @@ Build the ARM Trusted firmware binary
>>>    
>>>       $ git clone https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git
>>>       $ cd trusted-firmware-a
>>> -   $ export CROSS_COMPILE=aarch64-linux-gnu
>>> +   $ export CROSS_COMPILE=aarch64-linux-gnu-
>>>       $ export IMX_BOOT_UART_BASE=0x30860000
>>> +   $ # with optee
>>> +   $ make PLAT=imx8mp SPD=opteed bl31
>>> +   $ # without optee
>>>       $ make PLAT=imx8mp bl31
>>>    
>>> +Build the OP-TEE binary (optional)
>>> +----------------------------------
>>> +
>>> +.. code-block:: bash
>>> +
>>> +   $ git clone https://github.com/OP-TEE/optee_os.git
>>> +   $ cd optee_os
>>> +   $ make CFG_ARM64_core=y \
>>> +     CFG_TEE_BENCHMARK=n \
>>> +     CROSS_COMPILE=aarch64-linux-gnu- \
>>> +     CROSS_COMPILE_core=aarch64-linux-gnu- \
>>> +     CROSS_COMPILE_ta_arm32=arm-linux-gnueabihf- \
>>> +     CROSS_COMPILE_ta_arm64=aarch64-linux-gnu- \
>>> +     O=out/arm \
>>> +     PLATFORM=imx-mx8mpevk \
>>> +     CFG_TZDRAM_START=0x56000000 \
>>> +     CFG_DDR_SIZE=0x80000000 \
>>> +     CFG_UART_BASE=UART1_BASE
>>> +
>>>    Get the ddr firmware
>>>    --------------------
>>>    
>>> @@ -42,6 +65,7 @@ Copy binaries
>>>    .. code-block:: bash
>>>    
>>>       $ cp <TF-A dir>/build/imx8mp/release/bl31.bin .
>>> +   $ cp <OP-TEE dir>/out/arm/core/tee-raw.bin tee.bin
>>>       $ cp firmware-imx-8.19/firmware/ddr/synopsys/lpddr4*.bin .
>>>    
>>>    Build U-Boot
>>>
>>
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Upstream] [PATCH v2 2/6] phycore-imx8mp_defconfig: add optee load address
  2024-11-08 12:55     ` Yannic Moog
@ 2024-11-08 13:12       ` Wadim Egorov
  0 siblings, 0 replies; 21+ messages in thread
From: Wadim Egorov @ 2024-11-08 13:12 UTC (permalink / raw)
  To: Yannic Moog
  Cc: uboot-imx@nxp.com, manut@mecka.net, trini@konsulko.com,
	festevam@gmail.com, Yashwanth Varakala, PHYTEC Upstream,
	sbabic@denx.de, tharvey@gateworks.com, u-boot@lists.denx.de



Am 08.11.24 um 13:55 schrieb Yannic Moog:
> Hi Wadim,
> 
> On Thu, 2024-11-07 at 09:52 +0100, Wadim Egorov wrote:
>> Hi Yannic,
>>
>> Am 07.11.24 um 09:23 schrieb Yannic Moog:
>>> The phyBOARD-Pollux expects 0x56000000 address to load optee.
>>
>> You are already defining a default for IMX8M_OPTEE_LOAD_ADDR in the
>> mach-imx Kconf. You can drop the patches touching the defconfigs.
> 
> I would like to keep these patches as the default value is "only" there to get a functioning build.
> To me it would seem there has been no thought put in to what the actual correct value would be if
> left as default. Does that make sense?

Yes, this makes sense. Up to you, it was just a hint.

Well, it will become de facto standard if you put a default value into 
the Kconfig regardless if there has been no thoughts on the best 
location for it. Other boards will probably just follow and reuse it anyway.


> 
> Yannic
> 
>>
>> Regards,
>> Wadim
>>
>>>
>>> Signed-off-by: Yannic Moog <y.moog@phytec.de>
>>> ---
>>>    configs/phycore-imx8mp_defconfig | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/configs/phycore-imx8mp_defconfig b/configs/phycore-imx8mp_defconfig
>>> index 1240c7fcbd0..910109afd80 100644
>>> --- a/configs/phycore-imx8mp_defconfig
>>> +++ b/configs/phycore-imx8mp_defconfig
>>> @@ -5,6 +5,7 @@ CONFIG_SYS_MALLOC_LEN=0x2000000
>>>    CONFIG_SPL_GPIO=y
>>>    CONFIG_SPL_LIBCOMMON_SUPPORT=y
>>>    CONFIG_SPL_LIBGENERIC_SUPPORT=y
>>> +CONFIG_IMX8M_OPTEE_LOAD_ADDR=0x56000000
>>>    CONFIG_PHYTEC_SOM_DETECTION=y
>>>    CONFIG_SF_DEFAULT_SPEED=80000000
>>>    CONFIG_ENV_SIZE=0x10000
>>>
>>
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] arm: imx8m: add OP-TEE node
  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 13:49   ` Adam Ford
  2024-11-08 18:05     ` Tim Harvey
  1 sibling, 1 reply; 21+ messages in thread
From: Adam Ford @ 2024-11-08 13:49 UTC (permalink / raw)
  To: Yannic Moog
  Cc: Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team, Tom Rini,
	Tim Harvey, Manuel Traut, Benjamin Hahn, Teresa Remmet,
	Yashwanth Varakala, u-boot, upstream

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.
> +
> +
>  choice
>         prompt "NXP i.MX8M board select"
>         optional
>
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] arm: imx8m: add OP-TEE node
  2024-11-08 13:49   ` Adam Ford
@ 2024-11-08 18:05     ` Tim Harvey
  2024-11-11  7:48       ` Yannic Moog
  0 siblings, 1 reply; 21+ messages in thread
From: Tim Harvey @ 2024-11-08 18:05 UTC (permalink / raw)
  To: Adam Ford
  Cc: Yannic Moog, Stefano Babic, Fabio Estevam, NXP i.MX U-Boot Team,
	Tom Rini, Manuel Traut, Benjamin Hahn, Teresa Remmet,
	Yashwanth Varakala, u-boot, upstream

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

Best Regards,

Tim

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] arm: imx8m: add OP-TEE node
  2024-11-08 18:05     ` Tim Harvey
@ 2024-11-11  7:48       ` Yannic Moog
  2024-11-11 10:49         ` Adam Ford
  0 siblings, 1 reply; 21+ messages in thread
From: Yannic Moog @ 2024-11-11  7:48 UTC (permalink / raw)
  To: aford173@gmail.com, tharvey@gateworks.com
  Cc: manut@mecka.net, uboot-imx@nxp.com, trini@konsulko.com,
	festevam@gmail.com, Yashwanth Varakala, sbabic@denx.de,
	Benjamin Hahn, PHYTEC Upstream, u-boot@lists.denx.de,
	Teresa Remmet

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?

Yannic

> 
> Best Regards,
> 
> Tim


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] arm: imx8m: add OP-TEE node
  2024-11-11  7:48       ` Yannic Moog
@ 2024-11-11 10:49         ` Adam Ford
  2024-11-13  7:06           ` Yannic Moog
  0 siblings, 1 reply; 21+ messages in thread
From: Adam Ford @ 2024-11-11 10:49 UTC (permalink / raw)
  To: Yannic Moog
  Cc: tharvey@gateworks.com, manut@mecka.net, uboot-imx@nxp.com,
	trini@konsulko.com, festevam@gmail.com, Yashwanth Varakala,
	sbabic@denx.de, Benjamin Hahn, PHYTEC Upstream,
	u-boot@lists.denx.de, Teresa Remmet

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.

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
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] arm: imx8m: add OP-TEE node
  2024-11-11 10:49         ` Adam Ford
@ 2024-11-13  7:06           ` Yannic Moog
  2024-12-06 13:17             ` [Upstream] " Yannic Moog
  0 siblings, 1 reply; 21+ messages in thread
From: Yannic Moog @ 2024-11-13  7:06 UTC (permalink / raw)
  To: aford173@gmail.com
  Cc: manut@mecka.net, uboot-imx@nxp.com, trini@konsulko.com,
	festevam@gmail.com, Yashwanth Varakala, sbabic@denx.de,
	Benjamin Hahn, tharvey@gateworks.com, PHYTEC Upstream,
	u-boot@lists.denx.de, Teresa Remmet

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Upstream] [PATCH v2 1/6] arm: imx8m: add OP-TEE node
  2024-11-13  7:06           ` Yannic Moog
@ 2024-12-06 13:17             ` Yannic Moog
  0 siblings, 0 replies; 21+ messages in thread
From: Yannic Moog @ 2024-12-06 13:17 UTC (permalink / raw)
  To: aford173@gmail.com
  Cc: manut@mecka.net, uboot-imx@nxp.com, trini@konsulko.com,
	festevam@gmail.com, Yashwanth Varakala, upstream@lists.phytec.de,
	sbabic@denx.de, tharvey@gateworks.com, u-boot@lists.denx.de

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

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
> upstream@lists.phytec.de
> http://lists.phytec.de/cgi-bin/mailman/listinfo/upstream


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2024-12-06 13:17 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox