[PATCH 1/4] mkimage: also honour -B even without external data

[Sean Anderson]

On 9/28/23 03:10, Rasmus Villemoes wrote:
> On 27/09/2023 21.02, Sean Anderson wrote:
>> On 9/19/23 07:37, Rasmus Villemoes wrote:
>>> In some cases, using the "external data" feature is impossible or
>>> undesirable, but one may still want (or need) the FIT image to have a
>>> certain alignment. Also, given the current 'mkimage -h' output,
>>>
>>>     -B => align size in hex for FIT structure and header
>>>
>>> it is quite unexpected for -B to be effectively ignored without -E.
>>
>> FWIW, this behavior is documented in doc/mkimage.1 (which should also be
>> updated if this behavior is implemented):
>>
>> | The alignment, in hexadecimal, that external data will be aligned to.
>> | This option only has an effect when -E is specified.
> 
> I'll send a follow-up to fix that, thanks.
> 
>> And, for additional context, the documentation for -E is
>>
>> | After processing, move the image data outside the FIT and store a data
>> | offset in the FIT. Images will be placed one after the other
>> | immediately after the FIT, with each one aligned to a 4-byte boundary.
>> | The existing ‘data’ property in each image will be replaced with
>> | ‘data-offset’ and ‘data-size’ properties. A ‘data-offset’ of 0
>> | indicates that it starts in the first (4-byte-aligned) byte after the
>> | FIT.
>>
>> Based on this documentation and my understanding of the code as-is, -B
>> controls the alignment of the images themselves,
> 
> Yes, when -E is in effect. My patch does not affect the case where -E is
> present.

Wherever possible, flags should be orthogonal. That is, they should have the
same effect (or at least the same spirit) regardless of the presence of other
flags. This matches their UX, as orthogonally selectable options.

>> not the size multiple of the FIT.
> 
> It is _also_ that, but mostly as a "necessary side effect" of getting
> the first image aligned. In order to achieve the desired alignment of
> the first external image, the FIT structure itself is aligned to the -B
> value, and each image's size similarly aligned up to that value so the
> next image ends on a -B multiple again. So with both -E and -B, the FIT
> structure itself is indeed also padded to the -B value, as the `mkimage
> -h` output suggests.

The intent of -B is to align images. That the FIT itself is padded is an
implementation detail.

> What I want, and expected from `mkimage -h`, is to make the whole FIT
> have a certain size multiple, without using -E, so in that case the
> alignment of the FIT is the primary goal.

Why does mkimage have to do this? Can't you just use truncate or, in a
binman context, align-size?

>> However, from what I can tell, this patch does not actually
>> affect the alignment of the images,
> 
> No, because the images are (still) embedded within the FIT as ordinary
> values of the data= properties, and it's a basic property of the DTB
> format that those are always 4-byte aligned, and you can't (easily)
> change that [1]. Which, I suppose, is one of the reasons U-Boot invented
> the "external data" mechanism - so for example the .dtbs embedded in the
> FIT can all be guaranteed to be on an 8-byte boundary, and thus the
> selected one can be used in-place when passed to linux.
> 
>> but rather adjusts the size of the
>> overall FIT to a certain alignment. I find this rather unexpected.
> 
> Well, as I said, _I_ was surprised by -B having no effect based on the
> `mkimage -h` output, so the two sources of documentation were not in
> sync. The man page was/is correct wrt. the actual code.

The mkimage -h putput is too terse to record the complete behavior of
each option. Perhaps we should add a warning when -B is specified without
-E.

> Rasmus
> 
> [1] There's a nop tag one can insert, and I think I saw somebody
> actually suggesting to do that to align the embedded data= properties,
> but it's quite error-prone and inflexible, as any subsequent
> modification of the .dtb before that property will ruin the alignment.

This would indeed be the way to go. I don't think we should worry about
further modification of the fdt, as this could also ruin the alignment
of external images.

--Sean