[PATCH v3 0/6] Load ipxe from a standalone file

[PATCH v3 0/6] Load ipxe from a standalone file

[Wei Liu]

Addressed Jan and Roger's comments.

Wei Liu (6):
  Tools.mk.in: drop unused variables
  ipxe: produce a single binary from its build
  libxc: allow HVM guest to have modules
  tools: load IPXE from standalone file
  tools: provide --with-system-ipxe
  tools: --with-system-{ovmf,seabios,ipxe} should provide absolute paths

 config/Tools.mk.in                   |  3 +-
 tools/config.h.in                    |  3 ++
 tools/configure                      | 65 ++++++++++++++++++++++++++++++++++--
 tools/configure.ac                   | 30 +++++++++++++++--
 tools/firmware/Makefile              |  6 ++++
 tools/firmware/etherboot/Makefile    |  7 +++-
 tools/firmware/hvmloader/Makefile    |  9 +----
 tools/firmware/hvmloader/config.h    |  3 +-
 tools/firmware/hvmloader/hvmloader.c | 10 ++++--
 tools/firmware/hvmloader/ovmf.c      |  3 +-
 tools/firmware/hvmloader/rombios.c   | 24 +++++++++----
 tools/firmware/hvmloader/seabios.c   |  3 +-
 tools/libxc/xc_dom_x86.c             | 32 ++++++++++--------
 tools/libxl/libxl_dom.c              | 13 ++++++++
 tools/libxl/libxl_internal.h         |  1 +
 tools/libxl/libxl_paths.c            |  9 +++++
 16 files changed, 180 insertions(+), 41 deletions(-)

-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v3 1/6] Tools.mk.in: drop unused variables

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 config/Tools.mk.in | 2 --
 1 file changed, 2 deletions(-)

diff --git a/config/Tools.mk.in b/config/Tools.mk.in
index 2d6c440324..4cc9f29090 100644
--- a/config/Tools.mk.in
+++ b/config/Tools.mk.in
@@ -20,8 +20,6 @@ BCC                 := @BCC@
 IASL                := @IASL@
 AWK                 := @AWK@
 FETCHER             := @FETCHER@
-SEABIOS_PATH        := @seabios_path@
-OVMF_PATH           := @ovmf_path@
 
 # Extra folder for libs/includes
 PREPEND_INCLUDES    := @PREPEND_INCLUDES@
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v3 2/6] ipxe: produce a single binary from its build

[Wei Liu]

And switch hvmloader/Makefile to use that binary. This will help later
when we change hvmloader to pick a user provided binary.

No functional change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
v2: use intermediary file

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/firmware/etherboot/Makefile | 7 ++++++-
 tools/firmware/hvmloader/Makefile | 8 ++++----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/tools/firmware/etherboot/Makefile b/tools/firmware/etherboot/Makefile
index e33458d2fe..4de6d24a13 100644
--- a/tools/firmware/etherboot/Makefile
+++ b/tools/firmware/etherboot/Makefile
@@ -18,11 +18,16 @@ D=ipxe
 T=ipxe.tar.gz
 
 ROMS = $(addprefix $D/src/bin/, $(addsuffix .rom, $(ETHERBOOT_NICS)))
+ROM = $D/src/bin/ipxe.bin
 
 .NOTPARALLEL:
 
 .PHONY: all
-all: $(ROMS)
+all: $(ROM)
+
+$(ROM): $(ROMS)
+	cat $^ > $@.tmp
+	mv -f $@.tmp $@
 
 %.rom: $D/src/arch/i386/Makefile
 	$(MAKE) -C $D/src bin/$(*F).rom
diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index a5b4c32c1a..16255ebddd 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -51,7 +51,7 @@ 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)))
+ETHERBOOT_ROM := ../etherboot/ipxe/src/bin/ipxe.bin
 endif
 
 ROMS := 
@@ -60,7 +60,7 @@ ifeq ($(CONFIG_ROMBIOS),y)
 OBJS += optionroms.o 32bitbios_support.o rombios.o
 CFLAGS += -DENABLE_ROMBIOS
 ROMBIOS_ROM := $(ROMBIOS_DIR)/BIOS-bochs-latest
-ROMS += $(ROMBIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM) $(ETHERBOOT_ROMS)
+ROMS += $(ROMBIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM) $(ETHERBOOT_ROM)
 endif
 
 .PHONY: all
@@ -105,9 +105,9 @@ ifneq ($(CIRRUSVGA_ROM),)
 	sh ../../misc/mkhex vgabios_cirrusvga $(CIRRUSVGA_ROM) >> $@.new
 	echo "#endif" >> $@.new
 endif
-ifneq ($(ETHERBOOT_ROMS),)
+ifneq ($(ETHERBOOT_ROM),)
 	echo "#ifdef ROM_INCLUDE_ETHERBOOT" >> $@.new
-	sh ../../misc/mkhex etherboot $(ETHERBOOT_ROMS) >> $@.new
+	sh ../../misc/mkhex etherboot $(ETHERBOOT_ROM) >> $@.new
 	echo "#endif" >> $@.new
 endif
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v3 3/6] libxc: allow HVM guest to have modules

[Wei Liu]

