rockchip: add tpl and OPTEE support for rk3229

Move original spl to tpl, and add spl to load next stage firmware, adapt all the address and option for them.

Serial-changes: 2
- update upon latest source

Signed-off-by Kever Yang <[email protected]>

---

Changes in v4:
- remove non-used MACRO define

Changes in v3:
- do not init ddr region in spl

Changes in v2: None

 arch/arm/mach-rockchip/Kconfig               |  9 +++
 arch/arm/mach-rockchip/Makefile              |  3 +-
 arch/arm/mach-rockchip/rk322x-board-spl.c    | 63 +++++---------------
 arch/arm/mach-rockchip/rk322x-board-tpl.c    | 88 ++++++++++++++++++++++++++++
 arch/arm/mach-rockchip/rk322x/u-boot-tpl.lds | 13 ++++
 include/configs/rk322x_common.h              |  9 ++-
 6 files changed, 132 insertions(+), 53 deletions(-)
 create mode 100644 arch/arm/mach-rockchip/rk322x-board-tpl.c
 create mode 100644 arch/arm/mach-rockchip/rk322x/u-boot-tpl.lds

Move original spl to tpl, and add spl to load next stage firmware, adapt all the address and option for them.

Serial-changes: 2
- update upon latest source

Signed-off-by Kever Yang <[email protected]>

Acked-by Philipp Tomsich <[email protected]>

---

Changes in v4:
- remove non-used MACRO define

Changes in v3:
- do not init ddr region in spl

Changes in v2: None

 arch/arm/mach-rockchip/Kconfig               |  9 +++
 arch/arm/mach-rockchip/Makefile              |  3 +-
 arch/arm/mach-rockchip/rk322x-board-spl.c    | 63 +++++---------------
 arch/arm/mach-rockchip/rk322x-board-tpl.c    | 88 ++++++++++++++++++++++++++++
 arch/arm/mach-rockchip/rk322x/u-boot-tpl.lds | 13 ++++
 include/configs/rk322x_common.h              |  9 ++-
 6 files changed, 132 insertions(+), 53 deletions(-)
 create mode 100644 arch/arm/mach-rockchip/rk322x-board-tpl.c
 create mode 100644 arch/arm/mach-rockchip/rk322x/u-boot-tpl.lds

OP-TEE is an open source trusted OS, in armv7, its loading and running are like this:
loading:
- SPL load both OP-TEE and U-Boot
running:
- SPL run into OP-TEE in secure mode;
- OP-TEE run into U-Boot in non-secure mode;

More detail:
https://github.com/OP-TEE/optee_os
and search for 'boot arguments' for detail entry parameter in:
core/arch/arm/kernel/generic_entry_a32.S

Signed-off-by Kever Yang <[email protected]>

---

Changes in v4:
- use NULL instead of '0'
- add fdt_addr as arg2 of entry

Changes in v3: None
Changes in v2:
- Using new image type for op-tee

 common/spl/Kconfig     |  7 +++++++
 common/spl/Makefile    |  1 +
 common/spl/spl.c       |  9 +++++++++
 common/spl/spl_optee.S | 13 +++++++++++++
 include/spl.h          | 13 +++++++++++++
 5 files changed, 43 insertions(+)
 create mode 100644 common/spl/spl_optee.S

Quoted 24 lines

OP-TEE is an open source trusted OS, in armv7, its loading and running are like this:
loading:
- SPL load both OP-TEE and U-Boot
running:
- SPL run into OP-TEE in secure mode;
- OP-TEE run into U-Boot in non-secure mode;

More detail:
https://github.com/OP-TEE/optee_os
and search for 'boot arguments' for detail entry parameter in:
core/arch/arm/kernel/generic_entry_a32.S

Signed-off-by Kever Yang <[email protected]>

---

Changes in v4:
- use NULL instead of '0'
- add fdt_addr as arg2 of entry

Changes in v3: None
Changes in v2:
- Using new image type for op-tee

