Merge patch series "boot: Support priority for global bootmeths"

Simon Glass <[email protected]> says:

At present global bootmeths always run first, before all other
bootmeths. Optimisations in the code take advantage of this, putting
them at the end, so they can be used once and then forgotten.

In some cases it is useful to run global bootmeths later in the boot.
For example, the EFI-bootmgr bootmeth may itself scan devices and the
network, so running it first can hold up the boot significantly for
boards not actually relying on EFI-bootmgr to boot.

This series introduces a new field in global bootmeths which indicates
the priority, using the same scheme as is used with bootdev hunters.
Thus it is possible to insert the EFI-bootmgr bootmeth just before the
hunter for network bootdevs is invoked.

Despite the simplicity of the concept and the relatively small series,
this is a fairly significant enhancement. It is also quite tricky to
implement, largely due to the way the original code was written, with
global bootmeths being a small, size-optimised add-on to the original
bootstd implementation.

For now we only allow each global bootmeth to run at most once, but this
implementation is written in a way that we could relax that if needed.
Then the bootmeth itself could decide whether to run at any particular
point in the bootflow iteration.

Link: https://lore.kernel.org/r/[email protected]

3 files changed, 185 insertions, 16 deletions

diff --git a/boot/bootflow.c b/boot/bootflow.c
index 7ed076c898f..d8a4a81a838 100644
--- a/boot/bootflow.c
+++ b/boot/bootflow.c
@@ -17,6 +17,10 @@
 #include <dm/device-internal.h>
 #include <dm/uclass-internal.h>
 
+/* ensure BOOTMETH_MAX_COUNT fits in method_flags field */
+static_assert(BOOTMETH_MAX_COUNT <=
+	      (sizeof(((struct bootflow_iter *)NULL)->method_flags) * 8));
+
 /* error codes used to signal running out of things */
 enum {
 	BF_NO_MORE_PARTS	= -ESHUTDOWN,
@@ -109,11 +113,17 @@ int bootflow_iter_drop_bootmeth(struct bootflow_iter *iter,
 	    iter->method_order[iter->cur_method] != bmeth)
 		return -EINVAL;
 
+	log_debug("Dropping bootmeth '%s'\n", bmeth->name);
+
 	memmove(&iter->method_order[iter->cur_method],
 		&iter->method_order[iter->cur_method + 1],
 		(iter->num_methods - iter->cur_method - 1) * sizeof(void *));
 
 	iter->num_methods--;
+	if (iter->first_glob_method > 0) {
+		iter->first_glob_method--;
+		log_debug("first_glob_method %d\n", iter->first_glob_method);
+	}
 
 	return 0;
 }