Lift the loading code out of PVH specific branch. Take the chance to
make the debug message more useful.  Now the code needs to take into
account virt_base.

IPXE will be loaded as a module of Rombios.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxc/xc_dom_x86.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index e33a28847d..ed4973a997 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -1698,20 +1698,6 @@ static int bootlate_hvm(struct xc_dom_image *dom)
                                 ((uintptr_t)cmdline - (uintptr_t)start_info);
         }
 
-        for ( i = 0; i < dom->num_modules; i++ )
-        {
-            struct xc_hvm_firmware_module mod;
-
-            DOMPRINTF("Adding module %u", i);
-            mod.guest_addr_out =
-                dom->modules[i].seg.vstart - dom->parms.virt_base;
-            mod.length =
-                dom->modules[i].seg.vend - dom->modules[i].seg.vstart;
-
-            add_module_to_list(dom, &mod, dom->modules[i].cmdline,
-                               modlist, start_info);
-        }
-
         /* ACPI module 0 is the RSDP */
         start_info->rsdp_paddr = dom->acpi_modules[0].guest_addr_out ? : 0;
     }
@@ -1721,6 +1707,24 @@ static int bootlate_hvm(struct xc_dom_image *dom)
                            modlist, start_info);
     }
 
+    for ( i = 0; i < dom->num_modules; i++ )
+    {
+        struct xc_hvm_firmware_module mod;
+        uint64_t base = dom->parms.virt_base != UNSET_ADDR ?
+            dom->parms.virt_base : 0;
+
+        mod.guest_addr_out =
+            dom->modules[i].seg.vstart - base;
+        mod.length =
+            dom->modules[i].seg.vend - dom->modules[i].seg.vstart;
+
+        DOMPRINTF("Adding module %u guest_addr %"PRIx64" len %u",
+                  i, mod.guest_addr_out, mod.length);
+
+        add_module_to_list(dom, &mod, dom->modules[i].cmdline,
+                           modlist, start_info);
+    }
+
     if ( start_info->nr_modules )
     {
         start_info->modlist_paddr = (dom->start_info_seg.pfn << PAGE_SHIFT) +
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v3 4/6] tools: load IPXE from standalone file

[Wei Liu]

Do not embed IPXE into Rombios anymore. Instead, it is loaded by the
toolstack from a file as a separate module.

Ability to let user specify an IPXE blob will come later.

No user visible change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
v3: adjust libxl code a bit, addressed Jan's comment and added his
ack.

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: anthony.perard@citrix.com
---
 tools/firmware/Makefile              |  6 ++++++
 tools/firmware/hvmloader/Makefile    |  9 +--------
 tools/firmware/hvmloader/config.h    |  3 ++-
 tools/firmware/hvmloader/hvmloader.c | 10 ++++++++--
 tools/firmware/hvmloader/ovmf.c      |  3 ++-
 tools/firmware/hvmloader/rombios.c   | 24 +++++++++++++++++-------
 tools/firmware/hvmloader/seabios.c   |  3 ++-
 tools/libxl/libxl_dom.c              | 13 +++++++++++++
 tools/libxl/libxl_internal.h         |  1 +
 tools/libxl/libxl_paths.c            |  5 +++++
 10 files changed, 57 insertions(+), 20 deletions(-)

diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile
index 5a7cf7766d..0bef579637 100644
--- a/tools/firmware/Makefile
+++ b/tools/firmware/Makefile
@@ -55,6 +55,9 @@ endif
 ifeq ($(CONFIG_OVMF),y)
 	$(INSTALL_DATA) ovmf-dir/ovmf.bin $(INST_DIR)/ovmf.bin
 endif
+ifeq ($(CONFIG_IPXE),y)
+	$(INSTALL_DATA) etherboot/ipxe/src/bin/ipxe.bin $(INST_DIR)/ipxe.bin
+endif
 ifeq ($(CONFIG_PV_SHIM),y)
 	$(INSTALL_DATA) xen-dir/xen-shim $(INST_DIR)/xen-shim
 	$(INSTALL_DATA) xen-dir/xen-shim-syms $(DEBG_DIR)/xen-shim-syms
@@ -69,6 +72,9 @@ endif
 ifeq ($(CONFIG_OVMF),y)
 	rm -f $(INST_DIR)/ovmf.bin
 endif
+ifeq ($(CONFIG_IPXE),y)
+	rm -r $(INST_DIR)/ipxe.bin
+endif
 ifeq ($(CONFIG_PV_SHIM),y)
 	rm -f $(INST_DIR)/xen-shim
 	rm -f $(DEBG_DIR)/xen-shim-syms
diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index 16255ebddd..496ac72b77 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -51,7 +51,6 @@ CIRRUSVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.cirrus.debug.bin
 else
 CIRRUSVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.cirrus.bin
 endif
-ETHERBOOT_ROM := ../etherboot/ipxe/src/bin/ipxe.bin
 endif
 
 ROMS := 
@@ -60,7 +59,7 @@ ifeq ($(CONFIG_ROMBIOS),y)
 OBJS += optionroms.o 32bitbios_support.o rombios.o
 CFLAGS += -DENABLE_ROMBIOS
 ROMBIOS_ROM := $(ROMBIOS_DIR)/BIOS-bochs-latest
