Hi Heinrich, On Tue, 23 Mar 2021 at 14:22, Heinrich Schuchardt <[email protected]> wrote: > > Am 23. März 2021 01:57:00 MEZ schrieb Simon Glass <[email protected]>: > >Hi Heinrich, > > > >On Tue, 23 Mar 2021 at 08:45, Heinrich Schuchardt <[email protected]> > >wrote: > >> > >> On 3/22/21 7:16 PM, Simon Glass wrote: > >> > Hi Heinrich, > >> > > >> > On Tue, 23 Mar 2021 at 07:12, Heinrich Schuchardt > ><[email protected]> wrote: > >> >> > >> >> Hello Simon, > >> >> > >> >> using sandbox_defconfig on origin/master: > >> >> > >> >> Hit any key to stop autoboot: 0 > >> >> => exception sigsegv > >> >> > >> >> Segmentation violation > >> >> pc = 0x55d3566d04f9, pc_reloc = 0x554f9 > >> >> > >> >> $ > >> >> > >> >> Here the SIGSEGV is correctly handled by the sandbox. > >> >> > >> >> On origin/next: > >> >> > >> >> => exception sigsegv > >> >> > >> >> Segmentation violation > >> >> pc = 0x5567966da96b, pc_reloc = 0x5567866da96b > >> >> > >> >> Writing sandbox state > >> >> Segmentation fault > >> >> $ > >> >> > >> >> The same problem is visible when executing the poweroff command. > >> >> > >> >> => poweroff > >> >> poweroff ... > >> >> Segmentation fault > >> >> $ > >> >> > >> >> Bisecting points to your commit > >> >> > >> >> b308d9fd18fa > >> >> sandbox: Avoid using malloc() for system state > >> >> > >> >> The segmentation fault occurs when os_exit() calls dm_uninit(). > >> >> The value of gd is invalid at this point. > >> > > >> > Can you please check this patch? > >> > > >> > > >http://patchwork.ozlabs.org/project/uboot/patch/[email protected]/ > >> > > >> > Also, is there no test covering the above? > >> > > >> > Regards, > >> > Simon > >> > > >> > >> Hello Simon, > >> > >> We have a poweroff test but there is no detection for the > >'Segmentation > >> fault' string. > >> > >> For CONFIG_SANDBOX_CRASH_RESET=n the patch helps. > >> > >> For CONFIG_SANDBOX_CRASH_RESET=y you still get a segmentation fault > >when > >> executing 'exception sigsegv'. > > > >OK, is that caused by the same commit? > > Yes > > > The problem is that the commit > >is actually fixing a bug. I'll need to think about how to fix the fix. > > > >> > >> Unfortunately you decided to disable CONFIG_SANDBOX_CRASH_RESET in > >> sandbox_defconfig. Otherwise you would have detected the problem as > >> "FAILED test/py/tests/test_sandbox_exit.py::test_exception_reset". > > > >Well we don't generally want to reset when we get a crash, right? > > > > Resetting after a crash is what all other boards do. > > I personally don't have a need for the sandbox to behave differently.
I see sandbox as a normal program. If you run grimp and it crashes, it doesn't reset. The only reason we have a reset feature in sandbox is for testing purposes. > > >> > >> Please, adjust sandbox_reset(). > > > >What would you like it to do? > > Executing the 'reset' command fails with a crash. > > The crash occurs when dm_uninit() is invoked. This is definitely a tricky area. Since devices are allocated in the 'emulated' RAM we really can't close down sandbox before uniniting devices, and vice versa. Some separation is needed. I will take a look at this at some point, but it needs a fair bit of thought. Regards, Simon
