[PATCH v2 0/3] hvmloader: Make ROM dependencies optional

[PATCH v2 0/3] hvmloader: Make ROM dependencies optional

[Julian Pidancet]

This patch set mainly allows the user to build a seabios or rombios only
version of hvmloader.
In addition, when building a seabios only hvmloader, Option ROMs like
vgabios and etherboot are no longer required, and therefore can be disabled
from the build. Dependency on the bcc compiler can also be avoided the
same way.

v2: Separate patches for separate issues
    Introduced config option to select which NIC to build ROM for
    Fixed initial patch to build multiple etherboot ROMs in hvmloader
    Option ROMs are keyed off wether or not rombios is enabled, rather than
on an individual basis
    Introduced config options to select support for rombios/seabios
    

Julian Pidancet (3):
  hvmloader: Only compile 32bitbios_support.c when rombios is enabled
  firmware: Use mkhex from hvmloader directory for etherboot ROMs
  firmware: Introduce CONFIG_ROMBIOS and CONFIG_SEABIOS options

 Config.mk                            |    5 ++++
 tools/firmware/Makefile              |   21 +++++++++++-------
 tools/firmware/etherboot/Config      |    2 -
 tools/firmware/etherboot/Makefile    |   13 ++--------
 tools/firmware/hvmloader/Makefile    |   39 ++++++++++++++++++++-------------
 tools/firmware/hvmloader/hvmloader.c |    4 +++
 6 files changed, 49 insertions(+), 35 deletions(-)

-- 
Julian Pidancet

[PATCH v2 1/3] hvmloader: Only compile 32bitbios_support.c when rombios is enabled

[Julian Pidancet]

32bitbios_support.c only contains code specific to rombios, and should
not be built-in when building hvmloader for SeaBIOS only (as for
rombios.c).

Signed-off-by: Julian Pidancet <julian.pidancet@gmail.com>
---
 tools/firmware/hvmloader/Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index 41a4369..e82146a 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -29,7 +29,7 @@ LOADADDR = 0x100000
 CFLAGS += $(CFLAGS_xeninclude)
 
 OBJS  = hvmloader.o mp_tables.o util.o smbios.o 
-OBJS += 32bitbios_support.o smp.o cacheattr.o xenbus.o
+OBJS += smp.o cacheattr.o xenbus.o
 OBJS += e820.o pci.o pir.o ctype.o
 ifeq ($(debug),y)
 OBJS += tests.o
@@ -39,7 +39,7 @@ CIRRUSVGA_DEBUG ?= n
 
 ROMBIOS_DIR := ../rombios
 ifneq ($(ROMBIOS_DIR),)
-OBJS += rombios.o
+OBJS += 32bitbios_support.o rombios.o
 CFLAGS += -DENABLE_ROMBIOS
 ROMBIOS_ROM := $(ROMBIOS_DIR)/BIOS-bochs-latest
 endif
-- 
Julian Pidancet

[PATCH v2 2/3] firmware: Use mkhex from hvmloader directory for etherboot ROMs

[Julian Pidancet]

To remain consistent with how other ROMs are built into hvmloader,
call mkhex on etherboot ROMs from the hvmloader directory, instead of
the etherboot directory. In other words, eb-roms.h is not used any more.

Introduce ETHERBOOT_NICS config option to choose which ROMs should be
built (kept rtl8139 and 8086100e per default as before).

Signed-off-by: Julian Pidancet <julian.pidancet@gmail.com>
---
 Config.mk                         |    2 ++
 tools/firmware/etherboot/Config   |    2 --
 tools/firmware/etherboot/Makefile |   13 +++----------
 tools/firmware/hvmloader/Makefile |    9 ++++++---
 4 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/Config.mk b/Config.mk
index e2dc4b9..42508b8 100644
--- a/Config.mk
+++ b/Config.mk
@@ -222,6 +222,8 @@ endif
 QEMU_UPSTREAM_REVISION ?= master
 SEABIOS_UPSTREAM_TAG ?= rel-1.6.3.1
 