-ROMS += $(ROMBIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM) $(ETHERBOOT_ROM)
+ROMS += $(ROMBIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM)
 endif
 
 .PHONY: all
@@ -105,12 +104,6 @@ ifneq ($(CIRRUSVGA_ROM),)
 	sh ../../misc/mkhex vgabios_cirrusvga $(CIRRUSVGA_ROM) >> $@.new
 	echo "#endif" >> $@.new
 endif
-ifneq ($(ETHERBOOT_ROM),)
-	echo "#ifdef ROM_INCLUDE_ETHERBOOT" >> $@.new
-	sh ../../misc/mkhex etherboot $(ETHERBOOT_ROM) >> $@.new
-	echo "#endif" >> $@.new
-endif
-
 	mv $@.new $@
 
 .PHONY: clean
diff --git a/tools/firmware/hvmloader/config.h b/tools/firmware/hvmloader/config.h
index 6e00413f2e..d9b4713d63 100644
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -22,7 +22,8 @@ struct bios_config {
     /* ROMS */
     void (*load_roms)(void);
 
-    void (*bios_load)(const struct bios_config *config, void *addr, uint32_t size);
+    void (*bios_load)(const struct bios_config *config, void *addr,
+                      uint32_t size, void *extra_addr);
 
     void (*bios_info_setup)(void);
     void (*bios_info_finish)(void);
diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
index f603f68ded..598a226278 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -363,12 +363,18 @@ int main(void)
     {
         uint32_t paddr = bios_module->paddr;
 
-        bios->bios_load(bios, (void*)paddr, bios_module->size);
+        bios->bios_load(bios, (void *)paddr, bios_module->size, NULL);
     }
 #ifdef ENABLE_ROMBIOS
     else if ( bios == &rombios_config )
     {
-        bios->bios_load(bios, NULL, 0);
+        const struct hvm_modlist_entry *ipxe;
+        uint32_t paddr = 0;
+
+        ipxe = get_module_entry(hvm_start_info, "ipxe");
+        if ( ipxe )
+            paddr = ipxe->paddr;
+        bios->bios_load(bios, NULL, 0, (void *)paddr);
     }
 #endif
     else
diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
index a17a11c2f9..23610a0717 100644
--- a/tools/firmware/hvmloader/ovmf.c
+++ b/tools/firmware/hvmloader/ovmf.c
@@ -85,7 +85,8 @@ static void ovmf_finish_bios_info(void)
 }
 
 static void ovmf_load(const struct bios_config *config,
-                      void *bios_addr, uint32_t bios_length)
+                      void *bios_addr, uint32_t bios_length,
+                      void *unused_addr)
 {
     xen_pfn_t mfn;
     uint64_t addr = OVMF_END
diff --git a/tools/firmware/hvmloader/rombios.c b/tools/firmware/hvmloader/rombios.c
index c736fd9dea..46f331e465 100644
--- a/tools/firmware/hvmloader/rombios.c
+++ b/tools/firmware/hvmloader/rombios.c
@@ -63,6 +63,8 @@ static void rombios_setup_bios_info(void)
     memset(info, 0, sizeof(*info));
 }
 
