* [PATCH v8 01/13] libxc: Rework extra module initialisation
2016-08-18 14:13 [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
@ 2016-08-18 14:13 ` Wei Liu
2016-08-18 14:13 ` [PATCH v8 02/13] libxc: Prepare a start info structure for hvmloader Wei Liu
` (12 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-18 14:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
This patch use xc_dom_alloc_segment() to allocate the memory space for the
ACPI modules and the SMBIOS modules. This is to replace the arbitrary
placement of 1MB (+ extra for MB alignement) after the hvmloader image.
This patch can help if one add extra ACPI table and hvmloader contain
OVMF (OVMF is a 2MB binary), as in that case the extra ACPI table could
easily be loaded past the address 4MB, but hvmloader use a range of
memory from 4MB to 10MB to perform tests and in the process, clears the
memory, before loading the modules.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Changes in V6:
- in module_init_one(), check that the module is to be loaded bellow
dom->mmio_start instead of UINT32_MAX.
Changes in V5:
- rewrite patch description
Changes in V4:
- check that guest_addr_out have a reasonable value.
New patch in V3.
---
tools/libxc/xc_dom_hvmloader.c | 133 +++++++++++++----------------------------
1 file changed, 40 insertions(+), 93 deletions(-)
diff --git a/tools/libxc/xc_dom_hvmloader.c b/tools/libxc/xc_dom_hvmloader.c
index 330d5e8..8150b74 100644
--- a/tools/libxc/xc_dom_hvmloader.c
+++ b/tools/libxc/xc_dom_hvmloader.c
@@ -129,98 +129,54 @@ static elf_errorstatus xc_dom_parse_hvm_kernel(struct xc_dom_image *dom)
return rc;
}
-static int modules_init(struct xc_dom_image *dom,
- uint64_t vend, struct elf_binary *elf,
- uint64_t *mstart_out, uint64_t *mend_out)
+static int module_init_one(struct xc_dom_image *dom,
+ struct xc_hvm_firmware_module *module,
+ char *name)
{
-#define MODULE_ALIGN 1UL << 7
-#define MB_ALIGN 1UL << 20
-#define MKALIGN(x, a) (((uint64_t)(x) + (a) - 1) & ~(uint64_t)((a) - 1))
- uint64_t total_len = 0, offset1 = 0;
+ struct xc_dom_seg seg;
+ void *dest;
- if ( dom->acpi_module.length == 0 && dom->smbios_module.length == 0 )
- return 0;
-
- /* Find the total length for the firmware modules with a reasonable large
- * alignment size to align each the modules.
- */
- total_len = MKALIGN(dom->acpi_module.length, MODULE_ALIGN);
- offset1 = total_len;
- total_len += MKALIGN(dom->smbios_module.length, MODULE_ALIGN);
-
- /* Want to place the modules 1Mb+change behind the loader image. */
- *mstart_out = MKALIGN(elf->pend, MB_ALIGN) + (MB_ALIGN);
- *mend_out = *mstart_out + total_len;
-
- if ( *mend_out > vend )
- return -1;
-
- if ( dom->acpi_module.length != 0 )
- dom->acpi_module.guest_addr_out = *mstart_out;
- if ( dom->smbios_module.length != 0 )
- dom->smbios_module.guest_addr_out = *mstart_out + offset1;
+ if ( module->length )
+ {
+ if ( xc_dom_alloc_segment(dom, &seg, name, 0, module->length) )
+ goto err;
+ dest = xc_dom_seg_to_ptr(dom, &seg);
+ if ( dest == NULL )
+ {
+ DOMPRINTF("%s: xc_dom_seg_to_ptr(dom, &seg) => NULL",
+ __FUNCTION__);
+ goto err;
+ }
+ memcpy(dest, module->data, module->length);
+ module->guest_addr_out = seg.vstart;
+
+ assert(dom->mmio_start > 0 && dom->mmio_start < UINT32_MAX);
+ if ( module->guest_addr_out > dom->mmio_start ||
+ module->guest_addr_out + module->length > dom->mmio_start )
+ {
+ DOMPRINTF("%s: Module %s would be loaded abrove 4GB",
+ __FUNCTION__, name);
+ goto err;
+ }
+ }
return 0;
+err:
+ return -1;
}
-static int loadmodules(struct xc_dom_image *dom,
- uint64_t mstart, uint64_t mend,
- uint32_t domid)
+static int modules_init(struct xc_dom_image *dom)
{
- privcmd_mmap_entry_t *entries = NULL;
- unsigned long pfn_start;
- unsigned long pfn_end;
- size_t pages;
- uint32_t i;
- uint8_t *dest;
- int rc = -1;
- xc_interface *xch = dom->xch;
-
- if ( mstart == 0 || mend == 0 )
- return 0;
-
- pfn_start = (unsigned long)(mstart >> PAGE_SHIFT);
- pfn_end = (unsigned long)((mend + PAGE_SIZE - 1) >> PAGE_SHIFT);
- pages = pfn_end - pfn_start;
-
- /* Map address space for module list. */
- entries = calloc(pages, sizeof(privcmd_mmap_entry_t));
- if ( entries == NULL )
- goto error_out;
-
- for ( i = 0; i < pages; i++ )
- entries[i].mfn = (mstart >> PAGE_SHIFT) + i;
-
- dest = xc_map_foreign_ranges(
- xch, domid, pages << PAGE_SHIFT, PROT_READ | PROT_WRITE, 1 << PAGE_SHIFT,
- entries, pages);
- if ( dest == NULL )
- goto error_out;
-
- /* Zero the range so padding is clear between modules */
- memset(dest, 0, pages << PAGE_SHIFT);
-
- /* Load modules into range */
- if ( dom->acpi_module.length != 0 )
- {
- memcpy(dest,
- dom->acpi_module.data,
- dom->acpi_module.length);
- }
- if ( dom->smbios_module.length != 0 )
- {
- memcpy(dest + (dom->smbios_module.guest_addr_out - mstart),
- dom->smbios_module.data,
- dom->smbios_module.length);
- }
-
- munmap(dest, pages << PAGE_SHIFT);
- rc = 0;
+ int rc;
- error_out:
- free(entries);
+ rc = module_init_one(dom, &dom->acpi_module, "ACPI module");
+ if ( rc ) goto err;
+ rc = module_init_one(dom, &dom->smbios_module, "SMBIOS module");
+ if ( rc ) goto err;
- return rc;
+ return 0;
+err:
+ return -1;
}
static elf_errorstatus xc_dom_load_hvm_kernel(struct xc_dom_image *dom)
@@ -229,7 +185,6 @@ static elf_errorstatus xc_dom_load_hvm_kernel(struct xc_dom_image *dom)
privcmd_mmap_entry_t *entries = NULL;
size_t pages = (elf->pend - elf->pstart + PAGE_SIZE - 1) >> PAGE_SHIFT;
elf_errorstatus rc;
- uint64_t m_start = 0, m_end = 0;
int i;
/* Map address space for initial elf image. */
@@ -262,15 +217,7 @@ static elf_errorstatus xc_dom_load_hvm_kernel(struct xc_dom_image *dom)
munmap(elf->dest_base, elf->dest_size);
- rc = modules_init(dom, dom->total_pages << PAGE_SHIFT, elf, &m_start,
- &m_end);
- if ( rc != 0 )
- {
- DOMPRINTF("%s: insufficient space to load modules.", __func__);
- goto error;
- }
-
- rc = loadmodules(dom, m_start, m_end, dom->guest_domid);
+ rc = modules_init(dom);
if ( rc != 0 )
{
DOMPRINTF("%s: unable to load modules.", __func__);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v8 02/13] libxc: Prepare a start info structure for hvmloader
2016-08-18 14:13 [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
2016-08-18 14:13 ` [PATCH v8 01/13] libxc: Rework extra module initialisation Wei Liu
@ 2016-08-18 14:13 ` Wei Liu
2016-08-18 14:13 ` [PATCH v8 03/13] configure: #define SEABIOS_PATH and OVMF_PATH Wei Liu
` (11 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-18 14:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
... and load BIOS/UEFI firmware into guest memory.
This adds a new firmware module, system_firmware_module. It is loaded in
the guest memory and final location is provided to hvmloader via the
hvm_start_info struct.
This patch create the hvm_start_info struct for HVM guest that have a
device model, so this is now common code with HVM guest without device
model.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Changes in v8:
- rebased on top of staging
Changes in V6:
- change description (for verbose output) of the allocation of
hvm_start_info from "HVMlite start info" to "HVM start info".
Changes in V5:
- in alloc_magic_pages_hvm, check dom->device_model only once instead of
twice (fold second if into previous else)
- rework add_module_to_list to make it easier to read
- also comment about the intended memory layout of start_info and the
modules
- in bootlate_hvm(), drop start_page and use start_info as they point to
the same address
- rename xc_dom_image.bios_module to xc_dom_image.system_firmware_module
- rename module name to "firmware" (was "bios")
Changes in V4:
- change title to suggest the change of beavior
- remove code to load acpi tables (dsdt)
- Update public/xen.h about hvm_start_info available on other HVM guest
in %ebx.
Changes in V3:
- rename acpi_table_module to full_acpi_module.
- factorise module loading, using new function to load existing optinal
module, this should not change anything
- should now use the same code to loads modules as for HVMlite VMs.
this avoid duplication of code.
- no more generic cmdline with a list of modules, each module have its name
in the module specific cmdline.
- scope change for common code between hvmlite and hvmloader
---
tools/libxc/include/xc_dom.h | 3 +
tools/libxc/xc_dom_hvmloader.c | 3 +
tools/libxc/xc_dom_x86.c | 152 +++++++++++++++++++++++++++++------------
3 files changed, 115 insertions(+), 43 deletions(-)
diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
index cac4698..de7dca9 100644
--- a/tools/libxc/include/xc_dom.h
+++ b/tools/libxc/include/xc_dom.h
@@ -209,6 +209,9 @@ struct xc_dom_image {
/* If unset disables the setup of the IOREQ pages. */
bool device_model;
+ /* BIOS/Firmware passed to HVMLOADER */
+ struct xc_hvm_firmware_module system_firmware_module;
+
/* Extra ACPI tables passed to HVMLOADER */
struct xc_hvm_firmware_module acpi_module;
diff --git a/tools/libxc/xc_dom_hvmloader.c b/tools/libxc/xc_dom_hvmloader.c
index 8150b74..6eb8516 100644
--- a/tools/libxc/xc_dom_hvmloader.c
+++ b/tools/libxc/xc_dom_hvmloader.c
@@ -169,6 +169,9 @@ static int modules_init(struct xc_dom_image *dom)
{
int rc;
+ rc = module_init_one(dom, &dom->system_firmware_module,
+ "System Firmware module");
+ if ( rc ) goto err;
rc = module_init_one(dom, &dom->acpi_module, "ACPI module");
if ( rc ) goto err;
rc = module_init_one(dom, &dom->smbios_module, "SMBIOS module");
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index d9747b4..0eab8a7 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -70,6 +70,9 @@
#define round_up(addr, mask) ((addr) | (mask))
#define round_pg_up(addr) (((addr) + PAGE_SIZE_X86 - 1) & ~(PAGE_SIZE_X86 - 1))
+#define HVMLOADER_MODULE_MAX_COUNT 1
+#define HVMLOADER_MODULE_NAME_SIZE 10
+
struct xc_dom_params {
unsigned levels;
xen_vaddr_t vaddr_mask;
@@ -591,6 +594,7 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
xen_pfn_t special_array[X86_HVM_NR_SPECIAL_PAGES];
xen_pfn_t ioreq_server_array[NR_IOREQ_SERVER_PAGES];
xc_interface *xch = dom->xch;
+ size_t start_info_size = sizeof(struct hvm_start_info);
/* Allocate and clear special pages. */
for ( i = 0; i < X86_HVM_NR_SPECIAL_PAGES; i++ )
@@ -625,8 +629,6 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
if ( !dom->device_model )
{
- size_t start_info_size = sizeof(struct hvm_start_info);
-
if ( dom->cmdline )
{
dom->cmdline_size = ROUNDUP(strlen(dom->cmdline) + 1, 8);
@@ -636,17 +638,18 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
/* Limited to one module. */
if ( dom->ramdisk_blob )
start_info_size += sizeof(struct hvm_modlist_entry);
-
- rc = xc_dom_alloc_segment(dom, &dom->start_info_seg,
- "HVMlite start info", 0, start_info_size);
- if ( rc != 0 )
- {
- DOMPRINTF("Unable to reserve memory for the start info");
- goto out;
- }
}
else
{
+ start_info_size +=
+ sizeof(struct hvm_modlist_entry) * HVMLOADER_MODULE_MAX_COUNT;
+ /*
+ * Add extra space to write modules name.
+ * The HVMLOADER_MODULE_NAME_SIZE accounts for NUL byte.
+ */
+ start_info_size +=
+ HVMLOADER_MODULE_NAME_SIZE * HVMLOADER_MODULE_MAX_COUNT;
+
/*
* Allocate and clear additional ioreq server pages. The default
* server will use the IOREQ and BUFIOREQ special pages above.
@@ -673,6 +676,14 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
NR_IOREQ_SERVER_PAGES);
}
+ rc = xc_dom_alloc_segment(dom, &dom->start_info_seg,
+ "HVM start info", 0, start_info_size);
+ if ( rc != 0 )
+ {
+ DOMPRINTF("Unable to reserve memory for the start info");
+ goto out;
+ }
+
/*
* Identity-map page table is required for running with CR0.PG=0 when
* using Intel EPT. Create a 32-bit non-PAE page directory of superpages.
@@ -1690,42 +1701,89 @@ static int alloc_pgtables_hvm(struct xc_dom_image *dom)
return 0;
}
+/*
+ * The memory layout of the start_info page and the modules, and where the
+ * addresses are stored:
+ *
+ * /----------------------------------\
+ * | struct hvm_start_info |
+ * +----------------------------------+ <- start_info->modlist_paddr
+ * | struct hvm_modlist_entry[0] |
+ * +----------------------------------+
+ * | struct hvm_modlist_entry[1] |
+ * +----------------------------------+ <- modlist[0].cmdline_paddr
+ * | cmdline of module 0 |
+ * | char[HVMLOADER_MODULE_NAME_SIZE] |
+ * +----------------------------------+ <- modlist[1].cmdline_paddr
+ * | cmdline of module 1 |
+ * +----------------------------------+
+ */
+static void add_module_to_list(struct xc_dom_image *dom,
+ struct xc_hvm_firmware_module *module,
+ const char *name,
+ struct hvm_modlist_entry *modlist,
+ struct hvm_start_info *start_info)
+{
+ uint32_t index = start_info->nr_modules;
+ void *modules_cmdline_start = modlist + HVMLOADER_MODULE_MAX_COUNT;
+ uint64_t modlist_paddr = (dom->start_info_seg.pfn << PAGE_SHIFT) +
+ ((uintptr_t)modlist - (uintptr_t)start_info);
+ uint64_t modules_cmdline_paddr = modlist_paddr +
+ sizeof(struct hvm_modlist_entry) * HVMLOADER_MODULE_MAX_COUNT;
+
+ if ( module->length == 0 )
+ return;
+
+ assert(start_info->nr_modules < HVMLOADER_MODULE_MAX_COUNT);
+ assert(strnlen(name, HVMLOADER_MODULE_NAME_SIZE)
+ < HVMLOADER_MODULE_NAME_SIZE);
+
+ modlist[index].paddr = module->guest_addr_out;
+ modlist[index].size = module->length;
+
+ strncpy(modules_cmdline_start + HVMLOADER_MODULE_NAME_SIZE * index,
+ name, HVMLOADER_MODULE_NAME_SIZE);
+ modlist[index].cmdline_paddr =
+ modules_cmdline_paddr + HVMLOADER_MODULE_NAME_SIZE * index;
+
+ start_info->nr_modules++;
+}
+
static int bootlate_hvm(struct xc_dom_image *dom)
{
uint32_t domid = dom->guest_domid;
xc_interface *xch = dom->xch;
+ struct hvm_start_info *start_info;
+ size_t start_info_size;
+ struct hvm_modlist_entry *modlist;
- if ( !dom->device_model )
- {
- struct hvm_start_info *start_info;
- size_t start_info_size;
- void *start_page;
-
- start_info_size = sizeof(*start_info) + dom->cmdline_size;
- if ( dom->ramdisk_blob )
- start_info_size += sizeof(struct hvm_modlist_entry);
+ start_info_size = sizeof(*start_info) + dom->cmdline_size;
+ if ( dom->ramdisk_blob )
+ start_info_size += sizeof(struct hvm_modlist_entry);
- if ( start_info_size >
- dom->start_info_seg.pages << XC_DOM_PAGE_SHIFT(dom) )
- {
- DOMPRINTF("Trying to map beyond start_info_seg");
- return -1;
- }
+ if ( start_info_size >
+ dom->start_info_seg.pages << XC_DOM_PAGE_SHIFT(dom) )
+ {
+ DOMPRINTF("Trying to map beyond start_info_seg");
+ return -1;
+ }
- start_page = xc_map_foreign_range(xch, domid, start_info_size,
- PROT_READ | PROT_WRITE,
- dom->start_info_seg.pfn);
- if ( start_page == NULL )
- {
- DOMPRINTF("Unable to map HVM start info page");
- return -1;
- }
+ start_info = xc_map_foreign_range(xch, domid, start_info_size,
+ PROT_READ | PROT_WRITE,
+ dom->start_info_seg.pfn);
+ if ( start_info == NULL )
+ {
+ DOMPRINTF("Unable to map HVM start info page");
+ return -1;
+ }
- start_info = start_page;
+ modlist = (void*)(start_info + 1) + dom->cmdline_size;
+ if ( !dom->device_model )
+ {
if ( dom->cmdline )
{
- char *cmdline = start_page + sizeof(*start_info);
+ char *cmdline = (void*)(start_info + 1);
strncpy(cmdline, dom->cmdline, dom->cmdline_size);
start_info->cmdline_paddr = (dom->start_info_seg.pfn << PAGE_SHIFT) +
@@ -1734,22 +1792,30 @@ static int bootlate_hvm(struct xc_dom_image *dom)
if ( dom->ramdisk_blob )
{
- struct hvm_modlist_entry *modlist =
- start_page + sizeof(*start_info) + dom->cmdline_size;
modlist[0].paddr = dom->ramdisk_seg.vstart - dom->parms.virt_base;
modlist[0].size = dom->ramdisk_seg.vend - dom->ramdisk_seg.vstart;
- start_info->modlist_paddr = (dom->start_info_seg.pfn << PAGE_SHIFT) +
- ((uintptr_t)modlist - (uintptr_t)start_info);
start_info->nr_modules = 1;
}
-
- start_info->magic = XEN_HVM_START_MAGIC_VALUE;
-
- munmap(start_page, start_info_size);
}
else
{
+ add_module_to_list(dom, &dom->system_firmware_module, "firmware",
+ modlist, start_info);
+ }
+
+ if ( start_info->nr_modules )
+ {
+ start_info->modlist_paddr = (dom->start_info_seg.pfn << PAGE_SHIFT) +
+ ((uintptr_t)modlist - (uintptr_t)start_info);
+ }
+
+ start_info->magic = XEN_HVM_START_MAGIC_VALUE;
+
+ munmap(start_info, start_info_size);
+
+ if ( dom->device_model )
+ {
void *hvm_info_page;
if ( (hvm_info_page = xc_map_foreign_range(
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v8 03/13] configure: #define SEABIOS_PATH and OVMF_PATH
2016-08-18 14:13 [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
2016-08-18 14:13 ` [PATCH v8 01/13] libxc: Rework extra module initialisation Wei Liu
2016-08-18 14:13 ` [PATCH v8 02/13] libxc: Prepare a start info structure for hvmloader Wei Liu
@ 2016-08-18 14:13 ` Wei Liu
2016-08-18 14:13 ` [PATCH v8 04/13] firmware/makefile: install BIOS blob Wei Liu
` (10 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-18 14:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
Those paths are to be used by libxl, in order to load the firmware in
memory. If a system path is not defined via --with-system-seabios or
--with-system-ovmf, then default to the Xen firmware directory.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Please, run ./autogen.sh on this patch.
Change in V5:
- rename seabios.bin to bios.bin.
---
tools/config.h.in | 6 ++++++
tools/configure | 10 ++++++++++
tools/configure.ac | 6 ++++++
3 files changed, 22 insertions(+)
diff --git a/tools/config.h.in b/tools/config.h.in
index 478a2cc..f65eec4 100644
--- a/tools/config.h.in
+++ b/tools/config.h.in
@@ -96,6 +96,9 @@
/* libutil header file name */
#undef INCLUDE_LIBUTIL_H
+/* OVMF path */
+#undef OVMF_PATH
+
/* Define to the address where bug reports for this package should be sent. */
#undef PACKAGE_BUGREPORT
@@ -117,6 +120,9 @@
/* Qemu Xen path */
#undef QEMU_XEN_PATH
+/* SeaBIOS path */
+#undef SEABIOS_PATH
+
/* Define to 1 if you have the ANSI C header files. */
#undef STDC_HEADERS
diff --git a/tools/configure b/tools/configure
index 51f16c5..c182391 100755
--- a/tools/configure
+++ b/tools/configure
@@ -4451,6 +4451,11 @@ fi
+cat >>confdefs.h <<_ACEOF
+#define SEABIOS_PATH "${seabios_path:-$XENFIRMWAREDIR/bios.bin}"
+_ACEOF
+
+
# Check whether --with-system-ovmf was given.
if test "${with_system_ovmf+set}" = set; then :
@@ -4464,6 +4469,11 @@ fi
+cat >>confdefs.h <<_ACEOF
+#define OVMF_PATH "${ovmf_path:-$XENFIRMWAREDIR/ovmf.bin}"
+_ACEOF
+
+
# Check whether --with-extra-qemuu-configure-args was given.
if test "${with_extra_qemuu_configure_args+set}" = set; then :
diff --git a/tools/configure.ac b/tools/configure.ac
index 3a4abb5..ed10902 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -221,6 +221,9 @@ AC_ARG_WITH([system-seabios],
esac
],[])
AC_SUBST(seabios_path)
+AC_DEFINE_UNQUOTED([SEABIOS_PATH],
+ ["${seabios_path:-$XENFIRMWAREDIR/bios.bin}"],
+ [SeaBIOS path])
AC_ARG_WITH([system-ovmf],
AS_HELP_STRING([--with-system-ovmf@<:@=PATH@:>@],
@@ -232,6 +235,9 @@ AC_ARG_WITH([system-ovmf],
esac
],[])
AC_SUBST(ovmf_path)
+AC_DEFINE_UNQUOTED([OVMF_PATH],
+ ["${ovmf_path:-$XENFIRMWAREDIR/ovmf.bin}"],
+ [OVMF path])
AC_ARG_WITH([extra-qemuu-configure-args],
AS_HELP_STRING([--with-extra-qemuu-configure-args@<:@="--ARG1 ..."@:>@],
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v8 04/13] firmware/makefile: install BIOS blob ...
2016-08-18 14:13 [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
` (2 preceding siblings ...)
2016-08-18 14:13 ` [PATCH v8 03/13] configure: #define SEABIOS_PATH and OVMF_PATH Wei Liu
@ 2016-08-18 14:13 ` Wei Liu
2016-08-18 14:13 ` [PATCH v8 05/13] libxl: Load guest BIOS from file Wei Liu
` (9 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-18 14:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
... into the firmware directory, along with hvmloader.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
No change in V6.
- acked
Change in V5:
- remove use of "variable" for SEABIOS_ROM and OVMF_ROM location
there are static location
- install seabios as bios.bin instead of seabios.bin
Change in V4:
- remove install of acpi dsdt table
Change in V3:
- do not check if ROMs file exist before installing, they should exist
- change rules for dsdt_anycpu_qemu_xen.c in oder to generate both .c and
.aml files without changing temporarly the other dsdt_*.c rules.
---
tools/firmware/Makefile | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile
index 6cc86ce..82b1f6b 100644
--- a/tools/firmware/Makefile
+++ b/tools/firmware/Makefile
@@ -45,6 +45,16 @@ endif
install: all
[ -d $(INST_DIR) ] || $(INSTALL_DIR) $(INST_DIR)
[ ! -e $(TARGET) ] || $(INSTALL_DATA) $(TARGET) $(INST_DIR)
+ifeq ($(CONFIG_SEABIOS),y)
+ifeq ($(SEABIOS_PATH),)
+ $(INSTALL_DATA) seabios-dir/out/bios.bin $(INST_DIR)/bios.bin
+endif
+endif
+ifeq ($(CONFIG_OVMF),y)
+ifeq ($(OVMF_PATH),)
+ $(INSTALL_DATA) ovmf-dir/ovmf.bin $(INST_DIR)/ovmf.bin
+endif
+endif
.PHONY: clean
clean: subdirs-clean
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v8 05/13] libxl: Load guest BIOS from file
2016-08-18 14:13 [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
` (3 preceding siblings ...)
2016-08-18 14:13 ` [PATCH v8 04/13] firmware/makefile: install BIOS blob Wei Liu
@ 2016-08-18 14:13 ` Wei Liu
2016-08-19 14:43 ` Andrew Cooper
2016-08-23 20:00 ` Doug Goldstein
2016-08-18 14:13 ` [PATCH v8 06/13] hvmloader: Grab the hvm_start_info pointer Wei Liu
` (8 subsequent siblings)
13 siblings, 2 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-18 14:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
The path to the BIOS blob can be overriden by the xl's
bios_path_override option, or provided by u.hvm.bios_firmware in the
domain_build_info struct by other libxl user.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Changes in V6:
- use goto for error handling of libxl__load_hvm_firmware_module()
Changes in V5:
- man page, use B<> to highlight config option in description.
- rename config option from `bios_override` to `bios_path_override`
- store libxl_read_file_contents() return value into r instead of e
(just renamed the variable)
- rename domain_build_info.u.hvm.bios_firmware to system_firmware
Changes in V4:
- updating man page to have bios_override described.
- return ERROR_INVAL in libxl__load_hvm_firmware_module when the file is
empty.
Changes in V3:
- move seabios_path and ovmf_path to libxl_path.c (with renaming)
- fix some coding style
- warn for empty file
- remove rombios stuff (will still be built-in hvmloader)
- rename field bios_filename in domain_build_info to bios_firmware to
follow naming of acpi and smbios.
- log an error after libxl_read_file_contents() only when it return ENOENT
- return an error on empty file.
- added #define LIBXL_HAVE_BUILDINFO_HVM_BIOS_FIRMWARE
---
docs/man/xl.cfg.pod.5.in | 9 +++++++
tools/libxl/libxl.h | 8 ++++++
tools/libxl/libxl_dom.c | 61 ++++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 2 ++
tools/libxl/libxl_paths.c | 10 ++++++++
tools/libxl/libxl_types.idl | 1 +
tools/libxl/xl_cmdimpl.c | 11 +++++---
7 files changed, 99 insertions(+), 3 deletions(-)
diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index 48c9c0d..6feee52 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -1214,6 +1214,15 @@ Requires device_model_version=qemu-xen.
=back
+=item B<bios_path_override="PATH">
+
+Override the path to the blob to be used as BIOS. The blob provided here MUST
+be consistent with the B<bios=> which you have specified. You should not
+normally need to specify this option.
+
+This options does not have any effect if using B<bios="rombios"> or
+B<device_model_version="qemu-xen-traditional">.
+
=item B<pae=BOOLEAN>
Hide or expose the IA32 Physical Address Extensions. These extensions
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index ae21302..e4c25c4 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -950,6 +950,14 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
#define LIBXL_HAVE_CHECKPOINTED_STREAM 1
/*
+ * LIBXL_HAVE_BUILDINFO_HVM_SYSTEM_FIRMWARE
+ *
+ * libxl_domain_build_info has u.hvm.system_firmware field which can be use
+ * to provide a different firmware blob (like SeaBIOS or OVMF).
+ */
+#define LIBXL_HAVE_BUILDINFO_HVM_SYSTEM_FIRMWARE
+
+/*
* ERROR_REMUS_XXX error code only exists from Xen 4.5, Xen 4.6 and it
* is changed to ERROR_CHECKPOINT_XXX in Xen 4.7
*/
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index eef5045..c895649 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -862,6 +862,42 @@ err:
return ret;
}
+static int libxl__load_hvm_firmware_module(libxl__gc *gc,
+ const char *filename,
+ const char *what,
+ struct xc_hvm_firmware_module *m)
+{
+ int datalen = 0;
+ void *data = NULL;
+ int r, rc;
+
+ LOG(DEBUG, "Loading %s: %s", what, filename);
+ r = libxl_read_file_contents(CTX, filename, &data, &datalen);
+ if (r) {
+ /*
+ * Print a message only on ENOENT, other errors are logged by the
+ * function libxl_read_file_contents().
+ */
+ if (r == ENOENT)
+ LOGEV(ERROR, r, "failed to read %s file", what);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ libxl__ptr_add(gc, data);
+ if (datalen) {
+ /* Only accept non-empty files */
+ m->data = data;
+ m->length = datalen;
+ } else {
+ LOG(ERROR, "file %s for %s is empty", filename, what);
+ rc = ERROR_INVAL;
+ goto out;
+ }
+ rc = 0;
+out:
+ return rc;
+}
+
static int libxl__domain_firmware(libxl__gc *gc,
libxl_domain_build_info *info,
struct xc_dom_image *dom)
@@ -871,6 +907,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
int e, rc;
int datalen = 0;
void *data;
+ const char *bios_filename = NULL;
if (info->u.hvm.firmware)
firmware = info->u.hvm.firmware;
@@ -914,6 +951,30 @@ static int libxl__domain_firmware(libxl__gc *gc,
goto out;
}
+ if (info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
+ if (info->u.hvm.system_firmware) {
+ bios_filename = info->u.hvm.system_firmware;
+ } else {
+ switch (info->u.hvm.bios) {
+ case LIBXL_BIOS_TYPE_SEABIOS:
+ bios_filename = libxl__seabios_path();
+ break;
+ case LIBXL_BIOS_TYPE_OVMF:
+ bios_filename = libxl__ovmf_path();
+ break;
+ case LIBXL_BIOS_TYPE_ROMBIOS:
+ default:
+ abort();
+ }
+ }
+ }
+
+ if (bios_filename) {
+ rc = libxl__load_hvm_firmware_module(gc, bios_filename, "BIOS",
+ &dom->system_firmware_module);
+ if (rc) goto out;
+ }
+
if (info->u.hvm.smbios_firmware) {
data = NULL;
e = libxl_read_file_contents(ctx, info->u.hvm.smbios_firmware,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 1222ffa..ce8e17a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2318,6 +2318,8 @@ _hidden const char *libxl__xen_config_dir_path(void);
_hidden const char *libxl__xen_script_dir_path(void);
_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);
/*----- subprocess execution with timeout -----*/
diff --git a/tools/libxl/libxl_paths.c b/tools/libxl/libxl_paths.c
index 9b7b0d5..6972b90 100644
--- a/tools/libxl/libxl_paths.c
+++ b/tools/libxl/libxl_paths.c
@@ -35,6 +35,16 @@ const char *libxl__run_dir_path(void)
return XEN_RUN_DIR;
}
+const char *libxl__seabios_path(void)
+{
+ return SEABIOS_PATH;
+}
+
+const char *libxl__ovmf_path(void)
+{
+ return OVMF_PATH;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index ef614be..98bfc3a 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -513,6 +513,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
("timer_mode", libxl_timer_mode),
("nested_hvm", libxl_defbool),
("altp2m", libxl_defbool),
+ ("system_firmware", string),
("smbios_firmware", string),
("acpi_firmware", string),
("hdtype", libxl_hdtype),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 7f961e3..7540fb1 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1565,12 +1565,17 @@ static void parse_config_data(const char *config_source,
xlu_cfg_replace_string (config, "firmware_override",
&b_info->u.hvm.firmware, 0);
- if (!xlu_cfg_get_string(config, "bios", &buf, 0) &&
- libxl_bios_type_from_string(buf, &b_info->u.hvm.bios)) {
+ xlu_cfg_replace_string (config, "bios_path_override",
+ &b_info->u.hvm.system_firmware, 0);
+ if (!xlu_cfg_get_string(config, "bios", &buf, 0)) {
+ if (libxl_bios_type_from_string(buf, &b_info->u.hvm.bios)) {
fprintf(stderr, "ERROR: invalid value \"%s\" for \"bios\"\n",
buf);
exit (1);
- }
+ }
+ } else if (b_info->u.hvm.system_firmware)
+ fprintf(stderr, "WARNING: "
+ "bios_path_override given without specific bios name\n");
xlu_cfg_get_defbool(config, "pae", &b_info->u.hvm.pae, 0);
xlu_cfg_get_defbool(config, "apic", &b_info->u.hvm.apic, 0);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v8 05/13] libxl: Load guest BIOS from file
2016-08-18 14:13 ` [PATCH v8 05/13] libxl: Load guest BIOS from file Wei Liu
@ 2016-08-19 14:43 ` Andrew Cooper
2016-08-22 13:13 ` Wei Liu
2016-08-23 20:00 ` Doug Goldstein
1 sibling, 1 reply; 32+ messages in thread
From: Andrew Cooper @ 2016-08-19 14:43 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Anthony PERARD
On 18/08/16 15:13, Wei Liu wrote:
> From: Anthony PERARD <anthony.perard@citrix.com>
>
> The path to the BIOS blob can be overriden by the xl's
> bios_path_override option, or provided by u.hvm.bios_firmware in the
> domain_build_info struct by other libxl user.
>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
This introduces a regression, but I am not sure how best to fix it.
[root@xrtuk-09-12 xen-test-framework]# xl -vvv create -p
tests/selftest/test-hvm32-selftest.cfg
Parsing config from tests/selftest/test-hvm32-selftest.cfg
libxl: debug: libxl_create.c:1603:do_domain_create: ao 0xa6b9f0: create:
how=(nil) callback=(nil) poller=0xa6c120
libxl: debug: libxl_create.c:959:initiate_domain_create: running bootloader
libxl: debug: libxl_bootloader.c:324:libxl__bootloader_run: not a PV
domain, skipping bootloader
libxl: debug: libxl_event.c:686:libxl__ev_xswatch_deregister: watch
w=0xa6cc30: deregister unregistered
libxl: debug: libxl_numa.c:483:libxl__get_numa_candidate: New best NUMA
placement candidate found: nr_nodes=1, nr_cpus=12, nr_vcpus=17,
free_memkb=30611
libxl: detail: libxl_dom.c:182:numa_place_domain: NUMA placement
candidate with 1 nodes, 12 cpus and 30611 KB free selected
domainbuilder: detail: xc_dom_allocate: cmdline="(null)", features="(null)"
domainbuilder: detail: xc_dom_kernel_file:
filename="/opt/xen-test-framework/tests/selftest/test-hvm32-selftest"
domainbuilder: detail: xc_dom_malloc_filemap : 1090 kB
libxl: debug: libxl_dom.c:874:libxl__load_hvm_firmware_module: Loading
BIOS: /usr/libexec/xen/boot/bios.bin
libxl: error: libxl_dom.c:882:libxl__load_hvm_firmware_module: failed to
read BIOS file: No such file or directory
In this case, tools have been built with ./configure --disable-seabios
As a result, /usr/libexec/xen/boot/bios.bin (name altered a patch sent
separately) isn't built or installed.
I think libxl needs to logic to determine which firmware to use based on
the available CONFIG_* options it was built with.
> @@ -914,6 +951,30 @@ static int libxl__domain_firmware(libxl__gc *gc,
> goto out;
> }
>
> + if (info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> + if (info->u.hvm.system_firmware) {
> + bios_filename = info->u.hvm.system_firmware;
> + } else {
> + switch (info->u.hvm.bios) {
> + case LIBXL_BIOS_TYPE_SEABIOS:
> + bios_filename = libxl__seabios_path();
> + break;
> + case LIBXL_BIOS_TYPE_OVMF:
> + bios_filename = libxl__ovmf_path();
> + break;
At the very least, these need to be guarded by #ifdef
CONFIG_{SEABIOS,OVMF}, as it is explicitly permitted for them not to be
present in a build.
> + case LIBXL_BIOS_TYPE_ROMBIOS:
ROMBIOS certainly does function correctly with QEMU_XEN, and is how
XenServer is planning to start the migration from a qemu-trad world to a
qemu-upstream world. Even if libxl doesn't want to formally support
such a configuration, it shouldn't be excluded.
> + default:
> + abort();
This is completely antisocial. Under no circumstances is it ok for a
library to call abort(); fail an assertion if necessary, but this is a
configuration error and should pass an error back to its caller, not
take the entire process with it.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v8 05/13] libxl: Load guest BIOS from file
2016-08-19 14:43 ` Andrew Cooper
@ 2016-08-22 13:13 ` Wei Liu
2016-08-22 13:26 ` Andrew Cooper
0 siblings, 1 reply; 32+ messages in thread
From: Wei Liu @ 2016-08-22 13:13 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Anthony PERARD, Xen-devel, Wei Liu
On Fri, Aug 19, 2016 at 03:43:00PM +0100, Andrew Cooper wrote:
> On 18/08/16 15:13, Wei Liu wrote:
> > From: Anthony PERARD <anthony.perard@citrix.com>
> >
> > The path to the BIOS blob can be overriden by the xl's
> > bios_path_override option, or provided by u.hvm.bios_firmware in the
> > domain_build_info struct by other libxl user.
> >
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
>
> This introduces a regression, but I am not sure how best to fix it.
>
> [root@xrtuk-09-12 xen-test-framework]# xl -vvv create -p
> tests/selftest/test-hvm32-selftest.cfg
> Parsing config from tests/selftest/test-hvm32-selftest.cfg
> libxl: debug: libxl_create.c:1603:do_domain_create: ao 0xa6b9f0: create:
> how=(nil) callback=(nil) poller=0xa6c120
> libxl: debug: libxl_create.c:959:initiate_domain_create: running bootloader
> libxl: debug: libxl_bootloader.c:324:libxl__bootloader_run: not a PV
> domain, skipping bootloader
> libxl: debug: libxl_event.c:686:libxl__ev_xswatch_deregister: watch
> w=0xa6cc30: deregister unregistered
> libxl: debug: libxl_numa.c:483:libxl__get_numa_candidate: New best NUMA
> placement candidate found: nr_nodes=1, nr_cpus=12, nr_vcpus=17,
> free_memkb=30611
> libxl: detail: libxl_dom.c:182:numa_place_domain: NUMA placement
> candidate with 1 nodes, 12 cpus and 30611 KB free selected
> domainbuilder: detail: xc_dom_allocate: cmdline="(null)", features="(null)"
> domainbuilder: detail: xc_dom_kernel_file:
> filename="/opt/xen-test-framework/tests/selftest/test-hvm32-selftest"
> domainbuilder: detail: xc_dom_malloc_filemap : 1090 kB
> libxl: debug: libxl_dom.c:874:libxl__load_hvm_firmware_module: Loading
> BIOS: /usr/libexec/xen/boot/bios.bin
> libxl: error: libxl_dom.c:882:libxl__load_hvm_firmware_module: failed to
> read BIOS file: No such file or directory
>
> In this case, tools have been built with ./configure --disable-seabios
>
> As a result, /usr/libexec/xen/boot/bios.bin (name altered a patch sent
> separately) isn't built or installed.
>
> I think libxl needs to logic to determine which firmware to use based on
> the available CONFIG_* options it was built with.
I don' quite follow here.
Do you mean if user hasn't specified any bios= option, (s)he gets
whatever available?
I think we should stick with the seabios-default behaviour to avoid
surprising breakage.
If you don't want any bois, maybe we should provide a bios=none option?
>
> > @@ -914,6 +951,30 @@ static int libxl__domain_firmware(libxl__gc *gc,
> > goto out;
> > }
> >
> > + if (info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> > + if (info->u.hvm.system_firmware) {
> > + bios_filename = info->u.hvm.system_firmware;
> > + } else {
> > + switch (info->u.hvm.bios) {
> > + case LIBXL_BIOS_TYPE_SEABIOS:
> > + bios_filename = libxl__seabios_path();
> > + break;
> > + case LIBXL_BIOS_TYPE_OVMF:
> > + bios_filename = libxl__ovmf_path();
> > + break;
>
> At the very least, these need to be guarded by #ifdef
> CONFIG_{SEABIOS,OVMF}, as it is explicitly permitted for them not to be
> present in a build.
>
> > + case LIBXL_BIOS_TYPE_ROMBIOS:
>
> ROMBIOS certainly does function correctly with QEMU_XEN, and is how
> XenServer is planning to start the migration from a qemu-trad world to a
> qemu-upstream world. Even if libxl doesn't want to formally support
> such a configuration, it shouldn't be excluded.
>
There is no written statement, but I would rather not support this
configuration.
I expect this is an impossible situation to get into, since verification
should have been done a few steps before -- hence the abort(3) here is
justified. But I would need to double-check if that's not the case and
will do something about it either here or at the place I see
appropriate.
> > + default:
> > + abort();
>
> This is completely antisocial. Under no circumstances is it ok for a
> library to call abort(); fail an assertion if necessary, but this is a
> configuration error and should pass an error back to its caller, not
> take the entire process with it.
>
In general it is ok to call abort(3) in an internal function that only
expects valid input. And I don't see how switching to assert(3) help in
those cases, that ends up calling abort(3) anyway.
Wei.
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v8 05/13] libxl: Load guest BIOS from file
2016-08-22 13:13 ` Wei Liu
@ 2016-08-22 13:26 ` Andrew Cooper
2016-08-22 14:26 ` Wei Liu
0 siblings, 1 reply; 32+ messages in thread
From: Andrew Cooper @ 2016-08-22 13:26 UTC (permalink / raw)
To: Wei Liu; +Cc: Anthony PERARD, Xen-devel
On 22/08/16 14:13, Wei Liu wrote:
> On Fri, Aug 19, 2016 at 03:43:00PM +0100, Andrew Cooper wrote:
>> On 18/08/16 15:13, Wei Liu wrote:
>>> From: Anthony PERARD <anthony.perard@citrix.com>
>>>
>>> The path to the BIOS blob can be overriden by the xl's
>>> bios_path_override option, or provided by u.hvm.bios_firmware in the
>>> domain_build_info struct by other libxl user.
>>>
>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>> Acked-by: Wei Liu <wei.liu2@citrix.com>
>> This introduces a regression, but I am not sure how best to fix it.
>>
>> [root@xrtuk-09-12 xen-test-framework]# xl -vvv create -p
>> tests/selftest/test-hvm32-selftest.cfg
>> Parsing config from tests/selftest/test-hvm32-selftest.cfg
>> libxl: debug: libxl_create.c:1603:do_domain_create: ao 0xa6b9f0: create:
>> how=(nil) callback=(nil) poller=0xa6c120
>> libxl: debug: libxl_create.c:959:initiate_domain_create: running bootloader
>> libxl: debug: libxl_bootloader.c:324:libxl__bootloader_run: not a PV
>> domain, skipping bootloader
>> libxl: debug: libxl_event.c:686:libxl__ev_xswatch_deregister: watch
>> w=0xa6cc30: deregister unregistered
>> libxl: debug: libxl_numa.c:483:libxl__get_numa_candidate: New best NUMA
>> placement candidate found: nr_nodes=1, nr_cpus=12, nr_vcpus=17,
>> free_memkb=30611
>> libxl: detail: libxl_dom.c:182:numa_place_domain: NUMA placement
>> candidate with 1 nodes, 12 cpus and 30611 KB free selected
>> domainbuilder: detail: xc_dom_allocate: cmdline="(null)", features="(null)"
>> domainbuilder: detail: xc_dom_kernel_file:
>> filename="/opt/xen-test-framework/tests/selftest/test-hvm32-selftest"
>> domainbuilder: detail: xc_dom_malloc_filemap : 1090 kB
>> libxl: debug: libxl_dom.c:874:libxl__load_hvm_firmware_module: Loading
>> BIOS: /usr/libexec/xen/boot/bios.bin
>> libxl: error: libxl_dom.c:882:libxl__load_hvm_firmware_module: failed to
>> read BIOS file: No such file or directory
>>
>> In this case, tools have been built with ./configure --disable-seabios
>>
>> As a result, /usr/libexec/xen/boot/bios.bin (name altered a patch sent
>> separately) isn't built or installed.
>>
>> I think libxl needs to logic to determine which firmware to use based on
>> the available CONFIG_* options it was built with.
> I don' quite follow here.
>
> Do you mean if user hasn't specified any bios= option, (s)he gets
> whatever available?
>
> I think we should stick with the seabios-default behaviour to avoid
> surprising breakage.
>
> If you don't want any bois, maybe we should provide a bios=none option?
XenServer is built with ./configure --disable-seabios because we don't
use it (yet). This means that SeaBIOS is not built, installed, or
available.
Because of this change, libxl unconditionally assumes the presence of
SeaBIOS, and tries to use the installed seabios binary.
>
>>> @@ -914,6 +951,30 @@ static int libxl__domain_firmware(libxl__gc *gc,
>>> goto out;
>>> }
>>>
>>> + if (info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
>>> + if (info->u.hvm.system_firmware) {
>>> + bios_filename = info->u.hvm.system_firmware;
>>> + } else {
>>> + switch (info->u.hvm.bios) {
>>> + case LIBXL_BIOS_TYPE_SEABIOS:
>>> + bios_filename = libxl__seabios_path();
>>> + break;
>>> + case LIBXL_BIOS_TYPE_OVMF:
>>> + bios_filename = libxl__ovmf_path();
>>> + break;
>> At the very least, these need to be guarded by #ifdef
>> CONFIG_{SEABIOS,OVMF}, as it is explicitly permitted for them not to be
>> present in a build.
>>
>>> + case LIBXL_BIOS_TYPE_ROMBIOS:
>> ROMBIOS certainly does function correctly with QEMU_XEN, and is how
>> XenServer is planning to start the migration from a qemu-trad world to a
>> qemu-upstream world. Even if libxl doesn't want to formally support
>> such a configuration, it shouldn't be excluded.
>>
> There is no written statement, but I would rather not support this
> configuration.
Rightly or wrongly, it is already available, usable and working (until
this changeset), via supported configuration options.
> I expect this is an impossible situation to get into, since verification
> should have been done a few steps before -- hence the abort(3) here is
> justified. But I would need to double-check if that's not the case and
> will do something about it either here or at the place I see
> appropriate.
>
>>> + default:
>>> + abort();
>> This is completely antisocial. Under no circumstances is it ok for a
>> library to call abort(); fail an assertion if necessary, but this is a
>> configuration error and should pass an error back to its caller, not
>> take the entire process with it.
>>
> In general it is ok to call abort(3) in an internal function that only
> expects valid input.
No. It very much is not.
> And I don't see how switching to assert(3) help in
> those cases, that ends up calling abort(3) anyway.
The difference is some details of the problem going out on stderr.
abort() causes the process to cease to exist without any trace.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v8 05/13] libxl: Load guest BIOS from file
2016-08-22 13:26 ` Andrew Cooper
@ 2016-08-22 14:26 ` Wei Liu
2016-08-22 15:05 ` [PATCH 0/2] Fix issue with {OVMF,SEABIOS}_PATH Wei Liu
2016-08-22 15:09 ` [PATCH v8 05/13] libxl: Load guest BIOS from file Andrew Cooper
0 siblings, 2 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-22 14:26 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Anthony PERARD, Xen-devel, Wei Liu
On Mon, Aug 22, 2016 at 02:26:11PM +0100, Andrew Cooper wrote:
> On 22/08/16 14:13, Wei Liu wrote:
> > On Fri, Aug 19, 2016 at 03:43:00PM +0100, Andrew Cooper wrote:
> >> On 18/08/16 15:13, Wei Liu wrote:
> >>> From: Anthony PERARD <anthony.perard@citrix.com>
> >>>
> >>> The path to the BIOS blob can be overriden by the xl's
> >>> bios_path_override option, or provided by u.hvm.bios_firmware in the
> >>> domain_build_info struct by other libxl user.
> >>>
> >>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >>> Acked-by: Wei Liu <wei.liu2@citrix.com>
> >> This introduces a regression, but I am not sure how best to fix it.
> >>
> >> [root@xrtuk-09-12 xen-test-framework]# xl -vvv create -p
> >> tests/selftest/test-hvm32-selftest.cfg
> >> Parsing config from tests/selftest/test-hvm32-selftest.cfg
> >> libxl: debug: libxl_create.c:1603:do_domain_create: ao 0xa6b9f0: create:
> >> how=(nil) callback=(nil) poller=0xa6c120
> >> libxl: debug: libxl_create.c:959:initiate_domain_create: running bootloader
> >> libxl: debug: libxl_bootloader.c:324:libxl__bootloader_run: not a PV
> >> domain, skipping bootloader
> >> libxl: debug: libxl_event.c:686:libxl__ev_xswatch_deregister: watch
> >> w=0xa6cc30: deregister unregistered
> >> libxl: debug: libxl_numa.c:483:libxl__get_numa_candidate: New best NUMA
> >> placement candidate found: nr_nodes=1, nr_cpus=12, nr_vcpus=17,
> >> free_memkb=30611
> >> libxl: detail: libxl_dom.c:182:numa_place_domain: NUMA placement
> >> candidate with 1 nodes, 12 cpus and 30611 KB free selected
> >> domainbuilder: detail: xc_dom_allocate: cmdline="(null)", features="(null)"
> >> domainbuilder: detail: xc_dom_kernel_file:
> >> filename="/opt/xen-test-framework/tests/selftest/test-hvm32-selftest"
> >> domainbuilder: detail: xc_dom_malloc_filemap : 1090 kB
> >> libxl: debug: libxl_dom.c:874:libxl__load_hvm_firmware_module: Loading
> >> BIOS: /usr/libexec/xen/boot/bios.bin
> >> libxl: error: libxl_dom.c:882:libxl__load_hvm_firmware_module: failed to
> >> read BIOS file: No such file or directory
> >>
> >> In this case, tools have been built with ./configure --disable-seabios
> >>
> >> As a result, /usr/libexec/xen/boot/bios.bin (name altered a patch sent
> >> separately) isn't built or installed.
> >>
> >> I think libxl needs to logic to determine which firmware to use based on
> >> the available CONFIG_* options it was built with.
> > I don' quite follow here.
> >
> > Do you mean if user hasn't specified any bios= option, (s)he gets
> > whatever available?
> >
> > I think we should stick with the seabios-default behaviour to avoid
> > surprising breakage.
> >
> > If you don't want any bois, maybe we should provide a bios=none option?
>
> XenServer is built with ./configure --disable-seabios because we don't
> use it (yet). This means that SeaBIOS is not built, installed, or
> available.
>
> Because of this change, libxl unconditionally assumes the presence of
> SeaBIOS, and tries to use the installed seabios binary.
Right, this needs fixing.
We can restore the behaviour in libxl -- if you specify a not available
bios, libxl won't complain, hvmloader will crash in the same way as
before.
>
> >
> >>> @@ -914,6 +951,30 @@ static int libxl__domain_firmware(libxl__gc *gc,
> >>> goto out;
> >>> }
> >>>
> >>> + if (info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> >>> + if (info->u.hvm.system_firmware) {
> >>> + bios_filename = info->u.hvm.system_firmware;
> >>> + } else {
> >>> + switch (info->u.hvm.bios) {
> >>> + case LIBXL_BIOS_TYPE_SEABIOS:
> >>> + bios_filename = libxl__seabios_path();
> >>> + break;
> >>> + case LIBXL_BIOS_TYPE_OVMF:
> >>> + bios_filename = libxl__ovmf_path();
> >>> + break;
> >> At the very least, these need to be guarded by #ifdef
> >> CONFIG_{SEABIOS,OVMF}, as it is explicitly permitted for them not to be
> >> present in a build.
> >>
> >>> + case LIBXL_BIOS_TYPE_ROMBIOS:
> >> ROMBIOS certainly does function correctly with QEMU_XEN, and is how
> >> XenServer is planning to start the migration from a qemu-trad world to a
> >> qemu-upstream world. Even if libxl doesn't want to formally support
> >> such a configuration, it shouldn't be excluded.
> >>
> > There is no written statement, but I would rather not support this
> > configuration.
>
> Rightly or wrongly, it is already available, usable and working (until
> this changeset), via supported configuration options.
>
Ian, do you have more insight on whether that is supported?
> > I expect this is an impossible situation to get into, since verification
> > should have been done a few steps before -- hence the abort(3) here is
> > justified. But I would need to double-check if that's not the case and
> > will do something about it either here or at the place I see
> > appropriate.
> >
> >>> + default:
> >>> + abort();
> >> This is completely antisocial. Under no circumstances is it ok for a
> >> library to call abort(); fail an assertion if necessary, but this is a
> >> configuration error and should pass an error back to its caller, not
> >> take the entire process with it.
> >>
> > In general it is ok to call abort(3) in an internal function that only
> > expects valid input.
>
> No. It very much is not.
>
> > And I don't see how switching to assert(3) help in
> > those cases, that ends up calling abort(3) anyway.
>
> The difference is some details of the problem going out on stderr.
> abort() causes the process to cease to exist without any trace.
>
I forgot to mention that assert(3) will generate no code in non-debug
build. There is no point to continue the program, really.
Anyway, libxl is already strewn with abort(3). If you're interested in a
assert(3) vs abort(3) debate, please start a new thread.
Wei.
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 0/2] Fix issue with {OVMF,SEABIOS}_PATH
2016-08-22 14:26 ` Wei Liu
@ 2016-08-22 15:05 ` Wei Liu
2016-08-22 15:05 ` [PATCH 1/2] tools: only define {OVMF, SEABIOS}_PATH when they are enabled Wei Liu
` (3 more replies)
2016-08-22 15:09 ` [PATCH v8 05/13] libxl: Load guest BIOS from file Andrew Cooper
1 sibling, 4 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-22 15:05 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Andrew Cooper, Ian Jackson, Wei Liu
They shouldn't be available when the respective BIOS is disabled at build time.
This should fix the issus that causes xtf fail to launch hvm guests.
Wei Liu (2):
tools: only define {OVMF,SEABIOS}_PATH when they are enabled
libxl: only return {OVMF,SEABIOS}_PATH if available
tools/configure | 8 ++++++++
tools/configure.ac | 16 ++++++++++------
tools/libxl/libxl_paths.c | 8 ++++++++
3 files changed, 26 insertions(+), 6 deletions(-)
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 1/2] tools: only define {OVMF, SEABIOS}_PATH when they are enabled
2016-08-22 15:05 ` [PATCH 0/2] Fix issue with {OVMF,SEABIOS}_PATH Wei Liu
@ 2016-08-22 15:05 ` Wei Liu
2016-08-22 15:05 ` [PATCH 2/2] libxl: only return {OVMF, SEABIOS}_PATH if available Wei Liu
` (2 subsequent siblings)
3 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-22 15:05 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Andrew Cooper, Ian Jackson, Wei Liu
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Rerun autogen.sh
---
tools/configure | 8 ++++++++
tools/configure.ac | 16 ++++++++++------
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/tools/configure b/tools/configure
index 81ccf34..998090a 100755
--- a/tools/configure
+++ b/tools/configure
@@ -4449,12 +4449,16 @@ if test "${with_system_seabios+set}" = set; then :
fi
+if test "x$seabios" = "xy"; then :
+
cat >>confdefs.h <<_ACEOF
#define SEABIOS_PATH "${seabios_path:-$XENFIRMWAREDIR/bios.bin}"
_ACEOF
+fi
+
# Check whether --with-system-ovmf was given.
if test "${with_system_ovmf+set}" = set; then :
@@ -4468,12 +4472,16 @@ if test "${with_system_ovmf+set}" = set; then :
fi
+if test "x$ovmf" = "xy"; then :
+
cat >>confdefs.h <<_ACEOF
#define OVMF_PATH "${ovmf_path:-$XENFIRMWAREDIR/ovmf.bin}"
_ACEOF
+fi
+
# Check whether --with-extra-qemuu-configure-args was given.
if test "${with_extra_qemuu_configure_args+set}" = set; then :
diff --git a/tools/configure.ac b/tools/configure.ac
index c12ad79..1fb4a55 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -222,9 +222,11 @@ AC_ARG_WITH([system-seabios],
*) seabios_path=$withval ;;
esac
],[])
-AC_DEFINE_UNQUOTED([SEABIOS_PATH],
- ["${seabios_path:-$XENFIRMWAREDIR/bios.bin}"],
- [SeaBIOS path])
+AS_IF([test "x$seabios" = "xy"], [
+ AC_DEFINE_UNQUOTED([SEABIOS_PATH],
+ ["${seabios_path:-$XENFIRMWAREDIR/bios.bin}"],
+ [SeaBIOS path])
+])
AC_ARG_WITH([system-ovmf],
AS_HELP_STRING([--with-system-ovmf@<:@=PATH@:>@],
@@ -237,9 +239,11 @@ AC_ARG_WITH([system-ovmf],
*) ovmf_path=$withval ;;
esac
],[])
-AC_DEFINE_UNQUOTED([OVMF_PATH],
- ["${ovmf_path:-$XENFIRMWAREDIR/ovmf.bin}"],
- [OVMF path])
+AS_IF([test "x$ovmf" = "xy"], [
+ AC_DEFINE_UNQUOTED([OVMF_PATH],
+ ["${ovmf_path:-$XENFIRMWAREDIR/ovmf.bin}"],
+ [OVMF path])
+])
AC_ARG_WITH([extra-qemuu-configure-args],
AS_HELP_STRING([--with-extra-qemuu-configure-args@<:@="--ARG1 ..."@:>@],
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 2/2] libxl: only return {OVMF, SEABIOS}_PATH if available
2016-08-22 15:05 ` [PATCH 0/2] Fix issue with {OVMF,SEABIOS}_PATH Wei Liu
2016-08-22 15:05 ` [PATCH 1/2] tools: only define {OVMF, SEABIOS}_PATH when they are enabled Wei Liu
@ 2016-08-22 15:05 ` Wei Liu
2016-08-23 9:37 ` [PATCH 0/2] Fix issue with {OVMF,SEABIOS}_PATH Andrew Cooper
2016-08-24 11:38 ` Ian Jackson
3 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-22 15:05 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Andrew Cooper, Ian Jackson, Wei Liu
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
tools/libxl/libxl_paths.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/tools/libxl/libxl_paths.c b/tools/libxl/libxl_paths.c
index 6972b90..0643c1b 100644
--- a/tools/libxl/libxl_paths.c
+++ b/tools/libxl/libxl_paths.c
@@ -37,12 +37,20 @@ const char *libxl__run_dir_path(void)
const char *libxl__seabios_path(void)
{
+#ifdef SEABIOS_PATH
return SEABIOS_PATH;
+#else
+ return NULL;
+#endif
}
const char *libxl__ovmf_path(void)
{
+#ifdef OVMF_PATH
return OVMF_PATH;
+#else
+ return NULL;
+#endif
}
/*
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 0/2] Fix issue with {OVMF,SEABIOS}_PATH
2016-08-22 15:05 ` [PATCH 0/2] Fix issue with {OVMF,SEABIOS}_PATH Wei Liu
2016-08-22 15:05 ` [PATCH 1/2] tools: only define {OVMF, SEABIOS}_PATH when they are enabled Wei Liu
2016-08-22 15:05 ` [PATCH 2/2] libxl: only return {OVMF, SEABIOS}_PATH if available Wei Liu
@ 2016-08-23 9:37 ` Andrew Cooper
2016-08-24 11:38 ` Ian Jackson
3 siblings, 0 replies; 32+ messages in thread
From: Andrew Cooper @ 2016-08-23 9:37 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Anthony PERARD, Ian Jackson
On 22/08/16 16:05, Wei Liu wrote:
> They shouldn't be available when the respective BIOS is disabled at build time.
>
> This should fix the issus that causes xtf fail to launch hvm guests.
>
> Wei Liu (2):
> tools: only define {OVMF,SEABIOS}_PATH when they are enabled
> libxl: only return {OVMF,SEABIOS}_PATH if available
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 0/2] Fix issue with {OVMF,SEABIOS}_PATH
2016-08-22 15:05 ` [PATCH 0/2] Fix issue with {OVMF,SEABIOS}_PATH Wei Liu
` (2 preceding siblings ...)
2016-08-23 9:37 ` [PATCH 0/2] Fix issue with {OVMF,SEABIOS}_PATH Andrew Cooper
@ 2016-08-24 11:38 ` Ian Jackson
2016-08-24 11:59 ` Wei Liu
3 siblings, 1 reply; 32+ messages in thread
From: Ian Jackson @ 2016-08-24 11:38 UTC (permalink / raw)
To: Wei Liu; +Cc: Anthony PERARD, Xen-devel, Andrew Cooper
Wei Liu writes ("[PATCH 0/2] Fix issue with {OVMF,SEABIOS}_PATH"):
> They shouldn't be available when the respective BIOS is disabled at build time.
>
> This should fix the issus that causes xtf fail to launch hvm guests.
Both
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 0/2] Fix issue with {OVMF,SEABIOS}_PATH
2016-08-24 11:38 ` Ian Jackson
@ 2016-08-24 11:59 ` Wei Liu
0 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-24 11:59 UTC (permalink / raw)
To: Ian Jackson; +Cc: Anthony PERARD, Xen-devel, Wei Liu, Andrew Cooper
On Wed, Aug 24, 2016 at 12:38:06PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH 0/2] Fix issue with {OVMF,SEABIOS}_PATH"):
> > They shouldn't be available when the respective BIOS is disabled at build time.
> >
> > This should fix the issus that causes xtf fail to launch hvm guests.
>
> Both
>
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Series pushed. Thanks.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v8 05/13] libxl: Load guest BIOS from file
2016-08-22 14:26 ` Wei Liu
2016-08-22 15:05 ` [PATCH 0/2] Fix issue with {OVMF,SEABIOS}_PATH Wei Liu
@ 2016-08-22 15:09 ` Andrew Cooper
2016-08-22 15:13 ` Wei Liu
1 sibling, 1 reply; 32+ messages in thread
From: Andrew Cooper @ 2016-08-22 15:09 UTC (permalink / raw)
To: Wei Liu; +Cc: Anthony PERARD, Xen-devel
On 22/08/16 15:26, Wei Liu wrote:
> On Mon, Aug 22, 2016 at 02:26:11PM +0100, Andrew Cooper wrote:
>> On 22/08/16 14:13, Wei Liu wrote:
>>> On Fri, Aug 19, 2016 at 03:43:00PM +0100, Andrew Cooper wrote:
>>>> On 18/08/16 15:13, Wei Liu wrote:
>>>>> From: Anthony PERARD <anthony.perard@citrix.com>
>>>>>
>>>>> The path to the BIOS blob can be overriden by the xl's
>>>>> bios_path_override option, or provided by u.hvm.bios_firmware in the
>>>>> domain_build_info struct by other libxl user.
>>>>>
>>>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>>>> Acked-by: Wei Liu <wei.liu2@citrix.com>
>>>> This introduces a regression, but I am not sure how best to fix it.
>>>>
>>>> [root@xrtuk-09-12 xen-test-framework]# xl -vvv create -p
>>>> tests/selftest/test-hvm32-selftest.cfg
>>>> Parsing config from tests/selftest/test-hvm32-selftest.cfg
>>>> libxl: debug: libxl_create.c:1603:do_domain_create: ao 0xa6b9f0: create:
>>>> how=(nil) callback=(nil) poller=0xa6c120
>>>> libxl: debug: libxl_create.c:959:initiate_domain_create: running bootloader
>>>> libxl: debug: libxl_bootloader.c:324:libxl__bootloader_run: not a PV
>>>> domain, skipping bootloader
>>>> libxl: debug: libxl_event.c:686:libxl__ev_xswatch_deregister: watch
>>>> w=0xa6cc30: deregister unregistered
>>>> libxl: debug: libxl_numa.c:483:libxl__get_numa_candidate: New best NUMA
>>>> placement candidate found: nr_nodes=1, nr_cpus=12, nr_vcpus=17,
>>>> free_memkb=30611
>>>> libxl: detail: libxl_dom.c:182:numa_place_domain: NUMA placement
>>>> candidate with 1 nodes, 12 cpus and 30611 KB free selected
>>>> domainbuilder: detail: xc_dom_allocate: cmdline="(null)", features="(null)"
>>>> domainbuilder: detail: xc_dom_kernel_file:
>>>> filename="/opt/xen-test-framework/tests/selftest/test-hvm32-selftest"
>>>> domainbuilder: detail: xc_dom_malloc_filemap : 1090 kB
>>>> libxl: debug: libxl_dom.c:874:libxl__load_hvm_firmware_module: Loading
>>>> BIOS: /usr/libexec/xen/boot/bios.bin
>>>> libxl: error: libxl_dom.c:882:libxl__load_hvm_firmware_module: failed to
>>>> read BIOS file: No such file or directory
>>>>
>>>> In this case, tools have been built with ./configure --disable-seabios
>>>>
>>>> As a result, /usr/libexec/xen/boot/bios.bin (name altered a patch sent
>>>> separately) isn't built or installed.
>>>>
>>>> I think libxl needs to logic to determine which firmware to use based on
>>>> the available CONFIG_* options it was built with.
>>> I don' quite follow here.
>>>
>>> Do you mean if user hasn't specified any bios= option, (s)he gets
>>> whatever available?
>>>
>>> I think we should stick with the seabios-default behaviour to avoid
>>> surprising breakage.
>>>
>>> If you don't want any bois, maybe we should provide a bios=none option?
>> XenServer is built with ./configure --disable-seabios because we don't
>> use it (yet). This means that SeaBIOS is not built, installed, or
>> available.
>>
>> Because of this change, libxl unconditionally assumes the presence of
>> SeaBIOS, and tries to use the installed seabios binary.
> Right, this needs fixing.
>
> We can restore the behaviour in libxl -- if you specify a not available
> bios, libxl won't complain, hvmloader will crash in the same way as
> before.
HVMLoader works fine. I am not sure how you got this idea.
For XTF, we use firmware_override= to replace hvmloader with something
else. As bios= is specific to hvmloader, it shouldn't be mandatory, and
should probably have an explicit none option.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v8 05/13] libxl: Load guest BIOS from file
2016-08-22 15:09 ` [PATCH v8 05/13] libxl: Load guest BIOS from file Andrew Cooper
@ 2016-08-22 15:13 ` Wei Liu
0 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-22 15:13 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Anthony PERARD, Xen-devel, Wei Liu
On Mon, Aug 22, 2016 at 04:09:07PM +0100, Andrew Cooper wrote:
> On 22/08/16 15:26, Wei Liu wrote:
> > On Mon, Aug 22, 2016 at 02:26:11PM +0100, Andrew Cooper wrote:
> >> On 22/08/16 14:13, Wei Liu wrote:
> >>> On Fri, Aug 19, 2016 at 03:43:00PM +0100, Andrew Cooper wrote:
> >>>> On 18/08/16 15:13, Wei Liu wrote:
> >>>>> From: Anthony PERARD <anthony.perard@citrix.com>
> >>>>>
> >>>>> The path to the BIOS blob can be overriden by the xl's
> >>>>> bios_path_override option, or provided by u.hvm.bios_firmware in the
> >>>>> domain_build_info struct by other libxl user.
> >>>>>
> >>>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >>>>> Acked-by: Wei Liu <wei.liu2@citrix.com>
> >>>> This introduces a regression, but I am not sure how best to fix it.
> >>>>
> >>>> [root@xrtuk-09-12 xen-test-framework]# xl -vvv create -p
> >>>> tests/selftest/test-hvm32-selftest.cfg
> >>>> Parsing config from tests/selftest/test-hvm32-selftest.cfg
> >>>> libxl: debug: libxl_create.c:1603:do_domain_create: ao 0xa6b9f0: create:
> >>>> how=(nil) callback=(nil) poller=0xa6c120
> >>>> libxl: debug: libxl_create.c:959:initiate_domain_create: running bootloader
> >>>> libxl: debug: libxl_bootloader.c:324:libxl__bootloader_run: not a PV
> >>>> domain, skipping bootloader
> >>>> libxl: debug: libxl_event.c:686:libxl__ev_xswatch_deregister: watch
> >>>> w=0xa6cc30: deregister unregistered
> >>>> libxl: debug: libxl_numa.c:483:libxl__get_numa_candidate: New best NUMA
> >>>> placement candidate found: nr_nodes=1, nr_cpus=12, nr_vcpus=17,
> >>>> free_memkb=30611
> >>>> libxl: detail: libxl_dom.c:182:numa_place_domain: NUMA placement
> >>>> candidate with 1 nodes, 12 cpus and 30611 KB free selected
> >>>> domainbuilder: detail: xc_dom_allocate: cmdline="(null)", features="(null)"
> >>>> domainbuilder: detail: xc_dom_kernel_file:
> >>>> filename="/opt/xen-test-framework/tests/selftest/test-hvm32-selftest"
> >>>> domainbuilder: detail: xc_dom_malloc_filemap : 1090 kB
> >>>> libxl: debug: libxl_dom.c:874:libxl__load_hvm_firmware_module: Loading
> >>>> BIOS: /usr/libexec/xen/boot/bios.bin
> >>>> libxl: error: libxl_dom.c:882:libxl__load_hvm_firmware_module: failed to
> >>>> read BIOS file: No such file or directory
> >>>>
> >>>> In this case, tools have been built with ./configure --disable-seabios
> >>>>
> >>>> As a result, /usr/libexec/xen/boot/bios.bin (name altered a patch sent
> >>>> separately) isn't built or installed.
> >>>>
> >>>> I think libxl needs to logic to determine which firmware to use based on
> >>>> the available CONFIG_* options it was built with.
> >>> I don' quite follow here.
> >>>
> >>> Do you mean if user hasn't specified any bios= option, (s)he gets
> >>> whatever available?
> >>>
> >>> I think we should stick with the seabios-default behaviour to avoid
> >>> surprising breakage.
> >>>
> >>> If you don't want any bois, maybe we should provide a bios=none option?
> >> XenServer is built with ./configure --disable-seabios because we don't
> >> use it (yet). This means that SeaBIOS is not built, installed, or
> >> available.
> >>
> >> Because of this change, libxl unconditionally assumes the presence of
> >> SeaBIOS, and tries to use the installed seabios binary.
> > Right, this needs fixing.
> >
> > We can restore the behaviour in libxl -- if you specify a not available
> > bios, libxl won't complain, hvmloader will crash in the same way as
> > before.
>
> HVMLoader works fine. I am not sure how you got this idea.
>
Huh? If you don't embed seabios or ovmf in hvmloader and then specify
bios=seabios or bios=ovmf in guest config file, I'm sure it will crash
when trying to load bios.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v8 05/13] libxl: Load guest BIOS from file
2016-08-18 14:13 ` [PATCH v8 05/13] libxl: Load guest BIOS from file Wei Liu
2016-08-19 14:43 ` Andrew Cooper
@ 2016-08-23 20:00 ` Doug Goldstein
2016-08-24 9:16 ` Wei Liu
1 sibling, 1 reply; 32+ messages in thread
From: Doug Goldstein @ 2016-08-23 20:00 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Anthony PERARD
[-- Attachment #1.1.1: Type: text/plain, Size: 1336 bytes --]
On 8/18/16 10:13 AM, Wei Liu wrote:
>
> + if (info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> + if (info->u.hvm.system_firmware) {
> + bios_filename = info->u.hvm.system_firmware;
> + } else {
> + switch (info->u.hvm.bios) {
> + case LIBXL_BIOS_TYPE_SEABIOS:
> + bios_filename = libxl__seabios_path();
> + break;
> + case LIBXL_BIOS_TYPE_OVMF:
> + bios_filename = libxl__ovmf_path();
> + break;
> + case LIBXL_BIOS_TYPE_ROMBIOS:
> + default:
> + abort();
Wei,
Please consider another solution. I've been trying to use libxl from
Rust and the calls to abort() are just really hard to deal with. I know
libxl has 50+ calls currently but let's work on reducing these as much
as possible so that its possible to consume libxl in other languages.
abort() is just bad form for libraries because you don't give the caller
the ability to catch the error and handle it appropriately (which could
be as simple as displaying it to the user or potentially work around the
issue.
I know you and Anthony have gone through 8 revisions but please consider
changing this. I'm sorry for not speaking up sooner.
--
Doug Goldstein
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v8 05/13] libxl: Load guest BIOS from file
2016-08-23 20:00 ` Doug Goldstein
@ 2016-08-24 9:16 ` Wei Liu
0 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-24 9:16 UTC (permalink / raw)
To: Doug Goldstein; +Cc: Anthony PERARD, Xen-devel, Wei Liu
On Tue, Aug 23, 2016 at 04:00:24PM -0400, Doug Goldstein wrote:
> On 8/18/16 10:13 AM, Wei Liu wrote:
>
> >
> > + if (info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> > + if (info->u.hvm.system_firmware) {
> > + bios_filename = info->u.hvm.system_firmware;
> > + } else {
> > + switch (info->u.hvm.bios) {
> > + case LIBXL_BIOS_TYPE_SEABIOS:
> > + bios_filename = libxl__seabios_path();
> > + break;
> > + case LIBXL_BIOS_TYPE_OVMF:
> > + bios_filename = libxl__ovmf_path();
> > + break;
> > + case LIBXL_BIOS_TYPE_ROMBIOS:
> > + default:
> > + abort();
>
> Wei,
>
> Please consider another solution. I've been trying to use libxl from
> Rust and the calls to abort() are just really hard to deal with. I know
> libxl has 50+ calls currently but let's work on reducing these as much
> as possible so that its possible to consume libxl in other languages.
>
> abort() is just bad form for libraries because you don't give the caller
> the ability to catch the error and handle it appropriately (which could
> be as simple as displaying it to the user or potentially work around the
> issue.
>
> I know you and Anthony have gone through 8 revisions but please consider
> changing this. I'm sorry for not speaking up sooner.
As said, the abort() in internal function marks an impossible situation
to get into -- much like BUG_ON in hypervisor. I would expect the error
to be handled in some other places in libxl. In this particular
instance, I haven't checked, but if there is no such check elsewhere, I
will either fix it here or somewhere else appropriate.
Furthermore, I'm not averse to systematically removing abort(), but I
would like to at least have a rough idea how upper layer would want to
handle that, and what is the implication on other parts of libxl.
Wei.
> --
> Doug Goldstein
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v8 06/13] hvmloader: Grab the hvm_start_info pointer
2016-08-18 14:13 [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
` (4 preceding siblings ...)
2016-08-18 14:13 ` [PATCH v8 05/13] libxl: Load guest BIOS from file Wei Liu
@ 2016-08-18 14:13 ` Wei Liu
2016-08-18 14:13 ` [PATCH v8 07/13] hvmloader: Locate the BIOS blob Wei Liu
` (7 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-18 14:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes in V6:
- include xen/arch-x86/hvm/start_info.h
Change in V4:
- remove struct hvm_info_start redefinition, as it's moved to
public/xen.h in a previous patch.
Change in V3:
- remove cmdline parser
- load hvm_start_info pointer earlier, before calling main().
- Add struct hvm_start_info definition to hvmloader.
---
tools/firmware/hvmloader/hvmloader.c | 6 ++++++
tools/firmware/hvmloader/util.h | 3 +++
2 files changed, 9 insertions(+)
diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
index 47290e5..67a18af 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -28,6 +28,9 @@
#include "vnuma.h"
#include <xen/version.h>
#include <xen/hvm/params.h>
+#include <xen/arch-x86/hvm/start_info.h>
+
+const struct hvm_start_info *hvm_start_info;
asm (
" .text \n"
@@ -46,6 +49,8 @@ asm (
" ljmp $"STR(SEL_CODE32)",$1f \n"
"1: movl $stack_top,%esp \n"
" movl %esp,%ebp \n"
+ /* store HVM start info ptr */
+ " mov %ebx, hvm_start_info \n"
" call main \n"
/* Relocate real-mode trampoline to 0x0. */
" mov $trampoline_start,%esi \n"
@@ -270,6 +275,7 @@ int main(void)
memset((void *)HYPERCALL_PHYSICAL_ADDRESS, 0xc3 /* RET */, PAGE_SIZE);
printf("HVM Loader\n");
+ BUG_ON(hvm_start_info->magic != XEN_HVM_START_MAGIC_VALUE);
init_hypercalls();
diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
index b226df4..0fb266e 100644
--- a/tools/firmware/hvmloader/util.h
+++ b/tools/firmware/hvmloader/util.h
@@ -158,6 +158,9 @@ static inline void cpu_relax(void)
struct hvm_info_table *get_hvm_info_table(void) __attribute__ ((const));
#define hvm_info (get_hvm_info_table())
+/* HVM start info */
+extern const struct hvm_start_info *hvm_start_info;
+
/* String and memory functions */
int strcmp(const char *cs, const char *ct);
int strncmp(const char *s1, const char *s2, uint32_t n);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v8 07/13] hvmloader: Locate the BIOS blob
2016-08-18 14:13 [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
` (5 preceding siblings ...)
2016-08-18 14:13 ` [PATCH v8 06/13] hvmloader: Grab the hvm_start_info pointer Wei Liu
@ 2016-08-18 14:13 ` Wei Liu
2016-08-18 15:39 ` Jan Beulich
2016-08-18 14:13 ` [PATCH v8 08/13] hvmloader: Load SeaBIOS from hvm_start_info modules Wei Liu
` (6 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Wei Liu @ 2016-08-18 14:13 UTC (permalink / raw)
To: Xen-devel
Cc: Anthony PERARD, Wei Liu, Ian Jackson, Jan Beulich, Andrew Cooper
From: Anthony PERARD <anthony.perard@citrix.com>
The BIOS blob can be found an entry called "firmware" of the modlist of
the hvm_start_info struct.
The found BIOS blob is not loaded by this patch, but only passed as
argument to bios_load() function.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
No change in v8.
Changes in V6:
- cast addresses to uintptr_t instead of uint32_t.
- use UINTPTR_MAX for the upper boundary checks.
- Do a full check of every things that are used, check that modlist,
cmdlines, modules lives below 4GB and does not cross the boundary.
Changes in V5:
- don't BUG() on module's paddr having value 0, and just skip.
- fix some coding style
- rename module name to "firmware" (was "bios")
- less use of BUG_ON in get_module_entry() and skip entries instead.
Only BUG() if the module which match name is not accessible.
Changes in V4:
- add more BUG_ON into get_module_entry(). Check that modules paddr and
size are 32bits.
Changes in V3:
- fix some codying style
- use module.cmdline to look for a module name instead of the main cmdline
from hvm_start_info.
---
tools/firmware/hvmloader/config.h | 2 +-
tools/firmware/hvmloader/hvmloader.c | 60 ++++++++++++++++++++++++++++++++++--
tools/firmware/hvmloader/ovmf.c | 3 +-
tools/firmware/hvmloader/rombios.c | 3 +-
4 files changed, 63 insertions(+), 5 deletions(-)
diff --git a/tools/firmware/hvmloader/config.h b/tools/firmware/hvmloader/config.h
index da1e7cf..a4429f4 100644
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -22,7 +22,7 @@ struct bios_config {
/* ROMS */
void (*load_roms)(void);
- void (*bios_load)(const struct bios_config *config);
+ void (*bios_load)(const struct bios_config *config, void *addr, uint32_t size);
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 67a18af..1032e3c 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -266,10 +266,57 @@ static void acpi_enable_sci(void)
BUG_ON(!(pm1a_cnt_val & ACPI_PM1C_SCI_EN));
}
+const struct hvm_modlist_entry *get_module_entry(
+ const struct hvm_start_info *info,
+ const char *name)
+{
+ const struct hvm_modlist_entry *modlist =
+ (struct hvm_modlist_entry *)(uintptr_t)info->modlist_paddr;
+ unsigned int i;
+
+ if ( !modlist ||
+ info->modlist_paddr > UINTPTR_MAX ||
+ (info->modlist_paddr + info->nr_modules * sizeof(*modlist) - 1)
+ > UINTPTR_MAX
+ )
+ return NULL;
+
+ for ( i = 0; i < info->nr_modules; i++ )
+ {
+ char *module_name = (char*)(uintptr_t)modlist[i].cmdline_paddr;
+
+ /* Skip if the module or its cmdline is missing. */
+ if ( !module_name || !modlist[i].paddr )
+ continue;
+
+ /* Skip if the cmdline can not be read. */
+ if ( modlist[i].cmdline_paddr > UINTPTR_MAX ||
+ (modlist[i].cmdline_paddr + strlen(name)) > UINTPTR_MAX )
+ continue;
+
+ if ( !strcmp(name, module_name) )
+ {
+ if ( modlist[i].paddr > UINTPTR_MAX ||
+ modlist[i].size > UINTPTR_MAX ||
+ (modlist[i].paddr + modlist[i].size - 1) > UINTPTR_MAX )
+ {
+ printf("Can not load \"%s\" from 0x"PRIllx" (0x"PRIllx")\n",
+ name, PRIllx_arg(modlist[i].paddr),
+ PRIllx_arg(modlist[i].size));
+ BUG();
+ }
+ return &modlist[i];
+ }
+ }
+
+ return NULL;
+}
+
int main(void)
{
const struct bios_config *bios;
int acpi_enabled;
+ const struct hvm_modlist_entry *bios_module;
/* Initialise hypercall stubs with RET, rendering them no-ops. */
memset((void *)HYPERCALL_PHYSICAL_ADDRESS, 0xc3 /* RET */, PAGE_SIZE);
@@ -305,8 +352,17 @@ int main(void)
}
printf("Loading %s ...\n", bios->name);
- if ( bios->bios_load )
- bios->bios_load(bios);
+ bios_module = get_module_entry(hvm_start_info, "firmware");
+ if ( bios_module && bios->bios_load )
+ {
+ uint32_t paddr = bios_module->paddr;
+
+ bios->bios_load(bios, (void*)paddr, bios_module->size);
+ }
+ else if ( bios->bios_load )
+ {
+ bios->bios_load(bios, NULL, 0);
+ }
else
{
BUG_ON(bios->bios_address + bios->image_size >
diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
index e7d0785..60d000f 100644
--- a/tools/firmware/hvmloader/ovmf.c
+++ b/tools/firmware/hvmloader/ovmf.c
@@ -93,7 +93,8 @@ static void ovmf_finish_bios_info(void)
info->checksum = -checksum;
}
-static void ovmf_load(const struct bios_config *config)
+static void ovmf_load(const struct bios_config *config,
+ void *bios_addr, uint32_t bios_length)
{
xen_pfn_t mfn;
uint64_t addr = OVMF_BEGIN;
diff --git a/tools/firmware/hvmloader/rombios.c b/tools/firmware/hvmloader/rombios.c
index b66f8e9..9acf03f 100644
--- a/tools/firmware/hvmloader/rombios.c
+++ b/tools/firmware/hvmloader/rombios.c
@@ -121,7 +121,8 @@ static void rombios_load_roms(void)
option_rom_phys_addr + option_rom_sz - 1);
}
-static void rombios_load(const struct bios_config *config)
+static void rombios_load(const struct bios_config *config,
+ void *unused_addr, uint32_t unused_size)
{
uint32_t bioshigh;
struct rombios_info *info;
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v8 07/13] hvmloader: Locate the BIOS blob
2016-08-18 14:13 ` [PATCH v8 07/13] hvmloader: Locate the BIOS blob Wei Liu
@ 2016-08-18 15:39 ` Jan Beulich
2016-08-18 15:48 ` Andrew Cooper
0 siblings, 1 reply; 32+ messages in thread
From: Jan Beulich @ 2016-08-18 15:39 UTC (permalink / raw)
To: Anthony PERARD, Wei Liu; +Cc: Andrew Cooper, Ian Jackson, Xen-devel
>>> On 18.08.16 at 16:13, <wei.liu2@citrix.com> wrote:
> From: Anthony PERARD <anthony.perard@citrix.com>
>
> The BIOS blob can be found an entry called "firmware" of the modlist of
> the hvm_start_info struct.
>
> The found BIOS blob is not loaded by this patch, but only passed as
> argument to bios_load() function.
>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
provided Andrew confirms his concerns have got addressed and a
few minor things get taken care of:
> --- a/tools/firmware/hvmloader/hvmloader.c
> +++ b/tools/firmware/hvmloader/hvmloader.c
> @@ -266,10 +266,57 @@ static void acpi_enable_sci(void)
> BUG_ON(!(pm1a_cnt_val & ACPI_PM1C_SCI_EN));
> }
>
> +const struct hvm_modlist_entry *get_module_entry(
> + const struct hvm_start_info *info,
> + const char *name)
> +{
> + const struct hvm_modlist_entry *modlist =
> + (struct hvm_modlist_entry *)(uintptr_t)info->modlist_paddr;
> + unsigned int i;
> +
> + if ( !modlist ||
> + info->modlist_paddr > UINTPTR_MAX ||
> + (info->modlist_paddr + info->nr_modules * sizeof(*modlist) - 1)
> + > UINTPTR_MAX
> + )
Urgh?
> + return NULL;
> +
> + for ( i = 0; i < info->nr_modules; i++ )
> + {
> + char *module_name = (char*)(uintptr_t)modlist[i].cmdline_paddr;
> +
> + /* Skip if the module or its cmdline is missing. */
> + if ( !module_name || !modlist[i].paddr )
> + continue;
> +
> + /* Skip if the cmdline can not be read. */
cannot (afaik)
> + if ( modlist[i].cmdline_paddr > UINTPTR_MAX ||
> + (modlist[i].cmdline_paddr + strlen(name)) > UINTPTR_MAX )
> + continue;
> +
> + if ( !strcmp(name, module_name) )
> + {
> + if ( modlist[i].paddr > UINTPTR_MAX ||
> + modlist[i].size > UINTPTR_MAX ||
> + (modlist[i].paddr + modlist[i].size - 1) > UINTPTR_MAX )
> + {
> + printf("Can not load \"%s\" from 0x"PRIllx" (0x"PRIllx")\n",
Same here.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v8 07/13] hvmloader: Locate the BIOS blob
2016-08-18 15:39 ` Jan Beulich
@ 2016-08-18 15:48 ` Andrew Cooper
2016-08-18 15:51 ` Wei Liu
0 siblings, 1 reply; 32+ messages in thread
From: Andrew Cooper @ 2016-08-18 15:48 UTC (permalink / raw)
To: Jan Beulich, Anthony PERARD, Wei Liu; +Cc: Xen-devel, Ian Jackson
On 18/08/16 16:39, Jan Beulich wrote:
>>>> On 18.08.16 at 16:13, <wei.liu2@citrix.com> wrote:
>> From: Anthony PERARD <anthony.perard@citrix.com>
>>
>> The BIOS blob can be found an entry called "firmware" of the modlist of
>> the hvm_start_info struct.
>>
>> The found BIOS blob is not loaded by this patch, but only passed as
>> argument to bios_load() function.
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> provided Andrew confirms his concerns have got addressed and a
> few minor things get taken care of:
They haven't, but I am planning some fixes anyway, so I will adjust
things there. They are not worth letting this series drag on even longer.
>
>> --- a/tools/firmware/hvmloader/hvmloader.c
>> +++ b/tools/firmware/hvmloader/hvmloader.c
>> @@ -266,10 +266,57 @@ static void acpi_enable_sci(void)
>> BUG_ON(!(pm1a_cnt_val & ACPI_PM1C_SCI_EN));
>> }
>>
>> +const struct hvm_modlist_entry *get_module_entry(
>> + const struct hvm_start_info *info,
>> + const char *name)
>> +{
>> + const struct hvm_modlist_entry *modlist =
>> + (struct hvm_modlist_entry *)(uintptr_t)info->modlist_paddr;
>> + unsigned int i;
>> +
>> + if ( !modlist ||
>> + info->modlist_paddr > UINTPTR_MAX ||
>> + (info->modlist_paddr + info->nr_modules * sizeof(*modlist) - 1)
>> + > UINTPTR_MAX
>> + )
> Urgh?
>
>> + return NULL;
>> +
>> + for ( i = 0; i < info->nr_modules; i++ )
>> + {
>> + char *module_name = (char*)(uintptr_t)modlist[i].cmdline_paddr;
>> +
>> + /* Skip if the module or its cmdline is missing. */
>> + if ( !module_name || !modlist[i].paddr )
>> + continue;
>> +
>> + /* Skip if the cmdline can not be read. */
> cannot (afaik)
>
>> + if ( modlist[i].cmdline_paddr > UINTPTR_MAX ||
>> + (modlist[i].cmdline_paddr + strlen(name)) > UINTPTR_MAX )
>> + continue;
>> +
>> + if ( !strcmp(name, module_name) )
>> + {
>> + if ( modlist[i].paddr > UINTPTR_MAX ||
>> + modlist[i].size > UINTPTR_MAX ||
>> + (modlist[i].paddr + modlist[i].size - 1) > UINTPTR_MAX )
>> + {
>> + printf("Can not load \"%s\" from 0x"PRIllx" (0x"PRIllx")\n",
> Same here.
"can not" is valid, but it is very uncommon to use. The double n is
awkward to pronounce, which is why "cannot" (with only a single n to
pronounce) is the more common form.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v8 07/13] hvmloader: Locate the BIOS blob
2016-08-18 15:48 ` Andrew Cooper
@ 2016-08-18 15:51 ` Wei Liu
0 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-18 15:51 UTC (permalink / raw)
To: Andrew Cooper
Cc: Anthony PERARD, Ian Jackson, Wei Liu, Jan Beulich, Xen-devel
On Thu, Aug 18, 2016 at 04:48:20PM +0100, Andrew Cooper wrote:
> On 18/08/16 16:39, Jan Beulich wrote:
> >>>> On 18.08.16 at 16:13, <wei.liu2@citrix.com> wrote:
> >> From: Anthony PERARD <anthony.perard@citrix.com>
> >>
> >> The BIOS blob can be found an entry called "firmware" of the modlist of
> >> the hvm_start_info struct.
> >>
> >> The found BIOS blob is not loaded by this patch, but only passed as
> >> argument to bios_load() function.
> >>
> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > Acked-by: Jan Beulich <jbeulich@suse.com>
> > provided Andrew confirms his concerns have got addressed and a
> > few minor things get taken care of:
>
> They haven't, but I am planning some fixes anyway, so I will adjust
> things there. They are not worth letting this series drag on even longer.
>
Can I turn this into an ack?
I can fix the issues Jan pointed out why committing.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v8 08/13] hvmloader: Load SeaBIOS from hvm_start_info modules
2016-08-18 14:13 [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
` (6 preceding siblings ...)
2016-08-18 14:13 ` [PATCH v8 07/13] hvmloader: Locate the BIOS blob Wei Liu
@ 2016-08-18 14:13 ` Wei Liu
2016-08-18 14:13 ` [PATCH v8 09/13] hvmloader: Load OVMF from modules Wei Liu
` (5 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-18 14:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
... and do not include the SeaBIOS ROM into hvmloader anymore.
This also fix the dependency on roms.inc, hvmloader.o does not include it.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Change in V6:
acked
Change in V5:
- update BUG_ON in seabios_setup_e820().
Change in V4:
- check that seabios_config.bios_address have a probably good value
instead of checking only if it's set.
Change in V3:
- change makefile to not include seabios roms into roms.inc.
- update the struct bios_config with the location of the bios blob.
---
tools/firmware/hvmloader/Makefile | 15 +--------------
tools/firmware/hvmloader/seabios.c | 25 +++++++++++++++----------
2 files changed, 16 insertions(+), 24 deletions(-)
diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index f2f4791..ed0bfad 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -45,7 +45,6 @@ CIRRUSVGA_DEBUG ?= n
OVMF_DIR := ../ovmf-dir
ROMBIOS_DIR := ../rombios
-SEABIOS_DIR := ../seabios-dir
ifeq ($(CONFIG_ROMBIOS),y)
STDVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.bin
@@ -80,19 +79,13 @@ endif
ifeq ($(CONFIG_SEABIOS),y)
OBJS += seabios.o
CFLAGS += -DENABLE_SEABIOS
-ifeq ($(SEABIOS_PATH),)
- SEABIOS_ROM := $(SEABIOS_DIR)/out/bios.bin
-else
- SEABIOS_ROM := $(SEABIOS_PATH)
-endif
-ROMS += $(SEABIOS_ROM)
endif
.PHONY: all
all: subdirs-all
$(MAKE) hvmloader
-ovmf.o rombios.o seabios.o hvmloader.o: roms.inc
+ovmf.o rombios.o: roms.inc
smbios.o: CFLAGS += -D__SMBIOS_DATE__="\"$(SMBIOS_REL_DATE)\""
hvmloader: $(OBJS) acpi/acpi.a
@@ -109,12 +102,6 @@ ifneq ($(ROMBIOS_ROM),)
echo "#endif" >> $@.new
endif
-ifneq ($(SEABIOS_ROM),)
- echo "#ifdef ROM_INCLUDE_SEABIOS" >> $@.new
- sh ./mkhex seabios $(SEABIOS_ROM) >> $@.new
- echo "#endif" >> $@.new
-endif
-
ifneq ($(OVMF_ROM),)
echo "#ifdef ROM_INCLUDE_OVMF" >> $@.new
sh ./mkhex ovmf $(OVMF_ROM) >> $@.new
diff --git a/tools/firmware/hvmloader/seabios.c b/tools/firmware/hvmloader/seabios.c
index 2fbc3af..5c9a351 100644
--- a/tools/firmware/hvmloader/seabios.c
+++ b/tools/firmware/hvmloader/seabios.c
@@ -28,9 +28,6 @@
#include "acpi/acpi2_0.h"
#include "acpi/libacpi.h"
-#define ROM_INCLUDE_SEABIOS
-#include "roms.inc"
-
extern unsigned char dsdt_anycpu_qemu_xen[];
extern int dsdt_anycpu_qemu_xen_len;
@@ -128,22 +125,30 @@ static void seabios_setup_e820(void)
struct e820entry *e820 = scratch_alloc(sizeof(struct e820entry)*16, 0);
info->e820 = (uint32_t)e820;
+ /* Upper boundary already checked by seabios_load(). */
+ BUG_ON(seabios_config.bios_address < 0x000c0000);
/* SeaBIOS reserves memory in e820 as necessary so no low reservation. */
- info->e820_nr = build_e820_table(e820, 0, 0x100000-sizeof(seabios));
+ info->e820_nr = build_e820_table(e820, 0, seabios_config.bios_address);
dump_e820_table(e820, info->e820_nr);
}
-struct bios_config seabios_config = {
- .name = "SeaBIOS",
+static void seabios_load(const struct bios_config *bios,
+ void *bios_addr, uint32_t bios_length)
+{
+ unsigned int bios_dest = 0x100000 - bios_length;
- .image = seabios,
- .image_size = sizeof(seabios),
+ BUG_ON(bios_dest + bios_length > HVMLOADER_PHYSICAL_ADDRESS);
+ memcpy((void *)bios_dest, bios_addr, bios_length);
+ seabios_config.bios_address = bios_dest;
+ seabios_config.image_size = bios_length;
+}
- .bios_address = 0x100000 - sizeof(seabios),
+struct bios_config seabios_config = {
+ .name = "SeaBIOS",
.load_roms = NULL,
- .bios_load = NULL,
+ .bios_load = seabios_load,
.bios_info_setup = seabios_setup_bios_info,
.bios_info_finish = seabios_finish_bios_info,
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v8 09/13] hvmloader: Load OVMF from modules
2016-08-18 14:13 [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
` (7 preceding siblings ...)
2016-08-18 14:13 ` [PATCH v8 08/13] hvmloader: Load SeaBIOS from hvm_start_info modules Wei Liu
@ 2016-08-18 14:13 ` Wei Liu
2016-08-18 14:13 ` [PATCH v8 10/13] hvmloader: bios->bios_load() now needs to be defined Wei Liu
` (4 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-18 14:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
... and do not include the OVMF ROM into hvmloader anymore.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Change in V5:
- define OVMF_END macro
- fix some cast coding style
Change in V4:
- check if source and dest of ovmf binary does not overlaps
Change in V3:
- change makefile to not include ovmf rom into roms.inc.
- update the struct bios_config with bios destination
---
tools/firmware/hvmloader/Makefile | 15 +--------------
tools/firmware/hvmloader/ovmf.c | 31 ++++++++++++++-----------------
2 files changed, 15 insertions(+), 31 deletions(-)
diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index ed0bfad..dee1c6b 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -43,7 +43,6 @@ endif
CIRRUSVGA_DEBUG ?= n
-OVMF_DIR := ../ovmf-dir
ROMBIOS_DIR := ../rombios
ifeq ($(CONFIG_ROMBIOS),y)
@@ -61,12 +60,6 @@ ROMS :=
ifeq ($(CONFIG_OVMF),y)
OBJS += ovmf.o
CFLAGS += -DENABLE_OVMF
-ifeq ($(OVMF_PATH),)
- OVMF_ROM := $(OVMF_DIR)/ovmf.bin
-else
- OVMF_ROM := $(OVMF_PATH)
-endif
-ROMS += $(OVMF_ROM)
endif
ifeq ($(CONFIG_ROMBIOS),y)
@@ -85,7 +78,7 @@ endif
all: subdirs-all
$(MAKE) hvmloader
-ovmf.o rombios.o: roms.inc
+rombios.o: roms.inc
smbios.o: CFLAGS += -D__SMBIOS_DATE__="\"$(SMBIOS_REL_DATE)\""
hvmloader: $(OBJS) acpi/acpi.a
@@ -102,12 +95,6 @@ ifneq ($(ROMBIOS_ROM),)
echo "#endif" >> $@.new
endif
-ifneq ($(OVMF_ROM),)
- echo "#ifdef ROM_INCLUDE_OVMF" >> $@.new
- sh ./mkhex ovmf $(OVMF_ROM) >> $@.new
- echo "#endif" >> $@.new
-endif
-
ifneq ($(STDVGA_ROM),)
echo "#ifdef ROM_INCLUDE_VGABIOS" >> $@.new
sh ./mkhex vgabios_stdvga $(STDVGA_ROM) >> $@.new
diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
index 60d000f..b4bcc93 100644
--- a/tools/firmware/hvmloader/ovmf.c
+++ b/tools/firmware/hvmloader/ovmf.c
@@ -34,17 +34,11 @@
#include <xen/hvm/ioreq.h>
#include <xen/memory.h>
-#define ROM_INCLUDE_OVMF
-#include "roms.inc"
-
-#define OVMF_SIZE (sizeof(ovmf))
#define OVMF_MAXOFFSET 0x000FFFFFULL
-#define OVMF_BEGIN (0x100000000ULL - ((OVMF_SIZE + OVMF_MAXOFFSET) & ~OVMF_MAXOFFSET))
-#define OVMF_END (OVMF_BEGIN + OVMF_SIZE)
+#define OVMF_END 0x100000000ULL
#define LOWCHUNK_BEGIN 0x000F0000
#define LOWCHUNK_SIZE 0x00010000
#define LOWCHUNK_MAXOFFSET 0x0000FFFF
-#define LOWCHUNK_END (OVMF_BEGIN + OVMF_SIZE)
#define OVMF_INFO_PHYSICAL_ADDRESS 0x00001000
extern unsigned char dsdt_anycpu_qemu_xen[];
@@ -97,24 +91,31 @@ static void ovmf_load(const struct bios_config *config,
void *bios_addr, uint32_t bios_length)
{
xen_pfn_t mfn;
- uint64_t addr = OVMF_BEGIN;
+ uint64_t addr = OVMF_END
+ - ((bios_length + OVMF_MAXOFFSET) & ~OVMF_MAXOFFSET);
+ uint64_t ovmf_end = addr + bios_length;
+
+ ovmf_config.bios_address = addr;
+ ovmf_config.image_size = bios_length;
/* Copy low-reset vector portion. */
- memcpy((void *) LOWCHUNK_BEGIN, (uint8_t *) config->image
- + OVMF_SIZE
- - LOWCHUNK_SIZE,
+ memcpy((void *)LOWCHUNK_BEGIN,
+ (uint8_t *)bios_addr + bios_length - LOWCHUNK_SIZE,
LOWCHUNK_SIZE);
/* Ensure we have backing page prior to moving FD. */
- while ( (addr >> PAGE_SHIFT) != (OVMF_END >> PAGE_SHIFT) )
+ while ( (addr >> PAGE_SHIFT) != (ovmf_end >> PAGE_SHIFT) )
{
mfn = (uint32_t) (addr >> PAGE_SHIFT);
addr += PAGE_SIZE;
mem_hole_populate_ram(mfn, 1);
}
+ /* Check that source and destination does not overlaps. */
+ BUG_ON(addr + bios_length > (unsigned)bios_addr &&
+ addr < (unsigned)bios_addr + bios_length);
/* Copy FD. */
- memcpy((void *) OVMF_BEGIN, config->image, OVMF_SIZE);
+ memcpy((void *)ovmf_config.bios_address, bios_addr, bios_length);
}
static void ovmf_acpi_build_tables(void)
@@ -151,10 +152,6 @@ static void ovmf_setup_e820(void)
struct bios_config ovmf_config = {
.name = "OVMF",
- .image = ovmf,
- .image_size = sizeof(ovmf),
-
- .bios_address = OVMF_BEGIN,
.bios_load = ovmf_load,
.load_roms = 0,
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v8 10/13] hvmloader: bios->bios_load() now needs to be defined
2016-08-18 14:13 [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
` (8 preceding siblings ...)
2016-08-18 14:13 ` [PATCH v8 09/13] hvmloader: Load OVMF from modules Wei Liu
@ 2016-08-18 14:13 ` Wei Liu
2016-08-18 14:13 ` [PATCH v8 11/13] hvmloader: Always build-in SeaBIOS and OVMF loader Wei Liu
` (3 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-18 14:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
All BIOSes but ROMBIOS needs to be loaded via modules.
ROMBIOS is handled as a special case.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Change in V5:
- rename patch, was:
"hvmloader: Specific bios_load function required"
No change in V4.
Change in V3:
- reprint Main BIOS in bios map with now available information from bios
modules.
- handle rombios, and keep its built-in ROMs.
---
tools/firmware/hvmloader/hvmloader.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
index 1032e3c..0d56f7d 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -353,22 +353,26 @@ int main(void)
printf("Loading %s ...\n", bios->name);
bios_module = get_module_entry(hvm_start_info, "firmware");
- if ( bios_module && bios->bios_load )
+ if ( bios_module )
{
uint32_t paddr = bios_module->paddr;
bios->bios_load(bios, (void*)paddr, bios_module->size);
}
- else if ( bios->bios_load )
+#ifdef ENABLE_ROMBIOS
+ else if ( bios == &rombios_config )
{
bios->bios_load(bios, NULL, 0);
}
+#endif
else
{
- BUG_ON(bios->bios_address + bios->image_size >
- HVMLOADER_PHYSICAL_ADDRESS);
- memcpy((void *)bios->bios_address, bios->image,
- bios->image_size);
+ /*
+ * If there is no BIOS module supplied and if there is no embeded BIOS
+ * image, then we failed. Only rombios might have an embedded bios blob.
+ */
+ printf("no BIOS ROM image found\n");
+ BUG();
}
if ( (hvm_info->nr_vcpus > 1) || hvm_info->apic_mode )
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v8 11/13] hvmloader: Always build-in SeaBIOS and OVMF loader
2016-08-18 14:13 [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
` (9 preceding siblings ...)
2016-08-18 14:13 ` [PATCH v8 10/13] hvmloader: bios->bios_load() now needs to be defined Wei Liu
@ 2016-08-18 14:13 ` Wei Liu
2016-08-18 14:13 ` [PATCH v8 12/13] configure: do not depend on SEABIOS_PATH or OVMF_PATH Wei Liu
` (2 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-18 14:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
tools/firmware/hvmloader/Makefile | 11 +----------
tools/firmware/hvmloader/hvmloader.c | 4 ----
2 files changed, 1 insertion(+), 14 deletions(-)
diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index dee1c6b..9f7357f 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -37,6 +37,7 @@ OBJS = hvmloader.o mp_tables.o util.o smbios.o
OBJS += smp.o cacheattr.o xenbus.o vnuma.o
OBJS += e820.o pci.o pir.o ctype.o
OBJS += hvm_param.o
+OBJS += ovmf.o seabios.o
ifeq ($(debug),y)
OBJS += tests.o
endif
@@ -57,11 +58,6 @@ endif
ROMS :=
-ifeq ($(CONFIG_OVMF),y)
-OBJS += ovmf.o
-CFLAGS += -DENABLE_OVMF
-endif
-
ifeq ($(CONFIG_ROMBIOS),y)
OBJS += optionroms.o 32bitbios_support.o rombios.o
CFLAGS += -DENABLE_ROMBIOS
@@ -69,11 +65,6 @@ 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
-endif
-
.PHONY: all
all: subdirs-all
$(MAKE) hvmloader
diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
index 0d56f7d..e8d2cf5 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -222,12 +222,8 @@ struct bios_info {
#ifdef ENABLE_ROMBIOS
{ "rombios", &rombios_config, },
#endif
-#ifdef ENABLE_SEABIOS
{ "seabios", &seabios_config, },
-#endif
-#ifdef ENABLE_OVMF
{ "ovmf", &ovmf_config, },
-#endif
{ NULL, NULL }
};
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v8 12/13] configure: do not depend on SEABIOS_PATH or OVMF_PATH ...
2016-08-18 14:13 [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
` (10 preceding siblings ...)
2016-08-18 14:13 ` [PATCH v8 11/13] hvmloader: Always build-in SeaBIOS and OVMF loader Wei Liu
@ 2016-08-18 14:13 ` Wei Liu
2016-08-18 14:13 ` [PATCH v8 13/13] docs/misc/hvmlite: Point to the canonical definition of hvm_start_info Wei Liu
2016-08-18 16:23 ` [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
13 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-18 14:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
... to compile SeaBIOS and OVMF. Only depend on CONFIG_*.
If --with-system-* configure option is used, then set *_CONFIG=n to not
compile SEABIOS and OVMF.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Please, run ./autogen.sh on this patch.
No change in V5.
Change in V4:
- change subject prefix
---
tools/configure | 8 ++++----
tools/configure.ac | 6 ++++--
tools/firmware/Makefile | 8 --------
3 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/tools/configure b/tools/configure
index c182391..81ccf34 100755
--- a/tools/configure
+++ b/tools/configure
@@ -695,8 +695,6 @@ APPEND_INCLUDES
PREPEND_LIB
PREPEND_INCLUDES
EXTRA_QEMUU_CONFIGURE_ARGS
-ovmf_path
-seabios_path
qemu_xen_systemd
qemu_xen_path
qemu_xen
@@ -4442,6 +4440,8 @@ _ACEOF
# Check whether --with-system-seabios was given.
if test "${with_system_seabios+set}" = set; then :
withval=$with_system_seabios;
+ # Disable compilation of SeaBIOS.
+ seabios=n
case $withval in
no) seabios_path= ;;
*) seabios_path=$withval ;;
@@ -4450,7 +4450,6 @@ if test "${with_system_seabios+set}" = set; then :
fi
-
cat >>confdefs.h <<_ACEOF
#define SEABIOS_PATH "${seabios_path:-$XENFIRMWAREDIR/bios.bin}"
_ACEOF
@@ -4460,6 +4459,8 @@ _ACEOF
# Check whether --with-system-ovmf was given.
if test "${with_system_ovmf+set}" = set; then :
withval=$with_system_ovmf;
+ # Disable compilation of OVMF.
+ ovmf=n
case $withval in
no) ovmf_path= ;;
*) ovmf_path=$withval ;;
@@ -4468,7 +4469,6 @@ if test "${with_system_ovmf+set}" = set; then :
fi
-
cat >>confdefs.h <<_ACEOF
#define OVMF_PATH "${ovmf_path:-$XENFIRMWAREDIR/ovmf.bin}"
_ACEOF
diff --git a/tools/configure.ac b/tools/configure.ac
index ed10902..c12ad79 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -215,12 +215,13 @@ AC_ARG_WITH([system-seabios],
AS_HELP_STRING([--with-system-seabios@<:@=PATH@:>@],
[Use system supplied seabios PATH instead of building and installing
our own version]),[
+ # Disable compilation of SeaBIOS.
+ seabios=n
case $withval in
no) seabios_path= ;;
*) seabios_path=$withval ;;
esac
],[])
-AC_SUBST(seabios_path)
AC_DEFINE_UNQUOTED([SEABIOS_PATH],
["${seabios_path:-$XENFIRMWAREDIR/bios.bin}"],
[SeaBIOS path])
@@ -229,12 +230,13 @@ AC_ARG_WITH([system-ovmf],
AS_HELP_STRING([--with-system-ovmf@<:@=PATH@:>@],
[Use system supplied OVMF PATH instead of building and installing
our own version]),[
+ # Disable compilation of OVMF.
+ ovmf=n
case $withval in
no) ovmf_path= ;;
*) ovmf_path=$withval ;;
esac
],[])
-AC_SUBST(ovmf_path)
AC_DEFINE_UNQUOTED([OVMF_PATH],
["${ovmf_path:-$XENFIRMWAREDIR/ovmf.bin}"],
[OVMF path])
diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile
index 82b1f6b..cf09ad2 100644
--- a/tools/firmware/Makefile
+++ b/tools/firmware/Makefile
@@ -6,12 +6,8 @@ TARGET := hvmloader/hvmloader
INST_DIR := $(DESTDIR)$(XENFIRMWAREDIR)
SUBDIRS-y :=
-ifeq ($(OVMF_PATH),)
SUBDIRS-$(CONFIG_OVMF) += ovmf-dir
-endif
-ifeq ($(SEABIOS_PATH),)
SUBDIRS-$(CONFIG_SEABIOS) += seabios-dir
-endif
SUBDIRS-$(CONFIG_ROMBIOS) += rombios
SUBDIRS-$(CONFIG_ROMBIOS) += vgabios
SUBDIRS-$(CONFIG_ROMBIOS) += etherboot
@@ -46,15 +42,11 @@ install: all
[ -d $(INST_DIR) ] || $(INSTALL_DIR) $(INST_DIR)
[ ! -e $(TARGET) ] || $(INSTALL_DATA) $(TARGET) $(INST_DIR)
ifeq ($(CONFIG_SEABIOS),y)
-ifeq ($(SEABIOS_PATH),)
$(INSTALL_DATA) seabios-dir/out/bios.bin $(INST_DIR)/bios.bin
endif
-endif
ifeq ($(CONFIG_OVMF),y)
-ifeq ($(OVMF_PATH),)
$(INSTALL_DATA) ovmf-dir/ovmf.bin $(INST_DIR)/ovmf.bin
endif
-endif
.PHONY: clean
clean: subdirs-clean
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v8 13/13] docs/misc/hvmlite: Point to the canonical definition of hvm_start_info
2016-08-18 14:13 [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
` (11 preceding siblings ...)
2016-08-18 14:13 ` [PATCH v8 12/13] configure: do not depend on SEABIOS_PATH or OVMF_PATH Wei Liu
@ 2016-08-18 14:13 ` Wei Liu
2016-08-18 16:23 ` [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
13 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-18 14:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
The C struct in the document is no more in sync with the actual
definition of the PVHv2 boot start info.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
docs/misc/hvmlite.markdown | 20 ++------------------
1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/docs/misc/hvmlite.markdown b/docs/misc/hvmlite.markdown
index c1b75c6..69d90fe 100644
--- a/docs/misc/hvmlite.markdown
+++ b/docs/misc/hvmlite.markdown
@@ -37,24 +37,8 @@ following machine state:
All other processor registers and flag bits are unspecified. The OS is in
charge of setting up it's own stack, GDT and IDT.
-The format of the boot start info structure is the following (pointed to
-be %ebx):
-
- struct hvm_start_info {
- #define HVM_START_MAGIC_VALUE 0x336ec578
- uint32_t magic; /* Contains the magic value 0x336ec578 */
- /* ("xEn3" with the 0x80 bit of the "E" set).*/
- uint32_t flags; /* SIF_xxx flags. */
- uint32_t cmdline_paddr; /* Physical address of the command line. */
- uint32_t nr_modules; /* Number of modules passed to the kernel. */
- uint32_t modlist_paddr; /* Physical address of an array of */
- /* hvm_modlist_entry. */
- };
-
- struct hvm_modlist_entry {
- uint32_t paddr; /* Physical address of the module. */
- uint32_t size; /* Size of the module in bytes. */
- };
+The format of the boot start info structure (pointed to by %ebx) can be found
+in xen/include/public/arch-x86/hvm/start_info.h
Other relevant information needed in order to boot a guest kernel
(console page address, xenstore event channel...) can be obtained
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader
2016-08-18 14:13 [PATCH v8 00/13] Load BIOS via toolstack instead of been embedded in hvmloader Wei Liu
` (12 preceding siblings ...)
2016-08-18 14:13 ` [PATCH v8 13/13] docs/misc/hvmlite: Point to the canonical definition of hvm_start_info Wei Liu
@ 2016-08-18 16:23 ` Wei Liu
13 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2016-08-18 16:23 UTC (permalink / raw)
To: Xen-devel; +Cc: Ian Jackson, Wei Liu, Jan Beulich, Andrew Cooper
I've made the required adjustment and queued up this series.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 32+ messages in thread