+ETHERBOOT_NICS ?= rtl8139 8086100e
+
 # Specify which qemu-dm to use. This may be `ioemu' to use the old
 # Mercurial in-tree version, or a local directory, or a git URL.
 # CONFIG_QEMU ?= `pwd`/$(XEN_ROOT)/../qemu-xen.git
diff --git a/tools/firmware/etherboot/Config b/tools/firmware/etherboot/Config
index 143914f..69963b9 100644
--- a/tools/firmware/etherboot/Config
+++ b/tools/firmware/etherboot/Config
@@ -1,6 +1,4 @@
 
-NICS = rtl8139 8086100e
-
 CFLAGS += -UPXE_DHCP_STRICT
 CFLAGS += -DPXE_DHCP_STRICT
 
diff --git a/tools/firmware/etherboot/Makefile b/tools/firmware/etherboot/Makefile
index c09140e..a195888 100644
--- a/tools/firmware/etherboot/Makefile
+++ b/tools/firmware/etherboot/Makefile
@@ -17,23 +17,16 @@ IPXE_TARBALL_URL := $(XEN_EXTFILES_URL)/ipxe-git-$(IPXE_GIT_TAG).tar.gz
 D=ipxe
 T=ipxe.tar.gz
 
-ROMS = $(addprefix $D/src/bin/, $(addsuffix .rom, $(NICS)))
+ROMS = $(addprefix $D/src/bin/, $(addsuffix .rom, $(ETHERBOOT_NICS)))
 
 .NOTPARALLEL:
 
 .PHONY: all
-all: eb-roms.h
+all: $(ROMS)
 
 %.rom: $D/src/arch/i386/Makefile
 	$(MAKE) -C $D/src bin/$(*F).rom
 
-eb-roms.h.new: $(ROMS)
-	cat $^ | ../hvmloader/mkhex etherboot >$@
-
-eb-roms.h: Config
-	$(MAKE) NO_WERROR=1 $@.new
-	mv -f $@.new $@
-
 $T:
 	if ! wget -O _$T $(IPXE_TARBALL_URL); then \
 		$(GIT) clone $(IPXE_GIT_URL) $D.git; \
@@ -56,7 +49,7 @@ $D/src/bin/NIC: $D/src/arch/i386/Makefile
 
 .PHONY: clean
 clean:
-	rm -rf $D $D.git *~ eb-roms.h _$T $T
+	rm -rf $D $D.git *~ _$T $T
 
 .PHONY: distclean
 distclean: clean
diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index e82146a..1ea32db 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -58,6 +58,8 @@ else
 CIRRUSVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.cirrus.bin
 endif
 
+ETHERBOOT_ROMS := $(addprefix ../etherboot/ipxe/src/bin/, $(addsuffix .rom, $(ETHERBOOT_NICS)))
+
 .PHONY: all
 all: subdirs-all
 	$(MAKE) hvmloader
@@ -70,7 +72,7 @@ hvmloader: $(OBJS) acpi/acpi.a
 	$(OBJCOPY) hvmloader.tmp hvmloader
 	rm -f hvmloader.tmp
 
-roms.inc: $(ROMBIOS_ROM) $(SEABIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM) ../etherboot/eb-roms.h
+roms.inc: $(ROMBIOS_ROM) $(SEABIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM) $(ETHERBOOT_ROMS)
 	echo "/* Autogenerated file. DO NOT EDIT */" > $@.new
 
 ifneq ($(ROMBIOS_ROM),)
@@ -95,10 +97,11 @@ ifneq ($(CIRRUSVGA_ROM),)
 	sh ./mkhex vgabios_cirrusvga $(CIRRUSVGA_ROM) >> $@.new
 	echo "#endif" >> $@.new
 endif
-
+ifneq ($(ETHERBOOT_ROMS),)
 	echo "#ifdef ROM_INCLUDE_ETHERBOOT" >> $@.new
