[U-Boot] [PATCH] optee: support rockchip optee binary release

[U-Boot] [PATCH] optee: support rockchip optee binary release

[Kever Yang]

Rockchip provide tee binary release in 'rkbin' repository:
https://github.com/rockchip-linux/rkbin
For some historical reason, rockchip optee binary is using
'r1' instead of 'lr' as U-Boot entry.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

 common/spl/spl_optee.S | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/common/spl/spl_optee.S b/common/spl/spl_optee.S
index 8bd1949ddf..092307b3cc 100644
--- a/common/spl/spl_optee.S
+++ b/common/spl/spl_optee.S
@@ -8,5 +8,8 @@
 
 ENTRY(spl_optee_entry)
 	ldr lr, =CONFIG_SYS_TEXT_BASE
+#ifdef CONFIG_ARCH_ROCKCHIP
+	ldr r1, =CONFIG_SYS_TEXT_BASE
+#endif
 	mov pc, r3
 ENDPROC(spl_optee_entry)
-- 
2.20.1

[U-Boot] [PATCH] optee: support rockchip optee binary release

[Philipp Tomsich]

Kever,

> On 29.03.2019, at 12:21, Kever Yang <kever.yang@rock-chips.com> wrote:
> 
> Rockchip provide tee binary release in 'rkbin' repository:
> https://github.com/rockchip-linux/rkbin
> For some historical reason, rockchip optee binary is using
> 'r1' instead of 'lr' as U-Boot entry.
> 
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
> 
> common/spl/spl_optee.S | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/common/spl/spl_optee.S b/common/spl/spl_optee.S
> index 8bd1949ddf..092307b3cc 100644
> --- a/common/spl/spl_optee.S
> +++ b/common/spl/spl_optee.S
> @@ -8,5 +8,8 @@
> 
> ENTRY(spl_optee_entry)
> 	ldr lr, =CONFIG_SYS_TEXT_BASE
> +#ifdef CONFIG_ARCH_ROCKCHIP

Can we make this selectable based on a dedicated config-option?  We provide our
own OPTEE port for some of our modules and I would like to have this as an opt-in
or opt-out option in Kconfig.

> +	ldr r1, =CONFIG_SYS_TEXT_BASE
> +#endif
> 	mov pc, r3
> ENDPROC(spl_optee_entry)
> -- 
> 2.20.1
> 

[U-Boot] [PATCH] optee: support rockchip optee binary release

[Kever Yang]

Philipp,

On 03/29/2019 07:25 PM, Philipp Tomsich wrote:
> Kever,
>
>> On 29.03.2019, at 12:21, Kever Yang <kever.yang@rock-chips.com> wrote:
>>
>> Rockchip provide tee binary release in 'rkbin' repository:
>> https://github.com/rockchip-linux/rkbin
>> For some historical reason, rockchip optee binary is using
>> 'r1' instead of 'lr' as U-Boot entry.
>>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>>
>> common/spl/spl_optee.S | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/common/spl/spl_optee.S b/common/spl/spl_optee.S
>> index 8bd1949ddf..092307b3cc 100644
>> --- a/common/spl/spl_optee.S
>> +++ b/common/spl/spl_optee.S
>> @@ -8,5 +8,8 @@
>>
>> ENTRY(spl_optee_entry)
>> 	ldr lr, =CONFIG_SYS_TEXT_BASE
>> +#ifdef CONFIG_ARCH_ROCKCHIP
> Can we make this selectable based on a dedicated config-option?  We provide our
> own OPTEE port for some of our modules and I would like to have this as an opt-in
> or opt-out option in Kconfig.

I think you are using OPTEE for RK3368/RK3399, right? Then the use case
is different, you are using OPTEE as bl32 for armv8, and this spl_optee is
for armv7 only.
I'm OK to add a Kconfig option if you really have different usage,
and I think this patch does not break things because the no one use 'r1'
now.

Thanks,
- Kever
>
>> +	ldr r1, =CONFIG_SYS_TEXT_BASE
>> +#endif
>> 	mov pc, r3
>> ENDPROC(spl_optee_entry)
>> -- 
>> 2.20.1
>>
>

[U-Boot] [PATCH] optee: support rockchip optee binary release

[Philipp Tomsich]

