diff mbox

[v2] A new SLOF boot menu

Message ID 1496828011-24874-1-git-send-email-thuth@redhat.com
State Accepted
Headers show

Commit Message

Thomas Huth June 7, 2017, 9:33 a.m. UTC
The current SLOF boot menu heavily depends on the contents of the
"qemu,boot-list" and "qemu,boot-device"" properties in the device tree,
so that the menu entries either look very strange (when there is no
alias, see https://bugzilla.redhat.com/show_bug.cgi?id=1429832 ) or
are duplicated (https://bugzilla.redhat.com/show_bug.cgi?id=1446018).

A proper boot menu should rather show all available boot devices
instead, so this patch series introduces a new boot menu (written in
C this time) which is independent from the "qemu,boot-list/device"
properties by looking at the available aliases instead. It is now also
possible by selecting the entries with one key stroke only (you don't
have to press RETURN anymore), so this is now hopefully much more user
friendly than the old menu.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 v2:
 - Fixed nits from v1
 - Squashed all patches together

 board-qemu/Makefile           |   2 +-
 board-qemu/slof/Makefile      |   9 +-
 lib/Makefile                  |   2 +-
 lib/libbootmenu/Makefile      |  49 +++++++++++
 lib/libbootmenu/bootmenu.c    | 187 ++++++++++++++++++++++++++++++++++++++++++
 lib/libbootmenu/bootmenu.code |  20 +++++
 lib/libbootmenu/bootmenu.h    |  15 ++++
 lib/libbootmenu/bootmenu.in   |  15 ++++
 slof/fs/start-up.fs           |  71 ++--------------
 9 files changed, 303 insertions(+), 67 deletions(-)
 create mode 100644 lib/libbootmenu/Makefile
 create mode 100644 lib/libbootmenu/bootmenu.c
 create mode 100644 lib/libbootmenu/bootmenu.code
 create mode 100644 lib/libbootmenu/bootmenu.h
 create mode 100644 lib/libbootmenu/bootmenu.in

Comments

Alexey Kardashevskiy June 8, 2017, 6:28 a.m. UTC | #1
On 07/06/17 19:33, Thomas Huth wrote:
> The current SLOF boot menu heavily depends on the contents of the
> "qemu,boot-list" and "qemu,boot-device"" properties in the device tree,
> so that the menu entries either look very strange (when there is no
> alias, see https://bugzilla.redhat.com/show_bug.cgi?id=1429832 ) or
> are duplicated (https://bugzilla.redhat.com/show_bug.cgi?id=1446018).
> 
> A proper boot menu should rather show all available boot devices
> instead, so this patch series introduces a new boot menu (written in
> C this time) which is independent from the "qemu,boot-list/device"
> properties by looking at the available aliases instead. It is now also
> possible by selecting the entries with one key stroke only (you don't
> have to press RETURN anymore), so this is now hopefully much more user
> friendly than the old menu.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>


Thanks, applied.

This reminded me of an old problem if a patch adds a new file, and then you
do "git checkout HEAD^" (like with this one), dependencies are broken:

 ====== Building slof ======
        [CC]    Makefile.dep
make[2]: *** No rule to make target 'bootmenu.h', needed by 'paflof.o'.  Stop.
make[2]: *** Waiting for unfinished jobs....
Makefile:33: recipe for target 'slof' failed
make[1]: *** [slof] Error 2
Makefile:54: recipe for target 'qemu' failed
make: *** [qemu] Error 2


If/when you have time, could you please look at this too?


> ---
>  v2:
>  - Fixed nits from v1
>  - Squashed all patches together
diff mbox

Patch

diff --git a/board-qemu/Makefile b/board-qemu/Makefile
index 7208fcc..61a1367 100644
--- a/board-qemu/Makefile
+++ b/board-qemu/Makefile
@@ -15,7 +15,7 @@  BOARD_TARGETS = tools_build romfs_build stage1 subdirs
 SUBDIRS = slof
 
 COMMON_LIBS = libc libbootmsg libbases libnvram libelf libhvcall libvirtio \
-              libusb libveth libe1k libnet
+              libusb libveth libe1k libnet libbootmenu
 
 all: $(BOARD_TARGETS)
 	$(MAKE) boot_rom.bin
diff --git a/board-qemu/slof/Makefile b/board-qemu/slof/Makefile
index 02d819b..2263e75 100644
--- a/board-qemu/slof/Makefile
+++ b/board-qemu/slof/Makefile
@@ -21,7 +21,8 @@  all: version.o Makefile.dep OF.ffs paflof $(SLOFCMNDIR)/xvect.bin
 CPPFLAGS = -I$(LIBCMNDIR)/libbootmsg -I$(LIBCMNDIR)/libhvcall \
 	   -I$(LIBCMNDIR)/libvirtio -I$(LIBCMNDIR)/libnvram \
 	   -I$(LIBCMNDIR)/libusb -I$(LIBCMNDIR)/libveth \
-	   -I$(LIBCMNDIR)/libe1k -I$(LIBCMNDIR)/libnet
+	   -I$(LIBCMNDIR)/libe1k -I$(LIBCMNDIR)/libnet \
+	   -I$(LIBCMNDIR)/libbootmenu
 SLOF_LIBS = \
 	$(LIBCMNDIR)/libbootmsg.a \
 	$(LIBCMNDIR)/libelf.a \
@@ -31,7 +32,8 @@  SLOF_LIBS = \
 	$(LIBCMNDIR)/libnvram.a \
 	$(LIBCMNDIR)/libveth.a \
 	$(LIBCMNDIR)/libe1k.a \
-	$(LIBCMNDIR)/libnet.a
+	$(LIBCMNDIR)/libnet.a \
+	$(LIBCMNDIR)/libbootmenu.a
 BOARD_SLOF_IN = \
 	$(LIBCMNDIR)/libhvcall/hvcall.in \
 	$(LIBCMNDIR)/libvirtio/virtio.in \
@@ -42,7 +44,8 @@  BOARD_SLOF_IN = \
 	$(LIBCMNDIR)/libbases/libbases.in \
 	$(LIBCMNDIR)/libveth/veth.in \
 	$(LIBCMNDIR)/libe1k/e1k.in \
-	$(LIBCMNDIR)/libnet/libnet.in
+	$(LIBCMNDIR)/libnet/libnet.in \
+	$(LIBCMNDIR)/libbootmenu/bootmenu.in
 BOARD_SLOF_CODE = $(BOARD_SLOF_IN:%.in=%.code)
 
 include $(SLOFCMNDIR)/Makefile.inc
diff --git a/lib/Makefile b/lib/Makefile
index 6d9db66..a4d4bb2 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -11,7 +11,7 @@ 
 # ****************************************************************************/
 
 SUBDIRS = libc libipmi libbootmsg libbases libnvram libelf libhvcall libvirtio \
-          libusb libveth libe1k libbcm libnet
+          libusb libveth libe1k libbcm libnet libbootmenu
 
 all:  subdirs
 
diff --git a/lib/libbootmenu/Makefile b/lib/libbootmenu/Makefile
new file mode 100644
index 0000000..156bc8b
--- /dev/null
+++ b/lib/libbootmenu/Makefile
@@ -0,0 +1,49 @@ 
+# *****************************************************************************
+# * Copyright (c) 2004, 2008 IBM Corporation
+# * All rights reserved.
+# * This program and the accompanying materials
+# * are made available under the terms of the BSD License
+# * which accompanies this distribution, and is available at
+# * http://www.opensource.org/licenses/bsd-license.php
+# *
+# * Contributors:
+# *     IBM Corporation - initial implementation
+# ****************************************************************************/
+
+ifndef TOP
+  TOP = $(shell while ! test -e make.rules; do cd ..  ; done; pwd)
+  export TOP
+endif
+include $(TOP)/make.rules
+
+CFLAGS += -I. -I.. -I../libc/include -I$(SLOFCMNDIR) -I$(INCLCMNDIR)
+
+SRCS =	bootmenu.c
+
+OBJS = $(SRCS:%.c=%.o)
+
+TARGET = ../libbootmenu.a
+
+all: $(TARGET)
+
+$(TARGET): $(OBJS)
+	$(AR) -rc $@ $(OBJS)
+	$(RANLIB) $@
+
+clean:
+	$(RM) $(TARGET) $(OBJS)
+
+distclean: clean
+	$(RM) Makefile.dep
+
+
+# Rules for creating the dependency file:
+depend:
+	$(RM) Makefile.dep
+	$(MAKE) Makefile.dep
+
+Makefile.dep: Makefile
+	$(CC) -M $(CPPFLAGS) $(CFLAGS) $(SRCS) > Makefile.dep
+
+# Include dependency file if available:
+-include Makefile.dep
diff --git a/lib/libbootmenu/bootmenu.c b/lib/libbootmenu/bootmenu.c
new file mode 100644
index 0000000..e5cb4b2
--- /dev/null
+++ b/lib/libbootmenu/bootmenu.c
@@ -0,0 +1,187 @@ 
+/*****************************************************************************
+ * Boot menu: Displays boot devices and waits for user to select one
+ *
+ * Copyright 2017 Red Hat, Inc.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the BSD License
+ * which accompanies this distribution, and is available at
+ * http://www.opensource.org/licenses/bsd-license.php
+ *
+ * Contributors:
+ *     Thomas Huth, Red Hat Inc. - initial implementation
+ *****************************************************************************/
+
+#include <stdbool.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <paflof.h>
+#include <helpers.h>
+#include "bootmenu.h"
+
+#define MAX_DEVS 36        /* Enough for 10 digits + 26 letters */
+#define MAX_ALIAS_LEN 8    /* Maximum length of alias names */
+
+struct bootdev {
+	char alias[MAX_ALIAS_LEN];
+	char *path;
+};
+
+static int nr_devs;
+static struct bootdev bootdevs[MAX_DEVS];
+
+/**
+ * Look up an alias name.
+ * @return The NUL-terminated device tree path (should be released with free()
+ *         when it's not required anymore), or NULL if it can't be found.
+ */
+static char *find_alias(char *alias)
+{
+	char *path;
+	long len;
+
+	forth_push((unsigned long)alias);
+	forth_push(strlen(alias));
+	forth_eval("find-alias");
+
+	len = forth_pop();
+	if (!len)
+		return NULL;
+
+	path = malloc(len + 1);
+	if (!path) {
+		puts("Out of memory in find_alias");
+		return NULL;
+	}
+	memcpy(path, (void *)forth_pop(), len);
+	path[len] = '\0';
+
+	return path;
+}
+
+static void bootmenu_populate_devs_alias(const char *alias)
+{
+	int idx;
+
+	for (idx = 0; idx <= 9 && nr_devs < MAX_DEVS; idx++, nr_devs++) {
+		char *cur_alias = bootdevs[nr_devs].alias;
+		if (idx == 0)
+			strcpy(cur_alias, alias);
+		else
+			sprintf(cur_alias, "%s%i", alias, idx);
+		bootdevs[nr_devs].path = find_alias(cur_alias);
+		if (!bootdevs[nr_devs].path)
+			break;
+	}
+}
+
+static void bootmenu_populate_devs(void)
+{
+	bootmenu_populate_devs_alias("cdrom");
+	bootmenu_populate_devs_alias("disk");
+	bootmenu_populate_devs_alias("net");
+}
+
+static void bootmenu_free_devs(void)
+{
+	while (nr_devs-- > 0) {
+		free(bootdevs[nr_devs].path);
+		bootdevs[nr_devs].path = NULL;
+	}
+}
+
+static void bootmenu_show_devs(void)
+{
+	int i;
+
+	for (i = 0; i < nr_devs; i++) {
+		printf("%c) %6s : %s\n", i < 9 ? '1' + i : 'a' + i - 9,
+		       bootdevs[i].alias, bootdevs[i].path);
+	}
+}
+
+static bool has_key(void)
+{
+	forth_eval("key?");
+	return forth_pop();
+}
+
+static char get_key(void)
+{
+	forth_eval("key");
+	return forth_pop();
+}
+
+/* Flush pending key presses */
+static void flush_keys(void)
+{
+	uint32_t start;
+
+	start = SLOF_GetTimer();
+	while (SLOF_GetTimer() - start < 10) {
+		if (has_key()) {
+			get_key();
+			start = SLOF_GetTimer();
+		}
+	}
+}
+
+static int bootmenu_get_selection(void)
+{
+	char key = 0;
+	int sel;
+
+	do {
+		sel = -1;
+		if (!has_key())
+			continue;
+		key = get_key();
+		switch (key) {
+		case '0':
+			return -1;
+		case '1' ... '9':
+			sel = key - '1';
+			break;
+		case 'a' ... 'z':
+			sel = key - 'a' + 9;
+			break;
+		case 'A' ... 'Z':
+			sel = key - 'A' + 9;
+			break;
+		default:
+			/* Might be another escape code (F12) ... skip it */
+			flush_keys();
+			break;
+		}
+	} while (sel < 0 || sel >= nr_devs);
+
+	return sel;
+}
+
+void bootmenu(void)
+{
+	int sel;
+
+	bootmenu_populate_devs();
+	if (!nr_devs) {
+		puts("No available boot devices!");
+		return;
+	}
+
+	puts("\nSelect boot device (or press '0' to abort):");
+	bootmenu_show_devs();
+
+	if (has_key())		/* In case the user hammered on F12 */
+		flush_keys();
+
+	sel = bootmenu_get_selection();
+	if (sel < 0) {
+		forth_push(0);
+	} else {
+		forth_push((unsigned long)bootdevs[sel].alias);
+		forth_push(strlen(bootdevs[sel].alias));
+	}
+
+	bootmenu_free_devs();
+}
diff --git a/lib/libbootmenu/bootmenu.code b/lib/libbootmenu/bootmenu.code
new file mode 100644
index 0000000..2a55c09
--- /dev/null
+++ b/lib/libbootmenu/bootmenu.code
@@ -0,0 +1,20 @@ 
+/*****************************************************************************
+ * Boot menu: Glue code to Forth
+ *
+ * Copyright 2017 Red Hat, Inc.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the BSD License
+ * which accompanies this distribution, and is available at
+ * http://www.opensource.org/licenses/bsd-license.php
+ *
+ * Contributors:
+ *     Thomas Huth, Red Hat Inc. - initial implementation
+ *****************************************************************************/
+
+#include "bootmenu.h"
+
+// ( -- [str] len|0 )
+PRIM(boot_X2d_menu)
+	bootmenu();
+MIRP
diff --git a/lib/libbootmenu/bootmenu.h b/lib/libbootmenu/bootmenu.h
new file mode 100644
index 0000000..6cef237
--- /dev/null
+++ b/lib/libbootmenu/bootmenu.h
@@ -0,0 +1,15 @@ 
+/*****************************************************************************
+ * Boot menu definitions
+ *
+ * Copyright 2017 Red Hat, Inc.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the BSD License
+ * which accompanies this distribution, and is available at
+ * http://www.opensource.org/licenses/bsd-license.php
+ *
+ * Contributors:
+ *     Thomas Huth, Red Hat Inc. - initial implementation
+ *****************************************************************************/
+
+extern void bootmenu(void);
diff --git a/lib/libbootmenu/bootmenu.in b/lib/libbootmenu/bootmenu.in
new file mode 100644
index 0000000..5cb120e
--- /dev/null
+++ b/lib/libbootmenu/bootmenu.in
@@ -0,0 +1,15 @@ 
+/*****************************************************************************
+ * Boot menu: Definitions for Forth
+ *
+ * Copyright 2017 Red Hat, Inc.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the BSD License
+ * which accompanies this distribution, and is available at
+ * http://www.opensource.org/licenses/bsd-license.php
+ *
+ * Contributors:
+ *     Thomas Huth, Red Hat Inc. - initial implementation
+ *****************************************************************************/
+
+cod(boot-menu)
diff --git a/slof/fs/start-up.fs b/slof/fs/start-up.fs
index dc5d1ed..7020f5c 100644
--- a/slof/fs/start-up.fs
+++ b/slof/fs/start-up.fs
@@ -65,70 +65,17 @@ 
 \ Watchdog will be rearmed during load if use-load-watchdog variable is TRUE
 TRUE VALUE use-load-watchdog?
 
-1 value my-boot-dev
-1 value digit-val
-0 value boot-dev-no
-
-: boot-selected
-   1 to my-boot-dev
-   BEGIN parse-word dup WHILE
-      boot-dev-no my-boot-dev = IF
-         s" boot " 2swap $cat
-         ['] evaluate catch ?dup IF     \ and execute it
-            ." boot attempt returned: "
-            abort"-str @ count type cr
-            throw
-         THEN
-         0 0 load-list 2!
-         UNLOOP EXIT
-      ELSE
-         2drop
-      THEN
-      my-boot-dev 1 + to my-boot-dev
-   REPEAT 2drop 0 0 load-list 2!
-
-   (boot)
-;
-
-: boot-start
-   decimal
-   BEGIN parse-word dup WHILE
-      my-boot-dev (u.) s" . " $cat type 2dup type ." : " de-alias type cr
-      my-boot-dev 1 + to my-boot-dev
-   REPEAT 2drop 0 0 load-list 2!
-
-   \ Clear pending keys (to remove multiple F12 key presses for example)
-   BEGIN key? WHILE
-      key drop
-   REPEAT
-
-   cr
-   BEGIN
-      KEY
-      dup 1b = IF         \ ESC sequence ... could be yet another F12 key press
-         BEGIN key? WHILE
-            key drop
-         REPEAT
-      ELSE
-         dup emit
-      THEN
-      dup isdigit IF
-         dup 30 - to digit-val
-         boot-dev-no a * digit-val + to boot-dev-no
-      THEN
-   d = UNTIL
-
-   boot-dev-no my-boot-dev < IF
-      s" boot-selected " s" $bootdev" evaluate $cat strdup evaluate
-   ELSE
-      ." Invalid choice!" cr
-   THEN
-   hex
-;
 
 : boot-menu-start
-   ." Select boot device:" cr cr
-   s" boot-start " s" $bootdev" evaluate $cat strdup evaluate
+    boot-menu ?dup IF
+       s" boot " 2swap $cat
+       ['] evaluate catch ?dup IF
+           ." boot attempt returned: "
+           abort"-str @ count type cr
+           throw
+       THEN
+       0 0 load-list 2!
+    THEN
 ;
 
 : boot-menu-enabled? ( -- true|false )