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

[Simon Glass]

Hi Rasmus,

On Fri, 29 Sept 2023 at 07:16, Sean Anderson <seanga2 at gmail.com> wrote:
>
> 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.

Are you planning a new version of this series?

Regards,
Simon