+static void *ipxe_module_addr;
+
 static void rombios_load_roms(void)
 {
     int option_rom_sz = 0, vgabios_sz = 0, etherboot_sz = 0;
@@ -95,13 +97,17 @@ static void rombios_load_roms(void)
     etherboot_phys_addr = VGABIOS_PHYSICAL_ADDRESS + vgabios_sz;
     if ( etherboot_phys_addr < OPTIONROM_PHYSICAL_ADDRESS )
         etherboot_phys_addr = OPTIONROM_PHYSICAL_ADDRESS;
-    etherboot_sz = scan_etherboot_nic(OPTIONROM_PHYSICAL_END,
-                                      etherboot_phys_addr,
-                                      etherboot);
 
-    option_rom_phys_addr = etherboot_phys_addr + etherboot_sz;
-    option_rom_sz = pci_load_option_roms(OPTIONROM_PHYSICAL_END,
-                                         option_rom_phys_addr);
+    if ( ipxe_module_addr )
+    {
+        etherboot_sz = scan_etherboot_nic(OPTIONROM_PHYSICAL_END,
+                                          etherboot_phys_addr,
+                                          ipxe_module_addr);
+
+        option_rom_phys_addr = etherboot_phys_addr + etherboot_sz;
+        option_rom_sz = pci_load_option_roms(OPTIONROM_PHYSICAL_END,
+                                             option_rom_phys_addr);
+    }
 
     printf("Option ROMs:\n");
     if ( vgabios_sz )
@@ -119,7 +125,8 @@ static void rombios_load_roms(void)
 }
 
 static void rombios_load(const struct bios_config *config,
-                         void *unused_addr, uint32_t unused_size)
+                         void *unused_addr, uint32_t unused_size,
+                         void *ipxe_addr)
 {
     uint32_t bioshigh;
     struct rombios_info *info;
@@ -133,6 +140,9 @@ static void rombios_load(const struct bios_config *config,
 
     info = (struct rombios_info *)BIOS_INFO_PHYSICAL_ADDRESS;
     info->bios32_entry = bioshigh;
+
+    /* Stash ipxe address */
+    ipxe_module_addr = ipxe_addr;
 }
 
 /*
diff --git a/tools/firmware/hvmloader/seabios.c b/tools/firmware/hvmloader/seabios.c
index 801516daf7..444d118ddb 100644
--- a/tools/firmware/hvmloader/seabios.c
+++ b/tools/firmware/hvmloader/seabios.c
@@ -131,7 +131,8 @@ static void seabios_setup_e820(void)
 }
 
 static void seabios_load(const struct bios_config *bios,
-                         void *bios_addr, uint32_t bios_length)
+                         void *bios_addr, uint32_t bios_length,
+                         void *unused_addr)
 {
     unsigned int bios_dest = 0x100000 - bios_length;
 
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index f0fd5fd3a3..3cfe0d4808 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1128,6 +1128,19 @@ static int libxl__domain_firmware(libxl__gc *gc,
         if (rc) goto out;
     }
 
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM &&
+        info->u.hvm.bios == LIBXL_BIOS_TYPE_ROMBIOS &&
+        libxl__ipxe_path()) {
+        const char *fp = libxl__ipxe_path();
+        rc = xc_dom_module_file(dom, fp, "ipxe");
+
+        if (rc) {
+            LOGE(ERROR, "failed to load IPXE %s (%d)", fp, rc);
+            rc = ERROR_FAIL;
+            goto out;
+        }
+    }
+
     if (info->type == LIBXL_DOMAIN_TYPE_HVM &&
         info->u.hvm.smbios_firmware) {
         data = NULL;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index c582894589..518b75592c 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2362,6 +2362,7 @@ _hidden const char *libxl__lock_dir_path(void);
 _hidden const char *libxl__run_dir_path(void);
 _hidden const char *libxl__seabios_path(void);
 _hidden const char *libxl__ovmf_path(void);
+_hidden const char *libxl__ipxe_path(void);
 
 /*----- subprocess execution with timeout -----*/
 
diff --git a/tools/libxl/libxl_paths.c b/tools/libxl/libxl_paths.c
index 0643c1b3a4..8498f82781 100644
--- a/tools/libxl/libxl_paths.c
+++ b/tools/libxl/libxl_paths.c
@@ -53,6 +53,11 @@ const char *libxl__ovmf_path(void)
 #endif
 }
 
+const char *libxl__ipxe_path(void)
+{
+    return XENFIRMWAREDIR "/ipxe.bin";
+}
+
 /*
  * Local variables:
  * mode: C
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v3 5/6] tools: provide --with-system-ipxe

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v3: ipxe should require rombios

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 config/Tools.mk.in        |  1 +
 tools/config.h.in         |  3 +++
 tools/configure           | 58 +++++++++++++++++++++++++++++++++++++++++++++++
 tools/configure.ac        | 23 +++++++++++++++++++
 tools/libxl/libxl_paths.c |  6 ++++-
 5 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/config/Tools.mk.in b/config/Tools.mk.in
index 4cc9f29090..0964f6f9e9 100644
--- a/config/Tools.mk.in
+++ b/config/Tools.mk.in
@@ -50,6 +50,7 @@ FLASK_POLICY        := @xsmpolicy@
 CONFIG_OVMF         := @ovmf@
 CONFIG_ROMBIOS      := @rombios@
 CONFIG_SEABIOS      := @seabios@
+CONFIG_IPXE         := @ipxe@
 CONFIG_QEMU_TRAD    := @qemu_traditional@
 CONFIG_QEMU_XEN     := @qemu_xen@
 CONFIG_BLKTAP2      := @blktap2@
diff --git a/tools/config.h.in b/tools/config.h.in
index c66a78c9b3..5987f087b8 100644
--- a/tools/config.h.in
+++ b/tools/config.h.in
@@ -96,6 +96,9 @@
 /* libutil header file name */
 #undef INCLUDE_LIBUTIL_H
 
+/* IPXE path */
+#undef IPXE_PATH
+
 /* OVMF path */
 #undef OVMF_PATH
 
diff --git a/tools/configure b/tools/configure
index 4863f28306..4bff2c02fd 100755
--- a/tools/configure
+++ b/tools/configure
@@ -703,6 +703,7 @@ AS86
 qemu_traditional
 blktap2
 LINUX_BACKEND_MODULES
+ipxe
 seabios
 ovmf
 xsmpolicy
@@ -805,6 +806,7 @@ enable_ocamltools
 enable_xsmpolicy
 enable_ovmf
 enable_seabios
+enable_ipxe
 with_linux_backend_modules
 enable_blktap2
 enable_qemu_traditional
@@ -812,6 +814,7 @@ enable_rombios
 with_system_qemu
 with_system_seabios
 with_system_ovmf
+with_system_ipxe
 with_extra_qemuu_configure_args
 with_xenstored
 enable_systemd
@@ -1488,6 +1491,7 @@ Optional Features:
   --disable-xsmpolicy     Disable XSM policy compilation (default is ENABLED)
   --enable-ovmf           Enable OVMF (default is DISABLED)
   --disable-seabios       Disable SeaBIOS (default is ENABLED)
