Merge branch '2022-10-31-vbe-implement-the-full-firmware-flow'

To quote Simon:
This series provides an implementation of VBE from TPL through to U-Boot
proper, using VBE to load the relevant firmware stages. It buils a single
image.bin file containing all the phases:

   TPL - initial phase, loads VPL using binman symbols
   VPL - main firmware phase, loads SPL using VBE parameters
   SPL - loads U-Boot proper using VBE parameters
   U-Boot - final firmware phase, where OS booting is processed

This series does not include the OS-booting phase. That will be the
subject of a future series.

The implementation is entirely handled by sandbox. It should be possible
to enable this on a real board without much effort, but that is also the
subject of a future series.

16 files changed, 215 insertions, 49 deletions

diff --git a/test/boot/Makefile b/test/boot/Makefile
index 5bb3f88975..d724629d3b 100644
--- a/test/boot/Makefile
+++ b/test/boot/Makefile
@@ -3,6 +3,7 @@
 # Copyright 2021 Google LLC
 
 obj-$(CONFIG_BOOTSTD) += bootdev.o bootstd_common.o bootflow.o bootmeth.o
+obj-$(CONFIG_FIT) += image.o
 
 ifdef CONFIG_OF_LIVE
 obj-$(CONFIG_BOOTMETH_VBE_SIMPLE) += vbe_simple.o
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 1e8ea754bc..e1e0708210 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -330,7 +330,7 @@ static int bootflow_system(struct unit_test_state *uts)
 	struct udevice *dev;
 
 	if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR))
-		return 0;
+		return -EAGAIN;
 	ut_assertok(uclass_get_device_by_name(UCLASS_BOOTMETH, "efi_mgr",
 					      &dev));
 	sandbox_set_fake_efi_mgr_dev(dev, true);
