[PATCH] Makefile: doesn't need check stack size when dtb is not built
The commit 5fed97af20da ("Makefile: ensure DTB doesn't overflow into initial stack") adds an extra check for stack size in BSS if CONFIG_SYS_INIT_SP_BSS_OFFSET is enabled. This check, however, doesn't make sense under the configuration where control dtb won't be built in and it should be void in such cases.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org Fixes: 5fed97af20da ("Makefile: ensure DTB doesn't overflow into initial stack") --- Makefile | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index 0af89e0a7881..78efd7e9e250 100644 --- a/Makefile +++ b/Makefile @@ -1208,7 +1208,10 @@ binary_size_check: u-boot-nodtb.bin FORCE fi \ fi
-ifdef CONFIG_INIT_SP_RELATIVE +ifeq ($(CONFIG_INIT_SP_RELATIVE),y) +ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),) +# u-boot.dtb will be built only if one of those options is enabled + ifneq ($(CONFIG_SYS_MALLOC_F_LEN),) subtract_sys_malloc_f_len = space=$$(($${space} - $(CONFIG_SYS_MALLOC_F_LEN))) else @@ -1233,6 +1236,10 @@ init_sp_bss_offset_check: u-boot.dtb FORCE echo "(CONFIG_SYS_INIT_SP_BSS_OFFSET - CONFIG_SYS_MALLOC_F_LEN)" >&2 ; \ exit 1 ; \ fi +else +init_sp_bss_offset_check: + +endif endif
u-boot-nodtb.bin: u-boot FORCE
On 3/3/20 11:54 PM, AKASHI Takahiro wrote:
The commit 5fed97af20da ("Makefile: ensure DTB doesn't overflow into initial stack") adds an extra check for stack size in BSS if CONFIG_SYS_INIT_SP_BSS_OFFSET is enabled. This check, however, doesn't make sense under the configuration where control dtb won't be built in and it should be void in such cases.
Don't you want to edit the following hunk from the original patch instead or as well?
+ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),) +ALL-y += init_sp_bss_offset_check +endif
That's why there's no rule for init_sp_bss_offset_check in the else case.
On Wed, Mar 04, 2020 at 09:21:29AM -0700, Stephen Warren wrote:
On 3/3/20 11:54 PM, AKASHI Takahiro wrote:
The commit 5fed97af20da ("Makefile: ensure DTB doesn't overflow into initial stack") adds an extra check for stack size in BSS if CONFIG_SYS_INIT_SP_BSS_OFFSET is enabled. This check, however, doesn't make sense under the configuration where control dtb won't be built in and it should be void in such cases.
Don't you want to edit the following hunk from the original patch instead or as well?
+ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),) +ALL-y += init_sp_bss_offset_check +endif
That's why there's no rule for init_sp_bss_offset_check in the else case.
I intentionally left it as it is because someone may in the future want to add other *sanity checks* whether dtb is used or not. Rather, my concern is: Is "ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),)" sufficient and appropriate to guard the check?
Thanks, -Takahiro Akashi
On 3/4/20 5:15 PM, AKASHI Takahiro wrote:
On Wed, Mar 04, 2020 at 09:21:29AM -0700, Stephen Warren wrote:
On 3/3/20 11:54 PM, AKASHI Takahiro wrote:
The commit 5fed97af20da ("Makefile: ensure DTB doesn't overflow into initial stack") adds an extra check for stack size in BSS if CONFIG_SYS_INIT_SP_BSS_OFFSET is enabled. This check, however, doesn't make sense under the configuration where control dtb won't be built in and it should be void in such cases.
Don't you want to edit the following hunk from the original patch instead or as well?
+ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),) +ALL-y += init_sp_bss_offset_check +endif
That's why there's no rule for init_sp_bss_offset_check in the else case.
I intentionally left it as it is because someone may in the future want to add other *sanity checks* whether dtb is used or not.
I'd probably expect the following in that case:
ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),) ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),) ALL-y += init_sp_bss_offset_check endif ALL-y += some_other_check endif
Rather, my concern is: Is "ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),)" sufficient and appropriate to guard the check?
I think OF_SEPARATE is the only case this check makes sense. OF_EMBED sounds like the build process puts the DTB into .data or similar, so the issue doesn't apply. Since OF_HOSTFILE is a sandbox thing where the DT is read from a file, the check definitely doesn't apply.
On Wed, Mar 04, 2020 at 05:22:25PM -0700, Stephen Warren wrote:
On 3/4/20 5:15 PM, AKASHI Takahiro wrote:
On Wed, Mar 04, 2020 at 09:21:29AM -0700, Stephen Warren wrote:
On 3/3/20 11:54 PM, AKASHI Takahiro wrote:
The commit 5fed97af20da ("Makefile: ensure DTB doesn't overflow into initial stack") adds an extra check for stack size in BSS if CONFIG_SYS_INIT_SP_BSS_OFFSET is enabled. This check, however, doesn't make sense under the configuration where control dtb won't be built in and it should be void in such cases.
Don't you want to edit the following hunk from the original patch instead or as well?
+ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),) +ALL-y += init_sp_bss_offset_check +endif
That's why there's no rule for init_sp_bss_offset_check in the else case.
I intentionally left it as it is because someone may in the future want to add other *sanity checks* whether dtb is used or not.
I'd probably expect the following in that case:
ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),) ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),) ALL-y += init_sp_bss_offset_check endif ALL-y += some_other_check endif
Rather, my concern is: Is "ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),)" sufficient and appropriate to guard the check?
I think OF_SEPARATE is the only case this check makes sense. OF_EMBED sounds like the build process puts the DTB into .data or similar, so the issue doesn't apply. Since OF_HOSTFILE is a sandbox thing where the DT is read from a file, the check definitely doesn't apply.
Okay, sounds reasonable. (and the target should be dts/dt.dtb rather than u-boot.dtb for clarity?)
Thanks, -Takahiro Akashi
On 3/4/20 5:39 PM, AKASHI Takahiro wrote:
On Wed, Mar 04, 2020 at 05:22:25PM -0700, Stephen Warren wrote:
On 3/4/20 5:15 PM, AKASHI Takahiro wrote:
On Wed, Mar 04, 2020 at 09:21:29AM -0700, Stephen Warren wrote:
On 3/3/20 11:54 PM, AKASHI Takahiro wrote:
The commit 5fed97af20da ("Makefile: ensure DTB doesn't overflow into initial stack") adds an extra check for stack size in BSS if CONFIG_SYS_INIT_SP_BSS_OFFSET is enabled. This check, however, doesn't make sense under the configuration where control dtb won't be built in and it should be void in such cases.
Don't you want to edit the following hunk from the original patch instead or as well?
+ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),) +ALL-y += init_sp_bss_offset_check +endif
That's why there's no rule for init_sp_bss_offset_check in the else case.
I intentionally left it as it is because someone may in the future want to add other *sanity checks* whether dtb is used or not.
I'd probably expect the following in that case:
ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),) ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),) ALL-y += init_sp_bss_offset_check endif ALL-y += some_other_check endif
Rather, my concern is: Is "ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),)" sufficient and appropriate to guard the check?
I think OF_SEPARATE is the only case this check makes sense. OF_EMBED sounds like the build process puts the DTB into .data or similar, so the issue doesn't apply. Since OF_HOSTFILE is a sandbox thing where the DT is read from a file, the check definitely doesn't apply.
Okay, sounds reasonable. (and the target should be dts/dt.dtb rather than u-boot.dtb for clarity?)
By "target" I assume you mean the DTB filename in the following?
@dtb_size=$(shell wc -c u-boot.dtb | awk '{print $$1}') ; \
If so, it doesn't make any difference; u-boot.dtb is just a copy of dts/dt.dtb:
u-boot.dtb: dts/dt.dtb $(call cmd,copy)
participants (2)
-
AKASHI Takahiro -
Stephen Warren