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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 478BEC433EF for ; Thu, 28 Oct 2021 22:50:56 +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 76AFB610C7 for ; Thu, 28 Oct 2021 22:50:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 76AFB610C7 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 BE6A983513; Fri, 29 Oct 2021 00:50:52 +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="iY99ReqO"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 63E278356D; Fri, 29 Oct 2021 00:50:50 +0200 (CEST) Received: from mail-qv1-xf36.google.com (mail-qv1-xf36.google.com [IPv6:2607:f8b0:4864:20::f36]) (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 4D35383513 for ; Fri, 29 Oct 2021 00:50:44 +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-xf36.google.com with SMTP id u25so5174080qve.2 for ; Thu, 28 Oct 2021 15:50:44 -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; bh=cVyssUppEJRREmm06dLjAwBiKnAmXYGkb3l9p+ZKZpY=; b=iY99ReqO37/TOwXuTXJJkW0OiykZNGFKj+c1YyMKMq9YIl9nuTiIIhfNNDyebvAMXr thVSaYA5w2uPg0/nTBlMBS6K0TfzexJo7PI/7SAEJw9vynit0dIhBWxAPB6DgvPUVREi xN/KsyGeokLp9iO7MbnYtCbBcfMYMeEiVOxqc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=cVyssUppEJRREmm06dLjAwBiKnAmXYGkb3l9p+ZKZpY=; b=w2fHmqCfStAWnE9FefWGlwo6tSoapxF6GySVY4VOYKk9yJ6iIWeArUIIWwRV2wHv74 Nar2x9xPGe7JT3ZUG0VrHPl5sDD7quFsLA637M/0FoI6Iw5FAVwHd09fIxBp9zF5MDdG mc7Aw548Qn3kXOmpxTC/CRiZD+/mpdlUKfOvy2DcL4J43s42pPvDEmroXPYcl4iKCPqa 3Mby1PMmxTQuXXEeqCZeN8sob6i4OX7qSaBv7snSaXVA4eIrzWsvY0Aa9sE82E7RrZ+a psBvZSqktMx+2+QCUd2+Um0GDAPdMy2QjbFTUglU0XE1afe6QMYZ2fKoMLPixoANrNFk a3iQ== X-Gm-Message-State: AOAM5317SbW9ml8iY7mhoNHaqhWEDFcIO05TvFE195wBD7aqcGx8rxwu ChEwxrV1nxHUZpJVfdM+68QJ9w== X-Google-Smtp-Source: ABdhPJzTKvTsI3TXZ9QWFNm+YmyjyOWGr0rFzV18fRoXY0mcBvhwVCmRDlAN/IxHVG87R7Qcl88rVg== X-Received: by 2002:a05:6214:b68:: with SMTP id ey8mr7417640qvb.7.1635461442837; Thu, 28 Oct 2021 15:50:42 -0700 (PDT) Received: from bill-the-cat (2603-6081-7b01-cbda-cd13-3c38-2d32-68e0.res6.spectrum.com. [2603:6081:7b01:cbda:cd13:3c38:2d32:68e0]) by smtp.gmail.com with ESMTPSA id u15sm3054556qki.99.2021.10.28.15.50.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Oct 2021 15:50:41 -0700 (PDT) Date: Thu, 28 Oct 2021 18:50:39 -0400 From: Tom Rini To: Simon Glass Cc: U-Boot Mailing List , Michal Simek , Heinrich Schuchardt , Ilias Apalodimas , Daniel Schwierzeck , Steffen Jaeckel , Marek =?iso-8859-1?Q?Beh=FAn?= , Lukas Auer , Dennis Gilmore , Jaehoon Chung , Marek Vasut , Masahiro Yamada , Pavel Herrmann , Peng Fan , Stephen Warren , Stephen Warren Subject: Re: [PATCH v2 00/41] Initial implementation of standard boot Message-ID: <20211028225039.GP8284@bill-the-cat> References: <20211023232635.9195-1-sjg@chromium.org> <20211028162741.GA8284@bill-the-cat> <20211028175229.GF8284@bill-the-cat> <20211028183652.GJ8284@bill-the-cat> <20211028191903.GK8284@bill-the-cat> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="qj6EhVtbuE7iUtz+" Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett 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 --qj6EhVtbuE7iUtz+ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Oct 28, 2021 at 04:22:11PM -0600, Simon Glass wrote: > Hi Tom, >=20 > On Thu, 28 Oct 2021 at 13:19, Tom Rini wrote: > > > > On Thu, Oct 28, 2021 at 12:48:50PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Thu, 28 Oct 2021 at 12:36, Tom Rini wrote: > > > > > > > > On Thu, Oct 28, 2021 at 12:13:56PM -0600, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Thu, 28 Oct 2021 at 11:52, Tom Rini wrote: > > > > > > > > > > > > On Thu, Oct 28, 2021 at 11:29:35AM -0600, Simon Glass wrote: > > > > > > > Hi Tom, > > > > > > > > > > > > > > On Thu, 28 Oct 2021 at 10:27, Tom Rini w= rote: > > > > > > > > > > > > > > > > On Sat, Oct 23, 2021 at 05:25:54PM -0600, Simon Glass wrote: > > > > > > > > > > > > > > > > > The bootflow feature provide a built-in way for U-Boot to= automatically > > > > > > > > > boot an Operating System without custom scripting and oth= er customisation. > > > > > > > > > This is called 'standard boot' since it provides a standa= rd way for > > > > > > > > > U-Boot to boot a distro, without scripting. > > > > > > > > > > > > > > > > > > It introduces the following concepts: > > > > > > > > > > > > > > > > > > - bootdev - a device which can hold a distro > > > > > > > > > - bootmeth - a method to scan a bootdev to find bootfl= ows (owned by > > > > > > > > > U-Boot) > > > > > > > > > - bootflow - a description of how to boot (owned by th= e distro) > > > > > > > > > > > > > > > > > > This series provides an implementation of these, enabled = to scan for > > > > > > > > > bootflows from MMC, USB and Ethernet. It supports the exi= sting distro > > > > > > > > > boot as well as the EFI loader flow (bootefi/bootmgr). It= works > > > > > > > > > similiarly to the existing script-based approach, but is = native to > > > > > > > > > U-Boot. > > > > > > > > > > > > > > > > I'm going to break my feedback down in to a few threads, to= hopefully > > > > > > > > not confuse things too much. My first comment is that rpi_= arm64 grows > > > > > > > > in size by 17 kilobytes, with the whole series (pxe, env, t= his) applied. > > > > > > > > And while there's a few small changes in the pxe cleanup I'= m going to > > > > > > > > re-investigate on their own, it's really just this series, = right here, > > > > > > > > adding tons of code. To replace an admittedly complex bit = of > > > > > > > > environment scripting, with C. It's not even the earlier p= arts of the > > > > > > > > series to clean up / prepare, it starts at "bootstd: Add th= e bootstd > > > > > > > > uclass and core implementation" and keeps going from there. > > > > > > > > > > > > > > Yes it does add a lot of code, although it is a lot less if t= he > > > > > > > commands are disabled or simplified, e.g. to only support 'bo= otflow > > > > > > > scan -b'. At the moment it enables all dev features. > > > > > > > > > > > > OK, for the next go-round yes, please show what a typical enabl= ement > > > > > > would look like on Pi, for example. > > > > > > > > > > OK. Do understand that conceiving of this and implementing it is = quite > > > > > a bit of effort. At some point I just send things out, to get fee= dback > > > > > and to think some more. Apart from anything else, there is a risk= of > > > > > going into the weeds or never finishing it. > > > > > > > > > > > > > > > > > > It does save a small amount of data. E.g. rpi_3_32b environme= nt goes > > > > > > > drops by 3.5KB. > > > > > > > > > > > > So we're replacing 3.5KB of scripts with 17KB of code. That is= not a > > > > > > win. > > > > > > > > > > Certainly not on size! On testing, debug, visibility and control = of > > > > > the boot process, there are wins. > > > > > > > > I'm not sure if there's wins on those grounds either, and certainly= not > > > > enough to justify what this adds. > > > > > > > > > > > We should compare this with the EFI support which is about 90= KB, as I recall. > > > > > > > > > > > > No, we shouldn't. This isn't about replacing EFI, this is about > > > > > > replacing the generic distro boot macros, so that's what the si= ze > > > > > > comparison is to. At the end of the day, and looking towards n= on-legacy > > > > > > uses, a big common use case is "Find the EFI app to run". > > > > > > > > > > I just mean that EFI has been accepted as part of U-Boot and adds= 90KB. > > > > > > > > OK? I still don't see the relevance here. > > > > > > > > > > > If we continue down the path of making this feature use U-Boot > > > > > > > functions directly, instead the command line, I suspect we ca= n save > > > > > > > quite a bit more, since there is a lot of overhead with these > > > > > > > commands. At present it is impossible to boot without CONFIG_= CMDLINE > > > > > > > enabled, for example. > > > > > > > > > > > > Yes, this should be using the API and not the command interface. > > > > > > > > > > Right, but that's not something I am taking on right now. The PXE > > > > > refactoring gives an idea of what is needed. I did that work main= ly > > > > > because I don't like adding to code that desperately needs > > > > > refactoring. We need to do the same for dhcp, EFI and bootm/zboot= , but > > > > > that might take a year. > > > > > > > > Well, maybe this particular part of things get set aside for now, a= nd > > > > the generic distro boot framework just needs to be moved to the env > > > > update you did. > > > > > > > > > > > In any case, I think this feature is filling a gap in U-Boot = since at > > > > > > > present everything about boot is ad-hoc. This gives us a base= to build > > > > > > > on. Nothing is for free. > > > > > > > > > > > > I disagree. At present, booting is either intentionally per-bo= ard, or > > > > > > it's using the generic distro boot framework. That framework i= s what to > > > > > > further build on or perhaps make more readily simplified (for e= xample, > > > > > > making it just be "scan around for EFI" or just be "scan around= of > > > > > > extlinux.conf"). > > > > > > > > > > Well if the Universal Bootloader is only going to exist to boot E= FI, > > > > > then we should rename it :-) > > > > > > > > > > I am not sure that anyone wants something intentionally per-board, > > > > > > > > There's some cases, yes, where the system is supposed to do X (or Y= , or > > > > Z). Otherwise there's the generic framework. Or... > > > > > > > > > it's just that everything about the boot in U-Boot is really low-= level > > > > > (bootm, fixed addresses, etc.) We need the layer on top that can = deal > > > > > with these silly details. For example, yes there is a Chrome OS b= oot > > > > > script in chromebook_coral, but it is the same on all devices. I = would > > > > > rather just enable that bootmethod so that if Chrome OS is presen= t it > > > > > will boot. > > > > > > > > there's things like what Chrome OS wants. > > > > > > > > > Re the memory side, i don't like the vars that define the kernel > > > > > address, FDT address, etc. It seems to me that most/all are > > > > > unnecessary, if there is something able to deal with memory alloc= atoin > > > > > automatically. Perhaps we should use malloc() more, or use LMB mo= re > > > > > proactively. > > > > > > > > Well, in that for Linux, arm64 the Image format has a header that l= ets > > > > us avoid a number of problems that weren't possible on arm32, there= 's a > > > > tiny bit more flexibility there. But it sounds like you're talking > > > > about "bootm_size" and "bootm_low" now. > > > > > > > > > Even for custom flows, creating a bootmethod will have advantages= , I think. > > > > > > > > > > The other thing is that this allows further innovation. For verif= ied > > > > > boot, it makes it possible to sensibly deal with A/B;recovery, wh= ereas > > > > > at present that is all just scripting, certainly not handled by d= istro > > > > > boot. > > > > > > > > No distros implement A/B updates directly. When implemented on top= of > > > > them it's done via the environment, yes. I don't think adding Mend= er > > > > and RAUC and swupdate specific bootflow commands is a step in the r= ight > > > > direction at all. > > > > > > > > > > > Anyway, I can look at what the minimum size is with the above= points > > > > > > > and send that info through. > > > > > > > > > > > > I looked at the PXE stuff, and I think the minimal growth there= ends up > > > > > > being reasonable, fwiw. It comes down to adding sanity checks = in places > > > > > > where the code wasn't always sanity checking, as you reduced > > > > > > duplication. > > > > > > > > > > Yes and perhaps the growth can be reduced, too. > > > > > > > > It must be. You need to be a lot closer to parity with the existing > > > > generic boot mechanism and around that size. I am not liking what = I'm > > > > seeing here so far. > > > > > > Just on this point, I'm pretty sure this will kill it. We cannot add > > > code without increasing the size! It sounds like 17KB on ARM64 is too > > > much (perhaps 13.5KB including env reduction). Firstly, why, and > > > secondly, how much is acceptable? If the answer is zero, we are not > > > going to go anywhere with this, at least without a huge amount of > > > future refactoring, which is not going to happen since we cannot land > > > 100s of patches all at once. > > > > > > The big prize would be to be able to disable CONFIG_CMDLINE and still > > > boot. That saves 200KB or more. > > > > I think the problem is this just needs to get put on hold for a while. > > I am not convinced this is moving things in the right direction. > > Conceptually, it's duplicating a lot of the efi bootmgr functionality, > > but also adding in non-block devices. And I am not liking configuring > > this via device tree, rather than environment. Lets move the generic > > distro framework to the new env framework and continue finishing off > > other technical debts that we have, before we re-visit a brand new way > > of handling booting. >=20 > OMG, please, no :-) That would be a mess IMO. We should have done a > proper boot framework when distroboot was created, instead of all the > scripts. >=20 > Now it's seems we need to stick with that, or move to EFI bootmgr? I'm not sure if EFI bootmgr is going to cover the cases, or not. But yes, in so far as I have problems with the current distro boot, it's that dealing with strings in C kinda sucks. Having that be plain text will be good. > Is the new env framework agreed? I'd love to see some reviews from Wolfga= ng. I believe the new env framework is agreed, yes. So long as "complex" environments can be constructed and dropped in some way or another outside of the new tooling, that's good enough. >=20 > Regards, > Simon --=20 Tom --qj6EhVtbuE7iUtz+ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmF7KTwACgkQFHw5/5Y0 tyzgTQv5ATiYGsRvpmJyy2RCD5Ovp3OaG6PFrtCsp76fGNQkG6RxW6NcudoUvYrU LTH6LBjxoJ7OWr95ZSxckVhXRRKz4ct5pUiEnPDnpG7KIQ5XL6cen+2dD89zmdkL VNTXgus880y4GwMiIqcPXla7iwpBdfW6Wy2AHx5M8/Mo5KXRpYruzVqfHjd3fqPz JT0vUk939w95IU/c6gQTYSJ7p/CHo3vAOST044tMlCOkaa743ZztDc5LvbTUVN/2 hRkIfyYGGBNT067EUrL37wp7PVL9VpRriABUiedBRTe7HPfeW2FeasTMpV5d8zYr 6XvQ+s32uNjMH5IFnud08UQMf5r9TtMG/1DGuppY7tG2sn3JOdJzfdvUpOBp7SOA pQ9cRk0HQ5AUMPWj81msfYjZUTRjmpVPYCONdvw5bXj1w3kZaQIM7P4LP7cp2kyQ x0EsHOSYhA3QwZHyyMNRLEbNAza1aCUnXF7equb8b3cJEAntKPWYAjVs0JUyIsjX i5qLK00Z =nnjA -----END PGP SIGNATURE----- --qj6EhVtbuE7iUtz+--