@@ -178,18 +188,87 @@ static void scan_next_in_uclass(struct udevice **devp)
 }
 
 /**
+ * bootmeth_glob_allowed() - Check if a global bootmeth is usable at this point
+ *
+ * @iter: Bootflow iterator being used
+ * Return: true if the global bootmeth has a suitable priority and has not
+ * already been used
+ */
+static bool bootmeth_glob_allowed(struct bootflow_iter *iter, int meth_seq)
+{
+	struct udevice *meth = iter->method_order[meth_seq];
+	bool done = iter->methods_done & BIT(meth_seq);
+	struct bootmeth_uc_plat *ucp;
+
+	ucp = dev_get_uclass_plat(meth);
+	log_debug("considering glob '%s': done %d glob_prio %d\n", meth->name,
+		  done, ucp->glob_prio);
+
+	/*
+	 * if this one has already been used, or its priority is too low, try
+	 * the next
+	 */
+	if (done || ucp->glob_prio > iter->cur_prio)
+		return false;
+
+	return true;
+}
+
+/**
+ * next_glob_bootmeth() - Find the next global bootmeth to use
+ *
+ * Scans the global bootmeths to find the first unused one whose priority has
+ * been reached. If found, iter->cur_method and iter->method are set up and
+ * doing_global is set to true
+ *
+ * @iter: Bootflow iterator being used
+ * Return 0 if found, -ENOENT if no more global bootmeths are available
+ */
+static int next_glob_bootmeth(struct bootflow_iter *iter)
+{
+	log_debug("rescan global bootmeths have_global %d\n",
+		  iter->have_global);
+	if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) && iter->have_global) {
+		int i;
+
+		/* rescan the global bootmeths */
+		log_debug("first_glob_method %d num_methods %d methods_done %x\n",
+			  iter->first_glob_method, iter->num_methods,
+			  iter->methods_done);
+		for (i = iter->first_glob_method; i < iter->num_methods; i++) {
+			if (bootmeth_glob_allowed(iter, i)) {
+				iter->cur_method = i;
+				iter->method = iter->method_order[i];
+				iter->doing_global = true;
+				iter->dev = NULL;
+				return 0;
+			}
+		}
+	}
+
+	return -ENOENT;
+}
+
+/**
  * prepare_bootdev() - Get ready to use a bootdev
  *
  * @iter: Bootflow iterator being used
  * @dev: UCLASS_BOOTDEV device to use
  * @method_flags: Method flag for the bootdev
+ * @check_global: true to check global bootmeths before processing @dev
  * Return 0 if OK, -ve if the bootdev failed to probe
  */
 static int prepare_bootdev(struct bootflow_iter *iter, struct udevice *dev,
-			   int method_flags)
+			   int method_flags, bool check_global)
 {
 	int ret;
 
+	if (check_global && !next_glob_bootmeth(iter)) {
+		iter->pending_bootdev = dev;
+		iter->pending_method_flags = method_flags;
+		return 0;
+	}
+
 	/*
 	 * Probe the bootdev. This does not probe any attached block device,
 	 * since they are siblings
@@ -221,27 +300,77 @@ static int iter_incr(struct bootflow_iter *iter)
 	if (iter->err == BF_NO_MORE_DEVICES)
 		return BF_NO_MORE_DEVICES;
 
-	if (iter->err != BF_NO_MORE_PARTS) {
-		/* Get the next boothmethod */
-		if (++iter->cur_method < iter->num_methods) {
+	/* Get the next boothmethod */
+	for (iter->cur_method++; iter->cur_method < iter->num_methods;
+	     iter->cur_method++) {
+		/* loop until we find a global bootmeth we haven't used */
+		if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) && iter->doing_global) {
+			if (!bootmeth_glob_allowed(iter, iter->cur_method))
+				continue;
+
 			iter->method = iter->method_order[iter->cur_method];
+			log_debug("-> next global method '%s'\n",
+				  iter->method->name);
 			return 0;
 		}
 
+		/* at this point we are only considering non-global bootmeths */
+		if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) && iter->have_global &&
+		    iter->cur_method >= iter->first_glob_method)
+			break;
+
+		iter->method = iter->method_order[iter->cur_method];
+		return 0;
+	}
+
+	/*
+	 * If we have finished scanning the global bootmeths, start the
+	 * normal bootdev scan
+	 */
+	if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) && global) {
+		iter->doing_global = false;
+
 		/*
-		 * If we have finished scanning the global bootmeths, start the
-		 * normal bootdev scan
+		 * we've come to the end, so see if we should use a pending
+		 * bootdev from when we decided to rescan the global bootmeths
 		 */
-		if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) && global) {
-			iter->num_methods = iter->first_glob_method;
-			iter->doing_global = false;
-
-			/*
-			 * Don't move to the next dev as we haven't tried this
-			 * one yet!
-			 */
-			inc_dev = false;
+		if (iter->pending_bootdev) {
+			int meth_flags = iter->pending_method_flags;
+
+			dev = iter->pending_bootdev;
+			iter->pending_bootdev = NULL;
+			iter->pending_method_flags = 0;
+
+			ret = prepare_bootdev(iter, dev, meth_flags, false);
+			if (ret)
+				return log_msg_ret("ipb", ret);
+
+			iter->cur_method = 0;
+			iter->method = iter->method_order[iter->cur_method];
+
+			log_debug("-> using pending bootdev '%s' method '%s'\n",
+				  dev->name, iter->method->name);
+
+			return 0;
+		}
+
+		/* if this was the final global bootmeth check, we are done */
+		if (iter->cur_prio == BOOTDEVP_COUNT) {
+			log_debug("-> done global bootmeths\n");
+
+			/* print the same message as bootflow_iter_set_dev() */
+			if ((iter->flags & (BOOTFLOWIF_SHOW |
+					    BOOTFLOWIF_SINGLE_DEV)) ==
+					    BOOTFLOWIF_SHOW)
+				printf("No more bootdevs\n");
+			return BF_NO_MORE_DEVICES;
 		}
+
+		/*
+		 * Don't move to the next dev as we haven't tried this
+		 * one yet!
+		 */
+		inc_dev = false;
 	}
 
 	if (iter->flags & BOOTFLOWIF_SINGLE_PARTITION)
