[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

Simon Glass sjg at chromium.org
Tue Sep 20 23:34:06 CEST 2011


Hi Anthony,

On Tue, Sep 20, 2011 at 7:22 AM, GROYER, Anthony
<Anthony.GROYER at airliquide.com> wrote:
>
> Hello,
>
> I came back on a discussion started on April 2011.
>
> The use of the initial patches for the CONFIG_SYS_SKIP_ARM_RELOCATION features has revealed two issues.
>
> First issue: the calculation of the relocation offset was done only if the relocation is actually done. So we could reach a point where r9 has a wrong value, since it has never been used before (in my case, this bug happens without JTAG). The first diff moves the relocation offset calculation before the test of a relocation need.
>
> Second issue: board_init_r was thinking the memory area for the malloc is just below the code, whereas the board_init_f had allocated some space for the malloc at the end of the SDRAM. If the code is located at the base of the SDRAM with CONFIG_SYS_SKIP_ARM_RELOCATION defined, the malloc area does not point to a correct memory address.
> The 2 other diff store the calculated malloc start address in a global data member so that it can be used in board_init_r().

Thanks for coming back to this. Hopefully you can do patches for the
other CPU types also as Albert requests - and I can test armv7 when
you do that. This feature is one that I use a lot with an ICE and I
would like to see it in U-Boot.

Regards,
Simon

>
> Index: arch/arm/cpu/arm926ejs/start.S
> ===================================================================
> --- arch/arm/cpu/arm926ejs/start.S      (révision 1083)
> +++ arch/arm/cpu/arm926ejs/start.S      (révision 1113)
> @@ -196,6 +196,10 @@
>        mov     r4, r0  /* save addr_sp */
>        mov     r5, r1  /* save addr of gd */
>        mov     r6, r2  /* save addr of destination */
> +    /* set relocation offset here for all cases:
> +        relocation or not */
> +       ldr     r0, _TEXT_BASE  /* r0 <- Text base */
> +       sub     r9, r6, r0              /* r9 <- relocation offset */
>
>        /* Set up the stack                                                 */
>  stack_setup:
> @@ -218,8 +222,6 @@
>        /*
>         * fix .rel.dyn relocations
>         */
> -       ldr     r0, _TEXT_BASE          /* r0 <- Text base */
> -       sub     r9, r6, r0              /* r9 <- relocation offset */
>        ldr     r10, _dynsym_start_ofs  /* r10 <- sym table ofs */
>        add     r10, r10, r0            /* r10 <- sym table in FLASH */
>        ldr     r2, _rel_dyn_start_ofs  /* r2 <- rel dyn start ofs */
> Index: arch/arm/include/asm/global_data.h
> ===================================================================
> --- arch/arm/include/asm/global_data.h  (révision 1083)
> +++ arch/arm/include/asm/global_data.h  (copie de travail)
> @@ -69,6 +69,7 @@
>        unsigned long   mon_len;        /* monitor len */
>        unsigned long   irq_sp;         /* irq stack pointer */
>        unsigned long   start_addr_sp;  /* start_addr_stackpointer */
> +       unsigned long   start_addr_malloc;      /* start_addr_malloc */
>        unsigned long   reloc_off;
>  #if !(defined(CONFIG_SYS_NO_ICACHE) && defined(CONFIG_SYS_NO_DCACHE))
>        unsigned long   tlb_addr;
> Index: arch/arm/lib/board.c
> ===================================================================
> --- arch/arm/lib/board.c        (révision 1138)
> +++ arch/arm/lib/board.c        (copie de travail)
> @@ -367,6 +367,7 @@
>         * reserve memory for malloc() arena
>         */
>        addr_sp = addr - TOTAL_MALLOC_LEN;
> +    gd->start_addr_malloc = addr_sp;
>        debug ("Reserving %dk for malloc() at: %08lx\n",
>                        TOTAL_MALLOC_LEN >> 10, addr_sp);
>        /*
> @@ -445,7 +446,6 @@
>  {
>        char *s;
>        bd_t *bd;
> -       ulong malloc_start;
>  #if !defined(CONFIG_SYS_NO_FLASH)
>        ulong flash_size;
>  #endif
> @@ -473,9 +473,7 @@
>        post_output_backlog ();
>  #endif
>
> -       /* The Malloc area is immediately below the monitor copy in DRAM */
> -       malloc_start = dest_addr - TOTAL_MALLOC_LEN;
> -       mem_malloc_init (malloc_start, TOTAL_MALLOC_LEN);
> +       mem_malloc_init (gd->start_addr_malloc, TOTAL_MALLOC_LEN);
>
>  #if !defined(CONFIG_SYS_NO_FLASH)
>        puts ("Flash: ");
>
> Regards,
> Anthony Groyer
>
>>On Wed, Apr 20, 2011 at 11:56 PM, Aneesh V <ane... at ti.com> wrote:
>>> Hi Simon, Wolfgang,
>>>
>>> On Thursday 21 April 2011 12:04 AM, Simon Glass wrote:
>>>>
>>>> On Fri, Mar 25, 2011 at 11:35 AM, Albert ARIBAUD<albert.arib... at free.fr>
>>>>  wrote:
>>>>>
>>>>> Le 25/03/2011 17:12, Aneesh V a écrit :
>>>>>
>>>>>> Another problem I have with relocation is that it makes debugging with
>>>>>> JTAG debugers more difficult. The addresses of symbols in the ELF
>>>>>> target are no longer valid. Of course, you can load the symbols at an
>>>>>> offset from the original location. But one has to first enable debug
>>>>>> prints, find the relocation offset, use it while loading the symbols
>>>>>> before you can do source level debugging.
>>>>>
>>>>> Actually you don't need recompiling: simply set a breakpoint at the
>>>>> entry of relocate_code and once you hit the bp, look up r2: it contains
>>>>> the destination. If you want the offset rather than the absolute
>>>>> address, set the breakpoint right after the 'sub r9, r6, r0' round line
>>>>> 222: r9 will then give you the offset. Unload the current symbols,
>>>>> reload the symbols with the relevant offset, and there you go.
>>>>
>>>> I would like to revisit this thread. I'm not sure how other people do
>>>> development in U-Boot but I like to use an ICE and a source-level
>>>> debugger for any significant effort. I think it should be possible to
>>>> use a JTAG debugging just by loading the u-boot ELF file and running.
>>>>
>>>> With this patch (or something similar) this is possible. Without it,
>>>> it is painful.
>>>>
>>>> To be clear, we are not talking here about creating a platform that
>>>> doesn't use relocation, just that for development purposes it is
>>>> convenient to be able to disable it.
>>>
>>> Actually, I am not even sure why relocation shouldn't be disabled in my
>>> platform for good. It may be useful to have things such as the frame
>>> buffer at the end of available memory. But, IMHO, that can still be
>>> done without relocating u-boot. That's what the patch does.Am I missing
>>> something?
>>
>>Well relocation does remove a lot of this ad-hoc positioning of things
>>at compile time. I think it is desirable. My point is that it is not
>>engineer-friendly during development, and we should have an easy way
>>to disable it for debugging / JTAG purposes.
>>
>>Regards,
>>Simon
>>
>>>
>>>>
>>>> Looking at the December thread
>>>> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88067/focus=88262
>>>>
>>>> Aneesh:
>>>>>>
>>>>>> Shouldn't we provide a CONFIG option by which users can disable
>>>>>> Elf relocation?
>>>>
>>>> Wolfgang:
>>>>>
>>>>> Why should we?  It would just make the code even more complicated, and
>>>>> require a lot of additional test cases.
>>>>
>>>>  From what I can see this is a new CONFIG option, two ifdefs in the
>>>> board.c file, and optionally disabling the -pie position-independent
>>>> executable option to reduce size. It is pretty trivial:
>>>>
>>>>  arch/arm/config.mk   |    2 ++
>>>>  arch/arm/lib/board.c |    5 +++++
>>>>  2 files changed, 7 insertions(+), 0 deletions(-)
>>>>
>>>> Regards,
>>>> Simon
>>>>
>>>>>
>>>>> Amicalement,
>>>>> --
>>>>> Albert.
>>>>> _______________________________________________
>>>>> U-Boot mailing list
>>>>> U-Boot at lists.denx.de
>>>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>>>
>>>
>>_______________________________________________
>>U-Boot mailing list
>>U-Boot at lists.denx.de
>>http://lists.denx.de/mailman/listinfo/u-boot
>>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>


More information about the U-Boot mailing list