| Message ID | 1491322977-27353-1-git-send-email-thuth@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
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? > 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? > + 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? > + 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... > : boot-start > decimal > BEGIN parse-word dup WHILE > - my-boot-dev (u.) s" . " $cat type 2dup type ." : " de-alias type cr > + my-boot-dev (u.) s" . " $cat type Not new of course, but just my-boot-dev 0 u.r ." . " perhaps? Hope this is useful and not just nitpicking, Segher
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
Thomas Huth <thuth@redhat.com> writes: > If the user has started with some devices that have the bootindex > parameter set, these devices show up in a very ugly way in the boot > menu of SLOF. For example, if you start QEMU with: > > qemu-system-ppc64 ... -boot menu=on -device spapr-vscsi,id=scsi0 \ > -drive file=distro.iso,if=none,media=cdrom,readonly=on,id=img0 \ > -device scsi-cd,drive=img0,bootindex=1 From distant memory, I understand that "-boot" and bootindex are mutually exclusive. bootindex was introduced to qemu to get rid of restrictive -boot option. Just checked the qemu documentation as well - docs/bootindex.txt : == Mixing bootindex and boot order parameters == Note that it does not make sense to use the bootindex property together with the "-boot order=..." (or "-boot once=...") parameter. The guest firmware implementations normally either support the one or the other, but not both parameters at the same time. Mixing them will result in undefined behavior, and thus the guest firmware will likely not boot from the expected devices. Regards Nikunj
On 05.04.2017 10:12, Nikunj A Dadhania wrote: > Thomas Huth <thuth@redhat.com> writes: > >> If the user has started with some devices that have the bootindex >> parameter set, these devices show up in a very ugly way in the boot >> menu of SLOF. For example, if you start QEMU with: >> >> qemu-system-ppc64 ... -boot menu=on -device spapr-vscsi,id=scsi0 \ >> -drive file=distro.iso,if=none,media=cdrom,readonly=on,id=img0 \ >> -device scsi-cd,drive=img0,bootindex=1 > > From distant memory, I understand that "-boot" and bootindex are > mutually exclusive. That's "-boot order=...", but not "-boot strict=..." or "-boot menu=on". So AFAICT "-boot menu=on" should still work if you specify some devices with "bootindex", too. Thomas
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes: > Thomas Huth <thuth@redhat.com> writes: > >> If the user has started with some devices that have the bootindex >> parameter set, these devices show up in a very ugly way in the boot >> menu of SLOF. For example, if you start QEMU with: >> >> qemu-system-ppc64 ... -boot menu=on -device spapr-vscsi,id=scsi0 \ >> -drive file=distro.iso,if=none,media=cdrom,readonly=on,id=img0 \ >> -device scsi-cd,drive=img0,bootindex=1 > > From distant memory, I understand that "-boot" and bootindex are > mutually exclusive. bootindex was introduced to qemu to get rid of > restrictive -boot option. For example, with "-boot order=cd" if we have 2disks, there is no way to specify which one to boot from. SLOF will boot from the first disk alias. If a selection is needed: menu=on is added, and we can then press F12 to get to menu and select the second disk. While for bootindex, I can use qemu command-line(bootindex=1) or libvirt xml tag "<boot order='1'/>" for the second disk. So with bootindex, there is no need for "-boot menu=on,order=cd" Regards Nikunj
On 05.04.2017 10:26, Nikunj A Dadhania wrote: > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes: > >> Thomas Huth <thuth@redhat.com> writes: >> >>> If the user has started with some devices that have the bootindex >>> parameter set, these devices show up in a very ugly way in the boot >>> menu of SLOF. For example, if you start QEMU with: >>> >>> qemu-system-ppc64 ... -boot menu=on -device spapr-vscsi,id=scsi0 \ >>> -drive file=distro.iso,if=none,media=cdrom,readonly=on,id=img0 \ >>> -device scsi-cd,drive=img0,bootindex=1 >> >> From distant memory, I understand that "-boot" and bootindex are >> mutually exclusive. bootindex was introduced to qemu to get rid of >> restrictive -boot option. > > For example, with "-boot order=cd" if we have 2disks, there is no way to specify > which one to boot from. SLOF will boot from the first disk alias. > > If a selection is needed: menu=on is added, and we can then press F12 to > get to menu and select the second disk. > > While for bootindex, I can use qemu command-line(bootindex=1) or libvirt > xml tag "<boot order='1'/>" for the second disk. So with bootindex, > there is no need for "-boot menu=on,order=cd" Agreed, but it is still possible to enable the boot menu if you're also using "bootindex" ... so please ignore my original patch, but I think the boot menu code should also not take the devices in $bootdev into account that have been specified with bootindex. So likely the right solution is to simply skip all entries there that are not aliases (i.e. skip the entries that start with a slash). Thomas
Thomas Huth <thuth@redhat.com> writes: > On 05.04.2017 10:26, Nikunj A Dadhania wrote: >> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes: >> >>> Thomas Huth <thuth@redhat.com> writes: >>> >>>> If the user has started with some devices that have the bootindex >>>> parameter set, these devices show up in a very ugly way in the boot >>>> menu of SLOF. For example, if you start QEMU with: >>>> >>>> qemu-system-ppc64 ... -boot menu=on -device spapr-vscsi,id=scsi0 \ >>>> -drive file=distro.iso,if=none,media=cdrom,readonly=on,id=img0 \ >>>> -device scsi-cd,drive=img0,bootindex=1 >>> >>> From distant memory, I understand that "-boot" and bootindex are >>> mutually exclusive. bootindex was introduced to qemu to get rid of >>> restrictive -boot option. >> >> For example, with "-boot order=cd" if we have 2disks, there is no way to specify >> which one to boot from. SLOF will boot from the first disk alias. >> >> If a selection is needed: menu=on is added, and we can then press F12 to >> get to menu and select the second disk. >> >> While for bootindex, I can use qemu command-line(bootindex=1) or libvirt >> xml tag "<boot order='1'/>" for the second disk. So with bootindex, >> there is no need for "-boot menu=on,order=cd" > > Agreed, but it is still possible to enable the boot menu if you're also > using "bootindex" ... True, unfortunately :( > so please ignore my original patch, but I think > the boot menu code should also not take the devices in $bootdev into > account that have been specified with bootindex. > So likely the right solution is to simply skip all entries there that > are not aliases (i.e. skip the entries that start with a slash). Yes, that should be fine. Regards Nikunj
On 05.04.2017 10:55, Nikunj A Dadhania wrote: > Thomas Huth <thuth@redhat.com> writes: > >> On 05.04.2017 10:26, Nikunj A Dadhania wrote: >>> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes: >>> >>>> Thomas Huth <thuth@redhat.com> writes: >>>> >>>>> If the user has started with some devices that have the bootindex >>>>> parameter set, these devices show up in a very ugly way in the boot >>>>> menu of SLOF. For example, if you start QEMU with: >>>>> >>>>> qemu-system-ppc64 ... -boot menu=on -device spapr-vscsi,id=scsi0 \ >>>>> -drive file=distro.iso,if=none,media=cdrom,readonly=on,id=img0 \ >>>>> -device scsi-cd,drive=img0,bootindex=1 >>>> >>>> From distant memory, I understand that "-boot" and bootindex are >>>> mutually exclusive. bootindex was introduced to qemu to get rid of >>>> restrictive -boot option. >>> >>> For example, with "-boot order=cd" if we have 2disks, there is no way to specify >>> which one to boot from. SLOF will boot from the first disk alias. >>> >>> If a selection is needed: menu=on is added, and we can then press F12 to >>> get to menu and select the second disk. >>> >>> While for bootindex, I can use qemu command-line(bootindex=1) or libvirt >>> xml tag "<boot order='1'/>" for the second disk. So with bootindex, >>> there is no need for "-boot menu=on,order=cd" >> >> Agreed, but it is still possible to enable the boot menu if you're also >> using "bootindex" ... > > True, unfortunately :( > >> so please ignore my original patch, but I think >> the boot menu code should also not take the devices in $bootdev into >> account that have been specified with bootindex. >> So likely the right solution is to simply skip all entries there that >> are not aliases (i.e. skip the entries that start with a slash). > > Yes, that should be fine. Actually, the boot menu is also broken in another case: If you store something in the "boot-device" environment variable, it only shows that value. For example with qemu-system-ppc64 ... -boot menu=on -cdrom some.iso \ -prom-env boot-device=disk the boot menu only shows "disk", and ignores the CD-ROM. Fortunately, hardly anybody uses these environment variables with QEMU nowadays. But still, the behavior of the boot menu looks pretty wrong here to me. Thomas
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
On 05/04/17 21:21, Thomas Huth wrote: > On 05.04.2017 10:55, Nikunj A Dadhania wrote: >> Thomas Huth <thuth@redhat.com> writes: >> >>> On 05.04.2017 10:26, Nikunj A Dadhania wrote: >>>> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes: >>>> >>>>> Thomas Huth <thuth@redhat.com> writes: >>>>> >>>>>> If the user has started with some devices that have the bootindex >>>>>> parameter set, these devices show up in a very ugly way in the boot >>>>>> menu of SLOF. For example, if you start QEMU with: >>>>>> >>>>>> qemu-system-ppc64 ... -boot menu=on -device spapr-vscsi,id=scsi0 \ >>>>>> -drive file=distro.iso,if=none,media=cdrom,readonly=on,id=img0 \ >>>>>> -device scsi-cd,drive=img0,bootindex=1 >>>>> >>>>> From distant memory, I understand that "-boot" and bootindex are >>>>> mutually exclusive. bootindex was introduced to qemu to get rid of >>>>> restrictive -boot option. >>>> >>>> For example, with "-boot order=cd" if we have 2disks, there is no way to specify >>>> which one to boot from. SLOF will boot from the first disk alias. >>>> >>>> If a selection is needed: menu=on is added, and we can then press F12 to >>>> get to menu and select the second disk. >>>> >>>> While for bootindex, I can use qemu command-line(bootindex=1) or libvirt >>>> xml tag "<boot order='1'/>" for the second disk. So with bootindex, >>>> there is no need for "-boot menu=on,order=cd" >>> >>> Agreed, but it is still possible to enable the boot menu if you're also >>> using "bootindex" ... >> >> True, unfortunately :( >> >>> so please ignore my original patch, but I think >>> the boot menu code should also not take the devices in $bootdev into >>> account that have been specified with bootindex. >>> So likely the right solution is to simply skip all entries there that >>> are not aliases (i.e. skip the entries that start with a slash). >> >> Yes, that should be fine. If I personally wanted to use "-boot menu=on", I would expect SLOF to list all bootable devices, each menu item would be just a single long device-tree path without any alias. I do not see any use for bootindex with "menu=on" so I'd just ignore (or menu could run timeout and show what it will pick if the user does not make a choice). And the only case when I would want this would be having many devices (actually even 2 devices of the same type is enough for example) so the only way to tell what corresponds to what in the QEMU command line would be seeing 1) device type (virtio, e1000,...) 2) pci or vio address - and aliases hide this information. imho aliases only make some sense when the user got just few devices, preferably one of each type. > > Actually, the boot menu is also broken in another case: If you store > something in the "boot-device" environment variable, it only shows that > value. For example with > > qemu-system-ppc64 ... -boot menu=on -cdrom some.iso \ > -prom-env boot-device=disk > > the boot menu only shows "disk", and ignores the CD-ROM. Fortunately, > hardly anybody uses these environment variables with QEMU nowadays. Hacking directly on promenv calls for problems and whoever does this should understand the consequences well :) > But > still, the behavior of the boot menu looks pretty wrong here to me. imho the option itself is quite useless today.
On Mon, Apr 10, 2017 at 05:34:58PM +1000, Alexey Kardashevskiy wrote: > imho aliases only make some sense when the user got just few devices, > preferably one of each type. How so? Aliases are typically set up by platform-specific code, and such code will know what devices to call disk0 and disk1, or enet0 until enet3, etc. Aliases are useful because they a) are much shorter names, and b) are names that make more sense to the user. Segher
On 10/04/17 19:26, Segher Boessenkool wrote: > On Mon, Apr 10, 2017 at 05:34:58PM +1000, Alexey Kardashevskiy wrote: >> imho aliases only make some sense when the user got just few devices, >> preferably one of each type. > > How so? Aliases are typically set up by platform-specific code, and > such code will know what devices to call disk0 and disk1, or enet0 until > enet3, etc. > > Aliases are useful because they a) are much shorter names, and b) are > names that make more sense to the user. QEMU v2.9.0-rcsomething, 4 disks: -device virtio-scsi-pci,id=vscsi0 \ -drive id=DRIVE0,if=none,file=img/fc22_16G.qcow2,format=qcow2 \ -device scsi-disk,id=scsi-disk0,drive=DRIVE0 \ -drive id=DRIVE1,if=none,file=img/u16_04_32G_htx_406.qcow2,format=qcow2 \ -device virtio-blk-pci,id=vblk0,drive=DRIVE1 \ -device spapr-vscsi,id=svscsi0 \ -drive id=DRIVE2,if=none,file=img/noprep-16g.qcow2,format=qcow2 \ -device scsi-disk,id=scsi-disk1,drive=DRIVE2,bus=svscsi0.0 \ -drive id=DRIVE3,if=none,file=img/sle12_sp1_rc2_16G.qcow2,format=qcow2 \ -device scsi-disk,id=scsi-disk2,drive=DRIVE3 \ SLOF: 0 > devalias disk3 : /pci@800000020000000/scsi@2 disk2 : /pci@800000020000000/scsi@1/disk@101000000000000 disk1 : /pci@800000020000000/scsi@1/disk@100000000000000 usb0 : /pci@800000020000000/usb@0 hvterm : /vdevice/vty@71000100 scsi : /vdevice/v-scsi@71000001 disk : /vdevice/v-scsi@71000001/disk@8000000000000000 nvram : /vdevice/nvram@71000000 ok The command line order is: virtio-scsi-disk-1, virtio-blk, spapr-vscsi-disk, virtio-scsi-disk-2, The devalias order is: spapr-vscsi-disk, virtio-scsi-disk-1, virtio-scsi-disk-2, virtio-blk. The devalias order in QEMU v2.7.0 is (why - below): spapr-vscsi-disk, virtio-blk, virtio-scsi-disk-1, virtio-scsi-disk-2. The user needs to remember that: 1. SLOF scans VIO first, then it scans PCI 2. if virtio-scsi HBA happens to have smaller PCI slot number, then all disks attached to that HBA will appear in devalias before any virtio-block 3. We actually changed the order of PCI scanning recently, it was opposite. I fail to see how aliases make more sense here. They could help if they were not simple diskX but vscsi0, virtioblock0, virtioscsi0, virtioscsi1 but this is not the case and it won't help in general (we can construct really crazy machines with many PHBs, spapr-vscsi, etc). Personally I found aliases useful for: 1. typing "boot disk" in the firmware prompt of the actual console of Apple G5 machine - where I could not cut-n-paste - we do not care about this and fixing SLOF for JS22 is still in my list :) 2. this list shows (more or less) what SLOF can possibly boot from so I could cut-n-paste it to "boot /pci@800000020000000/scsi@1/disk@100000000000000" - when you cut-n-paste, the amount of letter does not make difference :)
On Tue, Apr 11, 2017 at 01:52:11PM +1000, Alexey Kardashevskiy wrote: > On 10/04/17 19:26, Segher Boessenkool wrote: > > On Mon, Apr 10, 2017 at 05:34:58PM +1000, Alexey Kardashevskiy wrote: > >> imho aliases only make some sense when the user got just few devices, > >> preferably one of each type. > > > > How so? Aliases are typically set up by platform-specific code, and > > such code will know what devices to call disk0 and disk1, or enet0 until > > enet3, etc. > > > > Aliases are useful because they a) are much shorter names, and b) are > > names that make more sense to the user. [ snip ] > SLOF: > 0 > devalias > disk3 : /pci@800000020000000/scsi@2 > disk2 : /pci@800000020000000/scsi@1/disk@101000000000000 > disk1 : /pci@800000020000000/scsi@1/disk@100000000000000 > usb0 : /pci@800000020000000/usb@0 > hvterm : /vdevice/vty@71000100 > scsi : /vdevice/v-scsi@71000001 > disk : /vdevice/v-scsi@71000001/disk@8000000000000000 > nvram : /vdevice/nvram@71000000 ok Those are set up some automated way, numbered in the order they are found. That is not very helpful if the configuration can change, no. If there is only ever one disk, or maybe two, having them automatically named "disk" and "disk1" is pretty handy. If the platform code does not set sane aliases, the user can do it himself: nvalias boot-disk /some/long/path > Personally I found aliases useful for: > > 1. typing "boot disk" in the firmware prompt of the actual console of Apple > G5 machine - where I could not cut-n-paste - You can use a telnet console on Apple OF. It works well :-) " enet:telnet,10.0.0.1" io Segher
On 11/04/17 16:17, Segher Boessenkool wrote: > On Tue, Apr 11, 2017 at 01:52:11PM +1000, Alexey Kardashevskiy wrote: >> On 10/04/17 19:26, Segher Boessenkool wrote: >>> On Mon, Apr 10, 2017 at 05:34:58PM +1000, Alexey Kardashevskiy wrote: >>>> imho aliases only make some sense when the user got just few devices, >>>> preferably one of each type. >>> >>> How so? Aliases are typically set up by platform-specific code, and >>> such code will know what devices to call disk0 and disk1, or enet0 until >>> enet3, etc. >>> >>> Aliases are useful because they a) are much shorter names, and b) are >>> names that make more sense to the user. > > [ snip ] > >> SLOF: >> 0 > devalias >> disk3 : /pci@800000020000000/scsi@2 >> disk2 : /pci@800000020000000/scsi@1/disk@101000000000000 >> disk1 : /pci@800000020000000/scsi@1/disk@100000000000000 >> usb0 : /pci@800000020000000/usb@0 >> hvterm : /vdevice/vty@71000100 >> scsi : /vdevice/v-scsi@71000001 >> disk : /vdevice/v-scsi@71000001/disk@8000000000000000 >> nvram : /vdevice/nvram@71000000 ok > > Those are set up some automated way, numbered in the order they are > found. That is not very helpful if the configuration can change, no. Not just configuration but QEMU version also can change this. > If there is only ever one disk, or maybe two, having them automatically > named "disk" and "disk1" is pretty handy. Sure but since this is KVM/QEMU and there are already ways to control boot order via other means (like bootindex property), aliases lost their initial handiness. In order to fix the initial bugreport, may be it could make sense to create aliases such as "bootindexN" for all devices with a valid bootindex property... Although I am not sure it is worth the effort and there will be users for it, ever. > > If the platform code does not set sane aliases, the user can do it > himself: > nvalias boot-disk /some/long/path > >> Personally I found aliases useful for: >> >> 1. typing "boot disk" in the firmware prompt of the actual console of Apple >> G5 machine - where I could not cut-n-paste - > > You can use a telnet console on Apple OF. It works well :-) > " enet:telnet,10.0.0.1" io But in reality I need it once - set up the correct boot command and that's it :) But good to know, thanks!
On 11.04.2017 09:18, Alexey Kardashevskiy wrote: [...] > In order to fix the initial bugreport, may be it could make sense to > create aliases such as "bootindexN" for all devices with a valid bootindex > property... Although I am not sure it is worth the effort and there will be > users for it, ever. bootindex aliases also sound quite ugly to me, and this way we still display devices twice, one time the "bootindex" version and one time the version that SLOF auto-detected during boot. Assuming that we still want to fix the boot menu somehow, I think we should rather ignore the $bootdev list here and simply show all devices instead that SLOF detected during boot, i.e. walk the cdromX, diskX and netX aliases here instead. Does that also sound OK to you? If so, I can work on a patch ... maybe also converting the menu code to C instead, since maintaining such boot lists in Forth is somewhat cumbersome... Thomas
On 25/04/17 01:40, Thomas Huth wrote: > On 11.04.2017 09:18, Alexey Kardashevskiy wrote: > [...] >> In order to fix the initial bugreport, may be it could make sense to >> create aliases such as "bootindexN" for all devices with a valid bootindex >> property... Although I am not sure it is worth the effort and there will be >> users for it, ever. > > bootindex aliases also sound quite ugly to me, and this way we still > display devices twice, one time the "bootindex" version and one time the > version that SLOF auto-detected during boot. > > Assuming that we still want to fix the boot menu somehow, I think we > should rather ignore the $bootdev list here and simply show all devices > instead that SLOF detected during boot, i.e. walk the cdromX, diskX and > netX aliases here instead. Does that also sound OK to you? What you propose may look nicer but the current situation lets me distinguish at least some items in the boot menu (the long ones, with bootindex), if the entire menu is all made of aliases - this I won't use at all :) Is it just me and others find aliases useful even when a driver name is not a part of an alias? > If so, I can > work on a patch ... maybe also converting the menu code to C instead, > since maintaining such boot lists in Forth is somewhat cumbersome...
On 25.04.2017 16:04, Alexey Kardashevskiy wrote: > On 25/04/17 01:40, Thomas Huth wrote: >> On 11.04.2017 09:18, Alexey Kardashevskiy wrote: >> [...] >>> In order to fix the initial bugreport, may be it could make sense to >>> create aliases such as "bootindexN" for all devices with a valid bootindex >>> property... Although I am not sure it is worth the effort and there will be >>> users for it, ever. >> >> bootindex aliases also sound quite ugly to me, and this way we still >> display devices twice, one time the "bootindex" version and one time the >> version that SLOF auto-detected during boot. >> >> Assuming that we still want to fix the boot menu somehow, I think we >> should rather ignore the $bootdev list here and simply show all devices >> instead that SLOF detected during boot, i.e. walk the cdromX, diskX and >> netX aliases here instead. Does that also sound OK to you? > > What you propose may look nicer but the current situation lets me > distinguish at least some items in the boot menu (the long ones, with > bootindex), if the entire menu is all made of aliases - this I won't use at > all :) Is it just me and others find aliases useful even when a driver name > is not a part of an alias? I did not mean to remove the full device tree strings in the output there. I'd keep the current way of printing both, the alias and the full device tree path. I just meant to change the way that the menu code gathers the list of devices that should be printed there. Currently it uses all devices from $bootdev - which is just bollocks as soon as the user specifies one of the devices with "bootindex" or changed the boot-device NVRAM variable. So let me summarize again: The boot menu should simply print *all* discovered devices, not only the ones from $bootdev. The simplest way to get a list of all discovered boot devices is to walk the cdromX, diskX and netX aliases (since we do not support FCODE or other devices, there should be no other format of aliases as far as I can see). This list should then be presented with "<nr>. <alias>: <path>" output lines to the users, so they have the full choice of all boot devices. Hope that helps to clarify my ideas ... Does that now sound OK to you? Thomas
On 26/04/17 00:19, Thomas Huth wrote: > On 25.04.2017 16:04, Alexey Kardashevskiy wrote: >> On 25/04/17 01:40, Thomas Huth wrote: >>> On 11.04.2017 09:18, Alexey Kardashevskiy wrote: >>> [...] >>>> In order to fix the initial bugreport, may be it could make sense to >>>> create aliases such as "bootindexN" for all devices with a valid bootindex >>>> property... Although I am not sure it is worth the effort and there will be >>>> users for it, ever. >>> >>> bootindex aliases also sound quite ugly to me, and this way we still >>> display devices twice, one time the "bootindex" version and one time the >>> version that SLOF auto-detected during boot. >>> >>> Assuming that we still want to fix the boot menu somehow, I think we >>> should rather ignore the $bootdev list here and simply show all devices >>> instead that SLOF detected during boot, i.e. walk the cdromX, diskX and >>> netX aliases here instead. Does that also sound OK to you? >> >> What you propose may look nicer but the current situation lets me >> distinguish at least some items in the boot menu (the long ones, with >> bootindex), if the entire menu is all made of aliases - this I won't use at >> all :) Is it just me and others find aliases useful even when a driver name >> is not a part of an alias? > > I did not mean to remove the full device tree strings in the output > there. I'd keep the current way of printing both, the alias and the full > device tree path. I realised this as as soon as I sent my mail :) >I just meant to change the way that the menu code > gathers the list of devices that should be printed there. Currently it > uses all devices from $bootdev - which is just bollocks as soon as the > user specifies one of the devices with "bootindex" or changed the > boot-device NVRAM variable. > > So let me summarize again: The boot menu should simply print *all* > discovered devices, not only the ones from $bootdev. The simplest way to > get a list of all discovered boot devices is to walk the cdromX, diskX > and netX aliases (since we do not support FCODE or other devices, there > should be no other format of aliases as far as I can see). > This list should then be presented with "<nr>. <alias>: <path>" output > lines to the users, so they have the full choice of all boot devices. > > Hope that helps to clarify my ideas ... Does that now sound OK to you? Yes, this sounds ok to me. Either forth or c is ok too. > > Thomas >
diff --git a/slof/fs/start-up.fs b/slof/fs/start-up.fs index dc5d1ed..7186482 100644 --- a/slof/fs/start-up.fs +++ b/slof/fs/start-up.fs @@ -90,10 +90,42 @@ TRUE VALUE use-load-watchdog? (boot) ; +\ Try to get the alias for a given device tree node path +: get-alias ( dstr dlen -- astr alen ) + over c@ [char] / <> IF EXIT THEN + s" /aliases" find-node ( dstr dlen phandle ) + dup >r + node>properties @ cell+ @ BEGIN + dup + WHILE + ( dstr dlen lfa R: phandle ) + dup link> >name name>string + ( 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 + str= IF ( dstr dlen lfa astr alen R: phandle ) + r> drop + >r >r 3drop r> r> + ( astr alen ) + EXIT + ELSE + 2drop + THEN + @ + REPEAT + ( dstr dlen lfa R: phandle ) + r> + 2drop +; + : boot-start decimal BEGIN parse-word dup WHILE - my-boot-dev (u.) s" . " $cat type 2dup type ." : " de-alias type cr + my-boot-dev (u.) s" . " $cat type + 2dup get-alias type + ." : " de-alias type cr my-boot-dev 1 + to my-boot-dev REPEAT 2drop 0 0 load-list 2!
If the user has started with some devices that have the bootindex parameter set, these devices show up in a very ugly way in the boot menu of SLOF. For example, if you start QEMU with: qemu-system-ppc64 ... -boot menu=on -device spapr-vscsi,id=scsi0 \ -drive file=distro.iso,if=none,media=cdrom,readonly=on,id=img0 \ -device scsi-cd,drive=img0,bootindex=1 and press F12 for the boot menu during boot, the first entry of the menu looks like this: 1. /vdevice/v-scsi@71000003/disk@8000000000000000: /vdevice/v-scsi@71000003/disk@8000000000000000 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, 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). Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1429832 Signed-off-by: Thomas Huth <thuth@redhat.com> --- slof/fs/start-up.fs | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-)