From 1facaadea1871c1b6962272dd21569a4aafda18c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 20 Oct 2022 18:22:48 -0600 Subject: test: Report skippped tests At present it is possible for a test to skip itself by returning -EAGAIN but this is not recorded. An existing example is in test_pre_run() with the "Console recording disabled" check. Keep a track of skipped tests and report the total at the end. Signed-off-by: Simon Glass Acked-by: Heinrich Schuchardt --- test/test-main.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) (limited to 'test') diff --git a/test/test-main.c b/test/test-main.c index a98a77d68f..2323cbaece 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -356,6 +356,19 @@ static int test_post_run(struct unit_test_state *uts, struct unit_test *test) return 0; } +/** + * 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 * @@ -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; @@ -558,6 +573,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 -- cgit v1.2.3 From c43635bdbc6cb1e4ba2d9e2f28f7f3cb3b287bf8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 20 Oct 2022 18:22:49 -0600 Subject: test: Update tests to use the skip feature Some tests currently return 0 when they want to be skipped. Update them to return -EAGAIN instead, so they are counted as skipped. A few tests are in two parts, with the latter part being skipped in certain situations. Split these into two and use the correct condition for the second part. Signed-off-by: Simon Glass --- test/boot/bootflow.c | 4 ++-- test/boot/bootmeth.c | 13 ++++++++++--- test/boot/vbe_fixup.c | 2 +- test/dm/of_platdata.c | 2 +- test/dm/ofnode.c | 25 +++++++++++++++---------- 5 files changed, 29 insertions(+), 17 deletions(-) (limited to 'test') 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/vbe_fixup.c b/test/boot/vbe_fixup.c index 1b488e25ab..7f0f809499 100644 --- a/test/boot/vbe_fixup.c +++ b/test/boot/vbe_fixup.c @@ -26,7 +26,7 @@ static int vbe_test_fixup(struct unit_test_state *uts) * flat tree, since device tree fix-ups do not yet support live tree. */ if (!working_fdt) - return 0; + return -EAGAIN; tree = oftree_from_fdt(working_fdt); ut_assert(oftree_valid(tree)); 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) { -- cgit v1.2.3 From cbd71fad6d468018727ab04b2bb912989aec0785 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 20 Oct 2022 18:22:50 -0600 Subject: test: Support tests which can only be run manually At present we normally write tests either in Python or in C. But most Python tests end up doing a lot of checks which would be better done in C. Checks done in C are orders of magnitude faster and it is possible to get full access to U-Boot's internal workings, rather than just relying on the command line. The model is to have a Python test set up some things and then use C code (in a unit test) to check that they were done correctly. But we don't want those checks to happen as part of normal test running, since each C unit tests is dependent on the associate Python tests, so cannot run without it. To acheive this, add a new UT_TESTF_MANUAL flag to use with the C 'check' tests, so that they can be skipped by default when the 'ut' command is used. Require that tests have a name ending with '_norun', so that pytest knows to skip them. Signed-off-by: Simon Glass --- arch/sandbox/cpu/spl.c | 2 +- doc/develop/tests_writing.rst | 27 +++++++++++++++++++++++++++ include/test/test.h | 8 ++++++++ include/test/ut.h | 4 +++- test/cmd_ut.c | 16 +++++++++++++--- test/dm/test-dm.c | 2 +- test/py/conftest.py | 8 +++++++- test/test-main.c | 27 ++++++++++++++++++++++++++- 8 files changed, 86 insertions(+), 8 deletions(-) (limited to 'test') diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c index 1d49a9bd10..9c59cc2616 100644 --- a/arch/sandbox/cpu/spl.c +++ b/arch/sandbox/cpu/spl.c @@ -89,7 +89,7 @@ void spl_board_init(void) int ret; ret = ut_run_list("spl", NULL, tests, count, - state->select_unittests, 1); + state->select_unittests, 1, false); /* continue execution into U-Boot */ } } diff --git a/doc/develop/tests_writing.rst b/doc/develop/tests_writing.rst index 1ddf7a353a..bb1145da26 100644 --- a/doc/develop/tests_writing.rst +++ b/doc/develop/tests_writing.rst @@ -74,6 +74,33 @@ NOT rely on running with sandbox, but instead should function correctly on any board supported by U-Boot. +Mixing Python and C +------------------- + +The best of both worlds is sometimes to have a Python test set things up and +perform some operations, with a 'checker' C unit test doing the checks +afterwards. This can be achieved with these steps: + +- Add the `UT_TESTF_MANUAL` flag to the checker test so that the `ut` command + does not run it by default +- Add a `_norun` suffix to the name so that pytest knows to skip it too + +In your Python test use the `-f` flag to the `ut` command to force the checker +test to run it, e.g.:: + + # Do the Python part + host load ... + bootm ... + + # Run the checker to make sure that everything worked + ut -f bootstd vbe_test_fixup_norun + +Note that apart from the `UT_TESTF_MANUAL` flag, the code in a 'manual' C test +is just like any other C test. It still uses ut_assert...() and other such +constructs, in this case to check that the expected things happened in the +Python test. + + How slow are Python tests? -------------------------- diff --git a/include/test/test.h b/include/test/test.h index c1853ce471..4ad74614af 100644 --- a/include/test/test.h +++ b/include/test/test.h @@ -28,6 +28,7 @@ * @other_fdt_size: Size of the other FDT (UT_TESTF_OTHER_FDT) * @of_other: Live tree for the other FDT * @runs_per_test: Number of times to run each test (typically 1) + * @force_run: true to run tests marked with the UT_TESTF_MANUAL flag * @expect_str: Temporary string used to hold expected string value * @actual_str: Temporary string used to hold actual string value */ @@ -48,6 +49,7 @@ struct unit_test_state { int other_fdt_size; struct device_node *of_other; int runs_per_test; + bool force_run; char expect_str[512]; char actual_str[512]; }; @@ -63,6 +65,12 @@ enum { /* do extra driver model init and uninit */ UT_TESTF_DM = BIT(6), UT_TESTF_OTHER_FDT = BIT(7), /* read in other device tree */ + /* + * Only run if explicitly requested with 'ut -f '. The + * test name must end in "_norun" so that pytest detects this also, + * since it cannot access the flags. + */ + UT_TESTF_MANUAL = BIT(8), }; /** diff --git a/include/test/ut.h b/include/test/ut.h index f7217aa8ac..e0e618b58c 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -409,9 +409,11 @@ void test_set_state(struct unit_test_state *uts); * @select_name: Name of a single test to run (from the list provided). If NULL * then all tests are run * @runs_per_test: Number of times to run each test (typically 1) + * @force_run: Run tests that are marked as manual-only (UT_TESTF_MANUAL) * Return: 0 if all tests passed, -1 if any failed */ int ut_run_list(const char *name, const char *prefix, struct unit_test *tests, - int count, const char *select_name, int runs_per_test); + int count, const char *select_name, int runs_per_test, + bool force_run); #endif 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/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/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/test-main.c b/test/test-main.c index 2323cbaece..ddfd89c089 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -508,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); @@ -529,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; @@ -563,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 */ -- cgit v1.2.3 From 606b926f9d76eaab11be2d95cfd7734644e1627c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 20 Oct 2022 18:22:54 -0600 Subject: dm: blk: Add udevice functions At present we have functions called blk_dread(), etc., which take a struct blk_desc * to refer to the block device. Add some functions which use udevice instead, since this is more in keeping with how driver model is supposed to work. Update one of the tests to use this. Note that it would be nice to update the functions in disk-uclass.c to use these new functions. However they are not quite the same. For example, disk_blk_read() adds the partition offset to 'start' when calling the cache read/fill functions, but does not with part_blk_read(), which does the addition itself. So as designed the code is duplicated. Signed-off-by: Simon Glass --- drivers/block/blk-uclass.c | 51 ++++++++++++++++++++++++++++++---------------- include/blk.h | 39 ++++++++++++++++++++++++++++++++++- test/dm/usb.c | 11 +++++++--- 3 files changed, 80 insertions(+), 21 deletions(-) (limited to 'test') diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index bcc14a684b..e82789f4a3 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -444,53 +444,70 @@ int blk_get_device(int uclass_id, int devnum, struct udevice **devp) return device_probe(*devp); } -unsigned long blk_dread(struct blk_desc *block_dev, lbaint_t start, - lbaint_t blkcnt, void *buffer) +long blk_read(struct udevice *dev, lbaint_t start, lbaint_t blkcnt, void *buf) { - struct udevice *dev = block_dev->bdev; + struct blk_desc *desc = dev_get_uclass_plat(dev); const struct blk_ops *ops = blk_get_ops(dev); ulong blks_read; if (!ops->read) return -ENOSYS; - if (blkcache_read(block_dev->uclass_id, block_dev->devnum, - start, blkcnt, block_dev->blksz, buffer)) + if (blkcache_read(desc->uclass_id, desc->devnum, + start, blkcnt, desc->blksz, buf)) return blkcnt; - blks_read = ops->read(dev, start, blkcnt, buffer); + blks_read = ops->read(dev, start, blkcnt, buf); if (blks_read == blkcnt) - blkcache_fill(block_dev->uclass_id, block_dev->devnum, - start, blkcnt, block_dev->blksz, buffer); + blkcache_fill(desc->uclass_id, desc->devnum, start, blkcnt, + desc->blksz, buf); return blks_read; } -unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t start, - lbaint_t blkcnt, const void *buffer) +long blk_write(struct udevice *dev, lbaint_t start, lbaint_t blkcnt, + const void *buf) { - struct udevice *dev = block_dev->bdev; + struct blk_desc *desc = dev_get_uclass_plat(dev); const struct blk_ops *ops = blk_get_ops(dev); if (!ops->write) return -ENOSYS; - blkcache_invalidate(block_dev->uclass_id, block_dev->devnum); - return ops->write(dev, start, blkcnt, buffer); + blkcache_invalidate(desc->uclass_id, desc->devnum); + + return ops->write(dev, start, blkcnt, buf); } -unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start, - lbaint_t blkcnt) +long blk_erase(struct udevice *dev, lbaint_t start, lbaint_t blkcnt) { - struct udevice *dev = block_dev->bdev; + struct blk_desc *desc = dev_get_uclass_plat(dev); const struct blk_ops *ops = blk_get_ops(dev); if (!ops->erase) return -ENOSYS; - blkcache_invalidate(block_dev->uclass_id, block_dev->devnum); + blkcache_invalidate(desc->uclass_id, desc->devnum); + return ops->erase(dev, start, blkcnt); } +ulong blk_dread(struct blk_desc *desc, lbaint_t start, lbaint_t blkcnt, + void *buffer) +{ + return blk_read(desc->bdev, start, blkcnt, buffer); +} + +ulong blk_dwrite(struct blk_desc *desc, lbaint_t start, lbaint_t blkcnt, + const void *buffer) +{ + return blk_write(desc->bdev, start, blkcnt, buffer); +} + +ulong blk_derase(struct blk_desc *desc, lbaint_t start, lbaint_t blkcnt) +{ + return blk_erase(desc->bdev, start, blkcnt); +} + int blk_get_from_parent(struct udevice *parent, struct udevice **devp) { struct udevice *dev; diff --git a/include/blk.h b/include/blk.h index d3ab9a10b9..e854166edb 100644 --- a/include/blk.h +++ b/include/blk.h @@ -273,6 +273,43 @@ unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t start, unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt); +/** + * blk_read() - Read from a block device + * + * @dev: Device to read from + * @start: Start block for the read + * @blkcnt: Number of blocks to read + * @buf: Place to put the data + * @return number of blocks read (which may be less than @blkcnt), + * or -ve on error. This never returns 0 unless @blkcnt is 0 + */ +long blk_read(struct udevice *dev, lbaint_t start, lbaint_t blkcnt, + void *buffer); + +/** + * blk_write() - Write to a block device + * + * @dev: Device to write to + * @start: Start block for the write + * @blkcnt: Number of blocks to write + * @buf: Data to write + * @return number of blocks written (which may be less than @blkcnt), + * or -ve on error. This never returns 0 unless @blkcnt is 0 + */ +long blk_write(struct udevice *dev, lbaint_t start, lbaint_t blkcnt, + const void *buffer); + +/** + * blk_erase() - Erase part of a block device + * + * @dev: Device to erase + * @start: Start block for the erase + * @blkcnt: Number of blocks to erase + * @return number of blocks erased (which may be less than @blkcnt), + * or -ve on error. This never returns 0 unless @blkcnt is 0 + */ +long blk_erase(struct udevice *dev, lbaint_t start, lbaint_t blkcnt); + /** * blk_find_device() - Find a block device * @@ -428,7 +465,7 @@ const char *blk_get_devtype(struct udevice *dev); /** * blk_get_by_device() - Get the block device descriptor for the given device - * @dev: Instance of a storage device + * @dev: Instance of a storage device (the parent of the block device) * * Return: With block device descriptor on success , NULL if there is no such * block device. diff --git a/test/dm/usb.c b/test/dm/usb.c index 5d6ceefce0..445b21a560 100644 --- a/test/dm/usb.c +++ b/test/dm/usb.c @@ -43,19 +43,24 @@ 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_asserteq(2, blk_read(blk, 0, 2, cmp)); ut_assertok(strcmp(cmp, "this is a test")); ut_assertok(usb_stop()); -- cgit v1.2.3 From 2ff3db3a1c5ef128a4f33c22d7be8ee37fca3613 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 20 Oct 2022 18:22:55 -0600 Subject: usb: Update the test to cover reading and writing Add test coverage for blk_write() as well. The blk_erase() is not tested for now as the USB stor interface does not support erase. Signed-off-by: Simon Glass --- drivers/scsi/scsi_emul.c | 9 +++++++++ drivers/usb/emul/sandbox_flash.c | 35 +++++++++++++++++++++++++++++++---- include/scsi.h | 10 ++++++++++ include/scsi_emul.h | 16 ++++++++++++---- test/dm/usb.c | 19 ++++++++++++++++++- 5 files changed, 80 insertions(+), 9 deletions(-) (limited to 'test') diff --git a/drivers/scsi/scsi_emul.c b/drivers/scsi/scsi_emul.c index 5ba364bdac..6b8468f799 100644 --- a/drivers/scsi/scsi_emul.c +++ b/drivers/scsi/scsi_emul.c @@ -62,6 +62,15 @@ int sb_scsi_emul_command(struct scsi_emul_info *info, ret = SCSI_EMUL_DO_READ; break; } + case SCSI_WRITE10: { + const struct scsi_write10_req *write_req = (void *)req; + + info->seek_block = be32_to_cpu(write_req->lba); + info->write_len = be16_to_cpu(write_req->xfer_len); + info->buff_used = info->write_len * info->block_size; + ret = SCSI_EMUL_DO_WRITE; + break; + } default: debug("Command not supported: %x\n", req->cmd[0]); ret = -EPROTONOSUPPORT; diff --git a/drivers/usb/emul/sandbox_flash.c b/drivers/usb/emul/sandbox_flash.c index 2589c708d8..6e8cfe1650 100644 --- a/drivers/usb/emul/sandbox_flash.c +++ b/drivers/usb/emul/sandbox_flash.c @@ -4,6 +4,8 @@ * Written by Simon Glass */ +#define LOG_CATEGORY UCLASS_USB + #include #include #include @@ -190,7 +192,8 @@ static int handle_ufi_command(struct sandbox_flash_priv *priv, const void *buff, ret = sb_scsi_emul_command(info, req, len); if (!ret) { setup_response(priv); - } else if (ret == SCSI_EMUL_DO_READ && priv->fd != -1) { + } else if ((ret == SCSI_EMUL_DO_READ || ret == SCSI_EMUL_DO_WRITE) && + priv->fd != -1) { os_lseek(priv->fd, info->seek_block * info->block_size, OS_SEEK_SET); setup_response(priv); @@ -217,6 +220,7 @@ static int sandbox_flash_bulk(struct udevice *dev, struct usb_device *udev, case SCSIPH_START: info->alloc_len = 0; info->read_len = 0; + info->write_len = 0; if (priv->error || len != UMASS_BBB_CBW_SIZE || cbw->dCBWSignature != CBWSIGNATURE) goto err; @@ -230,8 +234,31 @@ static int sandbox_flash_bulk(struct udevice *dev, struct usb_device *udev, return handle_ufi_command(priv, cbw->CBWCDB, cbw->bCDBLength); case SCSIPH_DATA: - debug("data out\n"); - break; + log_debug("data out, len=%x, info->write_len=%x\n", len, + info->write_len); + info->transfer_len = cbw->dCBWDataTransferLength; + priv->tag = cbw->dCBWTag; + if (!info->write_len) + return 0; + if (priv->fd != -1) { + ulong bytes_written; + + bytes_written = os_write(priv->fd, buff, len); + log_debug("bytes_written=%lx", bytes_written); + if (bytes_written != len) + return -EIO; + info->write_len -= len / info->block_size; + if (!info->write_len) + info->phase = SCSIPH_STATUS; + } else { + if (info->alloc_len && len > info->alloc_len) + len = info->alloc_len; + if (len > SANDBOX_FLASH_BUF_SIZE) + len = SANDBOX_FLASH_BUF_SIZE; + memcpy(info->buff, buff, len); + info->phase = SCSIPH_STATUS; + } + return len; default: break; } @@ -310,7 +337,7 @@ static int sandbox_flash_probe(struct udevice *dev) struct scsi_emul_info *info = &priv->eminfo; int ret; - priv->fd = os_open(plat->pathname, OS_O_RDONLY); + priv->fd = os_open(plat->pathname, OS_O_RDWR); if (priv->fd != -1) { ret = os_get_filesize(plat->pathname, &info->file_size); if (ret) diff --git a/include/scsi.h b/include/scsi.h index 94e1d8ccb2..9efefea99b 100644 --- a/include/scsi.h +++ b/include/scsi.h @@ -255,6 +255,16 @@ struct __packed scsi_read10_req { u8 spare2[3]; }; +/** struct scsi_write10_req - data for the write10 command */ +struct __packed scsi_write10_req { + u8 cmd; + u8 lun_flags; + u32 lba; + u8 spare; + u16 xfer_len; + u8 spare2[3]; +}; + /** * struct scsi_plat - stores information about SCSI controller * diff --git a/include/scsi_emul.h b/include/scsi_emul.h index 13c3f860b4..d439f7c00c 100644 --- a/include/scsi_emul.h +++ b/include/scsi_emul.h @@ -41,13 +41,20 @@ struct scsi_emul_info { enum scsi_cmd_phase phase; int buff_used; int read_len; + int write_len; uint seek_pos; int alloc_len; uint transfer_len; }; -/* Indicates that a read is being started */ -#define SCSI_EMUL_DO_READ 1 +/** + * Return value from sb_scsi_emul_command() indicates that a read or write is + * being started + */ +enum { + SCSI_EMUL_DO_READ = 1, + SCSI_EMUL_DO_WRITE = 2, +}; /** * sb_scsi_emul_command() - Process a SCSI command @@ -61,8 +68,9 @@ struct scsi_emul_info { * @info: Emulation information * @req: Request to process * @len: Length of request in bytes - * @return SCSI_EMUL_DO_READ if a read has started, 0 if some other operation - * has started, -ve if there was an error + * @return SCSI_EMUL_DO_READ if a read has started, SCSI_EMUL_DO_WRITE if a + * write has started, 0 if some other operation has started, -ve if there + * was an error */ int sb_scsi_emul_command(struct scsi_emul_info *info, const struct scsi_cmd *req, int len); diff --git a/test/dm/usb.c b/test/dm/usb.c index 445b21a560..7671ef156d 100644 --- a/test/dm/usb.c +++ b/test/dm/usb.c @@ -61,7 +61,24 @@ static int dm_test_usb_flash(struct unit_test_state *uts) ut_asserteq(512, dev_desc->blksz); memset(cmp, '\0', sizeof(cmp)); ut_asserteq(2, blk_read(blk, 0, 2, cmp)); - ut_assertok(strcmp(cmp, "this is a test")); + 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; -- cgit v1.2.3 From 44ad35a0f699b0b49ef6efaa1405b6f99c703ea8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 20 Oct 2022 18:23:03 -0600 Subject: image: Add the concept of a phase to FIT We want to be able to mark an image as related to a phase, so we can easily load all the images for SPL or for U-Boot proper. Add this to the FIT specification, along with some access functions. Signed-off-by: Simon Glass --- boot/image.c | 18 +++++++++ doc/uImage.FIT/source_file_format.txt | 3 ++ include/image.h | 74 ++++++++++++++++++++++++++++++++++- test/boot/Makefile | 1 + test/boot/image.c | 36 +++++++++++++++++ 5 files changed, 130 insertions(+), 2 deletions(-) create mode 100644 test/boot/image.c (limited to 'test') diff --git a/boot/image.c b/boot/image.c index 9f95b3260a..b33d1dfc6b 100644 --- a/boot/image.c +++ b/boot/image.c @@ -194,6 +194,13 @@ static const table_entry_t uimage_comp[] = { { -1, "", "", }, }; +static const table_entry_t uimage_phase[] = { + { IH_PHASE_NONE, "none", "any", }, + { IH_PHASE_U_BOOT, "u-boot", "U-Boot phase", }, + { IH_PHASE_SPL, "spl", "SPL Phase", }, + { -1, "", "", }, +}; + struct table_info { const char *desc; int count; @@ -215,6 +222,7 @@ static const struct table_info table_info[IH_COUNT] = { { "compression", IH_COMP_COUNT, uimage_comp }, { "operating system", IH_OS_COUNT, uimage_os }, { "image type", IH_TYPE_COUNT, uimage_type }, + { "phase", IH_PHASE_COUNT, uimage_phase }, }; /*****************************************************************************/ @@ -656,6 +664,11 @@ const char *genimg_get_comp_name(uint8_t comp) comp)); } +const char *genimg_get_phase_name(enum image_phase_t phase) +{ + return get_table_entry_name(uimage_phase, "Unknown Phase", phase); +} + static const char *genimg_get_short_name(const table_entry_t *table, int val) { table = get_table_entry(table, val); @@ -731,3 +744,8 @@ int genimg_get_comp_id(const char *name) { return (get_table_entry_id(uimage_comp, "Compression", name)); } + +int genimg_get_phase_id(const char *name) +{ + return get_table_entry_id(uimage_phase, "Phase", name); +} diff --git a/doc/uImage.FIT/source_file_format.txt b/doc/uImage.FIT/source_file_format.txt index 0a03c942bd..4640e38e3c 100644 --- a/doc/uImage.FIT/source_file_format.txt +++ b/doc/uImage.FIT/source_file_format.txt @@ -190,6 +190,9 @@ the '/images' node should have the following layout: Xilinx Zynq UltraScale+ (ZymqMP) device. "u-boot,zynqmp-fpga-enc" - encrypted FPGA bitstream for Xilinx Zynq UltraScale+ (ZynqMP) device. + - phase : U-Boot phase for which the image is intended. + "spl" - image is an SPL image + "u-boot" - image is a U-Boot image Optional nodes: - hash-1 : Each hash sub-node represents separate hash or checksum diff --git a/include/image.h b/include/image.h index 92efec0f04..7c3dcc407c 100644 --- a/include/image.h +++ b/include/image.h @@ -58,6 +58,7 @@ enum ih_category { IH_COMP, IH_OS, IH_TYPE, + IH_PHASE, IH_COUNT, }; @@ -184,8 +185,7 @@ enum { * New IDs *MUST* be appended at the end of the list and *NEVER* * inserted for backward compatibility. */ - -enum { +enum image_type_t { IH_TYPE_INVALID = 0, /* Invalid Image */ IH_TYPE_STANDALONE, /* Standalone Program */ IH_TYPE_KERNEL, /* OS Kernel Image */ @@ -252,6 +252,59 @@ enum { IH_COMP_COUNT, }; +/** + * Phases - images intended for particular U-Boot phases (SPL, etc.) + * + * @IH_PHASE_NONE: No phase information, can be loaded by any phase + * @IH_PHASE_U_BOOT: Only for U-Boot proper + * @IH_PHASE_SPL: Only for SPL + */ +enum image_phase_t { + IH_PHASE_NONE = 0, + IH_PHASE_U_BOOT, + IH_PHASE_SPL, + + IH_PHASE_COUNT, +}; + +#define IMAGE_PHASE_SHIFT 8 +#define IMAGE_PHASE_MASK (0xff << IMAGE_PHASE_SHIFT) +#define IMAGE_TYPE_MASK 0xff + +/** + * image_ph() - build a composite value combining and type + * + * @phase: Image phase value + * @type: Image type value + * Returns: Composite value containing both + */ +static inline int image_ph(enum image_phase_t phase, enum image_type_t type) +{ + return type | (phase << IMAGE_PHASE_SHIFT); +} + +/** + * image_ph_phase() - obtain the phase from a composite phase/type value + * + * @image_ph_type: Composite value to convert + * Returns: Phase value taken from the composite value + */ +static inline int image_ph_phase(int image_ph_type) +{ + return (image_ph_type & IMAGE_PHASE_MASK) >> IMAGE_PHASE_SHIFT; +} + +/** + * image_ph_type() - obtain the type from a composite phase/type value + * + * @image_ph_type: Composite value to convert + * Returns: Type value taken from the composite value + */ +static inline int image_ph_type(int image_ph_type) +{ + return image_ph_type & IMAGE_TYPE_MASK; +} + #define LZ4F_MAGIC 0x184D2204 /* LZ4 Magic Number */ #define IH_MAGIC 0x27051956 /* Image Magic Number */ #define IH_NMLEN 32 /* Image Name Length */ @@ -437,6 +490,22 @@ const char *genimg_get_os_short_name(uint8_t comp); const char *genimg_get_arch_name(uint8_t arch); +/** + * genimg_get_phase_name() - Get the friendly name for a phase + * + * @phase: Phase value to look up + * Returns: Friendly name for the phase (e.g. "U-Boot phase") + */ +const char *genimg_get_phase_name(enum image_phase_t phase); + +/** + * genimg_get_phase_id() - Convert a phase name to an ID + * + * @name: Name to convert (e.g. "u-boot") + * Returns: ID for that phase (e.g. IH_PHASE_U_BOOT) + */ +int genimg_get_phase_id(const char *name); + /** * genimg_get_arch_short_name() - get the short name for an architecture * @@ -955,6 +1024,7 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size, #define FIT_FPGA_PROP "fpga" #define FIT_FIRMWARE_PROP "firmware" #define FIT_STANDALONE_PROP "standalone" +#define FIT_PHASE_PROP "phase" #define FIT_MAX_HASH_LEN HASH_MAX_DIGEST_SIZE 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/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 + */ + +#include +#include +#include +#include +#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); -- cgit v1.2.3 From 98bedf42ea49ceec934e0a47aa35caa38aac31b7 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 20 Oct 2022 18:23:05 -0600 Subject: vbe: Rename vbe_fixup to vbe_request The vbe_fixup file handles device tree fixups, but these are called OS requests in VBE. Rename the file to reflect its wider purpose. Signed-off-by: Simon Glass --- boot/Makefile | 2 +- boot/vbe_fixup.c | 233 --------------------------------------- boot/vbe_request.c | 233 +++++++++++++++++++++++++++++++++++++++ test/py/tests/test_event_dump.py | 2 +- 4 files changed, 235 insertions(+), 235 deletions(-) delete mode 100644 boot/vbe_fixup.c create mode 100644 boot/vbe_request.c (limited to 'test') diff --git a/boot/Makefile b/boot/Makefile index dd45d786f8..19d628d748 100644 --- a/boot/Makefile +++ b/boot/Makefile @@ -47,5 +47,5 @@ ifdef CONFIG_SPL_BUILD obj-$(CONFIG_SPL_LOAD_FIT) += common_fit.o endif -obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE) += vbe.o vbe_fixup.o +obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE) += vbe.o vbe_request.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o diff --git a/boot/vbe_fixup.c b/boot/vbe_fixup.c deleted file mode 100644 index 53d88678c9..0000000000 --- a/boot/vbe_fixup.c +++ /dev/null @@ -1,233 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -/* - * Verified Boot for Embedded (VBE) device tree fixup functions - * - * Copyright 2022 Google LLC - * Written by Simon Glass - */ - -#define LOG_CATEGORY LOGC_BOOT - -#include -#include -#include -#include -#include -#include -#include - -#define VBE_PREFIX "vbe," -#define VBE_PREFIX_LEN (sizeof(VBE_PREFIX) - 1) -#define VBE_ERR_STR_LEN 128 -#define VBE_MAX_RAND_SIZE 256 - -struct vbe_result { - int errnum; - char err_str[VBE_ERR_STR_LEN]; -}; - -typedef int (*vbe_req_func)(ofnode node, struct vbe_result *result); - -static int handle_random_req(ofnode node, int default_size, - struct vbe_result *result) -{ - char buf[VBE_MAX_RAND_SIZE]; - struct udevice *dev; - u32 size; - int ret; - - if (!IS_ENABLED(CONFIG_DM_RNG)) - return -ENOTSUPP; - - if (ofnode_read_u32(node, "vbe,size", &size)) { - if (!default_size) { - snprintf(result->err_str, VBE_ERR_STR_LEN, - "Missing vbe,size property"); - return log_msg_ret("byt", -EINVAL); - } - size = default_size; - } - if (size > VBE_MAX_RAND_SIZE) { - snprintf(result->err_str, VBE_ERR_STR_LEN, - "vbe,size %#x exceeds max size %#x", size, - VBE_MAX_RAND_SIZE); - return log_msg_ret("siz", -E2BIG); - } - ret = uclass_first_device_err(UCLASS_RNG, &dev); - if (ret) { - snprintf(result->err_str, VBE_ERR_STR_LEN, - "Cannot find random-number device (err=%d)", ret); - return log_msg_ret("wr", ret); - } - ret = dm_rng_read(dev, buf, size); - if (ret) { - snprintf(result->err_str, VBE_ERR_STR_LEN, - "Failed to read random-number device (err=%d)", ret); - return log_msg_ret("rd", ret); - } - ret = ofnode_write_prop(node, "data", buf, size, true); - if (ret) - return log_msg_ret("wr", -EINVAL); - - return 0; -} - -static int vbe_req_random_seed(ofnode node, struct vbe_result *result) -{ - return handle_random_req(node, 0, result); -} - -static int vbe_req_aslr_move(ofnode node, struct vbe_result *result) -{ - return -ENOTSUPP; -} - -static int vbe_req_aslr_rand(ofnode node, struct vbe_result *result) -{ - return handle_random_req(node, 4, result); -} - -static int vbe_req_efi_runtime_rand(ofnode node, struct vbe_result *result) -{ - return handle_random_req(node, 4, result); -} - -static struct vbe_req { - const char *compat; - vbe_req_func func; -} vbe_reqs[] = { - /* address space layout randomization - move the OS in memory */ - { "aslr-move", vbe_req_aslr_move }, - - /* provide random data for address space layout randomization */ - { "aslr-rand", vbe_req_aslr_rand }, - - /* provide random data for EFI-runtime-services address */ - { "efi-runtime-rand", vbe_req_efi_runtime_rand }, - - /* generate random data bytes to see the OS's rand generator */ - { "random-rand", vbe_req_random_seed }, - -}; - -static int vbe_process_request(ofnode node, struct vbe_result *result) -{ - const char *compat, *req_name; - int i; - - compat = ofnode_read_string(node, "compatible"); - if (!compat) - return 0; - - if (strlen(compat) <= VBE_PREFIX_LEN || - strncmp(compat, VBE_PREFIX, VBE_PREFIX_LEN)) - return -EINVAL; - - req_name = compat + VBE_PREFIX_LEN; /* drop "vbe," prefix */ - for (i = 0; i < ARRAY_SIZE(vbe_reqs); i++) { - if (!strcmp(vbe_reqs[i].compat, req_name)) { - int ret; - - ret = vbe_reqs[i].func(node, result); - if (ret) - return log_msg_ret("req", ret); - return 0; - } - } - snprintf(result->err_str, VBE_ERR_STR_LEN, "Unknown request: %s", - req_name); - - return -ENOTSUPP; -} - -/** - * bootmeth_vbe_ft_fixup() - Process VBE OS requests and do device tree fixups - * - * If there are no images provided, this does nothing and returns 0. - * - * @ctx: Context for event - * @event: Event to process - * @return 0 if OK, -ve on error - */ -static int bootmeth_vbe_ft_fixup(void *ctx, struct event *event) -{ - const struct event_ft_fixup *fixup = &event->data.ft_fixup; - const struct bootm_headers *images = fixup->images; - ofnode parent, dest_parent, root, node; - oftree fit; - - if (!images || !images->fit_hdr_os) - return 0; - - /* Get the image node with requests in it */ - log_debug("fit=%p, noffset=%d\n", images->fit_hdr_os, - images->fit_noffset_os); - fit = oftree_from_fdt(images->fit_hdr_os); - root = oftree_root(fit); - if (of_live_active()) { - log_warning("Cannot fix up live tree\n"); - return 0; - } - if (!ofnode_valid(root)) - return log_msg_ret("rt", -EINVAL); - parent = noffset_to_ofnode(root, images->fit_noffset_os); - if (!ofnode_valid(parent)) - return log_msg_ret("img", -EINVAL); - dest_parent = oftree_path(fixup->tree, "/chosen"); - if (!ofnode_valid(dest_parent)) - return log_msg_ret("dst", -EINVAL); - - ofnode_for_each_subnode(node, parent) { - const char *name = ofnode_get_name(node); - struct vbe_result result; - ofnode dest; - int ret; - - log_debug("copy subnode: %s\n", name); - ret = ofnode_add_subnode(dest_parent, name, &dest); - if (ret && ret != -EEXIST) - return log_msg_ret("add", ret); - ret = ofnode_copy_props(node, dest); - if (ret) - return log_msg_ret("cp", ret); - - *result.err_str = '\0'; - ret = vbe_process_request(dest, &result); - if (ret) { - result.errnum = ret; - log_err("Failed to process VBE request %s (err=%d)\n", - ofnode_get_name(dest), ret); - if (*result.err_str) { - char *msg = strdup(result.err_str); - - if (!msg) - return log_msg_ret("msg", -ENOMEM); - ret = ofnode_write_string(dest, "vbe,error", - msg); - if (ret) { - free(msg); - return log_msg_ret("str", -ENOMEM); - } - } - if (result.errnum) { - ret = ofnode_write_u32(dest, "vbe,errnum", - result.errnum); - if (ret) - return log_msg_ret("num", -ENOMEM); - if (result.errnum != -ENOTSUPP) - return log_msg_ret("pro", - result.errnum); - if (result.errnum == -ENOTSUPP && - ofnode_read_bool(dest, "vbe,required")) { - log_err("Cannot handle required request: %s\n", - ofnode_get_name(dest)); - return log_msg_ret("req", - result.errnum); - } - } - } - } - - return 0; -} -EVENT_SPY(EVT_FT_FIXUP, bootmeth_vbe_ft_fixup); diff --git a/boot/vbe_request.c b/boot/vbe_request.c new file mode 100644 index 0000000000..fa01892463 --- /dev/null +++ b/boot/vbe_request.c @@ -0,0 +1,233 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Verified Boot for Embedded (VBE) OS request (device tree fixup) functions + * + * Copyright 2022 Google LLC + * Written by Simon Glass + */ + +#define LOG_CATEGORY LOGC_BOOT + +#include +#include +#include +#include +#include +#include +#include + +#define VBE_PREFIX "vbe," +#define VBE_PREFIX_LEN (sizeof(VBE_PREFIX) - 1) +#define VBE_ERR_STR_LEN 128 +#define VBE_MAX_RAND_SIZE 256 + +struct vbe_result { + int errnum; + char err_str[VBE_ERR_STR_LEN]; +}; + +typedef int (*vbe_req_func)(ofnode node, struct vbe_result *result); + +static int handle_random_req(ofnode node, int default_size, + struct vbe_result *result) +{ + char buf[VBE_MAX_RAND_SIZE]; + struct udevice *dev; + u32 size; + int ret; + + if (!IS_ENABLED(CONFIG_DM_RNG)) + return -ENOTSUPP; + + if (ofnode_read_u32(node, "vbe,size", &size)) { + if (!default_size) { + snprintf(result->err_str, VBE_ERR_STR_LEN, + "Missing vbe,size property"); + return log_msg_ret("byt", -EINVAL); + } + size = default_size; + } + if (size > VBE_MAX_RAND_SIZE) { + snprintf(result->err_str, VBE_ERR_STR_LEN, + "vbe,size %#x exceeds max size %#x", size, + VBE_MAX_RAND_SIZE); + return log_msg_ret("siz", -E2BIG); + } + ret = uclass_first_device_err(UCLASS_RNG, &dev); + if (ret) { + snprintf(result->err_str, VBE_ERR_STR_LEN, + "Cannot find random-number device (err=%d)", ret); + return log_msg_ret("wr", ret); + } + ret = dm_rng_read(dev, buf, size); + if (ret) { + snprintf(result->err_str, VBE_ERR_STR_LEN, + "Failed to read random-number device (err=%d)", ret); + return log_msg_ret("rd", ret); + } + ret = ofnode_write_prop(node, "data", buf, size, true); + if (ret) + return log_msg_ret("wr", -EINVAL); + + return 0; +} + +static int vbe_req_random_seed(ofnode node, struct vbe_result *result) +{ + return handle_random_req(node, 0, result); +} + +static int vbe_req_aslr_move(ofnode node, struct vbe_result *result) +{ + return -ENOTSUPP; +} + +static int vbe_req_aslr_rand(ofnode node, struct vbe_result *result) +{ + return handle_random_req(node, 4, result); +} + +static int vbe_req_efi_runtime_rand(ofnode node, struct vbe_result *result) +{ + return handle_random_req(node, 4, result); +} + +static struct vbe_req { + const char *compat; + vbe_req_func func; +} vbe_reqs[] = { + /* address space layout randomization - move the OS in memory */ + { "aslr-move", vbe_req_aslr_move }, + + /* provide random data for address space layout randomization */ + { "aslr-rand", vbe_req_aslr_rand }, + + /* provide random data for EFI-runtime-services address */ + { "efi-runtime-rand", vbe_req_efi_runtime_rand }, + + /* generate random data bytes to see the OS's rand generator */ + { "random-rand", vbe_req_random_seed }, + +}; + +static int vbe_process_request(ofnode node, struct vbe_result *result) +{ + const char *compat, *req_name; + int i; + + compat = ofnode_read_string(node, "compatible"); + if (!compat) + return 0; + + if (strlen(compat) <= VBE_PREFIX_LEN || + strncmp(compat, VBE_PREFIX, VBE_PREFIX_LEN)) + return -EINVAL; + + req_name = compat + VBE_PREFIX_LEN; /* drop "vbe," prefix */ + for (i = 0; i < ARRAY_SIZE(vbe_reqs); i++) { + if (!strcmp(vbe_reqs[i].compat, req_name)) { + int ret; + + ret = vbe_reqs[i].func(node, result); + if (ret) + return log_msg_ret("req", ret); + return 0; + } + } + snprintf(result->err_str, VBE_ERR_STR_LEN, "Unknown request: %s", + req_name); + + return -ENOTSUPP; +} + +/** + * bootmeth_vbe_ft_fixup() - Process VBE OS requests and do device tree fixups + * + * If there are no images provided, this does nothing and returns 0. + * + * @ctx: Context for event + * @event: Event to process + * @return 0 if OK, -ve on error + */ +static int bootmeth_vbe_ft_fixup(void *ctx, struct event *event) +{ + const struct event_ft_fixup *fixup = &event->data.ft_fixup; + const struct bootm_headers *images = fixup->images; + ofnode parent, dest_parent, root, node; + oftree fit; + + if (!images || !images->fit_hdr_os) + return 0; + + /* Get the image node with requests in it */ + log_debug("fit=%p, noffset=%d\n", images->fit_hdr_os, + images->fit_noffset_os); + fit = oftree_from_fdt(images->fit_hdr_os); + root = oftree_root(fit); + if (of_live_active()) { + log_warning("Cannot fix up live tree\n"); + return 0; + } + if (!ofnode_valid(root)) + return log_msg_ret("rt", -EINVAL); + parent = noffset_to_ofnode(root, images->fit_noffset_os); + if (!ofnode_valid(parent)) + return log_msg_ret("img", -EINVAL); + dest_parent = oftree_path(fixup->tree, "/chosen"); + if (!ofnode_valid(dest_parent)) + return log_msg_ret("dst", -EINVAL); + + ofnode_for_each_subnode(node, parent) { + const char *name = ofnode_get_name(node); + struct vbe_result result; + ofnode dest; + int ret; + + log_debug("copy subnode: %s\n", name); + ret = ofnode_add_subnode(dest_parent, name, &dest); + if (ret && ret != -EEXIST) + return log_msg_ret("add", ret); + ret = ofnode_copy_props(node, dest); + if (ret) + return log_msg_ret("cp", ret); + + *result.err_str = '\0'; + ret = vbe_process_request(dest, &result); + if (ret) { + result.errnum = ret; + log_err("Failed to process VBE request %s (err=%d)\n", + ofnode_get_name(dest), ret); + if (*result.err_str) { + char *msg = strdup(result.err_str); + + if (!msg) + return log_msg_ret("msg", -ENOMEM); + ret = ofnode_write_string(dest, "vbe,error", + msg); + if (ret) { + free(msg); + return log_msg_ret("str", -ENOMEM); + } + } + if (result.errnum) { + ret = ofnode_write_u32(dest, "vbe,errnum", + result.errnum); + if (ret) + return log_msg_ret("num", -ENOMEM); + if (result.errnum != -ENOTSUPP) + return log_msg_ret("pro", + result.errnum); + if (result.errnum == -ENOTSUPP && + ofnode_read_bool(dest, "vbe,required")) { + log_err("Cannot handle required request: %s\n", + ofnode_get_name(dest)); + return log_msg_ret("req", + result.errnum); + } + } + } + } + + return 0; +} +EVENT_SPY(EVT_FT_FIXUP, bootmeth_vbe_ft_fixup); diff --git a/test/py/tests/test_event_dump.py b/test/py/tests/test_event_dump.py index 674df2ea00..0984efaf3c 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_ft_fixup .*vbe_request.c:.* EVT_FT_FIXUP bootmeth_vbe_simple_ft_fixup .*vbe_simple.c:.* EVT_MISC_INIT_F sandbox_misc_init_f .*start.c:''' assert re.match(expect, out, re.MULTILINE) is not None -- cgit v1.2.3 From c263e21bcb01f19e6ccb2452fdcc601ff84942db Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 20 Oct 2022 18:23:11 -0600 Subject: vbe: Move OS implementation into a separate file Move this into its own file so it can be built only by U-Boot proper. Signed-off-by: Simon Glass --- boot/Makefile | 1 + boot/vbe_simple.c | 102 +-------------------------------------- boot/vbe_simple.h | 34 +++++++++++++ boot/vbe_simple_os.c | 89 ++++++++++++++++++++++++++++++++++ test/py/tests/test_event_dump.py | 1 - 5 files changed, 126 insertions(+), 101 deletions(-) create mode 100644 boot/vbe_simple_os.c (limited to 'test') diff --git a/boot/Makefile b/boot/Makefile index e5c27900ea..f0c3154921 100644 --- a/boot/Makefile +++ b/boot/Makefile @@ -50,3 +50,4 @@ endif obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE) += vbe.o vbe_request.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_FW) += vbe_simple_fw.o +obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_OS) += vbe_simple_os.o diff --git a/boot/vbe_simple.c b/boot/vbe_simple.c index 1ccd416e4b..59676d8613 100644 --- a/boot/vbe_simple.c +++ b/boot/vbe_simple.c @@ -17,37 +17,11 @@ #include #include #include -#include #include #include #include #include "vbe_simple.h" -enum { - MAX_VERSION_LEN = 256, - - NVD_HDR_VER_SHIFT = 0, - NVD_HDR_VER_MASK = 0xf, - NVD_HDR_SIZE_SHIFT = 4, - NVD_HDR_SIZE_MASK = 0xf << NVD_HDR_SIZE_SHIFT, - - /* Firmware key-version is in the top 16 bits of fw_ver */ - FWVER_KEY_SHIFT = 16, - FWVER_FW_MASK = 0xffff, - - NVD_HDR_VER_CUR = 1, /* current version */ -}; - -/** struct simple_state - state information read from media - * - * @fw_version: Firmware version string - * @fw_vernum: Firmware version number - */ -struct simple_state { - char fw_version[MAX_VERSION_LEN]; - u32 fw_vernum; -}; - /** struct simple_nvdata - storage format for non-volatile data */ struct simple_nvdata { u8 crc8; @@ -116,7 +90,7 @@ static int simple_read_nvdata(struct udevice *dev, struct blk_desc *desc, return 0; } -static int simple_read_state(struct udevice *dev, struct simple_state *state) +int vbe_simple_read_state(struct udevice *dev, struct simple_state *state) { ALLOC_CACHE_ALIGN_BUFFER(u8, buf, MMC_MAX_BLOCK_LEN); struct simple_priv *priv = dev_get_priv(dev); @@ -157,7 +131,7 @@ static int vbe_simple_get_state_desc(struct udevice *dev, char *buf, struct simple_state state; int ret; - ret = simple_read_state(dev, &state); + ret = vbe_simple_read_state(dev, &state); if (ret) return log_msg_ret("read", ret); @@ -206,78 +180,6 @@ static struct bootmeth_ops bootmeth_vbe_simple_ops = { .read_file = vbe_simple_read_file, }; -int vbe_simple_fixup_node(ofnode node, struct simple_state *state) -{ - char *version; - int ret; - - version = strdup(state->fw_version); - if (!version) - return log_msg_ret("dup", -ENOMEM); - - ret = ofnode_write_string(node, "cur-version", version); - if (ret) - return log_msg_ret("ver", ret); - ret = ofnode_write_u32(node, "cur-vernum", state->fw_vernum); - if (ret) - return log_msg_ret("num", ret); - ret = ofnode_write_string(node, "bootloader-version", version_string); - if (ret) - return log_msg_ret("bl", ret); - - return 0; -} - -/** - * bootmeth_vbe_simple_ft_fixup() - Write out all VBE simple data to the DT - * - * @ctx: Context for event - * @event: Event to process - * @return 0 if OK, -ve on error - */ -static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct event *event) -{ - oftree tree = event->data.ft_fixup.tree; - struct udevice *dev; - - /* - * Ideally we would have driver model support for fixups, but that does - * not exist yet. It is a step too far to try to do this before VBE is - * in place. - */ - for (vbe_find_first_device(&dev); dev; vbe_find_next_device(&dev)) { - struct simple_state state; - ofnode node, subnode; - int ret; - - if (strcmp("vbe_simple", dev->driver->name)) - continue; - - /* Check if there is a node to fix up */ - node = oftree_path(tree, "/chosen/fwupd"); - if (!ofnode_valid(node)) - continue; - subnode = ofnode_find_subnode(node, dev->name); - if (!ofnode_valid(subnode)) - continue; - - log_debug("Fixing up: %s\n", dev->name); - ret = device_probe(dev); - if (ret) - return log_msg_ret("probe", ret); - ret = simple_read_state(dev, &state); - if (ret) - return log_msg_ret("read", ret); - - ret = vbe_simple_fixup_node(subnode, &state); - if (ret) - return log_msg_ret("fix", ret); - } - - return 0; -} -EVENT_SPY(EVT_FT_FIXUP, bootmeth_vbe_simple_ft_fixup); - static int bootmeth_vbe_simple_probe(struct udevice *dev) { struct simple_priv *priv = dev_get_priv(dev); diff --git a/boot/vbe_simple.h b/boot/vbe_simple.h index e37a9fae37..56d319206f 100644 --- a/boot/vbe_simple.h +++ b/boot/vbe_simple.h @@ -9,6 +9,21 @@ #ifndef __VBE_SIMPLE_H #define __VBE_SIMPLE_H +enum { + MAX_VERSION_LEN = 256, + + NVD_HDR_VER_SHIFT = 0, + NVD_HDR_VER_MASK = 0xf, + NVD_HDR_SIZE_SHIFT = 4, + NVD_HDR_SIZE_MASK = 0xf << NVD_HDR_SIZE_SHIFT, + + /* Firmware key-version is in the top 16 bits of fw_ver */ + FWVER_KEY_SHIFT = 16, + FWVER_FW_MASK = 0xffff, + + NVD_HDR_VER_CUR = 1, /* current version */ +}; + /** struct simple_priv - information read from the device tree */ struct simple_priv { u32 area_start; @@ -21,6 +36,16 @@ struct simple_priv { const char *storage; }; +/** struct simple_state - state information read from media + * + * @fw_version: Firmware version string + * @fw_vernum: Firmware version number + */ +struct simple_state { + char fw_version[MAX_VERSION_LEN]; + u32 fw_vernum; +}; + /** * vbe_simple_read_fw_bootflow() - Read a bootflow for firmware * @@ -34,4 +59,13 @@ struct simple_priv { */ int vbe_simple_read_bootflow_fw(struct udevice *dev, struct bootflow *bflow); +/** + * vbe_simple_read_state() - Read the VBE simple state information + * + * @dev: VBE bootmeth + * @state: Place to put the state + * @return 0 if OK, -ve on error + */ +int vbe_simple_read_state(struct udevice *dev, struct simple_state *state); + #endif /* __VBE_SIMPLE_H */ diff --git a/boot/vbe_simple_os.c b/boot/vbe_simple_os.c new file mode 100644 index 0000000000..7761b9ef65 --- /dev/null +++ b/boot/vbe_simple_os.c @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Verified Boot for Embedded (VBE) loading firmware phases + * + * Copyright 2022 Google LLC + * Written by Simon Glass + */ + +#define LOG_CATEGORY LOGC_BOOT + +#include +#include +#include +#include +#include +#include +#include "vbe_simple.h" + +int vbe_simple_fixup_node(ofnode node, struct simple_state *state) +{ + char *version; + int ret; + + version = strdup(state->fw_version); + if (!version) + return log_msg_ret("dup", -ENOMEM); + + ret = ofnode_write_string(node, "cur-version", version); + if (ret) + return log_msg_ret("ver", ret); + ret = ofnode_write_u32(node, "cur-vernum", state->fw_vernum); + if (ret) + return log_msg_ret("num", ret); + ret = ofnode_write_string(node, "bootloader-version", version_string); + if (ret) + return log_msg_ret("bl", ret); + + return 0; +} + +/** + * bootmeth_vbe_simple_ft_fixup() - Write out all VBE simple data to the DT + * + * @ctx: Context for event + * @event: Event to process + * @return 0 if OK, -ve on error + */ +static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct event *event) +{ + oftree tree = event->data.ft_fixup.tree; + struct udevice *dev; + + /* + * Ideally we would have driver model support for fixups, but that does + * not exist yet. It is a step too far to try to do this before VBE is + * in place. + */ + for (vbe_find_first_device(&dev); dev; vbe_find_next_device(&dev)) { + struct simple_state state; + ofnode node, subnode; + int ret; + + if (strcmp("vbe_simple", dev->driver->name)) + continue; + + /* Check if there is a node to fix up */ + node = oftree_path(tree, "/chosen/fwupd"); + if (!ofnode_valid(node)) + continue; + subnode = ofnode_find_subnode(node, dev->name); + if (!ofnode_valid(subnode)) + continue; + + log_debug("Fixing up: %s\n", dev->name); + ret = device_probe(dev); + if (ret) + return log_msg_ret("probe", ret); + ret = simple_read_state(dev, &state); + if (ret) + return log_msg_ret("read", ret); + + ret = vbe_simple_fixup_node(subnode, &state); + if (ret) + return log_msg_ret("fix", ret); + } + + return 0; +} +EVENT_SPY(EVT_FT_FIXUP, bootmeth_vbe_simple_ft_fixup); diff --git a/test/py/tests/test_event_dump.py b/test/py/tests/test_event_dump.py index 0984efaf3c..972c383711 100644 --- a/test/py/tests/test_event_dump.py +++ b/test/py/tests/test_event_dump.py @@ -17,6 +17,5 @@ def test_event_dump(u_boot_console): expect = '''.*Event type Id Source location -------------------- ------------------------------ ------------------------------ EVT_FT_FIXUP bootmeth_vbe_ft_fixup .*vbe_request.c:.* -EVT_FT_FIXUP bootmeth_vbe_simple_ft_fixup .*vbe_simple.c:.* EVT_MISC_INIT_F sandbox_misc_init_f .*start.c:''' assert re.match(expect, out, re.MULTILINE) is not None -- cgit v1.2.3 From e45d22655aed0c81fa5890f47c1647c6e95bedb6 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 20 Oct 2022 18:23:12 -0600 Subject: vbe: Drop the U-Boot prefix from the version We don't need the U-Boot prefix on the version and in fact it is harmful since pytest gets confused seeing the U-Boot banner bring displayed when the version is printed. Drop the prefix from the string. We could produce an entirely new string from the component parts, but this adds to the rodata size and would break the use of version_string as the only thing which holds this information. Signed-off-by: Simon Glass --- boot/vbe_simple_os.c | 9 +++++++-- test/boot/vbe_simple.c | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) (limited to 'test') diff --git a/boot/vbe_simple_os.c b/boot/vbe_simple_os.c index 7761b9ef65..058db6154b 100644 --- a/boot/vbe_simple_os.c +++ b/boot/vbe_simple_os.c @@ -18,7 +18,7 @@ int vbe_simple_fixup_node(ofnode node, struct simple_state *state) { - char *version; + const char *version, *str; int ret; version = strdup(state->fw_version); @@ -31,7 +31,12 @@ int vbe_simple_fixup_node(ofnode node, struct simple_state *state) ret = ofnode_write_u32(node, "cur-vernum", state->fw_vernum); if (ret) return log_msg_ret("num", ret); - ret = ofnode_write_string(node, "bootloader-version", version_string); + + /* Drop the 'U-Boot ' at the start */ + str = version_string; + if (!strncmp("U-Boot ", str, 7)) + str += 7; + ret = ofnode_write_string(node, "bootloader-version", str); if (ret) return log_msg_ret("bl", ret); diff --git a/test/boot/vbe_simple.c b/test/boot/vbe_simple.c index faba9e8f90..a50785dbbf 100644 --- a/test/boot/vbe_simple.c +++ b/test/boot/vbe_simple.c @@ -77,7 +77,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; } -- cgit v1.2.3 From 4218456b3fac98966a320c3f2db36d543a32ec17 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 20 Oct 2022 18:23:13 -0600 Subject: vbe: Add Kconfig options for VPL Enable the various features needed in VPL, by adding Kconfig options. Update the defconfig for sandbox_vpl so that the build for each phase includes what is needed. Drop LZMA for now and make sure partition support is omitted in SPL, since it is not needed. Signed-off-by: Simon Glass --- boot/Kconfig | 137 +++++++++++++++++++++++++++++++++++++++ boot/vbe_simple_os.c | 2 +- common/spl/Kconfig.vpl | 30 +++++++++ configs/sandbox_vpl_defconfig | 13 +++- test/py/tests/test_event_dump.py | 1 + 5 files changed, 181 insertions(+), 2 deletions(-) (limited to 'test') diff --git a/boot/Kconfig b/boot/Kconfig index 6eb056def0..93344975a6 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -292,6 +292,57 @@ config SPL_FIT_GENERATOR endif # SPL +if VPL + +config VPL_FIT + bool "Support Flattened Image Tree within VPL" + depends on VPL + default y + select VPL_HASH + select VPL_OF_LIBFDT + +config VPL_FIT_PRINT + bool "Support FIT printing within VPL" + depends on VPL_FIT + default y + help + Support printing the content of the fitImage in a verbose manner in VPL. + +config VPL_FIT_FULL_CHECK + bool "Do a full check of the FIT before using it" + default y + help + Enable this do a full check of the FIT to make sure it is valid. This + helps to protect against carefully crafted FITs which take advantage + of bugs or omissions in the code. This includes a bad structure, + multiple root nodes and the like. + +config VPL_FIT_SIGNATURE + bool "Enable signature verification of FIT firmware within VPL" + depends on VPL_DM + depends on VPL_LOAD_FIT || VPL_LOAD_FIT_FULL + default y + select FIT_SIGNATURE + select VPL_FIT + select VPL_CRYPTO + select VPL_HASH + imply VPL_RSA + imply VPL_RSA_VERIFY + select VPL_IMAGE_SIGN_INFO + select VPL_FIT_FULL_CHECK + +config VPL_FIT_SIGNATURE_MAX_SIZE + hex "Max size of signed FIT structures in VPL" + depends on VPL_FIT_SIGNATURE + default 0x10000000 + help + This option sets a max size in bytes for verified FIT uImages. + A sane value of 256MB protects corrupted DTB structures from overlapping + device memory. Assure this size does not extend past expected storage + space. + +endif # VPL + endif # FIT config PXE_UTILS @@ -334,6 +385,26 @@ config BOOTSTD_FULL - support for selecting the ordering of bootdevs using the devicetree as well as the "boot_targets" environment variable +config SPL_BOOTSTD + bool "Standard boot support in VPL" + depends on SPL && SPL_DM && SPL_OF_CONTROL && SPL_BLK + default y if VPL + help + This enables standard boot in SPL. This is neeeded so that VBE + (Verified Boot for Embedded) can be used, since it depends on standard + boot. It is enabled by default since the main purpose of VPL is to + handle the firmware part of VBE. + +config VPL_BOOTSTD + bool "Standard boot support in VPL" + depends on VPL && VPL_DM && VPL_OF_CONTROL && VPL_BLK + default y + help + This enables standard boot in SPL. This is neeeded so that VBE + (Verified Boot for Embedded) can be used, since it depends on standard + boot. It is enabled by default since the main purpose of VPL is to + handle the firmware part of VBE. + if BOOTSTD config BOOTSTD_BOOTCOMMAND @@ -408,6 +479,24 @@ config BOOTMETH_VBE supports selection of various firmware components, seleciton of an OS to boot as well as updating these using fwupd. +config SPL_BOOTMETH_VBE + bool "Bootdev support for Verified Boot for Embedded (SPL)" + depends on SPL && FIT + default y if VPL + help + Enables support for VBE boot. This is a standard boot method which + supports selection of various firmware components, seleciton of an OS to + boot as well as updating these using fwupd. + +config VPL_BOOTMETH_VBE + bool "Bootdev support for Verified Boot for Embedded (VPL)" + depends on VPL && FIT + default y + help + Enables support for VBE boot. This is a standard boot method which + supports selection of various firmware components, seleciton of an OS to + boot as well as updating these using fwupd. + if BOOTMETH_VBE config BOOTMETH_VBE_SIMPLE @@ -418,6 +507,54 @@ config BOOTMETH_VBE_SIMPLE firmware image in boot media such as MMC. It does not support any sort of rollback, recovery or A/B boot. +config BOOTMETH_VBE_SIMPLE_OS + bool "Bootdev support for VBE 'simple' method OS phase" + default y + help + Enables support for the OS parts of VBE 'simple' boot. This includes + fixing up the device tree with the required VBE information, ready + for booting into the OS. This option is only enabled for U-Boot + proper, since it is the phase where device tree fixups happen. + +config SPL_BOOTMETH_VBE_SIMPLE + bool "Bootdev support for VBE 'simple' method (SPL)" + depends on SPL + default y if VPL + help + Enables support for VBE 'simple' boot. This allows updating a single + firmware image in boot media such as MMC. It does not support any sort + of rollback, recovery or A/B boot. + +config VPL_BOOTMETH_VBE_SIMPLE + bool "Bootdev support for VBE 'simple' method (VPL)" + depends on VPL + default y + help + Enables support for VBE 'simple' boot. This allows updating a single + firmware image in boot media such as MMC. It does not support any sort + of rollback, recovery or A/B boot. + +config SPL_BOOTMETH_VBE_SIMPLE_FW + bool "Bootdev support for VBE 'simple' method firmware phase (SPL)" + depends on VPL + default y + help + Enables support for the firmware parts of VBE 'simple' boot. This + includes an SPL loader which locates the correct U-Boot to boot into. + This option should really only be enabled for VPL, since it is the + phase where the SPL + U-Boot decision should be made. But for now, + SPL does its own FIT-configuration selection. + +config VPL_BOOTMETH_VBE_SIMPLE_FW + bool "Bootdev support for VBE 'simple' method firmware phase (VPL)" + depends on VPL + default y + help + Enables support for the firmware parts of VBE 'simple' boot. This + includes an SPL loader which locates the correct SPL to boot into. + This option enabled for VPL, since it is the phase where the SPL + decision is made. + endif # BOOTMETH_VBE config BOOTMETH_SANDBOX diff --git a/boot/vbe_simple_os.c b/boot/vbe_simple_os.c index 058db6154b..87778bba97 100644 --- a/boot/vbe_simple_os.c +++ b/boot/vbe_simple_os.c @@ -80,7 +80,7 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct event *event) ret = device_probe(dev); if (ret) return log_msg_ret("probe", ret); - ret = simple_read_state(dev, &state); + ret = vbe_simple_read_state(dev, &state); if (ret) return log_msg_ret("read", ret); diff --git a/common/spl/Kconfig.vpl b/common/spl/Kconfig.vpl index f33162276d..ae1a3c724f 100644 --- a/common/spl/Kconfig.vpl +++ b/common/spl/Kconfig.vpl @@ -133,6 +133,36 @@ config VPL_I2C_SUPPORT Enable support for the I2C bus in VPL. Vee SPL_I2C_SUPPORT for details. +config VPL_MMC + bool "Support MMC in VPL" + depends on VPL && MMC + default y if MMC + help + Enable support for MMC (Multimedia Card) within VPL This enables + the MMC protocol implementation and allows any enabled drivers to + be used within VPL. MMC can be used with or without disk partition + support depending on the application (SPL_LIBDISK_SUPPORT). Enable + this option to build the drivers in drivers/mmc as part of an VPL + build. + +config VPL_DM_MMC + bool "Enable MMC controllers using Driver Model in VPL" + depends on VPL_DM && DM_MMC + default y + help + This enables the MultiMediaCard (MMC) uclass which supports MMC and + Secure Digital I/O (SDIO) cards. Both removable (SD, micro-SD, etc.) + and non-removable (e.g. eMMC chip) devices are supported. These + appear as block devices in U-Boot and can support filesystems such + as EXT4 and FAT. + +config VPL_MMC_WRITE + bool "MMC/SD/SDIO card support for write operations in VPL" + depends on VPL_MMC + default y + help + Enable write access to MMC and SD Cards in VPL + config VPL_PCH_SUPPORT bool "Support PCH drivers" default y if TPL_PCH_SUPPORT diff --git a/configs/sandbox_vpl_defconfig b/configs/sandbox_vpl_defconfig index a2a1295549..557fdd141f 100644 --- a/configs/sandbox_vpl_defconfig +++ b/configs/sandbox_vpl_defconfig @@ -4,7 +4,10 @@ CONFIG_SPL_LIBGENERIC_SUPPORT=y CONFIG_NR_DRAM_BANKS=1 CONFIG_ENV_SIZE=0x2000 CONFIG_DEFAULT_DEVICE_TREE="sandbox" +CONFIG_SPL_TEXT_BASE=0x100000 +CONFIG_SPL_MMC=y CONFIG_SPL_SERIAL=y +CONFIG_TPL_TEXT_BASE=0x100000 CONFIG_TPL_LIBCOMMON_SUPPORT=y CONFIG_TPL_LIBGENERIC_SUPPORT=y CONFIG_TPL_SERIAL=y @@ -23,6 +26,7 @@ CONFIG_DISTRO_DEFAULTS=y CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y CONFIG_FIT_VERBOSE=y +CONFIG_FIT_BEST_MATCH=y CONFIG_SPL_LOAD_FIT=y # CONFIG_USE_SPL_FIT_GENERATOR is not set CONFIG_BOOTSTAGE=y @@ -47,6 +51,7 @@ CONFIG_TPL_I2C=y CONFIG_TPL_RTC=y CONFIG_VPL=y CONFIG_VPL_ENV_SUPPORT=y +CONFIG_VPL_TEXT_BASE=0x100000 CONFIG_CMD_CPU=y CONFIG_CMD_LICENSE=y CONFIG_CMD_BOOTZ=y @@ -98,7 +103,9 @@ CONFIG_CMD_CBFS=y CONFIG_CMD_CRAMFS=y CONFIG_CMD_EXT4_WRITE=y CONFIG_MAC_PARTITION=y -CONFIG_AMIGA_PARTITION=y +# CONFIG_SPL_MAC_PARTITION is not set +# CONFIG_SPL_DOS_PARTITION is not set +# CONFIG_SPL_EFI_PARTITION is not set CONFIG_OF_CONTROL=y CONFIG_SPL_OF_CONTROL=y CONFIG_TPL_OF_CONTROL=y @@ -113,6 +120,7 @@ CONFIG_NETCONSOLE=y CONFIG_IP_DEFRAG=y CONFIG_SPL_DM=y CONFIG_TPL_DM=y +CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_DM_DMA=y CONFIG_REGMAP=y CONFIG_SPL_REGMAP=y @@ -226,6 +234,8 @@ CONFIG_SPL_SYSRESET=y CONFIG_TPL_SYSRESET=y CONFIG_DM_THERMAL=y CONFIG_TIMER=y +CONFIG_SPL_TIMER=y +CONFIG_VPL_TIMER=y CONFIG_TIMER_EARLY=y CONFIG_SANDBOX_TIMER=y CONFIG_USB=y @@ -246,6 +256,7 @@ CONFIG_CMD_DHRYSTONE=y CONFIG_RSA_VERIFY_WITH_PKEY=y CONFIG_TPM=y CONFIG_LZ4=y +# CONFIG_VPL_LZMA is not set CONFIG_ERRNO_STR=y CONFIG_UNIT_TEST=y CONFIG_SPL_UNIT_TEST=y diff --git a/test/py/tests/test_event_dump.py b/test/py/tests/test_event_dump.py index 972c383711..1a46ca30f4 100644 --- a/test/py/tests/test_event_dump.py +++ b/test/py/tests/test_event_dump.py @@ -17,5 +17,6 @@ def test_event_dump(u_boot_console): expect = '''.*Event type Id Source location -------------------- ------------------------------ ------------------------------ 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 -- cgit v1.2.3 From a56f663f07073713042bb0fd08053aeb667e717b Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 20 Oct 2022 18:23:14 -0600 Subject: vbe: Add info about the VBE device to the fwupd node At present we put the driver in the /chosen node in U-Boot. This is a bit strange, since U-Boot doesn't normally use that node itself. It is better to put it under the bootstd node. To make this work we need to copy create the node under /chosen when fixing up the device tree. Copy over all the properties so that fwupd knows what to do. Update the sandbox device tree accordingly. Signed-off-by: Simon Glass --- arch/sandbox/dts/test.dts | 29 ++++++++++++++--------------- boot/vbe_simple_os.c | 26 ++++++++++++++++++-------- test/boot/vbe_simple.c | 7 ++++++- 3 files changed, 38 insertions(+), 24 deletions(-) (limited to 'test') diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index db72c64b1e..2bcc2e84a7 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -78,6 +78,7 @@ }; bootstd { + u-boot,dm-vpl; compatible = "u-boot,boot-std"; filename-prefixes = "/", "/boot/"; @@ -90,6 +91,19 @@ efi { compatible = "u-boot,distro-efi"; }; + + firmware0 { + u-boot,dm-vpl; + compatible = "fwupd,vbe-simple"; + storage = "mmc1"; + skip-offset = <0x200>; + area-start = <0x400>; + area-size = <0x1000>; + state-offset = <0x400>; + state-size = <0x40>; + version-offset = <0x800>; + version-size = <0x100>; + }; }; fuzzing-engine { @@ -1404,21 +1418,6 @@ compatible = "denx,u-boot-fdt-test"; reg = <9 1>; }; - - fwupd { - compatible = "simple-bus"; - firmware0 { - compatible = "fwupd,vbe-simple"; - storage = "mmc1"; - area-start = <0x400>; - area-size = <0x1000>; - skip-offset = <0x200>; - state-offset = <0x400>; - state-size = <0x40>; - version-offset = <0x800>; - version-size = <0x100>; - }; - }; }; translation-test@8000 { diff --git a/boot/vbe_simple_os.c b/boot/vbe_simple_os.c index 87778bba97..b2041a95a3 100644 --- a/boot/vbe_simple_os.c +++ b/boot/vbe_simple_os.c @@ -62,24 +62,34 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct event *event) */ for (vbe_find_first_device(&dev); dev; vbe_find_next_device(&dev)) { struct simple_state state; - ofnode node, subnode; + ofnode node, subnode, chosen; int ret; if (strcmp("vbe_simple", dev->driver->name)) continue; - /* Check if there is a node to fix up */ - node = oftree_path(tree, "/chosen/fwupd"); - if (!ofnode_valid(node)) - continue; - subnode = ofnode_find_subnode(node, dev->name); - if (!ofnode_valid(subnode)) + /* Check if there is a node to fix up, adding if not */ + chosen = oftree_path(tree, "/chosen"); + if (!ofnode_valid(chosen)) continue; + ret = ofnode_add_subnode(chosen, "fwupd", &node); + if (ret && ret != -EEXIST) + return log_msg_ret("fwu", ret); + + ret = ofnode_add_subnode(node, dev->name, &subnode); + if (ret && ret != -EEXIST) + return log_msg_ret("dev", ret); - log_debug("Fixing up: %s\n", dev->name); ret = device_probe(dev); if (ret) return log_msg_ret("probe", ret); + + /* Copy over the vbe properties for fwupd */ + log_debug("Fixing up: %s\n", dev->name); + ret = ofnode_copy_props(dev_ofnode(dev), subnode); + if (ret) + return log_msg_ret("cp", ret); + ret = vbe_simple_read_state(dev, &state); if (ret) return log_msg_ret("read", ret); diff --git a/test/boot/vbe_simple.c b/test/boot/vbe_simple.c index a50785dbbf..5e61840652 100644 --- a/test/boot/vbe_simple.c +++ b/test/boot/vbe_simple.c @@ -16,7 +16,12 @@ #include #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; -- cgit v1.2.3 From 2a5c67f50a438b266dc72c9401e578ec8b81db16 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 20 Oct 2022 18:23:17 -0600 Subject: vbe: Use a manual test Use a manual test for the VBE test, so we can make the pytest and the C unit test work together properly. Signed-off-by: Simon Glass --- test/boot/vbe_fixup.c | 19 ++++++++----------- test/py/tests/test_vbe.py | 7 ++----- 2 files changed, 10 insertions(+), 16 deletions(-) (limited to 'test') diff --git a/test/boot/vbe_fixup.c b/test/boot/vbe_fixup.c index 7f0f809499..eba5c4ebe6 100644 --- a/test/boot/vbe_fixup.c +++ b/test/boot/vbe_fixup.c @@ -13,21 +13,18 @@ #include #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 -EAGAIN; - 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/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] -- cgit v1.2.3 From 77bec9e3d8bd2dc307447b92a3d5cefd693a62ad Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 20 Oct 2022 18:23:20 -0600 Subject: vbe: Add a test for the VBE flow into U-Boot proper Add a test which checks that VBE boots correctly from TPL through to U-Boot proper. Signed-off-by: Simon Glass --- arch/sandbox/dts/test.dts | 37 +++++++++++++++++++++++++++++++++++++ test/py/tests/test_vbe_vpl.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 test/py/tests/test_vbe_vpl.py (limited to 'test') diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 75eeaf8ca1..25fd2bcab8 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -37,6 +37,8 @@ i2c0 = "/i2c@0"; mmc0 = "/mmc0"; mmc1 = "/mmc1"; + mmc2 = "/mmc2"; + mmc3 = "/mmc3"; pci0 = &pci0; pci1 = &pci1; pci2 = &pci2; @@ -92,6 +94,11 @@ compatible = "u-boot,distro-efi"; }; + /* + * This is used for the VBE OS-request tests. A FAT filesystem + * created in a partition with the VBE information appearing + * before the parititon starts + */ firmware0 { u-boot,dm-vpl; compatible = "fwupd,vbe-simple"; @@ -104,6 +111,28 @@ version-offset = <0x800>; version-size = <0x100>; }; + + /* + * This is used for the VBE VPL tests. The MMC device holds the + * binman image.bin file. The test progresses through each phase + * of U-Boot, loading each in turn from MMC. + * + * Note that the test enables this node (and mmc3) before + * running U-Boot + */ + firmware1 { + u-boot,dm-vpl; + status = "disabled"; + compatible = "fwupd,vbe-simple"; + storage = "mmc3"; + skip-offset = <0x400000>; + area-start = <0>; + area-size = <0xe00000>; + state-offset = <0xdffc00>; + state-size = <0x40>; + version-offset = <0xdffe00>; + version-size = <0x100>; + }; }; fuzzing-engine { @@ -976,6 +1005,14 @@ compatible = "sandbox,mmc"; }; + /* This is used for VBE VPL tests */ + mmc3 { + status = "disabled"; + compatible = "sandbox,mmc"; + filename = "image.bin"; + non-removable; + }; + pch { compatible = "sandbox,pch"; }; 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' -- cgit v1.2.3