Tom, Simon,

On Fri, Feb 14, 2020 at 12:16:33PM -0700, Simon Glass wrote:

Hi AKASHI,

On Thu, 13 Feb 2020 at 18:39, AKASHI Takahiro takahiro.akashi@linaro.org wrote:

...

Tom, Simon,

Is CONFIG_SYS_RELOC_GD_ENV_ADDR really needed on sandbox?

When I try to have a variable environment on emulated SPI flash, the U-Boot binary always crashes: (NOTE: assuming CONFIG_ENV_ADDR == 0) $ dd if=/dev/zero of=./spi.bin bs=1M count=4 $ u-boot -T U-Boot 2020.04-rc2-00015-gc9afef2b1938-dirty (Feb 14 2020 - 10:24:59 +0900)

Model: sandbox
DRAM:  128 MiB
WDT:   Started with servicing (60s timeout)
MMC:   mmc2: 2 (SD), mmc1: 1 (SD), mmc0: 0 (SD)
Loading Environment from SPI Flash... SF: Detected m25p16 with page size 256 Bytes, erase size 64 KiB, total 2 MiB
*** Warning - bad CRC, using default environment

Segmentation fault (core dumped)

If this configuration is disabled, panic doesn't happen. I think that it should be turned off in any sandbox*_defconfig.

In addition, please update

• doc/arch/sandbox.rst
• doc/SPI/README.sandbox-spi

Both two still mention already-removed command line option, --spi_sf. It is confusing.

I'm not an expert on this, but I can't see any use for this in sandbox. One problem might be that it should be using map_sysmem() instead of a cast.

No, map_sysmem() doesn't make sense here because gd->env_addr will be set to &default_environment[0], which is a global variable, not any (emulated) physical memory address, in env_init().

Please note that 'CONFIG_ENV_ADDR != 0' case doesn't make sense for SPI flash because it is not a "memory-mapped" memory in general.

More details:

In board_init_f(), * env_init() is called, and SPI flash is selected as backing storage. Then, gd->env_addr is set to &default_environment[0], with gd->env_valid set to ENV_VALID(!), because the initial content of SPI flash is all zero'ed.

After relocation, in board_init_r(), * initr_reloc_global_data() is called and gd->env_addr is forced to be offset with gd->reloc_off. * env_relocate() is called in intr_env(), then env_load() as gd->env_valid is ENV_VALID. Called in turns are: (Please also follow the backtrace output of gdb attached below for details.)

env_sf_load() env_import() himport_r() hsearch_r() env_callback_init() env_get(".callbacks") <== (1) env_get_f() env_get_char() <== (2) env_get_char_spec()

At (1), env_get_f() is called as GD_FLG_ENV_READY bit in gd->flags is not set yet. At (2), env_get_char_spec() is called as gd->env_valid is ENV_VALID(!).

As a result, env_get_char_spec() tries to access memory pointed to by gd->env_addr and causes a segmentation fault because it has been wrongly "relocated" due to CONFIG_SYS_RELOC_GD_ENV_ADDR.

Again, gd->env_addr should never be "relocated" here for sandbox as it is just a pointer to some global variable (in this case). (In other words, all the addresses of global variables have already been "relocated" from the beginning on sandbox. Right?)

Any thoughts?

Thanks, -Takahiro Akashi

===8<=== $ gdb u-boot (gdb) run -T Starting program: /home/akashi/x86/build/uboot_sandbox_cod/u-boot -T [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

U-Boot 2020.04-rc2-00017-g0e9304ed9276 (Feb 17 2020 - 14:13:04 +0900)

Model: sandbox DRAM: 128 MiB WDT: Started with servicing (60s timeout) MMC: mmc2: 2 (SD), mmc1: 1 (SD), mmc0: 0 (SD) Loading Environment from SPI Flash... SF: Detected m25p16 with page size 256 Bytes, erase size 64 KiB, total 2 MiB

Program received signal SIGSEGV, Segmentation fault. 0x00005555555e3b21 in env_get_char_spec (index=index@entry=0) at /home/akashi/arm/armv8/linaro/u-boot/env/env.c:169 169 return *(uchar *)(gd->env_addr + index); (gdb) bt #0 0x00005555555e3b21 in env_get_char_spec (index=index@entry=0) at /home/akashi/arm/armv8/linaro/u-boot/env/env.c:169 #1 0x00005555555e3b3c in env_get_char (index=index@entry=0) at /home/akashi/arm/armv8/linaro/u-boot/env/env.c:177 #2 0x000055555559720f in env_get_f (name=0x5555556c7cdc ".callbacks", buf=0x156fae80 "115200", len=len@entry=32) at /home/akashi/arm/armv8/linaro/u-boot/cmd/nvedit.c:708 #3 0x0000555555597332 in env_get (name=name@entry=0x5555556c7cdc ".callbacks") at /home/akashi/arm/armv8/linaro/u-boot/cmd/nvedit.c:678 #4 0x00005555555e486d in env_callback_init (var_entry=0x15b111d8) at /home/akashi/arm/armv8/linaro/u-boot/env/callback.c:54 #5 0x0000555555635c00 in hsearch_r (item=..., action=action@entry=ENV_ENTER, retval=retval@entry=0x7fffffffd868, htab=htab@entry=0x555555954670 <env_htab>, flag=flag@entry=0) at /home/akashi/arm/armv8/linaro/u-boot/lib/hashtable.c:389 #6 0x0000555555636287 in himport_r (htab=htab@entry=0x555555954670 <env_htab>, env=env@entry=0x15710154 "arch=sandbox", size=size@entry=2097148, sep=sep@entry=0 '\000', flag=flag@entry=0, crlf_is_lf=crlf_is_lf@entry=0, nvars=0, vars=0x0) at /home/akashi/arm/armv8/linaro/u-boot/lib/hashtable.c:935 #7 0x00005555555e37fd in env_import ( buf=buf@entry=0x15710150 "\352|\240\265arch=sandbox", check=check@entry=1) at /home/akashi/arm/armv8/linaro/u-boot/env/common.c:125 #8 0x00005555555e49ae in env_sf_load () at /home/akashi/arm/armv8/linaro/u-boot/env/sf.c:274 #9 0x00005555555e3bab in env_load () at /home/akashi/arm/armv8/linaro/u-boot/env/env.c:201 #10 0x00005555555e38f1 in env_relocate () at /home/akashi/arm/armv8/linaro/u-boot/env/common.c:242 #11 0x000055555559a4b0 in initr_env () at /home/akashi/arm/armv8/linaro/u-boot/common/board_r.c:480 #12 0x000055555559a6a6 in initcall_run_list ( init_sequence=0x55555594ff00 <init_sequence_r>) at /home/akashi/arm/armv8/linaro/u-boot/include/initcall.h:40 #13 board_init_r (new_gd=<optimized out>, dest_addr=<optimized out>) at /home/akashi/arm/armv8/linaro/u-boot/common/board_r.c:900 #14 0x000055555557a97a in main (argc=2, argv=0x7fffffffdc98) at /home/akashi/arm/armv8/linaro/u-boot/arch/sandbox/cpu/start.c:435