-	cat ../etherboot/eb-roms.h >> $@.new
+	sh ./mkhex etherboot $(ETHERBOOT_ROMS) >> $@.new
 	echo "#endif" >> $@.new
+endif
 
 	mv $@.new $@
 
-- 
Julian Pidancet

[PATCH v2 3/3] firmware: Introduce CONFIG_ROMBIOS and CONFIG_SEABIOS options

[Julian Pidancet]

This patch introduces configuration options allowing to built either
a rombios only or a seabios only hvmloader.

Building option ROMs like vgabios or etherboot is only enabled for
a rombios hvmloader, since SeaBIOS takes care or extracting option
ROMs itself from the PCI devices (these option ROMs are provided
by the device model and do not need to be built in hvmloader).

The Makefile in tools/firmware/ now only checks for bcc if rombios
is enabled.

These two configuration options are left on by default to remain
compatible.

Signed-off-by: Julian Pidancet <julian.pidancet@gmail.com>
---
 Config.mk                            |    3 +++
 tools/firmware/Makefile              |   21 +++++++++++++--------
 tools/firmware/hvmloader/Makefile    |   32 +++++++++++++++++++-------------
 tools/firmware/hvmloader/hvmloader.c |    4 ++++
 4 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/Config.mk b/Config.mk
index 42508b8..a43596c 100644
--- a/Config.mk
+++ b/Config.mk
@@ -224,6 +224,9 @@ SEABIOS_UPSTREAM_TAG ?= rel-1.6.3.1
 
 ETHERBOOT_NICS ?= rtl8139 8086100e
 
+CONFIG_ROMBIOS ?= y
+CONFIG_SEABIOS ?= y
+
 # Specify which qemu-dm to use. This may be `ioemu' to use the old
 # Mercurial in-tree version, or a local directory, or a git URL.
 # CONFIG_QEMU ?= `pwd`/$(XEN_ROOT)/../qemu-xen.git
diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile
index c3ec9a0..29d2041 100644
--- a/tools/firmware/Makefile
+++ b/tools/firmware/Makefile
@@ -5,19 +5,20 @@ include $(XEN_ROOT)/tools/Rules.mk
 TARGET      := hvmloader/hvmloader
 INST_DIR := $(DESTDIR)$(XENFIRMWAREDIR)
 
-SUBDIRS :=
-SUBDIRS += seabios-dir
-SUBDIRS += rombios
-SUBDIRS += vgabios
-SUBDIRS += etherboot
-SUBDIRS += hvmloader
+SUBDIRS-y :=
+SUBDIRS-$(CONFIG_SEABIOS) += seabios-dir
+SUBDIRS-$(CONFIG_ROMBIOS) += rombios
+SUBDIRS-$(CONFIG_ROMBIOS) += vgabios
+SUBDIRS-$(CONFIG_ROMBIOS) += etherboot
+SUBDIRS-y += hvmloader
 
 seabios-dir:
 	GIT=$(GIT) $(XEN_ROOT)/scripts/git-checkout.sh $(SEABIOS_UPSTREAM_URL) $(SEABIOS_UPSTREAM_TAG) seabios-dir
 	cp seabios-config seabios-dir/.config;
 
 .PHONY: all
-all: seabios-dir
+all:
+ifeq ($(CONFIG_ROMBIOS),y)
 	@set -e; if [ $$((`( bcc -v 2>&1 | grep version || echo 0.0.0 ) | cut -d' ' -f 3 | awk -F. '{ printf "0x%02x%02x%02x", $$1, $$2, $$3}'`)) -lt $$((0x00100e)) ] ; then \
 	echo "==========================================================================="; \
 	echo "Require dev86 rpm or bin86 & bcc debs version >= 0.16.14 to build firmware!"; \
@@ -25,7 +26,11 @@ all: seabios-dir
 	echo "==========================================================================="; \
 	false ; \
 	fi
