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,URIBL_BLOCKED,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 4D46BC07E99 for ; Mon, 12 Jul 2021 16:02:17 +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 8C5E26102A for ; Mon, 12 Jul 2021 16:02:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8C5E26102A 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 BAC3C82DEF; Mon, 12 Jul 2021 18:02:14 +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="QSX00Qs9"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id A15A482DF9; Mon, 12 Jul 2021 18:02:13 +0200 (CEST) Received: from mail-qv1-xf30.google.com (mail-qv1-xf30.google.com [IPv6:2607:f8b0:4864:20::f30]) (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 3CDE182DEB for ; Mon, 12 Jul 2021 18:02:10 +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-qv1-xf30.google.com with SMTP id i11so4849632qvv.12 for ; Mon, 12 Jul 2021 09:02:10 -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=Ayoh/fsYEFIunnEWaZYK5jCMJABKWUl5e1xmF/YYmfc=; b=QSX00Qs9NapaMBRTM3j75J3eameaTvHKhIQ8BVnKKvd60RrG7H3k86rRMPHj7WCAIX Zy/bV42mBezLamMkb4EESRNHh0hjP1LKEzCMJQdwFj3V/S4Nj7MHPEQ2JwM3FxGrTbNV IBRwNaQxDDQMAgPyOAYgOpwMdMvY1wHLuYl/I= 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=Ayoh/fsYEFIunnEWaZYK5jCMJABKWUl5e1xmF/YYmfc=; b=B/GtdAruMpEHW2wQ4Nsau+ihsLkXw6/BttHJP7RljG9+qZ+xBmMSUlVmh+fPVO51OY qYsXZrmTL3Su73O5FuJ1Ex7HlIJZaPMIjy2xZB4CUAO9XwKxQ9hCaDbGh86148Bvc65w eaQ4hAz46B2yZLrCwVyU9TY2gNvqEzujstuCZBwGXoAbL/vCzhbmNLHcMGTZgbpxLcB6 MQnkX9Yd7RYUdBNWFoN2O8JPpOCiR6YANjdd81Kr4krfG1J5v2n5DVQ8Nzhh0uR0VIGu 3eCrgOvmZDC6gVAQ3dO+cyT5jdnHoYyULJEmfPRV/rC+0pn6eih7o7XOpQQ4er72D6/c hzZg== X-Gm-Message-State: AOAM530stoUsGSiTb9BsQDxSY0nUMVNSdx8Yo5nGMcfij8Hc5WzuQRQi a93cx1l+RPR/pZwgOhxem9Tz/g== X-Google-Smtp-Source: ABdhPJxhMYAATNPxFsBcD7ORd95S0bg32lJHoo/rBF7ok9MTr2R1guI9xiZMZEhmAbWAGG8+nd9tAg== X-Received: by 2002:a0c:abc6:: with SMTP id k6mr11604663qvb.18.1626105728850; Mon, 12 Jul 2021 09:02:08 -0700 (PDT) Received: from bill-the-cat (2603-6081-7b01-cbda-8124-e22d-adbc-1eeb.res6.spectrum.com. [2603:6081:7b01:cbda:8124:e22d:adbc:1eeb]) by smtp.gmail.com with ESMTPSA id t20sm5731796qtx.48.2021.07.12.09.02.02 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Mon, 12 Jul 2021 09:02:03 -0700 (PDT) Date: Mon, 12 Jul 2021 12:02:01 -0400 From: Tom Rini To: Marek Vasut Cc: Bin Meng , Reuben Dowle , Simon Glass , "u-boot@lists.denx.de" Subject: Re: [PATCH] spl: Align device tree blob address at 8-byte boundary Message-ID: <20210712160201.GY9516@bill-the-cat> References: <20210712035231.26475-1-bmeng.cn@gmail.com> <58187afcb4aa481a8302777aca599fa7@4rf.com> <20210712151510.GT9516@bill-the-cat> <1fed7815-de64-27b4-77ed-4e888fb882da@denx.de> <20210712154316.GX9516@bill-the-cat> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="HjB83FewUGURIQKg" Content-Disposition: inline In-Reply-To: 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 --HjB83FewUGURIQKg Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jul 12, 2021 at 05:51:29PM +0200, Marek Vasut wrote: > On 7/12/21 5:43 PM, Tom Rini wrote: > > On Mon, Jul 12, 2021 at 05:38:33PM +0200, Marek Vasut wrote: > > > On 7/12/21 5:15 PM, Tom Rini wrote: > > > > On Mon, Jul 12, 2021 at 01:36:14PM +0800, Bin Meng wrote: > > > > > 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://github.com/u-= boot/u-boot/commit/5675ed7cb645f5ec13958726992daeeed16fd114), because it wa= s causing issues on some platforms that had FIT on 32 bit boundary. However= I continue to use it in production code, as without it the boot on my plat= form aborts. > > > > > >=20 > > > > > > I don't have time to investigate why this was happening, but yo= u need to check this code won't just cause exactly the same faults. > > > > >=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 SPL, the system s= imply > > > > > hangs. This is because on arm32, the fitImage and all of its cont= ent > > > > > can be aligned to 4 bytes and U-Boot expects just that." > > > > >=20 > > > > > I don't understand this. If an address is aligned to 8, it is alr= eady > > > > > aligned to 4, so how did this commit make the system hang on arm3= 2? > > > >=20 > > > > I think this had something to do with embedding contents 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 > > > If I recall this correctly, DT node alignment is 4 byte and that is w= hat DTC > > > emits. If you have fitImage with embedded data, you basically end up = with > > >=20 > > > / { > > > prop1 =3D "string1"; > > > prop2 =3D "string2"; > > > }; > > >=20 > > > where the "string2" is aligned to 4 bytes. And that is what U-Boot ex= pects > > > when it tries to access those data in-place in SPL. > > >=20 > > > The problem with the reverted patch was that it made U-Boot assume the > > > alignment is 8 bytes, and that actually works only if you use fitImag= e with > > > external data (mkimage -E), but with embedded data (mkimage default) = not so > > > much. That caused off-by-4 error in some cases and that made the SPL = hang. > > >=20 > > > > > Note, as I indicated in this patch, now with libfdt 1.6.1, the > > > > > alignment to 8 byte is a must-have. So we have to do such alignme= nt > > > > > anyway. > > > > >=20 > > > > > @Tom may fill in why libfdt commit commit 5e735860c478 ("libfdt: = Check > > > > > for 8-byte address alignment in fdt_ro_probe_()") was made to hav= e the > > > > > 8-byte alignment requirement. > > > >=20 > > > > Note that it's not so much since libfdt 1.6.1 but that since always= the > > > > device tree has required 8 byte alignment. > > >=20 > > > DT alignment was always 4 byte , no ? > >=20 > > I'm pretty sure, no, 8 byte base alignment is a pretty much always > > thing. I don't have a reference handy but I also know I couldn't have > > convinced dgibson to add the check otherwise. >=20 > DTSpec rev 0.3 says the following and I think you got confused by section > 5.6 which talks about alignment of the entire tree, not its nodes: >=20 > 5.4 Structure Block > " > Each token in the structure block, and thus the structure block itself, > shall be located at a 4-byte aligned offset from the > beginning of the devicetree blob (see 5.6). > " >=20 > 5.4.2 Tree structure > " > For each property of the node: > ... > =E2=80=93 FDT_PROP token > ... > * [zeroed padding bytes to align to a 4-byte boundary] > " >=20 > 5.5 Strings Block > " > The strings block contains strings representing all the property names us= ed > in the tree. These null terminated strings are > simply concatenated together in this section, and referred to from the > structure block by an offset into the strings block. > The strings block has no alignment constraints and may appear at any offs= et > from the beginning of the devicetree blob. > " >=20 > 5.6 Alignment > " > As described in the previous sections, the structure and strings blocks > shall have aligned offsets from the beginning of > the devicetree blob. To ensure the in-memory alignment of the blocks, it = is > sufficient to ensure that the devicetree as a > whole is loaded at an address aligned to the largest alignment of any of = the > subblocks, that is, to an 8-byte boundary. Right. A device tree must start at an 8-byte boundary and U-Boot was violating this both with: - All of the boards that use fdt_high=3D0xffffffff to disable relocation, and then then place things at arbitrary spots in memory that may or may not violate these requirements. - Perhaps some of the FIT internals where we have a device tree inside a device tree? And we need to fixup whatever we're doing there that's wrong. --=20 Tom --HjB83FewUGURIQKg Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmDsZ3UACgkQFHw5/5Y0 tyzXfgv/dKi99fZGYkMwMj7LcMuF2CgiJZI25nvsQvwUJ6SSG9Vcj468s4RAbnld jB3OXoSIR2cTOn490vLvS2wfoCR5iY68d1OdSz/ed5rUMZeglCXya2mroE18G8JS Fjpy7EsN3PYJCcPl/0FLuYNAXYcU93wJeVhwBMjhN2sej2RkRpRJfYe41h6x/rM2 ofO+4dZ2BZ/KQyCcoXRrrRuxCdhtDUBEQqEELoTCXw1U6/nc1zRQfkeB9nf56i2Z r+xtf3jJ7bMT1iCg0DwjIV7ueR9F8KMt13hQFlSfhpUTNheAuxfAwMDTgFn7KP73 kMVWlYUcUMY2vps7Kc0Tk5/hES6vHacDRyUMwGZrgGkYcEY5n7l4D42Y7hmAyTCR 8/tYBUlDQg4p2hsjY+IreFXG3GGdjm32cybezdglLuPnoepK0Lrj7F43UJ8AepmP kw7orYUXnOzEybh0B0+0OTI3zOpeaoigUJl9JhyeBhpGpvIP9eoSFxvBujDhIyWr 69fDSbVe =/qnR -----END PGP SIGNATURE----- --HjB83FewUGURIQKg--