public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Tom Rini <trini@konsulko.com>
To: Michal Simek <michal.simek@xilinx.com>
Cc: "Andre Przywara" <andre.przywara@arm.com>,
	u-boot@lists.denx.de, git@xilinx.com,
	"Bin Meng" <bmeng.cn@gmail.com>,
	"Chee Hong Ang" <chee.hong.ang@intel.com>,
	"Marek Behún" <marek.behun@nic.cz>,
	"Pali Rohár" <pali@kernel.org>, "Simon Glass" <sjg@chromium.org>
Subject: Re: [PATCH 07/10] Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems
Date: Thu, 19 Aug 2021 12:44:16 -0400	[thread overview]
Message-ID: <20210819164416.GA858@bill-the-cat> (raw)
In-Reply-To: <e9e13250-64ee-3472-2672-003ca87acdf1@xilinx.com>

[-- Attachment #1: Type: text/plain, Size: 2684 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

  reply	other threads:[~2021-08-19 16:44 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-19 11:19 [PATCH 00/10] xilinx: Add support for DTB reselection Michal Simek
2021-08-19 11:19 ` [PATCH 01/10] xilinx: fru: Replace spaces with \0 in detected name Michal Simek
2021-08-19 11:19 ` [PATCH 02/10] xilinx: Use variable for passing board_name Michal Simek
2021-08-19 11:19 ` [PATCH 03/10] xilinx: common: Change board_info[] handling Michal Simek
2021-08-19 11:19 ` [PATCH 04/10] xilinx: common: Free allocated structure Michal Simek
2021-08-19 11:19 ` [PATCH 05/10] xilinx: Add support for generic board detection Michal Simek
2021-08-19 11:19 ` [PATCH 06/10] xilinx: zynqmp: Check that DT is 64bit aligned Michal Simek
2021-08-19 11:19 ` [PATCH 07/10] Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems Michal Simek
2021-08-19 15:56   ` Andre Przywara
2021-08-19 16:01     ` Michal Simek
2021-08-19 16:18       ` Tom Rini
2021-08-19 16:31         ` Michal Simek
2021-08-19 16:44           ` Tom Rini [this message]
2021-08-19 11:19 ` [PATCH 08/10] arm64: dts: Make sure that all DTBs are 64bit aligned " Michal Simek
2021-08-19 16:10   ` Andre Przywara
2021-08-19 16:34     ` Michal Simek
2021-08-19 11:19 ` [PATCH 09/10] xilinx: zynqmp: Generate different u-boot.itb for MULTI_DTB_FIT Michal Simek
2021-08-19 11:19 ` [PATCH 10/10] xilinx: common: Enabling generic function for DT reselection Michal Simek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210819164416.GA858@bill-the-cat \
    --to=trini@konsulko.com \
    --cc=andre.przywara@arm.com \
    --cc=bmeng.cn@gmail.com \
    --cc=chee.hong.ang@intel.com \
    --cc=git@xilinx.com \
    --cc=marek.behun@nic.cz \
    --cc=michal.simek@xilinx.com \
    --cc=pali@kernel.org \
    --cc=sjg@chromium.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox