[PATCH] spl: bootstage: move bootstage_stash before jumping to image

[PATCH] spl: bootstage: move bootstage_stash before jumping to image

[Chanho Park]

For IH_OS_OPENSBI and IH_OS_LINUX, there is no chance to stash
bootstare record because it will not return after jumping to the image.
Hence, this patch moves the location of bootstage_stash before jumping
to image.

Signed-off-by: Chanho Park <chanho61.park@samsung.com>
---
 common/spl/spl.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 0062f3f45d9a..364d439f65e2 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -850,6 +850,14 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 			       ret);
 	}
 
+	bootstage_mark_name(get_bootstage_id(false), "end phase");
+#ifdef CONFIG_BOOTSTAGE_STASH
+	ret = bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
+			      CONFIG_BOOTSTAGE_STASH_SIZE);
+	if (ret)
+		debug("Failed to stash bootstage: err=%d\n", ret);
+#endif
+
 	switch (spl_image.os) {
 	case IH_OS_U_BOOT:
 		debug("Jumping to %s...\n", spl_phase_name(spl_next_phase()));
@@ -890,13 +898,6 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 	debug("SPL malloc() used 0x%lx bytes (%ld KB)\n", gd->malloc_ptr,
 	      gd->malloc_ptr / 1024);
 #endif
-	bootstage_mark_name(get_bootstage_id(false), "end phase");
-#ifdef CONFIG_BOOTSTAGE_STASH
-	ret = bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
-			      CONFIG_BOOTSTAGE_STASH_SIZE);
-	if (ret)
-		debug("Failed to stash bootstage: err=%d\n", ret);
-#endif
 
 	if (IS_ENABLED(CONFIG_SPL_VIDEO_REMOVE)) {
 		struct udevice *dev;
-- 
2.39.2

Re: [PATCH] spl: bootstage: move bootstage_stash before jumping to image

[Simon Glass]

Hi Chanho,

On Mon, 28 Aug 2023 at 03:46, Chanho Park <chanho61.park@samsung.com> wrote:
>
> For IH_OS_OPENSBI and IH_OS_LINUX, there is no chance to stash
> bootstare record because it will not return after jumping to the image.
> Hence, this patch moves the location of bootstage_stash before jumping
> to image.
>

Please fix your implementation instead. You must jump to the OS at the
end of this function and not before.

In fact I have a patch to move bootstage later in this function!

> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> ---
>  common/spl/spl.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 0062f3f45d9a..364d439f65e2 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -850,6 +850,14 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>                                ret);
>         }
>
> +       bootstage_mark_name(get_bootstage_id(false), "end phase");
> +#ifdef CONFIG_BOOTSTAGE_STASH
> +       ret = bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
> +                             CONFIG_BOOTSTAGE_STASH_SIZE);
> +       if (ret)
> +               debug("Failed to stash bootstage: err=%d\n", ret);
> +#endif
> +
>         switch (spl_image.os) {
>         case IH_OS_U_BOOT:
>                 debug("Jumping to %s...\n", spl_phase_name(spl_next_phase()));
> @@ -890,13 +898,6 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>         debug("SPL malloc() used 0x%lx bytes (%ld KB)\n", gd->malloc_ptr,
>               gd->malloc_ptr / 1024);
>  #endif
> -       bootstage_mark_name(get_bootstage_id(false), "end phase");
> -#ifdef CONFIG_BOOTSTAGE_STASH
> -       ret = bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
> -                             CONFIG_BOOTSTAGE_STASH_SIZE);
> -       if (ret)
> -               debug("Failed to stash bootstage: err=%d\n", ret);
> -#endif
>
>         if (IS_ENABLED(CONFIG_SPL_VIDEO_REMOVE)) {
>                 struct udevice *dev;
> --
> 2.39.2
>

Regards,
Simon

RE: [PATCH] spl: bootstage: move bootstage_stash before jumping to image

[Chanho Park]

Hi Simon,

> -----Original Message-----
> From: Simon Glass <sjg@chromium.org>
> Sent: Tuesday, August 29, 2023 2:55 AM
> To: Chanho Park <chanho61.park@samsung.com>
> Cc: Nikhil M Jain <n-jain1@ti.com>; Marek Vasut <marex@denx.de>; u-
> boot@lists.denx.de
> Subject: Re: [PATCH] spl: bootstage: move bootstage_stash before jumping
> to image
> 
> Hi Chanho,
> 
> On Mon, 28 Aug 2023 at 03:46, Chanho Park <chanho61.park@samsung.com>
> wrote:
> >
> > For IH_OS_OPENSBI and IH_OS_LINUX, there is no chance to stash
> > bootstare record because it will not return after jumping to the image.
> > Hence, this patch moves the location of bootstage_stash before jumping
> > to image.
> >
> 
> Please fix your implementation instead. You must jump to the OS at the end
> of this function and not before.

In case of OS_TEE/OPENSBI/LINUX images, they will not be returned to SPL so they can't get a chance to stash boot stage records.

> 
> In fact I have a patch to move bootstage later in this function!

Will you post the patch soon? Or it was already posted?

Best Regards,
Chanho Park

Re: [PATCH] spl: bootstage: move bootstage_stash before jumping to image

[Simon Glass]

Hi Chanho,

On Mon, 28 Aug 2023 at 21:46, Chanho Park <chanho61.park@samsung.com> wrote:
>
> Hi Simon,
>
> > -----Original Message-----
> > From: Simon Glass <sjg@chromium.org>
> > Sent: Tuesday, August 29, 2023 2:55 AM
> > To: Chanho Park <chanho61.park@samsung.com>
> > Cc: Nikhil M Jain <n-jain1@ti.com>; Marek Vasut <marex@denx.de>; u-
> > boot@lists.denx.de
> > Subject: Re: [PATCH] spl: bootstage: move bootstage_stash before jumping
> > to image
> >
> > Hi Chanho,
> >
> > On Mon, 28 Aug 2023 at 03:46, Chanho Park <chanho61.park@samsung.com>
> > wrote:
> > >
> > > For IH_OS_OPENSBI and IH_OS_LINUX, there is no chance to stash
> > > bootstare record because it will not return after jumping to the image.
> > > Hence, this patch moves the location of bootstage_stash before jumping
> > > to image.
> > >
> >
> > Please fix your implementation instead. You must jump to the OS at the end
> > of this function and not before.
>
> In case of OS_TEE/OPENSBI/LINUX images, they will not be returned to SPL so they can't get a chance to stash boot stage records.
>
> >
> > In fact I have a patch to move bootstage later in this function!
>
> Will you post the patch soon? Or it was already posted?

Not posted yet. I'll reply on your new patch.

Regards,
Simon