Quoted 8 lines

  common/spl/Kconfig     |  7 +++++++
  common/spl/Makefile    |  1 +
  common/spl/spl.c       |  9 +++++++++
  common/spl/spl_optee.S | 13 +++++++++++++
  include/spl.h          | 13 +++++++++++++
  5 files changed, 43 insertions(+)

create mode 100644 common/spl/spl_optee.S

Acked-by Philipp Tomsich <[email protected]>

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Quoted 33 lines · Philipp Tomsich

Quoted 24 lines

OP-TEE is an open source trusted OS, in armv7, its loading and running are like this:
loading:
- SPL load both OP-TEE and U-Boot
running:
- SPL run into OP-TEE in secure mode;
- OP-TEE run into U-Boot in non-secure mode;

More detail:
https://github.com/OP-TEE/optee_os
and search for 'boot arguments' for detail entry parameter in:
core/arch/arm/kernel/generic_entry_a32.S

Signed-off-by Kever Yang <[email protected]>

---

Changes in v4:
- use NULL instead of '0'
- add fdt_addr as arg2 of entry

Changes in v3: None
Changes in v2:
- Using new image type for op-tee

Quoted 8 lines

  common/spl/Kconfig     |  7 +++++++
  common/spl/Makefile    |  1 +
  common/spl/spl.c       |  9 +++++++++
  common/spl/spl_optee.S | 13 +++++++++++++
  include/spl.h          | 13 +++++++++++++
  5 files changed, 43 insertions(+)

create mode 100644 common/spl/spl_optee.S

Quoted 6 lines

Acked-by Philipp Tomsich <[email protected]>

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Kever, Philipp

I have patches that define a IH_TYPE_OPTEE as a bootable OPTEE image (one that never returns to u-boot).

The image type allows bootm to identify the type of binary it is booting and perform image-specific checks.

On this patch though, do you guys really need a new image type for SPL ? Couldn't you use IH_TYPE_TEE in the same way ?

common/spl/spl.c +7 -0

+#if CONFIG_IS_ENABLED(OPTEE)
+    case IH_TYPE_TEE:
+        debug("Jumping to U-Boot via OP-TEE\n");
+        spl_optee_entry(NULL, NULL, spl_image->fdt_addr,
+                (void *)spl_image.entry_point);
+        break;
+#endif

i.e. it appears to me as if you don't actually do anything image-specific with IH_OS_OP_TEE that couldn't be done with the existing IH_TYPE_TEE... as above you use spl_image->fdt_addr and spl_image.entry_point - as opposed to any header specific data from the binary you are booting..

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Bryan,

Quoted 43 lines · Bryan O'Donoghue

Quoted 33 lines · Philipp Tomsich

Quoted 24 lines

OP-TEE is an open source trusted OS, in armv7, its loading and running are like this:
loading:
- SPL load both OP-TEE and U-Boot
running:
- SPL run into OP-TEE in secure mode;
- OP-TEE run into U-Boot in non-secure mode;

More detail:
https://github.com/OP-TEE/optee_os
and search for 'boot arguments' for detail entry parameter in:
core/arch/arm/kernel/generic_entry_a32.S

Signed-off-by Kever Yang <[email protected]>

---

Changes in v4:
- use NULL instead of '0'
- add fdt_addr as arg2 of entry

Changes in v3: None
Changes in v2:
- Using new image type for op-tee

Quoted 8 lines

 common/spl/Kconfig     |  7 +++++++
 common/spl/Makefile    |  1 +
 common/spl/spl.c       |  9 +++++++++
 common/spl/spl_optee.S | 13 +++++++++++++
 include/spl.h          | 13 +++++++++++++
 5 files changed, 43 insertions(+)
 create mode 100644 common/spl/spl_optee.S

Quoted 6 lines

Acked-by Philipp Tomsich <[email protected]>

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Quoted 4 lines

Kever, Philipp

I have patches that define a IH_TYPE_OPTEE as a bootable OPTEE image (one that never returns to u-boot).

I don't understand how you use OP-TEE in your project. You are using U-Boot 'bootm' cmd to run into OP-TEE and 'PC' never get out of the OP-TEE os?
The upstream OP-TEE will always be back to the pointer in 'lr'.

