[U-Boot] [PATCH v2] spl: add support to booting with ATF

[Simon Glass]

Hi,

On 28 March 2017 at 08:37, Andre Przywara <andre.przywara at arm.com> wrote:
> Hi,
>
> On 28/03/17 15:16, Dan Handley wrote:
>> Hi Kever
>>
>>> -----Original Message-----
>>> From: Kever Yang [mailto:kever.yang at rock-chips.com]
>>> Sent: 28 March 2017 08:23
>>>
>>> Hi Andre,
>>>
>>>
>>> On 03/23/2017 05:22 PM, Andre Przywara wrote:
>>> > Hi Kever,
>>> >
>>> > I was wondering if we really need to copy in all those ATF definitions.
>>> > I think this is really an *interface* between the loader (SPL or BL2)
>>> > and the runtime services (BL31), so it's supposed to be stable and we
>>> > wouldn't need to pull in all those headers.
>>>
>>> Yes, you are right, this is an interface between the loader and bl31,
>>> and it's from ATF, just like android image header if from Android
>>> project, so my idea copy the header definition.
>>> >
>>> > So given that, can't we simply model the data structure, which at the
>>> > end of the day is just a bunch of simple data types:
>>>
>>> I'm sorry, I don't get your point here, what's the benefit for us to re-
>>> define the header data types?
>>> >
>> I guess Andre's point was to copy just the essential data structures,
>> not all the other baggage in that header file.
>
> Yes, that what my intention.
> I just wanted to avoid pasting this pretty big and somewhat foreign code
> into U-Boot, when all we need is four simple data structures.
>
>>> >
>>> > Whenever this gets changed we are in trouble anyway (because we break
>>> > compatibility).
>>>
>>> Yes, we hope it won't change, but it depends on ATF not U-Boot, no
>>> matter what kind if data type we using in U-Boot, we can not stop this,
>>> it's the ATF break the compatibility, not U-Boot SPL.
>
> But as mentioned below, ATF can handle multiple versions, so it can keep
> compatibility.
>
>>> > So what are the opinions on this? Pull in the ATF source or define our
>>> > own and mark it as an ATF copy?
>>>
>>> ATF is the one using all these header structure, if they change there
>>> API and break, yes, we need/have to sync again.
>>>
>>> Dan in CC is ATF maintainer.
>>>
>>> Hi Dan, are you able to provide some information for the header
>>> structure, for example, is it suppose to change/update in the future?
>>>
>> The ARM TF BL31 interface is defined here:
>> https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/firmware-design.md#required-cpu-state-when-calling-bl31_entrypoint-during-cold-boot
>>
>> In summary, the arguments passed to BL31 in X0 and X1 are platform
>> specific and not used by generic BL31 code. Conventionally, X0 has been
>> used by TF BL2 to pass a bl31_params structure, which is used by ARM
>> Limited platforms and possibly others but not necessarily all. This
>> structure has now been superseded by a new structure called bl_params,
>> which caters for AArch32 platforms and should be more futureproof. Both
>> bl31_params and bl_params are prefixed with metadata in a param_header,
>> which includes a version number. BL31 platform code could potentially
>> handle both bl31_params and bl_params by checking the version number but
>> ARM platforms expect one or the other depending on the value of a build
>> flag (LOAD_IMAGE_V2). It's hard to say what other platforms do but I
>> guess a lot of them use bl31_params.
>
> Yes, so with the versioning scheme I don't see many problems. A really
> versatile BL31 could handle both, so would be compatible.
> At the end of the day I think this is a theoretical issue, as you
> usually package ATF, SPL and U-Boot together and can make sure that they
> match. It's not like people insert their own bl31 into an existing
> image. And with everything being open source you can always rebuild anyway.
>
>> I suggest that U-Boot SPL mandates that platforms that wish to use TF
>> BL31 with U-Boot SPL must use the bl_params structure. It may also need
>> to mandate that this contains BL33 information specific to U-Boot.
>
> That sounds like a good idea.
>
>> Alternatively, U-Boot SPL could define its own structure(s) according to
>> its needs, and mandate that BL31 platforms it wishes to support conform
>> to this.
>
> Philipp had the idea of using a small device tree blob to convey
> information between the SPL (or any other loader, really) and BL31,
> similar to what the FIT does. In fact it could even be the FIT image. So
> the bl_params structure would just contain a pointer to that DT blob in
> memory, and bl31 then parses this to get as many information as it needs
> from the loader, without affecting the actual SPL->BL31 interface. Yes,
> that is another level of indirection, but I think worth to explore - at
> least for the future. This would for instance allow the loader to pass
> the address of the device DT to bl31, which could then make use of it -
> either for getting information or for inserting nodes (PSCI or other
> firmware services) - or for both.

Perhaps this could be future work? For now, I suggest we take this
patch once the header file has been cut down.

Regards,
Simon