Merge branch '2022-04-08-env-updates'

- Assorted env tooling updates
- Bug fix around multiple possible env locations and ENV_IS_NOWHERE
- Add 'indirect' sub-command to env
- Allow for FAT env to have the location overridden by the board code.

Makefile

@@ -2459,7 +2459,8 @@ endif
 
 quiet_cmd_genenv = GENENV  $@
 cmd_genenv = $(OBJCOPY) --dump-section .rodata.default_environment=$@ env/common.o; \
-	sed --in-place -e 's/\x00/\x0A/g' $@
+	sed --in-place -e 's/\x00/\x0A/g' $@; sed --in-place -e '/^\s*$$/d' $@; \
+	sort --field-separator== -k1,1 --stable $@ -o $@
 
 u-boot-initial-env: u-boot.bin
 	$(call if_changed,genenv)

cmd/Kconfig

@@ -518,6 +518,9 @@ config CMD_NVEDIT_EFI
 	  If enabled, we are allowed to set/print UEFI variables using
 	  "env" command with "-e" option without knowing details.
 
+config CMD_NVEDIT_INDIRECT
+	bool "env indirect - Sets environment value from another"
+
 config CMD_NVEDIT_INFO
 	bool "env info - print or evaluate environment information"
 	help

cmd/nvedit.c

@@ -1018,6 +1018,45 @@ sep_err:
 }
 #endif
 
+#if defined(CONFIG_CMD_NVEDIT_INDIRECT)
+static int do_env_indirect(struct cmd_tbl *cmdtp, int flag,
+		       int argc, char *const argv[])
+{
+	char *to = argv[1];
+	char *from = argv[2];
+	char *default_value = NULL;
+	int ret = 0;
+
+	if (argc < 3 || argc > 4) {
+		return CMD_RET_USAGE;
+	}
+
+	if (argc == 4) {
+		default_value = argv[3];
+	}
+
+	if (env_get(from) == NULL && default_value == NULL) {
+		printf("## env indirect: Environment variable for <from> (%s) does not exist.\n", from);
+
+		return CMD_RET_FAILURE;
+	}
+
+	if (env_get(from) == NULL) {
+		ret = env_set(to, default_value);
+	}
+	else {
+		ret = env_set(to, env_get(from));
+	}
+
+	if (ret == 0) {
+		return CMD_RET_SUCCESS;
+	}
+	else {
+		return CMD_RET_FAILURE;
+	}
+}
+#endif
+
 #if defined(CONFIG_CMD_NVEDIT_INFO)
 /*
  * print_env_info - print environment information
@@ -1181,6 +1220,9 @@ static struct cmd_tbl cmd_env_sub[] = {
 #if defined(CONFIG_CMD_IMPORTENV)
 	U_BOOT_CMD_MKENT(import, 5, 0, do_env_import, "", ""),
 #endif
+#if defined(CONFIG_CMD_NVEDIT_INDIRECT)
+	U_BOOT_CMD_MKENT(indirect, 3, 0, do_env_indirect, "", ""),
+#endif
 #if defined(CONFIG_CMD_NVEDIT_INFO)
 	U_BOOT_CMD_MKENT(info, 3, 0, do_env_info, "", ""),
 #endif
@@ -1265,6 +1307,9 @@ static char env_help_text[] =
 #if defined(CONFIG_CMD_IMPORTENV)
 	"env import [-d] [-t [-r] | -b | -c] addr [size] [var ...] - import environment\n"
 #endif
+#if defined(CONFIG_CMD_NVEDIT_INDIRECT)
+	"env indirect <to> <from> [default] - sets <to> to the value of <from>, using [default] when unset\n"
+#endif
 #if defined(CONFIG_CMD_NVEDIT_INFO)
 	"env info - display environment information\n"
 	"env info [-d] [-p] [-q] - evaluate environment information\n"

common/autoboot.c

@@ -446,6 +446,11 @@ const char *bootdelay_process(void)
 	s = env_get("bootdelay");
 	bootdelay = s ? (int)simple_strtol(s, NULL, 10) : CONFIG_BOOTDELAY;
 
+	/*
+	 * Does it really make sense that the devicetree overrides the user
+	 * setting? It is possibly helpful for security since the device tree
+	 * may be signed whereas the environment is often loaded from storage.
+	 */
 	if (IS_ENABLED(CONFIG_OF_CONTROL))
 		bootdelay = ofnode_conf_read_int("bootdelay", bootdelay);
 

doc/usage/environment.rst

@@ -120,7 +120,6 @@ bootdelay
     The default value is defined by CONFIG_BOOTDELAY.
     The value of 'bootdelay' is overridden by the /config/bootdelay value in
     the device-tree if CONFIG_OF_CONTROL=y.
-    Does it really make sense that the devicetree overrides the user setting?
 
 bootcmd
     The command that is run if the user does not enter the shell during the
