On 08.01.19 08:29, AKASHI Takahiro wrote: > 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 <takahiro.aka...@linaro.org> >>>>>>>> >>>>>>>> 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.
That definitely works for me as well, yes. Alex _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
