On Thu, Jan 14, 2021 at 02:36:23PM +0200, Ilias Apalodimas wrote: > On Thu, Jan 14, 2021 at 01:07:42PM +0100, Heinrich Schuchardt wrote: > > On 14.01.21 10:55, Ilias Apalodimas wrote: > > > On Thu, Jan 14, 2021 at 02:11:33PM +0900, AKASHI Takahiro wrote: > > >> Ilias, > > >> > > >> On Wed, Jan 13, 2021 at 01:11:49PM +0200, Ilias Apalodimas wrote: > > >>> The UEFI spec allow a packed array of UEFI device paths in the > > >>> FilePathList[] of an EFI_LOAD_OPTION. The first file path must > > >>> describe the laoded image but the rest are OS specific. > > >>> Previous patches parse the device path and try to use the second > > >>> member of the array as an initrd. So let's modify efidebug slightly > > >>> and install the second file described in the command line as the > > >>> initrd device path. > > >> > > >> I have a concern about your proposed command line syntax. > > >> It takes a lot of parameters as a whole which makes it > > >> hard to understand it at a glance, easily overflowing > > >> the width of terminal window. > > >> > > >> It will even get worse if we want to take dtb as a third > > >> device path, and what if we want to specify dtb, but not initrd? > > >> > > >> Moreover, if we want to add support for non-linux executabes which > > >> utilize extra device paths (neither initrd nor dtb) in a boot > > >> load option, the syntax will be problematic. > > >> > > > > > > Maybe we should add explicit commands in efidebug then? > > > Something like: > > > efidebug initrd add 0002 virtio 1 initrd_file > > > efidebug dtb add 0002 virtio 1 dtb > > > > Why "add"? If no file is provided, we could empty the device path. > > > > All other boot related subcommands start with efidebug boot. So how about: > > > > efidebug boot add 00B1 'Linux' mmc 0:1 vmlinux-5.99 UUID=1234 > > efidebug boot initrd 00B1 mmc 0:1 initrd-5.99 > > efidebug boot dtb mmc 00B1 0:1 /dtbs/5.99/vendor/board.dtb > > efidebug boot order 00B1
I have had the same idea in my mind. > > What will happen if you pass the following command next: > > > > efidebug boot add 00B1 'Linux' mmc 0:1 vmlinux-5.99.1 UUID=1234 > > > > Will initrd and and dtb be kept? > > That's one of the reasons the RFC had everything in one line. You enforce less > options to the user, which imho is always good :). > > > > > What will happen if you start with dtb or with initrd and not with add? > > IMHO we should return the usage in that case and explicitly request the boot > option to be set first. In principle the dtb and initrd are not load options. > They are appended DTB instances in an existing load option. If you dont do > 'add' first, there's no device path to begin with. > > So to answer your first question, in order to simplify the command line, I'd > suggest we overwrite the load_option when adding a new image. It's unlikely > the initrd will remain as is anyway, the dtb might remain the same, but it's > not worth the effort and complexity. You'll need a substantial amount of code > parsing the DP instances and placing elements in top/middle/bottom etc. > > > > > A device path with kernel.efi, no initrd, and dtb would have an initrd > > device path with only an end node. Should we install a LOAD FILE2 > > protocol in this case to disallow initrd on the command line? > > If you do that the user won't be able to load an initrd, unless a fixup is > applied directly on the DTB. I'd avoid that, I would simply defer from > installing the protocol overall if an end node is discovered on the initrd. > > > > > The boot options are relevant for all users, while the rest of efidebug > > is more developers oriented. Should we separate the boot related > > commands from the rest of efidebug and build with the new command by > > default? > > > > bootopt add 00B1 'Linux' mmc 0:1 vmlinux-5.99 UUID=1234 > > bootopt initrd 00B1 mmc 0:1 initrd-5.99 > > bootopt dtb mmc 00B1 0:1 /dtbs/5.99/vendor/board.dtb > > bootopt order 00B1 My long-standing hope was to step up the stage of "efidebug" from "debug" to "normal" command. Initially, I intended to implement this command as an alternative of EDK2's efishell (or poorman's shell), so most of the sub commands have a similar syntax and output formats to efisehell defined in EFI specification. But this idea was rejected by Alex :) I agree that "boot" sub command is a good candidate of a standalone command. Or alternatively, we may want to add options to bootefi command. > +1 on that, efidebug feels a bit weird for that. > This can go in on a different patchset I guess? > It would merely mean c/p the code in a different file and > edit a few makesfiles? Anyhow, when you go this way, please don't forget to update pytest scripts since efidebug is also used in secure boot/capsule update tests. -Takahiro Akashi > Cheers > /Ilias > > > > Best regards > > > > Heinrich > > > > > > > > That would untangle the do_efi_boot_add() function, make our lives easier > > > on > > > adding things like 'kernel <no initrd> valid dtb' and should be much > > > easier > > > to use. The user will just have to make sure the boot order numbers match > > > when > > > adding files > > > > > > [...] > > > > > > Cheers > > > /Ilias > > > > >