> On 29.03.2019, at 13:16, Kever Yang <kever.yang@rock-chips.com> wrote:
> 
> Philipp,
> 
> On 03/29/2019 07:25 PM, Philipp Tomsich wrote:
>> Kever,
>> 
>>> On 29.03.2019, at 12:21, Kever Yang <kever.yang@rock-chips.com> wrote:
>>> 
>>> Rockchip provide tee binary release in 'rkbin' repository:
>>> https://github.com/rockchip-linux/rkbin
>>> For some historical reason, rockchip optee binary is using
>>> 'r1' instead of 'lr' as U-Boot entry.
>>> 
>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>> ---
>>> 
>>> common/spl/spl_optee.S | 3 +++
>>> 1 file changed, 3 insertions(+)
>>> 
>>> diff --git a/common/spl/spl_optee.S b/common/spl/spl_optee.S
>>> index 8bd1949ddf..092307b3cc 100644
>>> --- a/common/spl/spl_optee.S
>>> +++ b/common/spl/spl_optee.S
>>> @@ -8,5 +8,8 @@
>>> 
>>> ENTRY(spl_optee_entry)
>>> 	ldr lr, =CONFIG_SYS_TEXT_BASE
>>> +#ifdef CONFIG_ARCH_ROCKCHIP
>> Can we make this selectable based on a dedicated config-option?  We provide our
>> own OPTEE port for some of our modules and I would like to have this as an opt-in
>> or opt-out option in Kconfig.
> 
> I think you are using OPTEE for RK3368/RK3399, right? Then the use case
> is different, you are using OPTEE as bl32 for armv8, and this spl_optee is
> for armv7 only.
> I'm OK to add a Kconfig option if you really have different usage,
> and I think this patch does not break things because the no one use 'r1'
> now.

Thanks for the clarification.
You’re right that I am only interested in ARMv8…

Thanks,
Philipp.

> Thanks,
> - Kever
>> 
>>> +	ldr r1, =CONFIG_SYS_TEXT_BASE
>>> +#endif
>>> 	mov pc, r3
>>> ENDPROC(spl_optee_entry)
>>> -- 
>>> 2.20.1
>>> 
>> 
> 
> 
> 

[U-Boot] [PATCH] optee: support rockchip optee binary release

[Heiko Stübner]

Hi Kever,

Am Freitag, 29. März 2019, 13:16:26 CET schrieb Kever Yang:
> On 03/29/2019 07:25 PM, Philipp Tomsich wrote:
> >> On 29.03.2019, at 12:21, Kever Yang <kever.yang@rock-chips.com> wrote:
> >>
> >> Rockchip provide tee binary release in 'rkbin' repository:
> >> https://github.com/rockchip-linux/rkbin
> >> For some historical reason, rockchip optee binary is using
> >> 'r1' instead of 'lr' as U-Boot entry.
> >>
> >> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> >> ---
> >>
> >> common/spl/spl_optee.S | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/common/spl/spl_optee.S b/common/spl/spl_optee.S
> >> index 8bd1949ddf..092307b3cc 100644
> >> --- a/common/spl/spl_optee.S
> >> +++ b/common/spl/spl_optee.S
> >> @@ -8,5 +8,8 @@
> >>
> >> ENTRY(spl_optee_entry)
> >> 	ldr lr, =CONFIG_SYS_TEXT_BASE
> >> +#ifdef CONFIG_ARCH_ROCKCHIP
> > Can we make this selectable based on a dedicated config-option?  We provide our
> > own OPTEE port for some of our modules and I would like to have this as an opt-in
> > or opt-out option in Kconfig.
> 
> I think you are using OPTEE for RK3368/RK3399, right? Then the use case
> is different, you are using OPTEE as bl32 for armv8, and this spl_optee is
> for armv7 only.
> I'm OK to add a Kconfig option if you really have different usage,
> and I think this patch does not break things because the no one use 'r1'
> now.

rk3229 has support in upstream op-tee, so possibly the calling convention
is different with that? And of course there may come a time when people
may want to use upstream-op-tee instead of a binary-blob.

Heiko

[U-Boot] [PATCH] optee: support rockchip optee binary release

[Kever Yang]

Hi Heiko,


