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. -- Tom
signature.asc
Description: PGP signature
