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