@@ -395,7 +395,7 @@ BOOTSTD_TEST(bootflow_iter_disable, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
 static int bootflow_scan_glob_bootmeth(struct unit_test_state *uts)
 {
 	if (!IS_ENABLED(CONFIG_BOOTMETH_GLOBAL))
-		return 0;
+		return -EAGAIN;
 
 	ut_assertok(bootstd_test_drop_bootdev_order(uts));
 
diff --git a/test/boot/bootmeth.c b/test/boot/bootmeth.c
index f0b5ab9adb..0098ef3efd 100644
--- a/test/boot/bootmeth.c
+++ b/test/boot/bootmeth.c
@@ -103,10 +103,17 @@ static int bootmeth_cmd_order(struct unit_test_state *uts)
 	ut_asserteq_str("efi syslinux", env_get("bootmeths"));
 	ut_assert_console_end();
 
-	/* Try with global bootmeths */
+	return 0;
+}
+BOOTSTD_TEST(bootmeth_cmd_order, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+
+/* Check 'bootmeth order' command with global bootmeths */
+static int bootmeth_cmd_order_glob(struct unit_test_state *uts)
+{
 	if (!IS_ENABLED(CONFIG_BOOTMETH_GLOBAL))
-		return 0;
+		return -EAGAIN;
 
+	console_record_reset_enable();
 	ut_assertok(run_command("bootmeth order \"efi firmware0\"", 0));
 	ut_assert_console_end();
 	ut_assertok(run_command("bootmeth list", 0));
@@ -122,7 +129,7 @@ static int bootmeth_cmd_order(struct unit_test_state *uts)
 
 	return 0;
 }
-BOOTSTD_TEST(bootmeth_cmd_order, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+BOOTSTD_TEST(bootmeth_cmd_order_glob, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
 
 /* Check 'bootmeths' env var */
 static int bootmeth_env(struct unit_test_state *uts)
diff --git a/test/boot/image.c b/test/boot/image.c
new file mode 100644
index 0000000000..2844b05785
--- /dev/null
+++ b/test/boot/image.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Test for vbe-simple bootmeth. All start with 'vbe_simple'
+ *
+ * Copyright 2023 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <common.h>
+#include <image.h>
+#include <test/suites.h>
+#include <test/ut.h>
+#include "bootstd_common.h"
+
+/* Test of image phase */
+static int test_image_phase(struct unit_test_state *uts)
+{
+	int val;
+
+	ut_asserteq_str("U-Boot phase", genimg_get_phase_name(IH_PHASE_U_BOOT));
+	ut_asserteq_str("SPL Phase", genimg_get_phase_name(IH_PHASE_SPL));
+	ut_asserteq_str("any", genimg_get_phase_name(IH_PHASE_NONE));
+	ut_asserteq_str("Unknown Phase", genimg_get_phase_name(-1));
+
+	ut_asserteq(IH_PHASE_U_BOOT, genimg_get_phase_id("u-boot"));
+	ut_asserteq(IH_PHASE_SPL, genimg_get_phase_id("spl"));
+	ut_asserteq(IH_PHASE_NONE, genimg_get_phase_id("none"));
+	ut_asserteq(-1, genimg_get_phase_id("fred"));
+
+	val = image_ph(IH_PHASE_SPL, IH_TYPE_FIRMWARE);
+	ut_asserteq(IH_PHASE_SPL, image_ph_phase(val));
+	ut_asserteq(IH_TYPE_FIRMWARE, image_ph_type(val));
+
+	return 0;
+}
+BOOTSTD_TEST(test_image_phase, 0);
diff --git a/test/boot/vbe_fixup.c b/test/boot/vbe_fixup.c
index 1b488e25ab..eba5c4ebe6 100644
--- a/test/boot/vbe_fixup.c
+++ b/test/boot/vbe_fixup.c
@@ -13,21 +13,18 @@
 #include <test/ut.h>
 #include "bootstd_common.h"
 
-/* Basic test of reading nvdata and updating a fwupd node in the device tree */
-static int vbe_test_fixup(struct unit_test_state *uts)
+/*
+ * Basic test of reading nvdata and updating a fwupd node in the device tree
+ * This test works when called from test_vbe.py and it must use the flat tree,
+ * since device tree fix-ups do not yet support live tree.
+ */
+static int vbe_test_fixup_norun(struct unit_test_state *uts)
 {
 	ofnode chosen, node;
 	const char *data;
 	oftree tree;
 	int size;
 
-	/*
-	 * This test works when called from test_vbe.py and it must use the
-	 * flat tree, since device tree fix-ups do not yet support live tree.
-	 */
-	if (!working_fdt)
-		return 0;
-
 	tree = oftree_from_fdt(working_fdt);
 	ut_assert(oftree_valid(tree));
 
@@ -55,5 +52,5 @@ static int vbe_test_fixup(struct unit_test_state *uts)
 
 	return 0;
 }
-BOOTSTD_TEST(vbe_test_fixup,
-	     UT_TESTF_DM | UT_TESTF_SCAN_FDT | UT_TESTF_FLAT_TREE);
+BOOTSTD_TEST(vbe_test_fixup_norun, UT_TESTF_DM | UT_TESTF_SCAN_FDT |
+	     UT_TESTF_FLAT_TREE | UT_TESTF_MANUAL);
diff --git a/test/boot/vbe_simple.c b/test/boot/vbe_simple.c
index faba9e8f90..5e61840652 100644
--- a/test/boot/vbe_simple.c
+++ b/test/boot/vbe_simple.c
@@ -16,7 +16,12 @@
 #include <test/ut.h>
 #include "bootstd_common.h"
 
-/* Basic test of reading nvdata and updating a fwupd node in the device tree */
+/*
+ * Basic test of reading nvdata and updating a fwupd node in the device tree
+ *
+ * This sets up its own VBE info in the device, using bootstd_setup_for_tests()
+ * then does a VBE fixup and checks that everything is present.
+ */
 static int vbe_simple_test_base(struct unit_test_state *uts)
 {
 	const char *version, *bl_version;
@@ -77,7 +82,7 @@ static int vbe_simple_test_base(struct unit_test_state *uts)
 
 	bl_version = ofnode_read_string(node, "bootloader-version");
 	ut_assertnonnull(bl_version);
-	ut_asserteq_str(version_string, bl_version);
+	ut_asserteq_str(version_string + 7, bl_version);
 
 	return 0;
 }
diff --git a/test/cmd_ut.c b/test/cmd_ut.c
index dc88c5fb88..beebd5ce38 100644
--- a/test/cmd_ut.c
+++ b/test/cmd_ut.c
@@ -19,16 +19,26 @@ int cmd_ut_category(const char *name, const char *prefix,
 		    int argc, char *const argv[])
 {
 	int runs_per_text = 1;
+	bool force_run = false;
 	int ret;
 
-	if (argc > 1 && !strncmp("-r", argv[1], 2)) {
-		runs_per_text = dectoul(argv[1] + 2, NULL);
+	while (argc > 1 && *argv[1] == '-') {
+		const char *str = argv[1];
+
+		switch (str[1]) {
+		case 'r':
+			runs_per_text = dectoul(str + 2, NULL);
+			break;
+		case 'f':
+			force_run = true;
+			break;
+		}
 		argv++;
 		argc++;
 	}
 
 	ret = ut_run_list(name, prefix, tests, n_ents,
-			  argc > 1 ? argv[1] : NULL, runs_per_text);
+			  argc > 1 ? argv[1] : NULL, runs_per_text, force_run);
 
 	return ret ? CMD_RET_FAILURE : 0;
 }
diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c
index 7af798b8d3..a241c42793 100644
--- a/test/dm/of_platdata.c
+++ b/test/dm/of_platdata.c
@@ -150,7 +150,7 @@ static int dm_test_of_plat_dev(struct unit_test_state *uts)
 
 	/* Skip this test if there is no platform data */
 	if (!CONFIG_IS_ENABLED(OF_PLATDATA_DRIVER_RT))
-		return 0;
+		return -EAGAIN;
 
 	/* Record the indexes that are found */
 	memset(found, '\0', sizeof(found));
diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c
index 41811ec3bb..8077affabb 100644
--- a/test/dm/ofnode.c
+++ b/test/dm/ofnode.c
@@ -753,10 +753,7 @@ static int make_ofnode_fdt(struct unit_test_state *uts, void *fdt, int size,
 
 static int dm_test_ofnode_root(struct unit_test_state *uts)
 {
-	char fdt[256];
-	oftree tree;
 	ofnode node;
-	int ret;
 
 	/* Check that aliases work on the control FDT */
 	node = ofnode_get_aliases_node("ethernet3");
@@ -765,14 +762,22 @@ static int dm_test_ofnode_root(struct unit_test_state *uts)
 
 	ut_assert(!oftree_valid(oftree_null()));
 
-	ut_assertok(make_ofnode_fdt(uts, fdt, sizeof(fdt), 0));
-	ret = get_oftree(uts, fdt, &tree);
+	return 0;
+}
+DM_TEST(dm_test_ofnode_root, UT_TESTF_SCAN_FDT);
 
-	/* skip the rest of this test if multiple FDTs are not supported */
-	if (ret == -EOVERFLOW)
-		return 0;
+static int dm_test_ofnode_root_mult(struct unit_test_state *uts)
+{
+	char fdt[256];
+	oftree tree;
+	ofnode node;
 
-	ut_assertok(ret);
+	/* skip this test if multiple FDTs are not supported */
+	if (!IS_ENABLED(CONFIG_OFNODE_MULTI_TREE))
+		return -EAGAIN;
+
+	ut_assertok(make_ofnode_fdt(uts, fdt, sizeof(fdt), 0));
+	ut_assertok(get_oftree(uts, fdt, &tree));
 	ut_assert(oftree_valid(tree));
 
 	/* Make sure they don't work on this new tree */
@@ -791,7 +796,7 @@ static int dm_test_ofnode_root(struct unit_test_state *uts)
 
 	return 0;
 }
-DM_TEST(dm_test_ofnode_root, UT_TESTF_SCAN_FDT);
+DM_TEST(dm_test_ofnode_root_mult, UT_TESTF_SCAN_FDT);
 
 static int dm_test_ofnode_livetree_writing(struct unit_test_state *uts)
 {
diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c
index eb3581333b..66cc2bc6cc 100644
--- a/test/dm/test-dm.c
+++ b/test/dm/test-dm.c
@@ -36,7 +36,7 @@ static int dm_test_run(const char *test_name, int runs_per_text)
 	int ret;
 
 	ret = ut_run_list("driver model", "dm_test_", tests, n_ents, test_name,
-			  runs_per_text);
+			  runs_per_text, false);
 
 	return ret ? CMD_RET_FAILURE : 0;
 }
diff --git a/test/dm/usb.c b/test/dm/usb.c
index 5d6ceefce0..7671ef156d 100644
--- a/test/dm/usb.c
+++ b/test/dm/usb.c
@@ -43,20 +43,42 @@ DM_TEST(dm_test_usb_base, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
  */
 static int dm_test_usb_flash(struct unit_test_state *uts)
 {
-	struct udevice *dev;
-	struct blk_desc *dev_desc;
+	struct blk_desc *dev_desc, *chk;
+	struct udevice *dev, *blk;
 	char cmp[1024];
 
 	state_set_skip_delays(true);
 	ut_assertok(usb_init());
 	ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 0, &dev));
 	ut_assertok(blk_get_device_by_str("usb", "0", &dev_desc));
+	chk = blk_get_by_device(dev);
+	ut_asserteq_ptr(chk, dev_desc);
+
+	ut_assertok(device_find_first_child_by_uclass(dev, UCLASS_BLK, &blk));
+	ut_asserteq_ptr(chk, blk_get_by_device(dev));
 
 	/* Read a few blocks and look for the string we expect */
 	ut_asserteq(512, dev_desc->blksz);
 	memset(cmp, '\0', sizeof(cmp));
-	ut_asserteq(2, blk_dread(dev_desc, 0, 2, cmp));
-	ut_assertok(strcmp(cmp, "this is a test"));
+	ut_asserteq(2, blk_read(blk, 0, 2, cmp));
+	ut_asserteq_str("this is a test", cmp);
+
+	strcpy(cmp, "another test");
+	ut_asserteq(1, blk_write(blk, 1, 1, cmp));
+
+	memset(cmp, '\0', sizeof(cmp));
+	ut_asserteq(2, blk_read(blk, 0, 2, cmp));
+	ut_asserteq_str("this is a test", cmp);
+	ut_asserteq_str("another test", cmp + 512);
+
+	memset(cmp, '\0', sizeof(cmp));
+	ut_asserteq(1, blk_write(blk, 1, 1, cmp));
+
+	memset(cmp, '\0', sizeof(cmp));
+	ut_asserteq(2, blk_read(blk, 0, 2, cmp));
+	ut_asserteq_str("this is a test", cmp);
+	ut_asserteq_str("", cmp + 512);
+
 	ut_assertok(usb_stop());
 
 	return 0;
diff --git a/test/py/conftest.py b/test/py/conftest.py
index 304e93164a..fc9dd3a83f 100644
--- a/test/py/conftest.py
+++ b/test/py/conftest.py
@@ -289,7 +289,13 @@ def generate_ut_subtest(metafunc, fixture_name, sym_path):
         m = re_ut_test_list.search(l)
         if not m:
             continue
-        vals.append(m.group(1) + ' ' + m.group(2))
+        suite, name = m.groups()
+
+        # Tests marked with _norun should only be run manually using 'ut -f'
+        if name.endswith('_norun'):
+            continue
+
+        vals.append(f'{suite} {name}')
 
     ids = ['ut_' + s.replace(' ', '_') for s in vals]
     metafunc.parametrize(fixture_name, vals, ids=ids)
diff --git a/test/py/tests/test_event_dump.py b/test/py/tests/test_event_dump.py
index 674df2ea00..1a46ca30f4 100644
--- a/test/py/tests/test_event_dump.py
+++ b/test/py/tests/test_event_dump.py
@@ -16,7 +16,7 @@ def test_event_dump(u_boot_console):
     out = util.run_and_log(cons, ['scripts/event_dump.py', sandbox])
     expect = '''.*Event type            Id                              Source location
 --------------------  ------------------------------  ------------------------------
-EVT_FT_FIXUP          bootmeth_vbe_ft_fixup           .*vbe_fixup.c:.*
-EVT_FT_FIXUP          bootmeth_vbe_simple_ft_fixup    .*vbe_simple.c:.*
+EVT_FT_FIXUP          bootmeth_vbe_ft_fixup           .*vbe_request.c:.*
+EVT_FT_FIXUP          bootmeth_vbe_simple_ft_fixup    .*vbe_simple_os.c:.*
 EVT_MISC_INIT_F       sandbox_misc_init_f             .*start.c:'''
     assert re.match(expect, out, re.MULTILINE) is not None
diff --git a/test/py/tests/test_vbe.py b/test/py/tests/test_vbe.py
index 559c291886..50b6c1cd91 100644
--- a/test/py/tests/test_vbe.py
+++ b/test/py/tests/test_vbe.py
@@ -85,7 +85,7 @@ bootm loados
 bootm prep
 fdt addr
 fdt print
-ut bootstd vbe_test_fixup
+ut bootstd -f vbe_test_fixup_norun
 '''
 
 @pytest.mark.boardspec('sandbox_flattree')
@@ -117,7 +117,4 @@ def test_vbe(u_boot_console):
     with cons.log.section('Kernel load'):
         output = cons.run_command_list(cmd.splitlines())
 
-    # This is a little wonky since there are two tests running in CI. The final
-    # one is the 'ut bootstd' command above
-    failures = [line for line in output if 'Failures' in line]
-    assert len(failures) >= 1 and 'Failures: 0' in failures[-1]
+    assert 'Failures: 0' in output[-1]
diff --git a/test/py/tests/test_vbe_vpl.py b/test/py/tests/test_vbe_vpl.py
new file mode 100644
index 0000000000..d1c9d0548a
--- /dev/null
+++ b/test/py/tests/test_vbe_vpl.py
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2022 Google LLC
+#
+# Test addition of VBE
+
+import os
+
+import pytest
+import u_boot_utils
+
+@pytest.mark.boardspec('sandbox_vpl')
+@pytest.mark.requiredtool('dtc')
+def test_vbe_vpl(u_boot_console):
+    cons = u_boot_console
+    #cmd = [cons.config.build_dir + fname, '-v']
+    ram = os.path.join(cons.config.build_dir, 'ram.bin')
+    fdt = os.path.join(cons.config.build_dir, 'arch/sandbox/dts/test.dtb')
+
+    # Enable firmware1 and the mmc that it uses. These are needed for the full
+    # VBE flow.
+    u_boot_utils.run_and_log(
+        cons, f'fdtput -t s {fdt} /bootstd/firmware0 status disabled')
+    u_boot_utils.run_and_log(
+        cons, f'fdtput -t s {fdt} /bootstd/firmware1 status okay')
+    u_boot_utils.run_and_log(
+        cons, f'fdtput -t s {fdt} /mmc3 status okay')
+
+    # Remove any existing RAM file, so we don't have old data present
+    if os.path.exists(ram):
+        os.remove(ram)
+    flags = ['-p', os.path.join(cons.config.build_dir, 'image.bin'), '-w',
+             '-s', 'state.dtb']
+    cons.restart_uboot_with_flags(flags)
+
+    # Make sure that VBE was used in both VPL (to load SPL) and SPL (to load
+    # U-Boot
+    output = cons.run_command('vbe state')
+    assert output == 'Phases: VPL SPL'
diff --git a/test/test-main.c b/test/test-main.c
index a98a77d68f..ddfd89c089 100644
--- a/test/test-main.c
+++ b/test/test-main.c
@@ -357,6 +357,19 @@ static int test_post_run(struct unit_test_state *uts, struct unit_test *test)
 }
 
 /**
+ * skip_test() - Handle skipping a test
+ *
+ * @uts: Test state to update
+ * @return -EAGAIN (always)
+ */
+static int skip_test(struct unit_test_state *uts)
+{
+	uts->skip_count++;
+
+	return -EAGAIN;
+}
+
+/**
  * ut_run_test() - Run a single test
  *
  * This runs the test, handling any preparation and clean-up needed. It prints
@@ -386,11 +399,13 @@ static int ut_run_test(struct unit_test_state *uts, struct unit_test *test,
 
 	ret = test_pre_run(uts, test);
 	if (ret == -EAGAIN)
-		return -EAGAIN;
+		return skip_test(uts);
 	if (ret)
 		return ret;
 
-	test->func(uts);
+	ret = test->func(uts);
+	if (ret == -EAGAIN)
+		skip_test(uts);
 
 	ret = test_post_run(uts, test);
 	if (ret)
@@ -424,7 +439,7 @@ static int ut_run_test_live_flat(struct unit_test_state *uts,
 	int runs;
 
 	if ((test->flags & UT_TESTF_OTHER_FDT) && !IS_ENABLED(CONFIG_SANDBOX))
-		return -EAGAIN;
+		return skip_test(uts);
 
 	/* Run with the live tree if possible */
 	runs = 0;
@@ -493,6 +508,30 @@ static int ut_run_tests(struct unit_test_state *uts, const char *prefix,
 
 		if (!test_matches(prefix, test_name, select_name))
 			continue;
+
+		if (test->flags & UT_TESTF_MANUAL) {
+			int len;
+
+			/*
+			 * manual tests must have a name ending "_norun" as this
+			 * is how pytest knows to skip them. See
+			 * generate_ut_subtest() for this check.
+			 */
+			len = strlen(test_name);
+			if (len < 6 || strcmp(test_name + len - 6, "_norun")) {
+				printf("Test %s is manual so must have a name ending in _norun\n",
+				       test_name);
+				uts->fail_count++;
+				return -EBADF;
+			}
+			if (!uts->force_run) {
+				if (select_name) {
+					printf("Test %s skipped as it is manual (use -f to run it)\n",
+					       test_name);
+				}
+				continue;
+			}
+		}
 		old_fail_count = uts->fail_count;
 		for (i = 0; i < uts->runs_per_test; i++)
 			ret = ut_run_test_live_flat(uts, test, select_name);
@@ -514,7 +553,7 @@ static int ut_run_tests(struct unit_test_state *uts, const char *prefix,
 
 int ut_run_list(const char *category, const char *prefix,
 		struct unit_test *tests, int count, const char *select_name,
-		int runs_per_test)
+		int runs_per_test, bool force_run)
 {
 	struct unit_test_state uts = { .fail_count = 0 };
 	bool has_dm_tests = false;
@@ -548,6 +587,7 @@ int ut_run_list(const char *category, const char *prefix,
 		}
 		memcpy(uts.fdt_copy, gd->fdt_blob, uts.fdt_size);
 	}
+	uts.force_run = force_run;
 	ret = ut_run_tests(&uts, prefix, tests, count, select_name);
 
 	/* Best efforts only...ignore errors */
@@ -558,6 +598,8 @@ int ut_run_list(const char *category, const char *prefix,
 		os_free(uts.other_fdt);
 	}
 
+	if (uts.skip_count)
+		printf("Skipped: %d, ", uts.skip_count);
 	if (ret == -ENOENT)
 		printf("Test '%s' not found\n", select_name);
 	else