On 14.02.2008 19:15, Myles Watson wrote:
> This patch removes elfboot and references to ELF from v3. Since lar
> parses ELF, there's no need to have v3 parse ELF. The ELF files that
> v3 could parse should always have been a subset of the ones that lar
> could parse anyway.
>
> Myles
>
> Signed-off-by: Myles Watson <[EMAIL PROTECTED]>
>
Patches which remove stuff are always very welcome. A few comments below.
If you commit, please make sure you remove the files with "svn rm"
instead of using patch to make them empty.
> Index: Kconfig
> ===================================================================
> --- Kconfig (revision 597)
> +++ Kconfig (working copy)
> @@ -96,29 +96,6 @@
> prompt "Payload type"
> default PAYLOAD_NONE
>
> -config PAYLOAD_PREPARSE_ELF
>
Isn't the removal of PAYLOAD_PREPARSE_ELF backwards? I would have
expected you to remove PAYLOAD_ELF.
> - bool "Pre-parse ELF file and convert ELF segments to LAR entries"
> - depends EXPERT
> - default n
> - help
> - Until now, coreboot has used ELF for the payload. There are many
> - problems with this, not least being the inefficiency -- the ELF has
> - to be decompressed to memory and then the segments have to be
> - copied. Plus, lar can't see the segments in the ELF -- to see all
> - segments, you have to extract the ELF and run readelf on it.
> -
> - There are problems with collisions of the decompressed ELF
> - location in memory and the segment locations in memory.
> - Finally, validation of the ELF is done at run time, once you have
> - flashed the FLASH and rebooted the machine. Boot time is really
> - not the time you want to find out your ELF payload is broken.
> -
> - With this option, coreboot will direct lar to break each ELF
> - segment into a LAR entry. ELF will not be used at all. Note that
> - (for now) coreboot is backward compatible -- if you put an ELF
> - payload in, coreboot can still parse it. We hope to remove ELF
> - entirely in the future.
> -
> config PAYLOAD_ELF
> bool "An ELF executable payload file"
> help
> @@ -126,6 +103,9 @@
> which coreboot should run as soon as the basic hardware
> initialization is completed.
>
> + With this option, coreboot will direct lar to break each ELF
> + segment into a LAR entry. ELF will not be used at all in coreboot.
> +
> You will be able to specify the location and file name of the
> payload image later.
>
> @@ -143,7 +123,7 @@
>
> config PAYLOAD_FILE
> string "Payload path and filename"
> - depends PAYLOAD_ELF || PAYLOAD_PREPARSE_ELF
> + depends PAYLOAD_ELF
>
See above.
> default "payload.elf"
> help
> The path and filename of the ELF executable file to use as payload.
> Index: include/post_code.h
> ===================================================================
> --- include/post_code.h (revision 597)
> +++ include/post_code.h (working copy)
> @@ -58,8 +58,5 @@
> #define POST_STAGE2_PCISCANBUS_ENTER 0x24
> #define POST_STAGE2_PCISCANBUS_DONEFORLOOP 0x25
> #define POST_STAGE2_PCISCANBUS_EXIT 0x55
> -#define POST_ELFBOOT_JUMPING_TO_BOOTCODE 0xfe
> -#define POST_ELFBOOT_LOADER_STARTED 0xf8
> -#define POST_ELFBOOT_LOADER_IMAGE_FAILED 0xff
>
> #endif /* POST_CODE_H */
> Index: mainboard/emulation/qemu-x86/defconfig
> ===================================================================
> --- mainboard/emulation/qemu-x86/defconfig (revision 597)
> +++ mainboard/emulation/qemu-x86/defconfig (working copy)
> @@ -83,6 +83,5 @@
> #
> # Payload
> #
> -# CONFIG_PAYLOAD_PREPARSE_ELF is not set
> # CONFIG_PAYLOAD_ELF is not set
> CONFIG_PAYLOAD_NONE=y
> Index: arch/x86/stage1.c
> ===================================================================
> --- arch/x86/stage1.c (revision 597)
> +++ arch/x86/stage1.c (working copy)
> @@ -29,7 +29,6 @@
> #include <cpu.h>
>
> /* ah, well, what a mess! This is a hard code. FIX ME but how?
> - * By getting rid of ELF ...
> */
> #define UNCOMPRESS_AREA (0x400000)
>
I think you can remove the whole comment and UNCOMPRESS_AREA as well.
>
> @@ -86,22 +85,6 @@
> return;
> }
>
> -/* until we get rid of elf */
> -int legacy(struct mem_file *archive, char *name, void *where, struct
> lb_memory *mem)
> -{
> - int ret;
> - int elfboot_mem(struct lb_memory *mem, void *where, int size);
> - ret = copy_file(archive, name, where);
> - if (ret) {
> - printk(BIOS_ERR, "'%s' found, but could not load it.\n", name);
> - }
> -
> - ret = elfboot_mem(mem, where, archive->reallen);
> -
> - printk(BIOS_ERR, "elfboot_mem returns %d\n", ret);
> - return -1;
> -}
> -
> /*
> * This function is called from assembler code with its argument on the
> * stack. Force the compiler to generate always correct code for this case.
> @@ -110,7 +93,6 @@
> {
> int ret;
> struct mem_file archive, result;
> - int elfboot_mem(struct lb_memory *mem, void *where, int size);
> void *entry;
>
> /* we can't statically init this hack. */
> @@ -203,8 +185,6 @@
> printk(BIOS_DEBUG, "Stage2 code done.\n");
>
> ret = find_file(&archive, "normal/payload", &result);
> - if (! ret)
> - legacy(&archive, "normal/payload", (void *)UNCOMPRESS_AREA,
> mem);
>
> entry = load_file_segments(&archive, "normal/payload");
> if (entry != (void*)-1) {
> Index: arch/x86/Makefile
> ===================================================================
> --- arch/x86/Makefile (revision 597)
> +++ arch/x86/Makefile (working copy)
> @@ -106,9 +106,9 @@
> # initram module and the various stages and payload files.
> #
>
> -STAGE0_LIB_OBJ = uart8250.o mem.o elfboot.o lar.o delay.o vtxprintf.o \
> +STAGE0_LIB_OBJ = uart8250.o mem.o lar.o delay.o vtxprintf.o \
> vsprintf.o console.o string.o $(DECOMPRESSORS)
> -STAGE0_ARCH_X86_OBJ = stage1.o serial.o archelfboot.o speaker.o \
> +STAGE0_ARCH_X86_OBJ = stage1.o serial.o speaker.o \
> udelay_io.o mc146818rtc.o post_code.o
>
> ifeq ($(CONFIG_CPU_I586),y)
> @@ -125,11 +125,7 @@
>
>
> # We now parse initram as ELF, so we need PARSEELF enabled unconditionally.
> -ifeq ($(CONFIG_PAYLOAD_PREPARSE_ELF), y)
> - PARSEELF = -e
> -else
> - PARSEELF = -e
> -endif
> +PARSEELF = -e
>
Why not remove PARSEELF altogether and hardcode -e for LAR?
>
> STAGE0_OBJ := $(patsubst %,$(obj)/lib/%,$(STAGE0_LIB_OBJ)) \
> $(patsubst %,$(obj)/arch/x86/%,$(STAGE0_ARCH_X86_OBJ)) \
>
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
--
coreboot mailing list
[email protected]
http://www.coreboot.org/mailman/listinfo/coreboot
