* [PATCH] Makefile: doesn't need check stack size when dtb is not built
@ 2020-03-04 6:54 AKASHI Takahiro
2020-03-04 16:21 ` Stephen Warren
0 siblings, 1 reply; 6+ messages in thread
From: AKASHI Takahiro @ 2020-03-04 6:54 UTC (permalink / raw)
To: u-boot
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
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] Makefile: doesn't need check stack size when dtb is not built
2020-03-04 6:54 [PATCH] Makefile: doesn't need check stack size when dtb is not built AKASHI Takahiro
@ 2020-03-04 16:21 ` Stephen Warren
2020-03-05 0:15 ` AKASHI Takahiro
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Warren @ 2020-03-04 16:21 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Makefile: doesn't need check stack size when dtb is not built
2020-03-04 16:21 ` Stephen Warren
@ 2020-03-05 0:15 ` AKASHI Takahiro
2020-03-05 0:22 ` Stephen Warren
0 siblings, 1 reply; 6+ messages in thread
From: AKASHI Takahiro @ 2020-03-05 0:15 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Makefile: doesn't need check stack size when dtb is not built
2020-03-05 0:15 ` AKASHI Takahiro
@ 2020-03-05 0:22 ` Stephen Warren
2020-03-05 0:39 ` AKASHI Takahiro
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Warren @ 2020-03-05 0:22 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Makefile: doesn't need check stack size when dtb is not built
2020-03-05 0:22 ` Stephen Warren
@ 2020-03-05 0:39 ` AKASHI Takahiro
2020-03-05 16:54 ` Stephen Warren
0 siblings, 1 reply; 6+ messages in thread
From: AKASHI Takahiro @ 2020-03-05 0:39 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Makefile: doesn't need check stack size when dtb is not built
2020-03-05 0:39 ` AKASHI Takahiro
@ 2020-03-05 16:54 ` Stephen Warren
0 siblings, 0 replies; 6+ messages in thread
From: Stephen Warren @ 2020-03-05 16:54 UTC (permalink / raw)
To: u-boot
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)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-05 16:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-04 6:54 [PATCH] Makefile: doesn't need check stack size when dtb is not built AKASHI Takahiro
2020-03-04 16:21 ` Stephen Warren
2020-03-05 0:15 ` AKASHI Takahiro
2020-03-05 0:22 ` Stephen Warren
2020-03-05 0:39 ` AKASHI Takahiro
2020-03-05 16:54 ` Stephen Warren
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.