We are using boot flow like ARMv8 for OP-TEE in armv7, could you introduce your flow?

bootrom(BL1, secure word)->SPL(BL2, secure word)->OP-TEE(BL31/32, secure word)
->U-Boot(BL33, non-secure word)->Linux kernel(non secure word). By the OP-TEE design we need jump to U-Boot with using 'lr' as pointer to next stage, that's why I add
new type for SPL image:
https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/kernel/generic_entry_a32.S#L186

Thanks,
- Kever

Quoted 21 lines

The image type allows bootm to identify the type of binary it is booting and perform image-specific checks.

On this patch though, do you guys really need a new image type for SPL ? Couldn't you use IH_TYPE_TEE in the same way ?

common/spl/spl.c +7 -0

+#if CONFIG_IS_ENABLED(OPTEE)
+    case IH_TYPE_TEE:
+        debug("Jumping to U-Boot via OP-TEE\n");
+        spl_optee_entry(NULL, NULL, spl_image->fdt_addr,
+                (void *)spl_image.entry_point);
+        break;
+#endif

i.e. it appears to me as if you don't actually do anything image-specific with IH_OS_OP_TEE that couldn't be done with the existing IH_TYPE_TEE... as above you use spl_image->fdt_addr and spl_image.entry_point - as opposed to any header specific data from the binary you are booting..

Quoted 4 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Quoted 57 lines · Kever Yang

Bryan,

Quoted 43 lines · Bryan O'Donoghue

Quoted 33 lines · Philipp Tomsich

Quoted 24 lines

OP-TEE is an open source trusted OS, in armv7, its loading and running are like this:
loading:
- SPL load both OP-TEE and U-Boot
running:
- SPL run into OP-TEE in secure mode;
- OP-TEE run into U-Boot in non-secure mode;

More detail:
https://github.com/OP-TEE/optee_os
and search for 'boot arguments' for detail entry parameter in:
core/arch/arm/kernel/generic_entry_a32.S

Signed-off-by Kever Yang <[email protected]>

---

Changes in v4:
- use NULL instead of '0'
- add fdt_addr as arg2 of entry

Changes in v3: None
Changes in v2:
- Using new image type for op-tee

Quoted 8 lines

  common/spl/Kconfig     |  7 +++++++
  common/spl/Makefile    |  1 +
  common/spl/spl.c       |  9 +++++++++
  common/spl/spl_optee.S | 13 +++++++++++++
  include/spl.h          | 13 +++++++++++++
  5 files changed, 43 insertions(+)

create mode 100644 common/spl/spl_optee.S

Quoted 6 lines

Acked-by Philipp Tomsich <[email protected]>

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Quoted 4 lines

Kever, Philipp

I have patches that define a IH_TYPE_OPTEE as a bootable OPTEE image (one that never returns to u-boot).

I don't understand how you use OP-TEE in your project. You are using U-Boot 'bootm' cmd to run into OP-TEE and 'PC' never get out of the OP-TEE os?
The upstream OP-TEE will always be back to the pointer in 'lr'.

Quoted 2 lines

We are using boot flow like ARMv8 for OP-TEE in armv7, could you introduce your flow?

The flow is ROM->U-Boot->OP-TEE->Linux.

Quoted 7 lines

Quoted 7 lines

common/spl/spl.c +7 -0

+#if CONFIG_IS_ENABLED(OPTEE)
+    case IH_TYPE_TEE:
+        debug("Jumping to U-Boot via OP-TEE\n");
+        spl_optee_entry(NULL, NULL, spl_image->fdt_addr,
+                (void *)spl_image.entry_point);
+        break;
+#endif

Quoted 58 lines · Peng Fan

Quoted 57 lines · Kever Yang

Bryan,

Quoted 43 lines · Bryan O'Donoghue

Quoted 33 lines · Philipp Tomsich

Quoted 24 lines