-	$(MAKE) subdirs-$@; \
+endif
+ifeq ($(CONFIG_SEABIOS),y)
+	$(MAKE) seabios-dir
+endif
+	$(MAKE) subdirs-$@
 
 
 .PHONY: install
diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index 1ea32db..730bbba 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -38,27 +38,33 @@ endif
 CIRRUSVGA_DEBUG ?= n
 
 ROMBIOS_DIR := ../rombios
-ifneq ($(ROMBIOS_DIR),)
-OBJS += 32bitbios_support.o rombios.o
-CFLAGS += -DENABLE_ROMBIOS
-ROMBIOS_ROM := $(ROMBIOS_DIR)/BIOS-bochs-latest
-endif
-
 SEABIOS_DIR := ../seabios-dir
-ifneq ($(SEABIOS_DIR),)
-OBJS += seabios.o
-CFLAGS += -DENABLE_SEABIOS
-SEABIOS_ROM := $(SEABIOS_DIR)/out/bios.bin
-endif
 
+ifeq ($(CONFIG_ROMBIOS),y)
 STDVGA_ROM    := ../vgabios/VGABIOS-lgpl-latest.bin
 ifeq ($(CIRRUSVGA_DEBUG),y)
 CIRRUSVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.cirrus.debug.bin
 else
 CIRRUSVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.cirrus.bin
 endif
-
 ETHERBOOT_ROMS := $(addprefix ../etherboot/ipxe/src/bin/, $(addsuffix .rom, $(ETHERBOOT_NICS)))
+endif
+
+ROMS := 
+
+ifeq ($(CONFIG_ROMBIOS),y)
+OBJS += 32bitbios_support.o rombios.o
+CFLAGS += -DENABLE_ROMBIOS
+ROMBIOS_ROM := $(ROMBIOS_DIR)/BIOS-bochs-latest
+ROMS += $(ROMBIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM) $(ETHERBOOT_ROMS)
+endif
+
+ifeq ($(CONFIG_SEABIOS),y)
+OBJS += seabios.o
+CFLAGS += -DENABLE_SEABIOS
+SEABIOS_ROM := $(SEABIOS_DIR)/out/bios.bin
+ROMS += $(SEABIOS_ROM)
+endif
 
 .PHONY: all
 all: subdirs-all
@@ -72,7 +78,7 @@ hvmloader: $(OBJS) acpi/acpi.a
 	$(OBJCOPY) hvmloader.tmp hvmloader
 	rm -f hvmloader.tmp
 
-roms.inc: $(ROMBIOS_ROM) $(SEABIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM) $(ETHERBOOT_ROMS)
+roms.inc: $(ROMS)
 	echo "/* Autogenerated file. DO NOT EDIT */" > $@.new
 
 ifneq ($(ROMBIOS_ROM),)
diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
index f120ffe..874ee30 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -147,6 +147,7 @@ static void init_hypercalls(void)
     printf("Detected Xen v%u.%u%s\n", eax >> 16, eax & 0xffff, extraversion);
 }
 
+#ifdef ENABLE_ROMBIOS
 /*
  * Scan the list of Option ROMs at @roms for one which supports 
  * PCI (@vendor_id, @device_id) found at slot @devfn. If one is found,
@@ -309,6 +310,7 @@ static int pci_load_option_roms(unsigned int option_rom_end,
 
     return rom_phys_addr - rom_base_addr;
 }
+#endif
 
 /* Replace possibly erroneous memory-size CMOS fields with correct values. */
 static void cmos_write_memory_size(void)
@@ -470,6 +472,7 @@ int main(void)
             bios->create_pir_tables();
     }
 
+#ifdef ENABLE_ROMBIOS
     if ( bios->load_roms )
     {
         switch ( virtual_vga )
@@ -506,6 +509,7 @@ int main(void)
         option_rom_sz = pci_load_option_roms(bios->optionrom_end,
                                              option_rom_phys_addr);
     }
+#endif
 
     acpi_enabled = !strncmp(xenstore_read("platform/acpi", "1"), "1", 1);
 
