[PATCH]Kconfig: fix QEmu build, reduce bootblock size

[Patrick Georgi]

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

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

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]

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

Anyways, both are Acked-by: Stefan Reinauer

[Patrick Georgi]

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

[Myles Watson]

> 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