[U-Boot] [PATCH v2 05/14] cmd: efishell: add devices command

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Wed, Dec 26, 2018 at 09:00:42AM +0100, Alexander Graf wrote:
> 
> 
> On 25.12.18 13:00, AKASHI Takahiro wrote:
> > On Sun, Dec 23, 2018 at 04:11:14AM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 03.12.18 08:02, AKASHI Takahiro wrote:
> >>> On Mon, Dec 03, 2018 at 12:46:20AM +0100, Alexander Graf wrote:
> >>>>
> >>>>
> >>>> On 05.11.18 10:06, AKASHI Takahiro wrote:
> >>>>> "devices" command prints all the uefi variables on the system.
> >>>>> => efishell devices
> >>>>> Device Name
> >>>>> ============================================
> >>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> >>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> >>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
> >>>>> 				HD(2,MBR,0x086246ba,0x40800,0x3f800)
> >>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
> >>>>> 				HD(1,MBR,0x086246ba,0x800,0x40000)
> >>>>>
> >>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >>>>> ---
> >>>>>  cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-
> >>>>>  1 file changed, 87 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/cmd/efishell.c b/cmd/efishell.c
> >>>>> index abc8216c7bd6..f4fa3fdf28a7 100644
> >>>>> --- a/cmd/efishell.c
> >>>>> +++ b/cmd/efishell.c
> >>>>> @@ -21,6 +21,8 @@
> >>>>>  
> >>>>>  DECLARE_GLOBAL_DATA_PTR;
> >>>>>  
> >>>>> +static const struct efi_boot_services *bs;
> >>>>
> >>>> Why do you need a local copy of this?
> >>>
> >>> Good point. It's because I followed the way boot manager does :)
> >>>
> >>> I think that it would be good to do so since either boot manager or
> >>> efishell should ultimately be an independent efi application
> >>> in its nature.
> >>>
> >>> What do you think?
> > 
> > Back to your original comment: Why do you need a local copy of this?
> > 
> > Do you think we should use systab.boottime,runtime instead?
> 
> Sure, that's one thing. Or you could make it a local variable. Or you
> could even do a #define in the file. But that static const here will
> most likely waste space in .bss and does not take into account when
> someone patches the systab.

OK, I will add a macro definition.

-Takahiro Akashi

> >> As mentioned in the other email thread, I think that we should
> >> definitely evaluate to add the edk2 shell as built-in option.
> > 
> > Do you expect that I will add a new config, say, CONFIG_EFI_SHELL_PATH? 
> 
> Doesn't have to be you, but it might be useful to have something like
> that, yes.
> 
> CONFIG_CMD_BOOTEFI_SHELL=y
> CONFIG_CMD_BOOTEFI_SHELL_PATH=shell.efi
> 
> which then would work the same as CONFIG_CMD_BOOTEFI_HELLO, but simply
> execute a precompiled version of the UEFI shell. IIRC the UEFI shell
> also supports passing arguments via the command line (load_options ->
> "bootargs" variable).
> 
> So with that you should be able to run
> 
> U-Boot# setenv bootargs memmap; bootefi shell
> 
> and get a list of the UEFI memory map.
> 
> 
> Alex