OP-TEE is an open source trusted OS, in armv7, its loading and running are like this:
loading:
- SPL load both OP-TEE and U-Boot
running:
- SPL run into OP-TEE in secure mode;
- OP-TEE run into U-Boot in non-secure mode;

More detail:
https://github.com/OP-TEE/optee_os
and search for 'boot arguments' for detail entry parameter in:
core/arch/arm/kernel/generic_entry_a32.S

Signed-off-by Kever Yang <[email protected]>

---

Changes in v4:
- use NULL instead of '0'
- add fdt_addr as arg2 of entry

Changes in v3: None
Changes in v2:
- Using new image type for op-tee

Quoted 8 lines

  common/spl/Kconfig     |  7 +++++++
  common/spl/Makefile    |  1 +
  common/spl/spl.c       |  9 +++++++++
  common/spl/spl_optee.S | 13 +++++++++++++
  include/spl.h          | 13 +++++++++++++
  5 files changed, 43 insertions(+)

create mode 100644 common/spl/spl_optee.S

Quoted 6 lines

Acked-by Philipp Tomsich <[email protected]>

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Quoted 4 lines

Kever, Philipp

I have patches that define a IH_TYPE_OPTEE as a bootable OPTEE image (one that never returns to u-boot).

I don't understand how you use OP-TEE in your project. You are using U-Boot 'bootm' cmd to run into OP-TEE and 'PC' never get out of the OP-TEE os?
The upstream OP-TEE will always be back to the pointer in 'lr'.

Quoted 4 lines

Quoted 2 lines

We are using boot flow like ARMv8 for OP-TEE in armv7, could you introduce your flow?

The flow is ROM->U-Boot->OP-TEE->Linux.

Hi Kever, this got buried in my mailbox...

Which I believe is the preferred flow for armv7

That said - if you look at the TI commits for IH_TYPE_TEE I believe *that* is the appropriate type for what you are doing with SPL.

As I read the TI code the TEE is installed into memory - jumped into and returned out of, which is the model you are using with SPL here.

Basically that's what arch/arm/mach-omap2/sec-common.c::secure_tee_install() does.

So, I think the SPL solution you have can co-exist quite easily with the model we have described above for imx7.

My only suggestion here is that you probably don't need to define a new image type for SPL - you can reuse IH_TYPE_TEE for SPL as below.

Other than that I think the SPL/bootm or indeed the TI method of installing a TEE are entirely able to (and should) live side-by-side.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

common/spl/spl.c +7 -0

+#if CONFIG_IS_ENABLED(OPTEE)
+    case IH_TYPE_TEE:
+        debug("Jumping to U-Boot via OP-TEE\n");
+        spl_optee_entry(NULL, NULL, spl_image->fdt_addr,
+                (void *)spl_image.entry_point);
+        break;
+#endif

I don't think we can reuse IH_TYPE_TEE， it use a optee.img type create by mkimage and it seem use more then one cpu.

Quoted 2 lines · Kever Yang

I don't think we can reuse IH_TYPE_TEE， it use a optee.img type create by mkimage and it seem use more then one cpu.

Don't really understand what you mean by using more than one CPU - can you give an example in the code ?

---
bod

Quoted 5 lines · Bryan O'Donoghue

Quoted 1 line · Kever Yang

I don't think we can reuse IH_TYPE_TEE， it use a optee.img type create by mkimage and it seem use more then one cpu.

Don't really understand what you mean by using more than one CPU - can you give an example in the code ?
---
bod

So - what's the proposed resolution here ?

Where do we stand on this: can we reuse IH_TYPE_TEE, will be use IH_TYPE_OPTEE or will there be a new IH_TYPE_OPTEE_SPL?

Quoted 1 line · Dr. Philipp Tomsich

Where do we stand on this: can we reuse IH_TYPE_TEE, will be use IH_TYPE_OPTEE or will there be a new IH_TYPE_OPTEE_SPL?

I think because you aren't doing anything different with the image type you can reuse IH_TYPE_TEE

This

(quoted diff) +7 -0

