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

[Thomas Huth]

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.

>> 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.

>> +    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.

>> +        dup
>> +    WHILE
>> +        ( dstr dlen lfa  R: phandle )
>> +        dup link> >name name>string
> 
> Esp. if you really need to look at internals this much.
> 
>> +        ( dstr dlen lfa astr alen  R: phandle )
>> +        2dup r@ get-property ABORT" alias not available"
>> +        ( dstr dlen lfa astr alen propdata proplen  R: phandle )
>> +        1-
>> +        6 pick 6 pick
> 
> "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?

 Thomas