[U-Boot] [PATCH v4 08/12] doc: TEE: Add documentation describing TEE in u-boot

[Andrew F. Davis]

On 02/26/2018 09:32 AM, Dr. Philipp Tomsich wrote:
> 
>> On 26 Feb 2018, at 16:26, Andrew F. Davis <afd at ti.com> wrote:
>>
>> On 02/26/2018 09:06 AM, Bryan O'Donoghue wrote:
>>>
>>>
>>> On 26/02/18 14:56, Dr. Philipp Tomsich wrote:
>>>> Now, I am confused.
>>>
>>> Fair enough.
>>>
>>> In my view there are really only two required paths i.e. kever's stuff
>>> can use the existing "tee" type, let's not discuss a third option further.
>>>
>>> So for clarity the proposal is
>>>
>>> 1. Maintain the existing "tee" as is.
>>>
>>>    As regards changing the name of "tee" to "tee-standalone" I'd like
>>>    to get Tom or Andrew (both) to say that's what is wanted.
>>>
>>>    Since it's TI boards that are using the "tee" name in mkimage
>>>    upstream the name-change is churn there.
>>>
>>>    Andrew, Tom ?
>>>
>>
>> I wouldn't be too opposed to the name change if we decide to go down
>> this two type path, but I'm still not convinced we are doing the right
>> thing here.
>>
>> Lets look at u-boot/include/image.h for a moment, the table of IH_TYPE_*
>> already has a type for what we are trying to do here: IH_TYPE_KERNEL.
>> The comment on this table describes "OS Kernel Images" as exactly what
>> you are doing with your TEE image.
>>
>> To me what you really want to do is add a new IH_OS_*, which are defined
>> in a different table above (I see this is already done for ATF
>> (IH_OS_ARM_TRUSTED_FIRMWARE) which shares a similar “boot-through” flow
>> like Philipp pointed out).
> 
> The IH_OS_ARM_TRUSTED_FIRMWARE (sorry for not shortening this, 
> when I added it) encodes a specific parameter passing convention for the
> ATF (i.e. bl31_params_t, platform-specific parameters — and in the future
> bl32_params_t for loading up an OPTEE on ARMv8).
> 
> The parameter passing convention for an OPTEE on ARMv7 is different
> (see https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/kernel/generic_entry_a32.S#L186)
> and therefore affords a separate OS type.
> 

I agree, that is what I am suggesting. A new IH_OS_ and not a new image
type IH_TYPE_ as IH_TYPE_KERNEL is already the right type.

> The thing I haven’t understood yet, though, is: how can the “returns to
> link-register” variant exist, if the parameter passing (and entry code) in
> the upstream OPTEE looks as it does...
> 


When the SMC call to install a TEE is made the ROM saves the non-secure
context, so in OP-TEE we simply retrieve this context [0] from the ROM
memory and use it as our non-secure context to return to, this puts us
right back to where the SMC came from when OP-TEE install finishes.

This allows us to do stuff like starting TEE from kernel dynamically at
runtime, something I'm not sure how the other flow could support if it
always expects to be an intermediate boot stage.. but I guess that is
their problem to solve not mine :)

[0]
https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/plat-ti/main.c#L128

Andrew


>>
>> So I'm still not sure what the technical reason you need a new "type" of
>> image, when adding your hooks to the existing IH_TYPE_KERNEL path in
>> U-Boot could be made to do the same thing when it encounters a IH_OS_TEE
>> OS image.
>>
>> Andrew
>>
>>> 2. Add a new bootable type
>>>    The set of names we have for that is
>>>
>>>    {tee-bootable, tee-chainload, tee-with-payload}
>>>
>>>    I have no strong feelings about the name for the new type either way
>>>
>>>    :)
>