@@ -171,7 +170,7 @@ autoload
     if set to "no" (any string beginning with 'n'),
     "bootp" and "dhcp" will just load perform a lookup of the
     configuration from the BOOTP server, but not try to
-    load any image using TFTP or DHCP.
+    load any image.
 
 autostart
     if set to "yes", an image loaded using the "bootp", "dhcp",

env/env.c

@@ -322,17 +322,18 @@ int env_init(void)
 
 		debug("%s: Environment %s init done (ret=%d)\n", __func__,
 		      drv->name, ret);
-
-		if (gd->env_valid == ENV_INVALID)
-			ret = -ENOENT;
 	}
 
-	if (!prio)
-		return -ENODEV;
+	if (!prio) {
+		gd->env_addr = (ulong)&default_environment[0];
+		gd->env_valid = ENV_INVALID;
+
+		return 0;
+	}
 
 	if (ret == -ENOENT) {
 		gd->env_addr = (ulong)&default_environment[0];
-		gd->env_valid = ENV_VALID;
+		gd->env_valid = ENV_INVALID;
 
 		return 0;
 	}

env/fat.c

@@ -32,7 +32,12 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static char *env_fat_device_and_part(void)
+__weak const char *env_fat_get_intf(void)
+{
+	return (const char *)CONFIG_ENV_FAT_INTERFACE;
+}
+
+__weak char *env_fat_get_dev_part(void)
 {
 #ifdef CONFIG_MMC
 	static char *part_str;
@@ -60,14 +65,15 @@ static int env_fat_save(void)
 	int dev, part;
 	int err;
 	loff_t size;
+	const char *ifname = env_fat_get_intf();
+	const char *dev_and_part = env_fat_get_dev_part();
 
 	err = env_export(&env_new);
 	if (err)
 		return err;
 
-	part = blk_get_device_part_str(CONFIG_ENV_FAT_INTERFACE,
-					env_fat_device_and_part(),
-					&dev_desc, &info, 1);
+	part = blk_get_device_part_str(ifname, dev_and_part,
+				       &dev_desc, &info, 1);
 	if (part < 0)
 		return 1;
 
@@ -77,8 +83,7 @@ static int env_fat_save(void)
 		 * This printf is embedded in the messages from env_save that
 		 * will calling it. The missing \n is intentional.
 		 */
-		printf("Unable to use %s %d:%d... \n",
-		       CONFIG_ENV_FAT_INTERFACE, dev, part);
+		printf("Unable to use %s %d:%d...\n", ifname, dev, part);
 		return 1;
 	}
 
@@ -93,8 +98,7 @@ static int env_fat_save(void)
 		 * This printf is embedded in the messages from env_save that
 		 * will calling it. The missing \n is intentional.
 		 */
-		printf("Unable to write \"%s\" from %s%d:%d... \n",
-			file, CONFIG_ENV_FAT_INTERFACE, dev, part);
+		printf("Unable to write \"%s\" from %s%d:%d...\n", file, ifname, dev, part);
 		return 1;
 	}
 
@@ -117,15 +121,16 @@ static int env_fat_load(void)
 	struct disk_partition info;
 	int dev, part;
 	int err1;
+	const char *ifname = env_fat_get_intf();
+	const char *dev_and_part = env_fat_get_dev_part();
 
 #ifdef CONFIG_MMC
-	if (!strcmp(CONFIG_ENV_FAT_INTERFACE, "mmc"))
+	if (!strcmp(ifname, "mmc"))
 		mmc_initialize(NULL);
 #endif
 
-	part = blk_get_device_part_str(CONFIG_ENV_FAT_INTERFACE,
-					env_fat_device_and_part(),
-					&dev_desc, &info, 1);
+	part = blk_get_device_part_str(ifname, dev_and_part,
+				       &dev_desc, &info, 1);
 	if (part < 0)
 		goto err_env_relocate;
 
@@ -135,8 +140,7 @@ static int env_fat_load(void)
 		 * This printf is embedded in the messages from env_save that
 		 * will calling it. The missing \n is intentional.
 		 */
-		printf("Unable to use %s %d:%d... \n",
-		       CONFIG_ENV_FAT_INTERFACE, dev, part);
+		printf("Unable to use %s %d:%d...\n", ifname, dev, part);
 		goto err_env_relocate;
 	}
 
@@ -154,7 +158,7 @@ static int env_fat_load(void)
 		 * will calling it. The missing \n is intentional.
 		 */
 		printf("Unable to read \"%s\" from %s%d:%d... \n",
-			CONFIG_ENV_FAT_FILE, CONFIG_ENV_FAT_INTERFACE, dev, part);
+			CONFIG_ENV_FAT_FILE, ifname, dev, part);
 		goto err_env_relocate;
 	}
 

include/env_internal.h

@@ -245,6 +245,26 @@ const char *env_ext4_get_dev_part(void);
  * Return:  an enum env_location value on success, or -ve error code.
  */
 enum env_location env_get_location(enum env_operation op, int prio);
