[PATCH 1/1] sandbox: fix sandbox_reset()

[PATCH 1/1] sandbox: fix sandbox_reset()

[Heinrich Schuchardt]

state_uninit() and dm_uninit() are mutually exclusive:

state_uninit() prints via drivers. So it cannot be executed after
dm_uninit().

dm_uninit() requires memory. So it cannot be executed after state_uninit()
which releases all memory.

Just skip dm_uninit() when resetting the sandbox. We will wake up in a new
process and allocate new memory. So this cleanup is not required. We don't
do it in sandbox_exit() either.

This avoids a segmentation error when efi_reset_system_boottime() is
invoked by a UEFI application.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 arch/sandbox/cpu/start.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index e87365e800..4ffd97ccbc 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -425,9 +425,6 @@ void sandbox_reset(void)
 	if (state_uninit())
 		os_exit(2);

-	if (dm_uninit())
-		os_exit(2);
-
 	/* Restart U-Boot */
 	os_relaunch(os_argv);
 }
--
2.30.2

[PATCH 1/1] sandbox: fix sandbox_reset()

[Vincent Stehlé]

On Wed, May 12, 2021 at 06:38:51PM +0200, Heinrich Schuchardt wrote:
> state_uninit() and dm_uninit() are mutually exclusive:
> 
> state_uninit() prints via drivers. So it cannot be executed after
> dm_uninit().
> 
> dm_uninit() requires memory. So it cannot be executed after state_uninit()
> which releases all memory.
> 
> Just skip dm_uninit() when resetting the sandbox. We will wake up in a new
> process and allocate new memory. So this cleanup is not required. We don't
> do it in sandbox_exit() either.
> 
> This avoids a segmentation error when efi_reset_system_boottime() is
> invoked by a UEFI application.

Hi Heinrich,

Thanks for fixing this!

Before, I was hitting the following segfault with the sandbox under qemu arm64
when running the UEFI SCT:

    Boot services test: ExitBootServices_Conf

    Iterations: 1/1

  System will cold reset after 2 second and test will be resumed after reboot.resetting ...
  Writing sandbox state
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped

With your patch I do not hit this segfault anymore.

FWIW, feel free to add (or not):

  Tested-by: Vincent Stehl? <vincent.stehle@arm.com>

Best regards,
Vincent.

> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  arch/sandbox/cpu/start.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
> index e87365e800..4ffd97ccbc 100644
> --- a/arch/sandbox/cpu/start.c
> +++ b/arch/sandbox/cpu/start.c
> @@ -425,9 +425,6 @@ void sandbox_reset(void)
>  	if (state_uninit())
>  		os_exit(2);
> 
> -	if (dm_uninit())
> -		os_exit(2);
> -
>  	/* Restart U-Boot */
>  	os_relaunch(os_argv);
>  }
> --
> 2.30.2
> 

[PATCH 1/1] sandbox: fix sandbox_reset()

[Simon Glass]

On Wed, 12 May 2021 at 10:39, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> state_uninit() and dm_uninit() are mutually exclusive:
>
> state_uninit() prints via drivers. So it cannot be executed after
> dm_uninit().
>
> dm_uninit() requires memory. So it cannot be executed after state_uninit()
> which releases all memory.
>
> Just skip dm_uninit() when resetting the sandbox. We will wake up in a new
> process and allocate new memory. So this cleanup is not required. We don't
> do it in sandbox_exit() either.
>
> This avoids a segmentation error when efi_reset_system_boottime() is
> invoked by a UEFI application.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  arch/sandbox/cpu/start.c | 3 ---
>  1 file changed, 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 1/1] sandbox: fix sandbox_reset()

[Simon Glass]

On Wed, 12 May 2021 at 10:39, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> state_uninit() and dm_uninit() are mutually exclusive:
>
> state_uninit() prints via drivers. So it cannot be executed after
> dm_uninit().
>
> dm_uninit() requires memory. So it cannot be executed after state_uninit()
> which releases all memory.
>
> Just skip dm_uninit() when resetting the sandbox. We will wake up in a new
> process and allocate new memory. So this cleanup is not required. We don't
> do it in sandbox_exit() either.
>
> This avoids a segmentation error when efi_reset_system_boottime() is
> invoked by a UEFI application.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  arch/sandbox/cpu/start.c | 3 ---
>  1 file changed, 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm/next, thanks!