+#if CONFIG_IS_ENABLED(OPTEE)
+	case IH_OS_OP_TEE:
+		debug("Jumping to U-Boot via OP-TEE\n");
+		spl_optee_entry(NULL, NULL, spl_image->fdt_addr,
+				(void *)spl_image.entry_point);
+		break;
+#endif

could just as easily be this

(quoted diff) +7 -0

+#if CONFIG_IS_ENABLED(OPTEE)
+	case IH_TYPE_TEE:
+		debug("Jumping to U-Boot via OP-TEE\n");
+		spl_optee_entry(NULL, NULL, spl_image->fdt_addr,
+				(void *)spl_image.entry_point);
+		break;
+#endif

should be a matter of just replacing the call to mkimage to use

mkimage -A arm -T tee

instead of

mkimage -A arm -T optee

and the suggested change above.. case IH_OS_OP_TEE -> case IH_TYPE_TEE

Bryan,

Quoted 31 lines · Bryan O'Donoghue

Quoted 1 line · Dr. Philipp Tomsich

Where do we stand on this: can we reuse IH_TYPE_TEE, will be use IH_TYPE_OPTEE or will there be a new IH_TYPE_OPTEE_SPL?

I think because you aren't doing anything different with the image type you can reuse IH_TYPE_TEE

This

(quoted diff) +7 -0

+#if CONFIG_IS_ENABLED(OPTEE)
+	case IH_OS_OP_TEE:
+		debug("Jumping to U-Boot via OP-TEE\n");
+		spl_optee_entry(NULL, NULL, spl_image->fdt_addr,
+				(void *)spl_image.entry_point);
+		break;
+#endif

could just as easily be this

(quoted diff) +7 -0

+#if CONFIG_IS_ENABLED(OPTEE)
+	case IH_TYPE_TEE:
+		debug("Jumping to U-Boot via OP-TEE\n");
+		spl_optee_entry(NULL, NULL, spl_image->fdt_addr,
+				(void *)spl_image.entry_point);
+		break;
+#endif

should be a matter of just replacing the call to mkimage to use

Quoted 7 lines

mkimage -A arm -T tee

instead of

mkimage -A arm -T optee

and the suggested change above.. case IH_OS_OP_TEE -> case IH_TYPE_TEE

Thanks for summarising your suggestion.

However, my mail was intended to test the waters to see what the consensus was. A it appears that none has yet emerged between all the involved parties (including our colleagues from TI that had also chimed in on the discussion).
So for now, I’ll sit back and wait until some sort of consensus (or at least a majority for one solution or the other) emerges.

Personally, I am not happy with having a ‘tee’ and an ‘optee’ both refering to OP-TEE and the upstream OP-TEE documentation suggesting that their envisioned boot process was to boot through the OP-TEE (i.e. what the ‘tee’ image type does). However, with the ‘tee’ image type already being defined, we seem to have already backed ourselves into a situation where the naming is non-intuitive.

Quoted 49 lines · Dr. Philipp Tomsich

Bryan,

Quoted 31 lines · Bryan O'Donoghue

Quoted 1 line · Dr. Philipp Tomsich

Where do we stand on this: can we reuse IH_TYPE_TEE, will be use IH_TYPE_OPTEE or will there be a new IH_TYPE_OPTEE_SPL?

I think because you aren't doing anything different with the image type you can reuse IH_TYPE_TEE

This

(quoted diff) +7 -0

+#if CONFIG_IS_ENABLED(OPTEE)
+	case IH_OS_OP_TEE:
+		debug("Jumping to U-Boot via OP-TEE\n");
+		spl_optee_entry(NULL, NULL, spl_image->fdt_addr,
+				(void *)spl_image.entry_point);
+		break;
+#endif

could just as easily be this

(quoted diff) +7 -0

+#if CONFIG_IS_ENABLED(OPTEE)
+	case IH_TYPE_TEE:
+		debug("Jumping to U-Boot via OP-TEE\n");
+		spl_optee_entry(NULL, NULL, spl_image->fdt_addr,
+				(void *)spl_image.entry_point);
+		break;
+#endif

