From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 209BBC07E95 for ; Tue, 13 Jul 2021 21:11:36 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 55B0F60FE9 for ; Tue, 13 Jul 2021 21:11:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 55B0F60FE9 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 821F482024; Tue, 13 Jul 2021 23:11:33 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="jM4Tn9UI"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 42AA982956; Tue, 13 Jul 2021 23:11:32 +0200 (CEST) Received: from mail-qk1-x72d.google.com (mail-qk1-x72d.google.com [IPv6:2607:f8b0:4864:20::72d]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 79C4680F27 for ; Tue, 13 Jul 2021 23:11:27 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-qk1-x72d.google.com with SMTP id e14so23118016qkl.9 for ; Tue, 13 Jul 2021 14:11:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Bv0/Q11HWBFY6wNm6Yg8u3IH1GZz5ZqjBxQFzD8wk3U=; b=jM4Tn9UIXotPhVE5sd88jyJbQU6/NUuFGsmO8Vpm2iOS9XlU2of8B0v3F3A9mQYEMD f9rO/2Lt8zar8NaHLxv6+uVvtbvd9uVt0lFui4XDojCzDIM+v4eQOmt4EM0eTICIDlf9 oskx8yYcwD5KBzGdfDTbpqQr2SJ/qUq1omNEM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Bv0/Q11HWBFY6wNm6Yg8u3IH1GZz5ZqjBxQFzD8wk3U=; b=RXB3hcBZEKE/hu6/Uw8rsSDEtnGMCDVIBsHW4kBMSuGsypYK1Nich7UXUP/+ZyjrZr XD8CgSxZup+kG9rQMiRmzuihqpmcMOux3IWVeuoLre84gvZpzDtsakC+QkiyHWcIkk+Q fBv9dC91K3z7nU9JsqiMzA/izS9pjcLwRQiwF8qZb0bigX/Ng7eYJ3GC14Ttm3DbavT1 uBsOJNRRR37I3hl+shvdcsaOC1rAHep1VwuIyo/3m+6cIG8VaENoVSSI3bfHccOztGKk bE6TNdgR+W3heSiQ8XSR5RPy1t6PT/aB3nHRj5nzZ9sW0wrHMj53DQ7BLkboEgD7+Fm4 ZlhA== X-Gm-Message-State: AOAM531ky19unvhYqkRAoW9/QOoM2FtyqGEAMGDPQqpTfP8j2o8iwF8j wiJaJuT26YgPcginG037PNN8HA== X-Google-Smtp-Source: ABdhPJxp4NDZkH8/8iAuqSqblsnrDp+XpdOQZGIzWrGfLvKgzr5i0YJ7w9lIST/LJDc/5WmuGdVbGg== X-Received: by 2002:a37:9a0a:: with SMTP id c10mr3993651qke.86.1626210685977; Tue, 13 Jul 2021 14:11:25 -0700 (PDT) Received: from bill-the-cat (2603-6081-7b01-cbda-4cf5-6df7-4b7d-752e.res6.spectrum.com. [2603:6081:7b01:cbda:4cf5:6df7:4b7d:752e]) by smtp.gmail.com with ESMTPSA id x125sm50859qkd.8.2021.07.13.14.11.23 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Tue, 13 Jul 2021 14:11:24 -0700 (PDT) Date: Tue, 13 Jul 2021 17:11:22 -0400 From: Tom Rini To: Marek Vasut Cc: Simon Glass , "Alex G." , Bin Meng , Reuben Dowle , "u-boot@lists.denx.de" , Wolfgang Denk Subject: Re: [PATCH] spl: Align device tree blob address at 8-byte boundary Message-ID: <20210713211122.GO9516@bill-the-cat> References: <20210712151510.GT9516@bill-the-cat> <300cbb2d-a343-aff8-2c73-00a81ec05af3@gmail.com> <20210713134703.GF9516@bill-the-cat> <940b2a89-c332-c534-102e-5fa25b5b14b4@denx.de> <20210713144151.GH9516@bill-the-cat> <357b8f2e-4aa5-acf7-96aa-a4d0f7d1fbde@denx.de> <20210713181105.GK9516@bill-the-cat> <14e093f8-4357-1fd8-5875-6afc038d672e@denx.de> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ddJkwolKDQNViLug" Content-Disposition: inline In-Reply-To: <14e093f8-4357-1fd8-5875-6afc038d672e@denx.de> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.9.4 (2018-02-28) X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean --ddJkwolKDQNViLug Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jul 13, 2021 at 10:35:03PM +0200, Marek Vasut wrote: > On 7/13/21 8:11 PM, Tom Rini wrote: > > On Tue, Jul 13, 2021 at 07:50:49PM +0200, Marek Vasut wrote: > > > On 7/13/21 6:47 PM, Simon Glass wrote: > > > > Hi Marek, > > > >=20 > > > > On Tue, 13 Jul 2021 at 08:53, Marek Vasut wrote: > > > > >=20 > > > > > On 7/13/21 4:41 PM, Tom Rini wrote: > > > > > > On Tue, Jul 13, 2021 at 04:35:38PM +0200, Marek Vasut wrote: > > > > > > > On 7/13/21 3:47 PM, Tom Rini wrote: > > > > > > > > On Mon, Jul 12, 2021 at 11:01:24AM -0500, Alex G. wrote: > > > > > > > > > On 7/12/21 10:15 AM, Tom Rini wrote: > > > > > > > > > > On Mon, Jul 12, 2021 at 01:36:14PM +0800, Bin Meng wrot= e: > > > > > > > > > > > On Mon, Jul 12, 2021 at 1:21 PM Reuben Dowle wrote: > > > > > > > > > > > >=20 > > > > > > > > > > > > I submitted an almost identical patch. See https://= github.com/u-boot/u-boot/commit/eb39d8ba5f0d1468b01b89a2a464d18612d3ea76 > > > > > > > > > > > >=20 > > > > > > > > > > > > This patch eventually had to be reverted (https://g= ithub.com/u-boot/u-boot/commit/5675ed7cb645f5ec13958726992daeeed16fd114), b= ecause it was causing issues on some platforms that had FIT on 32 bit bound= ary. However I continue to use it in production code, as without it the boo= t on my platform aborts. > > > > > > > > > > > >=20 > > > > > > > > > > > > I don't have time to investigate why this was happe= ning, but you need to check this code won't just cause exactly the same fau= lts. > > > > > > > > > > >=20 > > > > > > > > > > > Thanks for your information. > > > > > > > > > > >=20 > > > > > > > > > > > +Marek who did the revert > > > > > > > > > > >=20 > > > > > > > > > > > The revert commit message says: > > > > > > > > > > >=20 > > > > > > > > > > > "The commit breaks booting of fitImage by SP= L, the system simply > > > > > > > > > > > hangs. This is because on arm32, the fitImage and all= of its content > > > > > > > > > > > can be aligned to 4 bytes and U-Boot expects just tha= t." > > > > > > > > > > >=20 > > > > > > > > > > > I don't understand this. If an address is aligned to = 8, it is already > > > > > > > > > > > aligned to 4, so how did this commit make the system = hang on arm32? > > > > > > > > > >=20 > > > > > > > > > > I think this had something to do with embedding content= s somewhere in > > > > > > > > > > the image? There is a thread on the ML from then but I= don't know how > > > > > > > > > > informative it will end up being. > > > > > > > > >=20 > > > > > > > > > It's true that the flat devicetree spec requires an 8-byt= e alignment, even > > > > > > > > > on 32-bit. The issues here are specific to u-boot. > > > > > > > > >=20 > > > > > > > > > SPL and u-boot have to agree where u-boot's FDT is locate= d. We'll look at > > > > > > > > > two cases: > > > > > > > > > 1) u-boot as a FIT (binary and FDT separately loaded) > > > > > > > > > 2) u-boot with embedded FDT > > > > > > > > >=20 > > > > > > > > > In case (1) SPL must place the FDT at a location where u-= boot will find it. > > > > > > > > > The current logic is > > > > > > > > > SPL: fdt =3D ALIGN_4(u_boot + u_boot_size) > > > > > > > > > u-boot: fdt =3D ALIGN_4(u_boot + u_boot_size) > > > > > > > > >=20 > > > > > > > > > In case (2), SPL's view of the FDT is not relevant, but i= nstead the build > > > > > > > > > system must place the FDT correctly: > > > > > > > > > build: fdt >> u-boot.bin > > > > > > > > > u-boot: fdt =3D ALIGN_4(u_boot + u_boot_size) > > > > > > > > >=20 > > > > > > > > > We have 3 places that must agree. A correct and complete = patch could change > > > > > > > > > all three, but one has to consider compatibility issues w= hen crossing u-boot > > > > > > > > > and SPL versions. > > > > > > > > >=20 > > > > > > > > > I had proposed in the revert discussion that SPL use r2 o= r similar mechanism > > > > > > > > > to pass the location of the FDT to u-boot. > > > > > > > >=20 > > > > > > > > I'm not sure that we need to worry too much about mix-and-m= atch > > > > > > > > SPL/U-Boot, but documenting what to go change if you must d= o it > > > > > > > > somewhere under doc/ would be good. I think we can just sw= itch to > > > > > > > > ALIGN(8) not ALIGN(4) and be done with it? > > > > > > >=20 > > > > > > > Remember, there is also falcon boot. And we definitely have t= o be able to > > > > > > > have old u-boot (SPL) boot new fitImage and vice versa. > > > > > >=20 > > > > > > I don't follow you, sorry. But since you seem to have the best > > > > > > understanding of where all of the cases something could go wron= g here, > > > > > > can you perhaps post an RFC patch? That is likely to be cleare= r than > > > > > > another long thread here. > > > > >=20 > > > > > I don't follow you, sorry. I believe the revert did the right thi= ng and > > > > > new systems should use mkimage -E when generating fitImages, to a= void > > > > > the string alignment problem. That is all. > > > >=20 > > > > Using -E should be optional and things really should work without i= t. > > >=20 > > > See the DTSpec, I don't think that is possible unless you relocate fi= tImage > > > components, and if you want fast boot time esp. in SPL, that is not g= ood. > >=20 > > This is why I've asked you to make up some patch to perhaps highlight > > the problem. Ensuring that the device tree, which is small, is also > > 8-byte aligned, shouldn't be a big problem nor performance hit. I'm not > > sure where the problem case is that isn't "user put things they control > > in a bad spot, fail and tell them why" but I could just be missing a > > case. >=20 > The fail case is this: > - you update SPL with this 8 byte alignment change > - you have older kernel fitImage with embedded DT for falcon mode > - system no longer boots because there is off-by-4 error in the DT > address passed to the kernel OK. Then I think the answer is what I said recently in another part of this thread, we need to split "find the fdt" from "align the fdt". The fdt can come to us with any alignment it happens to have, but we can't use that fdt in-place unless it's correctly aligned. In the case of falcon mode, it needs to end up at CONFIG_SYS_SPL_ARGS_ADDR. The case of passing it on to U-Boot proper is where we have at best a hack right now (as noted by fdt_hack in common/spl/spl.c). That would be a place to, as has been also suggested in this thread, pass along more correctly where the device tree in memory is. --=20 Tom --ddJkwolKDQNViLug Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmDuAXcACgkQFHw5/5Y0 tyw17Av/TpsUpVrnXvIkRqpAzkMpBNKx5Rwx3gGQ9ETMyNCwZhoVbCM+EOmr9vmA XPtpSJC4sTTEQIJqFWg6+dwInYGpADpxMPb+Q8hhlFK72znMjaSm0f6oKxJxFawB A7yJj0kJp2SOwFVZkh7mXLJAnBaOIEvc3VWan+bXA2u+V8Glr0nyT9iRbSAXbvXV fWF8R+mUO2XnXNJeiHPUyAuXiPLSfCIfGu7mxkkSgGJOWAYtSHC/R5pbyIfsZwqM abRaP1x2gdJsxMC7Ze92eVvVoCJa81PU1zbvGgCn6Zzx/lryQLp8SCY0HuEVXJyr 3dgJkJtSpTr5y0Y6AMHe0guPoFBRWAA4B9svGHpNlgNuPJj7H9ZLLEAvPJnhSPbB d/l3alhpLhGiowUB/LPIwgH+IxSHD8rJrG/48DE8sA9k52HGoDrBUQDj6EX2NLNy X0sIfzRyysagUtnZcDfbXgqhg1hkzkAn9uvUb3WLglThxRSG7EX9XIt5Mk5jVVAq mpAcH9oE =86ls -----END PGP SIGNATURE----- --ddJkwolKDQNViLug--