Dear Albert ARIBAUD,
>>   include/configs/top9000_9xe.h  |    1 +
That's my board. Not in mainstream yet. Just sets the define there.

>> +#ifdef CONFIG_USE_C_RELOCATION
>> +    /* TODO: check for identical source and destination */
>> +    /* TODO: check for overlapping */
>> +    /* copy image, including initialized data */
>> +    debug ("memcpy(%08lx,%08lx,%ld)\n",
>> +            addr, _TEXT_BASE, _bss_start_ofs);
>> +    memcpy ((void *)addr, (void *)_TEXT_BASE, _bss_start_ofs);
>> +    /* now fix the code */
>> +    debug ("_dynsym_start_ofs=%08lx _rel_dyn_start_ofs=%08lx 
>> _rel_dyn_end_ofs=%08lx\n",
>> +            _dynsym_start_ofs, _rel_dyn_start_ofs, _rel_dyn_end_ofs);
>> +    for (rel_dyn_ptr = (Elf32_Rel *)(_TEXT_BASE + _rel_dyn_start_ofs);
>> +                    rel_dyn_ptr<   (Elf32_Rel *)(_TEXT_BASE + 
>> _rel_dyn_end_ofs);
>> +                    rel_dyn_ptr++) {
>> +            ulong *patchaddr = (ulong *) (rel_dyn_ptr->r_offset + 
>> gd->reloc_off);
>> +            debug ("patch %08lx : %08lx\n",
>> +                    (ulong)patchaddr, (ulong)rel_dyn_ptr->r_info);
>> +            switch (ELF32_R_TYPE(rel_dyn_ptr->r_info)) {
>> +            case 23: /* rel fixup */
>> +                    *patchaddr += gd->reloc_off;
>> +                    break;
>> +            case 2: /* abs fixup */
>> +                    {
>> +                            Elf32_Sym *sym = (Elf32_Sym *)(_TEXT_BASE + 
>> _dynsym_start_ofs);
>> +                            sym += ELF32_R_SYM(rel_dyn_ptr->r_info);
>> +                            *patchaddr = gd->reloc_off + sym->st_value;
>> +                    }
>> +                    break;
>> +            default: /* unhandled fixup */
>> +                    break;
>> +            }
>> +    }
>> +    /* clear BSS */
>> +# ifndef CONFIG_PRELOADER
>> +    debug ("clearing BSS %08lx..%08lx\n",
>> +            addr + _bss_start_ofs, addr + _bss_end_ofs);
>> +    for (p = (ulong *)(addr + _bss_start_ofs);
>> +                    p<   (ulong *)(addr + _bss_end_ofs);
>> +                    *p++ = 0)
>> +            ;
>> +# endif
>> +#endif
>
> All of _bss_start_ofs, _rel_dyn_start_ofs, _rel_dyn_end_ofs,
> _dynsym_start_ofs and _bss_end_ofs are going to be modified by the
> relocation fixup loop. The compiler *might* preload them into
> registers... or not; which could cause issues with ..._end_ofs ones,
> because it could cause the loops to overflow/underflow.

They are not going to be modified, they are in the unrelocated location!

>
>> +    debug ("calling relocate_code\n");
>>      relocate_code (addr_sp, id, addr);
>
> Have you not just relocated and fixed up in the C code? If so,
> relocate_code (which would then become a misnomer) should have its own
> relocate and fixup code removed (your patch does not), and only perform
> the C environment (stack, etc) switch.

Incorrect. I removed the relocation (#ifndef) in start.S leaving only
setting of the SP and calling of board_init_r active. True for the misnomer...
It was supposed to be a quick hack, beauty comes later ;)

> Side note: if board_init_f handles relocation and fixup and
> relocate_code just does stack switch between board_init_f and
> board_init_r, then I'd rather board_init_f *return* to start.S rather
> than *call* it if that's possible. That would make the control flow less
> circuitous: begin in start.S, set a minimal stack, call board_init_f,
> returns to start.S, switch to final stack, calls (branches to) board_init_r.

I agree here. I would have board_init_f return the desired SP value,
for example, or let it return the gd pointer - something along that line.

Or one completely skips returning back to asm and sets the sp with an
asm() statement in "C", obtains a function pointer to board_init_r, adjusts
that and calls that function.

Amicalement,

Reinhard
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to