| Message ID | 20181017073207.13150-1-takahiro.akashi@linaro.org |
|---|---|
| Headers | show |
| Series | efi: make efi and bootmgr more usable | expand |
On 17.10.18 09:32, AKASHI Takahiro wrote: > This patch set is a collection of patches to enhance efi user interfaces > /commands. It will help improve user experience on efi boot and make it > more usable without edk2's shell utility. That's amazing, thanks a lot :) > Patch#1 to #4 are for efishell. > Patch#5 and #6 are for bootmgr. > > Let's see how it works: > => efishell boot add 1 SHELL mmc 0:1 /Shell.efi "" > => efishell boot add 2 HELLO mmc 0:1 /hello.efi "" > => efishell boot dump IMHO those 3 belong semantically to the "bootmgr" command. I can see how "bootefi bootmgr add 1 SHELL ..." is terrible UX. But then again it's not too much more to type than "efishell boot", right? ;) So at the end of the day, these should probably be => bootefi bootmgr add 1 SHELL mmc 0:1 /Shell.efi "" => bootefi bootmgr add 2 HELLO mmc 0:1 /hello.efi "" => bootefi bootmgr dump > Boot0001: > attributes: A-- (0x00000001) > label: SHELL > file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(1,MBR,0x086246ba,0x800,0x40000)/\\Shell.efi > data: > Boot0002: > attributes: A-- (0x00000001) > label: HELLO > file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(1,MBR,0x086246ba,0x800,0x40000)/\\hello.efi > data: > > => efishell boot order 1 2 > => efishell boot order Same thing here :). > 1: Boot0001: SHELL > 2: Boot0002: HELLO > > => bootefi bootmgr -2 > WARNING: booting without device tree > Booting: HELLO > ## Starting EFI application at 000000007db8b040 ... > Hello, world! > ## Application terminated, r = 0 > > => efishell setvar PlatformLang en <--- important! That one is slightly more complicated. How about we introduce a -e flag to all the env operations? Then this would become => setenv -e PlatformLang en and you could print only EFI variables using => printenv -e maybe one day we could then also just implement partial variable storage for UEFI variables: => saveenv -e which we could then reuse in the ExitBootServices() call to persist EFI variables? > => efishell bootmgr -1 or efishell bootmgr > > (shell ...) > > # The only drawback is that it can be confusing to type > "bootefi ..." and "efi(shell) boot ..." :) Yes :). Alex
On Wed, Oct 17, 2018 at 10:06:58AM +0200, Alexander Graf wrote: > > > On 17.10.18 09:32, AKASHI Takahiro wrote: > > This patch set is a collection of patches to enhance efi user interfaces > > /commands. It will help improve user experience on efi boot and make it > > more usable without edk2's shell utility. > > That's amazing, thanks a lot :) > > > Patch#1 to #4 are for efishell. > > Patch#5 and #6 are for bootmgr. > > > > Let's see how it works: > > => efishell boot add 1 SHELL mmc 0:1 /Shell.efi "" > > => efishell boot add 2 HELLO mmc 0:1 /hello.efi "" > > => efishell boot dump > > IMHO those 3 belong semantically to the "bootmgr" command. I can see how > "bootefi bootmgr add 1 SHELL ..." is terrible UX. But then again it's > not too much more to type than "efishell boot", right? ;) > > So at the end of the day, these should probably be > > => bootefi bootmgr add 1 SHELL mmc 0:1 /Shell.efi "" > => bootefi bootmgr add 2 HELLO mmc 0:1 /hello.efi "" > => bootefi bootmgr dump To be frank, I hesitate to agree with you for several reasons. * Boot manager is a sort of boot loader application while adding/showing BootXXXX variables can be part of more generic system utility. (My interface here mimics uefi shell's bcfg command with slightly different syntax.) * In future, we may want to have another sub command, "driver," to support driver loading, namely DriverOrder/DriverXXXX. * Anyhow, we need another command for "setvar"( and "dumpvar"). > > Boot0001: > > attributes: A-- (0x00000001) > > label: SHELL > > file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(1,MBR,0x086246ba,0x800,0x40000)/\\Shell.efi > > data: > > Boot0002: > > attributes: A-- (0x00000001) > > label: HELLO > > file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(1,MBR,0x086246ba,0x800,0x40000)/\\hello.efi > > data: > > > > => efishell boot order 1 2 > > => efishell boot order > > Same thing here :). > > > 1: Boot0001: SHELL > > 2: Boot0002: HELLO > > > > => bootefi bootmgr -2 > > WARNING: booting without device tree > > Booting: HELLO > > ## Starting EFI application at 000000007db8b040 ... > > Hello, world! > > ## Application terminated, r = 0 > > > > => efishell setvar PlatformLang en <--- important! > > That one is slightly more complicated. How about we introduce a -e flag > to all the env operations? Then this would become > > => setenv -e PlatformLang en > > and you could print only EFI variables using > > => printenv -e > > maybe one day we could then also just implement partial variable storage > for UEFI variables: > > => saveenv -e > > which we could then reuse in the ExitBootServices() call to persist EFI > variables? I didn't get your point. Can you please elaborate it? > > => efishell bootmgr -1 or efishell bootmgr > > > > (shell ...) > > > > # The only drawback is that it can be confusing to type > > "bootefi ..." and "efi(shell) boot ..." :) > > Yes :). The compromise I can imagine is that efishell be also aliased to bootefi so that we can do: => efi(shell) boot add 1 ... => efi(shell) bootmgr -1 ( in my current syntax) Yet we still maintain an old/boot*-style interface: => bootefi bootmgr or <appl addr> Thanks, -Takahiro Akashi > > Alex
On 18.10.18 07:24, AKASHI Takahiro wrote: > On Wed, Oct 17, 2018 at 10:06:58AM +0200, Alexander Graf wrote: >> >> >> On 17.10.18 09:32, AKASHI Takahiro wrote: >>> This patch set is a collection of patches to enhance efi user interfaces >>> /commands. It will help improve user experience on efi boot and make it >>> more usable without edk2's shell utility. >> >> That's amazing, thanks a lot :) >> >>> Patch#1 to #4 are for efishell. >>> Patch#5 and #6 are for bootmgr. >>> >>> Let's see how it works: >>> => efishell boot add 1 SHELL mmc 0:1 /Shell.efi "" >>> => efishell boot add 2 HELLO mmc 0:1 /hello.efi "" >>> => efishell boot dump >> >> IMHO those 3 belong semantically to the "bootmgr" command. I can see how >> "bootefi bootmgr add 1 SHELL ..." is terrible UX. But then again it's >> not too much more to type than "efishell boot", right? ;) >> >> So at the end of the day, these should probably be >> >> => bootefi bootmgr add 1 SHELL mmc 0:1 /Shell.efi "" >> => bootefi bootmgr add 2 HELLO mmc 0:1 /hello.efi "" >> => bootefi bootmgr dump > > To be frank, I hesitate to agree with you for several reasons. > * Boot manager is a sort of boot loader application while adding/showing > BootXXXX variables can be part of more generic system utility. > (My interface here mimics uefi shell's bcfg command with slightly > different syntax.) If you look at it from a U-Boot perspective, the only consumer of BootXXXX variables will realistically be bootmgr. So I don't quite see how setting BootXXXX variables doesn't belong to the same command line syntax? > * In future, we may want to have another sub command, "driver," to support > driver loading, namely DriverOrder/DriverXXXX. Hm. Who would read that order? When would it get parsed? Would that be part of the script (and thus a command again) or would we want to have them get loaded at an earlier stage? My current stance would be that driver loading would happen very similar to application loading: Using a command from a script / the shell. So the same command could again be responsible for changing the order and adding/removing driver entries. > * Anyhow, we need another command for "setvar"( and "dumpvar"). I would much much rather like to integrate with existing U-Boot commands (setenv, printenv, print) than invent new commands. The UEFI bits in U-Boot shouldn't be alien - they should just seamlessly integrate :). > >>> Boot0001: >>> attributes: A-- (0x00000001) >>> label: SHELL >>> file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(1,MBR,0x086246ba,0x800,0x40000)/\\Shell.efi >>> data: >>> Boot0002: >>> attributes: A-- (0x00000001) >>> label: HELLO >>> file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(1,MBR,0x086246ba,0x800,0x40000)/\\hello.efi >>> data: >>> >>> => efishell boot order 1 2 >>> => efishell boot order >> >> Same thing here :). >> >>> 1: Boot0001: SHELL >>> 2: Boot0002: HELLO >>> >>> => bootefi bootmgr -2 >>> WARNING: booting without device tree >>> Booting: HELLO >>> ## Starting EFI application at 000000007db8b040 ... >>> Hello, world! >>> ## Application terminated, r = 0 >>> >>> => efishell setvar PlatformLang en <--- important! >> >> That one is slightly more complicated. How about we introduce a -e flag >> to all the env operations? Then this would become >> >> => setenv -e PlatformLang en >> >> and you could print only EFI variables using >> >> => printenv -e >> >> maybe one day we could then also just implement partial variable storage >> for UEFI variables: >> >> => saveenv -e >> >> which we could then reuse in the ExitBootServices() call to persist EFI >> variables? > > I didn't get your point. Can you please elaborate it? Currently we have: => help printenv printenv - print environment variables Usage: printenv [-a] - print [all] values of all environment variables printenv name ... - print value of environment variable 'name' => help setenv setenv - set environment variables Usage: setenv [-f] name value ... - [forcibly] set environment variable 'name' to 'value ...' setenv [-f] name - [forcibly] delete environment variable 'name' What if we add UEFI variable awareness to those commands? So instead of => efishell setvar PlatformLang en you would get => setenv -e PlatformLang en The same would apply for printenv. Something like => printenv -e would just show UEFI variables with potential additional decoding? Or was your question on saveenv? The saveenv command is used to make the variable storage persistent. One problem we have with variable storage is that you want to only persist variables that are a) marked to persist b) UEFI variables So for example if you're in the middle of a script and you want to change the boot order, you want to say "please make the UEFI variable store persistent now", but you definitely do now want to make the U-Boot variable store persistent, because your variables are in the middle of your script execution with helper variables. That same problem also occurs with booting. When calling ExitBootServices() we ideally want to persist the UEFI variable storage (well, the non-temporary variables in it). But for that we currently don't have any logic implemented to differentiate between UEFI variables and U-Boot variables on save. > >>> => efishell bootmgr -1 or efishell bootmgr >>> >>> (shell ...) >>> >>> # The only drawback is that it can be confusing to type >>> "bootefi ..." and "efi(shell) boot ..." :) >> >> Yes :). > > The compromise I can imagine is that efishell be also aliased > to bootefi so that we can do: > => efi(shell) boot add 1 ... > => efi(shell) bootmgr -1 ( in my current syntax) > Yet we still maintain an old/boot*-style interface: > => bootefi bootmgr or <appl addr> In a nutshell, I want to have UEFI commands be first class U-Boot commands :). The whole fact that we have "bootefi bootmgr" is bad. It shouldn't have been a bootefi subcommand really :/. Now the really big question is what an alternative *good* naming scheme is. And there I'm not sure yet... Alex