From mboxrd@z Thu Jan 1 00:00:00 1970 From: Albert ARIBAUD Date: Wed, 18 Jan 2012 08:28:47 +0100 Subject: [U-Boot] [PATCH v3 0/6] Introduce generic relocation feature In-Reply-To: <1324923878-8176-1-git-send-email-sjg@chromium.org> References: <1324923878-8176-1-git-send-email-sjg@chromium.org> Message-ID: <4F1674AF.9050000@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, Le 26/12/2011 19:24, Simon Glass a ?crit : > (I am resending this rebased so I can continue with this board-unification > work and allow people to review patches. There were some comments on the > v2 series but my questions have been sitting on the list for 2 weeks so it > is probably time for a new series.) > > 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. > > 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. Agreed. > 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. NAK for several reasons: 1. The code that goes on proc.S is already in start.S, which already contains "things which cannot be written in C and are common functions used by all ARM CPUs". Granted, right now start.S is duplicated in multiple CPU directories; so the thing to do is to merge all ARM start.S files into a single one, rather than merging only one of its parts. 2. There is no interest in moving this segment of code from all start.S files into a new proc.S file: there is no gain is code size obviously, and there is an increase in source file count. 3. I consider that files should contain 'things' based on their common /functionality/, not on their similar /nature/, and "going about starting up U-Boot" is a functionality. Note that eventually, having a single start.S will achieve the same effect as this 'proc.S' patch. Note also that this start.S commonalization should be a completely different patch set. > 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. Side question: is there any reason not to define these CONFIG options systematically? > 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). I am not too fond of the "later removal" approach. In my experience, this invariably tends to the "old bloat in the code no one knows the reason of" situations. > 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 > - Make relocation symbols global so we can use them outside start.S > > Changes in v3: > - Remove the 'reloc' tag from each commit > - Rebase to master > > Simon Glass (6): > Create reloc.h and include it where needed > define CONFIG_SYS_SKIP_RELOC for all archs > Add generic relocation feature > arm: Add processor function library > arm: Move over to generic relocation > arm: Remove unused code in start.S > > README | 4 + > arch/arm/cpu/arm1136/start.S | 133 ++------------ > arch/arm/cpu/arm1176/start.S | 214 ++------------------- > arch/arm/cpu/arm720t/start.S | 127 ++----------- > arch/arm/cpu/arm920t/start.S | 135 ++------------ > arch/arm/cpu/arm925t/start.S | 135 ++------------ > arch/arm/cpu/arm926ejs/davinci/spl.c | 1 + > arch/arm/cpu/arm926ejs/start.S | 144 ++------------- > arch/arm/cpu/arm946es/start.S | 130 ++----------- > arch/arm/cpu/arm_intcm/start.S | 135 ++------------ > arch/arm/cpu/armv7/omap-common/spl.c | 1 + > arch/arm/cpu/armv7/start.S | 140 ++------------ > arch/arm/cpu/ixp/start.S | 127 ++----------- > arch/arm/cpu/lh7a40x/start.S | 124 ++----------- > arch/arm/cpu/pxa/start.S | 138 ++------------ > arch/arm/cpu/s3c44b0/start.S | 127 ++----------- > arch/arm/cpu/sa1100/start.S | 124 ++----------- > arch/arm/include/asm/reloc.h | 56 ++++++ > arch/arm/lib/Makefile | 2 + > arch/arm/lib/board.c | 1 + > arch/arm/lib/proc.S | 40 ++++ > arch/avr32/config.mk | 3 + > arch/avr32/lib/board.c | 1 + > arch/blackfin/config.mk | 3 + > arch/m68k/config.mk | 3 + > arch/m68k/lib/board.c | 1 + > arch/microblaze/config.mk | 3 + > arch/mips/config.mk | 3 + > arch/mips/lib/board.c | 1 + > arch/nds32/config.mk | 3 + > arch/nds32/lib/board.c | 1 + > arch/nios2/config.mk | 3 + > arch/powerpc/config.mk | 3 + > arch/powerpc/lib/board.c | 1 + > arch/sandbox/config.mk | 3 + > arch/sh/config.mk | 3 + > arch/sparc/config.mk | 3 + > arch/x86/config.mk | 3 + > arch/x86/lib/board.c | 1 + > board/freescale/mpc8313erdb/mpc8313erdb.c | 1 + > board/freescale/mpc8315erdb/mpc8315erdb.c | 1 + > board/samsung/smdk6400/smdk6400_nand_spl.c | 1 + > board/sheldon/simpc8313/simpc8313.c | 1 + > common/Makefile | 4 + > common/reloc.c | 121 ++++++++++++ > doc/README.relocation | 87 +++++++++ > include/asm-generic/sections.h | 92 +++++++++ > include/common.h | 2 +- > include/reloc.h | 54 +++++ > nand_spl/board/freescale/mpc8536ds/nand_boot.c | 1 + > nand_spl/board/freescale/mpc8569mds/nand_boot.c | 1 + > nand_spl/board/freescale/mpc8572ds/nand_boot.c | 1 + > nand_spl/board/freescale/mx31pdk/Makefile | 8 +- > nand_spl/board/freescale/mx31pdk/u-boot.lds | 1 + > nand_spl/board/freescale/p1010rdb/nand_boot.c | 1 + > nand_spl/board/freescale/p1023rds/nand_boot.c | 1 + > nand_spl/board/freescale/p1_p2_rdb/nand_boot.c | 1 + > nand_spl/board/freescale/p1_p2_rdb_pc/nand_boot.c | 1 + > nand_spl/board/karo/tx25/Makefile | 8 +- > nand_spl/board/karo/tx25/u-boot.lds | 1 + > nand_spl/nand_boot_fsl_nfc.c | 1 + > 61 files changed, 705 insertions(+), 1765 deletions(-) > create mode 100644 arch/arm/include/asm/reloc.h > create mode 100644 arch/arm/lib/proc.S > create mode 100644 common/reloc.c > create mode 100644 doc/README.relocation > create mode 100644 include/asm-generic/sections.h > create mode 100644 include/reloc.h Amicalement, -- Albert.