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=-10.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 B2C24C433F5 for ; Fri, 3 Sep 2021 02:27: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 C071160FDC for ; Fri, 3 Sep 2021 02:27:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C071160FDC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.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 4C01982F10; Fri, 3 Sep 2021 04:27:53 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org 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=linaro.org header.i=@linaro.org header.b="mPxjb7FS"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 8D249832A0; Fri, 3 Sep 2021 04:27:51 +0200 (CEST) Received: from mail-pf1-x42d.google.com (mail-pf1-x42d.google.com [IPv6:2607:f8b0:4864:20::42d]) (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 E7B8282F00 for ; Fri, 3 Sep 2021 04:27:46 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=takahiro.akashi@linaro.org Received: by mail-pf1-x42d.google.com with SMTP id r13so3147504pff.7 for ; Thu, 02 Sep 2021 19:27:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=7Su36J5dDmQXvwnAF95Of+eUgFU5G5lSEK9d6L2VVyw=; b=mPxjb7FS9PgyhlidS5Vpo7bClUx1RG8SdK61ybw0zYrpuPLSnZPmK0ADZB+8+1+BiX DawyXsiyIjMdUAq7Ns4dBhGogAhY/AV4mQ8CxLmIiO1WWpsHfHMqf8UQrgZhXeEhU5iJ bLTYdMC5OJl5gKgttsOKJF0H5Gngw677fwnH7VNt/j+PJeGZpgebQA3VdXLyd6vYfveB 1HcYYG+sbKDGDtxD2dFuA0NtlRdbWScVOH0oLB1ln3eIsZ7Hda4QRVhKy1mAaTxsxdMe xnDJoUNqAiZ+7vcMVQf6YmlHZYejE8VSfeq/h5uiMBZlvnr06wFEB5bmC3JF+qdz5YzB 3hvg== 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 :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=7Su36J5dDmQXvwnAF95Of+eUgFU5G5lSEK9d6L2VVyw=; b=ZZOo0lHPDFmGxHYFPE7DUgd5qCDCP6JnnDKkk3VDWGibH+IsacrwM3mkfzjpFMcbFs yBfhySZWK+dOXmaNys2lBB/BcCMfMV/mK/06BOyH4SXpat7RoxsHjINTlBg9QS8/xdkZ QSI2OvpHdGuYdcC8R20LzFCKtA8hUNk4UyMWDBzGPKSE+uBBRBhi0Req+IfAv7ZQaGel hAvnNr8UrC58cbfgV0PyRRRh0eCuyR33bsAlaVMM/FDM3pTN2Jj40k6N2Tf3kO5C8UYO DNHSqlCc798OR+1qW/nd+Il/hgBm+90eq2W5DUvxd19Uc0vEBkthdOsUXYHnQestdlpR WRqA== X-Gm-Message-State: AOAM531NtaHnESK0lQDlww/nd9lFGmv1/qPpUzGzLCdjreJTJYOu3DhG CTFVIQa3ETMK4r57mmM2QEPLDg== X-Google-Smtp-Source: ABdhPJz03uA0GF5gYBaf3LBSfuq5jLa+Wie7Fz7/x358y0b+WXGW/ZiHxJ29BwYlWrUhazXA0pMJ9w== X-Received: by 2002:a62:a20b:0:b0:3f8:f5db:3256 with SMTP id m11-20020a62a20b000000b003f8f5db3256mr1399237pff.10.1630636064910; Thu, 02 Sep 2021 19:27:44 -0700 (PDT) Received: from laputa (p784a44f4.tkyea130.ap.so-net.ne.jp. [120.74.68.244]) by smtp.gmail.com with ESMTPSA id 132sm3370837pfy.190.2021.09.02.19.27.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Sep 2021 19:27:44 -0700 (PDT) Date: Fri, 3 Sep 2021 11:27:39 +0900 From: AKASHI Takahiro To: Simon Glass Cc: U-Boot Mailing List , Tom Rini , Heinrich Schuchardt , Ilias Apalodimas , Michal Simek , Alexandru Gagniuc , Bin Meng , Klaus Heinrich Kiwi , Masahisa Kojima , Steffen Jaeckel Subject: Re: [PATCH] RFC: Support an EFI-loader bootflow Message-ID: <20210903022739.GB47953@laputa> Mail-Followup-To: AKASHI Takahiro , Simon Glass , U-Boot Mailing List , Tom Rini , Heinrich Schuchardt , Ilias Apalodimas , Michal Simek , Alexandru Gagniuc , Bin Meng , Klaus Heinrich Kiwi , Masahisa Kojima , Steffen Jaeckel References: <20210828203521.111877-1-sjg@chromium.org> <20210831061436.GA69817@laputa> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 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 bootflow. > > > 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 scheme? > > 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. > > > > But anyway, we can use the following commands to run a specific > > boot flow in UEFI world: > > => 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. 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 file 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. > > > > > Chromium OS, Android, VBE) into pluggable > > > drivers and number them as we do with partitions. For now the sequence > > > 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 there 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=258654&state=* > > > > > [..] > > > > +static int efiload_read_file(struct blk_desc *desc, int partnum, > > > + struct bootflow *bflow) > > > +{ > > > + const struct udevice *media_dev; > > > + int size = 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 this */ > > > + ret = fs_set_blk_dev_with_part(desc, partnum); > > > + if (ret) > > > + return log_msg_ret("set", ret); > > > + > > > + buf = malloc(size + 1); > > > + if (!buf) > > > + return log_msg_ret("buf", -ENOMEM); > > > + addr = map_to_sysmem(buf); > > > + > > > + ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read); > > > + if (ret) { > > > + free(buf); > > > + return log_msg_ret("read", ret); > > > + } > > > + if (size != bytes_read) > > > + return log_msg_ret("bread", -EINVAL); > > > + buf[size] = '\0'; > > > + bflow->state = BOOTFLOWST_LOADED; > > > + bflow->buf = 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 commands. > > > > 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 function: > > 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 "loading" 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 integration is done, we need to pass on the size information to "bootefi " command implicitly or explicitly. > > > > > + * Once we can clean up the EFI code to make proper use of driver 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. Thanks, -Takahiro Akashi > Regards, > Simon