Hi Benoît, On Mon, 14 Oct 2013 12:22:31 +0200 (CEST), Benoît Thébaudeau <benoit.thebaud...@advansee.com> wrote:
> 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.b...@aribaud.net> wrote: > > > > > Hi Benoît, > > > > > > On Sun, 13 Oct 2013 17:00:25 +0200 (CEST), Benoît Thébaudeau > > > <benoit.thebaud...@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.b...@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. I suspect some people would argue that for SPL, the default should be to hang rather than log. > 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. Depends. You may want the log to o as faithful a state dump as feasible. > Best regards, > Benoît Amicalement, -- Albert. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
