From e21ec17d42aa086a2ab95a1ffb1bd09495b9c8e8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 15 Oct 2022 08:08:54 -0600 Subject: [PATCH 01/31] dm: regmap: Disable range checks in SPL A recent change to regmap breaks building of phycore-rk3288 for me. The difference is only a few bytes. Somehow CI seems to pass, even though it fails when I run docker locally. But it prevents me from sending any more pull requests. In any case this board is clearly near the limit. We could revert the offending change, but it is needed for sandbox tests. Instead, add a way to drop the range checks in SPL, since they end up doing nothing if everything is working as expected. This makes phycore-rk3288 build again for me and reduces the size of SPL slightly for a number of boards. Signed-off-by: Simon Glass Fixes: 947d4f132b4 ("regmap: fix range checks") --- drivers/core/regmap.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c index 5ccbf9abb8a..e33bb9d798d 100644 --- a/drivers/core/regmap.c +++ b/drivers/core/regmap.c @@ -4,6 +4,8 @@ * Written by Simon Glass */ +#define LOG_CATEGORY LOGC_DM + #include #include #include @@ -36,6 +38,22 @@ struct regmap_field { DECLARE_GLOBAL_DATA_PTR; +/** + * do_range_check() - Control whether range checks are done + * + * Returns: true to do range checks, false to skip + * + * This is used to reduce code size on SPL where range checks are known not to + * be needed + * + * Add this to the top of the file to enable them: #define LOG_DEBUG + */ +static inline bool do_range_check(void) +{ + return _LOG_DEBUG || !IS_ENABLED(CONFIG_SPL); + +} + /** * regmap_alloc() - Allocate a regmap with a given number of ranges. * @@ -391,7 +409,7 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset, struct regmap_range *range; void *ptr; - if (range_num >= map->range_count) { + if (do_range_check() && range_num >= map->range_count) { debug("%s: range index %d larger than range count\n", __func__, range_num); return -ERANGE; @@ -399,7 +417,8 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset, range = &map->ranges[range_num]; offset <<= map->reg_offset_shift; - if (offset + val_len > range->size || offset + val_len < offset) { + if (do_range_check() && + (offset + val_len > range->size || offset + val_len < offset)) { debug("%s: offset/size combination invalid\n", __func__); return -ERANGE; } From dfecd631922b61a062da3d1fa6a72f9fb93c0952 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:57:50 +0200 Subject: [PATCH 02/31] dm: core: Fix uclass_probe_all to really probe all devices uclass_probe_all uses uclass_first_device/uclass_next_device assigning the return value. The interface for getting meaningful error is uclass_first_device_check/uclass_next_device_check, use it. Also do not stop iteration when an error is encountered. Probing all devices includes those that happen to be after a failing device in the uclass order. Fixes: a59153dfeb ("dm: core: add function uclass_probe_all() to probe all devices") Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- drivers/core/uclass.c | 12 +++++------- test/dm/test-fdt.c | 19 +++++++++++++++---- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index 08d9ed82de2..a591e224032 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -799,20 +799,18 @@ int uclass_pre_remove_device(struct udevice *dev) int uclass_probe_all(enum uclass_id id) { struct udevice *dev; - int ret; + int ret, err; - ret = uclass_first_device(id, &dev); - if (ret || !dev) - return ret; + err = uclass_first_device_check(id, &dev); /* Scanning uclass to probe all devices */ while (dev) { - ret = uclass_next_device(&dev); + ret = uclass_next_device_check(&dev); if (ret) - return ret; + err = ret; } - return 0; + return err; } int uclass_id_count(enum uclass_id id) diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 012f2f455f6..1f14513d9f1 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -392,10 +392,10 @@ DM_TEST(dm_test_fdt_offset, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT | UT_TESTF_FLAT_TREE); /** - * Test various error conditions with uclass_first_device() and - * uclass_next_device() + * Test various error conditions with uclass_first_device(), + * uclass_next_device(), and uclass_probe_all() */ -static int dm_test_first_next_device(struct unit_test_state *uts) +static int dm_test_first_next_device_probeall(struct unit_test_state *uts) { struct dm_testprobe_pdata *pdata; struct udevice *dev, *parent = NULL; @@ -428,9 +428,20 @@ static int dm_test_first_next_device(struct unit_test_state *uts) device_remove(parent, DM_REMOVE_NORMAL); ut_asserteq(-ENOENT, uclass_first_device(UCLASS_TEST_PROBE, &dev)); + /* Now that broken devices are set up test probe_all */ + device_remove(parent, DM_REMOVE_NORMAL); + /* There are broken devices so an error should be returned */ + ut_assert(uclass_probe_all(UCLASS_TEST_PROBE) < 0); + /* but non-error device should be probed nonetheless */ + ut_assertok(uclass_get_device(UCLASS_TEST_PROBE, 2, &dev)); + ut_assert(dev_get_flags(dev) & DM_FLAG_ACTIVATED); + ut_assertok(uclass_get_device(UCLASS_TEST_PROBE, 3, &dev)); + ut_assert(dev_get_flags(dev) & DM_FLAG_ACTIVATED); + return 0; } -DM_TEST(dm_test_first_next_device, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); +DM_TEST(dm_test_first_next_device_probeall, + UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); /* Test iteration through devices in a uclass */ static int dm_test_uclass_foreach(struct unit_test_state *uts) From c0648b7b9dc10ed49e2d4b1bedaa5ccd17e23fb3 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:57:51 +0200 Subject: [PATCH 03/31] dm: treewide: Do not opencode uclass_probe_all() We already have a function for probing all devices of a specific class, use it. Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- drivers/cpu/cpu-uclass.c | 20 ++++---------------- drivers/virtio/virtio-uclass.c | 15 +-------------- test/dm/core.c | 17 +++-------------- test/test-main.c | 11 +---------- 4 files changed, 9 insertions(+), 54 deletions(-) diff --git a/drivers/cpu/cpu-uclass.c b/drivers/cpu/cpu-uclass.c index 71e5900d70e..a7548325265 100644 --- a/drivers/cpu/cpu-uclass.c +++ b/drivers/cpu/cpu-uclass.c @@ -20,25 +20,13 @@ DECLARE_GLOBAL_DATA_PTR; int cpu_probe_all(void) { - struct udevice *cpu; - int ret; + int ret = uclass_probe_all(UCLASS_CPU); - ret = uclass_first_device(UCLASS_CPU, &cpu); if (ret) { - debug("%s: No CPU found (err = %d)\n", __func__, ret); - return ret; + debug("%s: Error while probing CPUs (err = %d %s)\n", + __func__, ret, errno_str(ret)); } - - while (cpu) { - ret = uclass_next_device(&cpu); - if (ret) { - debug("%s: Error while probing CPU (err = %d)\n", - __func__, ret); - return ret; - } - } - - return 0; + return ret; } int cpu_is_current(struct udevice *cpu) diff --git a/drivers/virtio/virtio-uclass.c b/drivers/virtio/virtio-uclass.c index 9e2d0e06a1e..da4f2f26a63 100644 --- a/drivers/virtio/virtio-uclass.c +++ b/drivers/virtio/virtio-uclass.c @@ -183,21 +183,8 @@ void virtio_driver_features_init(struct virtio_dev_priv *priv, int virtio_init(void) { - struct udevice *bus; - int ret; - /* Enumerate all known virtio devices */ - ret = uclass_first_device(UCLASS_VIRTIO, &bus); - if (ret) - return ret; - - while (bus) { - ret = uclass_next_device(&bus); - if (ret) - break; - } - - return ret; + return uclass_probe_all(UCLASS_VIRTIO); } static int virtio_uclass_pre_probe(struct udevice *udev) diff --git a/test/dm/core.c b/test/dm/core.c index fd4d7569728..84eb76ed5fc 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -512,23 +512,15 @@ static int dm_test_leak(struct unit_test_state *uts) int i; for (i = 0; i < 2; i++) { - struct udevice *dev; int ret; - int id; dm_leak_check_start(uts); ut_assertok(dm_scan_plat(false)); ut_assertok(dm_scan_fdt(false)); - /* Scanning the uclass is enough to probe all the devices */ - for (id = UCLASS_ROOT; id < UCLASS_COUNT; id++) { - for (ret = uclass_first_device(UCLASS_TEST, &dev); - dev; - ret = uclass_next_device(&dev)) - ; - ut_assertok(ret); - } + ret = uclass_probe_all(UCLASS_TEST); + ut_assertok(ret); ut_assertok(dm_leak_check_end(uts)); } @@ -653,10 +645,7 @@ static int dm_test_children(struct unit_test_state *uts) ut_asserteq(2 + NODE_COUNT, dm_testdrv_op_count[DM_TEST_OP_PROBE]); /* Probe everything */ - for (ret = uclass_first_device(UCLASS_TEST, &dev); - dev; - ret = uclass_next_device(&dev)) - ; + ret = uclass_probe_all(UCLASS_TEST); ut_assertok(ret); ut_asserteq(total, dm_testdrv_op_count[DM_TEST_OP_PROBE]); diff --git a/test/test-main.c b/test/test-main.c index d74df297c43..a98a77d68fc 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -165,16 +165,7 @@ static int dm_test_post_run(struct unit_test_state *uts) /* Ensure all the test devices are probed */ static int do_autoprobe(struct unit_test_state *uts) { - struct udevice *dev; - int ret; - - /* Scanning the uclass is enough to probe all the devices */ - for (ret = uclass_first_device(UCLASS_TEST, &dev); - dev; - ret = uclass_next_device(&dev)) - ; - - return ret; + return uclass_probe_all(UCLASS_TEST); } /* From 5afe93a18c92a5f08dc6d38c4525e378ffb5067a Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:57:52 +0200 Subject: [PATCH 04/31] dm: pci: Fix device PCI iteration When there is no PCI bus uclass_first_device will return no bus and no error which will result in pci_find_first_device calling skip_to_next_device with no bus, and the bus is only checked at the end of the while cycle, not the beginning. Fixes: 76c3fbcd3d ("dm: pci: Add a way to iterate through all PCI devices") Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- drivers/pci/pci-uclass.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 058b2f6359d..5cff81ac443 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1217,7 +1217,7 @@ static int skip_to_next_device(struct udevice *bus, struct udevice **devp) * Scan through all the PCI controllers. On x86 there will only be one * but that is not necessarily true on other hardware. */ - do { + while (bus) { device_find_first_child(bus, &dev); if (dev) { *devp = dev; @@ -1226,7 +1226,7 @@ static int skip_to_next_device(struct udevice *bus, struct udevice **devp) ret = uclass_next_device(&bus); if (ret) return ret; - } while (bus); + } return 0; } From 28a22cd9a482b947b21646ae595e3284cdea3002 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:57:53 +0200 Subject: [PATCH 05/31] bootstd: Fix listing boot devices bootdev_list() uses uclass_*_device_err() to iterate devices. However, the only value _err adds is returning an error when the device pointer is null, and that's checked anyway. Also there is some intent to report errors, and that's what uclass_*_device_check() is for, use it. Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- boot/bootdev-uclass.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c index 13ac69eb392..9d98bee4549 100644 --- a/boot/bootdev-uclass.c +++ b/boot/bootdev-uclass.c @@ -195,7 +195,7 @@ void bootdev_list(bool probe) printf("Seq Probed Status Uclass Name\n"); printf("--- ------ ------ -------- ------------------\n"); if (probe) - ret = uclass_first_device_err(UCLASS_BOOTDEV, &dev); + ret = uclass_first_device_check(UCLASS_BOOTDEV, &dev); else ret = uclass_find_first_device(UCLASS_BOOTDEV, &dev); for (i = 0; dev; i++) { @@ -204,7 +204,7 @@ void bootdev_list(bool probe) ret ? simple_itoa(ret) : "OK", dev_get_uclass_name(dev_get_parent(dev)), dev->name); if (probe) - ret = uclass_next_device_err(&dev); + ret = uclass_next_device_check(&dev); else ret = uclass_find_next_device(&dev); } From 2cb43ef1c22302820061d4d11ddce85872e993e1 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:57:54 +0200 Subject: [PATCH 06/31] usb: ether: Fix error handling in usb_ether_init The code checks the return value from uclass_first_device as well as that the device exists but it passes on the return value which may be zero if there are no gadget devices. Just check that a device was returned and return -ENODEV otherwise. Also remove the dev variable which is not really used for anything. Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- drivers/usb/gadget/ether.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 6ce389de9f0..43aec7ffa70 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -2636,18 +2636,17 @@ static const struct eth_ops usb_eth_ops = { int usb_ether_init(void) { - struct udevice *dev; struct udevice *usb_dev; int ret; - ret = uclass_first_device(UCLASS_USB_GADGET_GENERIC, &usb_dev); - if (!usb_dev || ret) { + uclass_first_device(UCLASS_USB_GADGET_GENERIC, &usb_dev); + if (!usb_dev) { pr_err("No USB device found\n"); - return ret; + return -ENODEV; } - ret = device_bind_driver(usb_dev, "usb_ether", "usb_ether", &dev); - if (!dev || ret) { + ret = device_bind_driver(usb_dev, "usb_ether", "usb_ether", NULL); + if (ret) { pr_err("usb - not able to bind usb_ether device\n"); return ret; } From 7ff12631b44cdbb86c868ce02efd5a06429c7683 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:57:55 +0200 Subject: [PATCH 07/31] stdio: Fix class iteration in stdio_add_devices() There is a complaint in the code that iterates keyboards that we don't have the _check variant of class iterator but we in fact do, use it. In the code that iterates video devices there is an attempt to print errors but the simple iterator does not return a device when there is an error. Use the _check variant of the iterator as well. Also format error messages consistently. Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- common/stdio.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/common/stdio.c b/common/stdio.c index 13083842cbd..e316a355fae 100644 --- a/common/stdio.c +++ b/common/stdio.c @@ -314,7 +314,6 @@ int stdio_init_tables(void) int stdio_add_devices(void) { struct udevice *dev; - struct uclass *uc; int ret; if (IS_ENABLED(CONFIG_DM_KEYBOARD)) { @@ -324,24 +323,18 @@ int stdio_add_devices(void) * have a list of input devices to start up in the stdin * environment variable. That work probably makes more sense * when stdio itself is converted to driver model. - * - * TODO(sjg@chromium.org): Convert changing - * uclass_first_device() etc. to return the device even on - * error. Then we could use that here. */ - ret = uclass_get(UCLASS_KEYBOARD, &uc); - if (ret) - return ret; /* * Don't report errors to the caller - assume that they are * non-fatal */ - uclass_foreach_dev(dev, uc) { - ret = device_probe(dev); + for (ret = uclass_first_device_check(UCLASS_KEYBOARD, &dev); + dev; + ret = uclass_next_device_check(&dev)) { if (ret) - printf("Failed to probe keyboard '%s'\n", - dev->name); + printf("%s: Failed to probe keyboard '%s' (ret=%d)\n", + __func__, dev->name, ret); } } #if CONFIG_IS_ENABLED(SYS_I2C_LEGACY) @@ -361,13 +354,14 @@ int stdio_add_devices(void) int ret; if (!IS_ENABLED(CONFIG_SYS_CONSOLE_IS_IN_ENV)) { - for (ret = uclass_first_device(UCLASS_VIDEO, &vdev); - vdev; - ret = uclass_next_device(&vdev)) - ; - if (ret) - printf("%s: Video device failed (ret=%d)\n", - __func__, ret); + for (ret = uclass_first_device_check(UCLASS_VIDEO, + &vdev); + vdev; + ret = uclass_next_device_check(&vdev)) { + if (ret) + printf("%s: Failed to probe video device '%s' (ret=%d)\n", + __func__, vdev->name, ret); + } } if (IS_ENABLED(CONFIG_SPLASH_SCREEN) && IS_ENABLED(CONFIG_CMD_BMP)) From f42642347120d988e7cf6986139e7ec68e36bc6b Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:57:56 +0200 Subject: [PATCH 08/31] video: ipuv3: Fix error handling when getting the display The code checks that uclass_first_device returned a device but the returned value that is assigned is never used. Use uclass_first_device_err instead, and move the error return outside of the if block. Fixes: f4ec1ae08e ("mxc_ipuv3_fb.c: call display_enable") Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- drivers/video/imx/mxc_ipuv3_fb.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/video/imx/mxc_ipuv3_fb.c b/drivers/video/imx/mxc_ipuv3_fb.c index 49bbeefdd8e..8b01a1be112 100644 --- a/drivers/video/imx/mxc_ipuv3_fb.c +++ b/drivers/video/imx/mxc_ipuv3_fb.c @@ -609,12 +609,11 @@ static int ipuv3_video_probe(struct udevice *dev) return ret; #if defined(CONFIG_DISPLAY) - ret = uclass_first_device(UCLASS_DISPLAY, &disp_dev); - if (disp_dev) { + ret = uclass_first_device_err(UCLASS_DISPLAY, &disp_dev); + if (!ret) ret = display_enable(disp_dev, 16, NULL); - if (ret < 0) - return ret; - } + if (ret < 0) + return ret; #endif if (CONFIG_IS_ENABLED(PANEL)) { struct udevice *panel_dev; From 9244645f929bdff2ce87d1d967a78a0e05cebd80 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:57:57 +0200 Subject: [PATCH 09/31] w1: Fix bus counting in w1_get_bus Use uclass_first_device_check/uclass_next_device_check to correctly count buses that fail to probe. Fixes: d3e19cf919 ("w1: Add 1-Wire uclass") Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- drivers/w1/w1-uclass.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/drivers/w1/w1-uclass.c b/drivers/w1/w1-uclass.c index 52b519c21d2..de4f25bcf95 100644 --- a/drivers/w1/w1-uclass.c +++ b/drivers/w1/w1-uclass.c @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -182,24 +183,25 @@ static int w1_enumerate(struct udevice *bus) int w1_get_bus(int busnum, struct udevice **busp) { int ret, i = 0; - struct udevice *dev; - for (ret = uclass_first_device(UCLASS_W1, &dev); - dev && !ret; - ret = uclass_next_device(&dev), i++) { + for (ret = uclass_first_device_check(UCLASS_W1, &dev); + dev; + ret = uclass_next_device_check(&dev), i++) { if (i == busnum) { + if (ret) { + debug("Cannot probe w1 bus %d: %d (%s)\n", + busnum, ret, errno_str(ret)); + return ret; + } *busp = dev; return 0; } } - if (!ret) { - debug("Cannot find w1 bus %d\n", busnum); - ret = -ENODEV; - } + debug("Cannot find w1 bus %d\n", busnum); - return ret; + return -ENODEV; } u8 w1_get_device_family(struct udevice *dev) From 8676ae36ae4617b20b5cc49972c54c63c7b27bb3 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:57:58 +0200 Subject: [PATCH 10/31] cmd: List all uclass devices regardless of probe error There are a few commands that iterate uclass with uclass_first_device/uclass_next_device or the _err variant. Use the _check class iterator variant to get devices that fail to probe as well, and print the status. Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- cmd/adc.c | 24 ++++++++++-------------- cmd/demo.c | 15 +++++++++------ cmd/gpio.c | 15 +++++++++++---- cmd/pmic.c | 15 ++++++++------- cmd/regulator.c | 13 +++++++------ 5 files changed, 45 insertions(+), 37 deletions(-) diff --git a/cmd/adc.c b/cmd/adc.c index 1c5d3e10a39..a739d9e4641 100644 --- a/cmd/adc.c +++ b/cmd/adc.c @@ -12,23 +12,19 @@ static int do_adc_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct udevice *dev; - int ret; + int ret, err; - ret = uclass_first_device_err(UCLASS_ADC, &dev); - if (ret) { - printf("No available ADC device\n"); - return CMD_RET_FAILURE; + ret = err = uclass_first_device_check(UCLASS_ADC, &dev); + + while (dev) { + printf("- %s status: %i\n", dev->name, ret); + + ret = uclass_next_device_check(&dev); + if (ret) + err = ret; } - do { - printf("- %s\n", dev->name); - - ret = uclass_next_device(&dev); - if (ret) - return CMD_RET_FAILURE; - } while (dev); - - return CMD_RET_SUCCESS; + return err ? CMD_RET_FAILURE : CMD_RET_SUCCESS; } static int do_adc_info(struct cmd_tbl *cmdtp, int flag, int argc, diff --git a/cmd/demo.c b/cmd/demo.c index 571f562ec68..ebd5a241c36 100644 --- a/cmd/demo.c +++ b/cmd/demo.c @@ -64,20 +64,23 @@ static int do_demo_light(struct cmd_tbl *cmdtp, int flag, int argc, int do_demo_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct udevice *dev; - int i, ret; + int i, ret, err = 0; puts("Demo uclass entries:\n"); - for (i = 0, ret = uclass_first_device(UCLASS_DEMO, &dev); + for (i = 0, ret = uclass_first_device_check(UCLASS_DEMO, &dev); dev; - ret = uclass_next_device(&dev)) { - printf("entry %d - instance %08x, ops %08x, plat %08x\n", + ret = uclass_next_device_check(&dev)) { + printf("entry %d - instance %08x, ops %08x, plat %08x, status %i\n", i++, (uint)map_to_sysmem(dev), (uint)map_to_sysmem(dev->driver->ops), - (uint)map_to_sysmem(dev_get_plat(dev))); + (uint)map_to_sysmem(dev_get_plat(dev)), + ret); + if (ret) + err = ret; } - return cmd_process_error(cmdtp, ret); + return cmd_process_error(cmdtp, err); } static struct cmd_tbl demo_commands[] = { diff --git a/cmd/gpio.c b/cmd/gpio.c index 53e9ce666f9..f4565982ecd 100644 --- a/cmd/gpio.c +++ b/cmd/gpio.c @@ -77,17 +77,24 @@ static int do_gpio_status(bool all, const char *gpio_name) struct udevice *dev; int banklen; int flags; - int ret; + int ret, err = 0; flags = 0; if (gpio_name && !*gpio_name) gpio_name = NULL; - for (ret = uclass_first_device(UCLASS_GPIO, &dev); + for (ret = uclass_first_device_check(UCLASS_GPIO, &dev); dev; - ret = uclass_next_device(&dev)) { + ret = uclass_next_device_check(&dev)) { const char *bank_name; int num_bits; + if (ret) { + printf("GPIO device %s probe error %i\n", + dev->name, ret); + err = ret; + continue; + } + flags |= FLAG_SHOW_BANK; if (all) flags |= FLAG_SHOW_ALL; @@ -120,7 +127,7 @@ static int do_gpio_status(bool all, const char *gpio_name) flags |= FLAG_SHOW_NEWLINE; } - return ret; + return err; } #endif diff --git a/cmd/pmic.c b/cmd/pmic.c index 0cb44d07409..49a405fa297 100644 --- a/cmd/pmic.c +++ b/cmd/pmic.c @@ -51,25 +51,26 @@ static int do_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct udevice *dev; - int ret; + int ret, err = 0; printf("| %-*.*s| %-*.*s| %s @ %s\n", LIMIT_DEV, LIMIT_DEV, "Name", LIMIT_PARENT, LIMIT_PARENT, "Parent name", "Parent uclass", "seq"); - for (ret = uclass_first_device(UCLASS_PMIC, &dev); dev; - ret = uclass_next_device(&dev)) { + for (ret = uclass_first_device_check(UCLASS_PMIC, &dev); dev; + ret = uclass_next_device_check(&dev)) { if (ret) - continue; + err = ret; - printf("| %-*.*s| %-*.*s| %s @ %d\n", + printf("| %-*.*s| %-*.*s| %s @ %d | status: %i\n", LIMIT_DEV, LIMIT_DEV, dev->name, LIMIT_PARENT, LIMIT_PARENT, dev->parent->name, - dev_get_uclass_name(dev->parent), dev_seq(dev->parent)); + dev_get_uclass_name(dev->parent), dev_seq(dev->parent), + ret); } - if (ret) + if (err) return CMD_RET_FAILURE; return CMD_RET_SUCCESS; diff --git a/cmd/regulator.c b/cmd/regulator.c index 60a70036d68..ed4996dbd2b 100644 --- a/cmd/regulator.c +++ b/cmd/regulator.c @@ -205,7 +205,7 @@ static void do_status_detail(struct udevice *dev, constraint(" * mode id:", mode, mode_name); } -static void do_status_line(struct udevice *dev) +static void do_status_line(struct udevice *dev, int status) { struct dm_regulator_uclass_plat *pdata; int current, value, mode; @@ -231,6 +231,7 @@ static void do_status_line(struct udevice *dev) printf("%-10s", mode_name); else printf("%-10s", "-"); + printf(" %i", status); printf("\n"); } @@ -250,11 +251,11 @@ static int do_status(struct cmd_tbl *cmdtp, int flag, int argc, } /* Show all of them in a list, probing them as needed */ - printf("%-20s %-10s %10s %10s %-10s\n", "Name", "Enabled", "uV", "mA", - "Mode"); - for (ret = uclass_first_device(UCLASS_REGULATOR, &dev); dev; - ret = uclass_next_device(&dev)) - do_status_line(dev); + printf("%-20s %-10s %10s %10s %-10s %s\n", "Name", "Enabled", "uV", "mA", + "Mode", "Status"); + for (ret = uclass_first_device_check(UCLASS_REGULATOR, &dev); dev; + ret = uclass_next_device_check(&dev)) + do_status_line(dev, ret); return CMD_RET_SUCCESS; } From c726fc01cf669e3e2979da8665ef660e7d3ba464 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:57:59 +0200 Subject: [PATCH 11/31] dm: treewide: Use uclass_first_device_err when accessing one device There is a number of users that use uclass_first_device to access the first and (assumed) only device in uclass. Some check the return value of uclass_first_device and also that a device was returned which is exactly what uclass_first_device_err does. Some are not checking that a device was returned and can potentially crash if no device exists in the uclass. Finally there is one that returns NULL on error either way. Convert all of these to use uclass_first_device_err instead, the return value will be removed from uclass_first_device in a later patch. Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- arch/arm/mach-omap2/am33xx/board.c | 4 ++-- arch/x86/cpu/broadwell/cpu.c | 4 +--- arch/x86/cpu/intel_common/cpu.c | 4 +--- arch/x86/lib/pinctrl_ich6.c | 4 +--- board/intel/cougarcanyon2/cougarcanyon2.c | 4 +--- drivers/mmc/omap_hsmmc.c | 2 +- drivers/serial/serial-uclass.c | 2 +- drivers/serial/serial_bcm283x_mu.c | 2 +- drivers/serial/serial_bcm283x_pl011.c | 2 +- drivers/sysreset/sysreset_ast.c | 2 +- drivers/video/exynos/exynos_fb.c | 14 +++----------- drivers/video/mali_dp.c | 2 +- drivers/video/stm32/stm32_dsi.c | 2 +- drivers/video/tegra124/dp.c | 4 ++-- lib/acpi/acpi_table.c | 2 +- lib/efi_loader/efi_gop.c | 2 +- net/eth-uclass.c | 4 ++-- test/boot/bootmeth.c | 2 +- test/dm/acpi.c | 14 +++++++------- test/dm/devres.c | 4 ++-- test/dm/i2c.c | 8 ++++---- test/dm/virtio_device.c | 8 ++++---- test/dm/virtio_rng.c | 2 +- test/fuzz/cmd_fuzz.c | 2 +- test/fuzz/virtio.c | 2 +- 25 files changed, 43 insertions(+), 59 deletions(-) diff --git a/arch/arm/mach-omap2/am33xx/board.c b/arch/arm/mach-omap2/am33xx/board.c index 7f1b84e466d..f393ff91441 100644 --- a/arch/arm/mach-omap2/am33xx/board.c +++ b/arch/arm/mach-omap2/am33xx/board.c @@ -265,8 +265,8 @@ int arch_misc_init(void) struct udevice *dev; int ret; - ret = uclass_first_device(UCLASS_MISC, &dev); - if (ret || !dev) + ret = uclass_first_device_err(UCLASS_MISC, &dev); + if (ret) return ret; #if defined(CONFIG_DM_ETH) && defined(CONFIG_USB_ETHER) diff --git a/arch/x86/cpu/broadwell/cpu.c b/arch/x86/cpu/broadwell/cpu.c index 2adcf4b242c..7877961451a 100644 --- a/arch/x86/cpu/broadwell/cpu.c +++ b/arch/x86/cpu/broadwell/cpu.c @@ -31,11 +31,9 @@ static int broadwell_init_cpu(void *ctx, struct event *event) int ret; /* Start up the LPC so we have serial */ - ret = uclass_first_device(UCLASS_LPC, &dev); + ret = uclass_first_device_err(UCLASS_LPC, &dev); if (ret) return ret; - if (!dev) - return -ENODEV; ret = cpu_set_flex_ratio_to_tdp_nominal(); if (ret) return ret; diff --git a/arch/x86/cpu/intel_common/cpu.c b/arch/x86/cpu/intel_common/cpu.c index 96d05e2eb3a..8f489e6c651 100644 --- a/arch/x86/cpu/intel_common/cpu.c +++ b/arch/x86/cpu/intel_common/cpu.c @@ -61,11 +61,9 @@ int cpu_common_init(void) /* Early chipset init required before RAM init can work */ uclass_first_device(UCLASS_NORTHBRIDGE, &dev); - ret = uclass_first_device(UCLASS_LPC, &lpc); + ret = uclass_first_device_err(UCLASS_LPC, &lpc); if (ret) return ret; - if (!lpc) - return -ENODEV; /* Cause the SATA device to do its early init */ uclass_first_device(UCLASS_AHCI, &dev); diff --git a/arch/x86/lib/pinctrl_ich6.c b/arch/x86/lib/pinctrl_ich6.c index fd5e311b291..c93f245845d 100644 --- a/arch/x86/lib/pinctrl_ich6.c +++ b/arch/x86/lib/pinctrl_ich6.c @@ -160,11 +160,9 @@ static int ich6_pinctrl_probe(struct udevice *dev) u32 iobase = -1; debug("%s: start\n", __func__); - ret = uclass_first_device(UCLASS_PCH, &pch); + ret = uclass_first_device_err(UCLASS_PCH, &pch); if (ret) return ret; - if (!pch) - return -ENODEV; /* * Get the memory/io base address to configure every pins. diff --git a/board/intel/cougarcanyon2/cougarcanyon2.c b/board/intel/cougarcanyon2/cougarcanyon2.c index ce11eae59d5..7f61ef8b366 100644 --- a/board/intel/cougarcanyon2/cougarcanyon2.c +++ b/board/intel/cougarcanyon2/cougarcanyon2.c @@ -21,11 +21,9 @@ int board_early_init_f(void) struct udevice *pch; int ret; - ret = uclass_first_device(UCLASS_PCH, &pch); + ret = uclass_first_device_err(UCLASS_PCH, &pch); if (ret) return ret; - if (!pch) - return -ENODEV; /* Initialize LPC interface to turn on superio chipset decode range */ dm_pci_write_config16(pch, LPC_IO_DEC, COMA_DEC_RANGE | COMB_DEC_RANGE); diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c index b2f4a4e7219..a2595d19e7f 100644 --- a/drivers/mmc/omap_hsmmc.c +++ b/drivers/mmc/omap_hsmmc.c @@ -644,7 +644,7 @@ static int omap_hsmmc_execute_tuning(struct udevice *dev, uint opcode) ((mmc->selected_mode == UHS_SDR50) && (val & CAPA2_TSDR50)))) return 0; - ret = uclass_first_device(UCLASS_THERMAL, &thermal_dev); + ret = uclass_first_device_err(UCLASS_THERMAL, &thermal_dev); if (ret) { printf("Couldn't get thermal device for tuning\n"); return ret; diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index da3e1eb3ab1..83cda1f2040 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -143,7 +143,7 @@ static void serial_find_console_or_panic(void) #else if (!uclass_get_device_by_seq(UCLASS_SERIAL, INDEX, &dev) || !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) || - (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) { + !uclass_first_device_err(UCLASS_SERIAL, &dev)) { gd->cur_serial_dev = dev; return; } diff --git a/drivers/serial/serial_bcm283x_mu.c b/drivers/serial/serial_bcm283x_mu.c index 493a42b4ccc..12cbcb9858c 100644 --- a/drivers/serial/serial_bcm283x_mu.c +++ b/drivers/serial/serial_bcm283x_mu.c @@ -147,7 +147,7 @@ static bool bcm283x_is_serial_muxed(void) int serial_gpio = 15; struct udevice *dev; - if (uclass_first_device(UCLASS_PINCTRL, &dev) || !dev) + if (uclass_first_device_err(UCLASS_PINCTRL, &dev)) return false; if (pinctrl_get_gpio_mux(dev, 0, serial_gpio) != BCM2835_GPIO_ALT5) diff --git a/drivers/serial/serial_bcm283x_pl011.c b/drivers/serial/serial_bcm283x_pl011.c index fe746294cdc..7d172cdac0a 100644 --- a/drivers/serial/serial_bcm283x_pl011.c +++ b/drivers/serial/serial_bcm283x_pl011.c @@ -24,7 +24,7 @@ static bool bcm283x_is_serial_muxed(void) int serial_gpio = 15; struct udevice *dev; - if (uclass_first_device(UCLASS_PINCTRL, &dev) || !dev) + if (uclass_first_device_err(UCLASS_PINCTRL, &dev)) return false; if (pinctrl_get_gpio_mux(dev, 0, serial_gpio) != BCM2835_GPIO_ALT0) diff --git a/drivers/sysreset/sysreset_ast.c b/drivers/sysreset/sysreset_ast.c index d747ed00a7f..92fad96871b 100644 --- a/drivers/sysreset/sysreset_ast.c +++ b/drivers/sysreset/sysreset_ast.c @@ -18,7 +18,7 @@ static int ast_sysreset_request(struct udevice *dev, enum sysreset_t type) { struct udevice *wdt; u32 reset_mode; - int ret = uclass_first_device(UCLASS_WDT, &wdt); + int ret = uclass_first_device_err(UCLASS_WDT, &wdt); if (ret) return ret; diff --git a/drivers/video/exynos/exynos_fb.c b/drivers/video/exynos/exynos_fb.c index 69992b3c2ba..86970a6d5d2 100644 --- a/drivers/video/exynos/exynos_fb.c +++ b/drivers/video/exynos/exynos_fb.c @@ -640,25 +640,17 @@ static int exynos_fb_probe(struct udevice *dev) #endif exynos_fimd_lcd_init(dev); - ret = uclass_first_device(UCLASS_PANEL, &panel); + ret = uclass_first_device_err(UCLASS_PANEL, &panel); if (ret) { - printf("LCD panel failed to probe\n"); + printf("%s: LCD panel failed to probe %d\n", __func__, ret); return ret; } - if (!panel) { - printf("LCD panel not found\n"); - return -ENODEV; - } - ret = uclass_first_device(UCLASS_DISPLAY, &dp); + ret = uclass_first_device_err(UCLASS_DISPLAY, &dp); if (ret) { debug("%s: Display device error %d\n", __func__, ret); return ret; } - if (!dev) { - debug("%s: Display device missing\n", __func__); - return -ENODEV; - } ret = display_enable(dp, 18, NULL); if (ret) { debug("%s: Display enable error %d\n", __func__, ret); diff --git a/drivers/video/mali_dp.c b/drivers/video/mali_dp.c index ba1ddd64e08..cbcdb99e1f0 100644 --- a/drivers/video/mali_dp.c +++ b/drivers/video/mali_dp.c @@ -244,7 +244,7 @@ static int malidp_update_timings_from_edid(struct udevice *dev, struct udevice *disp_dev; int err; - err = uclass_first_device(UCLASS_DISPLAY, &disp_dev); + err = uclass_first_device_err(UCLASS_DISPLAY, &disp_dev); if (err) return err; diff --git a/drivers/video/stm32/stm32_dsi.c b/drivers/video/stm32/stm32_dsi.c index 5871ac7c4ff..e6347bb8da6 100644 --- a/drivers/video/stm32/stm32_dsi.c +++ b/drivers/video/stm32/stm32_dsi.c @@ -346,7 +346,7 @@ static int stm32_dsi_attach(struct udevice *dev) struct display_timing timings; int ret; - ret = uclass_first_device(UCLASS_PANEL, &priv->panel); + ret = uclass_first_device_err(UCLASS_PANEL, &priv->panel); if (ret) { dev_err(dev, "panel device error %d\n", ret); return ret; diff --git a/drivers/video/tegra124/dp.c b/drivers/video/tegra124/dp.c index ee4f09a0c49..b27b1633bab 100644 --- a/drivers/video/tegra124/dp.c +++ b/drivers/video/tegra124/dp.c @@ -1494,8 +1494,8 @@ int tegra_dp_enable(struct udevice *dev, int panel_bpp, return -ENOLINK; } - ret = uclass_first_device(UCLASS_VIDEO_BRIDGE, &sor); - if (ret || !sor) { + ret = uclass_first_device_err(UCLASS_VIDEO_BRIDGE, &sor); + if (ret) { debug("dp: failed to find SOR device: ret=%d\n", ret); return ret; } diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c index f8642f99420..7c4189e2434 100644 --- a/lib/acpi/acpi_table.c +++ b/lib/acpi/acpi_table.c @@ -40,7 +40,7 @@ int acpi_create_dmar(struct acpi_dmar *dmar, enum dmar_flags flags) struct udevice *cpu; int ret; - ret = uclass_first_device(UCLASS_CPU, &cpu); + ret = uclass_first_device_err(UCLASS_CPU, &cpu); if (ret) return log_msg_ret("cpu", ret); ret = cpu_get_info(cpu, &info); diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 5908b5c6466..20bd7fff086 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -482,7 +482,7 @@ efi_status_t efi_gop_register(void) struct video_priv *priv; /* We only support a single video output device for now */ - if (uclass_first_device(UCLASS_VIDEO, &vdev) || !vdev) { + if (uclass_first_device_err(UCLASS_VIDEO, &vdev)) { debug("WARNING: No video device\n"); return EFI_SUCCESS; } diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 0f6b45b002c..8c3f9cc31b7 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -91,8 +91,8 @@ struct udevice *eth_get_dev(void) eth_errno = uclass_get_device_by_seq(UCLASS_ETH, 0, &uc_priv->current); if (eth_errno) - eth_errno = uclass_first_device(UCLASS_ETH, - &uc_priv->current); + eth_errno = uclass_first_device_err(UCLASS_ETH, + &uc_priv->current); } return uc_priv->current; } diff --git a/test/boot/bootmeth.c b/test/boot/bootmeth.c index fb627313396..f0b5ab9adb3 100644 --- a/test/boot/bootmeth.c +++ b/test/boot/bootmeth.c @@ -156,7 +156,7 @@ static int bootmeth_state(struct unit_test_state *uts) struct udevice *dev; char buf[50]; - ut_assertok(uclass_first_device(UCLASS_BOOTMETH, &dev)); + ut_assertok(uclass_first_device_err(UCLASS_BOOTMETH, &dev)); ut_assertnonnull(dev); ut_assertok(bootmeth_get_state_desc(dev, buf, sizeof(buf))); diff --git a/test/dm/acpi.c b/test/dm/acpi.c index edad91329f9..9634fc2e900 100644 --- a/test/dm/acpi.c +++ b/test/dm/acpi.c @@ -169,28 +169,28 @@ static int dm_test_acpi_get_name(struct unit_test_state *uts) ut_asserteq_str("GHIJ", name); /* Test getting the name from acpi_device_get_name() */ - ut_assertok(uclass_first_device(UCLASS_I2C, &i2c)); + ut_assertok(uclass_first_device_err(UCLASS_I2C, &i2c)); ut_assertok(acpi_get_name(i2c, name)); ut_asserteq_str("I2C0", name); - ut_assertok(uclass_first_device(UCLASS_SPI, &spi)); + ut_assertok(uclass_first_device_err(UCLASS_SPI, &spi)); ut_assertok(acpi_get_name(spi, name)); ut_asserteq_str("SPI0", name); /* ACPI doesn't know about the timer */ - ut_assertok(uclass_first_device(UCLASS_TIMER, &timer)); + ut_assertok(uclass_first_device_err(UCLASS_TIMER, &timer)); ut_asserteq(-ENOENT, acpi_get_name(timer, name)); /* May as well test the rest of the cases */ - ut_assertok(uclass_first_device(UCLASS_SOUND, &sound)); + ut_assertok(uclass_first_device_err(UCLASS_SOUND, &sound)); ut_assertok(acpi_get_name(sound, name)); ut_asserteq_str("HDAS", name); - ut_assertok(uclass_first_device(UCLASS_PCI, &pci)); + ut_assertok(uclass_first_device_err(UCLASS_PCI, &pci)); ut_assertok(acpi_get_name(pci, name)); ut_asserteq_str("PCI0", name); - ut_assertok(uclass_first_device(UCLASS_ROOT, &root)); + ut_assertok(uclass_first_device_err(UCLASS_ROOT, &root)); ut_assertok(acpi_get_name(root, name)); ut_asserteq_str("\\_SB", name); @@ -219,7 +219,7 @@ static int dm_test_acpi_create_dmar(struct unit_test_state *uts) struct acpi_dmar dmar; struct udevice *cpu; - ut_assertok(uclass_first_device(UCLASS_CPU, &cpu)); + ut_assertok(uclass_first_device_err(UCLASS_CPU, &cpu)); ut_assertnonnull(cpu); ut_assertok(acpi_create_dmar(&dmar, DMAR_INTR_REMAP)); ut_asserteq(DMAR_INTR_REMAP, dmar.flags); diff --git a/test/dm/devres.c b/test/dm/devres.c index 524114c833c..3df0f64362d 100644 --- a/test/dm/devres.c +++ b/test/dm/devres.c @@ -165,8 +165,8 @@ static int dm_test_devres_phase(struct unit_test_state *uts) ut_asserteq(TEST_DEVRES_SIZE + TEST_DEVRES_SIZE3, stats.total_size); /* Probing the device should add one allocation */ - ut_assertok(uclass_first_device(UCLASS_TEST_DEVRES, &dev)); - ut_assert(dev != NULL); + ut_assertok(uclass_first_device_err(UCLASS_TEST_DEVRES, &dev)); + ut_assertnonnull(dev); devres_get_stats(dev, &stats); ut_asserteq(3, stats.allocs); ut_asserteq(TEST_DEVRES_SIZE + TEST_DEVRES_SIZE2 + TEST_DEVRES_SIZE3, diff --git a/test/dm/i2c.c b/test/dm/i2c.c index 74b20971956..b46a22e79b1 100644 --- a/test/dm/i2c.c +++ b/test/dm/i2c.c @@ -124,7 +124,7 @@ static int dm_test_i2c_bytewise(struct unit_test_state *uts) ut_asserteq_mem(buf, "\0\0\0\0\0", sizeof(buf)); /* Tell the EEPROM to only read/write one register at a time */ - ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom)); + ut_assertok(uclass_first_device_err(UCLASS_I2C_EMUL, &eeprom)); ut_assertnonnull(eeprom); sandbox_i2c_eeprom_set_test_mode(eeprom, SIE_TEST_MODE_SINGLE_BYTE); @@ -177,7 +177,7 @@ static int dm_test_i2c_offset(struct unit_test_state *uts) /* Do a transfer so we can find the emulator */ ut_assertok(dm_i2c_read(dev, 0, buf, 5)); - ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom)); + ut_assertok(uclass_first_device_err(UCLASS_I2C_EMUL, &eeprom)); /* Offset length 0 */ sandbox_i2c_eeprom_set_offset_len(eeprom, 0); @@ -250,7 +250,7 @@ static int dm_test_i2c_addr_offset(struct unit_test_state *uts) /* Do a transfer so we can find the emulator */ ut_assertok(dm_i2c_read(dev, 0, buf, 5)); - ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom)); + ut_assertok(uclass_first_device_err(UCLASS_I2C_EMUL, &eeprom)); /* Offset length 0 */ sandbox_i2c_eeprom_set_offset_len(eeprom, 0); @@ -315,7 +315,7 @@ static int dm_test_i2c_reg_clrset(struct unit_test_state *uts) /* Do a transfer so we can find the emulator */ ut_assertok(dm_i2c_read(dev, 0, buf, 5)); - ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom)); + ut_assertok(uclass_first_device_err(UCLASS_I2C_EMUL, &eeprom)); /* Dummy data for the test */ ut_assertok(dm_i2c_write(dev, 0, "\xff\x00\xff\x00\x10", 5)); diff --git a/test/dm/virtio_device.c b/test/dm/virtio_device.c index d0195e6bf09..b5c4523a028 100644 --- a/test/dm/virtio_device.c +++ b/test/dm/virtio_device.c @@ -22,7 +22,7 @@ static int dm_test_virtio_base(struct unit_test_state *uts) u8 status; /* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus); /* check the child virtio-rng device is bound */ @@ -60,7 +60,7 @@ static int dm_test_virtio_all_ops(struct unit_test_state *uts) struct virtqueue *vqs[2]; /* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus); /* check the child virtio-rng device is bound */ @@ -102,7 +102,7 @@ static int dm_test_virtio_remove(struct unit_test_state *uts) struct udevice *bus, *dev; /* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus); /* check the child virtio-rng device is bound */ @@ -134,7 +134,7 @@ static int dm_test_virtio_ring(struct unit_test_state *uts) u8 buffer[2][32]; /* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus); /* check the child virtio-blk device is bound */ diff --git a/test/dm/virtio_rng.c b/test/dm/virtio_rng.c index ff5646b4e11..8b9a04b1fde 100644 --- a/test/dm/virtio_rng.c +++ b/test/dm/virtio_rng.c @@ -28,7 +28,7 @@ static int dm_test_virtio_rng_check_len(struct unit_test_state *uts) u8 buffer[16]; /* check probe success */ - ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus)); + ut_assertok(uclass_first_device_err(UCLASS_VIRTIO, &bus)); ut_assertnonnull(bus); /* check the child virtio-rng device is bound */ diff --git a/test/fuzz/cmd_fuzz.c b/test/fuzz/cmd_fuzz.c index 0cc01dc199c..e2f44f3ecb6 100644 --- a/test/fuzz/cmd_fuzz.c +++ b/test/fuzz/cmd_fuzz.c @@ -29,7 +29,7 @@ static struct udevice *find_fuzzing_engine(void) { struct udevice *dev; - if (uclass_first_device(UCLASS_FUZZING_ENGINE, &dev)) + if (uclass_first_device_err(UCLASS_FUZZING_ENGINE, &dev)) return NULL; return dev; diff --git a/test/fuzz/virtio.c b/test/fuzz/virtio.c index e5363d5638e..8a47667e778 100644 --- a/test/fuzz/virtio.c +++ b/test/fuzz/virtio.c @@ -30,7 +30,7 @@ static int fuzz_vring(const uint8_t *data, size_t size) return 0; /* check probe success */ - if (uclass_first_device(UCLASS_VIRTIO, &bus) || !bus) + if (uclass_first_device_err(UCLASS_VIRTIO, &bus)) panic("Could not find virtio bus\n"); /* check the child virtio-rng device is bound */ From 1d0617bd747b59251782aa9f87a23e634cf543ac Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:58:00 +0200 Subject: [PATCH 12/31] dm: treewide: Use uclass_next_device_err when accessing second device There are a couple users of uclass_next_device return value that get the first device by other means and use uclass_next_device assuming the following device in the uclass is related to the first one. Use uclass_next_device_err because the return value from uclass_next_device will be removed in a later patch. Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- arch/arm/mach-k3/j721s2_init.c | 2 +- board/atmel/common/mac_eeprom.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-k3/j721s2_init.c b/arch/arm/mach-k3/j721s2_init.c index 12da8136f9e..dd0c7ba18f0 100644 --- a/arch/arm/mach-k3/j721s2_init.c +++ b/arch/arm/mach-k3/j721s2_init.c @@ -164,7 +164,7 @@ void board_init_f(ulong dummy) if (ret) panic("DRAM 0 init failed: %d\n", ret); - ret = uclass_next_device(&dev); + ret = uclass_next_device_err(&dev); if (ret) panic("DRAM 1 init failed: %d\n", ret); } diff --git a/board/atmel/common/mac_eeprom.c b/board/atmel/common/mac_eeprom.c index a723ba723c9..4606008c697 100644 --- a/board/atmel/common/mac_eeprom.c +++ b/board/atmel/common/mac_eeprom.c @@ -56,7 +56,7 @@ int at91_set_eth1addr(int offset) return ret; /* attempt to obtain a second eeprom device */ - ret = uclass_next_device(&dev); + ret = uclass_next_device_err(&dev); if (ret) return ret; From 9b7474d83b45d2b738866e9a4a46fe2b498a65d1 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:58:01 +0200 Subject: [PATCH 13/31] dm: blk: Do not use uclass_next_device_err blk_first_device_err/blk_next_device_err uses uclass_first_device_err/uclass_next_device_err for device iteration. Although the function names superficially match the return value from uclass_first_device_err/uclass_next_device_err is never used meaningfully, and uclass_first_device/uclass_next_device works equally well for this purpose. In the following patch the semantic of uclass_first_device_err/uclass_next_device_err will be changed to be based on uclass_first_device_check/uclass_next_device_check breaking this sole user that uses uclass_next_device_err for iteration. Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- drivers/block/blk-uclass.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index 7d12d5413f1..bcc14a684be 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -593,11 +593,9 @@ int blk_find_next(enum blk_flag_t flags, struct udevice **devp) int blk_first_device_err(enum blk_flag_t flags, struct udevice **devp) { - int ret; - - for (ret = uclass_first_device_err(UCLASS_BLK, devp); - !ret; - ret = uclass_next_device_err(devp)) { + for (uclass_first_device(UCLASS_BLK, devp); + *devp; + uclass_next_device(devp)) { if (!blk_flags_check(*devp, flags)) return 0; } @@ -607,11 +605,9 @@ int blk_first_device_err(enum blk_flag_t flags, struct udevice **devp) int blk_next_device_err(enum blk_flag_t flags, struct udevice **devp) { - int ret; - - for (ret = uclass_next_device_err(devp); - !ret; - ret = uclass_next_device_err(devp)) { + for (uclass_next_device(devp); + *devp; + uclass_next_device(devp)) { if (!blk_flags_check(*devp, flags)) return 0; } From 0736f7aa3b044bef9dc756902608133843696ed8 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:58:02 +0200 Subject: [PATCH 14/31] net: eth-uclass: Do not set device on error eth_get_dev relies on the broken behavior that returns an error but not the device on which the error happened which gives the caller no reasonable way to report or handle the error. In a later patch uclass_first_device_err will be changed to return the device on error but eth_get_dev stores the returned device pointer directly in a global state without checking the return value. Unset the pointer again in the error case. Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- net/eth-uclass.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 8c3f9cc31b7..f41da4b37b3 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -93,6 +93,8 @@ struct udevice *eth_get_dev(void) if (eth_errno) eth_errno = uclass_first_device_err(UCLASS_ETH, &uc_priv->current); + if (eth_errno) + uc_priv->current = NULL; } return uc_priv->current; } From 7b2aa218c7f642f1b60ea99cbaa62ed1c45f4d21 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:58:04 +0200 Subject: [PATCH 15/31] mpc83xx: gazerbeam: Update sysinfo_get error handling In a later patch sysinfo_get will be changed to return the device in cae of an error. Set sysinfo to NULL on error to preserve previous behavior. Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- board/gdsys/mpc8308/gazerbeam.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/board/gdsys/mpc8308/gazerbeam.c b/board/gdsys/mpc8308/gazerbeam.c index 3d4a7e57fee..ba88401f13d 100644 --- a/board/gdsys/mpc8308/gazerbeam.c +++ b/board/gdsys/mpc8308/gazerbeam.c @@ -49,8 +49,10 @@ int board_early_init_r(void) int mc = 0; int con = 0; - if (sysinfo_get(&sysinfo)) + if (sysinfo_get(&sysinfo)) { puts("Could not find sysinfo information device.\n"); + sysinfo = NULL; + } /* Initialize serdes */ uclass_get_device_by_phandle(UCLASS_MISC, sysinfo, "serdes", &serdes); @@ -92,8 +94,10 @@ int checksysinfo(void) int mc = 0; int con = 0; - if (sysinfo_get(&sysinfo)) + if (sysinfo_get(&sysinfo)) { puts("Could not find sysinfo information device.\n"); + sysinfo = NULL; + } sysinfo_get_int(sysinfo, BOARD_MULTICHANNEL, &mc); sysinfo_get_int(sysinfo, BOARD_VARIANT, &con); @@ -130,8 +134,10 @@ int last_stage_init(void) struct udevice *tpm; int ret; - if (sysinfo_get(&sysinfo)) + if (sysinfo_get(&sysinfo)) { puts("Could not find sysinfo information device.\n"); + sysinfo = NULL; + } if (sysinfo) { int res = sysinfo_get_int(sysinfo, BOARD_HWVERSION, From 801f71194c54c75e90d723b9be3434b6354fce71 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:58:05 +0200 Subject: [PATCH 16/31] dm: core: Switch uclass_foreach_dev_probe to use simple iterator The return value is not used for anythig, and in a later patch the behavior of the _err iterator will change in an incompatible way. Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass Update pvblock_probe() to avoid using internal var: Signed-off-by: Simon Glass --- drivers/xen/pvblock.c | 5 +---- include/dm/uclass.h | 4 ++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/xen/pvblock.c b/drivers/xen/pvblock.c index 970182cd904..95e298d7ddc 100644 --- a/drivers/xen/pvblock.c +++ b/drivers/xen/pvblock.c @@ -852,10 +852,7 @@ static int pvblock_probe(struct udevice *udev) ret = uclass_get(UCLASS_BLK, &uc); if (ret) return ret; - uclass_foreach_dev_probe(UCLASS_BLK, udev) { - if (_ret) - return _ret; - }; + uclass_foreach_dev_probe(UCLASS_BLK, udev); return 0; } diff --git a/include/dm/uclass.h b/include/dm/uclass.h index f6c0110b061..990e9c02d1e 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -491,7 +491,7 @@ int uclass_id_count(enum uclass_id id); * are no more devices. */ #define uclass_foreach_dev_probe(id, dev) \ - for (int _ret = uclass_first_device_err(id, &dev); !_ret && dev; \ - _ret = uclass_next_device_err(&dev)) + for (uclass_first_device(id, &dev); dev; \ + uclass_next_device(&dev)) #endif From e44d7e73fe0d649693d8d0a110cd7632bc919273 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 12 Oct 2022 21:58:06 +0200 Subject: [PATCH 17/31] dm: core: Switch uclass_*_device_err to use uclass_*_device_check The _err variant iterators use the simple iterators without suffix as basis. However, there is no user that uclass_next_device_err for iteration, many users of uclass_first_device_err use it to get the first and (assumed) only device of an uclass, and a couple that use uclass_next_device_err to get the device following a known device in the uclass list. While there are some truly singleton device classes in which more than one device cannot exist these are quite rare, and most classes can have multiple devices even if it is not the case on the SoC's EVB. In a later patch the simple iterators will be updated to not stop on error and return next device instead. With this in many cases the code that expects the first device or an error if it fails to probe may get the next device instead. Use the _check iterators as the basis of _err iterators to preserve the old behavior. Signed-off-by: Michal Suchanek Reviewed-by: Simon Glass --- drivers/core/uclass.c | 28 ++++++++++++++-------------- include/dm/uclass.h | 22 +++++++++++----------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index a591e224032..b7d11bdd23a 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -586,19 +586,6 @@ int uclass_first_device(enum uclass_id id, struct udevice **devp) return uclass_get_device_tail(dev, ret, devp); } -int uclass_first_device_err(enum uclass_id id, struct udevice **devp) -{ - int ret; - - ret = uclass_first_device(id, devp); - if (ret) - return ret; - else if (!*devp) - return -ENODEV; - - return 0; -} - int uclass_next_device(struct udevice **devp) { struct udevice *dev = *devp; @@ -611,11 +598,24 @@ int uclass_next_device(struct udevice **devp) return uclass_get_device_tail(dev, ret, devp); } +int uclass_first_device_err(enum uclass_id id, struct udevice **devp) +{ + int ret; + + ret = uclass_first_device_check(id, devp); + if (ret) + return ret; + else if (!*devp) + return -ENODEV; + + return 0; +} + int uclass_next_device_err(struct udevice **devp) { int ret; - ret = uclass_next_device(devp); + ret = uclass_next_device_check(devp); if (ret) return ret; else if (!*devp) diff --git a/include/dm/uclass.h b/include/dm/uclass.h index 990e9c02d1e..823a16527f7 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -332,17 +332,6 @@ int uclass_get_device_by_driver(enum uclass_id id, const struct driver *drv, */ int uclass_first_device(enum uclass_id id, struct udevice **devp); -/** - * uclass_first_device_err() - Get the first device in a uclass - * - * The device returned is probed if necessary, and ready for use - * - * @id: Uclass ID to look up - * @devp: Returns pointer to the first device in that uclass, or NULL if none - * Return: 0 if found, -ENODEV if not found, other -ve on error - */ -int uclass_first_device_err(enum uclass_id id, struct udevice **devp); - /** * uclass_next_device() - Get the next device in a uclass * @@ -358,6 +347,17 @@ int uclass_first_device_err(enum uclass_id id, struct udevice **devp); */ int uclass_next_device(struct udevice **devp); +/** + * uclass_first_device_err() - Get the first device in a uclass + * + * The device returned is probed if necessary, and ready for use + * + * @id: Uclass ID to look up + * @devp: Returns pointer to the first device in that uclass, or NULL if none + * Return: 0 if found, -ENODEV if not found, other -ve on error + */ +int uclass_first_device_err(enum uclass_id id, struct udevice **devp); + /** * uclass_next_device_err() - Get the next device in a uclass * From 13819f07ea6c60e87b708755a53954b8c0c99a32 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 11 Oct 2022 09:47:07 -0600 Subject: [PATCH 18/31] bootm: Change incorrect 'unsupported' error At present when bootm fails, it says: subcommand not supported and then prints help for the bootm command. This is not very useful, since generally the error is related to something else, such as fixups failing. It is quite confusing to see this in a test run. Change the error and show the error code. We could update the OS functions to return -ENOSYS when they do not support the bootm subcommand. But this involves some thought since this is arch-specific code and proper errno error codes are not always returned. Also, with the code as is, all required subcommands are of course supported - a problem would only come if someone added a new one or removed support for one from an existing OS. Therefore it seems better to leave that sort of effort for when our bootm tests are improved. Note: v1 of this patch generated a discussion[1] about printing error strings automatically using printf(). That is outside the scope of this patch but will be dealt with separately. [1] https://patchwork.ozlabs.org/project/uboot/patch/20220909151801.336551-3-sjg@chromium.org/ Signed-off-by: Simon Glass --- boot/bootm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boot/bootm.c b/boot/bootm.c index 5b20b418dba..a4c0870c0fe 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -790,7 +790,7 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc, /* Check for unsupported subcommand. */ if (ret) { - puts("subcommand not supported\n"); + printf("subcommand failed (err=%d)\n", ret); return ret; } From f06443f9d5dfc616b05d3526c258044bfce77419 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 11 Oct 2022 09:47:08 -0600 Subject: [PATCH 19/31] bootm: Avoid returning error codes from command Functions which implement commands must return a CMD_RET_... error code. At present bootm can return a negative errno value in some cases, thus causing strange behaviour such as trying to exit the shell and printing usage information. Fix this by returning the correct value. Signed-off-by: Simon Glass --- cmd/bootm.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cmd/bootm.c b/cmd/bootm.c index d764a27002d..f09b41c2c16 100644 --- a/cmd/bootm.c +++ b/cmd/bootm.c @@ -111,7 +111,7 @@ static int do_bootm_subcommand(struct cmd_tbl *cmdtp, int flag, int argc, bootm_get_addr(argc, argv) + image_load_offset); #endif - return ret; + return ret ? CMD_RET_FAILURE : 0; } /*******************************************************************/ @@ -120,6 +120,8 @@ static int do_bootm_subcommand(struct cmd_tbl *cmdtp, int flag, int argc, int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { + int ret; + #ifdef CONFIG_NEEDS_MANUAL_RELOC static int relocated = 0; @@ -152,7 +154,7 @@ int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return do_bootm_subcommand(cmdtp, flag, argc, argv); } - return do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START | + ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START | BOOTM_STATE_FINDOS | BOOTM_STATE_PRE_LOAD | BOOTM_STATE_FINDOTHER | BOOTM_STATE_LOADOS | #ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH @@ -163,6 +165,8 @@ int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) #endif BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO, &images, 1); + + return ret ? CMD_RET_FAILURE : 0; } int bootm_maybe_autostart(struct cmd_tbl *cmdtp, const char *cmd) From 2c0b61d562d3d5db46cc72cd4ae66fb13b3185cf Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 11 Oct 2022 09:47:09 -0600 Subject: [PATCH 20/31] bootm: Drop #ifdef from do_bootm() Drop the #ifdefs from this command by using a variable to hold the states that should be executed. Signed-off-by: Simon Glass --- cmd/bootm.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/cmd/bootm.c b/cmd/bootm.c index f09b41c2c16..37c2af96e08 100644 --- a/cmd/bootm.c +++ b/cmd/bootm.c @@ -120,6 +120,7 @@ static int do_bootm_subcommand(struct cmd_tbl *cmdtp, int flag, int argc, int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { + int states; int ret; #ifdef CONFIG_NEEDS_MANUAL_RELOC @@ -154,17 +155,15 @@ int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return do_bootm_subcommand(cmdtp, flag, argc, argv); } - ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START | - BOOTM_STATE_FINDOS | BOOTM_STATE_PRE_LOAD | BOOTM_STATE_FINDOTHER | - BOOTM_STATE_LOADOS | -#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH - BOOTM_STATE_RAMDISK | -#endif -#if defined(CONFIG_PPC) || defined(CONFIG_MIPS) - BOOTM_STATE_OS_CMDLINE | -#endif + states = BOOTM_STATE_START | BOOTM_STATE_FINDOS | BOOTM_STATE_PRE_LOAD | + BOOTM_STATE_FINDOTHER | BOOTM_STATE_LOADOS | BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO | - BOOTM_STATE_OS_GO, &images, 1); + BOOTM_STATE_OS_GO; + if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH)) + states |= BOOTM_STATE_RAMDISK; + if (IS_ENABLED(CONFIG_PPC) || IS_ENABLED(CONFIG_MIPS)) + states |= BOOTM_STATE_OS_CMDLINE; + ret = do_bootm_states(cmdtp, flag, argc, argv, states, &images, 1); return ret ? CMD_RET_FAILURE : 0; } From 19511935df44f0fec01c6538176f1b6ea4dd63d3 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 11 Oct 2022 09:47:10 -0600 Subject: [PATCH 21/31] boot: Correct handling of addresses in boot_relocate_fdt() This code uses casts between addresses and pointers, so does not work with sandbox. Update it so we can allow sandbox to do device tree fixups. Signed-off-by: Simon Glass --- boot/image-fdt.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/boot/image-fdt.c b/boot/image-fdt.c index 884e089f2d8..f651940d9b4 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -186,24 +186,25 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong *of_size) /* If fdt_high is set use it to select the relocation address */ fdt_high = env_get("fdt_high"); if (fdt_high) { - void *desired_addr = (void *)hextoul(fdt_high, NULL); + ulong desired_addr = hextoul(fdt_high, NULL); + ulong addr; - if (((ulong) desired_addr) == ~0UL) { + if (desired_addr == ~0UL) { /* All ones means use fdt in place */ of_start = fdt_blob; - lmb_reserve(lmb, (ulong)of_start, of_len); + lmb_reserve(lmb, map_to_sysmem(of_start), of_len); disable_relocation = 1; } else if (desired_addr) { - of_start = - (void *)(ulong) lmb_alloc_base(lmb, of_len, 0x1000, - (ulong)desired_addr); + addr = lmb_alloc_base(lmb, of_len, 0x1000, + desired_addr); + of_start = map_sysmem(addr, of_len); if (of_start == NULL) { puts("Failed using fdt_high value for Device Tree"); goto error; } } else { - of_start = - (void *)(ulong) lmb_alloc(lmb, of_len, 0x1000); + addr = lmb_alloc(lmb, of_len, 0x1000); + of_start = map_sysmem(addr, of_len); } } else { mapsize = env_get_bootm_mapsize(); @@ -224,9 +225,8 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong *of_size) * At least part of this DRAM bank is usable, try * using it for LMB allocation. */ - of_start = - (void *)(ulong) lmb_alloc_base(lmb, of_len, 0x1000, - start + usable); + of_start = map_sysmem((ulong)lmb_alloc_base(lmb, + of_len, 0x1000, start + usable), of_len); /* Allocation succeeded, use this block. */ if (of_start != NULL) break; From f337fb9ea8b8163e2b5cff7e2691a30d23841f3e Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 11 Oct 2022 09:47:11 -0600 Subject: [PATCH 22/31] fs: Quieten down the filesystems more When looking for a filesystem on a partition we should do so quietly. At present if the filesystem is very small (e.g. 512 bytes) we get a host of messages. Update these to only show when debugging. Signed-off-by: Simon Glass --- disk/part_efi.c | 15 +++++++-------- fs/btrfs/disk-io.c | 7 ++++--- fs/ext4/ext4_common.c | 2 +- fs/fs_internal.c | 3 +-- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/disk/part_efi.c b/disk/part_efi.c index ad94504ed90..26738a57d5d 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -264,20 +264,19 @@ int part_get_info_efi(struct blk_desc *dev_desc, int part, /* "part" argument must be at least 1 */ if (part < 1) { - printf("%s: Invalid Argument(s)\n", __func__); - return -1; + log_debug("Invalid Argument(s)\n"); + return -EINVAL; } /* This function validates AND fills in the GPT header and PTE */ if (find_valid_gpt(dev_desc, gpt_head, &gpt_pte) != 1) - return -1; + return -EINVAL; if (part > le32_to_cpu(gpt_head->num_partition_entries) || !is_pte_valid(&gpt_pte[part - 1])) { - debug("%s: *** ERROR: Invalid partition number %d ***\n", - __func__, part); + log_debug("*** ERROR: Invalid partition number %d ***\n", part); free(gpt_pte); - return -1; + return -EPERM; } /* The 'lbaint_t' casting may limit the maximum disk size to 2 TB */ @@ -300,8 +299,8 @@ int part_get_info_efi(struct blk_desc *dev_desc, int part, info->type_guid, UUID_STR_FORMAT_GUID); #endif - debug("%s: start 0x" LBAF ", size 0x" LBAF ", name %s\n", __func__, - info->start, info->size, info->name); + log_debug("start 0x" LBAF ", size 0x" LBAF ", name %s\n", info->start, + info->size, info->name); /* Remember to free pte */ free(gpt_pte); diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index c80f8e80283..3f0d9f1c113 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0+ #include #include +#include #include #include #include "kernel-shared/btrfs_tree.h" @@ -910,9 +911,9 @@ static int btrfs_scan_fs_devices(struct blk_desc *desc, if (round_up(BTRFS_SUPER_INFO_SIZE + BTRFS_SUPER_INFO_OFFSET, desc->blksz) > (part->size << desc->log2blksz)) { - error("superblock end %u is larger than device size " LBAFU, - BTRFS_SUPER_INFO_SIZE + BTRFS_SUPER_INFO_OFFSET, - part->size << desc->log2blksz); + log_debug("superblock end %u is larger than device size " LBAFU, + BTRFS_SUPER_INFO_SIZE + BTRFS_SUPER_INFO_OFFSET, + part->size << desc->log2blksz); return -EINVAL; } diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index d49ba4a9954..1185cb2c046 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -2415,7 +2415,7 @@ int ext4fs_mount(unsigned part_length) return 1; fail: - printf("Failed to mount ext2 filesystem...\n"); + log_debug("Failed to mount ext2 filesystem...\n"); fail_noerr: free(data); ext4fs_root = NULL; diff --git a/fs/fs_internal.c b/fs/fs_internal.c index ae1cb8584c7..111f91b355d 100644 --- a/fs/fs_internal.c +++ b/fs/fs_internal.c @@ -29,8 +29,7 @@ int fs_devread(struct blk_desc *blk, struct disk_partition *partition, /* Check partition boundaries */ if ((sector + ((byte_offset + byte_len - 1) >> log2blksz)) >= partition->size) { - log_err("%s read outside partition " LBAFU "\n", __func__, - sector); + log_debug("read outside partition " LBAFU "\n", sector); return 0; } From baf4141079aa18e5f40fcecaa9dabaeaa9437bee Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 11 Oct 2022 09:47:12 -0600 Subject: [PATCH 23/31] fdt: Show a message when the working FDT changes The working FDT is the one which comes from the OS and is fixed up by U-Boot. When the bootm command runs, it sets up the working FDT to be the one it is about to pass to the OS, so that fixups can happen. This seems like an important step, so add a message indicating that the working FDT has changed. This is shown during the running of the bootm command. Signed-off-by: Simon Glass --- cmd/fdt.c | 1 + doc/usage/cmd/fdt.rst | 1 + test/cmd/fdt.c | 11 ++++++++++- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/cmd/fdt.c b/cmd/fdt.c index 6fbd9205d38..4b2dcfec863 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -40,6 +40,7 @@ void set_working_fdt_addr(ulong addr) { void *buf; + printf("Working FDT set to %lx\n", addr); buf = map_sysmem(addr, 0); working_fdt = buf; env_set_hex("fdtaddr", addr); diff --git a/doc/usage/cmd/fdt.rst b/doc/usage/cmd/fdt.rst index 07fed732e45..36b8230877c 100644 --- a/doc/usage/cmd/fdt.rst +++ b/doc/usage/cmd/fdt.rst @@ -60,6 +60,7 @@ The second word shows the size of the FDT. Now set the working FDT to that address and expand it to 0xf000 in size:: => fdt addr 10000 f000 + Working FDT set to 10000 => md 10000 4 00010000: edfe0dd0 00f00000 78000000 7c270000 ...........x..'| diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c index 100a7ef5ebf..ba9eaa42c14 100644 --- a/test/cmd/fdt.c +++ b/test/cmd/fdt.c @@ -55,6 +55,7 @@ static int fdt_test_addr(struct unit_test_state *uts) /* The working fdt is not set, so this should fail */ set_working_fdt_addr(0); + ut_assert_nextline("Working FDT set to 0"); ut_asserteq(CMD_RET_FAILURE, run_command("fdt addr", 0)); ut_assert_nextline("libfdt fdt_check_header(): FDT_ERR_BADMAGIC"); ut_assertok(ut_check_console_end(uts)); @@ -63,18 +64,22 @@ static int fdt_test_addr(struct unit_test_state *uts) ut_assertok(make_test_fdt(uts, fdt, sizeof(fdt))); addr = map_to_sysmem(fdt); set_working_fdt_addr(addr); + ut_assert_nextline("Working FDT set to %lx", addr); ut_assertok(run_command("fdt addr", 0)); ut_assert_nextline("Working fdt: %08lx", (ulong)map_to_sysmem(fdt)); ut_assertok(ut_check_console_end(uts)); /* Set the working FDT */ set_working_fdt_addr(0); + ut_assert_nextline("Working FDT set to 0"); ut_assertok(run_commandf("fdt addr %08x", addr)); + ut_assert_nextline("Working FDT set to %lx", addr); ut_asserteq(addr, map_to_sysmem(working_fdt)); ut_assertok(ut_check_console_end(uts)); set_working_fdt_addr(0); + ut_assert_nextline("Working FDT set to 0"); - /* Set the working FDT */ + /* Set the control FDT */ fdt_blob = gd->fdt_blob; gd->fdt_blob = NULL; ret = run_commandf("fdt addr -c %08x", addr); @@ -93,6 +98,7 @@ static int fdt_test_addr(struct unit_test_state *uts) /* Test detecting an invalid FDT */ fdt[0] = 123; set_working_fdt_addr(addr); + ut_assert_nextline("Working FDT set to %lx", addr); ut_asserteq(1, run_commandf("fdt addr")); ut_assert_nextline("libfdt fdt_check_header(): FDT_ERR_BADMAGIC"); ut_assertok(ut_check_console_end(uts)); @@ -115,16 +121,19 @@ static int fdt_test_resize(struct unit_test_state *uts) /* Test setting and resizing the working FDT to a larger size */ ut_assertok(console_record_reset_enable()); ut_assertok(run_commandf("fdt addr %08x %x", addr, newsize)); + ut_assert_nextline("Working FDT set to %lx", addr); ut_assertok(ut_check_console_end(uts)); /* Try shrinking it */ ut_assertok(run_commandf("fdt addr %08x %x", addr, sizeof(fdt) / 4)); + ut_assert_nextline("Working FDT set to %lx", addr); ut_assert_nextline("New length %d < existing length %d, ignoring", (int)sizeof(fdt) / 4, newsize); ut_assertok(ut_check_console_end(uts)); /* ...quietly */ ut_assertok(run_commandf("fdt addr -q %08x %x", addr, sizeof(fdt) / 4)); + ut_assert_nextline("Working FDT set to %lx", addr); ut_assertok(ut_check_console_end(uts)); /* We cannot easily provoke errors in fdt_open_into(), so ignore that */ From bfdfc5d85398fba50e5e2b1e571df53f189f565a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 11 Oct 2022 09:47:13 -0600 Subject: [PATCH 24/31] bootstd: Move VBE setup into a shared function This information needs to be set up by the bootstd tests as well. Move it into a common function and ensure it is executed before any bootstd test is run. Make sure the 'images' parameter is set correctly for fixups. Signed-off-by: Simon Glass --- test/boot/bootstd_common.c | 48 ++++++++++++++++++++++++++++++++++++++ test/boot/bootstd_common.h | 16 +++++++++++++ test/boot/vbe_simple.c | 34 ++++----------------------- 3 files changed, 68 insertions(+), 30 deletions(-) diff --git a/test/boot/bootstd_common.c b/test/boot/bootstd_common.c index 05347d87106..59d46bef0c5 100644 --- a/test/boot/bootstd_common.c +++ b/test/boot/bootstd_common.c @@ -9,10 +9,46 @@ #include #include #include +#include +#include +#include #include #include +#include #include "bootstd_common.h" +bool vbe_setup_done; + +/* set up MMC for VBE tests */ +int bootstd_setup_for_tests(void) +{ + ALLOC_CACHE_ALIGN_BUFFER(u8, buf, MMC_MAX_BLOCK_LEN); + struct udevice *mmc; + struct blk_desc *desc; + int ret; + + /* Set up the version string */ + ret = uclass_get_device(UCLASS_MMC, 1, &mmc); + if (ret) + return log_msg_ret("mmc", -EIO); + desc = blk_get_by_device(mmc); + + memset(buf, '\0', MMC_MAX_BLOCK_LEN); + strcpy(buf, TEST_VERSION); + if (blk_dwrite(desc, VERSION_START_BLK, 1, buf) != 1) + return log_msg_ret("wr1", -EIO); + + /* Set up the nvdata */ + memset(buf, '\0', MMC_MAX_BLOCK_LEN); + buf[1] = ilog2(0x40) << 4 | 1; + *(u32 *)(buf + 4) = TEST_VERNUM; + buf[0] = crc8(0, buf + 1, 0x3f); + if (blk_dwrite(desc, NVDATA_START_BLK, 1, buf) != 1) + return log_msg_ret("wr2", -EIO); + + return 0; +} + int bootstd_test_drop_bootdev_order(struct unit_test_state *uts) { struct bootstd_priv *priv; @@ -30,6 +66,18 @@ int do_ut_bootstd(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) struct unit_test *tests = UNIT_TEST_SUITE_START(bootstd_test); const int n_ents = UNIT_TEST_SUITE_COUNT(bootstd_test); + if (!vbe_setup_done) { + int ret; + + ret = bootstd_setup_for_tests(); + if (ret) { + printf("Failed to set up for bootstd tests (err=%d)\n", + ret); + return CMD_RET_FAILURE; + } + vbe_setup_done = true; + } + return cmd_ut_category("bootstd", "bootstd_test_", tests, n_ents, argc, argv); } diff --git a/test/boot/bootstd_common.h b/test/boot/bootstd_common.h index 676ef0a57f9..c5e0fd1ceab 100644 --- a/test/boot/bootstd_common.h +++ b/test/boot/bootstd_common.h @@ -9,10 +9,17 @@ #ifndef __bootstd_common_h #define __bootstd_common_h +#include + /* Declare a new bootdev test */ #define BOOTSTD_TEST(_name, _flags) \ UNIT_TEST(_name, _flags, bootstd_test) +#define NVDATA_START_BLK ((0x400 + 0x400) / MMC_MAX_BLOCK_LEN) +#define VERSION_START_BLK ((0x400 + 0x800) / MMC_MAX_BLOCK_LEN) +#define TEST_VERSION "U-Boot v2022.04-local2" +#define TEST_VERNUM 0x00010002 + struct unit_test_state; /** @@ -24,4 +31,13 @@ struct unit_test_state; */ int bootstd_test_drop_bootdev_order(struct unit_test_state *uts); +/** + * bootstd_setup_for_tests() - Set up MMC data for VBE tests + * + * Some data is needed for VBE tests to work. This function sets that up. + * + * @return 0 if OK, -ve on error + */ +int bootstd_setup_for_tests(void); + #endif diff --git a/test/boot/vbe_simple.c b/test/boot/vbe_simple.c index 8acd777f4cd..faba9e8f90b 100644 --- a/test/boot/vbe_simple.c +++ b/test/boot/vbe_simple.c @@ -10,54 +10,27 @@ #include #include #include -#include -#include #include #include -#include -#include #include #include -#include #include "bootstd_common.h" -#define NVDATA_START_BLK ((0x400 + 0x400) / MMC_MAX_BLOCK_LEN) -#define VERSION_START_BLK ((0x400 + 0x800) / MMC_MAX_BLOCK_LEN) -#define TEST_VERSION "U-Boot v2022.04-local2" -#define TEST_VERNUM 0x00010002 - /* Basic test of reading nvdata and updating a fwupd node in the device tree */ static int vbe_simple_test_base(struct unit_test_state *uts) { - ALLOC_CACHE_ALIGN_BUFFER(u8, buf, MMC_MAX_BLOCK_LEN); const char *version, *bl_version; struct event_ft_fixup fixup; - struct udevice *dev, *mmc; + struct udevice *dev; struct device_node *np; - struct blk_desc *desc; char fdt_buf[0x400]; char info[100]; int node_ofs; ofnode node; u32 vernum; - /* Set up the version string */ - ut_assertok(uclass_get_device(UCLASS_MMC, 1, &mmc)); - desc = blk_get_by_device(mmc); - ut_assertnonnull(desc); - - memset(buf, '\0', MMC_MAX_BLOCK_LEN); - strcpy(buf, TEST_VERSION); - if (blk_dwrite(desc, VERSION_START_BLK, 1, buf) != 1) - return log_msg_ret("write", -EIO); - - /* Set up the nvdata */ - memset(buf, '\0', MMC_MAX_BLOCK_LEN); - buf[1] = ilog2(0x40) << 4 | 1; - *(u32 *)(buf + 4) = TEST_VERNUM; - buf[0] = crc8(0, buf + 1, 0x3f); - if (blk_dwrite(desc, NVDATA_START_BLK, 1, buf) != 1) - return log_msg_ret("write", -EIO); + /* Set up the VBE info */ + ut_assertok(bootstd_setup_for_tests()); /* Read the version back */ ut_assertok(vbe_find_by_any("firmware0", &dev)); @@ -90,6 +63,7 @@ static int vbe_simple_test_base(struct unit_test_state *uts) * * Two fix this we need image_setup_libfdt() is updated to use ofnode */ + fixup.images = NULL; ut_assertok(event_notify(EVT_FT_FIXUP, &fixup, sizeof(fixup))); node = oftree_path(fixup.tree, "/chosen/fwupd/firmware0"); From 0718c3154c428b694bff94fab178bc2c43ce3ff9 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 11 Oct 2022 09:47:14 -0600 Subject: [PATCH 25/31] sandbox: Support FDT fixups Add support for doing device tree fixups in sandbox. This allows us to test that functionality in CI. Signed-off-by: Simon Glass --- arch/sandbox/lib/bootm.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/arch/sandbox/lib/bootm.c b/arch/sandbox/lib/bootm.c index c1742f94de7..28f4a746fb6 100644 --- a/arch/sandbox/lib/bootm.c +++ b/arch/sandbox/lib/bootm.c @@ -50,8 +50,25 @@ int bootz_setup(ulong image, ulong *start, ulong *end) return ret; } +/* Subcommand: PREP */ +static int boot_prep_linux(struct bootm_headers *images) +{ + int ret; + + if (CONFIG_IS_ENABLED(LMB)) { + ret = image_setup_linux(images); + if (ret) + return ret; + } + + return 0; +} + int do_bootm_linux(int flag, int argc, char *argv[], struct bootm_headers *images) { + if (flag & BOOTM_STATE_OS_PREP) + return boot_prep_linux(images); + if (flag & (BOOTM_STATE_OS_GO | BOOTM_STATE_OS_FAKE_GO)) { bootstage_mark(BOOTSTAGE_ID_RUN_OS); printf("## Transferring control to Linux (at address %08lx)...\n", From 33ba72700b586babfad7f5ef1e408b6b399fe0ec Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 11 Oct 2022 09:47:15 -0600 Subject: [PATCH 26/31] boot: Pass the correct FDT to the EVT_FT_FIXUP event Now that we support multiple device trees with the ofnode interface, we can pass the correct FDT to this event. This allows the 'working' FDT to be fixed up, as expected, so long as OFNODE_MULTI_TREE is enabled. Also make sure we don't try to do this with livetree, which does not support fixups yet. Signed-off-by: Simon Glass --- boot/image-fdt.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/boot/image-fdt.c b/boot/image-fdt.c index f651940d9b4..b830a0ab418 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -665,15 +665,18 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, goto err; } } - if (CONFIG_IS_ENABLED(EVENT)) { + if (!of_live_active() && CONFIG_IS_ENABLED(EVENT)) { struct event_ft_fixup fixup; - fixup.tree = oftree_default(); + fixup.tree = oftree_from_fdt(blob); fixup.images = images; - ret = event_notify(EVT_FT_FIXUP, &fixup, sizeof(fixup)); - if (ret) { - printf("ERROR: fdt fixup event failed: %d\n", ret); - goto err; + if (oftree_valid(fixup.tree)) { + ret = event_notify(EVT_FT_FIXUP, &fixup, sizeof(fixup)); + if (ret) { + printf("ERROR: fdt fixup event failed: %d\n", + ret); + goto err; + } } } From 87c97cb1f9a45567488edd260539467b7b1a869a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 11 Oct 2022 09:47:16 -0600 Subject: [PATCH 27/31] boot: Tidy up logging and naming in vbe_simple Make sure the log_msg_ret() values are unique so that the log trace is unambiguous with LOG_ERROR_RETURN. Also avoid reusing the 'node' variable for two different nodes in bootmeth_vbe_simple_ft_fixup(), since this is confusing. Signed-off-by: Simon Glass --- boot/vbe_simple.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/boot/vbe_simple.c b/boot/vbe_simple.c index 61b6322ebe2..076b650c25a 100644 --- a/boot/vbe_simple.c +++ b/boot/vbe_simple.c @@ -6,6 +6,8 @@ * Written by Simon Glass */ +#define LOG_CATEGORY LOGC_BOOT + #include #include #include @@ -199,17 +201,17 @@ int vbe_simple_fixup_node(ofnode node, struct simple_state *state) version = strdup(state->fw_version); if (!version) - return log_msg_ret("ver", -ENOMEM); + 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("ver", ret); + return log_msg_ret("num", ret); ret = ofnode_write_string(node, "bootloader-version", version_string); if (ret) - return log_msg_ret("fix", ret); + return log_msg_ret("bl", ret); return 0; } @@ -233,7 +235,7 @@ 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; + ofnode node, subnode; int ret; if (strcmp("vbe_simple", dev->driver->name)) @@ -243,8 +245,8 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct event *event) node = oftree_path(tree, "/chosen/fwupd"); if (!ofnode_valid(node)) continue; - node = ofnode_find_subnode(node, dev->name); - if (!ofnode_valid(node)) + subnode = ofnode_find_subnode(node, dev->name); + if (!ofnode_valid(subnode)) continue; log_debug("Fixing up: %s\n", dev->name); @@ -255,7 +257,7 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct event *event) if (ret) return log_msg_ret("read", ret); - ret = vbe_simple_fixup_node(node, &state); + ret = vbe_simple_fixup_node(subnode, &state); if (ret) return log_msg_ret("fix", ret); } From a753190a0c544d211faf13a296bd80e830150158 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 11 Oct 2022 09:47:17 -0600 Subject: [PATCH 28/31] test: Move common FIT code into a separate fit_util file To avoid duplicating code, create a new fit_util module which provides various utility functions for FIT. Move this code out from the existing test_fit.py and refactor it with addition parameters. Fix up pylint warnings in the conversion. This involves no functional change. Signed-off-by: Simon Glass --- test/boot/bootstd_common.c | 21 ++++----- test/py/tests/fit_util.py | 90 ++++++++++++++++++++++++++++++++++++++ test/py/tests/test_fit.py | 79 ++++----------------------------- 3 files changed, 110 insertions(+), 80 deletions(-) create mode 100644 test/py/tests/fit_util.py diff --git a/test/boot/bootstd_common.c b/test/boot/bootstd_common.c index 59d46bef0c5..7a40836507a 100644 --- a/test/boot/bootstd_common.c +++ b/test/boot/bootstd_common.c @@ -17,6 +17,7 @@ #include #include "bootstd_common.h" +/* tracks whether bootstd_setup_for_tests() has been run yet */ bool vbe_setup_done; /* set up MMC for VBE tests */ @@ -27,6 +28,9 @@ int bootstd_setup_for_tests(void) struct blk_desc *desc; int ret; + if (vbe_setup_done) + return 0; + /* Set up the version string */ ret = uclass_get_device(UCLASS_MMC, 1, &mmc); if (ret) @@ -46,6 +50,8 @@ int bootstd_setup_for_tests(void) if (blk_dwrite(desc, NVDATA_START_BLK, 1, buf) != 1) return log_msg_ret("wr2", -EIO); + vbe_setup_done = true; + return 0; } @@ -65,17 +71,12 @@ int do_ut_bootstd(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct unit_test *tests = UNIT_TEST_SUITE_START(bootstd_test); const int n_ents = UNIT_TEST_SUITE_COUNT(bootstd_test); + int ret; - if (!vbe_setup_done) { - int ret; - - ret = bootstd_setup_for_tests(); - if (ret) { - printf("Failed to set up for bootstd tests (err=%d)\n", - ret); - return CMD_RET_FAILURE; - } - vbe_setup_done = true; + ret = bootstd_setup_for_tests(); + if (ret) { + printf("Failed to set up for bootstd tests (err=%d)\n", ret); + return CMD_RET_FAILURE; } return cmd_ut_category("bootstd", "bootstd_test_", diff --git a/test/py/tests/fit_util.py b/test/py/tests/fit_util.py new file mode 100644 index 00000000000..fcec4c43c3c --- /dev/null +++ b/test/py/tests/fit_util.py @@ -0,0 +1,90 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC + +"""Common utility functions for FIT tests""" + +import os + +import u_boot_utils as util + +def make_fname(cons, basename): + """Make a temporary filename + + Args: + cons (ConsoleBase): u_boot_console to use + basename (str): Base name of file to create (within temporary directory) + Return: + Temporary filename + """ + + return os.path.join(cons.config.build_dir, basename) + +def make_its(cons, base_its, params, basename='test.its'): + """Make a sample .its file with parameters embedded + + Args: + cons (ConsoleBase): u_boot_console to use + base_its (str): Template text for the .its file, typically containing + %() references + params (dict of str): Parameters to embed in the %() strings + basename (str): base name to write to (will be placed in the temp dir) + Returns: + str: Filename of .its file created + """ + its = make_fname(cons, basename) + with open(its, 'w', encoding='utf-8') as outf: + print(base_its % params, file=outf) + return its + +def make_fit(cons, mkimage, base_its, params, basename='test.fit'): + """Make a sample .fit file ready for loading + + This creates a .its script with the selected parameters and uses mkimage to + turn this into a .fit image. + + Args: + cons (ConsoleBase): u_boot_console to use + mkimage (str): Filename of 'mkimage' utility + base_its (str): Template text for the .its file, typically containing + %() references + params (dict of str): Parameters to embed in the %() strings + basename (str): base name to write to (will be placed in the temp dir) + Return: + Filename of .fit file created + """ + fit = make_fname(cons, basename) + its = make_its(cons, base_its, params) + util.run_and_log(cons, [mkimage, '-f', its, fit]) + return fit + +def make_kernel(cons, basename, text): + """Make a sample kernel with test data + + Args: + cons (ConsoleBase): u_boot_console to use + basename (str): base name to write to (will be placed in the temp dir) + text (str): Contents of the kernel file (will be repeated 100 times) + Returns: + str: Full path and filename of the kernel it created + """ + fname = make_fname(cons, basename) + data = '' + for i in range(100): + data += f'this {text} {i} is unlikely to boot\n' + with open(fname, 'w', encoding='utf-8') as outf: + print(data, file=outf) + return fname + +def make_dtb(cons, base_fdt, basename): + """Make a sample .dts file and compile it to a .dtb + + Returns: + cons (ConsoleBase): u_boot_console to use + Filename of .dtb file created + """ + src = make_fname(cons, f'{basename}.dts') + dtb = make_fname(cons, f'{basename}.dtb') + with open(src, 'w', encoding='utf-8') as outf: + outf.write(base_fdt) + util.run_and_log(cons, ['dtc', src, '-O', 'dtb', '-o', dtb]) + return dtb diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py index 5856960be23..f45848484eb 100755 --- a/test/py/tests/test_fit.py +++ b/test/py/tests/test_fit.py @@ -7,6 +7,7 @@ import os import pytest import struct import u_boot_utils as util +import fit_util # Define a base ITS which we can adjust using % and a dictionary base_its = ''' @@ -126,7 +127,6 @@ def test_fit(u_boot_console): Return: Temporary filename """ - return os.path.join(cons.config.build_dir, leaf) def filesize(fname): @@ -150,67 +150,6 @@ def test_fit(u_boot_console): with open(fname, 'rb') as fd: return fd.read() - def make_dtb(): - """Make a sample .dts file and compile it to a .dtb - - Returns: - Filename of .dtb file created - """ - src = make_fname('u-boot.dts') - dtb = make_fname('u-boot.dtb') - with open(src, 'w') as fd: - fd.write(base_fdt) - util.run_and_log(cons, ['dtc', src, '-O', 'dtb', '-o', dtb]) - return dtb - - def make_its(params): - """Make a sample .its file with parameters embedded - - Args: - params: Dictionary containing parameters to embed in the %() strings - Returns: - Filename of .its file created - """ - its = make_fname('test.its') - with open(its, 'w') as fd: - print(base_its % params, file=fd) - return its - - def make_fit(mkimage, params): - """Make a sample .fit file ready for loading - - This creates a .its script with the selected parameters and uses mkimage to - turn this into a .fit image. - - Args: - mkimage: Filename of 'mkimage' utility - params: Dictionary containing parameters to embed in the %() strings - Return: - Filename of .fit file created - """ - fit = make_fname('test.fit') - its = make_its(params) - util.run_and_log(cons, [mkimage, '-f', its, fit]) - with open(make_fname('u-boot.dts'), 'w') as fd: - fd.write(base_fdt) - return fit - - def make_kernel(filename, text): - """Make a sample kernel with test data - - Args: - filename: the name of the file you want to create - Returns: - Full path and filename of the kernel it created - """ - fname = make_fname(filename) - data = '' - for i in range(100): - data += 'this %s %d is unlikely to boot\n' % (text, i) - with open(fname, 'w') as fd: - print(data, file=fd) - return fname - def make_ramdisk(filename, text): """Make a sample ramdisk with test data @@ -321,10 +260,10 @@ def test_fit(u_boot_console): - run code coverage to make sure we are testing all the code """ # Set up invariant files - control_dtb = make_dtb() - kernel = make_kernel('test-kernel.bin', 'kernel') + control_dtb = fit_util.make_dtb(cons, base_fdt, 'u-boot') + kernel = fit_util.make_kernel(cons, 'test-kernel.bin', 'kernel') ramdisk = make_ramdisk('test-ramdisk.bin', 'ramdisk') - loadables1 = make_kernel('test-loadables1.bin', 'lenrek') + loadables1 = fit_util.make_kernel(cons, 'test-loadables1.bin', 'lenrek') loadables2 = make_ramdisk('test-loadables2.bin', 'ksidmar') kernel_out = make_fname('kernel-out.bin') fdt = make_fname('u-boot.dtb') @@ -372,7 +311,7 @@ def test_fit(u_boot_console): } # Make a basic FIT and a script to load it - fit = make_fit(mkimage, params) + fit = fit_util.make_fit(cons, mkimage, base_its, params) params['fit'] = fit cmd = base_script % params @@ -403,7 +342,7 @@ def test_fit(u_boot_console): # Now a kernel and an FDT with cons.log.section('Kernel + FDT load'): params['fdt_load'] = 'load = <%#x>;' % params['fdt_addr'] - fit = make_fit(mkimage, params) + fit = fit_util.make_fit(cons, mkimage, base_its, params) cons.restart_uboot() output = cons.run_command_list(cmd.splitlines()) check_equal(kernel, kernel_out, 'Kernel not loaded') @@ -415,7 +354,7 @@ def test_fit(u_boot_console): with cons.log.section('Kernel + FDT + Ramdisk load'): params['ramdisk_config'] = 'ramdisk = "ramdisk-1";' params['ramdisk_load'] = 'load = <%#x>;' % params['ramdisk_addr'] - fit = make_fit(mkimage, params) + fit = fit_util.make_fit(cons, mkimage, base_its, params) cons.restart_uboot() output = cons.run_command_list(cmd.splitlines()) check_equal(ramdisk, ramdisk_out, 'Ramdisk not loaded') @@ -427,7 +366,7 @@ def test_fit(u_boot_console): params['loadables1_addr']) params['loadables2_load'] = ('load = <%#x>;' % params['loadables2_addr']) - fit = make_fit(mkimage, params) + fit = fit_util.make_fit(cons, mkimage, base_its, params) cons.restart_uboot() output = cons.run_command_list(cmd.splitlines()) check_equal(loadables1, loadables1_out, @@ -441,7 +380,7 @@ def test_fit(u_boot_console): params['kernel'] = make_compressed(kernel) params['fdt'] = make_compressed(fdt) params['ramdisk'] = make_compressed(ramdisk) - fit = make_fit(mkimage, params) + fit = fit_util.make_fit(cons, mkimage, base_its, params) cons.restart_uboot() output = cons.run_command_list(cmd.splitlines()) check_equal(kernel, kernel_out, 'Kernel not loaded') From 8aaacd61368929cd2af2296f00ee15fa4bf0ae55 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 11 Oct 2022 09:47:18 -0600 Subject: [PATCH 29/31] vbe: Add fixups for a basic set of OS requests As a starting point, add support for providing random data, if requested by the OS. Also add ASLR, as a placeholder for now. Signed-off-by: Simon Glass (fixed up to use uclass_first_device_err() instead) --- boot/Makefile | 2 +- boot/vbe_fixup.c | 233 +++++++++++++++++++++++++++++++ test/py/tests/test_event_dump.py | 1 + 3 files changed, 235 insertions(+), 1 deletion(-) create mode 100644 boot/vbe_fixup.c diff --git a/boot/Makefile b/boot/Makefile index 67e335255f1..dd45d786f8c 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 +obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE) += vbe.o vbe_fixup.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o diff --git a/boot/vbe_fixup.c b/boot/vbe_fixup.c new file mode 100644 index 00000000000..53d88678c92 --- /dev/null +++ b/boot/vbe_fixup.c @@ -0,0 +1,233 @@ +// 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/test/py/tests/test_event_dump.py b/test/py/tests/test_event_dump.py index bc54149e8f2..e63c25df537 100644 --- a/test/py/tests/test_event_dump.py +++ b/test/py/tests/test_event_dump.py @@ -16,6 +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 .*boot/vbe_fixup.c:.* EVT_FT_FIXUP bootmeth_vbe_simple_ft_fixup .*boot/vbe_simple.c:.* EVT_MISC_INIT_F sandbox_misc_init_f .*arch/sandbox/cpu/start.c:''' assert re.match(expect, out, re.MULTILINE) is not None From e7a18f751117ab38567d3929eacdcd9f3d6f5693 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 11 Oct 2022 09:47:19 -0600 Subject: [PATCH 30/31] dm: core: Update docs about oftree_from_fdt() Update this function's comment and also the livetree documentation, so it is clear when to use the function. Signed-off-by: Simon Glass --- doc/develop/driver-model/livetree.rst | 2 +- include/dm/ofnode.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/develop/driver-model/livetree.rst b/doc/develop/driver-model/livetree.rst index 55aa3eac929..579eef5ca9f 100644 --- a/doc/develop/driver-model/livetree.rst +++ b/doc/develop/driver-model/livetree.rst @@ -255,7 +255,7 @@ So long as OF_LIVE is disabled, it is possible to do fixups using the ofnode interface. The OF_LIVE support required addition of the flattening step at the end. -See dm_test_ofnode_root() for some examples. The ofnode_path_root() function +See dm_test_ofnode_root() for some examples. The oftree_from_fdt() function causes a flat device tree to be 'registered' such that it can be used by the ofnode interface. diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index 7aae2c29ef1..fa9865602d8 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -59,6 +59,9 @@ __attribute_const__ int ofnode_to_offset(ofnode node); /** * oftree_from_fdt() - Returns an oftree from a flat device tree pointer * + * If @fdt is not already registered in the list of current device trees, it is + * added to the list. + * * @fdt: Device tree to use * * Returns: reference to the given node From ae0bf2214b81b56a5670819958234947443680be Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 11 Oct 2022 09:47:20 -0600 Subject: [PATCH 31/31] vbe: Add a test for VBE device tree fixups When a FIT includes some OS requests, U-Boot should process these and add the requested info to corresponding subnodes of the /chosen node. Add a pytest for this, which sets up the FIT, runs bootm and then uses a C unit test to check that everything looks OK. The test needs to run on sandbox_flattree since we don't support device tree fixups on sandbox (live tree) yet. So enable BOOTMETH_VBE and disable bootflow_system(), since EFI is not supported on sandbox_flattree. Add a link to the initial documentation. Signed-off-by: Simon Glass --- configs/sandbox_flattree_defconfig | 2 +- doc/develop/vbe.rst | 3 +- test/boot/Makefile | 1 + test/boot/bootflow.c | 2 + test/boot/vbe_fixup.c | 59 ++++++++++++++ test/py/tests/fit_util.py | 5 +- test/py/tests/test_vbe.py | 123 +++++++++++++++++++++++++++++ 7 files changed, 192 insertions(+), 3 deletions(-) create mode 100644 test/boot/vbe_fixup.c create mode 100644 test/py/tests/test_vbe.py diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index fdd7b351189..9d8da25648d 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -11,7 +11,6 @@ CONFIG_DISTRO_DEFAULTS=y CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y CONFIG_FIT_VERBOSE=y -# CONFIG_BOOTMETH_VBE is not set CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y CONFIG_BOOTSTAGE_FDT=y @@ -84,6 +83,7 @@ CONFIG_REGMAP=y CONFIG_SYSCON=y CONFIG_DEVRES=y CONFIG_DEBUG_DEVRES=y +CONFIG_OFNODE_MULTI_TREE=y CONFIG_ADC=y CONFIG_ADC_SANDBOX=y CONFIG_AXI=y diff --git a/doc/develop/vbe.rst b/doc/develop/vbe.rst index 8f147fd9360..cca193c8fd4 100644 --- a/doc/develop/vbe.rst +++ b/doc/develop/vbe.rst @@ -19,8 +19,9 @@ listing methods and getting the status for a method. For a detailed overview of VBE, see vbe-intro_. A fuller description of bootflows is at vbe-bootflows_ and the firmware-update mechanism is described at -vbe-fwupdate_. +vbe-fwupdate_. VBE OS requests are described at vbe-osrequests_. .. _vbe-intro: https://docs.google.com/document/d/e/2PACX-1vQjXLPWMIyVktaTMf8edHZYDrEvMYD_iNzIj1FgPmKF37fpglAC47Tt5cvPBC5fvTdoK-GA5Zv1wifo/pub .. _vbe-bootflows: https://docs.google.com/document/d/e/2PACX-1vR0OzhuyRJQ8kdeOibS3xB1rVFy3J4M_QKTM5-3vPIBNcdvR0W8EXu9ymG-yWfqthzWoM4JUNhqwydN/pub .. _vbe-fwupdate: https://docs.google.com/document/d/e/2PACX-1vTnlIL17vVbl6TVoTHWYMED0bme7oHHNk-g5VGxblbPiKIdGDALE1HKId8Go5f0g1eziLsv4h9bocbk/pub +.. _vbe-osrequests: https://docs.google.com/document/d/e/2PACX-1vTHhxX7WSZe68i9rAkW-DHdx6koU-jxYHhamLhZn9GQ9QT2_epSBosMV1_r7yPHOXZccx71rF_t0PXL/pub diff --git a/test/boot/Makefile b/test/boot/Makefile index 9e9d5ae21f3..5bb3f889759 100644 --- a/test/boot/Makefile +++ b/test/boot/Makefile @@ -7,3 +7,4 @@ obj-$(CONFIG_BOOTSTD) += bootdev.o bootstd_common.o bootflow.o bootmeth.o ifdef CONFIG_OF_LIVE obj-$(CONFIG_BOOTMETH_VBE_SIMPLE) += vbe_simple.o endif +obj-$(CONFIG_BOOTMETH_VBE) += vbe_fixup.o diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c index 85305234e01..1e8ea754bcd 100644 --- a/test/boot/bootflow.c +++ b/test/boot/bootflow.c @@ -329,6 +329,8 @@ static int bootflow_system(struct unit_test_state *uts) { struct udevice *dev; + if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) + return 0; ut_assertok(uclass_get_device_by_name(UCLASS_BOOTMETH, "efi_mgr", &dev)); sandbox_set_fake_efi_mgr_dev(dev, true); diff --git a/test/boot/vbe_fixup.c b/test/boot/vbe_fixup.c new file mode 100644 index 00000000000..1b488e25ab6 --- /dev/null +++ b/test/boot/vbe_fixup.c @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test for VBE device tree fix-ups + * + * Copyright 2022 Google LLC + * Written by Simon Glass + */ + +#include +#include +#include +#include +#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) +{ + 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)); + + chosen = oftree_path(tree, "/chosen"); + ut_assert(ofnode_valid(chosen)); + + /* check the things set up for the FIT in test_vbe.py */ + node = ofnode_find_subnode(chosen, "random"); + + /* ignore if this test is run on its own */ + if (!ofnode_valid(node)) + return 0; + data = ofnode_read_prop(node, "data", &size); + ut_asserteq(0x40, size); + + node = ofnode_find_subnode(chosen, "aslr2"); + ut_assert(ofnode_valid(node)); + data = ofnode_read_prop(node, "data", &size); + ut_asserteq(4, size); + + node = ofnode_find_subnode(chosen, "efi-runtime"); + ut_assert(ofnode_valid(node)); + data = ofnode_read_prop(node, "data", &size); + ut_asserteq(4, size); + + return 0; +} +BOOTSTD_TEST(vbe_test_fixup, + UT_TESTF_DM | UT_TESTF_SCAN_FDT | UT_TESTF_FLAT_TREE); diff --git a/test/py/tests/fit_util.py b/test/py/tests/fit_util.py index fcec4c43c3c..79718d431a0 100644 --- a/test/py/tests/fit_util.py +++ b/test/py/tests/fit_util.py @@ -36,7 +36,7 @@ def make_its(cons, base_its, params, basename='test.its'): print(base_its % params, file=outf) return its -def make_fit(cons, mkimage, base_its, params, basename='test.fit'): +def make_fit(cons, mkimage, base_its, params, basename='test.fit', base_fdt=None): """Make a sample .fit file ready for loading This creates a .its script with the selected parameters and uses mkimage to @@ -55,6 +55,9 @@ def make_fit(cons, mkimage, base_its, params, basename='test.fit'): fit = make_fname(cons, basename) its = make_its(cons, base_its, params) util.run_and_log(cons, [mkimage, '-f', its, fit]) + if base_fdt: + with open(make_fname(cons, 'u-boot.dts'), 'w') as fd: + fd.write(base_fdt) return fit def make_kernel(cons, basename, text): diff --git a/test/py/tests/test_vbe.py b/test/py/tests/test_vbe.py new file mode 100644 index 00000000000..559c2918868 --- /dev/null +++ b/test/py/tests/test_vbe.py @@ -0,0 +1,123 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2022 Google LLC +# +# Test addition of VBE + +import pytest + +import fit_util + +# Define a base ITS which we can adjust using % and a dictionary +base_its = ''' +/dts-v1/; + +/ { + description = "Example kernel"; + + images { + kernel-1 { + data = /incbin/("%(kernel)s"); + type = "kernel"; + arch = "sandbox"; + os = "linux"; + load = <0x40000>; + entry = <0x8>; + compression = "%(compression)s"; + + random { + compatible = "vbe,random-rand"; + vbe,size = <0x40>; + vbe,required; + }; + aslr1 { + compatible = "vbe,aslr-move"; + vbe,align = <0x100000>; + }; + aslr2 { + compatible = "vbe,aslr-rand"; + }; + efi-runtime { + compatible = "vbe,efi-runtime-rand"; + }; + wibble { + compatible = "vbe,wibble"; + }; + }; + + fdt-1 { + description = "snow"; + data = /incbin/("%(fdt)s"); + type = "flat_dt"; + arch = "sandbox"; + load = <%(fdt_addr)#x>; + compression = "%(compression)s"; + }; + }; + configurations { + default = "conf-1"; + conf-1 { + kernel = "kernel-1"; + fdt = "fdt-1"; + }; + }; +}; +''' + +# Define a base FDT - currently we don't use anything in this +base_fdt = ''' +/dts-v1/; + +/ { + chosen { + }; +}; +''' + +# This is the U-Boot script that is run for each test. First load the FIT, +# then run the 'bootm' command, then run the unit test which checks that the +# working tree has the required things filled in according to the OS requests +# above (random, aslr2, etc.) +base_script = ''' +host load hostfs 0 %(fit_addr)x %(fit)s +fdt addr %(fit_addr)x +bootm start %(fit_addr)x +bootm loados +bootm prep +fdt addr +fdt print +ut bootstd vbe_test_fixup +''' + +@pytest.mark.boardspec('sandbox_flattree') +@pytest.mark.requiredtool('dtc') +def test_vbe(u_boot_console): + cons = u_boot_console + kernel = fit_util.make_kernel(cons, 'vbe-kernel.bin', 'kernel') + fdt = fit_util.make_dtb(cons, base_fdt, 'vbe-fdt') + fdt_out = fit_util.make_fname(cons, 'fdt-out.dtb') + + params = { + 'fit_addr' : 0x1000, + + 'kernel' : kernel, + + 'fdt' : fdt, + 'fdt_out' : fdt_out, + 'fdt_addr' : 0x80000, + 'fdt_size' : 0x1000, + + 'compression' : 'none', + } + mkimage = cons.config.build_dir + '/tools/mkimage' + fit = fit_util.make_fit(cons, mkimage, base_its, params, 'test-vbe.fit', + base_fdt) + params['fit'] = fit + cmd = base_script % params + + 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]