+  --disable-ipxe          Disable IPXE (default is ENABLED)
   --enable-blktap2        Enable blktap2, (DEFAULT is off)
   --enable-qemu-traditional
                           Enable qemu traditional device model, (DEFAULT is on
@@ -1527,6 +1531,9 @@ Optional Packages:
   --with-system-ovmf[=PATH]
                           Use system supplied OVMF PATH instead of building
                           and installing our own version
+  --with-system-ipxe[=PATH]
+                          Use system supplied IPXE PATH instead of building
+                          and installing our own version
   --with-extra-qemuu-configure-args[="--ARG1 ..."]
                           List of additional configure options for upstream
                           qemu
@@ -4184,6 +4191,29 @@ seabios=$ax_cv_seabios
 
 
 
+# Check whether --enable-ipxe was given.
+if test "${enable_ipxe+set}" = set; then :
+  enableval=$enable_ipxe;
+fi
+
+
+if test "x$enable_ipxe" = "xno"; then :
+
+    ax_cv_ipxe="n"
+
+elif test "x$enable_ipxe" = "xyes"; then :
+
+    ax_cv_ipxe="y"
+
+elif test -z $ax_cv_ipxe; then :
+
+    ax_cv_ipxe="y"
+
+fi
+ipxe=$ax_cv_ipxe
+
+
+
 
 # Check whether --with-linux-backend-modules was given.
 if test "${with_linux_backend_modules+set}" = set; then :
@@ -4573,6 +4603,34 @@ _ACEOF
 fi
 
 
+# Check whether --with-system-ipxe was given.
+if test "${with_system_ipxe+set}" = set; then :
+  withval=$with_system_ipxe;
+    # Disable compilation of IPXE.
+    ipxe=n
+    case $withval in
+        no) ipxe_path= ;;
+        *)  ipxe_path=$withval ;;
+    esac
+
+    # IPXE depends on Rombios
+    if test "x$enable_rombios" = "xno"; then
+        as_fn_error $? "Rombios is required for using IPXE" "$LINENO" 5
+    fi
+
+fi
+
+if test "x$ipxe" = "xy" -o -n "$ipxe_path" ; then :
+
+
+cat >>confdefs.h <<_ACEOF
+#define IPXE_PATH "${ipxe_path:-$XENFIRMWAREDIR/ipxe.bin}"
+_ACEOF
+
+
+fi
+
+
 # Check whether --with-extra-qemuu-configure-args was given.
 if test "${with_extra_qemuu_configure_args+set}" = set; then :
   withval=$with_extra_qemuu_configure_args;
diff --git a/tools/configure.ac b/tools/configure.ac
index 0826af8cbc..2db2356380 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -84,6 +84,7 @@ AX_ARG_DEFAULT_ENABLE([ocamltools], [Disable Ocaml tools])
 AX_ARG_DEFAULT_ENABLE([xsmpolicy], [Disable XSM policy compilation])
 AX_ARG_DEFAULT_DISABLE([ovmf], [Enable OVMF])
 AX_ARG_DEFAULT_ENABLE([seabios], [Disable SeaBIOS])
