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 44A0EC07E95 for ; Tue, 13 Jul 2021 18:11:19 +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 44B5561374 for ; Tue, 13 Jul 2021 18:11:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 44B5561374 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 293E982C0B; Tue, 13 Jul 2021 20:11:15 +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="XtmZhTOV"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 9865582C0B; Tue, 13 Jul 2021 20:11:13 +0200 (CEST) Received: from mail-qt1-x833.google.com (mail-qt1-x833.google.com [IPv6:2607:f8b0:4864:20::833]) (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 BE4ED82BFA for ; Tue, 13 Jul 2021 20:11:09 +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-qt1-x833.google.com with SMTP id z25so14595140qto.12 for ; Tue, 13 Jul 2021 11:11:09 -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=bUIyci5zkfvovDsyslQgBAxErvgGfje47wPPsjzTcx4=; b=XtmZhTOVh72c7vapO+AVXPisrmlo4cNcM8zHDkpX8N8DczKeYfLP9zDhuBMaaMqU2o osZA2E+NSlClMoRl3TYGs3d6Ldcs2BB1wJ1IxlnLF0DOkde+jqvd2msbSWK5X3JZ7VnR pkRjX9QliwUbOxp7Df3EWfY5Pb9r3Ue8+6Lf4= 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=bUIyci5zkfvovDsyslQgBAxErvgGfje47wPPsjzTcx4=; b=qbo8NOskaYIEiG/kAn4acZV1B+uefa/lW5Cdy+J7i8g0SiKPT5eQMyF3KieAG5GCcf +piBCnT8EWlIJOwz+KhmW4B8NpncMZYr3JbycIaa9LKKW18erhMwdbxIfj5IdFP2REcv 6xxd9Wy8wsJR0YU4pHolJuRqNGoev86/xP9vMwYIUGJXzU/sDnywjTfgXRv728sQq/Sg 8mq5H9s/0bBOhJ67sHYGtCaRLLFMrzRdGKcKUYoCwWlXjjJrtt8l2qFcs+f/sfPcLG3e jm3qxzoj45f0ZqvAC7/hfrZfzyl3n/PPq2rattJw8S5XRXUmlT5bmQRzxw8s2qsu0o3H zeDg== X-Gm-Message-State: AOAM532dviIxkvIAvRb60J/011gAbp79qMUh2ANuUUxGhwvddbSddgmc Ue5/H9T0xDMFdNVeEQ8nhkQvhw== X-Google-Smtp-Source: ABdhPJzrgQgQ9cpBjhwo/z1IOL+7Eg3HzY4z/vszgcPFnBa9d644jSe7grCzl+J4de38nEUicvAhVQ== X-Received: by 2002:ac8:59c5:: with SMTP id f5mr5122842qtf.50.1626199868547; Tue, 13 Jul 2021 11:11:08 -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 c2sm620309qkd.57.2021.07.13.11.11.07 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Tue, 13 Jul 2021 11:11:07 -0700 (PDT) Date: Tue, 13 Jul 2021 14:11:05 -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: <20210713181105.GK9516@bill-the-cat> References: <58187afcb4aa481a8302777aca599fa7@4rf.com> <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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="3qMFcuxEC4IuatV4" Content-Disposition: inline In-Reply-To: <357b8f2e-4aa5-acf7-96aa-a4d0f7d1fbde@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 --3qMFcuxEC4IuatV4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 wrote: > > > > > > > > > On Mon, Jul 12, 2021 at 1:21 PM Reuben Dowle wrote: > > > > > > > > > >=20 > > > > > > > > > > I submitted an almost identical patch. See https://gith= ub.com/u-boot/u-boot/commit/eb39d8ba5f0d1468b01b89a2a464d18612d3ea76 > > > > > > > > > >=20 > > > > > > > > > > This patch eventually had to be reverted (https://githu= b.com/u-boot/u-boot/commit/5675ed7cb645f5ec13958726992daeeed16fd114), becau= se it was 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 platform aborts. > > > > > > > > > >=20 > > > > > > > > > > I don't have time to investigate why this was happening= , but you 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, th= e 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 that." > > > > > > > > >=20 > > > > > > > > > I don't understand this. If an address is aligned to 8, i= t 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 contents so= mewhere 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-byte al= ignment, 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 located. W= e'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 inste= ad 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 patc= h could change > > > > > > > all three, but one has to consider compatibility issues when = crossing u-boot > > > > > > > and SPL versions. > > > > > > >=20 > > > > > > > I had proposed in the revert discussion that SPL use r2 or si= milar 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-match > > > > > > SPL/U-Boot, but documenting what to go change if you must do it > > > > > > somewhere under doc/ would be good. I think we can just switch= to > > > > > > ALIGN(8) not ALIGN(4) and be done with it? > > > > >=20 > > > > > Remember, there is also falcon boot. And we definitely have to 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 wrong he= re, > > > > can you perhaps post an RFC patch? That is likely to be clearer th= an > > > > another long thread here. > > >=20 > > > I don't follow you, sorry. I believe the revert did the right thing a= nd > > > new systems should use mkimage -E when generating fitImages, to avoid > > > the string alignment problem. That is all. > >=20 > > Using -E should be optional and things really should work without it. >=20 > See the DTSpec, I don't think that is possible unless you relocate fitIma= ge > components, and if you want fast boot time esp. in SPL, that is not good. 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 Tom --3qMFcuxEC4IuatV4 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmDt1zIACgkQFHw5/5Y0 tyypkgv/WIxa/Nga1Xlamu5Au9bksnGsaofF7VjQM4JAvGb3I0FwSVd9kfnGUIR5 14DpQMCYWOznY8YUFiLky/iYsXeCHvVd8/tRmcwl/iglbxcoF2cyN+J4M8pYzHXL uQPn/3yo8oMtlfZXObUnBp5ETEumDXw8b/6IIoqWIdJEx2Iz37VfN9iljfXu69Cv vjCCYqQTk9+WdrNqENcaq3Q9CxoppgeusXxI95CFnAnn1m1uYpoj61Zzxjlq16ke 0A8/xQJw8ECXNabMDuPekty5h5zU4r1zIZPiaO+2M/VxQRVscvuGNPVBDM2FW170 bnDIcxxivqUlp7JtiiLtYbMWXgutYDcSqpFetO8DR/YuCr8SJ0VG3S4pa9A9mcjc RTyt7GEYksSI1ZMW7dtQZ5wuCiKAgDHnAjg7wQ/YHkqIiBDTWv5kI///iQN0mdJb dMDNq9nMjW57gYFCksiL2DX2nW/s1zhSlhuPXgwMN7Bt/zPNXn6WjLE92gQOMwg/ R+OhzKsp =CFLD -----END PGP SIGNATURE----- --3qMFcuxEC4IuatV4--