should be a matter of just replacing the call to mkimage to use

Quoted 7 lines

mkimage -A arm -T tee

instead of

mkimage -A arm -T optee

and the suggested change above.. case IH_OS_OP_TEE -> case IH_TYPE_TEE

Thanks for summarising your suggestion.

However, my mail was intended to test the waters to see what the consensus was. A it appears that none has yet emerged between all the involved parties (including our colleagues from TI that had also chimed in on the discussion).
So for now, I’ll sit back and wait until some sort of consensus (or at least a majority for one solution or the other) emerges.

Quoted 5 lines

Personally, I am not happy with having a ‘tee’ and an ‘optee’ both refering to OP-TEE and the upstream OP-TEE documentation suggesting that their envisioned boot process was to boot through the OP-TEE (i.e. what the ‘tee’ image type does). However, with the ‘tee’ image type already being defined, we seem to have already backed ourselves into a situation where the naming is non-intuitive.

Alright, I'm a little confused here. I guess first, there's a disconnect in upstream OP-TEE land about how 32bit ARM should be done? I guess we have three different implemented and upstreamed flows:
- SPL->U-Boot->OP-TEE->U-Boot->Linux
- SPL->U-Boot->OP-TEE->Linux
- SPL->OP-TEE->U-Boot->Linux

And in this last case we have a combined image that is passed from SPL to OP-TEE.

Since all 3 of these flows are in upstream OP-TEE, we need to support them all.

The biggest constraint is that we have the first flow already in and named "tee" (my fault, I should have made sure everyone would use the same flow). So we need to have descriptive enough names for the other flows that we're going to add so that it's clear what's what. How about "tee-standalone" for "U-Boot starts OP-TEE, and is done"

"tee-combo" for "SPL gives OP-TEE an image to deal with". This could even in theory I suspect be SPL gives OP-TEE a Linux kernel to boot. I'm also happy to hear better suffixes but I don't want "tee" and "optee". And if we can cover two flows under the same name, that's good too, we just need to name the last flow "tee-something". Thanks all!

Quoted 55 lines · Tom Rini

Quoted 49 lines · Dr. Philipp Tomsich

Bryan,

Quoted 31 lines · Bryan O'Donoghue

Quoted 1 line · Dr. Philipp Tomsich

Where do we stand on this: can we reuse IH_TYPE_TEE, will be use IH_TYPE_OPTEE or will there be a new IH_TYPE_OPTEE_SPL?

I think because you aren't doing anything different with the image type you can reuse IH_TYPE_TEE

This

(quoted diff) +7 -0

+#if CONFIG_IS_ENABLED(OPTEE)
+	case IH_OS_OP_TEE:
+		debug("Jumping to U-Boot via OP-TEE\n");
+		spl_optee_entry(NULL, NULL, spl_image->fdt_addr,
+				(void *)spl_image.entry_point);
+		break;
+#endif

could just as easily be this

(quoted diff) +7 -0

+#if CONFIG_IS_ENABLED(OPTEE)
+	case IH_TYPE_TEE:
+		debug("Jumping to U-Boot via OP-TEE\n");
+		spl_optee_entry(NULL, NULL, spl_image->fdt_addr,
+				(void *)spl_image.entry_point);
+		break;
+#endif

should be a matter of just replacing the call to mkimage to use

Quoted 7 lines

mkimage -A arm -T tee

instead of

mkimage -A arm -T optee

and the suggested change above.. case IH_OS_OP_TEE -> case IH_TYPE_TEE

Thanks for summarising your suggestion.

However, my mail was intended to test the waters to see what the consensus was. A it appears that none has yet emerged between all the involved parties (including our colleagues from TI that had also chimed in on the discussion).
So for now, I’ll sit back and wait until some sort of consensus (or at least a majority for one solution or the other) emerges.

Quoted 5 lines

Personally, I am not happy with having a ‘tee’ and an ‘optee’ both refering to OP-TEE and the upstream OP-TEE documentation suggesting that their envisioned boot process was to boot through the OP-TEE (i.e. what the ‘tee’ image type does). However, with the ‘tee’ image type already being defined, we seem to have already backed ourselves into a situation where the naming is non-intuitive.