+AX_ARG_DEFAULT_ENABLE([ipxe], [Disable IPXE])
 
 AC_ARG_WITH([linux-backend-modules],
     AS_HELP_STRING([--with-linux-backend-modules="mod1 mod2"],
@@ -241,6 +242,28 @@ AS_IF([test "x$ovmf" = "xy" -o -n "$ovmf_path" ], [
                        [OVMF path])
 ])
 
+AC_ARG_WITH([system-ipxe],
+    AS_HELP_STRING([--with-system-ipxe@<:@=PATH@:>@],
+       [Use system supplied IPXE PATH instead of building and installing
+        our own version]),[
+    # Disable compilation of IPXE.
+    ipxe=n
+    case $withval in
+        no) ipxe_path= ;;
+        *)  ipxe_path=$withval ;;
+    esac
+
+    # IPXE depends on Rombios
+    if test "x$enable_rombios" = "xno"; then
+        AC_MSG_ERROR([Rombios is required for using IPXE])
+    fi
+],[])
+AS_IF([test "x$ipxe" = "xy" -o -n "$ipxe_path" ], [
+    AC_DEFINE_UNQUOTED([IPXE_PATH],
+                       ["${ipxe_path:-$XENFIRMWAREDIR/ipxe.bin}"],
+                       [IPXE path])
+])
+
 AC_ARG_WITH([extra-qemuu-configure-args],
     AS_HELP_STRING([--with-extra-qemuu-configure-args@<:@="--ARG1 ..."@:>@],
        [List of additional configure options for upstream qemu]),[
diff --git a/tools/libxl/libxl_paths.c b/tools/libxl/libxl_paths.c
index 8498f82781..3f6a33628e 100644
--- a/tools/libxl/libxl_paths.c
+++ b/tools/libxl/libxl_paths.c
@@ -55,7 +55,11 @@ const char *libxl__ovmf_path(void)
 
 const char *libxl__ipxe_path(void)
 {
-    return XENFIRMWAREDIR "/ipxe.bin";
+#ifdef IPXE_PATH
+    return IPXE_PATH;
+#else
+    return NULL;
+#endif
 }
 
 /*
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v3 6/6] tools: --with-system-{ovmf, seabios, ipxe} should provide absolute paths

[Wei Liu]

The paths shouldn't be set to "yes".

Reported-by: Anthony Perard <anthony.perard@citrix.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v3: really check for absolute paths.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>

new in v2
---
 tools/configure    | 9 ++++++---
 tools/configure.ac | 9 ++++++---
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/tools/configure b/tools/configure
index 4bff2c02fd..3b55fb2e36 100755
--- a/tools/configure
+++ b/tools/configure
@@ -4564,7 +4564,8 @@ if test "${with_system_seabios+set}" = set; then :
     seabios=n
     case $withval in
         no) seabios_path= ;;
-        *)  seabios_path=$withval ;;
+        /*)  seabios_path=$withval ;;
+        *) as_fn_error $? "Seabios specified, but is not an absolute path" "$LINENO" 5 ;;
     esac
 
 fi
@@ -4587,7 +4588,8 @@ if test "${with_system_ovmf+set}" = set; then :
     ovmf=n
     case $withval in
         no) ovmf_path= ;;
-        *)  ovmf_path=$withval ;;
+        /*)  ovmf_path=$withval ;;
+        *) as_fn_error $? "OVMF specified, but is not an absolute path" "$LINENO" 5 ;;
     esac
 
 fi
@@ -4610,7 +4612,8 @@ if test "${with_system_ipxe+set}" = set; then :
     ipxe=n
     case $withval in
         no) ipxe_path= ;;
-        *)  ipxe_path=$withval ;;
+        /*)  ipxe_path=$withval ;;
+        *) as_fn_error $? "IPXE specified, but is not an absolute path" "$LINENO" 5 ;;
     esac
 
     # IPXE depends on Rombios
diff --git a/tools/configure.ac b/tools/configure.ac
index 2db2356380..0f85472602 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -216,7 +216,8 @@ AC_ARG_WITH([system-seabios],
     seabios=n
     case $withval in
         no) seabios_path= ;;
-        *)  seabios_path=$withval ;;
+        /*)  seabios_path=$withval ;;
+        *) AC_MSG_ERROR([Seabios specified, but is not an absolute path]) ;;
     esac
 ],[])
 AS_IF([test "x$seabios" = "xy" -o -n "$seabios_path" ], [
@@ -233,7 +234,8 @@ AC_ARG_WITH([system-ovmf],
     ovmf=n
     case $withval in
         no) ovmf_path= ;;
-        *)  ovmf_path=$withval ;;
+        /*)  ovmf_path=$withval ;;
+        *) AC_MSG_ERROR([OVMF specified, but is not an absolute path]) ;;
     esac
 ],[])
 AS_IF([test "x$ovmf" = "xy" -o -n "$ovmf_path" ], [
@@ -250,7 +252,8 @@ AC_ARG_WITH([system-ipxe],
     ipxe=n
     case $withval in
         no) ipxe_path= ;;
-        *)  ipxe_path=$withval ;;
+        /*)  ipxe_path=$withval ;;
+        *) AC_MSG_ERROR([IPXE specified, but is not an absolute path]) ;;
     esac
 
     # IPXE depends on Rombios
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 3/6] libxc: allow HVM guest to have modules

[Ian Jackson]

Wei Liu writes ("[PATCH v3 3/6] libxc: allow HVM guest to have modules"):
> Lift the loading code out of PVH specific branch. Take the chance to
> make the debug message more useful.  Now the code needs to take into
> account virt_base.

I'm afraid this commit message's note about virt_base is a bit opaque,
and looks wrong to me.

AIUI you mean something more like

  Now the code needs to handle virt_base being UNSET_ADDR, which it is
  for HVM guests.  In that case seg.vstart is interpreted by ???
  as an ??? offset from zero ???

(As you can tell, I'm not very familiar with this area fo the code.)

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 3/6] libxc: allow HVM guest to have modules

[Wei Liu]

On Thu, Jul 05, 2018 at 11:29:41AM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v3 3/6] libxc: allow HVM guest to have modules"):
> > Lift the loading code out of PVH specific branch. Take the chance to
> > make the debug message more useful.  Now the code needs to take into
> > account virt_base.
> 
> I'm afraid this commit message's note about virt_base is a bit opaque,
> and looks wrong to me.
> 
> AIUI you mean something more like
> 
>   Now the code needs to handle virt_base being UNSET_ADDR, which it is
>   for HVM guests.  In that case seg.vstart is interpreted by ???
>   as an ??? offset from zero ???

Now the code needs to handle virt_base being UNSET_ADDR, which it is for
HVM guest. In case PVH and PV, virt_base is set by the respective loader
by parsing the binary.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 3/6] libxc: allow HVM guest to have modules

[Wei Liu]

On Thu, Jul 05, 2018 at 11:34:27AM +0100, Wei Liu wrote:
> On Thu, Jul 05, 2018 at 11:29:41AM +0100, Ian Jackson wrote:
> > Wei Liu writes ("[PATCH v3 3/6] libxc: allow HVM guest to have modules"):
> > > Lift the loading code out of PVH specific branch. Take the chance to
> > > make the debug message more useful.  Now the code needs to take into
> > > account virt_base.
> > 
> > I'm afraid this commit message's note about virt_base is a bit opaque,
> > and looks wrong to me.
> > 
> > AIUI you mean something more like
> > 
> >   Now the code needs to handle virt_base being UNSET_ADDR, which it is
> >   for HVM guests.  In that case seg.vstart is interpreted by ???
> >   as an ??? offset from zero ???
> 
> Now the code needs to handle virt_base being UNSET_ADDR, which it is for
> HVM guest. In case PVH and PV, virt_base is set by the respective loader
> by parsing the binary.

In case virt_base is not set, it should be treated as zero.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 4/6] tools: load IPXE from standalone file

[Ian Jackson]

Wei Liu writes ("[Xen-devel] [PATCH v3 4/6] tools: load IPXE from standalone file"):
> Do not embed IPXE into Rombios anymore. Instead, it is loaded by the
> toolstack from a file as a separate module.
...
> -    void (*bios_load)(const struct bios_config *config, void *addr, uint32_t size);
> +    void (*bios_load)(const struct bios_config *config, void *addr,
> +                      uint32_t size, void *extra_addr);
...
>  #ifdef ENABLE_ROMBIOS
>      else if ( bios == &rombios_config )
>      {
> -        bios->bios_load(bios, NULL, 0);
> +        const struct hvm_modlist_entry *ipxe;
> +        uint32_t paddr = 0;
> +
> +        ipxe = get_module_entry(hvm_start_info, "ipxe");
> +        if ( ipxe )
> +            paddr = ipxe->paddr;
> +        bios->bios_load(bios, NULL, 0, (void *)paddr);
...
>  static void ovmf_load(const struct bios_config *config,
> -                      void *bios_addr, uint32_t bios_length)
> +                      void *bios_addr, uint32_t bios_length,
> +                      void *unused_addr)
...
> +static void *ipxe_module_addr;
...
> +    if ( ipxe_module_addr )
> +    {
> +        etherboot_sz = scan_etherboot_nic(OPTIONROM_PHYSICAL_END,
> +                                          etherboot_phys_addr,
> +                                          ipxe_module_addr);
> +
> +        option_rom_phys_addr = etherboot_phys_addr + etherboot_sz;
> +        option_rom_sz = pci_load_option_roms(OPTIONROM_PHYSICAL_END,
> +                                             option_rom_phys_addr);
> +    }
...
>  static void rombios_load(const struct bios_config *config,
> -                         void *unused_addr, uint32_t unused_size)
> +                         void *unused_addr, uint32_t unused_size,
> +                         void *ipxe_addr)
>  {
>      uint32_t bioshigh;
>      struct rombios_info *info;
> @@ -133,6 +140,9 @@ static void rombios_load(const struct bios_config *config,
>  
>      info = (struct rombios_info *)BIOS_INFO_PHYSICAL_ADDRESS;
>      info->bios32_entry = bioshigh;
> +
> +    /* Stash ipxe address */
> +    ipxe_module_addr = ipxe_addr;

This seems to me to be a layering violation, and quite an ugly one at
that.  I'm afraid that at the very least, IMO this needs to be better
documented both in the code and the commit message.

Is ipxe rombios-specific ?  Forgive my ignorance.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 5/6] tools: provide --with-system-ipxe

[Ian Jackson]

Wei Liu writes ("[PATCH v3 5/6] tools: provide --with-system-ipxe"):
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

But actually I think the effect of --with-system-ipxe is to simply not
load any ipxe.  Surely that isn't right ?

Perhaps I am just misunderstanding something, in which case more
words (perhaps in the commit message) are necessary.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 6/6] tools: --with-system-{ovmf, seabios, ipxe} should provide absolute paths