@@ -339,7 +468,18 @@ static int iter_incr(struct bootflow_iter *iter)
 		if (ret)
 			bootflow_iter_set_dev(iter, NULL, 0);
 		else
-			ret = prepare_bootdev(iter, dev, method_flags);
+			ret = prepare_bootdev(iter, dev, method_flags, true);
+	}
+
+	if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) && ret) {
+		log_debug("no more bootdevs, trying global\n");
+
+		/* allow global bootmeths with any priority */
+		iter->cur_prio = BOOTDEVP_COUNT;
+		if (!next_glob_bootmeth(iter)) {
+			log_debug("-> next method '%s'\n", iter->method->name);
+			return 0;
+		}
 	}
 
 	/* if there are no more bootdevs, give up */
@@ -429,6 +569,10 @@ int bootflow_scan_first(struct udevice *dev, const char *label,
 		bootflow_iter_set_dev(iter, dev, method_flags);
 	}
 
+	if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL)) {
+		iter->methods_done |= BIT(iter->cur_method);
+		log_debug("methods_done now %x\n", iter->cur_method);
+	}
 	ret = bootflow_check(iter, bflow);
 	if (ret) {
 		log_debug("check - ret=%d\n", ret);
@@ -456,6 +600,11 @@ int bootflow_scan_next(struct bootflow_iter *iter, struct bootflow *bflow)
 			return log_msg_ret("done", ret);
 
 		if (!ret) {
+			if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL)) {
+				iter->methods_done |= BIT(iter->cur_method);
+				log_debug("methods_done now %x\n",
+					  iter->cur_method);
+			}
 			ret = bootflow_check(iter, bflow);
 			log_debug("check - ret=%d\n", ret);
 			if (!ret)
diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c
index bb2dd8447cf..a9709465f6e 100644
--- a/boot/bootmeth-uclass.c
+++ b/boot/bootmeth-uclass.c
@@ -209,7 +209,18 @@ int bootmeth_setup_iter_order(struct bootflow_iter *iter, bool include_global)
 	    iter->first_glob_method != -1 && iter->first_glob_method != count) {
 		iter->cur_method = iter->first_glob_method;
 		iter->doing_global = true;
+		iter->have_global = true;
 	}
+
+	/*
+	 * check we don't exceed the maximum bits in methods_done when tracking
+	 * which global bootmeths have run
+	 */
+	if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) && count > BOOTMETH_MAX_COUNT) {
+		free(order);
+		return log_msg_ret("tmb", -ENOSPC);
+	}
+
 	iter->method_order = order;
 	iter->num_methods = count;
 
diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
index 42b8863815e..05fc35d01a9 100644
--- a/boot/bootmeth_efi_mgr.c
+++ b/boot/bootmeth_efi_mgr.c
@@ -99,6 +99,15 @@ static int bootmeth_efi_mgr_bind(struct udevice *dev)
 	plat->desc = "EFI bootmgr flow";
 	plat->flags = BOOTMETHF_GLOBAL;
 
+	/*
+	 * bootmgr scans all available devices which can take a while,
+	 * especially for network devices. So choose the priority so that it
+	 * comes just before the 'very slow' devices. This allows systems which
+	 * don't rely on bootmgr to boot quickly, while allowing bootmgr to run
+	 * on systems which need it.
+	 */
+	plat->glob_prio = BOOTDEVP_6_NET_BASE;
+
 	return 0;
 }