[v4 01/24] buildman: Use oldconfig when adjusting the config
From: Simon Glass sjg@chromium.org
We cannot be sure that the new config is consistent, particularly when changing a major item like CONFIG_CMDLINE. Use 'make oldconfig' to check that and avoid any such problems.
Signed-off-by: Simon Glass sjg@chromium.org --- tools/buildman/builder.py | 2 +- tools/buildman/builderthread.py | 6 ++++++ tools/buildman/func_test.py | 4 +++- 3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 5305477c5be6..782e59dd5cca 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -476,7 +476,7 @@ class Builder: Args: commit: Commit object that is being built brd: Board object that is being built - stage: Stage that we are at (mrproper, config, build) + stage: Stage that we are at (mrproper, config, oldconfig, build) cwd: Directory where make should be run args: Arguments to pass to make kwargs: Arguments to pass to command.run_pipe() diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 6a61f64da1d4..a8599c0bb2a8 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -426,6 +426,12 @@ class BuilderThread(threading.Thread):
# Now do the build, if everything looks OK if result.return_code == 0: + if adjust_cfg: + oldc_args = list(args) + ['oldconfig'] + oldc_result = self.make(commit, brd, 'oldconfig', cwd, + *oldc_args, env=env) + if oldc_result.return_code: + return oldc_result result = self._build(commit, brd, cwd, args, env, cmd_list, config_only) if adjust_cfg: diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py index 55dd494fe8ee..6b88ed815d65 100644 --- a/tools/buildman/func_test.py +++ b/tools/buildman/func_test.py @@ -439,6 +439,8 @@ class TestFunctional(unittest.TestCase): tools.write_file(fname, b'CONFIG_SOMETHING=1') return command.CommandResult(return_code=0, combined='Test configuration complete') + elif stage == 'oldconfig': + return command.CommandResult(return_code=0) elif stage == 'build': stderr = '' fname = os.path.join(cwd or '', out_dir, 'u-boot') @@ -461,7 +463,7 @@ Some images are invalid''' return command.CommandResult(return_code=0)
# Not handled, so abort - print('make', stage) + print('_HandleMake failure: make', stage) sys.exit(1)
# Example function to print output lines
As VIRTIO_NET is the symbol for enabling network devices, make this depend on NETDEVICES
Signed-off-by: Tom Rini trini@konsulko.com --- Cc: Bin Meng bmeng.cn@gmail.com --- drivers/virtio/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index 852f6735b602..1de68867d52e 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -56,7 +56,7 @@ config VIRTIO_SANDBOX
config VIRTIO_NET bool "virtio net driver" - depends on VIRTIO + depends on VIRTIO && NETDEVICES help This is the virtual net driver for virtio. It can be used with QEMU based targets.
In order to do a DFU update over TFTP we need to have some network device available, so make this depend on NETDEVICES
Signed-off-by: Tom Rini trini@konsulko.com --- drivers/dfu/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig index 4e80e85d10d7..8771678ca5a0 100644 --- a/drivers/dfu/Kconfig +++ b/drivers/dfu/Kconfig @@ -19,6 +19,7 @@ config DFU_WRITE_ALT
config DFU_TFTP bool "DFU via TFTP" + depends on NETDEVICES select UPDATE_COMMON select DFU_OVER_TFTP help
In order to be able to disable all commands we need to construct our version string in a common file, and have the version command reference that string, like the other users of it do. Create common/version.c with just the strings.
Signed-off-by: Tom Rini trini@konsulko.com --- cmd/version.c | 9 --------- common/Makefile | 1 + common/version.c | 16 ++++++++++++++++ 3 files changed, 17 insertions(+), 9 deletions(-) create mode 100644 common/version.c
diff --git a/cmd/version.c b/cmd/version.c index 87e1fa4159c1..d99a44f19fb3 100644 --- a/cmd/version.c +++ b/cmd/version.c @@ -7,21 +7,12 @@ #include <common.h> #include <command.h> #include <display_options.h> -#include <timestamp.h> -#include <version.h> #include <version_string.h> #include <linux/compiler.h> #ifdef CONFIG_SYS_COREBOOT #include <asm/cb_sysinfo.h> #endif
-#define U_BOOT_VERSION_STRING U_BOOT_VERSION " (" U_BOOT_DATE " - " \ - U_BOOT_TIME " " U_BOOT_TZ ")" CONFIG_IDENT_STRING - -const char version_string[] = U_BOOT_VERSION_STRING; -const unsigned short version_num = U_BOOT_VERSION_NUM; -const unsigned char version_num_patch = U_BOOT_VERSION_NUM_PATCH; - static int do_version(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { diff --git a/common/Makefile b/common/Makefile index cdeadf72026c..b711dc29b65e 100644 --- a/common/Makefile +++ b/common/Makefile @@ -10,6 +10,7 @@ obj-y += main.o obj-y += exports.o obj-$(CONFIG_HUSH_PARSER) += cli_hush.o obj-$(CONFIG_AUTOBOOT) += autoboot.o +obj-y += version.o
# # boards obj-y += board_f.o diff --git a/common/version.c b/common/version.c new file mode 100644 index 000000000000..6e27bb80e398 --- /dev/null +++ b/common/version.c @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2000-2009 + * Wolfgang Denk, DENX Software Engineering, wd@denx.de. + */ + +#include <timestamp.h> +#include <version.h> +#include <version_string.h> + +#define U_BOOT_VERSION_STRING U_BOOT_VERSION " (" U_BOOT_DATE " - " \ + U_BOOT_TIME " " U_BOOT_TZ ")" CONFIG_IDENT_STRING + +const char version_string[] = U_BOOT_VERSION_STRING; +const unsigned short version_num = U_BOOT_VERSION_NUM; +const unsigned char version_num_patch = U_BOOT_VERSION_NUM_PATCH;
Rather than selecting CMD_QFW, we should make the option itself by enabled by default on these platforms. Then in the board-specific Kconfig we should select the appropriate back-end as needed if the command is enabled.
Signed-off-by: Tom Rini trini@konsulko.com --- Cc: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi Cc: Bin Meng bmeng.cn@gmail.com --- board/emulation/qemu-arm/Kconfig | 3 +-- board/emulation/qemu-x86/Kconfig | 2 +- cmd/Kconfig | 2 ++ 3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/board/emulation/qemu-arm/Kconfig b/board/emulation/qemu-arm/Kconfig index 09c95413a541..ac2d078f42a1 100644 --- a/board/emulation/qemu-arm/Kconfig +++ b/board/emulation/qemu-arm/Kconfig @@ -5,8 +5,7 @@ config TEXT_BASE
config BOARD_SPECIFIC_OPTIONS # dummy def_bool y - select CMD_QFW - select QFW_MMIO + select QFW_MMIO if CMD_QFW imply VIRTIO_MMIO imply VIRTIO_PCI imply VIRTIO_NET diff --git a/board/emulation/qemu-x86/Kconfig b/board/emulation/qemu-x86/Kconfig index 787751abba4f..01dc1d497aec 100644 --- a/board/emulation/qemu-x86/Kconfig +++ b/board/emulation/qemu-x86/Kconfig @@ -20,7 +20,7 @@ config BOARD_SPECIFIC_OPTIONS # dummy def_bool y select X86_RESET_VECTOR select QEMU - select QFW_PIO + select QFW_PIO if CMD_QFW select BOARD_ROMSIZE_KB_1024 imply VIRTIO_PCI imply VIRTIO_NET diff --git a/cmd/Kconfig b/cmd/Kconfig index 5bc0a92d57ad..5554bd14e2ef 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -2239,6 +2239,8 @@ config CMD_SYSBOOT config CMD_QFW bool "qfw" select QFW + default y if TARGET_QEMU_ARM_32BIT || TARGET_QEMU_ARM_64BIT || \ + TARGET_QEMU_X86 || TARGET_QEMU_X86_64 help This provides access to the QEMU firmware interface. The main feature is to allow easy loading of files passed to qemu-system
Move CONFIG_SYS_CBSIZE (console buffer size) and CONFIG_SYS_PBSIZE (console print buffer size) out of cmd/Kconfig and in to common/Kconfig. Create help entries for both which explain their usage and why they are both not entirely command centric.
Signed-off-by: Tom Rini trini@konsulko.com --- cmd/Kconfig | 14 -------------- common/Kconfig | 23 +++++++++++++++++++++++ 2 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index 5554bd14e2ef..ecf25b062ad6 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -75,20 +75,6 @@ config SYS_MAXARGS int "Maximum number arguments accepted by commands" default 16
-config SYS_CBSIZE - int "Console input buffer size" - default 2048 if ARCH_TEGRA || ARCH_VERSAL || ARCH_ZYNQ || ARCH_ZYNQMP || \ - RCAR_GEN3 || TARGET_SOCFPGA_SOC64 - default 512 if ARCH_MX5 || ARCH_MX6 || ARCH_MX7 || FSL_LSCH2 || \ - FSL_LSCH3 || X86 - default 256 if M68K || PPC - default 1024 - -config SYS_PBSIZE - int "Buffer size for console output" - default 1024 if ARCH_SUNXI - default 1044 - config SYS_XTRACE bool "Command execution tracer" depends on CMDLINE diff --git a/common/Kconfig b/common/Kconfig index 93c96f23b013..fba1548e135b 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -52,6 +52,29 @@ config CONSOLE_RECORD_IN_SIZE The buffer is allocated immediately after the malloc() region is ready.
+config SYS_CBSIZE + int "Console input buffer size" + default 2048 if ARCH_TEGRA || ARCH_VERSAL || ARCH_ZYNQ || ARCH_ZYNQMP || \ + RCAR_GEN3 || TARGET_SOCFPGA_SOC64 + default 512 if ARCH_MX5 || ARCH_MX6 || ARCH_MX7 || FSL_LSCH2 || \ + FSL_LSCH3 || X86 + default 256 if M68K || PPC + default 1024 + help + Set the size of the console input buffer. This is used both in the + case of reading input literally from the user in some manner as well + as when we need to construct or modify that type of input, for + example when constructing "bootargs" for the OS. + +config SYS_PBSIZE + int "Console output buffer size" + default 1024 if ARCH_SUNXI + default 1044 + help + Set the size of the console output buffer. This is used when we need + to work with some form of a buffer while providing output in some + form to the user. + config DISABLE_CONSOLE bool "Add functionality to disable console completely" help
Inside of env/common.c we already have our helper env_set_xxx functions, and even have a comment that explains why env_set() itself wasn't moved. We now handle that move. This requires that we rename the previous _do_env_set() to env_do_env_set() and note it as an internal env function. Add comments about this function to explain why we do this when we add the prototype. Add a new function, env_inc_id() to allow for the counter to be updated by both commands and callers, and document this as well by the prototype.
Signed-off-by: Tom Rini trini@konsulko.com --- cmd/Makefile | 4 +- cmd/nvedit.c | 122 ++--------------------------------------- env/common.c | 113 ++++++++++++++++++++++++++++++++++++-- include/env.h | 8 +++ include/env_internal.h | 12 ++++ 5 files changed, 135 insertions(+), 124 deletions(-)
diff --git a/cmd/Makefile b/cmd/Makefile index 44db5f22861e..27a0045e7f94 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -128,6 +128,7 @@ endif obj-$(CONFIG_CMD_MUX) += mux.o obj-$(CONFIG_CMD_NAND) += nand.o obj-$(CONFIG_CMD_NET) += net.o +obj-$(CONFIG_ENV_SUPPORT) += nvedit.o obj-$(CONFIG_CMD_NVEDIT_EFI) += nvedit_efi.o obj-$(CONFIG_CMD_ONENAND) += onenand.o obj-$(CONFIG_CMD_OSD) += osd.o @@ -244,9 +245,6 @@ endif # !CONFIG_SPL_BUILD
obj-$(CONFIG_$(SPL_)CMD_TLV_EEPROM) += tlv_eeprom.o
-# core command -obj-y += nvedit.o - obj-$(CONFIG_CMD_BCM_EXT_UTILS) += broadcom/
filechk_data_gz = (echo "static const char data_gz[] ="; cat $< | scripts/bin2c; echo ";") diff --git a/cmd/nvedit.c b/cmd/nvedit.c index daf1ad37f9be..e77338f81394 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -49,20 +49,6 @@ DECLARE_GLOBAL_DATA_PTR; */ #define MAX_ENV_SIZE (1 << 20) /* 1 MiB */
-/* - * This variable is incremented on each do_env_set(), so it can - * be used via env_get_id() as an indication, if the environment - * has changed or not. So it is possible to reread an environment - * variable only if the environment was changed ... done so for - * example in NetInitLoop() - */ -static int env_id = 1; - -int env_get_id(void) -{ - return env_id; -} - #ifndef CONFIG_SPL_BUILD /* * Command interface: print one or all environment variables @@ -198,104 +184,6 @@ DONE: #endif #endif /* CONFIG_SPL_BUILD */
-/* - * Set a new environment variable, - * or replace or delete an existing one. - */ -static int _do_env_set(int flag, int argc, char *const argv[], int env_flag) -{ - int i, len; - char *name, *value, *s; - struct env_entry e, *ep; - - debug("Initial value for argc=%d\n", argc); - -#if !IS_ENABLED(CONFIG_SPL_BUILD) && IS_ENABLED(CONFIG_CMD_NVEDIT_EFI) - if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e') - return do_env_set_efi(NULL, flag, --argc, ++argv); -#endif - - while (argc > 1 && **(argv + 1) == '-') { - char *arg = *++argv; - - --argc; - while (*++arg) { - switch (*arg) { - case 'f': /* force */ - env_flag |= H_FORCE; - break; - default: - return CMD_RET_USAGE; - } - } - } - debug("Final value for argc=%d\n", argc); - name = argv[1]; - - if (strchr(name, '=')) { - printf("## Error: illegal character '='" - "in variable name "%s"\n", name); - return 1; - } - - env_id++; - - /* Delete only ? */ - if (argc < 3 || argv[2] == NULL) { - int rc = hdelete_r(name, &env_htab, env_flag); - - /* If the variable didn't exist, don't report an error */ - return rc && rc != -ENOENT ? 1 : 0; - } - - /* - * Insert / replace new value - */ - for (i = 2, len = 0; i < argc; ++i) - len += strlen(argv[i]) + 1; - - value = malloc(len); - if (value == NULL) { - printf("## Can't malloc %d bytes\n", len); - return 1; - } - for (i = 2, s = value; i < argc; ++i) { - char *v = argv[i]; - - while ((*s++ = *v++) != '\0') - ; - *(s - 1) = ' '; - } - if (s != value) - *--s = '\0'; - - e.key = name; - e.data = value; - hsearch_r(e, ENV_ENTER, &ep, &env_htab, env_flag); - free(value); - if (!ep) { - printf("## Error inserting "%s" variable, errno=%d\n", - name, errno); - return 1; - } - - return 0; -} - -int env_set(const char *varname, const char *varvalue) -{ - const char * const argv[4] = { "setenv", varname, varvalue, NULL }; - - /* before import into hashtable */ - if (!(gd->flags & GD_FLG_ENV_READY)) - return 1; - - if (varvalue == NULL || varvalue[0] == '\0') - return _do_env_set(0, 2, (char * const *)argv, H_PROGRAMMATIC); - else - return _do_env_set(0, 3, (char * const *)argv, H_PROGRAMMATIC); -} - #ifndef CONFIG_SPL_BUILD static int do_env_set(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -303,7 +191,7 @@ static int do_env_set(struct cmd_tbl *cmdtp, int flag, int argc, if (argc < 2) return CMD_RET_USAGE;
- return _do_env_set(flag, argc, argv, H_INTERACTIVE); + return env_do_env_set(flag, argc, argv, H_INTERACTIVE); }
/* @@ -381,7 +269,7 @@ int do_env_ask(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) }
/* Continue calling setenv code */ - return _do_env_set(flag, len, local_args, H_INTERACTIVE); + return env_do_env_set(flag, len, local_args, H_INTERACTIVE); } #endif
@@ -561,12 +449,12 @@ static int do_env_edit(struct cmd_tbl *cmdtp, int flag, int argc, if (buffer[0] == '\0') { const char * const _argv[3] = { "setenv", argv[1], NULL };
- return _do_env_set(0, 2, (char * const *)_argv, H_INTERACTIVE); + return env_do_env_set(0, 2, (char * const *)_argv, H_INTERACTIVE); } else { const char * const _argv[4] = { "setenv", argv[1], buffer, NULL };
- return _do_env_set(0, 3, (char * const *)_argv, H_INTERACTIVE); + return env_do_env_set(0, 3, (char * const *)_argv, H_INTERACTIVE); } } #endif /* CONFIG_CMD_EDITENV */ @@ -679,7 +567,7 @@ static int do_env_delete(struct cmd_tbl *cmdtp, int flag, } debug("Final value for argc=%d\n", argc);
- env_id++; + env_inc_id();
while (--argc > 0) { char *name = *++argv; diff --git a/env/common.c b/env/common.c index eb1a91379539..656748c1f5b7 100644 --- a/env/common.c +++ b/env/common.c @@ -37,11 +37,116 @@ struct hsearch_data env_htab = { };
/* - * This env_set() function is defined in cmd/nvedit.c, since it calls - * _do_env_set(), whis is a static function in that file. - * - * int env_set(const char *varname, const char *varvalue); + * This variable is incremented each time we set an environment variable so we + * can be check via env_get_id() to see if the environment has changed or not. + * This makes it possible to reread an environment variable only if the + * environment was changed, typically used by networking code. */ +static int env_id = 1; + +int env_get_id(void) +{ + return env_id; +} + +void env_inc_id(void) +{ + env_id++; +} + +int env_do_env_set(int flag, int argc, char *const argv[], int env_flag) +{ + int i, len; + char *name, *value, *s; + struct env_entry e, *ep; + + debug("Initial value for argc=%d\n", argc); + +#if !IS_ENABLED(CONFIG_SPL_BUILD) && IS_ENABLED(CONFIG_CMD_NVEDIT_EFI) + if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e') + return do_env_set_efi(NULL, flag, --argc, ++argv); +#endif + + while (argc > 1 && **(argv + 1) == '-') { + char *arg = *++argv; + + --argc; + while (*++arg) { + switch (*arg) { + case 'f': /* force */ + env_flag |= H_FORCE; + break; + default: + return CMD_RET_USAGE; + } + } + } + debug("Final value for argc=%d\n", argc); + name = argv[1]; + + if (strchr(name, '=')) { + printf("## Error: illegal character '='" + "in variable name "%s"\n", name); + return 1; + } + + env_inc_id(); + + /* Delete only ? */ + if (argc < 3 || argv[2] == NULL) { + int rc = hdelete_r(name, &env_htab, env_flag); + + /* If the variable didn't exist, don't report an error */ + return rc && rc != -ENOENT ? 1 : 0; + } + + /* + * Insert / replace new value + */ + for (i = 2, len = 0; i < argc; ++i) + len += strlen(argv[i]) + 1; + + value = malloc(len); + if (value == NULL) { + printf("## Can't malloc %d bytes\n", len); + return 1; + } + for (i = 2, s = value; i < argc; ++i) { + char *v = argv[i]; + + while ((*s++ = *v++) != '\0') + ; + *(s - 1) = ' '; + } + if (s != value) + *--s = '\0'; + + e.key = name; + e.data = value; + hsearch_r(e, ENV_ENTER, &ep, &env_htab, env_flag); + free(value); + if (!ep) { + printf("## Error inserting "%s" variable, errno=%d\n", + name, errno); + return 1; + } + + return 0; +} + +int env_set(const char *varname, const char *varvalue) +{ + const char * const argv[4] = { "setenv", varname, varvalue, NULL }; + + /* before import into hashtable */ + if (!(gd->flags & GD_FLG_ENV_READY)) + return 1; + + if (varvalue == NULL || varvalue[0] == '\0') + return env_do_env_set(0, 2, (char * const *)argv, H_PROGRAMMATIC); + else + return env_do_env_set(0, 3, (char * const *)argv, H_PROGRAMMATIC); +}
/** * Set an environment variable to an integer value diff --git a/include/env.h b/include/env.h index 430c4fa94a42..9778e3e4f2ce 100644 --- a/include/env.h +++ b/include/env.h @@ -72,6 +72,14 @@ enum env_redund_flags { */ int env_get_id(void);
+/** + * env_inc_id() - Increase the sequence number for the environment + * + * Increment the value that is used by env_get_id() to inform callers + * if the environment has changed since they last checked. + */ +void env_inc_id(void); + /** * env_init() - Set up the pre-relocation environment * diff --git a/include/env_internal.h b/include/env_internal.h index 6a6949464689..ae7816d38e58 100644 --- a/include/env_internal.h +++ b/include/env_internal.h @@ -193,6 +193,18 @@ struct env_driver {
extern struct hsearch_data env_htab;
+/** + * env_do_env_set() - Perform the actual setting of an environment variable + * + * Due to the number of places we may need to set an environmental variable + * from we have an exposed internal function that performs the real work and + * then call this from both the command line function as well as other + * locations. + * + * Return: 0 on success or 1 on failure + */ +int env_do_env_set(int flag, int argc, char *const argv[], int env_flag); + /** * env_ext4_get_intf() - Provide the interface for env in EXT4 *
From: Simon Glass sjg@chromium.org
Many tests make some use of the command line, so require it for all test code.
This could be teased apart, perhaps with a test flag indicating that it uses the command line. Leave that for later.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com --- test/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/test/Kconfig b/test/Kconfig index ca648d23376f..c3db727c58e3 100644 --- a/test/Kconfig +++ b/test/Kconfig @@ -2,6 +2,7 @@ menu "Testing"
config UNIT_TEST bool "Unit tests" + depends on CMDLINE help Select this to compile in unit tests for various parts of U-Boot. Test suites will be subcommands of the "ut" command.
From: Simon Glass sjg@chromium.org
While it is nice to have the font command, using 'select' makes it impossible to build the console code without it. Stop using 'select' and make it default if CONSOLE_TRUETYPE is enabled when asking the command.
Signed-off-by: Simon Glass sjg@chromium.org Signed-off-by: Tom Rini trini@konsulko.com --- Changes in v4: - Rework to have the command itself be default y if CONSOLE_TRUETYPE instead of selecting it. (Tom) --- cmd/Kconfig | 2 +- drivers/video/Kconfig | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index ecf25b062ad6..16e5cb8f0633 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -2340,7 +2340,7 @@ config CMD_VIDCONSOLE config CMD_SELECT_FONT bool "select font size" depends on VIDEO - default n + default y if CONSOLE_TRUETYPE help Enabling this will provide 'font' command. Allows font selection at runtime. diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index ab927641bb7a..6f319ba0d544 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -180,7 +180,6 @@ config CONSOLE_ROTATION
config CONSOLE_TRUETYPE bool "Support a console that uses TrueType fonts" - select CMD_SELECT_FONT help TrueTrype fonts can provide outline-drawing capability rather than needing to provide a bitmap for each font and size that is needed.
The interactive portion of our non-HUSH 'simple' parser is guarded by CONFIG_CMDLINE already. Much of the code behind this simple parser is also used as "input" parser, such as from menu interfaces and so forth and not strictly command line input. To support this, always build the assorted cli object files, but guard the interactive portions of cli_simple.c with a CMDLINE check.
Signed-off-by: Tom Rini trini@konsulko.com --- common/Makefile | 2 +- common/cli_simple.c | 77 +++++++++++++++++++++++---------------------- 2 files changed, 40 insertions(+), 39 deletions(-)
diff --git a/common/Makefile b/common/Makefile index b711dc29b65e..1495436d5d45 100644 --- a/common/Makefile +++ b/common/Makefile @@ -8,6 +8,7 @@ ifndef CONFIG_SPL_BUILD obj-y += init/ obj-y += main.o obj-y += exports.o +obj-y += cli_getch.o cli_simple.o cli_readline.o obj-$(CONFIG_HUSH_PARSER) += cli_hush.o obj-$(CONFIG_AUTOBOOT) += autoboot.o obj-y += version.o @@ -38,7 +39,6 @@ obj-$(CONFIG_SPLASH_SOURCE) += splash_source.o obj-$(CONFIG_MENU) += menu.o obj-$(CONFIG_UPDATE_COMMON) += update.o obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o -obj-$(CONFIG_CMDLINE) += cli_getch.o cli_readline.o cli_simple.o
endif # !CONFIG_SPL_BUILD
diff --git a/common/cli_simple.c b/common/cli_simple.c index e80ba488a5eb..f89ba92d1b05 100644 --- a/common/cli_simple.c +++ b/common/cli_simple.c @@ -22,44 +22,6 @@ #define debug_parser(fmt, args...) \ debug_cond(DEBUG_PARSER, fmt, ##args)
- -int cli_simple_parse_line(char *line, char *argv[]) -{ - int nargs = 0; - - debug_parser("%s: "%s"\n", __func__, line); - while (nargs < CONFIG_SYS_MAXARGS) { - /* skip any white space */ - while (isblank(*line)) - ++line; - - if (*line == '\0') { /* end of line, no more args */ - argv[nargs] = NULL; - debug_parser("%s: nargs=%d\n", __func__, nargs); - return nargs; - } - - argv[nargs++] = line; /* begin of argument string */ - - /* find end of string */ - while (*line && !isblank(*line)) - ++line; - - if (*line == '\0') { /* end of line, no more args */ - argv[nargs] = NULL; - debug_parser("parse_line: nargs=%d\n", nargs); - return nargs; - } - - *line++ = '\0'; /* terminate current arg */ - } - - printf("** Too many args (max. %d) **\n", CONFIG_SYS_MAXARGS); - - debug_parser("%s: nargs=%d\n", __func__, nargs); - return nargs; -} - int cli_simple_process_macros(const char *input, char *output, int max_size) { char c, prev; @@ -172,6 +134,44 @@ int cli_simple_process_macros(const char *input, char *output, int max_size) return ret; }
+#ifdef CONFIG_CMDLINE +int cli_simple_parse_line(char *line, char *argv[]) +{ + int nargs = 0; + + debug_parser("%s: "%s"\n", __func__, line); + while (nargs < CONFIG_SYS_MAXARGS) { + /* skip any white space */ + while (isblank(*line)) + ++line; + + if (*line == '\0') { /* end of line, no more args */ + argv[nargs] = NULL; + debug_parser("%s: nargs=%d\n", __func__, nargs); + return nargs; + } + + argv[nargs++] = line; /* begin of argument string */ + + /* find end of string */ + while (*line && !isblank(*line)) + ++line; + + if (*line == '\0') { /* end of line, no more args */ + argv[nargs] = NULL; + debug_parser("parse_line: nargs=%d\n", nargs); + return nargs; + } + + *line++ = '\0'; /* terminate current arg */ + } + + printf("** Too many args (max. %d) **\n", CONFIG_SYS_MAXARGS); + + debug_parser("%s: nargs=%d\n", __func__, nargs); + return nargs; +} + /* * WARNING: * @@ -346,3 +346,4 @@ int cli_simple_run_command_list(char *cmd, int flag)
return rcode; } +#endif
From: Simon Glass sjg@chromium.org
The command should not be used to enable library functionality. Add a new BOOTEFI_BOOTMGR Kconfig for that. Adjust the conditions so that the same code is built.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org --- Cc: Heinrich Schuchardt heinrich.schuchardt@canonical.com Cc: Ilias Apalodimas ilias.apalodimas@linaro.org Changes in v4: - Integrate AKASHI Takahiro's feedback from v3 - Reword the help text on CMD_BOOTEFI_BOOTMGR slightly --- cmd/Kconfig | 11 ++++++++++- lib/efi_loader/Kconfig | 6 +++--- lib/efi_loader/Makefile | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index 16e5cb8f0633..872cb49150cc 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -379,6 +379,15 @@ config CMD_BOOTEFI help Boot an EFI image from memory.
+config CMD_BOOTEFI_BOOTMGR + bool "UEFI Boot Manager command" + depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI + default y + help + Select this option to enable the 'bootmgr' subcommand of 'bootefi'. + This subcommand will allow you to select the UEFI binary to be booted + via UEFI variables Boot####, BootOrder, and BootNext. + config CMD_BOOTEFI_HELLO_COMPILE bool "Compile a standard EFI hello world binary for testing" depends on CMD_BOOTEFI && !CPU_V7M @@ -2110,7 +2119,7 @@ config CMD_EFIDEBUG config CMD_EFICONFIG bool "eficonfig - provide menu-driven uefi variables maintenance interface" default y if !HAS_BOARD_SIZE_LIMIT - depends on CMD_BOOTEFI_BOOTMGR + depends on BOOTEFI_BOOTMGR select MENU help Enable the 'eficonfig' command which provides the menu-driven UEFI diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index d20aaab6dba4..13cad6342c36 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -32,14 +32,14 @@ config EFI_LOADER
if EFI_LOADER
-config CMD_BOOTEFI_BOOTMGR +config BOOTEFI_BOOTMGR bool "UEFI Boot Manager" default y select BOOTMETH_GLOBAL if BOOTSTD help Select this option if you want to select the UEFI binary to be booted - via UEFI variables Boot####, BootOrder, and BootNext. This enables the - 'bootefi bootmgr' command. + via UEFI variables Boot####, BootOrder, and BootNext. You should also + normally enable CMD_BOOTEFI_BOOTMGR so that the command is available.
choice prompt "Store for non-volatile UEFI variables" diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 8d31fc61c601..0a2cb6e3c476 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -42,7 +42,7 @@ targets += initrddump.o endif
obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o -obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o +obj-$(CONFIG_BOOTEFI_BOOTMGR) += efi_bootmgr.o obj-y += efi_boottime.o obj-y += efi_helper.o obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
On 19.10.23 17:00, Tom Rini wrote:
From: Simon Glass sjg@chromium.org
The command should not be used to enable library functionality. Add a new BOOTEFI_BOOTMGR Kconfig for that. Adjust the conditions so that the same code is built.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org
Cc: Heinrich Schuchardt heinrich.schuchardt@canonical.com Cc: Ilias Apalodimas ilias.apalodimas@linaro.org Changes in v4:
- Integrate AKASHI Takahiro's feedback from v3
- Reword the help text on CMD_BOOTEFI_BOOTMGR slightly
cmd/Kconfig | 11 ++++++++++- lib/efi_loader/Kconfig | 6 +++--- lib/efi_loader/Makefile | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index 16e5cb8f0633..872cb49150cc 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -379,6 +379,15 @@ config CMD_BOOTEFI help Boot an EFI image from memory.
+config CMD_BOOTEFI_BOOTMGR
- bool "UEFI Boot Manager command"
- depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
- default y
- help
Select this option to enable the 'bootmgr' subcommand of 'bootefi'.This subcommand will allow you to select the UEFI binary to be bootedvia UEFI variables Boot####, BootOrder, and BootNext.- config CMD_BOOTEFI_HELLO_COMPILE bool "Compile a standard EFI hello world binary for testing" depends on CMD_BOOTEFI && !CPU_V7M
@@ -2110,7 +2119,7 @@ config CMD_EFIDEBUG config CMD_EFICONFIG bool "eficonfig - provide menu-driven uefi variables maintenance interface" default y if !HAS_BOARD_SIZE_LIMIT
- depends on CMD_BOOTEFI_BOOTMGR
- depends on BOOTEFI_BOOTMGR select MENU help Enable the 'eficonfig' command which provides the menu-driven UEFI
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index d20aaab6dba4..13cad6342c36 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -32,14 +32,14 @@ config EFI_LOADER
if EFI_LOADER
-config CMD_BOOTEFI_BOOTMGR +config BOOTEFI_BOOTMGR bool "UEFI Boot Manager" default y select BOOTMETH_GLOBAL if BOOTSTD help Select this option if you want to select the UEFI binary to be booted
via UEFI variables Boot####, BootOrder, and BootNext. This enables the'bootefi bootmgr' command.
via UEFI variables Boot####, BootOrder, and BootNext. You should alsonormally enable CMD_BOOTEFI_BOOTMGR so that the command is available.choice prompt "Store for non-volatile UEFI variables"
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 8d31fc61c601..0a2cb6e3c476 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -42,7 +42,7 @@ targets += initrddump.o endif
obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o -obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o +obj-$(CONFIG_BOOTEFI_BOOTMGR) += efi_bootmgr.o obj-y += efi_boottime.o obj-y += efi_helper.o obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
This patch looks wrong.
Symbol CONFIG_CMD_BOOTEFI_BOOTMGR is used in a lot of places where it is not related to the 'bootefi bootmgr' subcommand.
I see no benefit in two separate symbols. If you want to rename the symbol, please, replace *all* occurrences:
%s/CONFIG_CMD_BOOTEFI_BOOTMGR/CONFIG_BOOTEFI_BOOTMGR/
Best regards
Heinrich
On Thu, Oct 19, 2023 at 05:16:28PM +0200, Heinrich Schuchardt wrote:
On 19.10.23 17:00, Tom Rini wrote:
From: Simon Glass sjg@chromium.org
The command should not be used to enable library functionality. Add a new BOOTEFI_BOOTMGR Kconfig for that. Adjust the conditions so that the same code is built.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org
Cc: Heinrich Schuchardt heinrich.schuchardt@canonical.com Cc: Ilias Apalodimas ilias.apalodimas@linaro.org Changes in v4:
- Integrate AKASHI Takahiro's feedback from v3
- Reword the help text on CMD_BOOTEFI_BOOTMGR slightly
cmd/Kconfig | 11 ++++++++++- lib/efi_loader/Kconfig | 6 +++--- lib/efi_loader/Makefile | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index 16e5cb8f0633..872cb49150cc 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -379,6 +379,15 @@ config CMD_BOOTEFI help Boot an EFI image from memory. +config CMD_BOOTEFI_BOOTMGR
- bool "UEFI Boot Manager command"
- depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
- default y
- help
Select this option to enable the 'bootmgr' subcommand of 'bootefi'.This subcommand will allow you to select the UEFI binary to be bootedvia UEFI variables Boot####, BootOrder, and BootNext.- config CMD_BOOTEFI_HELLO_COMPILE bool "Compile a standard EFI hello world binary for testing" depends on CMD_BOOTEFI && !CPU_V7M
@@ -2110,7 +2119,7 @@ config CMD_EFIDEBUG config CMD_EFICONFIG bool "eficonfig - provide menu-driven uefi variables maintenance interface" default y if !HAS_BOARD_SIZE_LIMIT
- depends on CMD_BOOTEFI_BOOTMGR
- depends on BOOTEFI_BOOTMGR select MENU help Enable the 'eficonfig' command which provides the menu-driven UEFI
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index d20aaab6dba4..13cad6342c36 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -32,14 +32,14 @@ config EFI_LOADER if EFI_LOADER -config CMD_BOOTEFI_BOOTMGR +config BOOTEFI_BOOTMGR bool "UEFI Boot Manager" default y select BOOTMETH_GLOBAL if BOOTSTD help Select this option if you want to select the UEFI binary to be booted
via UEFI variables Boot####, BootOrder, and BootNext. This enables the'bootefi bootmgr' command.
via UEFI variables Boot####, BootOrder, and BootNext. You should alsonormally enable CMD_BOOTEFI_BOOTMGR so that the command is available.diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 8d31fc61c601..0a2cb6e3c476 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -42,7 +42,7 @@ targets += initrddump.o endif obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o -obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o +obj-$(CONFIG_BOOTEFI_BOOTMGR) += efi_bootmgr.o obj-y += efi_boottime.o obj-y += efi_helper.o obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
This patch looks wrong.
Symbol CONFIG_CMD_BOOTEFI_BOOTMGR is used in a lot of places where it is not related to the 'bootefi bootmgr' subcommand.
I see no benefit in two separate symbols. If you want to rename the symbol, please, replace *all* occurrences:
%s/CONFIG_CMD_BOOTEFI_BOOTMGR/CONFIG_BOOTEFI_BOOTMGR/
Yes, there's work on the EFI_LOADER side of things to support the use case of "boot to menu" (or, "boot to efi bootmgr") of which this is the starting point. The follow-up work that I'm hoping you or someone else with more EFI_LOADER experience will pick up is splitting cmd/bootefi.c such that we can call in to starting an EFI payload (or bootmgr) without the command line.
On 19.10.23 17:19, Tom Rini wrote:
On Thu, Oct 19, 2023 at 05:16:28PM +0200, Heinrich Schuchardt wrote:
On 19.10.23 17:00, Tom Rini wrote:
From: Simon Glass sjg@chromium.org
The command should not be used to enable library functionality. Add a new BOOTEFI_BOOTMGR Kconfig for that. Adjust the conditions so that the same code is built.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org
Cc: Heinrich Schuchardt heinrich.schuchardt@canonical.com Cc: Ilias Apalodimas ilias.apalodimas@linaro.org Changes in v4:
- Integrate AKASHI Takahiro's feedback from v3
- Reword the help text on CMD_BOOTEFI_BOOTMGR slightly
cmd/Kconfig | 11 ++++++++++- lib/efi_loader/Kconfig | 6 +++--- lib/efi_loader/Makefile | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index 16e5cb8f0633..872cb49150cc 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -379,6 +379,15 @@ config CMD_BOOTEFI help Boot an EFI image from memory. +config CMD_BOOTEFI_BOOTMGR
- bool "UEFI Boot Manager command"
- depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
- default y
- help
Select this option to enable the 'bootmgr' subcommand of 'bootefi'.This subcommand will allow you to select the UEFI binary to be bootedvia UEFI variables Boot####, BootOrder, and BootNext.- config CMD_BOOTEFI_HELLO_COMPILE bool "Compile a standard EFI hello world binary for testing" depends on CMD_BOOTEFI && !CPU_V7M
@@ -2110,7 +2119,7 @@ config CMD_EFIDEBUG config CMD_EFICONFIG bool "eficonfig - provide menu-driven uefi variables maintenance interface" default y if !HAS_BOARD_SIZE_LIMIT
- depends on CMD_BOOTEFI_BOOTMGR
- depends on BOOTEFI_BOOTMGR select MENU help Enable the 'eficonfig' command which provides the menu-driven UEFI
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index d20aaab6dba4..13cad6342c36 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -32,14 +32,14 @@ config EFI_LOADER if EFI_LOADER -config CMD_BOOTEFI_BOOTMGR +config BOOTEFI_BOOTMGR bool "UEFI Boot Manager" default y select BOOTMETH_GLOBAL if BOOTSTD help Select this option if you want to select the UEFI binary to be booted
via UEFI variables Boot####, BootOrder, and BootNext. This enables the'bootefi bootmgr' command.
via UEFI variables Boot####, BootOrder, and BootNext. You should alsonormally enable CMD_BOOTEFI_BOOTMGR so that the command is available.diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 8d31fc61c601..0a2cb6e3c476 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -42,7 +42,7 @@ targets += initrddump.o endif obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o -obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o +obj-$(CONFIG_BOOTEFI_BOOTMGR) += efi_bootmgr.o obj-y += efi_boottime.o obj-y += efi_helper.o obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
This patch looks wrong.
Symbol CONFIG_CMD_BOOTEFI_BOOTMGR is used in a lot of places where it is not related to the 'bootefi bootmgr' subcommand.
I see no benefit in two separate symbols. If you want to rename the symbol, please, replace *all* occurrences:
%s/CONFIG_CMD_BOOTEFI_BOOTMGR/CONFIG_BOOTEFI_BOOTMGR/
Yes, there's work on the EFI_LOADER side of things to support the use case of "boot to menu" (or, "boot to efi bootmgr") of which this is the starting point. The follow-up work that I'm hoping you or someone else with more EFI_LOADER experience will pick up is splitting cmd/bootefi.c such that we can call in to starting an EFI payload (or bootmgr) without the command line.
Even after factoring out the boot functionality I would not know why we should have two separate symbols. I am fine with a rename which makes it clear that this symbol is about a library functionality.
Best regards
Heinrich
On Thu, Oct 19, 2023 at 05:24:33PM +0200, Heinrich Schuchardt wrote:
On 19.10.23 17:19, Tom Rini wrote:
On Thu, Oct 19, 2023 at 05:16:28PM +0200, Heinrich Schuchardt wrote:
On 19.10.23 17:00, Tom Rini wrote:
From: Simon Glass sjg@chromium.org
The command should not be used to enable library functionality. Add a new BOOTEFI_BOOTMGR Kconfig for that. Adjust the conditions so that the same code is built.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org
Cc: Heinrich Schuchardt heinrich.schuchardt@canonical.com Cc: Ilias Apalodimas ilias.apalodimas@linaro.org Changes in v4:
- Integrate AKASHI Takahiro's feedback from v3
- Reword the help text on CMD_BOOTEFI_BOOTMGR slightly
cmd/Kconfig | 11 ++++++++++- lib/efi_loader/Kconfig | 6 +++--- lib/efi_loader/Makefile | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index 16e5cb8f0633..872cb49150cc 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -379,6 +379,15 @@ config CMD_BOOTEFI help Boot an EFI image from memory. +config CMD_BOOTEFI_BOOTMGR
- bool "UEFI Boot Manager command"
- depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
- default y
- help
Select this option to enable the 'bootmgr' subcommand of 'bootefi'.This subcommand will allow you to select the UEFI binary to be bootedvia UEFI variables Boot####, BootOrder, and BootNext.- config CMD_BOOTEFI_HELLO_COMPILE bool "Compile a standard EFI hello world binary for testing" depends on CMD_BOOTEFI && !CPU_V7M
@@ -2110,7 +2119,7 @@ config CMD_EFIDEBUG config CMD_EFICONFIG bool "eficonfig - provide menu-driven uefi variables maintenance interface" default y if !HAS_BOARD_SIZE_LIMIT
- depends on CMD_BOOTEFI_BOOTMGR
- depends on BOOTEFI_BOOTMGR select MENU help Enable the 'eficonfig' command which provides the menu-driven UEFI
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index d20aaab6dba4..13cad6342c36 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -32,14 +32,14 @@ config EFI_LOADER if EFI_LOADER -config CMD_BOOTEFI_BOOTMGR +config BOOTEFI_BOOTMGR bool "UEFI Boot Manager" default y select BOOTMETH_GLOBAL if BOOTSTD help Select this option if you want to select the UEFI binary to be booted
via UEFI variables Boot####, BootOrder, and BootNext. This enables the'bootefi bootmgr' command.
via UEFI variables Boot####, BootOrder, and BootNext. You should alsonormally enable CMD_BOOTEFI_BOOTMGR so that the command is available.diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 8d31fc61c601..0a2cb6e3c476 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -42,7 +42,7 @@ targets += initrddump.o endif obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o -obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o +obj-$(CONFIG_BOOTEFI_BOOTMGR) += efi_bootmgr.o obj-y += efi_boottime.o obj-y += efi_helper.o obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
This patch looks wrong.
Symbol CONFIG_CMD_BOOTEFI_BOOTMGR is used in a lot of places where it is not related to the 'bootefi bootmgr' subcommand.
I see no benefit in two separate symbols. If you want to rename the symbol, please, replace *all* occurrences:
%s/CONFIG_CMD_BOOTEFI_BOOTMGR/CONFIG_BOOTEFI_BOOTMGR/
Yes, there's work on the EFI_LOADER side of things to support the use case of "boot to menu" (or, "boot to efi bootmgr") of which this is the starting point. The follow-up work that I'm hoping you or someone else with more EFI_LOADER experience will pick up is splitting cmd/bootefi.c such that we can call in to starting an EFI payload (or bootmgr) without the command line.
Even after factoring out the boot functionality I would not know why we should have two separate symbols. I am fine with a rename which makes it clear that this symbol is about a library functionality.
Because there's the library functionality and there's the literal command code. If you're arguing that long term we should have the command side of all library functionality be automatic based on CMDLINE=y/CMDLINE=n, that's another discussion for later. This just start the split. I had made a very very rough pass at starting the split:
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 20e5c94a33a4..fe4f3b4fe4bc 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -27,379 +27,6 @@ #include <asm-generic/sections.h> #include <linux/linkage.h>
-DECLARE_GLOBAL_DATA_PTR; - -static struct efi_device_path *bootefi_image_path; -static struct efi_device_path *bootefi_device_path; -static void *image_addr; -static size_t image_size; - -/** - * efi_get_image_parameters() - return image parameters - * - * @img_addr: address of loaded image in memory - * @img_size: size of loaded image - */ -void efi_get_image_parameters(void **img_addr, size_t *img_size) -{ - *img_addr = image_addr; - *img_size = image_size; -} - -/** - * efi_clear_bootdev() - clear boot device - */ -static void efi_clear_bootdev(void) -{ - efi_free_pool(bootefi_device_path); - efi_free_pool(bootefi_image_path); - bootefi_device_path = NULL; - bootefi_image_path = NULL; - image_addr = NULL; - image_size = 0; -} - -/** - * efi_set_bootdev() - set boot device - * - * This function is called when a file is loaded, e.g. via the 'load' command. - * We use the path to this file to inform the UEFI binary about the boot device. - * - * @dev: device, e.g. "MMC" - * @devnr: number of the device, e.g. "1:2" - * @path: path to file loaded - * @buffer: buffer with file loaded - * @buffer_size: size of file loaded - */ -void efi_set_bootdev(const char *dev, const char *devnr, const char *path, - void *buffer, size_t buffer_size) -{ - struct efi_device_path *device, *image; - efi_status_t ret; - - log_debug("dev=%s, devnr=%s, path=%s, buffer=%p, size=%zx\n", dev, - devnr, path, buffer, buffer_size); - - /* Forget overwritten image */ - if (buffer + buffer_size >= image_addr && - image_addr + image_size >= buffer) - efi_clear_bootdev(); - - /* Remember only PE-COFF and FIT images */ - if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) { - if (IS_ENABLED(CONFIG_FIT) && - !fit_check_format(buffer, IMAGE_SIZE_INVAL)) { - /* - * FIT images of type EFI_OS are started via command - * bootm. We should not use their boot device with the - * bootefi command. - */ - buffer = 0; - buffer_size = 0; - } else { - log_debug("- not remembering image\n"); - return; - } - } - - /* efi_set_bootdev() is typically called repeatedly, recover memory */ - efi_clear_bootdev(); - - image_addr = buffer; - image_size = buffer_size; - - ret = efi_dp_from_name(dev, devnr, path, &device, &image); - if (ret == EFI_SUCCESS) { - bootefi_device_path = device; - if (image) { - /* FIXME: image should not contain device */ - struct efi_device_path *image_tmp = image; - - efi_dp_split_file_path(image, &device, &image); - efi_free_pool(image_tmp); - } - bootefi_image_path = image; - log_debug("- boot device %pD\n", device); - if (image) - log_debug("- image %pD\n", image); - } else { - log_debug("- efi_dp_from_name() failed, err=%lx\n", ret); - efi_clear_bootdev(); - } -} - -/** - * efi_env_set_load_options() - set load options from environment variable - * - * @handle: the image handle - * @env_var: name of the environment variable - * @load_options: pointer to load options (output) - * Return: status code - */ -static efi_status_t efi_env_set_load_options(efi_handle_t handle, - const char *env_var, - u16 **load_options) -{ - const char *env = env_get(env_var); - size_t size; - u16 *pos; - efi_status_t ret; - - *load_options = NULL; - if (!env) - return EFI_SUCCESS; - size = sizeof(u16) * (utf8_utf16_strlen(env) + 1); - pos = calloc(size, 1); - if (!pos) - return EFI_OUT_OF_RESOURCES; - *load_options = pos; - utf8_utf16_strcpy(&pos, env); - ret = efi_set_load_options(handle, size, *load_options); - if (ret != EFI_SUCCESS) { - free(*load_options); - *load_options = NULL; - } - return ret; -} - -#if !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) - -/** - * copy_fdt() - Copy the device tree to a new location available to EFI - * - * The FDT is copied to a suitable location within the EFI memory map. - * Additional 12 KiB are added to the space in case the device tree needs to be - * expanded later with fdt_open_into(). - * - * @fdtp: On entry a pointer to the flattened device tree. - * On exit a pointer to the copy of the flattened device tree. - * FDT start - * Return: status code - */ -static efi_status_t copy_fdt(void **fdtp) -{ - unsigned long fdt_ram_start = -1L, fdt_pages; - efi_status_t ret = 0; - void *fdt, *new_fdt; - u64 new_fdt_addr; - uint fdt_size; - int i; - - for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { - u64 ram_start = gd->bd->bi_dram[i].start; - u64 ram_size = gd->bd->bi_dram[i].size; - - if (!ram_size) - continue; - - if (ram_start < fdt_ram_start) - fdt_ram_start = ram_start; - } - - /* - * Give us at least 12 KiB of breathing room in case the device tree - * needs to be expanded later. - */ - fdt = *fdtp; - fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000); - fdt_size = fdt_pages << EFI_PAGE_SHIFT; - - ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, - EFI_ACPI_RECLAIM_MEMORY, fdt_pages, - &new_fdt_addr); - if (ret != EFI_SUCCESS) { - log_err("ERROR: Failed to reserve space for FDT\n"); - goto done; - } - new_fdt = (void *)(uintptr_t)new_fdt_addr; - memcpy(new_fdt, fdt, fdt_totalsize(fdt)); - fdt_set_totalsize(new_fdt, fdt_size); - - *fdtp = (void *)(uintptr_t)new_fdt_addr; -done: - return ret; -} - -/** - * get_config_table() - get configuration table - * - * @guid: GUID of the configuration table - * Return: pointer to configuration table or NULL - */ -static void *get_config_table(const efi_guid_t *guid) -{ - size_t i; - - for (i = 0; i < systab.nr_tables; i++) { - if (!guidcmp(guid, &systab.tables[i].guid)) - return systab.tables[i].table; - } - return NULL; -} - -#endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */ - -/** - * efi_install_fdt() - install device tree - * - * If fdt is not EFI_FDT_USE_INTERNAL, the device tree located at that memory - * address will will be installed as configuration table, otherwise the device - * tree located at the address indicated by environment variable fdt_addr or as - * fallback fdtcontroladdr will be used. - * - * On architectures using ACPI tables device trees shall not be installed as - * configuration table. - * - * @fdt: address of device tree or EFI_FDT_USE_INTERNAL to use the - * the hardware device tree as indicated by environment variable - * fdt_addr or as fallback the internal device tree as indicated by - * the environment variable fdtcontroladdr - * Return: status code - */ -efi_status_t efi_install_fdt(void *fdt) -{ - /* - * The EBBR spec requires that we have either an FDT or an ACPI table - * but not both. - */ -#if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) - if (fdt) { - log_warning("WARNING: Can't have ACPI table and device tree - ignoring DT.\n"); - return EFI_SUCCESS; - } -#else - struct bootm_headers img = { 0 }; - efi_status_t ret; - - if (fdt == EFI_FDT_USE_INTERNAL) { - const char *fdt_opt; - uintptr_t fdt_addr; - - /* Look for device tree that is already installed */ - if (get_config_table(&efi_guid_fdt)) - return EFI_SUCCESS; - /* Check if there is a hardware device tree */ - fdt_opt = env_get("fdt_addr"); - /* Use our own device tree as fallback */ - if (!fdt_opt) { - fdt_opt = env_get("fdtcontroladdr"); - if (!fdt_opt) { - log_err("ERROR: need device tree\n"); - return EFI_NOT_FOUND; - } - } - fdt_addr = hextoul(fdt_opt, NULL); - if (!fdt_addr) { - log_err("ERROR: invalid $fdt_addr or $fdtcontroladdr\n"); - return EFI_LOAD_ERROR; - } - fdt = map_sysmem(fdt_addr, 0); - } - - /* Install device tree */ - if (fdt_check_header(fdt)) { - log_err("ERROR: invalid device tree\n"); - return EFI_LOAD_ERROR; - } - - /* Prepare device tree for payload */ - ret = copy_fdt(&fdt); - if (ret) { - log_err("ERROR: out of memory\n"); - return EFI_OUT_OF_RESOURCES; - } - - if (image_setup_libfdt(&img, fdt, 0, NULL)) { - log_err("ERROR: failed to process device tree\n"); - return EFI_LOAD_ERROR; - } - - /* Create memory reservations as indicated by the device tree */ - efi_carve_out_dt_rsv(fdt); - - efi_try_purge_kaslr_seed(fdt); - - if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) { - ret = efi_tcg2_measure_dtb(fdt); - if (ret == EFI_SECURITY_VIOLATION) { - log_err("ERROR: failed to measure DTB\n"); - return ret; - } - } - - /* Install device tree as UEFI table */ - ret = efi_install_configuration_table(&efi_guid_fdt, fdt); - if (ret != EFI_SUCCESS) { - log_err("ERROR: failed to install device tree\n"); - return ret; - } -#endif /* GENERATE_ACPI_TABLE */ - - return EFI_SUCCESS; -} - -/** - * do_bootefi_exec() - execute EFI binary - * - * The image indicated by @handle is started. When it returns the allocated - * memory for the @load_options is freed. - * - * @handle: handle of loaded image - * @load_options: load options - * Return: status code - * - * Load the EFI binary into a newly assigned memory unwinding the relocation - * information, install the loaded image protocol, and call the binary. - */ -static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options) -{ - efi_status_t ret; - efi_uintn_t exit_data_size = 0; - u16 *exit_data = NULL; - - /* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */ - switch_to_non_secure_mode(); - - /* - * The UEFI standard requires that the watchdog timer is set to five - * minutes when invoking an EFI boot option. - * - * Unified Extensible Firmware Interface (UEFI), version 2.7 Errata A - * 7.5. Miscellaneous Boot Services - EFI_BOOT_SERVICES.SetWatchdogTimer - */ - ret = efi_set_watchdog(300); - if (ret != EFI_SUCCESS) { - log_err("ERROR: Failed to set watchdog timer\n"); - goto out; - } - - /* Call our payload! */ - ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data)); - if (ret != EFI_SUCCESS) { - log_err("## Application failed, r = %lu\n", - ret & ~EFI_ERROR_MASK); - if (exit_data) { - log_err("## %ls\n", exit_data); - efi_free_pool(exit_data); - } - } - - efi_restore_gd(); - -out: - free(load_options); - - if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) { - if (efi_initrd_deregister() != EFI_SUCCESS) - log_err("Failed to remove loadfile2 for initrd\n"); - } - - /* Control is returned to U-Boot, disable EFI watchdog */ - efi_set_watchdog(0); - - return ret; -} - /** * do_efibootmgr() - execute EFI boot manager * @@ -461,6 +88,9 @@ static int do_bootefi_image(const char *image_opt, const char *size_opt) return CMD_RET_USAGE; efi_clear_bootdev(); } else { + size_t image_size; + void *image_addr; + efi_get_image_parameters(&image_addr, &image_size); if (image_buf != image_addr) { log_err("No UEFI binary known at %s\n", image_opt); @@ -477,135 +107,7 @@ static int do_bootefi_image(const char *image_opt, const char *size_opt) return CMD_RET_SUCCESS; }
-/** - * efi_run_image() - run loaded UEFI image - * - * @source_buffer: memory address of the UEFI image - * @source_size: size of the UEFI image - * Return: status code - */ -efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) -{ - efi_handle_t mem_handle = NULL, handle; - struct efi_device_path *file_path = NULL; - struct efi_device_path *msg_path; - efi_status_t ret, ret2; - u16 *load_options; - - if (!bootefi_device_path || !bootefi_image_path) { - log_debug("Not loaded from disk\n"); - /* - * Special case for efi payload not loaded from disk, - * such as 'bootefi hello' or for example payload - * loaded directly into memory via JTAG, etc: - */ - file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, - (uintptr_t)source_buffer, - source_size); - /* - * Make sure that device for device_path exist - * in load_image(). Otherwise, shell and grub will fail. - */ - ret = efi_install_multiple_protocol_interfaces(&mem_handle, - &efi_guid_device_path, - file_path, NULL); - if (ret != EFI_SUCCESS) - goto out; - msg_path = file_path; - } else { - file_path = efi_dp_append(bootefi_device_path, - bootefi_image_path); - msg_path = bootefi_image_path; - log_debug("Loaded from disk\n"); - } - - log_info("Booting %pD\n", msg_path); - - ret = EFI_CALL(efi_load_image(false, efi_root, file_path, source_buffer, - source_size, &handle)); - if (ret != EFI_SUCCESS) { - log_err("Loading image failed\n"); - goto out; - } - - /* Transfer environment variable as load options */ - ret = efi_env_set_load_options(handle, "bootargs", &load_options); - if (ret != EFI_SUCCESS) - goto out; - - ret = do_bootefi_exec(handle, load_options); - -out: - ret2 = efi_uninstall_multiple_protocol_interfaces(mem_handle, - &efi_guid_device_path, - file_path, NULL); - efi_free_pool(file_path); - return (ret != EFI_SUCCESS) ? ret : ret2; -} - #ifdef CONFIG_CMD_BOOTEFI_SELFTEST -static efi_status_t bootefi_run_prepare(const char *load_options_path, - struct efi_device_path *device_path, - struct efi_device_path *image_path, - struct efi_loaded_image_obj **image_objp, - struct efi_loaded_image **loaded_image_infop) -{ - efi_status_t ret; - u16 *load_options; - - ret = efi_setup_loaded_image(device_path, image_path, image_objp, - loaded_image_infop); - if (ret != EFI_SUCCESS) - return ret; - - /* Transfer environment variable as load options */ - return efi_env_set_load_options((efi_handle_t)*image_objp, - load_options_path, - &load_options); -} - -/** - * bootefi_test_prepare() - prepare to run an EFI test - * - * Prepare to run a test as if it were provided by a loaded image. - * - * @image_objp: pointer to be set to the loaded image handle - * @loaded_image_infop: pointer to be set to the loaded image protocol - * @path: dummy file path used to construct the device path - * set in the loaded image protocol - * @load_options_path: name of a U-Boot environment variable. Its value is - * set as load options in the loaded image protocol. - * Return: status code - */ -static efi_status_t bootefi_test_prepare - (struct efi_loaded_image_obj **image_objp, - struct efi_loaded_image **loaded_image_infop, const char *path, - const char *load_options_path) -{ - efi_status_t ret; - - /* Construct a dummy device path */ - bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0); - if (!bootefi_device_path) - return EFI_OUT_OF_RESOURCES; - - bootefi_image_path = efi_dp_from_file(NULL, path); - if (!bootefi_image_path) { - ret = EFI_OUT_OF_RESOURCES; - goto failure; - } - - ret = bootefi_run_prepare(load_options_path, bootefi_device_path, - bootefi_image_path, image_objp, - loaded_image_infop); - if (ret == EFI_SUCCESS) - return ret; - -failure: - efi_clear_bootdev(); - return ret; -} - /** * do_efi_selftest() - execute EFI selftest * diff --git a/include/efi_loader.h b/include/efi_loader.h index e24410505f40..6663a972c124 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -91,6 +91,7 @@ efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len); * back to u-boot world */ void efi_restore_gd(void); +void efi_clear_bootdev(void); /* Call this to set the current device name */ void efi_set_bootdev(const char *dev, const char *devnr, const char *path, void *buffer, size_t buffer_size); @@ -526,12 +527,15 @@ efi_status_t efi_bootmgr_get_unused_bootoption(u16 *buf, efi_status_t efi_bootmgr_update_media_device_boot_option(void); /* Delete selected boot option */ efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index); +efi_status_t efi_env_set_load_options(efi_handle_t handle, const char *env_var, + u16 **load_options); /* search the boot option index in BootOrder */ bool efi_search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *index); /* Set up console modes */ void efi_setup_console_size(void); /* Install device tree */ efi_status_t efi_install_fdt(void *fdt); +efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options); /* Run loaded UEFI image */ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size); /* Initialize variable services */ @@ -885,6 +889,9 @@ efi_status_t __efi_runtime EFIAPI efi_set_time(struct efi_time *time); */ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle, struct efi_system_table *systab); +efi_status_t bootefi_test_prepare (struct efi_loaded_image_obj **image_objp, + struct efi_loaded_image **loaded_image_infop, const char *path, + const char *load_options_path); #endif
efi_status_t EFIAPI efi_get_variable(u16 *variable_name, diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 8d31fc61c601..529a9756a98c 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -44,6 +44,7 @@ endif obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o obj-y += efi_boottime.o +obj-y += bootefi.o obj-y += efi_helper.o obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o diff --git a/lib/efi_loader/bootefi.c b/lib/efi_loader/bootefi.c new file mode 100644 index 000000000000..7b02b6f46eb5 --- /dev/null +++ b/lib/efi_loader/bootefi.c @@ -0,0 +1,530 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * EFI application loader + * + * Copyright (c) 2016 Alexander Graf + */ + +#define LOG_CATEGORY LOGC_EFI + +#include <common.h> +#include <bootm.h> +#include <charset.h> +#include <command.h> +#include <dm.h> +#include <efi_loader.h> +#include <efi_selftest.h> +#include <env.h> +#include <errno.h> +#include <image.h> +#include <log.h> +#include <malloc.h> +#include <asm/global_data.h> +#include <linux/libfdt.h> +#include <linux/libfdt_env.h> +#include <mapmem.h> +#include <memalign.h> +#include <asm-generic/sections.h> +#include <linux/linkage.h> + +DECLARE_GLOBAL_DATA_PTR; + +static struct efi_device_path *bootefi_image_path; +static struct efi_device_path *bootefi_device_path; +static void *image_addr; +static size_t image_size; + +/** + * efi_get_image_parameters() - return image parameters + * + * @img_addr: address of loaded image in memory + * @img_size: size of loaded image + */ +void efi_get_image_parameters(void **img_addr, size_t *img_size) +{ + *img_addr = image_addr; + *img_size = image_size; +} + +/** + * efi_clear_bootdev() - clear boot device + */ +void efi_clear_bootdev(void) +{ + efi_free_pool(bootefi_device_path); + efi_free_pool(bootefi_image_path); + bootefi_device_path = NULL; + bootefi_image_path = NULL; + image_addr = NULL; + image_size = 0; +} + +/** + * efi_set_bootdev() - set boot device + * + * This function is called when a file is loaded, e.g. via the 'load' command. + * We use the path to this file to inform the UEFI binary about the boot device. + * + * @dev: device, e.g. "MMC" + * @devnr: number of the device, e.g. "1:2" + * @path: path to file loaded + * @buffer: buffer with file loaded + * @buffer_size: size of file loaded + */ +void efi_set_bootdev(const char *dev, const char *devnr, const char *path, + void *buffer, size_t buffer_size) +{ + struct efi_device_path *device, *image; + efi_status_t ret; + + log_debug("dev=%s, devnr=%s, path=%s, buffer=%p, size=%zx\n", dev, + devnr, path, buffer, buffer_size); + + /* Forget overwritten image */ + if (buffer + buffer_size >= image_addr && + image_addr + image_size >= buffer) + efi_clear_bootdev(); + + /* Remember only PE-COFF and FIT images */ + if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) { + if (IS_ENABLED(CONFIG_FIT) && + !fit_check_format(buffer, IMAGE_SIZE_INVAL)) { + /* + * FIT images of type EFI_OS are started via command + * bootm. We should not use their boot device with the + * bootefi command. + */ + buffer = 0; + buffer_size = 0; + } else { + log_debug("- not remembering image\n"); + return; + } + } + + /* efi_set_bootdev() is typically called repeatedly, recover memory */ + efi_clear_bootdev(); + + image_addr = buffer; + image_size = buffer_size; + + ret = efi_dp_from_name(dev, devnr, path, &device, &image); + if (ret == EFI_SUCCESS) { + bootefi_device_path = device; + if (image) { + /* FIXME: image should not contain device */ + struct efi_device_path *image_tmp = image; + + efi_dp_split_file_path(image, &device, &image); + efi_free_pool(image_tmp); + } + bootefi_image_path = image; + log_debug("- boot device %pD\n", device); + if (image) + log_debug("- image %pD\n", image); + } else { + log_debug("- efi_dp_from_name() failed, err=%lx\n", ret); + efi_clear_bootdev(); + } +} + +/** + * efi_env_set_load_options() - set load options from environment variable + * + * @handle: the image handle + * @env_var: name of the environment variable + * @load_options: pointer to load options (output) + * Return: status code + */ +efi_status_t efi_env_set_load_options(efi_handle_t handle, + const char *env_var, + u16 **load_options) +{ + const char *env = env_get(env_var); + size_t size; + u16 *pos; + efi_status_t ret; + + *load_options = NULL; + if (!env) + return EFI_SUCCESS; + size = sizeof(u16) * (utf8_utf16_strlen(env) + 1); + pos = calloc(size, 1); + if (!pos) + return EFI_OUT_OF_RESOURCES; + *load_options = pos; + utf8_utf16_strcpy(&pos, env); + ret = efi_set_load_options(handle, size, *load_options); + if (ret != EFI_SUCCESS) { + free(*load_options); + *load_options = NULL; + } + return ret; +} + +#if !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) + +/** + * copy_fdt() - Copy the device tree to a new location available to EFI + * + * The FDT is copied to a suitable location within the EFI memory map. + * Additional 12 KiB are added to the space in case the device tree needs to be + * expanded later with fdt_open_into(). + * + * @fdtp: On entry a pointer to the flattened device tree. + * On exit a pointer to the copy of the flattened device tree. + * FDT start + * Return: status code + */ +static efi_status_t copy_fdt(void **fdtp) +{ + unsigned long fdt_ram_start = -1L, fdt_pages; + efi_status_t ret = 0; + void *fdt, *new_fdt; + u64 new_fdt_addr; + uint fdt_size; + int i; + + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { + u64 ram_start = gd->bd->bi_dram[i].start; + u64 ram_size = gd->bd->bi_dram[i].size; + + if (!ram_size) + continue; + + if (ram_start < fdt_ram_start) + fdt_ram_start = ram_start; + } + + /* + * Give us at least 12 KiB of breathing room in case the device tree + * needs to be expanded later. + */ + fdt = *fdtp; + fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000); + fdt_size = fdt_pages << EFI_PAGE_SHIFT; + + ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, + EFI_ACPI_RECLAIM_MEMORY, fdt_pages, + &new_fdt_addr); + if (ret != EFI_SUCCESS) { + log_err("ERROR: Failed to reserve space for FDT\n"); + goto done; + } + new_fdt = (void *)(uintptr_t)new_fdt_addr; + memcpy(new_fdt, fdt, fdt_totalsize(fdt)); + fdt_set_totalsize(new_fdt, fdt_size); + + *fdtp = (void *)(uintptr_t)new_fdt_addr; +done: + return ret; +} + +/** + * get_config_table() - get configuration table + * + * @guid: GUID of the configuration table + * Return: pointer to configuration table or NULL + */ +static void *get_config_table(const efi_guid_t *guid) +{ + size_t i; + + for (i = 0; i < systab.nr_tables; i++) { + if (!guidcmp(guid, &systab.tables[i].guid)) + return systab.tables[i].table; + } + return NULL; +} + +#endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */ + +/** + * efi_install_fdt() - install device tree + * + * If fdt is not EFI_FDT_USE_INTERNAL, the device tree located at that memory + * address will will be installed as configuration table, otherwise the device + * tree located at the address indicated by environment variable fdt_addr or as + * fallback fdtcontroladdr will be used. + * + * On architectures using ACPI tables device trees shall not be installed as + * configuration table. + * + * @fdt: address of device tree or EFI_FDT_USE_INTERNAL to use the + * the hardware device tree as indicated by environment variable + * fdt_addr or as fallback the internal device tree as indicated by + * the environment variable fdtcontroladdr + * Return: status code + */ +efi_status_t efi_install_fdt(void *fdt) +{ + /* + * The EBBR spec requires that we have either an FDT or an ACPI table + * but not both. + */ +#if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) + if (fdt) { + log_warning("WARNING: Can't have ACPI table and device tree - ignoring DT.\n"); + return EFI_SUCCESS; + } +#else + struct bootm_headers img = { 0 }; + efi_status_t ret; + + if (fdt == EFI_FDT_USE_INTERNAL) { + const char *fdt_opt; + uintptr_t fdt_addr; + + /* Look for device tree that is already installed */ + if (get_config_table(&efi_guid_fdt)) + return EFI_SUCCESS; + /* Check if there is a hardware device tree */ + fdt_opt = env_get("fdt_addr"); + /* Use our own device tree as fallback */ + if (!fdt_opt) { + fdt_opt = env_get("fdtcontroladdr"); + if (!fdt_opt) { + log_err("ERROR: need device tree\n"); + return EFI_NOT_FOUND; + } + } + fdt_addr = hextoul(fdt_opt, NULL); + if (!fdt_addr) { + log_err("ERROR: invalid $fdt_addr or $fdtcontroladdr\n"); + return EFI_LOAD_ERROR; + } + fdt = map_sysmem(fdt_addr, 0); + } + + /* Install device tree */ + if (fdt_check_header(fdt)) { + log_err("ERROR: invalid device tree\n"); + return EFI_LOAD_ERROR; + } + + /* Prepare device tree for payload */ + ret = copy_fdt(&fdt); + if (ret) { + log_err("ERROR: out of memory\n"); + return EFI_OUT_OF_RESOURCES; + } + + if (image_setup_libfdt(&img, fdt, 0, NULL)) { + log_err("ERROR: failed to process device tree\n"); + return EFI_LOAD_ERROR; + } + + /* Create memory reservations as indicated by the device tree */ + efi_carve_out_dt_rsv(fdt); + + efi_try_purge_kaslr_seed(fdt); + + if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) { + ret = efi_tcg2_measure_dtb(fdt); + if (ret == EFI_SECURITY_VIOLATION) { + log_err("ERROR: failed to measure DTB\n"); + return ret; + } + } + + /* Install device tree as UEFI table */ + ret = efi_install_configuration_table(&efi_guid_fdt, fdt); + if (ret != EFI_SUCCESS) { + log_err("ERROR: failed to install device tree\n"); + return ret; + } +#endif /* GENERATE_ACPI_TABLE */ + + return EFI_SUCCESS; +} + +#ifdef CONFIG_CMD_BOOTEFI_SELFTEST +static efi_status_t bootefi_run_prepare(const char *load_options_path, + struct efi_device_path *device_path, + struct efi_device_path *image_path, + struct efi_loaded_image_obj **image_objp, + struct efi_loaded_image **loaded_image_infop) +{ + efi_status_t ret; + u16 *load_options; + + ret = efi_setup_loaded_image(device_path, image_path, image_objp, + loaded_image_infop); + if (ret != EFI_SUCCESS) + return ret; + + /* Transfer environment variable as load options */ + return efi_env_set_load_options((efi_handle_t)*image_objp, + load_options_path, + &load_options); +} + +/** + * bootefi_test_prepare() - prepare to run an EFI test + * + * Prepare to run a test as if it were provided by a loaded image. + * + * @image_objp: pointer to be set to the loaded image handle + * @loaded_image_infop: pointer to be set to the loaded image protocol + * @path: dummy file path used to construct the device path + * set in the loaded image protocol + * @load_options_path: name of a U-Boot environment variable. Its value is + * set as load options in the loaded image protocol. + * Return: status code + */ +efi_status_t bootefi_test_prepare (struct efi_loaded_image_obj **image_objp, + struct efi_loaded_image **loaded_image_infop, const char *path, + const char *load_options_path) +{ + efi_status_t ret; + + /* Construct a dummy device path */ + bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0); + if (!bootefi_device_path) + return EFI_OUT_OF_RESOURCES; + + bootefi_image_path = efi_dp_from_file(NULL, path); + if (!bootefi_image_path) { + ret = EFI_OUT_OF_RESOURCES; + goto failure; + } + + ret = bootefi_run_prepare(load_options_path, bootefi_device_path, + bootefi_image_path, image_objp, + loaded_image_infop); + if (ret == EFI_SUCCESS) + return ret; + +failure: + efi_clear_bootdev(); + return ret; +} +#endif + +/** + * do_bootefi_exec() - execute EFI binary + * + * The image indicated by @handle is started. When it returns the allocated + * memory for the @load_options is freed. + * + * @handle: handle of loaded image + * @load_options: load options + * Return: status code + * + * Load the EFI binary into a newly assigned memory unwinding the relocation + * information, install the loaded image protocol, and call the binary. + */ +efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options) +{ + efi_status_t ret; + efi_uintn_t exit_data_size = 0; + u16 *exit_data = NULL; + + /* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */ + switch_to_non_secure_mode(); + + /* + * The UEFI standard requires that the watchdog timer is set to five + * minutes when invoking an EFI boot option. + * + * Unified Extensible Firmware Interface (UEFI), version 2.7 Errata A + * 7.5. Miscellaneous Boot Services - EFI_BOOT_SERVICES.SetWatchdogTimer + */ + ret = efi_set_watchdog(300); + if (ret != EFI_SUCCESS) { + log_err("ERROR: Failed to set watchdog timer\n"); + goto out; + } + + /* Call our payload! */ + ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data)); + if (ret != EFI_SUCCESS) { + log_err("## Application failed, r = %lu\n", + ret & ~EFI_ERROR_MASK); + if (exit_data) { + log_err("## %ls\n", exit_data); + efi_free_pool(exit_data); + } + } + + efi_restore_gd(); + +out: + free(load_options); + + if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) { + if (efi_initrd_deregister() != EFI_SUCCESS) + log_err("Failed to remove loadfile2 for initrd\n"); + } + + /* Control is returned to U-Boot, disable EFI watchdog */ + efi_set_watchdog(0); + + return ret; +} + +/** + * efi_run_image() - run loaded UEFI image + * + * @source_buffer: memory address of the UEFI image + * @source_size: size of the UEFI image + * Return: status code + */ +efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) +{ + efi_handle_t mem_handle = NULL, handle; + struct efi_device_path *file_path = NULL; + struct efi_device_path *msg_path; + efi_status_t ret, ret2; + u16 *load_options; + + if (!bootefi_device_path || !bootefi_image_path) { + log_debug("Not loaded from disk\n"); + /* + * Special case for efi payload not loaded from disk, + * such as 'bootefi hello' or for example payload + * loaded directly into memory via JTAG, etc: + */ + file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, + (uintptr_t)source_buffer, + source_size); + /* + * Make sure that device for device_path exist + * in load_image(). Otherwise, shell and grub will fail. + */ + ret = efi_install_multiple_protocol_interfaces(&mem_handle, + &efi_guid_device_path, + file_path, NULL); + if (ret != EFI_SUCCESS) + goto out; + msg_path = file_path; + } else { + file_path = efi_dp_append(bootefi_device_path, + bootefi_image_path); + msg_path = bootefi_image_path; + log_debug("Loaded from disk\n"); + } + + log_info("Booting %pD\n", msg_path); + + ret = EFI_CALL(efi_load_image(false, efi_root, file_path, source_buffer, + source_size, &handle)); + if (ret != EFI_SUCCESS) { + log_err("Loading image failed\n"); + goto out; + } + + /* Transfer environment variable as load options */ + ret = efi_env_set_load_options(handle, "bootargs", &load_options); + if (ret != EFI_SUCCESS) + goto out; + + ret = do_bootefi_exec(handle, load_options); + +out: + ret2 = efi_uninstall_multiple_protocol_interfaces(mem_handle, + &efi_guid_device_path, + file_path, NULL); + efi_free_pool(file_path); + return (ret != EFI_SUCCESS) ? ret : ret2; +}
But that left needing to implement an API to start bootefi / bootefi bootmgr itself as we still end up in C making a string and calling the command, and ideally we would not be doing that with CMDLINE disabled.
On Thu, Oct 19, 2023 at 11:19:33AM -0400, Tom Rini wrote:
On Thu, Oct 19, 2023 at 05:16:28PM +0200, Heinrich Schuchardt wrote:
On 19.10.23 17:00, Tom Rini wrote:
From: Simon Glass sjg@chromium.org
The command should not be used to enable library functionality. Add a new BOOTEFI_BOOTMGR Kconfig for that. Adjust the conditions so that the same code is built.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org
Cc: Heinrich Schuchardt heinrich.schuchardt@canonical.com Cc: Ilias Apalodimas ilias.apalodimas@linaro.org Changes in v4:
- Integrate AKASHI Takahiro's feedback from v3
- Reword the help text on CMD_BOOTEFI_BOOTMGR slightly
cmd/Kconfig | 11 ++++++++++- lib/efi_loader/Kconfig | 6 +++--- lib/efi_loader/Makefile | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index 16e5cb8f0633..872cb49150cc 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -379,6 +379,15 @@ config CMD_BOOTEFI help Boot an EFI image from memory. +config CMD_BOOTEFI_BOOTMGR
- bool "UEFI Boot Manager command"
- depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
- default y
- help
Select this option to enable the 'bootmgr' subcommand of 'bootefi'.This subcommand will allow you to select the UEFI binary to be bootedvia UEFI variables Boot####, BootOrder, and BootNext.- config CMD_BOOTEFI_HELLO_COMPILE bool "Compile a standard EFI hello world binary for testing" depends on CMD_BOOTEFI && !CPU_V7M
@@ -2110,7 +2119,7 @@ config CMD_EFIDEBUG config CMD_EFICONFIG bool "eficonfig - provide menu-driven uefi variables maintenance interface" default y if !HAS_BOARD_SIZE_LIMIT
- depends on CMD_BOOTEFI_BOOTMGR
- depends on BOOTEFI_BOOTMGR select MENU help Enable the 'eficonfig' command which provides the menu-driven UEFI
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index d20aaab6dba4..13cad6342c36 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -32,14 +32,14 @@ config EFI_LOADER if EFI_LOADER -config CMD_BOOTEFI_BOOTMGR +config BOOTEFI_BOOTMGR bool "UEFI Boot Manager" default y select BOOTMETH_GLOBAL if BOOTSTD help Select this option if you want to select the UEFI binary to be booted
via UEFI variables Boot####, BootOrder, and BootNext. This enables the'bootefi bootmgr' command.
via UEFI variables Boot####, BootOrder, and BootNext. You should alsonormally enable CMD_BOOTEFI_BOOTMGR so that the command is available.diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 8d31fc61c601..0a2cb6e3c476 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -42,7 +42,7 @@ targets += initrddump.o endif obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o -obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o +obj-$(CONFIG_BOOTEFI_BOOTMGR) += efi_bootmgr.o obj-y += efi_boottime.o obj-y += efi_helper.o obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
This patch looks wrong.
Symbol CONFIG_CMD_BOOTEFI_BOOTMGR is used in a lot of places where it is not related to the 'bootefi bootmgr' subcommand.
I see no benefit in two separate symbols. If you want to rename the symbol, please, replace *all* occurrences:
%s/CONFIG_CMD_BOOTEFI_BOOTMGR/CONFIG_BOOTEFI_BOOTMGR/
Yes, there's work on the EFI_LOADER side of things to support the use case of "boot to menu" (or, "boot to efi bootmgr") of which this is the starting point. The follow-up work that I'm hoping you or someone else with more EFI_LOADER experience will pick up is splitting cmd/bootefi.c such that we can call in to starting an EFI payload (or bootmgr) without the command line.
I will be able to take care unless Heinrich wants to do by himself. # Test would be another issue now that "bootefi" may handle various boot scenarios.
-Takahiro Akashi
-- Tom
On Fri, Oct 20, 2023 at 09:21:00PM +0900, AKASHI Takahiro wrote:
On Thu, Oct 19, 2023 at 11:19:33AM -0400, Tom Rini wrote:
On Thu, Oct 19, 2023 at 05:16:28PM +0200, Heinrich Schuchardt wrote:
On 19.10.23 17:00, Tom Rini wrote:
From: Simon Glass sjg@chromium.org
The command should not be used to enable library functionality. Add a new BOOTEFI_BOOTMGR Kconfig for that. Adjust the conditions so that the same code is built.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: AKASHI Takahiro takahiro.akashi@linaro.org
Cc: Heinrich Schuchardt heinrich.schuchardt@canonical.com Cc: Ilias Apalodimas ilias.apalodimas@linaro.org Changes in v4:
- Integrate AKASHI Takahiro's feedback from v3
- Reword the help text on CMD_BOOTEFI_BOOTMGR slightly
cmd/Kconfig | 11 ++++++++++- lib/efi_loader/Kconfig | 6 +++--- lib/efi_loader/Makefile | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index 16e5cb8f0633..872cb49150cc 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -379,6 +379,15 @@ config CMD_BOOTEFI help Boot an EFI image from memory. +config CMD_BOOTEFI_BOOTMGR
- bool "UEFI Boot Manager command"
- depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
- default y
- help
Select this option to enable the 'bootmgr' subcommand of 'bootefi'.This subcommand will allow you to select the UEFI binary to be bootedvia UEFI variables Boot####, BootOrder, and BootNext.- config CMD_BOOTEFI_HELLO_COMPILE bool "Compile a standard EFI hello world binary for testing" depends on CMD_BOOTEFI && !CPU_V7M
@@ -2110,7 +2119,7 @@ config CMD_EFIDEBUG config CMD_EFICONFIG bool "eficonfig - provide menu-driven uefi variables maintenance interface" default y if !HAS_BOARD_SIZE_LIMIT
- depends on CMD_BOOTEFI_BOOTMGR
- depends on BOOTEFI_BOOTMGR select MENU help Enable the 'eficonfig' command which provides the menu-driven UEFI
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index d20aaab6dba4..13cad6342c36 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -32,14 +32,14 @@ config EFI_LOADER if EFI_LOADER -config CMD_BOOTEFI_BOOTMGR +config BOOTEFI_BOOTMGR bool "UEFI Boot Manager" default y select BOOTMETH_GLOBAL if BOOTSTD help Select this option if you want to select the UEFI binary to be booted
via UEFI variables Boot####, BootOrder, and BootNext. This enables the'bootefi bootmgr' command.
via UEFI variables Boot####, BootOrder, and BootNext. You should alsonormally enable CMD_BOOTEFI_BOOTMGR so that the command is available.diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 8d31fc61c601..0a2cb6e3c476 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -42,7 +42,7 @@ targets += initrddump.o endif obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o -obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o +obj-$(CONFIG_BOOTEFI_BOOTMGR) += efi_bootmgr.o obj-y += efi_boottime.o obj-y += efi_helper.o obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
This patch looks wrong.
Symbol CONFIG_CMD_BOOTEFI_BOOTMGR is used in a lot of places where it is not related to the 'bootefi bootmgr' subcommand.
I see no benefit in two separate symbols. If you want to rename the symbol, please, replace *all* occurrences:
%s/CONFIG_CMD_BOOTEFI_BOOTMGR/CONFIG_BOOTEFI_BOOTMGR/
Yes, there's work on the EFI_LOADER side of things to support the use case of "boot to menu" (or, "boot to efi bootmgr") of which this is the starting point. The follow-up work that I'm hoping you or someone else with more EFI_LOADER experience will pick up is splitting cmd/bootefi.c such that we can call in to starting an EFI payload (or bootmgr) without the command line.
I will be able to take care unless Heinrich wants to do by himself. # Test would be another issue now that "bootefi" may handle various boot scenarios.
At the very high level, this is solved for "bootm" today, and I _think_ booti/bootz also but I'm less sure of that. The non-command side logic is in boot/bootm.c rather than cmd/bootm.c and so we exercise the core still via the normal tests. I don't know if the best answer is boot/bootefi.c or lib/efi_loader/something.c. Tests of new functionality will require some thought tho, yes.
Today, the bootmeth for using the EFI loader via bootefi depends on calling the bootefi command directly, so make this in turn depend on CMD_BOOTEFI.
Signed-off-by: Tom Rini trini@konsulko.com --- boot/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/boot/Kconfig b/boot/Kconfig index a01e6cb8aafe..e0ded3249343 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -504,7 +504,7 @@ config BOOTMETH_EXTLINUX_PXE
config BOOTMETH_EFILOADER bool "Bootdev support for EFI boot" - depends on EFI_LOADER + depends on CMD_BOOTEFI default y help Enables support for EFI boot using bootdevs. This makes the @@ -539,7 +539,7 @@ config BOOTMETH_DISTRO select BOOTMETH_SCRIPT # E.g. Armbian uses scripts select BOOTMETH_EXTLINUX # E.g. Debian uses these select BOOTMETH_EXTLINUX_PXE if CMD_PXE && CMD_NET && DM_ETH - select BOOTMETH_EFILOADER if EFI_LOADER # E.g. Ubuntu uses this + select BOOTMETH_EFILOADER if CMD_BOOTEFI # E.g. Ubuntu uses this
config SPL_BOOTMETH_VBE bool "Bootdev support for Verified Boot for Embedded (SPL)"
From: Simon Glass sjg@chromium.org
Make AUTOBOOT depend on CMDLINE since it is mostly meaningless without it.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com --- boot/Kconfig | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/boot/Kconfig b/boot/Kconfig index e0ded3249343..0dbd10a93469 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -1162,14 +1162,16 @@ menu "Autoboot options"
config AUTOBOOT bool "Autoboot" + depends on CMDLINE default y help This enables the autoboot. See doc/README.autoboot for detail.
+if AUTOBOOT + config BOOTDELAY int "delay in seconds before automatically booting" default 2 - depends on AUTOBOOT help Delay before automatically running bootcmd; set to 0 to autoboot with no delay, but you can stop it by key input. @@ -1191,9 +1193,11 @@ config AUTOBOOT_KEYED U-Boot automatic booting process and bring the device to the U-Boot prompt for user input.
+if AUTOBOOT_KEYED + config AUTOBOOT_FLUSH_STDIN bool "Enable flushing stdin before starting to read the password" - depends on AUTOBOOT_KEYED && !SANDBOX + depends on !SANDBOX help When this option is enabled stdin buffer will be flushed before starting to read the password. @@ -1202,7 +1206,6 @@ config AUTOBOOT_FLUSH_STDIN
config AUTOBOOT_PROMPT string "Autoboot stop prompt" - depends on AUTOBOOT_KEYED default "Autoboot in %d seconds\n" help This string is displayed before the boot delay selected by @@ -1218,7 +1221,6 @@ config AUTOBOOT_PROMPT
config AUTOBOOT_ENCRYPTION bool "Enable encryption in autoboot stopping" - depends on AUTOBOOT_KEYED help This option allows a string to be entered into U-Boot to stop the autoboot. @@ -1245,7 +1247,7 @@ config AUTOBOOT_SHA256_FALLBACK
config AUTOBOOT_DELAY_STR string "Delay autobooting via specific input key / string" - depends on AUTOBOOT_KEYED && !AUTOBOOT_ENCRYPTION + depends on !AUTOBOOT_ENCRYPTION help This option delays the automatic boot feature by issuing a specific input key or string. If CONFIG_AUTOBOOT_DELAY_STR @@ -1257,7 +1259,7 @@ config AUTOBOOT_DELAY_STR
config AUTOBOOT_STOP_STR string "Stop autobooting via specific input key / string" - depends on AUTOBOOT_KEYED && !AUTOBOOT_ENCRYPTION + depends on !AUTOBOOT_ENCRYPTION help This option enables stopping (aborting) of the automatic boot feature only by issuing a specific input key or @@ -1269,7 +1271,7 @@ config AUTOBOOT_STOP_STR
config AUTOBOOT_KEYED_CTRLC bool "Enable Ctrl-C autoboot interruption" - depends on AUTOBOOT_KEYED && !AUTOBOOT_ENCRYPTION + depends on !AUTOBOOT_ENCRYPTION help This option allows for the boot sequence to be interrupted by ctrl-c, in addition to the "bootdelaykey" and "bootstopkey". @@ -1278,7 +1280,7 @@ config AUTOBOOT_KEYED_CTRLC
config AUTOBOOT_NEVER_TIMEOUT bool "Make the password entry never time-out" - depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION && CRYPT_PW + depends on AUTOBOOT_ENCRYPTION && CRYPT_PW help This option removes the timeout from the password entry when the user first presses the <Enter> key before entering @@ -1286,7 +1288,7 @@ config AUTOBOOT_NEVER_TIMEOUT
config AUTOBOOT_STOP_STR_ENABLE bool "Enable fixed string to stop autobooting" - depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION + depends on AUTOBOOT_ENCRYPTION help This option enables the feature to add a fixed stop string that is defined at compile time. @@ -1317,9 +1319,12 @@ config AUTOBOOT_STOP_STR_SHA256 includes a ":", the portion prior to the ":" will be treated as a salt value.
+endif # AUTOBOOT_KEYED + +if !AUTOBOOT_KEYED + config AUTOBOOT_USE_MENUKEY bool "Allow a specify key to run a menu from the environment" - depends on !AUTOBOOT_KEYED help If a specific key is pressed to stop autoboot, then the commands in the environment variable 'menucmd' are executed before boot starts. @@ -1334,6 +1339,10 @@ config AUTOBOOT_MENUKEY For example, 33 means "!" in ASCII, so pressing ! at boot would take this action.
+endif + +endif # AUTOBOOT + config AUTOBOOT_MENU_SHOW bool "Show a menu on boot" depends on CMD_BOOTMENU
The implementation of DISTRO_DEFAULTS is done in environment scripts and requires the command line in order to work. Because of this, select CMDLINE here.
Signed-off-by: Tom Rini trini@konsulko.com --- boot/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/boot/Kconfig b/boot/Kconfig index 0dbd10a93469..2075ecd34b1f 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -777,6 +777,7 @@ endmenu # Boot images
config DISTRO_DEFAULTS bool "(deprecated) Script-based booting of Linux distributions" + select CMDLINE select BOOT_DEFAULTS select AUTO_COMPLETE select CMDLINE_EDITING
We split BOOT_DEFAULTS to have BOOT_DEFAULTS_FEATURES and BOOT_DEFAULTS_CMDS that in turn list general features or commands that we want enabled when BOOT_DEFAULTS is selected. We only select BOOT_DEFAULTS_CMDS if CMDLINE is set.
Signed-off-by: Tom Rini trini@konsulko.com --- boot/Kconfig | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/boot/Kconfig b/boot/Kconfig index 2075ecd34b1f..88b9296ee1bf 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -346,8 +346,16 @@ config PXE_UTILS help Utilities for parsing PXE file formats.
-config BOOT_DEFAULTS - bool # Common defaults for standard boot and distroboot +config BOOT_DEFAULTS_FEATURES + bool + select SUPPORT_RAW_INITRD + select ENV_VARS_UBOOT_CONFIG + imply USB_STORAGE + imply EFI_PARTITION + imply ISO_PARTITION + +config BOOT_DEFAULTS_CMDS + bool imply USE_BOOTCOMMAND select CMD_ENV_EXISTS select CMD_EXT2 @@ -358,14 +366,14 @@ config BOOT_DEFAULTS select CMD_DHCP if CMD_NET select CMD_PING if CMD_NET select CMD_PXE if CMD_NET - select SUPPORT_RAW_INITRD - select ENV_VARS_UBOOT_CONFIG select CMD_BOOTI if ARM64 select CMD_BOOTZ if ARM && !ARM64 imply CMD_MII if NET - imply USB_STORAGE - imply EFI_PARTITION - imply ISO_PARTITION + +config BOOT_DEFAULTS + bool # Common defaults for standard boot and distroboot + select BOOT_DEFAULTS_FEATURES + select BOOT_DEFAULTS_CMDS if CMDLINE help These are not required but are commonly needed to support a good selection of booting methods. Enable this to improve the capability @@ -431,7 +439,6 @@ config BOOTSTD_FULL config BOOTSTD_DEFAULTS bool "Select some common defaults for standard boot" depends on BOOTSTD - imply USE_BOOTCOMMAND select BOOT_DEFAULTS select BOOTMETH_DISTRO help
This particular option is required for booting all image types, regardless of if we are starting an OS via command line or something else. Move the question for SYS_BOOTM_LEN to be by the question for LEGACY_IMAGE_FORMAT, as that's where our generic OS questions start.
Signed-off-by: Tom Rini trini@konsulko.com --- boot/Kconfig | 11 +++++++++++ cmd/Kconfig | 11 ----------- 2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/boot/Kconfig b/boot/Kconfig index 88b9296ee1bf..7c92e0974c5f 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -692,6 +692,17 @@ config LEGACY_IMAGE_FORMAT loaded. If a board needs the legacy image format support in this case, enable it here.
+config SYS_BOOTM_LEN + hex "Maximum size of a decompresed OS image" + depends on CMD_BOOTM || CMD_BOOTI || CMD_BOOTZ || \ + LEGACY_IMAGE_FORMAT || SPL_LEGACY_IMAGE_FORMAT + default 0x4000000 if PPC || ARM64 + default 0x1000000 if X86 || ARCH_MX6 || ARCH_MX7 + default 0x800000 + help + This is the maximum size of the buffer that is used to decompress the OS + image in to if attempting to boot a compressed image. + config SUPPORT_RAW_INITRD bool "Enable raw initrd images" help diff --git a/cmd/Kconfig b/cmd/Kconfig index 872cb49150cc..099ec444ae99 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -361,17 +361,6 @@ config BOOTM_VXWORKS help Support booting VxWorks images via the bootm command.
-config SYS_BOOTM_LEN - hex "Maximum size of a decompresed OS image" - depends on CMD_BOOTM || CMD_BOOTI || CMD_BOOTZ || \ - LEGACY_IMAGE_FORMAT || SPL_LEGACY_IMAGE_FORMAT - default 0x4000000 if PPC || ARM64 - default 0x1000000 if X86 || ARCH_MX6 || ARCH_MX7 - default 0x800000 - help - This is the maximum size of the buffer that is used to decompress the OS - image in to, if passing a compressed image to bootm/booti/bootz. - config CMD_BOOTEFI bool "bootefi" depends on EFI_LOADER
In order to use bootmeth_cros, at least on non-X86, we need to be able to start any type of kernel that the "bootm" code paths can handle. Add these objects to the required list for this option.
Signed-off-by: Tom Rini trini@konsulko.com --- Cc: Simon Glass sjg@chromium.org --- boot/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/boot/Makefile b/boot/Makefile index ad608598d298..3fd048bb41ab 100644 --- a/boot/Makefile +++ b/boot/Makefile @@ -28,7 +28,7 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootstd-uclass.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EXTLINUX) += bootmeth_extlinux.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EXTLINUX_PXE) += bootmeth_pxe.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EFILOADER) += bootmeth_efi.o -obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_CROS) += bootmeth_cros.o +obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_CROS) += bootm.o bootm_os.o bootmeth_cros.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SANDBOX) += bootmeth_sandbox.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SCRIPT) += bootmeth_script.o ifdef CONFIG_$(SPL_TPL_)BOOTSTD_FULL
As this particular bootmeth requires the command line and assorted commands to function, make sure we have CMDLINE enabled.
Signed-off-by: Tom Rini trini@konsulko.com --- boot/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/boot/Kconfig b/boot/Kconfig index 7c92e0974c5f..40a04f43ee3d 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -543,7 +543,7 @@ config BOOTMETH_VBE
config BOOTMETH_DISTRO bool # Options needed to boot any distro - select BOOTMETH_SCRIPT # E.g. Armbian uses scripts + select BOOTMETH_SCRIPT if CMDLINE # E.g. Armbian uses scripts select BOOTMETH_EXTLINUX # E.g. Debian uses these select BOOTMETH_EXTLINUX_PXE if CMD_PXE && CMD_NET && DM_ETH select BOOTMETH_EFILOADER if CMD_BOOTEFI # E.g. Ubuntu uses this @@ -671,6 +671,7 @@ config BOOTMETH_SANDBOX config BOOTMETH_SCRIPT bool "Bootdev support for U-Boot scripts" default y if BOOTSTD_FULL + depends on CMDLINE select HUSH_PARSER help Enables support for booting a distro via a U-Boot script. This makes
In order for a predefined "preboot" or "bootcmd" to be executed by the running system we must have a command line. Add CMDLINE as a dependency.
Signed-off-by: Tom Rini trini@konsulko.com --- boot/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/boot/Kconfig b/boot/Kconfig index 40a04f43ee3d..fabf6fec2195 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -1558,6 +1558,7 @@ config BOOTARGS_SUBST
config USE_BOOTCOMMAND bool "Enable a default value for bootcmd" + depends on CMDLINE help Provide a default value for the bootcmd entry in the environment. If autoboot is enabled this is what will be run automatically. Enable @@ -1577,6 +1578,7 @@ config BOOTCOMMAND
config USE_PREBOOT bool "Enable preboot" + depends on CMDLINE help When this option is enabled, the existence of the environment variable "preboot" will be checked immediately before starting the
From: Simon Glass sjg@chromium.org
If we disable CMDLINE then we should not ask about enabling the hush parser nor any of the commands that would be run on the command line as it is no longer available. Convert the CMDLINE option into a menuconfig and make every command referenced under cmd/Kconfig depend on it.
This leaves as future work moving the commands that are not under the cmd/ hierarchy as future work.
Signed-off-by: Simon Glass sjg@chromium.org Signed-off-by: Tom Rini trini@konsulko.com --- Changes in v4: - Reword the commit message slightly. - Make this not depend on other patches --- Makefile | 2 +- cmd/Kconfig | 20 ++++++++------------ 2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/Makefile b/Makefile index e0040a40d330..969f3f74a74d 100644 --- a/Makefile +++ b/Makefile @@ -851,7 +851,7 @@ HAVE_VENDOR_COMMON_LIB = $(if $(wildcard $(srctree)/board/$(VENDOR)/common/Makef libs-$(CONFIG_API) += api/ libs-$(HAVE_VENDOR_COMMON_LIB) += board/$(VENDOR)/common/ libs-y += boot/ -libs-y += cmd/ +libs-$(CONFIG_CMDLINE) += cmd/ libs-y += common/ libs-$(CONFIG_OF_EMBED) += dts/ libs-y += env/ diff --git a/cmd/Kconfig b/cmd/Kconfig index 099ec444ae99..91297cb53f9a 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1,7 +1,5 @@ -menu "Command line interface" - -config CMDLINE - bool "Support U-Boot commands" +menuconfig CMDLINE + bool "Command line interface" default y help Enable U-Boot's command-line functions. This provides a means @@ -11,9 +9,10 @@ config CMDLINE Depending on the number of commands enabled, this can add substantially to the size of U-Boot.
+if CMDLINE + config HUSH_PARSER bool "Use hush shell" - depends on CMDLINE help This option enables the "hush" shell (from Busybox) as command line interpreter, thus enabling powerful command line syntax like @@ -25,7 +24,6 @@ config HUSH_PARSER
config CMDLINE_EDITING bool "Enable command line editing" - depends on CMDLINE default y help Enable editing and History functions for interactive command line @@ -40,15 +38,13 @@ config CMDLINE_PS_SUPPORT
config AUTO_COMPLETE bool "Enable auto complete using TAB" - depends on CMDLINE default y help Enable auto completion of commands using TAB.
config SYS_LONGHELP bool "Enable long help messages" - depends on CMDLINE - default y if CMDLINE + default y help Defined when you want long help messages included Do not set this option when short of memory. @@ -77,8 +73,7 @@ config SYS_MAXARGS
config SYS_XTRACE bool "Command execution tracer" - depends on CMDLINE - default y if CMDLINE + default y help This option enables the possiblity to print all commands before executing them and after all variables are evaluated (similar @@ -2872,4 +2867,5 @@ config CMD_MESON default y help Enable useful commands for the Meson Soc family developed by Amlogic Inc. -endmenu + +endif
From: Simon Glass sjg@chromium.org
This is not used for sandbox, so drop it. Enable the things that it controls to avoid dstrastic changes in the config settings for sandbox builds.
The end result is that these are enabled:
BOOTMETH_DISTRO BOOTSTD_DEFAULTS
and these are disabled:
USE_BOOTCOMMAND BOOTCOMMAND (was "run distro_bootcmd") DISTRO_DEFAULTS
Note that the tools-only build has already disabled DISTRO_DEFAULTS and BOOTSTD_FULL
Signed-off-by: Simon Glass sjg@chromium.org Signed-off-by: Tom Rini trini@konsulko.com --- Changes in v4: - Only modify sandbox and restrict the changes to only DISTRO_DEFAULTS --- arch/Kconfig | 3 +++ configs/sandbox64_defconfig | 1 - configs/sandbox_defconfig | 1 - configs/sandbox_flattree_defconfig | 1 - configs/sandbox_noinst_defconfig | 1 - configs/sandbox_spl_defconfig | 1 - configs/sandbox_vpl_defconfig | 1 - 7 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig index 19f2891ba1c5..edfa3f7f83aa 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -208,6 +208,9 @@ config SANDBOX imply PHYSMEM imply GENERATE_ACPI_TABLE imply BINMAN + imply BOOTSTD_DEFAULTS if BOOTSTD_FULL && CMDLINE + imply BOOTMETH_DISTRO if BOOTSTD_FULL && CMDLINE + imply CMD_SYSBOOT if BOOTSTD_FULL
config SH bool "SuperH architecture" diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index 1a033b22018b..ff895b930170 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -14,7 +14,6 @@ CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y CONFIG_FIT_VERBOSE=y CONFIG_LEGACY_IMAGE_FORMAT=y -CONFIG_DISTRO_DEFAULTS=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y CONFIG_BOOTSTAGE_FDT=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 47417cb0391d..5230b81be2c4 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -14,7 +14,6 @@ CONFIG_FIT_RSASSA_PSS=y CONFIG_FIT_CIPHER=y CONFIG_FIT_VERBOSE=y CONFIG_LEGACY_IMAGE_FORMAT=y -CONFIG_DISTRO_DEFAULTS=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y CONFIG_BOOTSTAGE_FDT=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 29ae4532c508..8df2a82c521c 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -12,7 +12,6 @@ CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y CONFIG_FIT_VERBOSE=y CONFIG_LEGACY_IMAGE_FORMAT=y -CONFIG_DISTRO_DEFAULTS=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y CONFIG_BOOTSTAGE_FDT=y diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig index db05e6308329..44e55f4452f3 100644 --- a/configs/sandbox_noinst_defconfig +++ b/configs/sandbox_noinst_defconfig @@ -25,7 +25,6 @@ CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y CONFIG_FIT_VERBOSE=y CONFIG_SPL_LOAD_FIT=y -CONFIG_DISTRO_DEFAULTS=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y CONFIG_BOOTSTAGE_FDT=y diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 56072b15ad2d..015e0a59d085 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -20,7 +20,6 @@ CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y CONFIG_FIT_VERBOSE=y CONFIG_SPL_LOAD_FIT=y -CONFIG_DISTRO_DEFAULTS=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y CONFIG_BOOTSTAGE_FDT=y diff --git a/configs/sandbox_vpl_defconfig b/configs/sandbox_vpl_defconfig index 5bd0281796d4..9935575352b2 100644 --- a/configs/sandbox_vpl_defconfig +++ b/configs/sandbox_vpl_defconfig @@ -27,7 +27,6 @@ CONFIG_FIT=y CONFIG_FIT_VERBOSE=y CONFIG_FIT_BEST_MATCH=y CONFIG_SPL_LOAD_FIT=y -CONFIG_DISTRO_DEFAULTS=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y CONFIG_BOOTSTAGE_FDT=y
From: Simon Glass sjg@chromium.org
Add some dependencies on features that we had been selecting so that we can still disable CMDLINE.
Signed-off-by: Simon Glass sjg@chromium.org Signed-off-by: Tom Rini trini@konsulko.com --- Changes in v4: - Reword the commit slightly (Tom) - Rework overall to select if CMDLINE
Changes in v3: - Reorder the Kconfig options a little --- arch/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig index edfa3f7f83aa..78727050aecd 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -134,7 +134,7 @@ config SANDBOX select ARCH_SUPPORTS_LTO select BOARD_LATE_INIT select BZIP2 - select CMD_POWEROFF + select CMD_POWEROFF if CMDLINE select DM select DM_EVENT select DM_FUZZING_ENGINE @@ -152,10 +152,10 @@ config SANDBOX select PCI_ENDPOINT select SPI select SUPPORT_OF_CONTROL - select SYSRESET_CMD_POWEROFF + select SYSRESET_CMD_POWEROFF if CMD_POWEROFF select SYS_CACHE_SHIFT_4 select IRQ - select SUPPORT_EXTENSION_SCAN + select SUPPORT_EXTENSION_SCAN if CMDLINE select SUPPORT_ACPI imply BITREVERSE select BLOBLIST
Add a mostly empty asm/barrier.h file for sandbox where we define nop() to be an empty function.
Signed-off-by: Tom Rini trini@konsulko.com --- arch/sandbox/include/asm/barrier.h | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 arch/sandbox/include/asm/barrier.h
diff --git a/arch/sandbox/include/asm/barrier.h b/arch/sandbox/include/asm/barrier.h new file mode 100644 index 000000000000..0928a78cbf8b --- /dev/null +++ b/arch/sandbox/include/asm/barrier.h @@ -0,0 +1,3 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ + +#define nop()
On 10/19/23 11:01, Tom Rini wrote:
Add a mostly empty asm/barrier.h file for sandbox where we define nop() to be an empty function.
Signed-off-by: Tom Rini trini@konsulko.com
arch/sandbox/include/asm/barrier.h | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 arch/sandbox/include/asm/barrier.h
diff --git a/arch/sandbox/include/asm/barrier.h b/arch/sandbox/include/asm/barrier.h new file mode 100644 index 000000000000..0928a78cbf8b --- /dev/null +++ b/arch/sandbox/include/asm/barrier.h @@ -0,0 +1,3 @@ +/* SPDX-License-Identifier: GPL-2.0+ */
+#define nop()
Reviewed-by: Sean Anderson seanga2@gmail.com
Now that sandbox has <asm/barrier.h> and defines nop() there we should include that in our driver for clarity and then remove our local nop() from <k210/pll.h>.
Signed-off-by: Tom Rini trini@konsulko.com --- I can see that our ARM <asm/barriers.h> should be <asm/barrier.h> and updated in a few other ways to match how the kernel is currently. This is not a big deal yet as this driver is only for sandbox for risc-v
Cc: Sean Anderson seanga2@gmail.com --- drivers/clk/clk_k210.c | 1 + include/k210/pll.h | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/clk/clk_k210.c b/drivers/clk/clk_k210.c index c534cc07e092..b9469b93853b 100644 --- a/drivers/clk/clk_k210.c +++ b/drivers/clk/clk_k210.c @@ -16,6 +16,7 @@ #include <dt-bindings/mfd/k210-sysctl.h> #include <k210/pll.h> #include <linux/bitfield.h> +#include <asm/barrier.h>
DECLARE_GLOBAL_DATA_PTR;
diff --git a/include/k210/pll.h b/include/k210/pll.h index fd16a89cb203..175c47f6f233 100644 --- a/include/k210/pll.h +++ b/include/k210/pll.h @@ -16,9 +16,6 @@ struct k210_pll_config { #ifdef CONFIG_UNIT_TEST TEST_STATIC int k210_pll_calc_config(u32 rate, u32 rate_in, struct k210_pll_config *best); -#ifndef nop -#define nop() -#endif
#endif #endif /* K210_PLL_H */
From: Simon Glass sjg@chromium.org
Now that everything is working, add a test to make sure that this builds correctly.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com --- test/py/tests/test_sandbox_opts.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 test/py/tests/test_sandbox_opts.py
diff --git a/test/py/tests/test_sandbox_opts.py b/test/py/tests/test_sandbox_opts.py new file mode 100644 index 000000000000..91790b3374b4 --- /dev/null +++ b/test/py/tests/test_sandbox_opts.py @@ -0,0 +1,20 @@ +# SPDX-License-Identifier: GPL-2.0 +# Copyright 2022 Google LLC +# Written by Simon Glass sjg@chromium.org + +import pytest + +import u_boot_utils as util + +# This is needed for Azure, since the default '..' directory is not writeable +TMPDIR = '/tmp/test_cmdline' + +@pytest.mark.slow +@pytest.mark.boardspec('sandbox') +def test_sandbox_cmdline(u_boot_console): + """Test building sandbox without CONFIG_CMDLINE""" + cons = u_boot_console + + out = util.run_and_log( + cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox', + '-a', '~CMDLINE', '-o', TMPDIR])
The primary motivation for having a sandbox without LTO build in CI is to ensure that we don't have that option break. We now have the ability to run tests of specific options being enabled/disabled, so drop the parts of CI that build and test that configuration specifically and add a build test instead. We still test that "NO_LTO=1" rather than editing the config file works via the ftrace tests.
Signed-off-by: Tom Rini trini@konsulko.com --- This creates a small bisectability gap in CI itself, but I think is more reasonable than reworking the introduction of test/py/tests/test_sandbox_opts.py
Cc: Simon Glass sjg@chromium.org --- .azure-pipelines.yml | 3 --- .gitlab-ci.yml | 12 ------------ test/py/tests/test_sandbox_opts.py | 10 ++++++++++ 3 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 6f91553e8613..65533b36dde4 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -287,9 +287,6 @@ stages: sandbox64_clang: TEST_PY_BD: "sandbox64" OVERRIDE: "-O clang-16" - sandbox_nolto: - TEST_PY_BD: "sandbox" - BUILD_ENV: "NO_LTO=1" sandbox_spl: TEST_PY_BD: "sandbox_spl" TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl" diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6decdfdee334..97c964fb8079 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -258,12 +258,6 @@ sandbox with clang test.py: OVERRIDE: "-O clang-16" <<: *buildman_and_testpy_dfn
-sandbox without LTO test.py: - variables: - TEST_PY_BD: "sandbox" - BUILD_ENV: "NO_LTO=1" - <<: *buildman_and_testpy_dfn - sandbox64 test.py: variables: TEST_PY_BD: "sandbox64" @@ -275,12 +269,6 @@ sandbox64 with clang test.py: OVERRIDE: "-O clang-16" <<: *buildman_and_testpy_dfn
-sandbox64 without LTO test.py: - variables: - TEST_PY_BD: "sandbox64" - BUILD_ENV: "NO_LTO=1" - <<: *buildman_and_testpy_dfn - sandbox_spl test.py: variables: TEST_PY_BD: "sandbox_spl" diff --git a/test/py/tests/test_sandbox_opts.py b/test/py/tests/test_sandbox_opts.py index 91790b3374b4..422b43cb3bc1 100644 --- a/test/py/tests/test_sandbox_opts.py +++ b/test/py/tests/test_sandbox_opts.py @@ -18,3 +18,13 @@ def test_sandbox_cmdline(u_boot_console): out = util.run_and_log( cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox', '-a', '~CMDLINE', '-o', TMPDIR]) + +@pytest.mark.slow +@pytest.mark.boardspec('sandbox') +def test_sandbox_lto(u_boot_console): + """Test building sandbox without CONFIG_LTO""" + cons = u_boot_console + + out = util.run_and_log( + cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox', + '-a', '~LTO', '-o', TMPDIR])
On Fri, 20 Oct 2023 at 14:53, Tom Rini trini@konsulko.com wrote:
The primary motivation for having a sandbox without LTO build in CI is to ensure that we don't have that option break. We now have the ability to run tests of specific options being enabled/disabled, so drop the parts of CI that build and test that configuration specifically and add a build test instead. We still test that "NO_LTO=1" rather than editing the config file works via the ftrace tests.
Signed-off-by: Tom Rini trini@konsulko.com
This creates a small bisectability gap in CI itself, but I think is more reasonable than reworking the introduction of test/py/tests/test_sandbox_opts.py
Cc: Simon Glass sjg@chromium.org
.azure-pipelines.yml | 3 --- .gitlab-ci.yml | 12 ------------ test/py/tests/test_sandbox_opts.py | 10 ++++++++++ 3 files changed, 10 insertions(+), 15 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Q below
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 6f91553e8613..65533b36dde4 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -287,9 +287,6 @@ stages: sandbox64_clang: TEST_PY_BD: "sandbox64" OVERRIDE: "-O clang-16"
sandbox_nolto:TEST_PY_BD: "sandbox"BUILD_ENV: "NO_LTO=1" sandbox_spl: TEST_PY_BD: "sandbox_spl" TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl"diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6decdfdee334..97c964fb8079 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -258,12 +258,6 @@ sandbox with clang test.py: OVERRIDE: "-O clang-16" <<: *buildman_and_testpy_dfn
-sandbox without LTO test.py:
- variables:
- TEST_PY_BD: "sandbox"
- BUILD_ENV: "NO_LTO=1"
- <<: *buildman_and_testpy_dfn
sandbox64 test.py: variables: TEST_PY_BD: "sandbox64" @@ -275,12 +269,6 @@ sandbox64 with clang test.py: OVERRIDE: "-O clang-16" <<: *buildman_and_testpy_dfn
-sandbox64 without LTO test.py:
- variables:
- TEST_PY_BD: "sandbox64"
- BUILD_ENV: "NO_LTO=1"
- <<: *buildman_and_testpy_dfn
sandbox_spl test.py: variables: TEST_PY_BD: "sandbox_spl" diff --git a/test/py/tests/test_sandbox_opts.py b/test/py/tests/test_sandbox_opts.py index 91790b3374b4..422b43cb3bc1 100644 --- a/test/py/tests/test_sandbox_opts.py +++ b/test/py/tests/test_sandbox_opts.py @@ -18,3 +18,13 @@ def test_sandbox_cmdline(u_boot_console): out = util.run_and_log( cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox', '-a', '~CMDLINE', '-o', TMPDIR])
+@pytest.mark.slow +@pytest.mark.boardspec('sandbox') +def test_sandbox_lto(u_boot_console):
- """Test building sandbox without CONFIG_LTO"""
- cons = u_boot_console
- out = util.run_and_log(
cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox','-a', '~LTO', '-o', TMPDIR])
Don't you need sandbox64 here?
-- 2.34.1
Regards, Simon
On Sat, Oct 21, 2023 at 08:43:00AM -0700, Simon Glass wrote:
On Fri, 20 Oct 2023 at 14:53, Tom Rini trini@konsulko.com wrote:
The primary motivation for having a sandbox without LTO build in CI is to ensure that we don't have that option break. We now have the ability to run tests of specific options being enabled/disabled, so drop the parts of CI that build and test that configuration specifically and add a build test instead. We still test that "NO_LTO=1" rather than editing the config file works via the ftrace tests.
Signed-off-by: Tom Rini trini@konsulko.com
This creates a small bisectability gap in CI itself, but I think is more reasonable than reworking the introduction of test/py/tests/test_sandbox_opts.py
Cc: Simon Glass sjg@chromium.org
.azure-pipelines.yml | 3 --- .gitlab-ci.yml | 12 ------------ test/py/tests/test_sandbox_opts.py | 10 ++++++++++ 3 files changed, 10 insertions(+), 15 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Q below
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 6f91553e8613..65533b36dde4 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -287,9 +287,6 @@ stages: sandbox64_clang: TEST_PY_BD: "sandbox64" OVERRIDE: "-O clang-16"
sandbox_nolto:TEST_PY_BD: "sandbox"BUILD_ENV: "NO_LTO=1" sandbox_spl: TEST_PY_BD: "sandbox_spl" TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl"diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6decdfdee334..97c964fb8079 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -258,12 +258,6 @@ sandbox with clang test.py: OVERRIDE: "-O clang-16" <<: *buildman_and_testpy_dfn
-sandbox without LTO test.py:
- variables:
- TEST_PY_BD: "sandbox"
- BUILD_ENV: "NO_LTO=1"
- <<: *buildman_and_testpy_dfn
sandbox64 test.py: variables: TEST_PY_BD: "sandbox64" @@ -275,12 +269,6 @@ sandbox64 with clang test.py: OVERRIDE: "-O clang-16" <<: *buildman_and_testpy_dfn
-sandbox64 without LTO test.py:
- variables:
- TEST_PY_BD: "sandbox64"
- BUILD_ENV: "NO_LTO=1"
- <<: *buildman_and_testpy_dfn
sandbox_spl test.py: variables: TEST_PY_BD: "sandbox_spl" diff --git a/test/py/tests/test_sandbox_opts.py b/test/py/tests/test_sandbox_opts.py index 91790b3374b4..422b43cb3bc1 100644 --- a/test/py/tests/test_sandbox_opts.py +++ b/test/py/tests/test_sandbox_opts.py @@ -18,3 +18,13 @@ def test_sandbox_cmdline(u_boot_console): out = util.run_and_log( cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox', '-a', '~CMDLINE', '-o', TMPDIR])
+@pytest.mark.slow +@pytest.mark.boardspec('sandbox') +def test_sandbox_lto(u_boot_console):
- """Test building sandbox without CONFIG_LTO"""
- cons = u_boot_console
- out = util.run_and_log(
cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox','-a', '~LTO', '-o', TMPDIR])Don't you need sandbox64 here?
No, I don't think it's providing further value to just build sandbox64 without LTO. I'm also not 100% sure this patch is really needed in that we're trying to test for "did we disable LTO via make arguments" and in turn only the ftrace test really catches that, as it will fail if we do have LTO enabled, yes?
Hi Tom,
On Sat, 21 Oct 2023 at 11:34, Tom Rini trini@konsulko.com wrote:
On Sat, Oct 21, 2023 at 08:43:00AM -0700, Simon Glass wrote:
On Fri, 20 Oct 2023 at 14:53, Tom Rini trini@konsulko.com wrote:
The primary motivation for having a sandbox without LTO build in CI is to ensure that we don't have that option break. We now have the ability to run tests of specific options being enabled/disabled, so drop the parts of CI that build and test that configuration specifically and add a build test instead. We still test that "NO_LTO=1" rather than editing the config file works via the ftrace tests.
Signed-off-by: Tom Rini trini@konsulko.com
This creates a small bisectability gap in CI itself, but I think is more reasonable than reworking the introduction of test/py/tests/test_sandbox_opts.py
Cc: Simon Glass sjg@chromium.org
.azure-pipelines.yml | 3 --- .gitlab-ci.yml | 12 ------------ test/py/tests/test_sandbox_opts.py | 10 ++++++++++ 3 files changed, 10 insertions(+), 15 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Q below
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 6f91553e8613..65533b36dde4 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -287,9 +287,6 @@ stages: sandbox64_clang: TEST_PY_BD: "sandbox64" OVERRIDE: "-O clang-16"
sandbox_nolto:TEST_PY_BD: "sandbox"BUILD_ENV: "NO_LTO=1" sandbox_spl: TEST_PY_BD: "sandbox_spl" TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl"diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6decdfdee334..97c964fb8079 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -258,12 +258,6 @@ sandbox with clang test.py: OVERRIDE: "-O clang-16" <<: *buildman_and_testpy_dfn
-sandbox without LTO test.py:
- variables:
- TEST_PY_BD: "sandbox"
- BUILD_ENV: "NO_LTO=1"
- <<: *buildman_and_testpy_dfn
sandbox64 test.py: variables: TEST_PY_BD: "sandbox64" @@ -275,12 +269,6 @@ sandbox64 with clang test.py: OVERRIDE: "-O clang-16" <<: *buildman_and_testpy_dfn
-sandbox64 without LTO test.py:
- variables:
- TEST_PY_BD: "sandbox64"
- BUILD_ENV: "NO_LTO=1"
- <<: *buildman_and_testpy_dfn
sandbox_spl test.py: variables: TEST_PY_BD: "sandbox_spl" diff --git a/test/py/tests/test_sandbox_opts.py b/test/py/tests/test_sandbox_opts.py index 91790b3374b4..422b43cb3bc1 100644 --- a/test/py/tests/test_sandbox_opts.py +++ b/test/py/tests/test_sandbox_opts.py @@ -18,3 +18,13 @@ def test_sandbox_cmdline(u_boot_console): out = util.run_and_log( cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox', '-a', '~CMDLINE', '-o', TMPDIR])
+@pytest.mark.slow +@pytest.mark.boardspec('sandbox') +def test_sandbox_lto(u_boot_console):
- """Test building sandbox without CONFIG_LTO"""
- cons = u_boot_console
- out = util.run_and_log(
cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox','-a', '~LTO', '-o', TMPDIR])Don't you need sandbox64 here?
No, I don't think it's providing further value to just build sandbox64 without LTO. I'm also not 100% sure this patch is really needed in that we're trying to test for "did we disable LTO via make arguments" and in turn only the ftrace test really catches that, as it will fail if we do have LTO enabled, yes?
Really the test is to make sure that sandbox builds without LTO. With the build time being. For development, LTO serves no useful purpose and just triples the incremental build time.
I suggest we keep sandbox64 just to prevent a regression on non-LTO, but I suspect it is unlikely to happen in practice.
Regards, Simon
On Mon, Oct 23, 2023 at 12:05:34AM -0700, Simon Glass wrote:
Hi Tom,
On Sat, 21 Oct 2023 at 11:34, Tom Rini trini@konsulko.com wrote:
On Sat, Oct 21, 2023 at 08:43:00AM -0700, Simon Glass wrote:
On Fri, 20 Oct 2023 at 14:53, Tom Rini trini@konsulko.com wrote:
The primary motivation for having a sandbox without LTO build in CI is to ensure that we don't have that option break. We now have the ability to run tests of specific options being enabled/disabled, so drop the parts of CI that build and test that configuration specifically and add a build test instead. We still test that "NO_LTO=1" rather than editing the config file works via the ftrace tests.
Signed-off-by: Tom Rini trini@konsulko.com
This creates a small bisectability gap in CI itself, but I think is more reasonable than reworking the introduction of test/py/tests/test_sandbox_opts.py
Cc: Simon Glass sjg@chromium.org
.azure-pipelines.yml | 3 --- .gitlab-ci.yml | 12 ------------ test/py/tests/test_sandbox_opts.py | 10 ++++++++++ 3 files changed, 10 insertions(+), 15 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Q below
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 6f91553e8613..65533b36dde4 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -287,9 +287,6 @@ stages: sandbox64_clang: TEST_PY_BD: "sandbox64" OVERRIDE: "-O clang-16"
sandbox_nolto:TEST_PY_BD: "sandbox"BUILD_ENV: "NO_LTO=1" sandbox_spl: TEST_PY_BD: "sandbox_spl" TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl"diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6decdfdee334..97c964fb8079 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -258,12 +258,6 @@ sandbox with clang test.py: OVERRIDE: "-O clang-16" <<: *buildman_and_testpy_dfn
-sandbox without LTO test.py:
- variables:
- TEST_PY_BD: "sandbox"
- BUILD_ENV: "NO_LTO=1"
- <<: *buildman_and_testpy_dfn
sandbox64 test.py: variables: TEST_PY_BD: "sandbox64" @@ -275,12 +269,6 @@ sandbox64 with clang test.py: OVERRIDE: "-O clang-16" <<: *buildman_and_testpy_dfn
-sandbox64 without LTO test.py:
- variables:
- TEST_PY_BD: "sandbox64"
- BUILD_ENV: "NO_LTO=1"
- <<: *buildman_and_testpy_dfn
sandbox_spl test.py: variables: TEST_PY_BD: "sandbox_spl" diff --git a/test/py/tests/test_sandbox_opts.py b/test/py/tests/test_sandbox_opts.py index 91790b3374b4..422b43cb3bc1 100644 --- a/test/py/tests/test_sandbox_opts.py +++ b/test/py/tests/test_sandbox_opts.py @@ -18,3 +18,13 @@ def test_sandbox_cmdline(u_boot_console): out = util.run_and_log( cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox', '-a', '~CMDLINE', '-o', TMPDIR])
+@pytest.mark.slow +@pytest.mark.boardspec('sandbox') +def test_sandbox_lto(u_boot_console):
- """Test building sandbox without CONFIG_LTO"""
- cons = u_boot_console
- out = util.run_and_log(
cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox','-a', '~LTO', '-o', TMPDIR])Don't you need sandbox64 here?
No, I don't think it's providing further value to just build sandbox64 without LTO. I'm also not 100% sure this patch is really needed in that we're trying to test for "did we disable LTO via make arguments" and in turn only the ftrace test really catches that, as it will fail if we do have LTO enabled, yes?
Really the test is to make sure that sandbox builds without LTO. With the build time being. For development, LTO serves no useful purpose and just triples the incremental build time.
I suggest we keep sandbox64 just to prevent a regression on non-LTO, but I suspect it is unlikely to happen in practice.
I don't follow you here. This adds the build time test for disabling LTO, as that's at least an option for sandbox (other platforms require it because otherwise we won't fit on the hardware). I'm mainly not sure if this test is right in that what we really want to know is "does NO_LTO=1" pass things as expected, and the ftrace test covers that.
Hi Tom,
On Mon, 23 Oct 2023 at 06:37, Tom Rini trini@konsulko.com wrote:
On Mon, Oct 23, 2023 at 12:05:34AM -0700, Simon Glass wrote:
Hi Tom,
On Sat, 21 Oct 2023 at 11:34, Tom Rini trini@konsulko.com wrote:
On Sat, Oct 21, 2023 at 08:43:00AM -0700, Simon Glass wrote:
On Fri, 20 Oct 2023 at 14:53, Tom Rini trini@konsulko.com wrote:
The primary motivation for having a sandbox without LTO build in CI is to ensure that we don't have that option break. We now have the ability to run tests of specific options being enabled/disabled, so drop the parts of CI that build and test that configuration specifically and add a build test instead. We still test that "NO_LTO=1" rather than editing the config file works via the ftrace tests.
Signed-off-by: Tom Rini trini@konsulko.com
This creates a small bisectability gap in CI itself, but I think is more reasonable than reworking the introduction of test/py/tests/test_sandbox_opts.py
Cc: Simon Glass sjg@chromium.org
.azure-pipelines.yml | 3 --- .gitlab-ci.yml | 12 ------------ test/py/tests/test_sandbox_opts.py | 10 ++++++++++ 3 files changed, 10 insertions(+), 15 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Q below
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 6f91553e8613..65533b36dde4 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -287,9 +287,6 @@ stages: sandbox64_clang: TEST_PY_BD: "sandbox64" OVERRIDE: "-O clang-16"
sandbox_nolto:TEST_PY_BD: "sandbox"BUILD_ENV: "NO_LTO=1" sandbox_spl: TEST_PY_BD: "sandbox_spl" TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl"diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6decdfdee334..97c964fb8079 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -258,12 +258,6 @@ sandbox with clang test.py: OVERRIDE: "-O clang-16" <<: *buildman_and_testpy_dfn
-sandbox without LTO test.py:
- variables:
- TEST_PY_BD: "sandbox"
- BUILD_ENV: "NO_LTO=1"
- <<: *buildman_and_testpy_dfn
sandbox64 test.py: variables: TEST_PY_BD: "sandbox64" @@ -275,12 +269,6 @@ sandbox64 with clang test.py: OVERRIDE: "-O clang-16" <<: *buildman_and_testpy_dfn
-sandbox64 without LTO test.py:
- variables:
- TEST_PY_BD: "sandbox64"
- BUILD_ENV: "NO_LTO=1"
- <<: *buildman_and_testpy_dfn
sandbox_spl test.py: variables: TEST_PY_BD: "sandbox_spl" diff --git a/test/py/tests/test_sandbox_opts.py b/test/py/tests/test_sandbox_opts.py index 91790b3374b4..422b43cb3bc1 100644 --- a/test/py/tests/test_sandbox_opts.py +++ b/test/py/tests/test_sandbox_opts.py @@ -18,3 +18,13 @@ def test_sandbox_cmdline(u_boot_console): out = util.run_and_log( cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox', '-a', '~CMDLINE', '-o', TMPDIR])
+@pytest.mark.slow +@pytest.mark.boardspec('sandbox') +def test_sandbox_lto(u_boot_console):
- """Test building sandbox without CONFIG_LTO"""
- cons = u_boot_console
- out = util.run_and_log(
cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox','-a', '~LTO', '-o', TMPDIR])Don't you need sandbox64 here?
No, I don't think it's providing further value to just build sandbox64 without LTO. I'm also not 100% sure this patch is really needed in that we're trying to test for "did we disable LTO via make arguments" and in turn only the ftrace test really catches that, as it will fail if we do have LTO enabled, yes?
Really the test is to make sure that sandbox builds without LTO. With the build time being. For development, LTO serves no useful purpose and just triples the incremental build time.
I suggest we keep sandbox64 just to prevent a regression on non-LTO, but I suspect it is unlikely to happen in practice.
I don't follow you here. This adds the build time test for disabling LTO, as that's at least an option for sandbox (other platforms require it because otherwise we won't fit on the hardware). I'm mainly not sure if this test is right in that what we really want to know is "does NO_LTO=1" pass things as expected, and the ftrace test covers that.
OK, I see. Well let's run with it and see how we go. I have wanted to have these sorts of tests for a while, so we can check combinations that are not in common use.
BTW buildman supports -L which disabled LTO using the NO_LTO=1 option
Regards, Simon
On Mon, Oct 23, 2023 at 10:13:52AM -0700, Simon Glass wrote:
[snip]
BTW buildman supports -L which disabled LTO using the NO_LTO=1 option
I worry about putting sandbox-specific flags in buildman. Outside of sandbox, targets that enable LTO require LTO, just like any other CONFIG option.
Hi Tom,
On Mon, 23 Oct 2023 at 10:28, Tom Rini trini@konsulko.com wrote:
On Mon, Oct 23, 2023 at 10:13:52AM -0700, Simon Glass wrote:
[snip]
BTW buildman supports -L which disabled LTO using the NO_LTO=1 option
I worry about putting sandbox-specific flags in buildman. Outside of sandbox, targets that enable LTO require LTO, just like any other CONFIG option.
Some problems with LTO and why I don't normally develop with it enabled:
- build time - code moves around all over the place so it is hard to compare size growth
At least for my IDE flow, I use -L in most cases. Yes there are some boards which won't fit without LTO, but I don't see them much.
So this is mostly a dev convenience / productivity tool.
Regards, Simon
On Tue, Oct 24, 2023 at 11:02:06AM -0700, Simon Glass wrote:
Hi Tom,
On Mon, 23 Oct 2023 at 10:28, Tom Rini trini@konsulko.com wrote:
On Mon, Oct 23, 2023 at 10:13:52AM -0700, Simon Glass wrote:
[snip]
BTW buildman supports -L which disabled LTO using the NO_LTO=1 option
I worry about putting sandbox-specific flags in buildman. Outside of sandbox, targets that enable LTO require LTO, just like any other CONFIG option.
Some problems with LTO and why I don't normally develop with it enabled:
- build time
- code moves around all over the place so it is hard to compare size growth
At least for my IDE flow, I use -L in most cases. Yes there are some boards which won't fit without LTO, but I don't see them much.
So this is mostly a dev convenience / productivity tool.
Yes, it does take longer to link. And yes, a more complex optimization does make some size tracking harder to understand (since growth or shrinkage allows for different optimizations to be made around it). But everything in configs/ that enables LTO needs LTO.
Hi Tom,
On Tue, 24 Oct 2023 at 11:07, Tom Rini trini@konsulko.com wrote:
On Tue, Oct 24, 2023 at 11:02:06AM -0700, Simon Glass wrote:
Hi Tom,
On Mon, 23 Oct 2023 at 10:28, Tom Rini trini@konsulko.com wrote:
On Mon, Oct 23, 2023 at 10:13:52AM -0700, Simon Glass wrote:
[snip]
BTW buildman supports -L which disabled LTO using the NO_LTO=1 option
I worry about putting sandbox-specific flags in buildman. Outside of sandbox, targets that enable LTO require LTO, just like any other CONFIG option.
Some problems with LTO and why I don't normally develop with it enabled:
- build time
- code moves around all over the place so it is hard to compare size growth
At least for my IDE flow, I use -L in most cases. Yes there are some boards which won't fit without LTO, but I don't see them much.
So this is mostly a dev convenience / productivity tool.
Yes, it does take longer to link. And yes, a more complex optimization does make some size tracking harder to understand (since growth or shrinkage allows for different optimizations to be made around it). But everything in configs/ that enables LTO needs LTO.
I thought we were planning to enable it for all of ARM, though? Clearly most of those boards don't *need* it.
Regards, Simon
On Tue, Oct 24, 2023 at 02:39:49PM -0700, Simon Glass wrote:
Hi Tom,
On Tue, 24 Oct 2023 at 11:07, Tom Rini trini@konsulko.com wrote:
On Tue, Oct 24, 2023 at 11:02:06AM -0700, Simon Glass wrote:
Hi Tom,
On Mon, 23 Oct 2023 at 10:28, Tom Rini trini@konsulko.com wrote:
On Mon, Oct 23, 2023 at 10:13:52AM -0700, Simon Glass wrote:
[snip]
BTW buildman supports -L which disabled LTO using the NO_LTO=1 option
I worry about putting sandbox-specific flags in buildman. Outside of sandbox, targets that enable LTO require LTO, just like any other CONFIG option.
Some problems with LTO and why I don't normally develop with it enabled:
- build time
- code moves around all over the place so it is hard to compare size growth
At least for my IDE flow, I use -L in most cases. Yes there are some boards which won't fit without LTO, but I don't see them much.
So this is mostly a dev convenience / productivity tool.
Yes, it does take longer to link. And yes, a more complex optimization does make some size tracking harder to understand (since growth or shrinkage allows for different optimizations to be made around it). But everything in configs/ that enables LTO needs LTO.
I thought we were planning to enable it for all of ARM, though? Clearly most of those boards don't *need* it.
Yes, closer to when it was introduced the ideal was to make it default, because there are so many "on the edge" platforms, with respect to size limits. And, we could possibly make it default now and see what that does to CI build times too. But no, today, platforms opt-in for it because they need it, and the most common need is because of size rather than speed (or, speed gained through size reduction).
And also yes, that it does make size comparison trickier is another reason I've not aggressively pushed for it globally.
But yes, I still don't think disabling LTO is a general feature, and only sandbox has it architecture-wide enabled by default (for testing) and that -L/--no-lto is not a good generic feature for buildman.
But I too don't wish to argue points around LTO nor gc-sections either anymore.
On 10/19/23 11:01, Tom Rini wrote:
Now that sandbox has <asm/barrier.h> and defines nop() there we should include that in our driver for clarity and then remove our local nop() from <k210/pll.h>.
Signed-off-by: Tom Rini trini@konsulko.com
I can see that our ARM <asm/barriers.h> should be <asm/barrier.h> and updated in a few other ways to match how the kernel is currently. This is not a big deal yet as this driver is only for sandbox for risc-v
Cc: Sean Anderson seanga2@gmail.com
drivers/clk/clk_k210.c | 1 + include/k210/pll.h | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/clk/clk_k210.c b/drivers/clk/clk_k210.c index c534cc07e092..b9469b93853b 100644 --- a/drivers/clk/clk_k210.c +++ b/drivers/clk/clk_k210.c @@ -16,6 +16,7 @@ #include <dt-bindings/mfd/k210-sysctl.h> #include <k210/pll.h> #include <linux/bitfield.h> +#include <asm/barrier.h>
DECLARE_GLOBAL_DATA_PTR;
diff --git a/include/k210/pll.h b/include/k210/pll.h index fd16a89cb203..175c47f6f233 100644 --- a/include/k210/pll.h +++ b/include/k210/pll.h @@ -16,9 +16,6 @@ struct k210_pll_config { #ifdef CONFIG_UNIT_TEST TEST_STATIC int k210_pll_calc_config(u32 rate, u32 rate_in, struct k210_pll_config *best); -#ifndef nop -#define nop() -#endif
#endif #endif /* K210_PLL_H */
Reviewed-by: Sean Anderson seanga2@gmail.com
participants (5)
-
AKASHI Takahiro -
Heinrich Schuchardt -
Sean Anderson -
Simon Glass -
Tom Rini