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.2 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 46A20C4338F for ; Mon, 26 Jul 2021 13:42:12 +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 C08EF60243 for ; Mon, 26 Jul 2021 13:42:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C08EF60243 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 1D876833C2; Mon, 26 Jul 2021 15:41:21 +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="brdk+RVh"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 88F198337D; Mon, 26 Jul 2021 15:39:35 +0200 (CEST) Received: from mail-qt1-x835.google.com (mail-qt1-x835.google.com [IPv6:2607:f8b0:4864:20::835]) (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 1C6478338F for ; Mon, 26 Jul 2021 15:38:36 +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-x835.google.com with SMTP id l24so7008758qtj.4 for ; Mon, 26 Jul 2021 06:38:36 -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=SLA160qH7CR34Y7Ypq68Pi9bld8vi7aD9TAq2aN596s=; b=brdk+RVhKkbWUBGRjsATO78BhmPCiZ3PlX+2rieZHhwBtCtZUnbY06RTX1qGzNe18I HJ6392Wsw1e6fPKfRj3hkbgMAJgM42o0X18LdCNtwIbMol5hWepbBgoA+60Vya9JY/xL PbjYXvTfKVj2VCkJnReUydJUL81Ckgi5eorR0= 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=SLA160qH7CR34Y7Ypq68Pi9bld8vi7aD9TAq2aN596s=; b=lHX52j7u6lJw24js0hb6yRALE77+5/OimijRhiyqjyxnJuYynL2I4sy4kN+R2YV2RT ZRdtB1xi1y5C7QygnVceOzhGlN/fRGEmusImMzQ9IkoAQo/R1bN8u9/qiCIup7BYuQQT jSFhYr0TzLPLr8rGnhzxIkHdw6Zi/di6qTC+qdJrbJFd85t/sp+uK6tWd9j04sIl9RRx iv6XXluxKjVKfWz4fZnSjlWtde0Rg0Q1DhnjHJK814TkVvbaNUnyaxg7Qt4R/3XKBaac KoU392+LbOe1JMSH17PTzaRAPSY+b7lJ4+Wy1iLpuNFpHw9BT0zvidnnYNfet8Q7eb4X mbGQ== X-Gm-Message-State: AOAM531baxVNLkDaL5MTRsrLKXjuw3AOUebi8YcO2rvyvusxybH3sWza xu515SfrtBli8zWUE3NArdI1vw== X-Google-Smtp-Source: ABdhPJwALRow0q9y7Liw+EwULulQF+klsJstAfGvJ0Tq9S7vimYoSkvSI/A1HU2TKmYGy5S0V3znGQ== X-Received: by 2002:ac8:7310:: with SMTP id x16mr14918295qto.178.1627306714789; Mon, 26 Jul 2021 06:38:34 -0700 (PDT) Received: from bill-the-cat (2603-6081-7b01-cbda-5449-b89b-3de4-1412.res6.spectrum.com. [2603:6081:7b01:cbda:5449:b89b:3de4:1412]) by smtp.gmail.com with ESMTPSA id j3sm18510095qka.96.2021.07.26.06.38.33 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Mon, 26 Jul 2021 06:38:33 -0700 (PDT) Date: Mon, 26 Jul 2021 09:38:31 -0400 From: Tom Rini To: Bin Meng Cc: Marek Vasut , Simon Glass , "Alex G." , Reuben Dowle , "u-boot@lists.denx.de" , Wolfgang Denk Subject: Re: [PATCH] spl: Align device tree blob address at 8-byte boundary Message-ID: <20210726133831.GT9379@bill-the-cat> References: <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> <20210713211122.GO9516@bill-the-cat> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="qwNWzpZrOR6kNuUY" 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 --qwNWzpZrOR6kNuUY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jul 26, 2021 at 09:26:26PM +0800, Bin Meng wrote: > On Wed, Jul 14, 2021 at 5:11 AM Tom Rini wrote: > > > > 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, > > > > > > > > > > > > On Tue, 13 Jul 2021 at 08:53, Marek Vasut wrote: > > > > > > > > > > > > > > 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: > > > > > > > > > > > > > > > > > > > > > > > > > > > > I submitted an almost identical patch. See http= s://github.com/u-boot/u-boot/commit/eb39d8ba5f0d1468b01b89a2a464d18612d3ea76 > > > > > > > > > > > > > > > > > > > > > > > > > > > > This patch eventually had to be reverted (https= ://github.com/u-boot/u-boot/commit/5675ed7cb645f5ec13958726992daeeed16fd114= ), because it was causing issues on some platforms that had FIT on 32 bit b= oundary. However I continue to use it in production code, as without it the= boot on my platform aborts. > > > > > > > > > > > > > > > > > > > > > > > > > > > > I don't have time to investigate why this was h= appening, but you need to check this code won't just cause exactly the same= faults. > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for your information. > > > > > > > > > > > > > > > > > > > > > > > > > > +Marek who did the revert > > > > > > > > > > > > > > > > > > > > > > > > > > The revert commit message says: > > > > > > > > > > > > > > > > > > > > > > > > > > "The commit breaks booting of fitImage b= y SPL, 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= that." > > > > > > > > > > > > > > > > > > > > > > > > > > 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 sys= tem hang on arm32? > > > > > > > > > > > > > > > > > > > > > > > > I think this had something to do with embedding con= tents somewhere in > > > > > > > > > > > > the image? There is a thread on the ML from then b= ut I don't know how > > > > > > > > > > > > informative it will end up being. > > > > > > > > > > > > > > > > > > > > > > It's true that the flat devicetree spec requires an 8= -byte alignment, even > > > > > > > > > > > on 32-bit. The issues here are specific to u-boot. > > > > > > > > > > > > > > > > > > > > > > SPL and u-boot have to agree where u-boot's FDT is lo= cated. We'll look at > > > > > > > > > > > two cases: > > > > > > > > > > > 1) u-boot as a FIT (binary and FDT separately lo= aded) > > > > > > > > > > > 2) u-boot with embedded FDT > > > > > > > > > > > > > > > > > > > > > > In case (1) SPL must place the FDT at a location wher= e 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) > > > > > > > > > > > > > > > > > > > > > > In case (2), SPL's view of the FDT is not relevant, b= ut instead the build > > > > > > > > > > > system must place the FDT correctly: > > > > > > > > > > > build: fdt >> u-boot.bin > > > > > > > > > > > u-boot: fdt =3D ALIGN_4(u_boot + u_boot_size) > > > > > > > > > > > > > > > > > > > > > > We have 3 places that must agree. A correct and compl= ete patch could change > > > > > > > > > > > all three, but one has to consider compatibility issu= es when crossing u-boot > > > > > > > > > > > and SPL versions. > > > > > > > > > > > > > > > > > > > > > > I had proposed in the revert discussion that SPL use = r2 or similar mechanism > > > > > > > > > > > to pass the location of the FDT to u-boot. > > > > > > > > > > > > > > > > > > > > I'm not sure that we need to worry too much about mix-a= nd-match > > > > > > > > > > SPL/U-Boot, but documenting what to go change if you mu= st do it > > > > > > > > > > somewhere under doc/ would be good. I think we can jus= t switch to > > > > > > > > > > ALIGN(8) not ALIGN(4) and be done with it? > > > > > > > > > > > > > > > > > > Remember, there is also falcon boot. And we definitely ha= ve to be able to > > > > > > > > > have old u-boot (SPL) boot new fitImage and vice versa. > > > > > > > > > > > > > > > > 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 here, > > > > > > > > can you perhaps post an RFC patch? That is likely to be cl= earer than > > > > > > > > another long thread here. > > > > > > > > > > > > > > I don't follow you, sorry. I believe the revert did the right= thing and > > > > > > > new systems should use mkimage -E when generating fitImages, = to avoid > > > > > > > the string alignment problem. That is all. > > > > > > > > > > > > Using -E should be optional and things really should work witho= ut it. > > > > > > > > > > See the DTSpec, I don't think that is possible unless you relocat= e fitImage > > > > > components, and if you want fast boot time esp. in SPL, that is n= ot good. > > > > > > > > This is why I've asked you to make up some patch to perhaps highlig= ht > > > > 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 con= trol > > > > in a bad spot, fail and tell them why" but I could just be missing a > > > > case. > > > > > > 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 > Where are we on this issue? Waiting for someone to do what I suggested I think. I've not yet had time to take a look. --=20 Tom --qwNWzpZrOR6kNuUY Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmD+us8ACgkQFHw5/5Y0 tyz5Rwv/ToYk8ZznTNwW5ZLeHjpxUkkAYQXxVrKbUmhuyTPrRxoCESvDbzzop16t BbjoIsRlADgEw469d+QlaukHbsfK+rESDPXozFArBpVQjG7HTDhk+nDWNoDgw3Xi fBpGvUi5vxaPTV655Ge18dwSSKIVmnXMG6g9hnEMCjDFkClOL48RdSx2rXLkmsvA N0JaZVSGcOvBnU63HEMTKWoq0MmkryknWLR2GY1vAqyKQnbkbyv1MM5QGgnnaCyt 67X6FXdjYR7u95of/4aA40s3y8RdHQ2Hr2X/DpZUUuyks5WRE+DWg636PUnrZarP QA0WpEuSATO0y1//07ZkSjnX//7Q0C8PTSi5vB2LDZYWyQ6jg3F9uwRRW84uyg/U PlVi4VSONmuGDn2stm80CT6P+pfO4u/kHjcd5OS5v/3pjmmaT3yP4oJG/S3lkXYI fqo0oBFgGI2ogF0AvKe29MXD44mZzlfL94w7Nf7LnBRDNzGF9FFm2upIKzS7foc1 cP1IVLQf =YB9c -----END PGP SIGNATURE----- --qwNWzpZrOR6kNuUY--