Hi Albert,
On Monday, October 14, 2013 8:59:17 AM, Albert ARIBAUD wrote:
On Sun, 13 Oct 2013 19:16:33 +0200, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hi Benoît,
On Sun, 13 Oct 2013 17:00:25 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
Hi Albert,
On Sunday, October 13, 2013 9:10:28 AM, Albert ARIBAUD wrote:
Remove the last uses of symbol offsets in ARM U-Boot. Remove some needless uses of _TEXT_BASE. Remove all _TEXT_BASE definitions.
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net
_TEXT_BASE was only used by ARM to allow resolution of symbol offsets, themselves only needed due to absolute relocations.
In some places, _TEXT_BASE was locally defined only to provide a literal for CONFIG_SYS_TEXT_BASE when the latter could have been used directly.
Sometimes even, _TEXT_BASE was defined but unused.
Since all relocations in ARM are relative, offsets, _TEXT_BASE and CONFIG_SYS_SYM_OFFSETS can be completely removed, and their uses can be replaced with adequate use of compiler-generated symbols from sections.c file.
[...]
diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S index bd1e067..d15124b 100644 --- a/arch/arm/cpu/arm1136/start.S +++ b/arch/arm/cpu/arm1136/start.S
[...]
@@ -295,7 +269,6 @@ cpu_init_crit: #ifdef CONFIG_SPL_BUILD .align 5 do_hang:
- ldr sp, _TEXT_BASE /* use 32 words about stack */ bl hang /* hang and never return */
#else /* !CONFIG_SPL_BUILD */ .align 5
Is this change (and the same change in the other start.S files) safe?
lib/hang.c/hang() may need a valid stack pointer because the functions that it calls may use the stack.
When the CPU lands in do_hang, it's because some exception occurred, which may follow a situation having corrupted sp. If sp is corrupted, the CPU won't be able to push the exception context onto the stack, but it might still be able to run the exception vector.
Setting sp to *_TEXT_BASE was not great, but at least this provided a few valid words of RAM for the stack.
Yes, there is a call to hang() which might or might not imply saving on the stack depending on code generation, and sp might be incorrect depending on the exception raised. I'll reintroduce the sp setting but use the runtime value of _start as stack top. As you say, not great but hey.
Thinking further about hangs: when we call do_hang(), by definition we are in a critical situation where for some reason we cannot proceed any more; and in the worst scenario, the only guaranteed valid register we have is pc. Therefore, I'm not sure it is wise to try to do anything else than actually hanging.
OTOH, I do understand that we may also want to do something else than hanging, such as trying to diagnose, but we should choose more clearly:
either we hang for the purpose of being post-mortem-debugged from there, and therefore, we limit alterations to the system state as much as we can, by only affecting pc (to the point that 'bl hang' should be turned into 'b do_hang' so that lr is also preserved);
or we want e.g. to tell the operator about it, and we make sure we have a correct setting, that is, we reserve and use a proper stack rather than set it just below _start and hope for luck.
Choosing between one or the other would be done through a configuration option such as for instance CONFIG_SYS_LOG_HANGS.
Regarding the stack, we could use some existing exception stack, but it might be better if it was preserved, so that its contents could be looked into.
Comments?
This configuration option would be great. The verbose log should be enabled by default, indicating the configuration option to change in order to investigate the hang.
In the log case, we care about having a valid stack, not about preserving the possible exception stack contents. In the post-mortem debug case, this is the opposite.
Best regards, Benoît