On 04/01/2019 05:02 AM, Heiko Stübner wrote:
> Hi Kever,
>
> Am Freitag, 29. März 2019, 13:16:26 CET schrieb Kever Yang:
>> On 03/29/2019 07:25 PM, Philipp Tomsich wrote:
>>>> On 29.03.2019, at 12:21, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>
>>>> Rockchip provide tee binary release in 'rkbin' repository:
>>>> https://github.com/rockchip-linux/rkbin
>>>> For some historical reason, rockchip optee binary is using
>>>> 'r1' instead of 'lr' as U-Boot entry.
>>>>
>>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>>> ---
>>>>
>>>> common/spl/spl_optee.S | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/common/spl/spl_optee.S b/common/spl/spl_optee.S
>>>> index 8bd1949ddf..092307b3cc 100644
>>>> --- a/common/spl/spl_optee.S
>>>> +++ b/common/spl/spl_optee.S
>>>> @@ -8,5 +8,8 @@
>>>>
>>>> ENTRY(spl_optee_entry)
>>>> 	ldr lr, =CONFIG_SYS_TEXT_BASE
>>>> +#ifdef CONFIG_ARCH_ROCKCHIP
>>> Can we make this selectable based on a dedicated config-option?  We provide our
>>> own OPTEE port for some of our modules and I would like to have this as an opt-in
>>> or opt-out option in Kconfig.
>> I think you are using OPTEE for RK3368/RK3399, right? Then the use case
>> is different, you are using OPTEE as bl32 for armv8, and this spl_optee is
>> for armv7 only.
>> I'm OK to add a Kconfig option if you really have different usage,
>> and I think this patch does not break things because the no one use 'r1'
>> now.
> rk3229 has support in upstream op-tee, so possibly the calling convention
> is different with that? And of course there may come a time when people
> may want to use upstream-op-tee instead of a binary-blob.

Upstream op-tee is using 'lr' and rockchip binary is now using 'r1' for
u-boot entry.
So upstream op-tee can be boot if my patch series for rk3229 is merged,
and with this patch, people can chose to use rockchip binary for rk3229
or other
armv7 SoCs.

eg. I'll add op-tee support for rk3288 later because of the psci needed
by mainline kernel.

Thanks,
- Kever
>
> Heiko
>
>
>

[U-Boot] [PATCH] optee: support rockchip optee binary release

[Heiko Stübner]

Am Montag, 1. April 2019, 04:32:28 CEST schrieb Kever Yang:
> Hi Heiko,
> 
> 
> On 04/01/2019 05:02 AM, Heiko Stübner wrote:
> > Hi Kever,
> >
> > Am Freitag, 29. März 2019, 13:16:26 CET schrieb Kever Yang:
> >> On 03/29/2019 07:25 PM, Philipp Tomsich wrote:
> >>>> On 29.03.2019, at 12:21, Kever Yang <kever.yang@rock-chips.com> wrote:
> >>>>
> >>>> Rockchip provide tee binary release in 'rkbin' repository:
> >>>> https://github.com/rockchip-linux/rkbin
> >>>> For some historical reason, rockchip optee binary is using
> >>>> 'r1' instead of 'lr' as U-Boot entry.
> >>>>
> >>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> >>>> ---
> >>>>
> >>>> common/spl/spl_optee.S | 3 +++
> >>>> 1 file changed, 3 insertions(+)
> >>>>
> >>>> diff --git a/common/spl/spl_optee.S b/common/spl/spl_optee.S
> >>>> index 8bd1949ddf..092307b3cc 100644
> >>>> --- a/common/spl/spl_optee.S
> >>>> +++ b/common/spl/spl_optee.S
> >>>> @@ -8,5 +8,8 @@
> >>>>
> >>>> ENTRY(spl_optee_entry)
> >>>> 	ldr lr, =CONFIG_SYS_TEXT_BASE
> >>>> +#ifdef CONFIG_ARCH_ROCKCHIP
> >>> Can we make this selectable based on a dedicated config-option?  We provide our
> >>> own OPTEE port for some of our modules and I would like to have this as an opt-in
> >>> or opt-out option in Kconfig.
> >> I think you are using OPTEE for RK3368/RK3399, right? Then the use case
> >> is different, you are using OPTEE as bl32 for armv8, and this spl_optee is
> >> for armv7 only.
> >> I'm OK to add a Kconfig option if you really have different usage,
> >> and I think this patch does not break things because the no one use 'r1'
> >> now.
> > rk3229 has support in upstream op-tee, so possibly the calling convention
> > is different with that? And of course there may come a time when people
> > may want to use upstream-op-tee instead of a binary-blob.
> 
> Upstream op-tee is using 'lr' and rockchip binary is now using 'r1' for
> u-boot entry.
> So upstream op-tee can be boot if my patch series for rk3229 is merged,
> and with this patch, people can chose to use rockchip binary for rk3229
> or other
> armv7 SoCs.

