Re: [SeaBIOS] [PATCH v2 6/6] boot: add F11 shortcut for network boot

[Kevin O'Connor]

On Wed, Oct 17, 2012 at 06:23:48PM +0200, Paolo Bonzini wrote:
> This patch adds a shortcut for network boot, similar to what is present
> on bare-metal machines.  F11 will prioritize BCV/BEV entries for network
> devices over all the other items of the boot list.
> Signed-off-by: Paolo Bonzini
>
> src/boot.c | 36 +++++++++++++++++++++++++++++++-----
>  1 file modificato, 31 inserzioni(+), 5 rimozioni(-)
>
> diff --git a/src/boot.c b/src/boot.c
> index c136ad4..682f239 100644
> --- a/src/boot.c
> +++ b/src/boot.c
> @@ -292,6 +292,7 @@ static struct bootentry_s *BootList;
>  #define IPL_TYPE_BEV         0x50
>  #define IPL_TYPE_BCV         0x60
>  #define IPL_TYPE_MASK        0xF0
> +#define IPL_TYPE_CLASS       0x0F
>
> static void
>  bootentry_add(int type, int prio, u32 data, const char *desc)
> @@ -406,26 +407,51 @@ interactive_bootmenu(void)
>      while (get_keystroke(0) >= 0)
>          ;
> printf("Press F12 for boot menu.\n\n");
> wait_threads();
> struct bootentry_s *pos;
> for (pos = BootList; pos; pos = pos->next) {
>    if ((pos->type & IPL_TYPE_CLASS) == PCI_BASE_CLASS_NETWORK)
>        break;
> }
> printf("Press %sF12 for boot menu.\n\n"
>       , pos ? "F11 for network boot, or " : "");

The wait_threads() only has an impact when CONFIG_THREAD_OPTIONROMS is
set.  When it is set, I think we really do want threads to be able to
run up until the end of the boot menu delay, as this can notably
reduce overall boot time on real hardware.  So, adding the call to
wait_threads doesn't seem right.

I suppose the code could always print the F11 message regardless of
whether or not a network card has been detected.
>  u32 menutime = romfile_loadint("etc/boot-menu-wait", DEFAULT_BOOTMENU_WAIT);
>  enable_bootsplash();
>  int scan_code = get_keystroke(menutime);
>  disable_bootsplash();
>  switch (scan_code) {
> case 0x85: { // F11
>    // Prioritize BEV/BCV entries
>    if (!pos)
>        return;

This function is getting a bit busy.  Instead of moving everything
into a switch (as in patch 4) I suggest breaking the different key
actions into separate functions.

-Kevin