On Thu, Aug 19, 2021 at 06:31:25PM +0200, Michal Simek wrote:
> 
> 
> On 8/19/21 6:18 PM, Tom Rini wrote:
> > On Thu, Aug 19, 2021 at 06:01:39PM +0200, Michal Simek wrote:
> >> Hi Andre,
> >>
> >> On 8/19/21 5:56 PM, Andre Przywara wrote:
> >>> On 8/19/21 12:19 PM, Michal Simek wrote:
> >>>
> >>> Hi,
> >>>
> >>>> Enabling MULTI_DTB_FIT and DTB_RESELECT can end up with multi DTBs in FIT
> >>>> image placed and aligned only by 32bits (4bytes). For 64bit systems there
> >>>> is 64bit (8bytes) alignment required. That's why make sure that
> >>>> fit-dtb.blob and u-boot.itb as our primary target images for Xilinx
> >>>> ZynqMP
> >>>> are all 64bit aligned. The patch is using CONFIG_PHYS_64BIT macro to
> >>>> identify 64bit systems (including 32bit systems with PAE).
> >>>>
> >>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >>>> ---
> >>>>
> >>>>   Makefile | 7 +++++++
> >>>>   1 file changed, 7 insertions(+)
> >>>>
> >>>> diff --git a/Makefile b/Makefile
> >>>> index 269e353a28ad..1bbe95595efe 100644
> >>>> --- a/Makefile
> >>>> +++ b/Makefile
> >>>> @@ -1169,6 +1169,10 @@ MKIMAGEFLAGS_fit-dtb.blob = -f auto -A $(ARCH)
> >>>> -T firmware -C none -O u-boot \
> >>>>       -a 0 -e 0 -E \
> >>>>       $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst
> >>>> ",,$(CONFIG_OF_LIST))) -d /dev/null
> >>>>   +ifeq ($(CONFIG_PHYS_64BIT),y)
> >>>
> >>> Why is this restricted to 64-bit "systems"? The DT spec[1] clearly
> >>> states that some DT parts (/memreserved/ block, for instance), must be
> >>> 64-bit aligned, which means the whole blobs needs to be 64-bit aligned.
> >>> Granted this probably does not cause real issues on 32-bit systems, but
> >>> is violating the spec anyway.
> >>> So I'd say we add the alignment requirement unconditionally.
> >>
> >> That's even better for me and we need to make sure that dtbs itself are
> >> aligned and also dtbs inside FIT image are aligned too.
> > 
> > Right, all dtbs need to be 8 byte aligned to start with.  Enforcing this
> > is what will unblock moving to a newer libfdt where that's checked for
> > up-front now.
> 
> As is in the second thread. Does it make sense to also align the end?
> I did that in 8/10 patch.
> The problem with these alignments is that you also need to align the
> start of FIT image. Maybe would the best to copy fdt to different
> aligned location.

Right, so a good question.  I have suggested before that we stop
assuming that we (U-Boot SPL, etc) can use the device tree in-place and
instead move it somewhere aligned.  I'm not sure off-hand what ends up
being best, someone would need to investigate a bit.

-- 
Tom