Yes, an that is the reason there should be a Kconfig symbol for that :-)


> eg. I'll add op-tee support for rk3288 later because of the psci needed
> by mainline kernel.

please keep in mind that old device-trees in mainline need to stay working
aka you cannot expect people to update their firmware just to keep using
mainline kernels. So u-boot should modify the kernel-devicetree accordingly
to enable psci, before starting the kernel.

In my ATF adventure I already did some smallish adaptions on the kernel
side for this:
https://github.com/mmind/linux-rockchip/commit/27bce39800a14abe08d5a5994abef29f9b543c68


Heiko

[U-Boot] [PATCH] optee: support rockchip optee binary release

[Kever Yang]

Heiko,


On 04/01/2019 01:42 PM, Heiko Stübner wrote:
> Am Montag, 1. April 2019, 04:32:28 CEST schrieb Kever Yang:
>> Hi Heiko,
>>
>>
>> On 04/01/2019 05:02 AM, Heiko Stübner wrote:
>>> Hi Kever,
>>>
>>> Am Freitag, 29. März 2019, 13:16:26 CET schrieb Kever Yang:
>>>> On 03/29/2019 07:25 PM, Philipp Tomsich wrote:
>>>>>> On 29.03.2019, at 12:21, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>>
>>>>>> Rockchip provide tee binary release in 'rkbin' repository:
>>>>>> https://github.com/rockchip-linux/rkbin
>>>>>> For some historical reason, rockchip optee binary is using
>>>>>> 'r1' instead of 'lr' as U-Boot entry.
>>>>>>
>>>>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>>>>> ---
>>>>>>
>>>>>> common/spl/spl_optee.S | 3 +++
>>>>>> 1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/common/spl/spl_optee.S b/common/spl/spl_optee.S
>>>>>> index 8bd1949ddf..092307b3cc 100644
>>>>>> --- a/common/spl/spl_optee.S
>>>>>> +++ b/common/spl/spl_optee.S
>>>>>> @@ -8,5 +8,8 @@
>>>>>>
>>>>>> ENTRY(spl_optee_entry)
>>>>>> 	ldr lr, =CONFIG_SYS_TEXT_BASE
>>>>>> +#ifdef CONFIG_ARCH_ROCKCHIP
>>>>> Can we make this selectable based on a dedicated config-option?  We provide our
>>>>> own OPTEE port for some of our modules and I would like to have this as an opt-in
>>>>> or opt-out option in Kconfig.
>>>> I think you are using OPTEE for RK3368/RK3399, right? Then the use case
>>>> is different, you are using OPTEE as bl32 for armv8, and this spl_optee is
>>>> for armv7 only.
>>>> I'm OK to add a Kconfig option if you really have different usage,
>>>> and I think this patch does not break things because the no one use 'r1'
>>>> now.
>>> rk3229 has support in upstream op-tee, so possibly the calling convention
>>> is different with that? And of course there may come a time when people
>>> may want to use upstream-op-tee instead of a binary-blob.
>> Upstream op-tee is using 'lr' and rockchip binary is now using 'r1' for
>> u-boot entry.
>> So upstream op-tee can be boot if my patch series for rk3229 is merged,
>> and with this patch, people can chose to use rockchip binary for rk3229
>> or other
>> armv7 SoCs.
> Yes, an that is the reason there should be a Kconfig symbol for that :-)

I don't think we need the Kconfig symbol now because I think user don't have
to make this choice in SPL, user only need to use the firmware they need.
With this patch, both upstream and Rockchip OP-TEE can be supported,
it does not break support for upstream op-tee.
The Kconfig symbol is needed if only one choice is available, isn't it?
>
>
>> eg. I'll add op-tee support for rk3288 later because of the psci needed
>> by mainline kernel.
> please keep in mind that old device-trees in mainline need to stay working
> aka you cannot expect people to update their firmware just to keep using
> mainline kernels. So u-boot should modify the kernel-devicetree accordingly
> to enable psci, before starting the kernel.

Well, I think the U-Boot should support op-tee first, and then kernel can
decide to use PSCI or not, user can still use old style if they don't
want a psci.
But if U-Boot don't support op-tee, then kernel is not able to use PSCI.

Thanks,
- Kever
>
> In my ATF adventure I already did some smallish adaptions on the kernel
> side for this:
> https://github.com/mmind/linux-rockchip/commit/27bce39800a14abe08d5a5994abef29f9b543c68
>
>
> Heiko
>
>
>

