Re: sandbox: CONFIG_SYS_RELOC_GD_ENV_ADDR?
Hi AKASHI, On Sun, 16 Feb 2020 at 22:01, AKASHI Takahiro wrote: > > 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 > > 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 _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. OK I see, thank you. Yes it seems like this should not be used on sandbox. Regards, Simon [..]
Re: sandbox: CONFIG_SYS_RELOC_GD_ENV_ADDR?
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
> 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 _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 _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.
0x555e3b21 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 0x555e3b21 in env_get_char_spec (index=index@entry=0)
at /home/akashi/arm/armv8/linaro/u-boot/env/env.c:169
#1 0x555e3b3c in env_get_char (index=index@entry=0)
at /home/akashi/arm/armv8/linaro/u-boot/env/env.c:177
#2 0x5559720f in env_get_f (name=0x556c7cdc ".callbacks",
buf=0x156fae80 "115200", len=len@entry=32)
at /home/akashi/arm/armv8/linaro/u-boot/cmd/nvedit.c:708
#3 0x55597332 in env_get (name=name@entry=0x556c7cdc ".callbacks")
at /home/akashi/arm/armv8/linaro/u-boot/cmd/nvedit.c:678
#4 0x555e486d in env_callback_init (var_entry=0x15b111d8)
at /home/akashi/arm/armv8/linaro/u-boot/env/callback.c:54
#5 0x55635c00 in hsearch_r (item=..., action=action@entry=ENV_ENTER,
retval=retval@entry=0x7fffd868,
htab=htab@entry=0x55954670 , flag=flag@entry=0)
at /home/akashi/arm/armv8/linaro/u-boot/lib/hashtable.c:389
#6 0x55636287 in himport_r (htab=htab@entry=0x55954670 ,
Re: sandbox: CONFIG_SYS_RELOC_GD_ENV_ADDR?
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 > 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. If the option needs to be dropped for things to work, we should just drop it. When migrating some of the logic here in to CONFIG symbols I did match, I'm pretty certain, the previous behavior. But there's been a few other cases I got backwards, so this could have been another. So long as 'make tests' is happy after the change, we should just do it. -- Tom signature.asc Description: PGP signature
Re: sandbox: CONFIG_SYS_RELOC_GD_ENV_ADDR?
Hi AKASHI, On Thu, 13 Feb 2020 at 18:39, AKASHI Takahiro 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. Regards, Simon
