Re: [Qemu-devel] [Resend][Seabios PATCH] don't boot from un-selected devices

[Amos Kong <akong@redhat.com>]

On Tue, Dec 25, 2012 at 08:37:07AM +0200, Gleb Natapov wrote:
> On Tue, Dec 25, 2012 at 11:58:08AM +0800, Amos Kong wrote:
> > On Wed, Dec 19, 2012 at 11:32:08AM +0200, Gleb Natapov wrote:
> > > On Wed, Dec 19, 2012 at 03:24:45PM +0800, Amos Kong wrote:
> > > > Current seabios will try to boot from selected devices first,
> > > > if they are all failed, seabios will also try to boot from
> > > > un-selected devices.
> > > > 
> > > > For example:
> > > > @ qemu-kvm -boot order=n,menu=on ...
> > > > 
> > > > Guest will boot from network first, if it's failed, guest will try to
> > > > boot from other un-selected devices (floppy, cdrom, disk) one by one.
> > > > 
> > > > Sometimes, user don't want to boot from some devices. This patch changes
> > 
> > Hi Gleb,
> > 
> > > And sometimes he want. The patch changes behaviour unconditionally. New
> > > behaviour should be user selectable. Something line -boot order=strict
> > > on qemu command line.
> > 
> > Sometimes, user don't know which devices are in boot list of seabios,
> > so they could not disable them through qemu cmdline.

Hi Gleb, Kevin

> This is not what I suggested though. And currently we do not have a way
> to remove one device from the boot process. This is separate issue and
> requires separate patch.

issue 1: add a qemu parameter to tell seabios don't boot from unselected device
issue 2: add the ability to seabios to ignore unselected device

right?
 
> > I didn't describe the purpose clearly. Currently we can assign boot
> > order by "-boot order=...", if fails to boot from all devices in order
> > parameters, other devices in seabios's boot table will also be tried. 
> order= is an old style. Use bootindex instead.
 

(I'm trying to describe the old/new principles, correct me if sth is
wrong, thanks)

+ Old style: order=...

There are two registers in Bochs/qemu's CMOS map, which are used for
boot sequence.
(number of boot devices <= 3)
-------------------------------------------------------
0x38 S    eltorito boot sequence + boot signature check
     bits
     0    floppy boot signature check (1: disabled, 0: enabled)
     7-4  boot drive #3 (0: unused, 1: fd, 2: hd, 3:cd, else: fd)

0x3d S    eltorito boot sequence (see above)
     bits
     3-0  boot drive #1
     7-4  boot drive #2

==> set order in qemu:
hw/pc.c: set_boot_dev(ISADevice *s, const char *boot_device, ..) {
    ...
    rtc_set_memory(s, 0x3d, (bds[1] << 4) | bds[0]);
    rtc_set_memory(s, 0x38, (bds[2] << 4) | (fd_bootchk ? 0x0 : 0x1));
}

==> process order in seabios:
seabios/src/boot.c: boot_setup():
        u32 bootorder = (inb_cmos(CMOS_BIOS_BOOTFLAG2)
              | ((inb_cmos(CMOS_BIOS_BOOTFLAG1) & 0xf0) << 4));
        DefaultFloppyPrio = DefaultCDPrio = DefaultHDPrio
            = DefaultBEVPrio = DEFAULT_PRIO;

^^^ device priority will be set to 9999 before processing
        int i;
        for (i=101; i<104; i++) {
            u32 val = bootorder & 0x0f;
            bootorder >>= 4;
            switch (val) {
            case 1: DefaultFloppyPrio = i; break;
            case 2: DefaultHDPrio = i;     break;
            case 3: DefaultCDPrio = i;     break;
            case 4: DefaultBEVPrio = i;    break;

^^^ if one device is selected in cmos register, its priority will be
set to 10*, the priority of un-selected devices are still 9999.
            }
        }


+ New style: bootindex
  a bootorder string which contains the descriptions of boot devices.
  (eg. /pci@i0cf8/ethernet@3/ethernet-phy@0\n...)
  it's loaded through 'bootorder' file in rom by seabios.
  So 'selected' device can be found in this string.
  seabios will use the table index as the priority of selected devices.

  Seabios uses boot_add_{floppy,floppy,hd,bcv,cd,cbfs} to add devices
  to boot entry list. the priority will be processed at this point by:
    defPrio(prio, Default...Prio)

  'prio' is got when init the device, two conditions:
      -1: unselected
      index of bootorder table: selected in bootorder rom file.

  So after processing, priority of unselected devices will be re-set to 9999.

