[PATCH] spl: Jump to image at end of board_init_r
spl_board_prepare_for_boot() is not called before jumping/invoking atf, optee, opensbi or linux images.
Jump to image at the end of board_init_r() to fix this.
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- This patch have dependencies on the following patches:
spl: add __noreturn attribute to spl_invoke_opensbi function https://patchwork.ozlabs.org/patch/1827057/
spl: add __noreturn attribute to spl_invoke_atf function https://patchwork.ozlabs.org/patch/1831366/
spl: Drop the switch() statement for OS selection from the "spl: Preparation for Universal Payload" series https://patchwork.ozlabs.org/patch/1839731/ --- common/spl/spl.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index f7608f14e365..79c39820262a 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -647,6 +647,8 @@ void board_init_r(gd_t *dummy1, ulong dummy2) BOOT_DEVICE_NONE, BOOT_DEVICE_NONE, }; + typedef void __noreturn (*jump_to_image_t)(struct spl_image_info *); + jump_to_image_t jump_to_image = &jump_to_image_no_args; struct spl_image_info spl_image; int ret, os;
@@ -735,20 +737,20 @@ void board_init_r(gd_t *dummy1, ulong dummy2) } else if (CONFIG_IS_ENABLED(ATF) && os == IH_OS_ARM_TRUSTED_FIRMWARE) { debug("Jumping to U-Boot via ARM Trusted Firmware\n"); spl_fixup_fdt(spl_image_fdt_addr(&spl_image)); - spl_invoke_atf(&spl_image); + jump_to_image = &spl_invoke_atf; } else if (CONFIG_IS_ENABLED(OPTEE_IMAGE) && os == IH_OS_TEE) { debug("Jumping to U-Boot via OP-TEE\n"); spl_board_prepare_for_optee(spl_image_fdt_addr(&spl_image)); - jump_to_image_optee(&spl_image); + jump_to_image = &jump_to_image_optee; } else if (CONFIG_IS_ENABLED(OPENSBI) && os == IH_OS_OPENSBI) { debug("Jumping to U-Boot via RISC-V OpenSBI\n"); - spl_invoke_opensbi(&spl_image); + jump_to_image = &spl_invoke_opensbi; } else if (CONFIG_IS_ENABLED(OS_BOOT) && os == IH_OS_LINUX) { debug("Jumping to Linux\n"); if (IS_ENABLED(CONFIG_SPL_OS_BOOT)) spl_fixup_fdt((void *)SPL_PAYLOAD_ARGS_ADDR); spl_board_prepare_for_linux(); - jump_to_image_linux(&spl_image); + jump_to_image = &jump_to_image_linux; } else { debug("Unsupported OS image.. Jumping nevertheless..\n"); } @@ -788,7 +790,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2) }
spl_board_prepare_for_boot(); - jump_to_image_no_args(&spl_image); + jump_to_image(&spl_image); }
/*
On Wed, 27 Sept 2023 at 15:44, Jonas Karlman jonas@kwiboo.se wrote:
spl_board_prepare_for_boot() is not called before jumping/invoking atf, optee, opensbi or linux images.
Jump to image at the end of board_init_r() to fix this.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
This patch have dependencies on the following patches:
spl: add __noreturn attribute to spl_invoke_opensbi function https://patchwork.ozlabs.org/patch/1827057/
spl: add __noreturn attribute to spl_invoke_atf function https://patchwork.ozlabs.org/patch/1831366/
spl: Drop the switch() statement for OS selection from the "spl: Preparation for Universal Payload" series https://patchwork.ozlabs.org/patch/1839731/
common/spl/spl.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Very nice to see this!
On Fri Sep 29, 2023 at 1:42 AM CEST, Simon Glass wrote:
On Wed, 27 Sept 2023 at 15:44, Jonas Karlman jonas@kwiboo.se wrote:
spl_board_prepare_for_boot() is not called before jumping/invoking atf, optee, opensbi or linux images.
Jump to image at the end of board_init_r() to fix this.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
This patch have dependencies on the following patches:
spl: add __noreturn attribute to spl_invoke_opensbi function https://patchwork.ozlabs.org/patch/1827057/
spl: add __noreturn attribute to spl_invoke_atf function https://patchwork.ozlabs.org/patch/1831366/
spl: Drop the switch() statement for OS selection from the "spl: Preparation for Universal Payload" series https://patchwork.ozlabs.org/patch/1839731/
common/spl/spl.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Very nice to see this!
Nice indeed!
On 9/27/23 23:44, Jonas Karlman wrote:
spl_board_prepare_for_boot() is not called before jumping/invoking atf, optee, opensbi or linux images.
Jump to image at the end of board_init_r() to fix this.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
This patch have dependencies on the following patches:
spl: add __noreturn attribute to spl_invoke_opensbi function https://patchwork.ozlabs.org/patch/1827057/
spl: add __noreturn attribute to spl_invoke_atf function https://patchwork.ozlabs.org/patch/1831366/
spl: Drop the switch() statement for OS selection from the "spl: Preparation for Universal Payload" series https://patchwork.ozlabs.org/patch/1839731/
common/spl/spl.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index f7608f14e365..79c39820262a 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -647,6 +647,8 @@ void board_init_r(gd_t *dummy1, ulong dummy2) BOOT_DEVICE_NONE, BOOT_DEVICE_NONE, };
- typedef void __noreturn (*jump_to_image_t)(struct spl_image_info *);
- jump_to_image_t jump_to_image = &jump_to_image_no_args; struct spl_image_info spl_image; int ret, os;
@@ -735,20 +737,20 @@ void board_init_r(gd_t *dummy1, ulong dummy2) } else if (CONFIG_IS_ENABLED(ATF) && os == IH_OS_ARM_TRUSTED_FIRMWARE) { debug("Jumping to U-Boot via ARM Trusted Firmware\n"); spl_fixup_fdt(spl_image_fdt_addr(&spl_image));
spl_invoke_atf(&spl_image);
jump_to_image = &spl_invoke_atf;
jump_to_image_optee(&spl_image);
jump_to_image = &jump_to_image_optee;
spl_invoke_opensbi(&spl_image);
jump_to_image = &spl_invoke_opensbi;
jump_to_image_linux(&spl_image);
jump_to_image = &jump_to_image_linux;@@ -788,7 +790,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2) }
spl_board_prepare_for_boot();
- jump_to_image_no_args(&spl_image);
jump_to_image(&spl_image); }
/*
In SPL we are fighting for every byte of binary size.
What is the impact of this change on the code size?
I would expect that your increasing it; especially if only one of the CONFIG_OPTIONS is enabled.
If so, NAK to this patch despite all elegance.
Best regards
Heinrich
Hi Heinrich,
On Fri Sep 29, 2023 at 2:57 AM CEST, Heinrich Schuchardt wrote:
In SPL we are fighting for every byte of binary size.
What is the impact of this change on the code size?
I would expect that your increasing it; especially if only one of the CONFIG_OPTIONS is enabled.
If so, NAK to this patch despite all elegance.
Would you prefer removing `spl_board_prepare_for_boot()` instead? A handful of boards make use for it so I'd say this is a bug if it never gets ran on those.
Or would it be better if it was enabled with a CONFIG_ option?
Cheers.
Best regards
Heinrich
On 9/29/23 09:55, Ferass El Hafidi wrote:
Hi Heinrich,
On Fri Sep 29, 2023 at 2:57 AM CEST, Heinrich Schuchardt wrote:
In SPL we are fighting for every byte of binary size.
What is the impact of this change on the code size?
I would expect that your increasing it; especially if only one of the CONFIG_OPTIONS is enabled.
If so, NAK to this patch despite all elegance.
Would you prefer removing `spl_board_prepare_for_boot()` instead? A handful of boards make use for it so I'd say this is a bug if it never gets ran on those.
Or would it be better if it was enabled with a CONFIG_ option?
We should understand the implications of your patch on the code size. If it leads to binary code growth, we should evaluate alternatives.
Best regards
Heinrich
On Fri, Sep 29, 2023 at 02:57:42AM +0200, Heinrich Schuchardt wrote:
On 9/27/23 23:44, Jonas Karlman wrote:
spl_board_prepare_for_boot() is not called before jumping/invoking atf, optee, opensbi or linux images.
Jump to image at the end of board_init_r() to fix this.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
This patch have dependencies on the following patches:
spl: add __noreturn attribute to spl_invoke_opensbi function https://patchwork.ozlabs.org/patch/1827057/
spl: add __noreturn attribute to spl_invoke_atf function https://patchwork.ozlabs.org/patch/1831366/
spl: Drop the switch() statement for OS selection from the "spl: Preparation for Universal Payload" series https://patchwork.ozlabs.org/patch/1839731/
common/spl/spl.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index f7608f14e365..79c39820262a 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -647,6 +647,8 @@ void board_init_r(gd_t *dummy1, ulong dummy2) BOOT_DEVICE_NONE, BOOT_DEVICE_NONE, };
- typedef void __noreturn (*jump_to_image_t)(struct spl_image_info *);
- jump_to_image_t jump_to_image = &jump_to_image_no_args; struct spl_image_info spl_image; int ret, os;
@@ -735,20 +737,20 @@ void board_init_r(gd_t *dummy1, ulong dummy2) } else if (CONFIG_IS_ENABLED(ATF) && os == IH_OS_ARM_TRUSTED_FIRMWARE) { debug("Jumping to U-Boot via ARM Trusted Firmware\n"); spl_fixup_fdt(spl_image_fdt_addr(&spl_image));
spl_invoke_atf(&spl_image);
jump_to_image = &spl_invoke_atf;
jump_to_image_optee(&spl_image);
jump_to_image = &jump_to_image_optee;
spl_invoke_opensbi(&spl_image);
jump_to_image = &spl_invoke_opensbi;
jump_to_image_linux(&spl_image);
jump_to_image = &jump_to_image_linux;@@ -788,7 +790,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2) }
spl_board_prepare_for_boot();
- jump_to_image_no_args(&spl_image);
jump_to_image(&spl_image); }
/*
In SPL we are fighting for every byte of binary size.
What is the impact of this change on the code size?
I would expect that your increasing it; especially if only one of the CONFIG_OPTIONS is enabled.
If so, NAK to this patch despite all elegance.
We aren't _that_ strict, no. And a very quick peek shows that this seems fine overall. Since you raised the question I'll do a quick world build but socfpga_agilex_vab (as a config I had size change results for in front of me for something else) shrank by 4 bytes with just the prerequsites and this patch applied.
On 29.09.23 13:56, Tom Rini wrote:
On Fri, Sep 29, 2023 at 02:57:42AM +0200, Heinrich Schuchardt wrote:
On 9/27/23 23:44, Jonas Karlman wrote:
spl_board_prepare_for_boot() is not called before jumping/invoking atf, optee, opensbi or linux images.
Jump to image at the end of board_init_r() to fix this.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
This patch have dependencies on the following patches:
spl: add __noreturn attribute to spl_invoke_opensbi function https://patchwork.ozlabs.org/patch/1827057/
spl: add __noreturn attribute to spl_invoke_atf function https://patchwork.ozlabs.org/patch/1831366/
spl: Drop the switch() statement for OS selection from the "spl: Preparation for Universal Payload" series https://patchwork.ozlabs.org/patch/1839731/
common/spl/spl.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index f7608f14e365..79c39820262a 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -647,6 +647,8 @@ void board_init_r(gd_t *dummy1, ulong dummy2) BOOT_DEVICE_NONE, BOOT_DEVICE_NONE, };
- typedef void __noreturn (*jump_to_image_t)(struct spl_image_info *);
- jump_to_image_t jump_to_image = &jump_to_image_no_args; struct spl_image_info spl_image; int ret, os;
@@ -735,20 +737,20 @@ void board_init_r(gd_t *dummy1, ulong dummy2) } else if (CONFIG_IS_ENABLED(ATF) && os == IH_OS_ARM_TRUSTED_FIRMWARE) { debug("Jumping to U-Boot via ARM Trusted Firmware\n"); spl_fixup_fdt(spl_image_fdt_addr(&spl_image));
spl_invoke_atf(&spl_image);
jump_to_image = &spl_invoke_atf;
jump_to_image_optee(&spl_image);
jump_to_image = &jump_to_image_optee;
spl_invoke_opensbi(&spl_image);
jump_to_image = &spl_invoke_opensbi;
jump_to_image_linux(&spl_image);
jump_to_image = &jump_to_image_linux;@@ -788,7 +790,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2) }
spl_board_prepare_for_boot();
- jump_to_image_no_args(&spl_image);
jump_to_image(&spl_image); }
/*
In SPL we are fighting for every byte of binary size.
What is the impact of this change on the code size?
I would expect that your increasing it; especially if only one of the CONFIG_OPTIONS is enabled.
If so, NAK to this patch despite all elegance.
We aren't _that_ strict, no. And a very quick peek shows that this seems fine overall. Since you raised the question I'll do a quick world build but socfpga_agilex_vab (as a config I had size change results for in front of me for something else) shrank by 4 bytes with just the prerequsites and this patch applied.
Shrinking the size sounds great. Thanks for measuring.
Best regards
Heinrich
On Fri, Sep 29, 2023 at 07:56:49AM -0400, Tom Rini wrote:
On Fri, Sep 29, 2023 at 02:57:42AM +0200, Heinrich Schuchardt wrote:
On 9/27/23 23:44, Jonas Karlman wrote:
spl_board_prepare_for_boot() is not called before jumping/invoking atf, optee, opensbi or linux images.
Jump to image at the end of board_init_r() to fix this.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
This patch have dependencies on the following patches:
spl: add __noreturn attribute to spl_invoke_opensbi function https://patchwork.ozlabs.org/patch/1827057/
spl: add __noreturn attribute to spl_invoke_atf function https://patchwork.ozlabs.org/patch/1831366/
spl: Drop the switch() statement for OS selection from the "spl: Preparation for Universal Payload" series https://patchwork.ozlabs.org/patch/1839731/
common/spl/spl.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index f7608f14e365..79c39820262a 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -647,6 +647,8 @@ void board_init_r(gd_t *dummy1, ulong dummy2) BOOT_DEVICE_NONE, BOOT_DEVICE_NONE, };
- typedef void __noreturn (*jump_to_image_t)(struct spl_image_info *);
- jump_to_image_t jump_to_image = &jump_to_image_no_args; struct spl_image_info spl_image; int ret, os;
@@ -735,20 +737,20 @@ void board_init_r(gd_t *dummy1, ulong dummy2) } else if (CONFIG_IS_ENABLED(ATF) && os == IH_OS_ARM_TRUSTED_FIRMWARE) { debug("Jumping to U-Boot via ARM Trusted Firmware\n"); spl_fixup_fdt(spl_image_fdt_addr(&spl_image));
spl_invoke_atf(&spl_image);
jump_to_image = &spl_invoke_atf;
jump_to_image_optee(&spl_image);
jump_to_image = &jump_to_image_optee;
spl_invoke_opensbi(&spl_image);
jump_to_image = &spl_invoke_opensbi;
jump_to_image_linux(&spl_image);
jump_to_image = &jump_to_image_linux;@@ -788,7 +790,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2) }
spl_board_prepare_for_boot();
- jump_to_image_no_args(&spl_image);
jump_to_image(&spl_image); }
/*
In SPL we are fighting for every byte of binary size.
What is the impact of this change on the code size?
I would expect that your increasing it; especially if only one of the CONFIG_OPTIONS is enabled.
If so, NAK to this patch despite all elegance.
We aren't _that_ strict, no. And a very quick peek shows that this seems fine overall. Since you raised the question I'll do a quick world build but socfpga_agilex_vab (as a config I had size change results for in front of me for something else) shrank by 4 bytes with just the prerequsites and this patch applied.
Yes, this generally is a small shrink or a small growth and fine.
On Wed, Sep 27, 2023 at 09:44:13PM +0000, Jonas Karlman wrote:
spl_board_prepare_for_boot() is not called before jumping/invoking atf, optee, opensbi or linux images.
Jump to image at the end of board_init_r() to fix this.
Signed-off-by: Jonas Karlman jonas@kwiboo.se Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (5)
-
Ferass El Hafidi -
Heinrich Schuchardt -
Jonas Karlman -
Simon Glass -
Tom Rini