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 39081C433FE for ; Thu, 28 Oct 2021 22:22:38 +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 20711610E5 for ; Thu, 28 Oct 2021 22:22:36 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 20711610E5 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org 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 0BC1C83116; Fri, 29 Oct 2021 00:22:33 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org 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=chromium.org header.i=@chromium.org header.b="O7mlyKKp"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 8E8F583116; Fri, 29 Oct 2021 00:22:30 +0200 (CEST) Received: from mail-ua1-x935.google.com (mail-ua1-x935.google.com [IPv6:2607:f8b0:4864:20::935]) (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 0397882F33 for ; Fri, 29 Oct 2021 00:22:24 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-ua1-x935.google.com with SMTP id q13so14540710uaq.2 for ; Thu, 28 Oct 2021 15:22:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=oym1yl7T7hKjKm2WhU14fiYQG9qSYD+FL4esllVgaJM=; b=O7mlyKKpledImzd+t1UphZmwLPHiAG1PwUa1T6z8KXTs+8sguysqcH5/RNdvGtFBq/ TUQFEuTuur9o2bXbx3Qr5cR88IEbPGzJg6XeDQZKH9P0165MnwawcEOO8cegqAA6+C0Z FZaB4K2BIGejVSkENSd8SBHVNlB6+Z/5t6qQc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=oym1yl7T7hKjKm2WhU14fiYQG9qSYD+FL4esllVgaJM=; b=hOp0tyCKdb2bhosF0xPb9SHId97yVrlhBQKH195y9fgpe5iS7NH2Y0qzJzS8WRoYzY qKqlK56l57Wc04JDpn4jHlhwskbB1bVbdXopX6rbQ6aBH4idc8KwbWwjd5lBDVPcLXxY ASECwni2ZGZmeeL56ZeJHcwpCF6cP4hiI1GQMqHlQFfziagcP3dgAw4xX4WESISoP14F FG/Vqf+cwus/RS7gbAd1+60dfb6ZQvKtuomVt3j507usj9IFoMTMJyzrs9ZpOLvsP+7m ejCJwIFkSAWvTeF274R4IntuStvKsjC4Ytw0BUKIeDXNkb035eZJ8JWmslYi6aux0EnO yl/A== X-Gm-Message-State: AOAM5333XYjxqYN1/YRYTw4uq9sAUelTZf7ZlmU21VjNGwwPSrrQ3ILB dlvzk58rCQNzfmZwMN0/fC0+Vx0gC4mCqdFvhhxAPA== X-Google-Smtp-Source: ABdhPJyU83VDA4AKKRoNo4D1owc/g83bFZHGuthkzljeFele4GbKDZTuTuUskZNqJUH9Ilxz6qcMjFpD8QzYMEMdY6E= X-Received: by 2002:a67:3087:: with SMTP id w129mr7679743vsw.15.1635459743458; Thu, 28 Oct 2021 15:22:23 -0700 (PDT) MIME-Version: 1.0 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> In-Reply-To: <20211028191903.GK8284@bill-the-cat> From: Simon Glass Date: Thu, 28 Oct 2021 16:22:11 -0600 Message-ID: Subject: Re: [PATCH v2 00/41] Initial implementation of standard boot To: Tom Rini Cc: U-Boot Mailing List , Michal Simek , Heinrich Schuchardt , Ilias Apalodimas , Daniel Schwierzeck , Steffen Jaeckel , =?UTF-8?B?TWFyZWsgQmVow7pu?= , Lukas Auer , Dennis Gilmore , Jaehoon Chung , Marek Vasut , Masahiro Yamada , Pavel Herrmann , Peng Fan , Stephen Warren , Stephen Warren Content-Type: text/plain; charset="UTF-8" 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 Hi Tom, 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 wrote: > > > > > > > > > > > > > > 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 other customisation. > > > > > > > > This is called 'standard boot' since it provides a standard 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 bootflows (owned by > > > > > > > > U-Boot) > > > > > > > > - bootflow - a description of how to boot (owned by the distro) > > > > > > > > > > > > > > > > This series provides an implementation of these, enabled to scan for > > > > > > > > bootflows from MMC, USB and Ethernet. It supports the existing 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, this) 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 parts of the > > > > > > > series to clean up / prepare, it starts at "bootstd: Add the 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 the > > > > > > commands are disabled or simplified, e.g. to only support 'bootflow > > > > > > scan -b'. At the moment it enables all dev features. > > > > > > > > > > OK, for the next go-round yes, please show what a typical enablement > > > > > 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 feedback > > > > 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 environment 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 90KB, 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 size > > > > > comparison is to. At the end of the day, and looking towards non-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 can 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 mainly > > > > 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, and > > > 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-board, or > > > > > it's using the generic distro boot framework. That framework is what to > > > > > further build on or perhaps make more readily simplified (for example, > > > > > 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 EFI, > > > > 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 boot > > > > 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 present 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 allocatoin > > > > automatically. Perhaps we should use malloc() more, or use LMB more > > > > proactively. > > > > > > Well, in that for Linux, arm64 the Image format has a header that lets > > > 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 verified > > > > boot, it makes it possible to sensibly deal with A/B;recovery, whereas > > > > at present that is all just scripting, certainly not handled by distro > > > > 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 Mender > > > and RAUC and swupdate specific bootflow commands is a step in the right > > > 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. 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. Now it's seems we need to stick with that, or move to EFI bootmgr? Is the new env framework agreed? I'd love to see some reviews from Wolfgang. Regards, Simon