On Wed, Jan 28, 2026 at 05:02:06PM +0100, Marek Vasut wrote:
> On 1/28/26 4:50 PM, Tom Rini wrote:
> > On Wed, Jan 28, 2026 at 04:42:47PM +0100, Marek Vasut wrote:
> > > On 1/28/26 4:26 PM, Padhi, Beleswar wrote:
> > > > 
> > > > On 1/28/2026 8:26 PM, Marek Vasut wrote:
> > > > > On 1/28/26 3:18 PM, Beleswar Padhi wrote:
> > > > > > From: Marek Vasut <[email protected]>
> > > > > 
> > > > > Please drop this , it isnt needed.
> > > > > 
> > > > > > When CONFIG_SPL_MULTI_DTB_FIT is enabled, multiple device trees are
> > > > > > packed inside the multidtb.fit FIT image. While the individual DTBs
> > > > > > and the FIT image start address are 8-byte aligned, the DTBs 
> > > > > > embedded
> > > > > > within the FIT image are not guaranteed to maintain 8-byte 
> > > > > > alignment.
> > > > > > 
> > > > > > This misalignment causes -FDT_ERR_ALIGNMENT failure in
> > > > > > setup_multi_dtb_fit() when locating the next available DTB within 
> > > > > > the
> > > > > > FIT blob and setting gd->fdt_blob, because of the recent libfdt
> > > > > > hardening since commit 0535e46d55d7 ("scripts/dtc: Update to 
> > > > > > upstream
> > > > > > version v1.7.2-35-g52f07dcca47c")
> > > > > > 
> > > > > > To fix this, check the image type when extracting
> > > > > 
> > > > > "extracting" ? This code changes mkimage, so not "extracting" but
> > > > > "packing" (into fitImage), right ?
> > > > 
> > > > 
> > > > Well the function name is fit_extract_data(). It is documented as
> > > > extracting
> > > > all the data properties of a node to the end of the fit. So just keeping
> > > > the
> > > > same terminology. Its obviously not in the same context as extracting 
> > > > data
> > > > out of a FIT image to consume information...
> > > 
> > > ... check the image type when moving image data at the end of the tree ...
> > > or something like that ?
> > > 
> > > > > > FIT image data and
> > > > > > set the alignment size to 8 bytes (if not already) only for flat_dt
> > > > > > images. This ensures correct alignment for device tree blobs as per 
> > > > > > the
> > > > > > DT spec, while also allowing different alignment sizes for other 
> > > > > > image
> > > > > > types within the FIT.
> > > > > 
> > > > > [...]
> > > > > 
> > > > > > diff --git a/tools/fit_image.c b/tools/fit_image.c
> > > > > > index e865f65a400..f842c845771 100644
> > > > > > --- a/tools/fit_image.c
> > > > > > +++ b/tools/fit_image.c
> > > > > > @@ -682,9 +682,17 @@ static int fit_extract_data(struct
> > > > > > image_tool_params *params, const char *fname)
> > > > > >        for (node = fdt_first_subnode(fdt, images);
> > > > > >             node >= 0;
> > > > > >             node = fdt_next_subnode(fdt, node)) {
> > > > > > -        const char *data;
> > > > > > +        const char *data, *type;
> > > > > >            int len;
> > > > > >    +        /* Fallback to 8-byte alignment for DTBs if unaligned */
> > > > > > +        type = fdt_getprop(fdt, node, FIT_TYPE_PROP, &len);
> > > > > > +        if (type &&
> > > > > > +            len == sizeof("flat_dt") &&
> > > > > > +            !memcmp(type, "flat_dt", len) &&
> > > > > > +            align_size & 0x7)
> > > > > 
> > > > > What will be the resulting alignment if align_size = 0x1f ? 8 right
> > > > > ? I think it should be 0x20 .
> > > > 
> > > > 
> > > > I thought we agreed on resetting align_size to 8 if its not already 
> > > > aligned
> > > > (regardless if < or > 8) in v3 version of this patch[0]. Do we really
> > > > have to
> > > > align the align_size var itself? It seems like a overkill to me...
> > > Maybe simply call round_up(8) on the align_size , to align to the next
> > > 8-byte aligned offset , but somewhat respect user wishes ?
> > > 
> > > But looking at this code one more time , look at the calloc() in this
> > > function, I think you might also have to allocate a bit more memory to
> > > really hold all the newly aligned DTs, right ?
> > > 
> > > Also, don't you need to align the buf_ptr as well ? Consider a scenario
> > > where the fitImage contains two images, one ends at 4-byte aligned 
> > > address,
> > > followed by a DT. The users passes -B 4 to mkimage, and I think buf_ptr
> > > would then be 4-byte aligned, so will the DT, no ?
> > 
> > I'm confused. Where did we go from "default should be 8 not 4, for
> > device trees" to "take what the user passed, make sure is 8b aligned" ?
> > If the user tells us to do something, we should do it, and they can
> > suffer the potential consequences. If the user doesn't tell us to do
> > something, we should pick a reasonable default.
> You can remove the "the user passes -B 4" part from my example, keep the
> rest, the DT will likely end up 4-byte aligned, no ?

And 4-byte aligned is not a reasonable default.

-- 
Tom

Attachment: signature.asc
Description: PGP signature

Reply via email to