From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Tue, 8 Jan 2019 16:29:30 +0900 Subject: [U-Boot] [PATCH v3 8/8] cmd: env: add "-e" option for handling UEFI variables In-Reply-To: <20190107074711.GH9033@linaro.org> References: <20181218050510.20308-1-takahiro.akashi@linaro.org> <20181218050510.20308-9-takahiro.akashi@linaro.org> <20181219014915.GJ14562@linaro.org> <7e32d99f-967b-038a-e6b5-7357cb1fa972@suse.de> <20181225084451.GD14405@linaro.org> <4da7abe8-2475-7a1f-0aa9-b49f67ea56dd@suse.de> <20190107074711.GH9033@linaro.org> Message-ID: <20190108072929.GL9033@linaro.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Mon, Jan 07, 2019 at 04:47:13PM +0900, AKASHI Takahiro wrote: > Heinrich, > > On Wed, Dec 26, 2018 at 10:20:32PM +0100, Alexander Graf wrote: > > > > > > On 25.12.18 09:44, AKASHI Takahiro wrote: > > > On Sun, Dec 23, 2018 at 02:56:40AM +0100, Alexander Graf wrote: > > >> > > >> > > >> On 19.12.18 13:23, Heinrich Schuchardt wrote: > > >>> On 12/19/18 2:49 AM, AKASHI Takahiro wrote: > > >>>> Heinrich, > > >>>> > > >>>> On Tue, Dec 18, 2018 at 07:07:02AM +0100, Heinrich Schuchardt wrote: > > >>>>> On 12/18/18 6:05 AM, AKASHI Takahiro wrote: > > >>>>>> "env [print|set] -e" allows for handling uefi variables without > > >>>>>> knowing details about mapping to corresponding u-boot variables. > > >>>>>> > > >>>>>> Signed-off-by: AKASHI Takahiro > > >>>>> > > >>>>> Hello Takahiro, > > >>>>> > > >>>>> in several patch series you are implementing multiple interactive > > >>>>> commands that concern > > >>>>> > > >>>>> - handling of EFI variables > > >>>>> - executing EFI binaries > > >>>>> - managing boot sequence > > >>>>> > > >>>>> I very much appreciate your effort to provide an independent UEFI shell > > >>>>> implementation. What I am worried about is that your current patches > > >>>>> make it part of the monolithic U-Boot binary. > > >>>> > > >>>> First of all, in v3, CONFIG_CMD_EFISHELL was introduced after Alex's > > >>>> comment on v2. So you can disable efishell command if you don't want it. > > >>>> > > >>>> Are you still worried? > > >>>> > > >>>>> This design has multiple drawbacks: > > >>>>> > > >>>>> The memory size available for U-Boot is very limited for many devices. > > >>>>> We already had to disable EFI_LOADER for some boards due to this > > >>>>> limitations. Hence we want to keep everything out of the U-Boot binary > > >>>>> that does not serve the primary goal of loading and executing the next > > >>>>> binary. > > >>>> > > >>>> I don't know your point here. If EFI_LOADER is disabled, efishell > > >>>> will never be compiled in. > > >>>> > > >>>>> The UEFI forum has published a UEFI Shell specification which is very > > >>>>> extensive. We still have a lot of deficiencies in U-Boot's UEFI API > > >>>>> implementation. By merging in parts of an UEFI shell implementation our > > >>>>> project looses focus. > > >>>> > > >>>> What is "our project?" What is "focus?" > > >>>> I'm just asking as I want to share that information with you. > > >>>> > > >>>>> There is an EDK2 implementation of said > > >>>>> specification. If we fix the remaining bugs of the EFI API > > >>>>> implementation in U-Boot we could simply run the EDK2 shell which > > >>>>> provides all that is needed for interactive work. > > >>>>> > > >>>>> With you monolithic approach your UEFI shell implementation can neither > > >>>>> be used with other UEFI API implementations than U-Boot nor can it be > > >>>>> tested against other API implementations. > > >>>> > > >>>> Let me explain my stance. > > >>>> My efishell is basically something like a pursuit as well as > > >>>> a debug/test tool which was and is still quite useful for me. > > >>>> Without it, I would have completed (most of) my efi-related work so far. > > >>>> So I believe that it will also be useful for other people who may want > > >>>> to get involved and play with u-boot's efi environment. > > >>> > > >>> On SD-Cards U-Boot is installed between the MBR and the first partition. > > >>> On other devices it is put into a very small ROM. Both ways the maximum > > >>> size is rather limited. > > >>> > > >>> U-Boot provides all that is needed to load and execute an EFI binary. So > > >>> you can put your efishell as file into the EFI partition like you would > > >>> install the EDK2 shell. > > >>> > > >>> The only hardshift this approach brings is that you have to implement > > >>> your own printf because UEFI does not offer formatted output. But this > > >>> can be copied from lib/efi_selftest/efi_selftest_console.c. > > >>> > > >>> The same decision I took for booting from iSCSI. I did not try to put an > > >>> iSCSI driver into U-Boot instead I use iPXE as an executable that is > > >>> loaded from the EFI partition. > > >>> > > >>>> > > >>>> I have never intended to fully implement a shell which is to be compliant > > >>>> with UEFI specification while I'm trying to mimick some command > > >>>> interfaces for convenience. UEFI shell, as you know, provides plenty > > >>>> of "protocols" on which some UEFI applications, including UEFI SCT, > > >>>> reply. I will never implement it with my efishell. > > >>>> > > >>>> I hope that my efishell is a quick and easy way of learning more about > > >>>> u-boot's uefi environment. I will be even happier if more people > > >>>> get involved there. > > >>>> > > >>>>> Due to these considerations I suggest that you build your UEFI shell > > >>>>> implementation as a separate UEFI binary (like helloworld.efi). You may > > >>>>> offer an embedding of the binary (like the bootefi hello command) into > > >>>>> the finally linked U-Boot binary via a configuration variable. Please, > > >>>>> put the shell implementation into a separate directory. You may want to > > >>>>> designate yourself as maintainer (in file MAINTAINERS). > > >>>> > > >>>> Yeah, your suggestion is reasonable and I have thought of it before. > > >>>> There are, however, several reasons that I haven't done so; particularly, > > >>>> efishell is implemented not only with boottime services but also > > >>>> other helper functions, say, from device path utilities. Exporting them > > >>>> as libraries is possible but I don't think that it would be so valuable. > > >>>> > > >>>> Even if efishell is a separate application, it will not contribute to > > >>>> reduce the total footprint if it is embedded along with u-boot binary. > > >>> > > >>> That is why CONFIG_CMD_BOOTEFI_HELLO - which embeds helloworld.efi into > > >>> the U-Boot binary - is default no. Same I would do for efishell.efi. > > >> > > >> One big drawback with a separate binary is the missing command line > > >> integration. It becomes quite awkward to execute efi debug commands > > >> then, since you'll have to run them through a special bootefi subcommand. > > >> > > >> If you really want to have a "uefi shell", I think the sanest option is > > >> to just provide a built-in copy of the edk2 uefi shell, similar to the > > >> hello world binary. The big benefit of this patch set however, is not > > >> that we get a shell - it's that we get quick and tiny debug > > >> introspectability into efi_loader data structures. > > > > > > And my command can be used for simple testing. > > > > Exactly, that would give us the best of both worlds. > > > > > > > >> I think the biggest problem here really is the name of the code. Why > > >> don't we just call it "debugefi"? It would be default N except for debug > > >> targets (just like bootefi_hello). > > >> > > >> That way when someone wants to just quickly introspect internal data > > >> structures, they can. I also hope that if the name contains debug, > > >> nobody will expect command line compatibility going forward, so we have > > >> much more freedom to change internals (which is my biggest concern). > > >> > > >> So in my opinion, if you fix the 2 other comments from Heinrich and > > >> rename everything from "efishell" to "debugefi" (so it aligns with > > >> bootefi), we should be good. > > > > > > If Heinrich agrees, I will fix the name although I'm not a super fan > > > of this new name :) > > > > Well, feel free to come up with a new one, but it definitely must have a > > ring to it that it's a tiny, debug only feature that is not intended for > > normal use ;). > > Do you have any idea/preference about this command's name? I prefer efidebug/efidbg or efitool so that we can use a shorthand name, efi, at command line in most cases. -Takahiro Akashi > -Takahiro Akashi > > > For normal operation, we need to come up with mechanisms that integrate > > much deeper into U-Boot's generic command structure. > > > > > > Alex