From mboxrd@z Thu Jan 1 00:00:00 1970 From: Albert ARIBAUD Date: Sun, 11 Dec 2011 15:47:32 +0100 Subject: [U-Boot] [PATCH v2 0/6] reboard: Introduce generic relocation feature In-Reply-To: <1323544576-25940-1-git-send-email-sjg@chromium.org> References: <1323544576-25940-1-git-send-email-sjg@chromium.org> Message-ID: <4EE4C284.1020707@aribaud.net> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, General comments here, detailed comments in reply to n/6 patches. Le 10/12/2011 20:16, Simon Glass a ?crit : > This is the second patch series aiming to unify the various board.c files > in each architecture into a single one. This series implements a generic > relocation feature, which is the bridge between board_init_f() and > board_init_r(). It then moves ARM over to use this framework, as an > example. > > On ARM the relocation code is duplicated for each CPU yet it > is the same. We can bring this up to the arch level. But since (I believe) > Elf relocation is basically the same process for all archs, there is no > reason not to bring it up to the generic level. Actually most of start.S is very similar across all its avatars in arch/arm, and is a good candidate for being generalized. I would prefer a generalization of start.S with the vector table, generic startup sequence prepare for calling board_init_f, jump to board_init_r with a possible stack switch, exception handlers) , and anything specific moved into the adequate subdirectory. > Each architecture which uses this framework needs to provide a function > called arch_elf_relocate_entry() which processes a single relocation > entry. This is a static inline function to reduce code size overhead. I assume this is due to some arch other than armnot using the same set of entry types, right? If so, I think it would be better to keep arch-specific entry relocation code under conditional complilation in the generic ELF relication source file, so that all ELF-structure dependent code is in a single file. > For ARM, a new arch/arm/lib/proc.S file is created, which holds generic > ARM assembler code (things that cannot be written in C and are common > functions used by all ARM CPUs). This helps reduce duplication. Interrupt > handling code and perhaps even some startup code can move there later. > > It may be useful for other architectures with a lot of different CPUs > to have a similar file. Indeed, but I think start.S is a better candidate for this, see comment above. > Code size on my ARMv7 system increases by 54 bytes with generic > relocation. This overhead is mostly just literal pool access and setting > up to call the relocated U-Boot at the end. > > On my system, execution time increases from 10.8ms to 15.6ms due to the > less efficient C implementations of the copy and zero loops. If execution > time is of concern, you can define CONFIG_USE_ARCH_MEMSET and > CONFIG_USE_ARCH_MEMCPY to reduce it. For met this reduces relocation time > to 5.4ms, i.e. twice as fast as the old system. > One problem remains which causes mx31pdk to fail to build. It doesn't > have string.c in its SPL code and the architecture-specific versions of > memset()/memcpy() are too large. I propose to add a local change to > reloc.c that uses inline code for boards that use the old legacy SPL > framework. We can remove it later. This is not included in v2 but I am > interested in comments on this approach. An alternative would be just > to add simple memset()/memcpy() functions just for this board (and one > other affected MX31 board). > > Changes in v2: > - Use CONFIG_SYS_SKIP_RELOC instead of CONFIG_SYS_LEGACY_BOARD > - Import asm-generic/sections.h from Linux and add U-Boot extras > - Squash generic link symbols patch into generic relocation patch > - Move reloc.c into common/ > - Add function comments > - Use memset, memcpy instead of inline code > - Add README file for relocation > - Invalidate I-cache when we jump to relocated code > - Use an inline relocation function to reduce code size Seeing as inline will only avoid pushing a couple argument registers and doing a branch, and OTOH may require additional register shuffling in the calling code, how much code does this inlining save? > - Make relocation symbols global so we can use them outside start.S Why should they relocation symbols ever be used outside of actually relocating? Amicalement, -- Albert.