sbabic / libubootenv

Generic library and tools to access and modify U-Boot environment from User Space
72 stars 39 forks source link

Error "Cannot read environment, using default" - after upgrade from Yocto gatesgarth to langdale - 0.3.2 and 0.3.3 #25

Open DavidAntliff opened 1 year ago

DavidAntliff commented 1 year ago

I originally posted this as a comment to issue #20, however it's probably easier to manage if I move it into a new issue here.

I have a similar issue to #20 after updating from gatesgarth to langdale - with both libubootenv 0.3.2 and 0.3.3.

Note: this was working for 0.3.2 in gatesgarth but has broken for me in langdale with both 0.3.2 and 0.3.3.

The U-Boot config is stored in SPI Flash, and U-Boot reports this when booting:

Loading Environment from SPIFlash... SF: Detected mt25qu02g with page size 512 Bytes, erase size 128 KiB, total 512 MiB

fw_printenv and fw_setenv were working in gatesgarth with the config:

/dev/mtd0       0x2e00000   0x40000     0x40000       0       1

U-Boot config:

CONFIG_ENV_OFFSET=0x2E00000
CONFIG_ENV_OFFSET_REDUND=0x1E80000
CONFIG_ENV_IS_IN_SPI_FLASH=y
CONFIG_ENV_SIZE=0x40000
CONFIG_ENV_SECT_SIZE=0x40000

But with langdale, and no changes to U-Boot's config, I now see:

# fw_printenv -c fw_env.config bootcmd
Cannot read environment, using default
Cannot read default environment from file

I understand that the second line is due to a missing u-boot-initial-env file. It's the first line I'm concerned about.

Also, with strace, I can see that the environment is actually read from the Flash (I have a variable 'altbootcmd', which appears below), but it is then discarded for some reason and the error emitted instead:

# strace fw_printenv -c fw_env.config bootcmd
...
openat(AT_FDCWD, "/dev/mtd0", O_RDONLY) = 4
lseek(4, 48234496, SEEK_SET)            = 48234496
read(4, "\245a\30\0\0altbootcmd=run sqc_boot_alt"..., 262144) = 262144
close(4)                                = 0
munmap(0xffffba48f000, 266240)          = 0
write(2, "Cannot read environment, using d"..., 39Cannot read environment, using default
) = 39
...

The first bytes of the relevant section of Flash look like this:

# hexdump -C -s 0x2E00000 -n 64 /dev/mtd0
02e00000  a5 61 18 00 00 61 6c 74  62 6f 6f 74 63 6d 64 3d  |.a...altbootcmd=|
02e00010  72 75 6e 20 73 71 63 5f  62 6f 6f 74 5f 61 6c 74  |run sqc_boot_alt|
02e00020  00 61 72 63 68 3d 61 72  6d 00 62 61 75 64 72 61  |.arch=arm.baudra|
02e00030  74 65 3d 31 31 35 32 30  30 00 62 6f 61 72 64 3d  |te=115200.board=|

U-Boot does not report a corrupted environment, and in the course of this testing I have done env default -f -a ; saveenv several times to make sure it is valid.

I'll dig into the source code next, and try to figure out why the 262144 (0x40000) bytes are not processed as a valid environment.

DavidAntliff commented 1 year ago

Ah, it turns out that I had CONFIG_SYS_REDUNDAND_ENVIRONMENT enabled in the langdale project (an upstream default changed, maybe? I did not have this set in gatesgarth), which puts U-Boot into a mode where it maintains two instances of the environment. The structures in libubootenv source, struct uboot_env_noredund and struct uboot_env_redund, differ in the bytes that are included in the CRC. Because my fw_env.config only had one entry, this put fw_printenv into "noredund" mode, and it used the wrong data structure to calculate the CRC, which resulted in a mismatch (error).

There are two possible fixes for my situation:

  1. Modify fw_env.config to supply the two environment sections:
    /dev/mtd0               0x2e00000       0x40000     0x40000       0       1
    /dev/mtd0               0x1e00000       0x40000     0x40000       0       1
  2. Disable redundant environment in U-Boot.

May I suggest a minor improvement where, if the CRC fails in libuboot_load() and copies == 1, it might be helpful to others to include in the error message something along the lines of "Environment CRC failed. Your config file only has one entry - check that U-Boot is not configured to use a redundant environment."

Happy for this issue to be closed if you prefer to leave it as-is. Will leave that up to you. If you approve output similar to above (or suggest something else), let me know here and I can submit a patch via the mailing list to help out.