[Ian Jackson]

Wei Liu writes ("[PATCH v3 6/6] tools: --with-system-{ovmf,seabios,ipxe} should provide absolute paths"):
> The paths shouldn't be set to "yes".

This is rather anomalous compared to the way that --with-system-blah
normally works.  Surely, instead, we should search for the right thing
and use it ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 3/6] libxc: allow HVM guest to have modules

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v3 3/6] libxc: allow HVM guest to have modules"):
> On Thu, Jul 05, 2018 at 11:29:41AM +0100, Ian Jackson wrote:
> > AIUI you mean something more like
> > 
> >   Now the code needs to handle virt_base being UNSET_ADDR, which it is
> >   for HVM guests.  In that case seg.vstart is interpreted by ???
> >   as an ??? offset from zero ???
> 
> Now the code needs to handle virt_base being UNSET_ADDR, which it is for
> HVM guest. In case PVH and PV, virt_base is set by the respective loader
> by parsing the binary.

That doesn't explain how the necessary value of seg.vstart can be
calculated without needing to know the not-yet-calculated value of
virt_base.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 3/6] libxc: allow HVM guest to have modules

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v3 3/6] libxc: allow HVM guest to have modules"):
> In case virt_base is not set, it should be treated as zero.

Why ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 6/6] tools: --with-system-{ovmf, seabios, ipxe} should provide absolute paths

[Wei Liu]

On Thu, Jul 05, 2018 at 11:41:32AM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v3 6/6] tools: --with-system-{ovmf,seabios,ipxe} should provide absolute paths"):
> > The paths shouldn't be set to "yes".
> 
> This is rather anomalous compared to the way that --with-system-blah
> normally works.  Surely, instead, we should search for the right thing
> and use it ?