[U-Boot] [PATCH] optee: support rockchip optee binary release

[Heiko Stübner]

Hi Kever,

Am Montag, 1. April 2019, 11:49:35 CEST schrieb Kever Yang:
> On 04/01/2019 01:42 PM, Heiko Stübner wrote:
> > Am Montag, 1. April 2019, 04:32:28 CEST schrieb Kever Yang:
> >> On 04/01/2019 05:02 AM, Heiko Stübner wrote:
> >>> Am Freitag, 29. März 2019, 13:16:26 CET schrieb Kever Yang:
> >>>> On 03/29/2019 07:25 PM, Philipp Tomsich wrote:
> >>>>>> On 29.03.2019, at 12:21, Kever Yang <kever.yang@rock-chips.com> wrote:
> >>>>>>
> >>>>>> Rockchip provide tee binary release in 'rkbin' repository:
> >>>>>> https://github.com/rockchip-linux/rkbin
> >>>>>> For some historical reason, rockchip optee binary is using
> >>>>>> 'r1' instead of 'lr' as U-Boot entry.
> >>>>>>
> >>>>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> >>>>>> ---
> >>>>>>
> >>>>>> common/spl/spl_optee.S | 3 +++
> >>>>>> 1 file changed, 3 insertions(+)
> >>>>>>
> >>>>>> diff --git a/common/spl/spl_optee.S b/common/spl/spl_optee.S
> >>>>>> index 8bd1949ddf..092307b3cc 100644
> >>>>>> --- a/common/spl/spl_optee.S
> >>>>>> +++ b/common/spl/spl_optee.S
> >>>>>> @@ -8,5 +8,8 @@
> >>>>>>
> >>>>>> ENTRY(spl_optee_entry)
> >>>>>> 	ldr lr, =CONFIG_SYS_TEXT_BASE
> >>>>>> +#ifdef CONFIG_ARCH_ROCKCHIP
> >>>>> Can we make this selectable based on a dedicated config-option?  We provide our
> >>>>> own OPTEE port for some of our modules and I would like to have this as an opt-in
> >>>>> or opt-out option in Kconfig.
> >>>> I think you are using OPTEE for RK3368/RK3399, right? Then the use case
> >>>> is different, you are using OPTEE as bl32 for armv8, and this spl_optee is
> >>>> for armv7 only.
> >>>> I'm OK to add a Kconfig option if you really have different usage,
> >>>> and I think this patch does not break things because the no one use 'r1'
> >>>> now.
> >>> rk3229 has support in upstream op-tee, so possibly the calling convention
> >>> is different with that? And of course there may come a time when people
> >>> may want to use upstream-op-tee instead of a binary-blob.
> >> Upstream op-tee is using 'lr' and rockchip binary is now using 'r1' for
> >> u-boot entry.
> >> So upstream op-tee can be boot if my patch series for rk3229 is merged,
> >> and with this patch, people can chose to use rockchip binary for rk3229
> >> or other
> >> armv7 SoCs.
> > Yes, an that is the reason there should be a Kconfig symbol for that :-)
> 
> I don't think we need the Kconfig symbol now because I think user don't have
> to make this choice in SPL, user only need to use the firmware they need.
> With this patch, both upstream and Rockchip OP-TEE can be supported,
> it does not break support for upstream op-tee.
> The Kconfig symbol is needed if only one choice is available, isn't it?

In a related spl-atf change from last week, I saw today, I also saw the
comment to not mix architecture hacks with common code, so I guess
having a "ifdef ROCKCHIP" in common applies here.

From what I understood right now, all Rockchip vendor OP-Tees use r1,
so at least on rk3229 there is already a choice to use either upstream
op-tee build from source or the binary rockchip one.


> >> eg. I'll add op-tee support for rk3288 later because of the psci needed
> >> by mainline kernel.
> > please keep in mind that old device-trees in mainline need to stay working
> > aka you cannot expect people to update their firmware just to keep using
> > mainline kernels. So u-boot should modify the kernel-devicetree accordingly
> > to enable psci, before starting the kernel.
> 
> Well, I think the U-Boot should support op-tee first, and then kernel can
> decide to use PSCI or not, user can still use old style if they don't
> want a psci.
> But if U-Boot don't support op-tee, then kernel is not able to use PSCI.

of course ... this was not meant to be criticism of any sort ;-)

Just a reminder, before somebody starts posting kernel patches converting
the in-kernel devicetree to only psci.


Heiko