[SLOF] [PATCH] Fix ugly boot menu if devices have been specified with bootindex

[Segher Boessenkool]

On Wed, Apr 05, 2017 at 09:56:09AM +0200, Thomas Huth wrote:
> On 05.04.2017 01:37, Segher Boessenkool wrote:
> > On Tue, Apr 04, 2017 at 06:22:57PM +0200, Thomas Huth wrote:
> >> So the device tree path is printed twice here. Normally, the first
> >> part before the colon should be the short-hand alias of the device
> >> instead. This happens because QEMU passes the full device path to
> >> SLOF here,
> > 
> > Should it?  Should it pass the alias name instead?
> 
> QEMU does not have a clue about the alias names that SLOF will generate,
> so it can only pass the full device tree path to SLOF.

Oh, I thought it would get it from some user configuration :-)

> >> so SLOF does not add the boot device via an alias in
> >> this case. Fix this issue by looking up the alias for the device
> >> tree node if the boot device in the list starts with a "/" (i.e.
> >> it is not an alias yet).
> > 
> > The problem with this is that you will find _a_ alias (there can be more
> > than one for the same device); is that the one you want displayed?
> 
> That should be fine - anything that is more "user friendly" than the
> full path should be fine here.

Yeah...  I suppose you don't have very many aliases, and nothing historical
that will only confuse the user.

> >> +    s" /aliases" find-node              ( dstr dlen phandle )
> >> +    dup >r
> >> +    node>properties @ cell+ @ BEGIN
> > 
> > Is there no better way to walk over aliases?  Maybe you can factor out
> > some helper?
> 
> I've stolen^W lent that code from the ".properties" word in
> slof/fs/property.fs ... I did not find a better way for this yet, but
> I'm open for suggestions.

Well, .properties naturally can use "deep" knowledge of how things are
actually laid out in memory.  I wouldn't duplicate that though; instead,
make some helper that iterates over the properties.  Maybe just something
with next-property and get-property ?  That isn't so very efficient of
course (you walk the property wordlist every time to find the property
by name), a) but everything else does the same anyway, and b) you could
make some versions that use some wordlist link (or the >name or similar)
instead.

> > "pick" is a clear sign you need to factor more.  "6 pick"?  Uhhhh...
> 
> Ack, that was maybe a little bit too much ...
> 
> ... but looking at the whole problem from a distance again, I'm
> currently unsure whether that boot menu code is doing the right thing at
> all. First, if you start QEMU with both, some devices that have the
> "bootindex=..." set and "-boot strict=off", the devices will show up
> twice in the menu (one time without alias if my patch hasn't been
> applied yet, and one time with alias). Second, if you use "-boot
> strict=on", the boot menu will *not* show up the devices anymore that do
> not have the "bootindex" set (and the menu will show a bogus "HALT"
> option, too).
> 
> So I think that menu code should simply not rely on $bootdev but rather
> build a list of possible boot devices by looking at the available disk,
> cdrom and net aliases?

That sounds quite reasonable.


Segher