Quoted 6 lines

Alright, I'm a little confused here. I guess first, there's a disconnect in upstream OP-TEE land about how 32bit ARM should be done? I guess we have three different implemented and upstreamed flows:
- SPL->U-Boot->OP-TEE->U-Boot->Linux
- SPL->U-Boot->OP-TEE->Linux
- SPL->OP-TEE->U-Boot->Linux

I'd call that

- SPL/BootROM->U-Boot->OP-TEE->U-Boot->Linux
- SPL/BootROM->U-Boot->OP-TEE->Linux
- SPL/BootROM->OP-TEE->U-Boot->Linux

But yes - fundamentally your flow is correct.

Quoted 4 lines

And in this last case we have a combined image that is passed from SPL to OP-TEE.
Since all 3 of these flows are in upstream OP-TEE, we need to support them all.

Agreed.

Quoted 11 lines

The biggest constraint is that we have the first flow already in and named "tee" (my fault, I should have made sure everyone would use the same flow). So we need to have descriptive enough names for the other flows that we're going to add so that it's clear what's what. How about "tee-standalone" for "U-Boot starts OP-TEE, and is done"

"tee-combo" for "SPL gives OP-TEE an image to deal with". This could even in theory I suspect be SPL gives OP-TEE a Linux kernel to boot. I'm also happy to hear better suffixes but I don't want "tee" and "optee". And if we can cover two flows under the same name, that's good too, we just need to name the last flow "tee-something". Thanks all!

Yes I take your point "tee" and "optee" are the opposite of descriptive names.

- SPL/BootROM->U-Boot->OP-TEE->U-Boot->Linux
Currently called "tee" - has type IH_TEE - maintained as is for compatibility - deserves some documentation.

- SPL/BootROM->U-Boot->OP-TEE->Linux
New type "tee-standalone" this would be the type I've proposed in my patch set. New type IH_TYPE_TEE_STANDALONE

- SPL/BootROM->OP-TEE->U-Boot->Linux
New type "tee-combo" this would be Kever's type IH_TYPE_TEE_COMBO

I've suggested to Kever that he doesn't actually need a separate type (though I could be wrong).

I resend my previous patchset renaming "optee" to "tee-standalone" and add the type IH_TYPE_TEE_STANDALONE.

Quoted 66 lines · Tom Rini

Quoted 55 lines · Dr. Philipp Tomsich

Bryan,

Quoted 34 lines

On 2 Feb 2018, at 16:37, Bryan O'Donoghue
<[email protected]> wrote:

Quoted 2 lines · Dr. Philipp Tomsich

Where do we stand on this: can we reuse IH_TYPE_TEE, will be use IH_TYPE_OPTEE or will there be a new IH_TYPE_OPTEE_SPL?

I think because you aren't doing anything different with the image type you can reuse IH_TYPE_TEE

This

(quoted diff) +7 -0

+#if CONFIG_IS_ENABLED(OPTEE)
+    case IH_OS_OP_TEE:
+        debug("Jumping to U-Boot via OP-TEE\n");
+        spl_optee_entry(NULL, NULL, spl_image->fdt_addr,
+                (void *)spl_image.entry_point);
+        break;
+#endif

could just as easily be this

(quoted diff) +7 -0

+#if CONFIG_IS_ENABLED(OPTEE)
+    case IH_TYPE_TEE:
+        debug("Jumping to U-Boot via OP-TEE\n");
+        spl_optee_entry(NULL, NULL, spl_image->fdt_addr,
+                (void *)spl_image.entry_point);
+        break;
+#endif

should be a matter of just replacing the call to mkimage to use

Quoted 7 lines

mkimage -A arm -T tee

instead of

mkimage -A arm -T optee

and the suggested change above.. case IH_OS_OP_TEE -> case IH_TYPE_TEE

Thanks for summarising your suggestion.

