[U-Boot] [PATCH v2] mkimage: fit: spl: Add an optional static offset for external data

[Simon Glass]

Hi Teddy,

On 9 June 2016 at 19:02, Teddy Reed <teddy.reed at gmail.com> wrote:
> Thanks for the review Simon!
>
> On Thu, Jun 9, 2016 at 6:03 PM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Teddy,
>>
>> On 5 June 2016 at 15:18, Teddy Reed <teddy.reed at gmail.com> wrote:
>>>
>>> When building a FIT with external data (-E), an SPL may require absolute
>>> positioning for executing the external firmware. To acheive this use the
>>> (-p) switch, which will replace the amended 'data-offset' with 'data-position'
>>> indicating the absolute position of external data.
>>
>> Why does SPL require that? Is it because it is at an external address,
>> not inside the FIT?
>>
>
> Ah, it's more so the U-Boot proper that expects to have been built
> with a TEXT_BASE known to it and the SPL. I'm trying to accommodate a
> scenario where U-Boot proper executes after being verified by the SPL,
> without needing arguments or relocation.
>
> Given an execute-in-place without arguments U-Boot proper must know
> TEXT_BASE at build time. This is clobbered when the external FIT is
> generated, if the firmware is placed at a relative position. With a -p
> your U_BOOT_TEXT_BASE remains constant between build and external FIT
> generation.

I'm stil not quite clear on this.

SPL has to know where to load U-Boot. The existing FIT support uses
CONFIG_SYS_TEXT_BASE for this. spl_load_simple_fit() should load
U-Boot proper to that address. The position of U-Boot in the FIT
itself should not be relevant.

What sort of relocation are you trrying to avoid?


>
>>>
>>> It is considered an error if the requested absolute position overlaps with the
>>> initial data required for the compact FIT.
>>>
>>> Cc: Simon Glass <sjg at chromium.org>
>>> ---
>>>
>>> Changes in v2:
>>> - Add -p argument to mkimage.1
>>> - Add -E, -p arguments to mkimage usage text
>>> - Add notes for static position within uImage.FIT docs
>>> - Rebased onto master
>>>
>>>  doc/mkimage.1                         |  6 ++++++
>>>  doc/uImage.FIT/source_file_format.txt |  4 ++++
>>>  tools/fit_image.c                     | 19 ++++++++++++++++++-
>>>  tools/imagetool.h                     |  1 +
>>>  tools/mkimage.c                       | 13 +++++++++++--
>>>  5 files changed, 40 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/doc/mkimage.1 b/doc/mkimage.1
>>> index 4b3a255..ffa7d60 100644
>>> --- a/doc/mkimage.1
>>> +++ b/doc/mkimage.1
>>> @@ -152,6 +152,12 @@ verification. Typically the file here is the device tree binary used by
>>>  CONFIG_OF_CONTROL in U-Boot.
>>>
>>>  .TP
>>> +.BI "\-p [" "external position" "]"
>>> +Place external data at a static external position. See \-E. Instead of writing
>>> +a 'data-offset' property defining the offset from the end of the FIT, \-p will
>>> +use 'data-position' as the absolute position from the base of the FIT.
>>> +
>>> +.TP
>>>  .BI "\-r
>>>  Specifies that keys used to sign the FIT are required. This means that they
>>>  must be verified for the image to boot. Without this option, the verification
>>> diff --git a/doc/uImage.FIT/source_file_format.txt b/doc/uImage.FIT/source_file_format.txt
>>> index 3f54180..ee72740 100644
>>> --- a/doc/uImage.FIT/source_file_format.txt
>>> +++ b/doc/uImage.FIT/source_file_format.txt
>>> @@ -282,6 +282,10 @@ In this case the 'data' property is omitted. Instead you can use:
>>>      aligned to a 4-byte boundary.
>>>    - data-size : size of the data in bytes
>>>
>>> +The 'data-offset' property can be substituted with 'data-position', which
>>> +defines a static position or address from the base of the FIT as the offset.
>>> +A static position is helpful when booting U-Boot proper before performing
>>> +relocation.
>>
>> This confuses me since it mentions a static address, but then
>> references the base of the FIT, suggesting it is relative.
>>
>> Can you use the word 'absolute' instead of 'static'?
>>
>
> Absolutely! ;)

Regards,
Simon