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 bryan.odonoghue@linaro.org 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