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=-9.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING, NICE_REPLY_A,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 E04C0C11F66 for ; Tue, 13 Jul 2021 20:46:49 +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 46DE861284 for ; Tue, 13 Jul 2021 20:46:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 46DE861284 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.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 1A1A682C0B; Tue, 13 Jul 2021 22:46:47 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="NKiOPqYs"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 1FC1E82DBD; Tue, 13 Jul 2021 22:46:45 +0200 (CEST) Received: from mail-oi1-x233.google.com (mail-oi1-x233.google.com [IPv6:2607:f8b0:4864:20::233]) (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 4D66F82B64 for ; Tue, 13 Jul 2021 22:46:41 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=mr.nuke.me@gmail.com Received: by mail-oi1-x233.google.com with SMTP id w188so5548282oif.10 for ; Tue, 13 Jul 2021 13:46:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=7l/HgwHes41795c3RphFj2zEKBs0p9lzeBOuYOWT1hI=; b=NKiOPqYsH34wrs1CkVgg5D+pPN1MvsjSxl7CYXI0WCctTHf5L8pJYZONEPE7oYjAPb IqYo48su4tAk2Fw/TwZRF/lrAv8NZ82RFAOw78WVbwX860o6dUeD1z+Ns0EQycsMKsWO WksLTFF0qFP4OXPZ9zL/vZQjlCZ++YxtjyAPV285BQ+06l+KKVWTx08tUBcHvqR5SMrZ wHOB68vZhuw6HXgTUbpBoOXd8SuRtDEKcK6PpYg7RyZZMlm7wsdrsnSYV/x6c9rRK/X4 CZcyh3PfodRXeN6NKM54fidnf7hv/Nlm+qsydn3ralwhmPK37F7UJCG6F9dIgwOAt184 r1Qw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=7l/HgwHes41795c3RphFj2zEKBs0p9lzeBOuYOWT1hI=; b=Zgx0O+SsCUuQKkPyuZR2k6LbWKA3lwRPJhWPB+5hpOMQbhDsWloDeF/1j1mEiVXmaB LggejfA6eZ+V7PfXTlRMDsZq7UGyYvWqyu8NfFEtkCFxZpsvOEB3bvN0Fc/xlK1UkBm4 VdOStdjCRwsO8NPsaJ5uTor8dMEZ7OWkJdsWMDJ0kT+GztCwjPdrWr+rHyd913QOnOfs 4fsi/3agATagv4wmP1XozWVxmTHUMy/PI2k7cyTLOBaQEfNIgpPu2BZBiEhwTzSgZF5c jnDVOu0vci1GFnuBL5sd7o3el8Ac9OLszRv4hRHWaWhJMXA4Zb5rBNXj/vNVEPi4nQ1P fMeg== X-Gm-Message-State: AOAM532bw5TW+aAdK7P48LhEZhlfrE+zpVmzjL2XPxQJEM51NupTM4WX ZIRBGuljapywPoJTgjaoAc8= X-Google-Smtp-Source: ABdhPJw1NzArXghyP2cJHOf7bQhjDDQNn2wRkiMc0m6SsEpE3Bj0vZC+CxkI5K4C08bbnXz/mODQyw== X-Received: by 2002:aca:eb86:: with SMTP id j128mr4620695oih.73.1626209199690; Tue, 13 Jul 2021 13:46:39 -0700 (PDT) Received: from nukespec.gtech (rrcs-67-78-34-122.sw.biz.rr.com. [67.78.34.122]) by smtp.gmail.com with ESMTPSA id v42sm3981833ott.70.2021.07.13.13.46.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 13 Jul 2021 13:46:38 -0700 (PDT) Subject: Re: [PATCH] spl: Align device tree blob address at 8-byte boundary To: Marek Vasut , Tom Rini Cc: Simon Glass , Bin Meng , Reuben Dowle , "u-boot@lists.denx.de" , Wolfgang Denk 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> <20210713181105.GK9516@bill-the-cat> <14e093f8-4357-1fd8-5875-6afc038d672e@denx.de> From: Alex G Message-ID: <0cf76b88-9db0-9fdd-ba0c-6aa161fe1a82@gmail.com> Date: Tue, 13 Jul 2021 15:46:37 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <14e093f8-4357-1fd8-5875-6afc038d672e@denx.de> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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 On 7/13/21 3:35 PM, 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 >>>>>>>>>>>> https://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 boundary. 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 happening, 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 by 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 system hang on >>>>>>>>>>> arm32? >>>>>>>>>> >>>>>>>>>> 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. >>>>>>>>> >>>>>>>>> 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 located. >>>>>>>>> We'll look at >>>>>>>>> two cases: >>>>>>>>>      1) u-boot as a FIT (binary and FDT separately loaded) >>>>>>>>>      2) u-boot with embedded FDT >>>>>>>>> >>>>>>>>> In case (1) SPL must place the FDT at a location where u-boot >>>>>>>>> will find it. >>>>>>>>> The current logic is >>>>>>>>>      SPL:    fdt = ALIGN_4(u_boot + u_boot_size) >>>>>>>>>      u-boot: fdt = ALIGN_4(u_boot + u_boot_size) >>>>>>>>> >>>>>>>>> In case (2), SPL's view of the FDT is not relevant, but instead >>>>>>>>> the build >>>>>>>>> system must place the FDT correctly: >>>>>>>>>      build:  fdt >> u-boot.bin >>>>>>>>>      u-boot: fdt = ALIGN_4(u_boot + u_boot_size) >>>>>>>>> >>>>>>>>> We have 3 places that must agree. A correct and complete patch >>>>>>>>> could change >>>>>>>>> all three, but one has to consider compatibility issues 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-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? >>>>>>> >>>>>>> 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. >>>>>> >>>>>> 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 clearer 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 without it. >>> >>> See the DTSpec, I don't think that is possible unless you relocate >>> fitImage >>> 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. > > 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 I'm not sure how falcon mode would break the kernel. It passes to the kernel the load address of the fdt. The concern here is loading u-boot. > I hope this is clear now.