However, my mail was intended to test the waters to see what the consensus was.
A it appears that none has yet emerged between all the involved parties (including
our colleagues from TI that had also chimed in on the discussion). So for now, I’ll sit back and wait until some sort of consensus (or at least a majority
for one solution or the other) emerges.

Quoted 9 lines

Personally, I am not happy with having a ‘tee’ and an ‘optee’ both refering to OP-TEE
and the upstream OP-TEE documentation suggesting that their envisioned boot
process was to boot through the OP-TEE (i.e. what the ‘tee’ image type does).
However, with the ‘tee’ image type already being defined, we seem to have already
backed ourselves into a situation where the naming is non-intuitive.

Quoted 6 lines

Alright, I'm a little confused here.  I guess first, there's a disconnect in upstream OP-TEE land about how 32bit ARM should be done? I guess we have three different implemented and upstreamed flows:
- SPL->U-Boot->OP-TEE->U-Boot->Linux
- SPL->U-Boot->OP-TEE->Linux
- SPL->OP-TEE->U-Boot->Linux

I'd call that

- SPL/BootROM->U-Boot->OP-TEE->U-Boot->Linux
- SPL/BootROM->U-Boot->OP-TEE->Linux
- SPL/BootROM->OP-TEE->U-Boot->Linux

But yes - fundamentally your flow is correct.

Quoted 6 lines

And in this last case we have a combined image that is passed from SPL to OP-TEE.

Since all 3 of these flows are in upstream OP-TEE, we need to support them all.

Agreed.

Quoted 11 lines

The biggest constraint is that we have the first flow already in and named "tee" (my fault, I should have made sure everyone would use the same flow).  So we need to have descriptive enough names for the other flows that we're going to add so that it's clear what's what.  How about "tee-standalone" for "U-Boot starts OP-TEE, and is done"

"tee-combo" for "SPL gives OP-TEE an image to deal with".  This could even in theory I suspect be SPL gives OP-TEE a Linux kernel to boot. I'm also happy to hear better suffixes but I don't want "tee" and "optee".  And if we can cover two flows under the same name, that's good too, we just need to name the last flow "tee-something".  Thanks all!

Yes I take your point "tee" and "optee" are the opposite of descriptive names.

- SPL/BootROM->U-Boot->OP-TEE->U-Boot->Linux
  Currently called "tee" - has type IH_TEE - maintained as is for compatibility - deserves some documentation.

- SPL/BootROM->U-Boot->OP-TEE->Linux
  New type "tee-standalone" this would be the type I've proposed in my patch set. New type IH_TYPE_TEE_STANDALONE

- SPL/BootROM->OP-TEE->U-Boot->Linux
  New type "tee-combo" this would be Kever's type IH_TYPE_TEE_COMBO

I've suggested to Kever that he doesn't actually need a separate type (though I could be wrong).

I resend my previous patchset renaming "optee" to "tee-standalone" and add the type IH_TYPE_TEE_STANDALONE.

I leave it to Kever to decide next steps for his patches.

---
bod

OP-TEE is an open source trusted OS, in armv7, its loading and running are like this:
loading:
- SPL load both OP-TEE and U-Boot
running:
- SPL run into OP-TEE in secure mode;
- OP-TEE run into U-Boot in non-secure mode;

More detail:
https://github.com/OP-TEE/optee_os
and search for 'boot arguments' for detail entry parameter in:
core/arch/arm/kernel/generic_entry_a32.S

Signed-off-by Kever Yang <[email protected]>

Acked-by Philipp Tomsich <[email protected]>

---

Changes in v4:
- use NULL instead of '0'
- add fdt_addr as arg2 of entry

Changes in v3: None
Changes in v2:
- Using new image type for op-tee

 common/spl/Kconfig     |  7 +++++++
 common/spl/Makefile    |  1 +
 common/spl/spl.c       |  9 +++++++++
 common/spl/spl_optee.S | 13 +++++++++++++
 include/spl.h          | 13 +++++++++++++
 5 files changed, 43 insertions(+)
 create mode 100644 common/spl/spl_optee.S