Hi Anthony,
On Tue, Sep 20, 2011 at 7:22 AM, GROYER, Anthony
<anthony.gro...@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...@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...@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@lists.denx.de
>>>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>>>
>>>
>>_______________________________________________
>>U-Boot mailing list
>>U-Boot@lists.denx.de
>>http://lists.denx.de/mailman/listinfo/u-boot
>>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