How can we know where to search?

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 5/6] tools: provide --with-system-ipxe

[Wei Liu]

On Thu, Jul 05, 2018 at 11:40:36AM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v3 5/6] tools: provide --with-system-ipxe"):
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> But actually I think the effect of --with-system-ipxe is to simply not
> load any ipxe.  Surely that isn't right ?

Its purpose is for packager to provide an ipxe binary to be loaded
for rombios. That also means we don't need to build the in-tree ipxe
anymore. Just like --with-system-qemu.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 3/6] libxc: allow HVM guest to have modules

[Wei Liu]

On Thu, Jul 05, 2018 at 11:42:50AM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH v3 3/6] libxc: allow HVM guest to have modules"):
> > In case virt_base is not set, it should be treated as zero.
> 
> Why ?

Virt_base is the virtual address base for loading a binary. If it is not
set it should be treated as 0, otherwise nothing good will come out from
it.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 4/6] tools: load IPXE from standalone file

[Wei Liu]

On Thu, Jul 05, 2018 at 11:39:46AM +0100, Ian Jackson wrote:
> Wei Liu writes ("[Xen-devel] [PATCH v3 4/6] tools: load IPXE from standalone file"):
> > Do not embed IPXE into Rombios anymore. Instead, it is loaded by the
> > toolstack from a file as a separate module.
> ...
> > -    void (*bios_load)(const struct bios_config *config, void *addr, uint32_t size);
> > +    void (*bios_load)(const struct bios_config *config, void *addr,
> > +                      uint32_t size, void *extra_addr);
> ...
> >  #ifdef ENABLE_ROMBIOS
> >      else if ( bios == &rombios_config )
> >      {
> > -        bios->bios_load(bios, NULL, 0);
> > +        const struct hvm_modlist_entry *ipxe;
> > +        uint32_t paddr = 0;
> > +
> > +        ipxe = get_module_entry(hvm_start_info, "ipxe");
> > +        if ( ipxe )
> > +            paddr = ipxe->paddr;
> > +        bios->bios_load(bios, NULL, 0, (void *)paddr);
> ...
> >  static void ovmf_load(const struct bios_config *config,
> > -                      void *bios_addr, uint32_t bios_length)
> > +                      void *bios_addr, uint32_t bios_length,
> > +                      void *unused_addr)
> ...
> > +static void *ipxe_module_addr;
> ...
> > +    if ( ipxe_module_addr )
> > +    {
> > +        etherboot_sz = scan_etherboot_nic(OPTIONROM_PHYSICAL_END,
> > +                                          etherboot_phys_addr,
> > +                                          ipxe_module_addr);
> > +
> > +        option_rom_phys_addr = etherboot_phys_addr + etherboot_sz;
> > +        option_rom_sz = pci_load_option_roms(OPTIONROM_PHYSICAL_END,
> > +                                             option_rom_phys_addr);
> > +    }
> ...
> >  static void rombios_load(const struct bios_config *config,
> > -                         void *unused_addr, uint32_t unused_size)
> > +                         void *unused_addr, uint32_t unused_size,
> > +                         void *ipxe_addr)
> >  {
> >      uint32_t bioshigh;
> >      struct rombios_info *info;
> > @@ -133,6 +140,9 @@ static void rombios_load(const struct bios_config *config,
> >  
> >      info = (struct rombios_info *)BIOS_INFO_PHYSICAL_ADDRESS;
> >      info->bios32_entry = bioshigh;
> > +
> > +    /* Stash ipxe address */
> > +    ipxe_module_addr = ipxe_addr;
> 
> This seems to me to be a layering violation, and quite an ugly one at
> that.  I'm afraid that at the very least, IMO this needs to be better
> documented both in the code and the commit message.
> 

There isn't a better way to do this at the moment. Plus that ...

> Is ipxe rombios-specific ?  Forgive my ignorance.

... ipxe is rombios-specific at the moment, so Jan is fine with changes
like this.

Wei.

> 
> Thanks,
> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 3/6] libxc: allow HVM guest to have modules

[Wei Liu]

On Thu, Jul 05, 2018 at 11:42:34AM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH v3 3/6] libxc: allow HVM guest to have modules"):
> > On Thu, Jul 05, 2018 at 11:29:41AM +0100, Ian Jackson wrote:
> > > AIUI you mean something more like
> > > 
> > >   Now the code needs to handle virt_base being UNSET_ADDR, which it is
> > >   for HVM guests.  In that case seg.vstart is interpreted by ???
> > >   as an ??? offset from zero ???
> > 
> > Now the code needs to handle virt_base being UNSET_ADDR, which it is for
> > HVM guest. In case PVH and PV, virt_base is set by the respective loader
> > by parsing the binary.
> 
> That doesn't explain how the necessary value of seg.vstart can be
> calculated without needing to know the not-yet-calculated value of
> virt_base.

virt_base should already be calculated at this point. It is either set
or not. There is possibility for setting 0 at some point but it doesn't
change how we interpret UNSET_ADDR.

Wei.

> 
> Thanks,
> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel