From mboxrd@z Thu Jan  1 00:00:00 1970
From: Albert ARIBAUD <albert.aribaud@free.fr>
Date: Tue, 05 Oct 2010 18:30:44 +0200
Subject: [U-Boot] [PATCH] futile c relocation attempt
In-Reply-To: <1286290132-2935-1-git-send-email-u-boot@emk-elektronik.de>
References: <1286290132-2935-1-git-send-email-u-boot@emk-elektronik.de>
Message-ID: <4CAB52B4.1080303@free.fr>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Le 05/10/2010 16:48, Reinhard Meyer a ?crit :

>  include/configs/top9000_9xe.h  |    1 +

Can't find this one in my u-boot tree. Did I miss something?

> +#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.

> +	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.

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.

Amicalement,
-- 
Albert.