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 <[email protected]>
---
Cc: Heinrich Schuchardt <[email protected]>
Cc: Jerome Forissier <[email protected]>
Cc: Simon Glass <[email protected]>
Cc: Tom Rini <[email protected]>
Cc: [email protected]
---
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)

Reply via email to