Patchwork Kconfig: fix QEmu build, reduce bootblock size

login
register
about
Submitter Patrick Georgi
Date 2009-08-13 15:18:50
Message ID <4A842EDA.2060203@georgi-clan.de>
Download mbox | patch
Permalink /patch/122/
State Accepted
Headers show

Comments

Patrick Georgi - 2009-08-13 15:18:50
Hi,

attached patches do:
20090813-1:
  AMD selected a couple of options that are incompatible with QEmu (and
probably others). Only select them for AMD

20090813-2:
  Make the bootblock smaller (only one copy of it), and don't pad the
bootblock using dd(1), but top-align inside cbfstool, to reduce
dependencies on unix tools.

Signed-off-by: Patrick Georgi <patrick.georgi@coresystems.de>

Index: coreboot-v2/src/arch/i386/Makefile.inc
===================================================================
--- coreboot-v2.orig/src/arch/i386/Makefile.inc
+++ coreboot-v2/src/arch/i386/Makefile.inc
@@ -8,12 +8,14 @@ subdirs-y += smp
 obj-y += ../../option_table.o
 
 ifdef POST_EVALUATION
+BOOTBLOCK_SIZE=65536
+
 #######################################################################
 # Build the final rom image
 
 $(obj)/coreboot.rom: $(obj)/coreboot.bootblock $(obj)/coreboot_ram $(CBFSTOOL)
 	$(Q)rm -f $@
-	$(Q)$(CBFSTOOL) $@ create $(shell expr 1024 \* $(CONFIG_COREBOOT_ROMSIZE_KB)) 131072 $(obj)/coreboot.bootblock
+	$(Q)$(CBFSTOOL) $@ create $(shell expr 1024 \* $(CONFIG_COREBOOT_ROMSIZE_KB)) $(BOOTBLOCK_SIZE) $(obj)/coreboot.bootblock
 	$(Q)if [ -f fallback/coreboot_apc ]; \
 	then \
 		$(CBFSTOOL) $@ add-stage fallback/coreboot_apc fallback/coreboot_apc $(CBFS_COMPRESS_FLAG); \
@@ -31,14 +33,7 @@ endif
 #######################################################################
 # Build the bootblock
 
-BOOTBLOCK_SIZE=65536
-
-$(obj)/coreboot.bootblock: $(obj)/coreboot.strip
-	$(Q)printf "    CREATE     $(subst $(obj)/,,$(@))\n"
-	$(Q)dd if=$< of=$(obj)/coreboot.bootblock.one obs=$(BOOTBLOCK_SIZE) conv=sync
-	$(Q)cat $(obj)/coreboot.bootblock.one $(obj)/coreboot.bootblock.one > $(obj)/coreboot.bootblock
-
-$(obj)/coreboot.strip: $(obj)/coreboot
+$(obj)/coreboot.bootblock: $(obj)/coreboot
 	$(Q)printf "    OBJCOPY    $(subst $(obj)/,,$(@))\n"
 	$(Q)$(OBJCOPY) -O binary $< $@
 
Index: coreboot-v2/util/cbfstool/util.c
===================================================================
--- coreboot-v2.orig/util/cbfstool/util.c
+++ coreboot-v2/util/cbfstool/util.c
@@ -256,7 +256,7 @@ int add_bootblock(struct rom *rom, const
 	}
 
 	/* Copy the bootblock into place at the end of the file */
-	ret = copy_from_fd(fd, ROM_PTR(rom, rom->size - ntohl(rom->header->bootblocksize)), size);
+	ret = copy_from_fd(fd, ROM_PTR(rom, rom->size - size), size);
 
 	close(fd);
Stefan Reinauer - 2009-08-13 15:26:14
On 8/13/09 5:18 PM, Patrick Georgi wrote:
> Hi,
>
> attached patches do:
> 20090813-1:
>   AMD selected a couple of options that are incompatible with QEmu (and
> probably others). Only select them for AMD
>   

I think putting "HAVE_INIT_TIMER" in AMD model specific code is
generally wrong. It's not an AMD specific option. Also enabling options
per default that are very cpu specific is bad. Is there no better way to
do this?

> 20090813-2:
>   Make the bootblock smaller (only one copy of it), and don't pad the
> bootblock using dd(1), but top-align inside cbfstool, to reduce
> dependencies on unix tools.
>
> Signed-off-by: Patrick Georgi <patrick.georgi@coresystems.de>
>   
Anyways, both are Acked-by: Stefan Reinauer <stepan@coresystems.de>
Myles Watson - 2009-08-13 15:28:30
> attached patches do:
> 20090813-1:
>   AMD selected a couple of options that are incompatible with QEmu (and
> probably others). Only select them for AMD
Can we include that file conditionally instead?  That file should never be
included with Qemu, right?

Thanks,
Myles
Patrick Georgi - 2009-08-13 15:31:00
Stefan Reinauer schrieb:
> I think putting "HAVE_INIT_TIMER" in AMD model specific code is
> generally wrong. It's not an AMD specific option. Also enabling options
> per default that are very cpu specific is bad. Is there no better way to
> do this?
>   
The AMD code merely enables it. It's defined in src/Kconfig as disabled,
and then can be enabled wherever appropriate (though I don't know if
_this_ place in the AMD code is appropriate).

When enabling variables, it just has to be conditional on the
responsible component, otherwise it's enabled for everyone.


Patrick

Patch

Index: coreboot-v2/src/cpu/amd/model_fxx/Kconfig
===================================================================
--- coreboot-v2.orig/src/cpu/amd/model_fxx/Kconfig
+++ coreboot-v2/src/cpu/amd/model_fxx/Kconfig
@@ -1,13 +1,16 @@ 
 config HAVE_INIT_TIMER
 	bool
 	default y
+	depends on CPU_AMD_SOCKET_F
 
 config HAVE_MOVNTI
 	bool
 	default y
+	depends on CPU_AMD_SOCKET_F
 
 config CPU_ADDR_BITS
 	int
 	default 40
+	depends on CPU_AMD_SOCKET_F