-- 
Julian Pidancet

Re: [PATCH v2 1/3] hvmloader: Only compile 32bitbios_support.c when rombios is enabled

[Ian Campbell]

On Fri, 2012-02-10 at 01:53 +0000, Julian Pidancet wrote:
> 32bitbios_support.c only contains code specific to rombios, and should
> not be built-in when building hvmloader for SeaBIOS only (as for
> rombios.c).
> 
> Signed-off-by: Julian Pidancet <julian.pidancet@gmail.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  tools/firmware/hvmloader/Makefile |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
> index 41a4369..e82146a 100644
> --- a/tools/firmware/hvmloader/Makefile
> +++ b/tools/firmware/hvmloader/Makefile
> @@ -29,7 +29,7 @@ LOADADDR = 0x100000
>  CFLAGS += $(CFLAGS_xeninclude)
>  
>  OBJS  = hvmloader.o mp_tables.o util.o smbios.o 
> -OBJS += 32bitbios_support.o smp.o cacheattr.o xenbus.o
> +OBJS += smp.o cacheattr.o xenbus.o
>  OBJS += e820.o pci.o pir.o ctype.o
>  ifeq ($(debug),y)
>  OBJS += tests.o
> @@ -39,7 +39,7 @@ CIRRUSVGA_DEBUG ?= n
>  
>  ROMBIOS_DIR := ../rombios
>  ifneq ($(ROMBIOS_DIR),)
> -OBJS += rombios.o
> +OBJS += 32bitbios_support.o rombios.o
>  CFLAGS += -DENABLE_ROMBIOS
>  ROMBIOS_ROM := $(ROMBIOS_DIR)/BIOS-bochs-latest
>  endif

Re: [PATCH v2 2/3] firmware: Use mkhex from hvmloader directory for etherboot ROMs

[Ian Campbell]

On Fri, 2012-02-10 at 01:53 +0000, Julian Pidancet wrote:
> @@ -95,10 +97,11 @@ ifneq ($(CIRRUSVGA_ROM),)
>  	sh ./mkhex vgabios_cirrusvga $(CIRRUSVGA_ROM) >> $@.new
>  	echo "#endif" >> $@.new
>  endif
> -
> +ifneq ($(ETHERBOOT_ROMS),)
>  	echo "#ifdef ROM_INCLUDE_ETHERBOOT" >> $@.new
> -	cat ../etherboot/eb-roms.h >> $@.new
> +	sh ./mkhex etherboot $(ETHERBOOT_ROMS) >> $@.new

mkhex uses $1 as the array name and $2 as the binary data. However if
ETHERBOOT_ROMS contains two or more items then the second and subsequent
roms will be $3,$4 etc which are then not included.

I think you need to either quote something somewhere, use shift and $@
in mkhex or stick with the "cat $(ETHERBOOT_ROMS) | ./mkhex ..." style.

The rest of the patch looked good, thanks.

Ian.

Re: [PATCH v2 3/3] firmware: Introduce CONFIG_ROMBIOS and CONFIG_SEABIOS options

[Ian Campbell]

On Fri, 2012-02-10 at 01:53 +0000, Julian Pidancet wrote:
> @@ -470,6 +472,7 @@ int main(void)
>              bios->create_pir_tables();
>      }
>  
> +#ifdef ENABLE_ROMBIOS
>      if ( bios->load_roms )

Can we make bios->load_roms a function pointer instead of a bool. It can
be NULL for SeaBIOS and contain this block on ROMBIOS? Then all this
code (and pci_load_option_roms) could be moved to rombios.c.

>      {
>          switch ( virtual_vga )
> @@ -506,6 +509,7 @@ int main(void)
>          option_rom_sz = pci_load_option_roms(bios->optionrom_end,
>                                               option_rom_phys_addr);
>      }
> +#endif
>  
>      acpi_enabled = !strncmp(xenstore_read("platform/acpi", "1"), "1", 1);
>