Bryan, > On 21 Feb 2018, at 04:27, Bryan O'Donoghue <[email protected]> wrote: > > 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).
My guess is that TEE_COMBO should cover Kever’s case. We’ll probably need to wait and see if he encounters issues with this, once he’s back after the Chinese New Year holiday. > I resend my previous patchset renaming "optee" to "tee-standalone" and add > the type IH_TYPE_TEE_STANDALONE. As it looks as you’ll have your series revised first—could you add some documentation that summarises the various the ways of how OPTEE and some info on your use-case? I’d then make sure (prior to merging) that Kever provides an update against this file that provides more info on his use-case and implementation. Thanks, Philipp. _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
