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=-8.1 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,MAILING_LIST_MULTI, NICE_REPLY_A,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 3A65AC433F5 for ; Fri, 3 Sep 2021 16:24:20 +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 250B860F12 for ; Fri, 3 Sep 2021 16:24:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 250B860F12 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmx.de 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 5A5F4834F6; Fri, 3 Sep 2021 18:24:16 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=gmx.de 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; secure) header.d=gmx.net header.i=@gmx.net header.b="I2nv0r6K"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 8D28E834C0; Fri, 3 Sep 2021 18:24:14 +0200 (CEST) Received: from mout.gmx.net (mout.gmx.net [212.227.15.19]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 16678834C0 for ; Fri, 3 Sep 2021 18:24:10 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=xypron.glpk@gmx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1630686242; bh=hl/1GE/dut/cjwMRPZam8Jkc8D0JW/K/zFV/3vqRjbk=; h=X-UI-Sender-Class:Subject:To:References:From:Cc:Date:In-Reply-To; b=I2nv0r6K6GLWT9Ix9BBjRSPPgI5089Bu2nFqJO8tQf3KsuS8+5NDKJwMlB+heVwkh U7iuLPZndFJMZQQCmLJ8aofSoR3MOHXbNtBdWVYN5SzuGq2kFsZkNEkSjtwXnKWLC6 +cZmKibOoeh9BUecIL9Zcq85P3oYVbRDGq/UDgn0= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [192.168.123.55] ([88.152.144.157]) by mail.gmx.net (mrgmx004 [212.227.17.190]) with ESMTPSA (Nemesis) id 1Mr9Bk-1mivbw25B8-00oE8S; Fri, 03 Sep 2021 18:24:02 +0200 Subject: Re: [PATCH] RFC: Support an EFI-loader bootflow To: Simon Glass References: <20210828203521.111877-1-sjg@chromium.org> <20210831061436.GA69817@laputa> <20210903022739.GB47953@laputa> From: Heinrich Schuchardt Cc: AKASHI Takahiro , Steffen Jaeckel , Klaus Heinrich Kiwi , Ilias Apalodimas , Michal Simek , Tom Rini , Alexandru Gagniuc , U-Boot Mailing List , Masahisa Kojima , Bin Meng Message-ID: Date: Fri, 3 Sep 2021 18:23:57 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:oJkzC/da8e1LTNtFT7UTRyVIKDABICM/KKrEA/PMRWYHO5ekT5t QX93aTIiMz7itGc2QCDLjoef/Q1SBBPAnvJwbcRlDhVZ/DQd/3x8V2SIDhzyvZWq3ZqNsKW +cLrFtEzZlyVF6fNb0QvyipweQWnMeym3nldhCQoDwkZIKMjKKsmJsNSG/fLYsftrT0TrGL xHhsoLse6TajG/U4n6ulg== X-UI-Out-Filterresults: notjunk:1;V03:K0:CeqdBQ7UXcI=:1zxjuCT3eUjIr1ROvrzrEU TD2583pFCe0hlOGvV21xOLwx3EozABUb0KYR47nYfmXb6wEe7BsGI9uN4CWnVCTG7meOQT8/H AQNHjNXidHTxs/Kr4XMibUhz5DF9jQBMaFlcM3rtqhd7q3zCt9CvnxXcSlift04ZlXyjO7bV5 MPwSB3wnxRwe2gDKaInwlQCvQ3dYUbJvVPlH1cbaHBnSQeVtj28Ko2EfHAuCCNyA+Qu8MejMs VrguWZ82XTD2wSk09NP1TjugJvXTwuvkIzcXkiCKSekjDaOE63k7u0JCznrhf1PidN++23BgN o6fMmxfccYy1wo4Wis6MLZrlH3e+XFV3C5oA3uLPl1FQdbvAbv3J3eDg7T6FdTfusbDwzJa9c OiXmZWhFt0jeMX9kdakFyF9+Kd84UBKbvXCHHETTa+V3MeWFu17kzw5rugwTtKJAoexW5WJ5N +j8zL1LwVMYMnCNMndvIpm0fsVMhDXUOgMkC0NNGPH2j8ZiwZceJdCljKyFWK2a2/RRxL3R6g LztRIVCHK0JrRWlYntJfOUWtIJK/xNHkGFoErLiZ61F3xiVSfTEscZpqF+/gGqQ6khBZffylj aMkShNIpPo86Ticp0Guhae6Zw5CcHMw01ido72XB8nFXyODEwmINpBEL0qYTEJ/iC2BI1/C2Z xZUaRXChyMJQkz/WIPQUACAMhelJGok8XdVtSEMBZS4kgqOxwwGhy9RYeI8mQ1uGwWgsBZV8z klU9tXvjR+FtmUSWw6jutLk6T6fEgje773dVnj6txk4nPmZUT3Wzhp4emsM3h6+re35C1AMBD ZTUbtemMC881v9qu0AWs04HoS3og+vb2VUeUD9b1INI6Mu01s2SbVZbs13jR/ks/YFJ5N+irS THJAdW69cmG0XjjDlkSEKGi3gRQ1by9a0HblzkZuwJ2F075zCqG6qf5oZrdyhm1795DG53aHH gSG0qA/S8LtstP1+/1xcE++YNDcg4FFL2wHTi1EHK8/XmKQ8apTKIKqUHf1lMJ5vSvc5cxX2W RlLRLQk27TgU6cDbdHCB8DucPHyHlZ0j79BWL99i5n/kksvg2WJz7H+G7vDKh0X7kW55+4Mw8 nBHKO264KKytUvRU2tje0IdpY4HmqWOZsSlnuuD3gj+nvimGHlblc3rrg== 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 9/3/21 10:53 AM, Simon Glass wrote: > Hi Takahiro, > > On Thu, 2 Sept 2021 at 20:27, AKASHI Takahiro > wrote: >> >> Simon, >> >> On Thu, Sep 02, 2021 at 10:40:57AM -0600, Simon Glass wrote: >>> Hi Takahiro, >>> >>> On Tue, 31 Aug 2021 at 00:14, AKASHI Takahiro >>> wrote: >>>> >>>> Simon, >>>> >>>> On Sat, Aug 28, 2021 at 02:35:21PM -0600, Simon Glass wrote: >>>>> This is just a demonstration of how to support EFI loader using boot= flow. >>>>> Various things need cleaning up, not least that the naming needs to = be >>>>> finalised. I will deal with that in the v2 series. >>>>> >>>>> In order to support multiple methods of booting from the same device= , we >>>>> should probably separate out the different implementations (syslinux= , >>>>> EFI loader >>>> >>>> I still believe that we'd better add "removable media" support >>>> to UEFI boot manager first (and then probably call this functionality >> ^^^^^^^^^^^^^^^^^^^^^^^^^^ >> >>>> from bootflow?). >>>> >>>> I admit that, in this case, we will have an issue that we will not >>>> recognize any device which is plugged in dynamically after UEFI >>>> subsystem is initialized. But this issue will potentially exist >>>> even with your approach. >>> >>> That can be fixed by dropping the UEFI tables and using driver model >>> instead. I may have mentioned that :-) >> >> I'm afraid that you don't get my point above. >> >>>> >>>>> and soon bootmgr, >>>> >>>> What will you expect in UEFI boot manager case? >>>> Boot parameters (options) as well as the boot order are well defined >>>> by BootXXXX and BootOrder variables. How are they fit into your schem= e? >>> >>> I haven't looked at boot manager yet, but I can't imagine it >>> presenting an insurmountable challenge. >> >> I don't say it's challenging. >> Since you have not yet explained your idea about how to specify >> the *boot order* in your scheme, I wonder how "BootXXXX"/"BootOrder" >> be treated and honored. >> There might be a parallel world again. > > Well as I mentioned, I haven't looked at it yet. The original question > was how to do EFI LOADER and I did a patch to show that. > > Are we likely to see mixed-boot environments, that use distro boot for > some OSes and EFI for others? I hope not as it would be confusing. EFI > is the parallel world, as I see it. > > It should be easy enough for the 'bootmgr' bootflow to read the EFI > variables and select the correct ordering. As I understand it, EFI > does not support lazy boot, so it is acceptable to probe all the > devices before selecting one? > >> >>>> >>>> But anyway, we can use the following commands to run a specific >>>> boot flow in UEFI world: >>>> =3D> efidebug boot next 1(or whatever else); bootefi bootmgr >>> >>> OK. >>> >>> As you probably noticed I was trying to have bootflow connect directly >>> to the code that does the booting so that 'CONFIG_CMDLINE' can be >>> disabled (e.g. for security reasons) and the boot will still work. >> >> # Maybe, it sounds kinda chicken and egg. >> >> Even now, you can code this way :) >> >> efi_set_variable(u"BootNext", ..., u"Boot0001"); >> do_efibootmgr(); >> >> That's it. My concern is what I mentioned above. > > OK. But then you would need to export those functions. I think it > would be better to split up the logic a bit and move things out of the > cmd/ directory (at some point). > >> >> Just a note: >> In the current distro_bootcmd, UEFI boot manager is also called >> *every time* one of boot media in "boot_targets" is scanned/enumerated. >> But it will make little sense because the current boot manager only >> allows/requires users to specify both the boot device and the image fil= e >> path explicitly in a boot option, i.e. "BootXXXX" variable, and tries >> all the boot options in "BootOrder" until it successfully launches >> one of those images. > > Yes, is the idea of lazy boot entirely impossible? Or is it still > possible to do that to some extent, e.g. by scanning until you find > the first thing in the boot order? > >> >>>> >>>>> Chromium OS, Android, VBE) into pluggable >>>>> drivers and number them as we do with partitions. For now the sequen= ce >>>>> number is used to determine both the partition number and the >>>>> implementation to use. >>>>> >>>>> The same boot command is used as before ('bootflow scan -lb') so the= re is >>>>> no change to that. It can boot both Fedora 31 and 34, for example. >>>>> >>>>> Signed-off-by: Simon Glass >>>>> --- >>>>> See u-boot-dm/bmea for the tree containing this patch and the series >>>>> that it relies on: >>>>> >>>>> https://patchwork.ozlabs.org/project/uboot/list/?series=3D258654&= state=3D* >>>>> >>> >>> [..] >>> >>>>> +static int efiload_read_file(struct blk_desc *desc, int partnum, >>>>> + struct bootflow *bflow) >>>>> +{ >>>>> + const struct udevice *media_dev; >>>>> + int size =3D bflow->size; >>>>> + char devnum_str[9]; >>>>> + char dirname[200]; >>>>> + loff_t bytes_read; >>>>> + char *last_slash; >>>>> + ulong addr; >>>>> + char *buf; >>>>> + int ret; >>>>> + >>>>> + /* Sadly FS closes the file after fs_size() so we must redo th= is */ >>>>> + ret =3D fs_set_blk_dev_with_part(desc, partnum); >>>>> + if (ret) >>>>> + return log_msg_ret("set", ret); >>>>> + >>>>> + buf =3D malloc(size + 1); >>>>> + if (!buf) >>>>> + return log_msg_ret("buf", -ENOMEM); >>>>> + addr =3D map_to_sysmem(buf); >>>>> + >>>>> + ret =3D fs_read(bflow->fname, addr, 0, 0, &bytes_read); >>>>> + if (ret) { >>>>> + free(buf); >>>>> + return log_msg_ret("read", ret); >>>>> + } >>>>> + if (size !=3D bytes_read) >>>>> + return log_msg_ret("bread", -EINVAL); >>>>> + buf[size] =3D '\0'; >>>>> + bflow->state =3D BOOTFLOWST_LOADED; >>>>> + bflow->buf =3D buf; >>>>> + >>>>> + /* >>>>> + * This is a horrible hack to tell EFI about this boot device.= Once we >>>>> + * unify EFI with the rest of U-Boot we can clean this up. The= same hack >>>>> + * exists in multiple places, e.g. in the fs, tftp and load co= mmands. >>>> >>>> Which part do you call a "horrible hack"? efi_set_bootdev()? >>>> In fact, there are a couple of reason why we need to call this functi= on: >>>> 1. to remember a device to create a dummy device path for the loaded >>>> image later, >>>> 2. to remember a size of loaded image which is used for sanity check = and >>>> image authentication later, >>>> 3. to avoid those parameters being remembered accidentally by "loadin= g" dtb >>>> and/or other binaries than the image itself, >>>> >>>> I hope that (1) and (2) will be avoidable if we modify the current >>>> implementation (and bootefi syntax), and then we won't need (3). >>> >>> Yes thank you...I do understand why it is needed now, but it is >>> basically due to the the fat that EFI has its own driver structures. >>> Once we stop those, it will go away. >> >> Here, my point is, even under the current implementation, we will >> be able to eliminate efi_set_bootdev() with some tweaks. >> >> In other words, even you could integrate UEFI into the device model, >> the issue (2), for example, would still remain unsolved. >> In case of (2), we use the *size* information for sanity check against >> image's header information as well as calculating a hash value for >> UEFI secure boot when efi_load_image() is called. Even if the integrati= on >> is done, we need to pass on the size information to "bootefi " >> command implicitly or explicitly. > > If you look at 'struct bootflow' you can see that it stores the size. > It can easily store other info as needed by EFI. So I don't think we > need that hack in the end, when things are using bootflow. > >> >>>> >>>>> + * Once we can clean up the EFI code to make proper use of dri= ver model, >>>>> + * this can go away. >>>> >>>> My point is, however, that this kind of cleanup is irrelevant to >>>> whether we use driver model or not. >>> >>> Are you sure? Without driver model how are you going to reference a >>> udevice? If not that, how are you going to reference a device? The >>> tables in the UEFI implementation were specifically added to avoid >>> relying on driver model. It is a crying shame that I did not push back >>> harder on this at the time. >> >> I hope you will get my point in the previous comment now. > > Well yes I do, but I don't understand why there is such resistance to > sorting out the EFI implementation? It is quite a mess at the moment. > I think the first step is to drop the non-DM code, but the second is > to drop the parallel tables that EFI keeps about each device. Concerning these "parallel" tables. Please, have a look at the UEFI spec to understand what a handle, a protocol interface, and an event are. I also gave as short overview in https://archive.fosdem.org/2020/schedule/event/firmware_duwu/ starting at slide 10 or 08:00 of the video. Handles may refer to devices, to drivers or anything else. Handles are both to be created by U-Boot and by EFI binaries that we execute. On these handles protocol interfaces can be installed. These may be those defined in the UEFI spec or something completely independent. The protocol interfaces contain pointers to functions and additional data. Further objects are events. These may contain references to an event handler function that is called when the event is triggered. None of the above objects matches one to one to objects in the driver model. But we can use some integration: When a U-Boot device is probed we must create the UEFI handle and install the expected protocols on it. When a U-Boot device is removed we must destroy the handle. There are certain conditions under which a protocol must not be removed from a handle and the handle cannot be destroyed. In this case removing a device should not be possible. Best regards Heinrich