[PATCH v2 2/3] env: Add single to redundant environment upgrade path
Heinrich Schuchardt
xypron.glpk at gmx.de
Tue Dec 23 18:43:00 CET 2025
On 12/23/25 15:31, Marek Vasut wrote:
> Add support for converting single-copy environment to redundant environment.
> In case CRC checks on both redundant environment copies fail, try one more
> CRC check on the primary environment copy and treat it as single environment.
Why would a CRC check suddenly succeed if it has failed before?
This needs some more explanation.
> If that check does pass, rewrite the single-copy environment into redundant
> environment format, indicate the environment is valid, and import that as
> usual primary copy of redundant environment. Follow up 'env save' will then
> store two environment copies and the system will continue to operate as
> regular redundant environment system.
>
> Add test which validates this upgrade path. The test starts with spi.bin
> which is pre-populated as single-copy environment and then upgrades that
> environment to dual-copy environment.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas at mailbox.org>
> ---
> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Cc: Jerome Forissier <jerome.forissier at linaro.org>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Tom Rini <trini at konsulko.com>
> Cc: u-boot at lists.denx.de
> ---
> V2: - Gate the option behind ENV_REDUNDANT_UPGRADE
> - Fix up mkenvimage path in env test
> ---
> env/Kconfig | 11 ++++++
> env/common.c | 31 +++++++++++++++-
> test/py/tests/test_env.py | 74 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 115 insertions(+), 1 deletion(-)
>
> diff --git a/env/Kconfig b/env/Kconfig
> index 4430669964c..b312f9b5324 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -489,6 +489,17 @@ config ENV_REDUNDANT
> which is used by env import/export commands which are independent of
> storing variables to redundant location on a non volatile device.
>
> +config ENV_REDUNDANT_UPGRADE
> + bool "Enable single-copy to redundant environment upgrade support"
> + depends on ENV_REDUNDANT
> + help
> + Normally, redundant environment is expected to always operate on
> + two copies of the environment. However, hardware that may have
> + originally shipped with single-copy environment can be upgraded
%s/with single-copy/with a single-copy/
%s/can be/that can be/
> + to redundant environment without loss of existing environment
> + content by correctly configuring the location of the redundant
> + environment copy and by enabling this option.
Why do we have to make this an option?
Shouldn't we always try to restore the environment?
> +
> config ENV_FAT_INTERFACE
> string "Name of the block device for the environment"
> depends on ENV_IS_IN_FAT
> diff --git a/env/common.c b/env/common.c
> index 05e78d63874..b2adbe93dbe 100644
> --- a/env/common.c
> +++ b/env/common.c
> @@ -473,14 +473,24 @@ int env_import(const char *buf, int check, int flags)
> #ifdef CONFIG_ENV_REDUNDANT
> static unsigned char env_flags;
>
> +#define ENV_SINGLE_HEADER_SIZE (sizeof(uint32_t))
> +#define ENV_SINGLE_SIZE (CONFIG_ENV_SIZE - ENV_SINGLE_HEADER_SIZE)
> +
> +typedef struct {
> + uint32_t crc; /* CRC32 over data bytes */
> + unsigned char data[ENV_SINGLE_SIZE]; /* Environment data */
> +} env_single_t;
> +
> int env_check_redund(const char *buf1, int buf1_read_fail,
> const char *buf2, int buf2_read_fail)
> {
> - int crc1_ok = 0, crc2_ok = 0;
> + int crc1_ok = 0, crc2_ok = 0, i;
> env_t *tmp_env1, *tmp_env2;
> + env_single_t *tmp_envs;
>
> tmp_env1 = (env_t *)buf1;
> tmp_env2 = (env_t *)buf2;
> + tmp_envs = (env_single_t *)buf1;
>
> if (buf1_read_fail && buf2_read_fail) {
> puts("*** Error - No Valid Environment Area found\n");
> @@ -498,6 +508,25 @@ int env_check_redund(const char *buf1, int buf1_read_fail,
> tmp_env2->crc;
>
> if (!crc1_ok && !crc2_ok) {
Do we really have a third location to copy from when both store 1 and
store 2 are defective? I would have expected that if a vendor provides a
single copy then exactly one of crc1_ok or crc2_ok is true and the other
is false.
Please, provide a documentation update explaining how this all works.
Best regards
Heinrich
> + /*
> + * Upgrade single-copy environment to redundant environment.
> + * In case CRC checks on both environment copies fail, try
> + * one more CRC check on the primary environment copy and
> + * treat it as single-copy environment. If that check does
> + * pass, rewrite the single-copy environment into redundant
> + * environment format and indicate the environment is valid.
> + * The follow up calls will import the environment as if it
> + * was a redundant environment. Follow up 'env save' will
> + * then store two environment copies.
> + */
> + if (CONFIG_IS_ENABLED(ENV_REDUNDANT_UPGRADE) && !buf1_read_fail &&
> + crc32(0, tmp_envs->data, ENV_SINGLE_SIZE) == tmp_envs->crc) {
> + for (i = ENV_SIZE - 1; i >= 0; i--)
> + tmp_env1->data[i] = tmp_envs->data[i];
> + tmp_env1->flags = 0;
> + gd->env_valid = ENV_VALID;
> + return 0;
> + }
> gd->env_valid = ENV_INVALID;
> return -ENOMSG; /* needed for env_load() */
> } else if (crc1_ok && !crc2_ok) {
> diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
> index 48e31f19b3c..f8713a59ba9 100644
> --- a/test/py/tests/test_env.py
> +++ b/test/py/tests/test_env.py
> @@ -477,6 +477,22 @@ def mk_env_spi_flash(state_test_env):
> utils.run_and_log(c, ['cp', '-f', persistent, spi_flash_img])
> return spi_flash_img
>
> +def mk_env_spi_flash_single(state_test_env):
> +
> + """Create an single-copy SPI NOR image with foo=bar entry."""
> + c = state_test_env.ubman
> + filename = 'spi.bin'
> + spi_flash_img = c.config.source_dir + '/' + filename
> +
> + try:
> + mkenvimage = os.path.join(c.config.build_dir, 'tools/mkenvimage')
> + call('( echo foo=bar | %s -s 8192 -p 0x00 - ; dd if=/dev/zero bs=2088960 count=1 2>/dev/null ) > %s' % ( mkenvimage , spi_flash_img ), shell=True)
> + except CalledProcessError:
> + call('rm -f %s' % spi_flash_img, shell=True)
> + raise
> +
> + return spi_flash_img
> +
> @pytest.mark.boardspec('sandbox')
> @pytest.mark.buildconfigspec('cmd_echo')
> @pytest.mark.buildconfigspec('cmd_nvedit_info')
> @@ -574,6 +590,64 @@ def test_env_spi_flash(state_test_env):
>
> """Test ENV in SPI NOR on sandbox."""
> c = state_test_env.ubman
> + spi_flash_img = ''
> + try:
> + spi_flash_img = mk_env_spi_flash_single(state_test_env)
> +
> + response = c.run_command('sf probe')
> + assert 'SF: Detected m25p16 with page size 256 Bytes, erase size 64 KiB, total 2 MiB' in response
> +
> + # force env location: SF
> + response = c.run_command('env select SPIFlash')
> + assert 'Select Environment on SPIFlash: OK' in response
> +
> + response = c.run_command('env load')
> + assert 'Loading Environment from SPIFlash... OK' in response
> +
> + response = c.run_command('env print foo')
> + assert 'foo=bar' in response
> +
> + response = c.run_command('env save')
> + assert 'Saving Environment to SPIFlash' in response
> +
> + response = c.run_command('env load')
> + assert 'Loading Environment from SPIFlash... OK' in response
> +
> + response = c.run_command('env print foo')
> + assert 'foo=bar' in response
> +
> + response = c.run_command('env save')
> + assert 'Saving Environment to SPIFlash' in response
> +
> + response = c.run_command('env save')
> + assert 'Saving Environment to SPIFlash' in response
> +
> + response = c.run_command('env load')
> + assert 'Loading Environment from SPIFlash... OK' in response
> +
> + response = c.run_command('env print foo')
> + assert 'foo=bar' in response
> +
> + # restore env location: NOWHERE (prio 0 in sandbox)
> + response = c.run_command('env select nowhere')
> + assert 'Select Environment on nowhere: OK' in response
> +
> + response = c.run_command('env load')
> + assert 'Loading Environment from nowhere... OK' in response
> +
> + response = c.run_command('env info')
> + assert 'env_valid = invalid' in response
> + assert 'env_ready = true' in response
> + assert 'env_use_default = true' in response
> +
> + response = c.run_command('env info -p -d')
> + assert 'Default environment is used' in response
> + assert 'Environment cannot be persisted' in response
> +
> + finally:
> + if spi_flash_img:
> + call('rm -f %s' % spi_flash_img, shell=True)
> +
> spi_flash_img = ''
> try:
> spi_flash_img = mk_env_spi_flash(state_test_env)
More information about the U-Boot
mailing list