On 19/02/18 15:44, Tom Rini wrote:
On Fri, Feb 02, 2018 at 04:56:57PM +0100, Dr. Philipp Tomsich wrote:
Bryan,

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



On 02/02/18 15:02, Dr. Philipp Tomsich wrote:
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

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

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

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.

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.


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.

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
_______________________________________________
U-Boot mailing list
[email protected]
https://lists.denx.de/listinfo/u-boot

Reply via email to