Based on this, we can judge if device is selected or not by checking
if priority is 9999. The only problem should be legacy FD/HD.

However, it's better to add a flag ('selected') in struct bev_s. If
qemu tells seabios to ignore unselected devices, we can ignore them in
do_boot().

// Determine next boot method and attempt a boot using it.
static void do_boot(int seq_nr) {
    ....
    // Boot the given BEV type. 
    struct bev_s *ie = &BEV[seq_nr];
    if (!ie->selected)
        boot_fail();
    ...

> > The exact request should be "only boot from selected devices".
> You described the purpose clearly first time. This is how I understood
> it :)
> 
> > 
> > I agree to make it configurable.
> > eg:  qemu -boot order=nd,strict=on,menu=on
> >      strick: on  (only boot from selected devices)
> >      strick: off  (will try to boot from all devices in seabios' boot table)
> >      default strick should be 'off' as current behavior.
> >  
> Yes, this is my suggestion.
> 
> > Thanks, Amos
> > 
> > > > seabios to boot only from selected devices.
> > > > 
> > > > If user choose first boot device from menu, then seabios will try all
> > > > the devices, even some of them are not selected.
> > >

Hi Kevin,

> > > The BIOS Boot Specification (BBS) is quite complex and your patch
> > > changes SeaBIOS' behavior in subtle ways that I'm not convinced is
> > > safe.  (For example, by not always adding an FD/HD entry it may no
> > > longer be possible to boot from a legacy option rom which emulates
> > > an
> > > FD.)

Yes, not easy as my solution.

> > > Instead of altering the core algorithm for this feature, a different
> > > approach would be to add a new "boot device" (eg, IPL_TYPE_HALT)
> > > that
> > > is only registered if found in the bootorder file and just calls
> > > boot_fail() when attempted.

But how to know if device is seleted or not? Bios processes the
bootorder complexly. But I find the priority is a clew.

Thanks, Amos

>   That way users get the default
> > > behaviour
> > > unless they explicitly request a halt in boot at a given priority.
> > > 
> > > -Kevin

> > > > Signed-off-by: Amos Kong <akong@redhat.com>
> > > > ---
> > > > Resend for CCing seabios maillist.
> > > > ---
> > > >  src/boot.c |   13 ++++++++-----
> > > >  1 files changed, 8 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/src/boot.c b/src/boot.c
> > > > index 3ca7960..ee810ac 100644
> > > > --- a/src/boot.c
> > > > +++ b/src/boot.c
> > > > @@ -424,6 +424,10 @@ interactive_bootmenu(void)
> > > >          maxmenu++;
> > > >          printf("%d. %s\n", maxmenu
> > > >                 , strtcpy(desc, pos->description, ARRAY_SIZE(desc)));
> > > > +        /* If user chooses first boot device from menu, we will treat
> > > > +	    all the devices as selected. */
> > > > +        if (pos->priority == DEFAULT_PRIO)
> > > > +            pos->priority = DEFAULT_PRIO - 1;
> > > >          pos = pos->next;
> > > >      }
> > > >  
> > > > @@ -490,7 +494,10 @@ boot_prep(void)
> > > >  
> > > >      // Map drives and populate BEV list
> > > >      struct bootentry_s *pos = BootList;
> > > > -    while (pos) {
> > > > +
> > > > +    /* The priority of un-selected device is not changed,
> > > > +       we only boot from user selected devices. */
> > > > +    while (pos && pos->priority != DEFAULT_PRIO) {
> > > >          switch (pos->type) {
> > > >          case IPL_TYPE_BCV:
> > > >              call_bcv(pos->vector.seg, pos->vector.offset);
> > > > @@ -513,10 +520,6 @@ boot_prep(void)
> > > >          }
> > > >          pos = pos->next;
> > > >      }
> > > > -
> > > > -    // If nothing added a floppy/hd boot - add it manually.
> > > > -    add_bev(IPL_TYPE_FLOPPY, 0);
> > > > -    add_bev(IPL_TYPE_HARDDISK, 0);