+
+/**
+ * env_fat_get_intf() - Provide the interface for env in FAT
+ *
+ * It is a weak function allowing board to overidde the default interface for
+ * U-Boot env in FAT: CONFIG_ENV_FAT_INTERFACE
+ *
+ * Return: string of interface, empty if not supported
+ */
+const char *env_fat_get_intf(void);
+
+/**
+ * env_fat_get_dev_part() - Provide the device and partition for env in FAT
+ *
+ * It is a weak function allowing board to overidde the default device and
+ * partition used for U-Boot env in FAT: CONFIG_ENV_FAT_DEVICE_AND_PART
+ *
+ * Return: string of device and partition
+ */
+char *env_fat_get_dev_part(void);
 #endif /* DO_DEPS_ONLY */
 
 #endif /* _ENV_INTERNAL_H_ */

scripts/env2string.awk

@@ -81,7 +81,10 @@ END {
 	if (do_output) {
 		printf("%s", "#define CONFIG_EXTRA_ENV_TEXT \"")
 
-		# Print out all the variables
+		# Print out all the variables by alphabetic order, if using
+		# gawk. This allows test_env_test.py to work on both awk (where
+		# this next line does nothing)
+		PROCINFO["sorted_in"] = "@ind_str_asc"
 		for (var in vars) {
 			env = vars[var]
 			print var "=" vars[var] "\\0"

scripts/get_default_envs.sh

@@ -35,8 +35,8 @@ cp ${env_obj_file_path} ${ENV_OBJ_FILE_COPY}
 ${OBJCOPY} --dump-section .rodata.default_environment=${ENV_OBJ_FILE_COPY} \
 	${env_obj_file_path}
 
-# Replace default '\0' with '\n' and sort entries
-tr '\0' '\n' < ${ENV_OBJ_FILE_COPY} | sort --field-separator== -k1,1 --stable
+# Replace default '\0' with '\n' , remove blank lines and sort entries
+tr '\0' '\n' < ${ENV_OBJ_FILE_COPY} | sed -e '/^\s*$/d' | sort --field-separator== -k1,1 --stable
 
 rm ${ENV_OBJ_FILE_COPY}
 

test/py/tests/test_env.py

@@ -554,42 +554,42 @@ def test_env_text(u_boot_console):
 
     # two vars
     check_script('''fred=123
-ernie=456''', 'fred=123\\0ernie=456\\0')
+mary=456''', 'fred=123\\0mary=456\\0')
 
     # blank lines
     check_script('''fred=123
 
 
-ernie=456
+mary=456
 
-''', 'fred=123\\0ernie=456\\0')
+''', 'fred=123\\0mary=456\\0')
 
     # append
     check_script('''fred=123
-ernie=456
-fred+= 456''', 'fred=123 456\\0ernie=456\\0')
+mary=456
+fred+= 456''', 'fred=123 456\\0mary=456\\0')
 
     # append from empty
     check_script('''fred=
-ernie=456
-fred+= 456''', 'fred= 456\\0ernie=456\\0')
+mary=456
+fred+= 456''', 'fred= 456\\0mary=456\\0')
 
     # variable with + in it
-    check_script('fred+ernie=123', 'fred+ernie=123\\0')
+    check_script('fred+mary=123', 'fred+mary=123\\0')
 
     # ignores variables that are empty
     check_script('''fred=
 fred+=
-ernie=456''', 'ernie=456\\0')
+mary=456''', 'mary=456\\0')
 
     # single-character env name
-    check_script('''f=123
+    check_script('''m=123
 e=456
-f+= 456''', 'e=456\\0f=123 456\\0')
+m+= 456''', 'e=456\\0m=123 456\\0')
 
     # contains quotes
     check_script('''fred="my var"
-ernie=another"''', 'fred=\\"my var\\"\\0ernie=another\\"\\0')
+mary=another"''', 'fred=\\"my var\\"\\0mary=another\\"\\0')
 
     # variable name ending in +
     check_script('''fred\\+=my var
@@ -598,7 +598,7 @@ fred++= again''', 'fred+=my var again\\0')
     # variable name containing +
     check_script('''fred+jane=both
 fred+jane+=again
-ernie=456''', 'fred+jane=bothagain\\0ernie=456\\0')
+mary=456''', 'fred+jane=bothagain\\0mary=456\\0')
 
     # multi-line vars - new vars always start at column 1
     check_script('''fred=first
@@ -607,7 +607,7 @@ ernie=456''', 'fred+jane=bothagain\\0ernie=456\\0')
 
    after blank
  confusing=oops
-ernie=another"''', 'fred=first second third with tab after blank confusing=oops\\0ernie=another\\"\\0')
+mary=another"''', 'fred=first second third with tab after blank confusing=oops\\0mary=another\\"\\0')
 
     # real-world example
     check_script('''ubifs_boot=