[PATCH 5/6] efidebug: add multiple device path instances on Boot####

Ilias Apalodimas ilias.apalodimas at linaro.org
Fri Mar 12 08:19:03 CET 2021


Akashi-san,

[...]
> > > > Then you have to deserialize the existing stored device path in Boot####,
> > > > append the initrd and serialize it again (and last time I checked this is not
> > > > as easy as it sounds).
> > > 
> > > If we take the format like:
> > >   kernel path,end(0xff),
> > >   VenMedia(INITRD),initrd1 path,end(0xff),
> > >   VenMedia(INITRD),initrd2 path,end(0xff),
> > > 
> > > all that we have to do is
> > > - deserialize the boot option variable,
> > > - append initrd device path to a list (after kernel device path),
> > > - serialize all the option together,
> > 
> > Sure but the serialize/deserialize is not as easy as it sounds and requires
> > code as well for the optional data etc.
> 
> I don't still understand why it's not so easy.
> Because I'm the author of that function?

You have to deserialize the load option, split it on the device path end node,
attach a new device path, and append the optional data, while doing the
necessary utf8/16 conversions at the same time. The alternative is do nothing
at all and serialize it in one go.  It's not hard, but it's not something it 
would be my first priority to code. Especially if 99.9% of the use cases will 
have a single initrd.

> 
> > Again I am not saying it's not doable, or even more elegant.  I am saying the
> > extra code doesn't seemt to worth the effort right now. Especially since we
> > support a *single* initrd on the loading anyway and no dtbs.
> 
> Better user interface would pay the efforts of implementation.
> 
> > > 
> > > Appending is quite simple, isn't it?
> > > (because we will not have to parse a device path list.)
> > > 
> > 
> > Yes and i've mentioned most of this on the mailing list. 
> > We did choose the other format though...
> 
> Simply, who objected it?
> I remember that Heinrich has a similar idea.
> So who else?

Again that proposal is part of my mail as well. 
Heinrich, Samer and I agreed this is the format we want to use since
since you dont need to replicate the VenMEdia node for each array
element.

> 
> > > > Also what happens if you edit a Boot#### option and that option has an initrd? 
> > > If I were you,
> > > 
> > > > You have to pick up the existing initrd and move it over? Or do we silently 
> > > > delete it?
> > > > Something like:
> > > > efidebug boot add 0001 
> > > > efidebug boot add-initrd 0001 
> > > > efidebug boot add 0001
> > > 
> > > even with the current implementation,
> > > > efidebug boot add 0001 
> > > > efidebug boot add 0001 
> > > those sequence will overwrite the existing variable and,
> > > deeleting initrd by the second "efidebug boot add" would make sense.
> > 
> > Yea but that feels 'natural' because it's a single command. Which is also the
> > case with the current code. In multiple commands you wouldn't expect the
> > initrd to away, unless you knew it was stored in a Boot option.
> 
> Well, if it's your concern, we can print a warning, say
>   You're trying to rewrite BootXXXX variable (you will lost the existing data).
>   Are you sure [y/n]?
> 
> But I don't think it's even worth doing so
> because you can simply type "Ctrl-p" twice at the command line
> if you want to run "efidebug boot add-initrd ..." again :)

That's the least of my worries tbh. I don't really mind if we delete it
silenmtly, as long as it's documented/justified.

> 
> -Takahiro Akashi
> 
> 
> > 
Thanks
/Ilias


More information about the U-Boot mailing list