[PATCH v6 00/14] Load BIOS via toolstack instead of been embedded in hvmloader.

[PATCH v6 00/14] Load BIOS via toolstack instead of been embedded in hvmloader.

[Anthony PERARD]

Hi all,

V6:
  few changes detailed in patches.

  Missing acks on the following patches:
    1 libxc: Rework extra module initialisation
    2 libxc: Prepare a start info structure for hvmloader
    5 libxl: Load guest BIOS from file
    6 xen: Move the hvm_start_info C representation from libxc to public/xen.h
    8 hvmloader: Locate the BIOS blob
    9 hvmloader: Check modules whereabouts in perform_tests

Thanks.

Changes in V5:
  quite a few rework

  it as been suggest that "bios" was not quite right to name SeaBIOS/OVMF, so
  when I introduce a new name/variable instead of bios, I tried to use
  system_firmware which is more appropriate, I think, with regards to OVMF.
  Details of the changes in the patches.

  Other changes detailed patches description.

I've look at loading the BIOS via the toolstack instead of having them embedded
in the hvmloader binary. After this patch series, hvmloader compilation would
be indenpendant from SeaBIOS and OVMF compilation.

Here is a general view of the few step to load the BIOS:
- libxl load the BIOS blob into it's memory and add it to struct
  xc_hvm_build_args.system_firmware_module
- libxc load the blob into the guest memory and fill the struct
  hvm_start_info and store a name for each module into the module cmdline.
- hvmloader read the addresses from the hvm_start_info, find out which
  module to load and copy the blob to the right place.

A git tree can be found here:
git://xenbits.xen.org/people/aperard/xen-unstable.git
tag: hvmloader-with-separated-bios-v6

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>

Regards,

Anthony PERARD (14):
  libxc: Rework extra module initialisation
  libxc: Prepare a start info structure for hvmloader
  configure: #define SEABIOS_PATH and OVMF_PATH
  firmware/makefile: install BIOS blob ...
  libxl: Load guest BIOS from file
  xen: Move the hvm_start_info C representation from libxc to
    public/xen.h
  hvmloader: Grab the hvm_start_info pointer
  hvmloader: Locate the BIOS blob
  hvmloader: Check modules whereabouts in perform_tests
  hvmloader: Load SeaBIOS from hvm_start_info modules
  hvmloader: Load OVMF from modules
  hvmloader: bios->bios_load() now needs to be defined
  hvmloader: Always build-in SeaBIOS and OVMF loader
  configure: do not depend on SEABIOS_PATH or OVMF_PATH ...

 docs/man/xl.cfg.pod.5.in                     |   9 ++
 tools/configure.ac                           |  12 ++-
 tools/firmware/Makefile                      |  10 +-
 tools/firmware/hvmloader/Makefile            |  39 +------
 tools/firmware/hvmloader/config.h            |   2 +-
 tools/firmware/hvmloader/hvmloader.c         |  82 ++++++++++++--
 tools/firmware/hvmloader/ovmf.c              |  34 +++---
 tools/firmware/hvmloader/rombios.c           |   3 +-
 tools/firmware/hvmloader/seabios.c           |  25 +++--
 tools/firmware/hvmloader/tests.c             |  76 ++++++++++++-
 tools/firmware/hvmloader/util.h              |   3 +
 tools/libxc/include/xc_dom.h                 |  34 +-----
 tools/libxc/xc_dom_hvmloader.c               | 136 ++++++++----------------
 tools/libxc/xc_dom_x86.c                     | 153 +++++++++++++++++++--------
 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 +-
 xen/include/public/arch-x86/hvm/start_info.h |  53 ++++++++++
 xen/include/public/xen.h                     |   2 +-
 22 files changed, 511 insertions(+), 255 deletions(-)
 create mode 100644 xen/include/public/arch-x86/hvm/start_info.h

-- 
Anthony PERARD


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

[PATCH v6 01/14] libxc: Rework extra module initialisation

[Anthony PERARD]

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>

---
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__);
-- 
Anthony PERARD


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

[PATCH v6 02/14] libxc: Prepare a start info structure for hvmloader

[Anthony PERARD]

... 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>

---
CC: boris.ostrovsky@oracle.com
CC: roger.pau@citrix.com

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 +++++++++++++++++++++++++++++------------
 xen/include/public/xen.h       |   2 +-
 4 files changed, 116 insertions(+), 44 deletions(-)

diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
index 6cb10c4..0629971 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 021f8a8..bc2dbb2 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -69,6 +69,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;
@@ -590,6 +593,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++ )
@@ -624,8 +628,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);
@@ -635,17 +637,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.
@@ -672,6 +675,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.
@@ -1689,42 +1700,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) +
@@ -1733,22 +1791,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(
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index b9e5e0f..427e74f 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -821,7 +821,7 @@ struct start_info {
 typedef struct start_info start_info_t;
 
 /*
- * Start of day structure passed to PVH guests in %ebx.
+ * Start of day structure passed to PVH guests and to HVM guests in %ebx.
  *
  * NOTE: nothing will be loaded at physical address 0, so a 0 value in any
  * of the address fields should be treated as not present.
-- 
Anthony PERARD


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

[PATCH v6 03/14] configure: #define SEABIOS_PATH and OVMF_PATH

[Anthony PERARD]

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/configure.ac | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/configure.ac b/tools/configure.ac
index 87e14a8..af3b6e9 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -220,6 +220,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@:>@],
@@ -231,6 +234,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 ..."@:>@],
-- 
Anthony PERARD


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

[PATCH v6 04/14] firmware/makefile: install BIOS blob ...

[Anthony PERARD]

... 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
-- 
Anthony PERARD


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

[PATCH v6 05/14] libxl: Load guest BIOS from file

[Anthony PERARD]

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>

---
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 3bb27d0..a685b83 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -1212,6 +1212,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 2c0f868..2b1f678 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -928,6 +928,14 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, 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 ec29060..2a1793d 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 e7ab85d..e6a199b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2317,6 +2317,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 d1fcfa4..1d06598 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1562,12 +1562,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);
-- 
Anthony PERARD


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

[PATCH v6 06/14] xen: Move the hvm_start_info C representation from libxc to public/xen.h

[Anthony PERARD]

Instead of having several representation of hvm_start_info in C, define
it in public/xen.h so both libxc and hvmloader can use it.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

---
Changes in V6:
- move C representation to public/arch-x86/hvm/start_info.h instead of
  public/xen.h

Change in V5:
- remove packed attribute.

New in V4.
---
 tools/libxc/include/xc_dom.h                 | 31 ----------------
 tools/libxc/xc_dom_x86.c                     |  1 +
 xen/include/public/arch-x86/hvm/start_info.h | 53 ++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 31 deletions(-)
 create mode 100644 xen/include/public/arch-x86/hvm/start_info.h

diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
index 0629971..de7dca9 100644
--- a/tools/libxc/include/xc_dom.h
+++ b/tools/libxc/include/xc_dom.h
@@ -219,37 +219,6 @@ struct xc_dom_image {
     struct xc_hvm_firmware_module smbios_module;
 };
 
-#if defined(__i386__) || defined(__x86_64__)
-/* C representation of the x86/HVM start info layout.
- *
- * The canonical definition of this layout resides in public/xen.h, this
- * is just a way to represent the layout described there using C types.
- *
- * NB: the packed attribute is not really needed, but it helps us enforce
- * the fact this this is just a representation, and it might indeed
- * be required in the future if there are alignment changes.
- */
-struct hvm_start_info {
-    uint32_t magic;             /* Contains the magic value 0x336ec578       */
-                                /* ("xEn3" with the 0x80 bit of the "E" set).*/
-    uint32_t version;           /* Version of this structure.                */
-    uint32_t flags;             /* SIF_xxx flags.                            */
-    uint32_t nr_modules;        /* Number of modules passed to the kernel.   */
-    uint64_t modlist_paddr;     /* Physical address of an array of           */
-                                /* hvm_modlist_entry.                        */
-    uint64_t cmdline_paddr;     /* Physical address of the command line.     */
-    uint64_t rsdp_paddr;        /* Physical address of the RSDP ACPI data    */
-                                /* structure.                                */
-} __attribute__((packed));
-
-struct hvm_modlist_entry {
-    uint64_t paddr;             /* Physical address of the module.           */
-    uint64_t size;              /* Size of the module in bytes.              */
-    uint64_t cmdline_paddr;     /* Physical address of the command line.     */
-    uint64_t reserved;
-} __attribute__((packed));
-#endif /* x86 */
-
 /* --- pluggable kernel loader ------------------------------------- */
 
 struct xc_dom_loader {
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index bc2dbb2..0eab8a7 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -32,6 +32,7 @@
 #include <xen/foreign/x86_32.h>
 #include <xen/foreign/x86_64.h>
 #include <xen/hvm/hvm_info_table.h>
+#include <xen/arch-x86/hvm/start_info.h>
 #include <xen/io/protocols.h>
 
 #include "xg_private.h"
diff --git a/xen/include/public/arch-x86/hvm/start_info.h b/xen/include/public/arch-x86/hvm/start_info.h
new file mode 100644
index 0000000..6981187
--- /dev/null
+++ b/xen/include/public/arch-x86/hvm/start_info.h
@@ -0,0 +1,53 @@
+/*
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright (c) 2016, Citrix Systems, Inc.
+ */
+
+#ifndef __XEN_PUBLIC_ARCH_X86_HVM_START_INFO_H__
+#define __XEN_PUBLIC_ARCH_X86_HVM_START_INFO_H__
+
+/*
+ * C representation of the x86/HVM start info layout.
+ *
+ * The canonical definition of this layout resides in public/xen.h, this
+ * is just a way to represent the layout described there using C types.
+ *
+ */
+struct hvm_start_info {
+    uint32_t magic;             /* Contains the magic value 0x336ec578       */
+                                /* ("xEn3" with the 0x80 bit of the "E" set).*/
+    uint32_t version;           /* Version of this structure.                */
+    uint32_t flags;             /* SIF_xxx flags.                            */
+    uint32_t nr_modules;        /* Number of modules passed to the kernel.   */
+    uint64_t modlist_paddr;     /* Physical address of an array of           */
+                                /* hvm_modlist_entry.                        */
+    uint64_t cmdline_paddr;     /* Physical address of the command line.     */
+    uint64_t rsdp_paddr;        /* Physical address of the RSDP ACPI data    */
+                                /* structure.                                */
+};
+
+struct hvm_modlist_entry {
+    uint64_t paddr;             /* Physical address of the module.           */
+    uint64_t size;              /* Size of the module in bytes.              */
+    uint64_t cmdline_paddr;     /* Physical address of the command line.     */
+    uint64_t reserved;
+};
+
+#endif /* __XEN_PUBLIC_ARCH_X86_HVM_START_INFO_H__ */
-- 
Anthony PERARD


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

[PATCH v6 07/14] hvmloader: Grab the hvm_start_info pointer

[Anthony PERARD]

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>

---
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 716d03c..77c70e0 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"
@@ -258,6 +263,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 3126817..9808016 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);
-- 
Anthony PERARD


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

[PATCH v6 08/14] hvmloader: Locate the BIOS blob

[Anthony PERARD]

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>

---
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 b838cf9..4c6d8ad 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 77c70e0..5c57ab3 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -254,10 +254,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);
@@ -293,8 +340,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 db9fa7a..858a2d4 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 1f15b94..2ded844 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;
-- 
Anthony PERARD


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

[PATCH v6 09/14] hvmloader: Check modules whereabouts in perform_tests

[Anthony PERARD]

As perform_tests() is going to clear memory past 4MB, we check that the
memory can be use or we skip the tests.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

---
Changes in V6:
- define and use TEST_START and PT_END.
- cast addresses to uintptr_t instead of uint32_t.
- use UINTPTR_MAX for upper limit checks, instead of UINT_MAX.
- fix typos
- include xen/arch-x86/hvm/start_info.h
- better check for the cmdlines, now check if a string would cross the
  4GB boundary.

Changes in V5:
- also account for the pages table
- fix coding style
- also check modules cmdline and main cmdline
  and modlist_paddr
- make use of check_overlap.

Changes in v4:
- move the check into the perform_test() function.
- skip tests instead of using BUG.

New in V3
---
 tools/firmware/hvmloader/tests.c | 76 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/tools/firmware/hvmloader/tests.c b/tools/firmware/hvmloader/tests.c
index fea3ad3..4b5cb93 100644
--- a/tools/firmware/hvmloader/tests.c
+++ b/tools/firmware/hvmloader/tests.c
@@ -20,6 +20,8 @@
  */
 
 #include "util.h"
+#include "config.h"
+#include <xen/arch-x86/hvm/start_info.h>
 
 #define TEST_FAIL 0
 #define TEST_PASS 1
@@ -32,8 +34,10 @@
  *  4 page table pages reside at 8MB+4kB to 8MB+20kB.
  *  Pagetables identity-map 0-16MB, except 4kB at va 6MB maps to pa 5MB.
  */
+#define TEST_START (4ul << 20)
 #define PD_START (8ul << 20)
 #define PT_START (PD_START + 4096)
+#define PT_END (PT_START + 4 * PAGE_SIZE)
 
 static void setup_paging(void)
 {
@@ -189,6 +193,37 @@ static int shadow_gs_test(void)
     return (ebx == 2) ? TEST_PASS : TEST_FAIL;
 }
 
+static bool check_test_overlap(uint64_t start, uint64_t size)
+{
+    if ( start )
+        return check_overlap(start, size,
+                             TEST_START,
+                             PT_END - TEST_START);
+    return false;
+}
+
+/* Only return true if the string overlap with the TEST_START,PT_END */
+static bool check_string_overlap_with_test(uint64_t paddr)
+{
+    unsigned len = 0;
+    const char *s;
+
+    if ( !paddr || paddr > UINTPTR_MAX )
+        return false;
+
+    s = (char *)(uintptr_t)paddr;
+    while ( *s && (uintptr_t)s < UINTPTR_MAX )
+        s++;
+
+    /* Not valid string, ignore it. */
+    if ( (uintptr_t)s == UINTPTR_MAX && *s )
+        return false;
+
+    len = (uintptr_t)s - paddr;
+
+    return check_test_overlap(paddr, len);
+}
+
 void perform_tests(void)
 {
     int i, passed, skipped;
@@ -210,11 +245,50 @@ void perform_tests(void)
         return;
     }
 
+    /* Check that tests does not use memory where modules are stored */
+    if ( check_test_overlap((uintptr_t)hvm_start_info,
+                            sizeof(*hvm_start_info)) )
+    {
+        printf("Skipping tests due to memory used by hvm_start_info\n");
+        return;
+    }
+    if ( check_test_overlap(hvm_start_info->modlist_paddr,
+                            hvm_start_info->nr_modules *
+                              sizeof(struct hvm_modlist_entry)) )
+    {
+        printf("Skipping tests due to memory used by"
+               " hvm_start_info->modlist\n");
+        return;
+    }
+    for ( i = 0; i < hvm_start_info->nr_modules; i++ )
+    {
+        const struct hvm_modlist_entry *modlist =
+            (void *)(uintptr_t)hvm_start_info->modlist_paddr;
+
+        if ( check_test_overlap(modlist[i].paddr, modlist[i].size) )
+        {
+            printf("Skipping tests due to memory used by module[%d]\n", i);
+            return;
+        }
+        if ( check_string_overlap_with_test(modlist[i].cmdline_paddr) )
+        {
+            printf("Skipping tests due to memory used by"
+                   " module[%d]'s cmdline\n", i);
+            return;
+        }
+    }
+    if ( check_string_overlap_with_test(hvm_start_info->cmdline_paddr) )
+    {
+        printf("Skipping tests due to memory used by the"
+               " hvm_start_info->cmdline\n");
+        return;
+    }
+
     passed = skipped = 0;
     for ( i = 0; tests[i].test; i++ )
     {
         printf(" - %s ... ", tests[i].description);
-        memset((char *)(4ul << 20), 0, 4ul << 20);
+        memset((char *)TEST_START, 0, 4ul << 20);
         setup_paging();
         switch ( (*tests[i].test)() )
         {
-- 
Anthony PERARD


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

[PATCH v6 10/14] hvmloader: Load SeaBIOS from hvm_start_info modules

[Anthony PERARD]

... 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>

---
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 c6b3d9f..c5d2b34 100644
--- a/tools/firmware/hvmloader/seabios.c
+++ b/tools/firmware/hvmloader/seabios.c
@@ -27,9 +27,6 @@
 #include "smbios_types.h"
 #include "acpi/acpi2_0.h"
 
-#define ROM_INCLUDE_SEABIOS
-#include "roms.inc"
-
 extern unsigned char dsdt_anycpu_qemu_xen[];
 extern int dsdt_anycpu_qemu_xen_len;
 
@@ -127,22 +124,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,
-- 
Anthony PERARD


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

[PATCH v6 11/14] hvmloader: Load OVMF from modules

[Anthony PERARD]

... 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>

---
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 858a2d4..59d0e45 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,
-- 
Anthony PERARD


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

[PATCH v6 12/14] hvmloader: bios->bios_load() now needs to be defined

[Anthony PERARD]

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 5c57ab3..b5f51f6 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -341,22 +341,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 )
-- 
Anthony PERARD


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

[PATCH v6 13/14] hvmloader: Always build-in SeaBIOS and OVMF loader

[Anthony PERARD]

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 b5f51f6..61875a0 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -210,12 +210,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 }
 };
 
-- 
Anthony PERARD


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

[PATCH v6 14/14] configure: do not depend on SEABIOS_PATH or OVMF_PATH ...

[Anthony PERARD]

... 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.ac      | 6 ++++--
 tools/firmware/Makefile | 8 --------
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/tools/configure.ac b/tools/configure.ac
index af3b6e9..0229d44 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -214,12 +214,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])
@@ -228,12 +229,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
-- 
Anthony PERARD


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

Re: [PATCH v6 06/14] xen: Move the hvm_start_info C representation from libxc to public/xen.h

[Wei Liu]

On Tue, Jul 12, 2016 at 03:42:43PM +0100, Anthony PERARD wrote:
> Instead of having several representation of hvm_start_info in C, define
> it in public/xen.h so both libxc and hvmloader can use it.
> 

Commit message is wrong. I think this can be easily handled during
committing though.

Now we need an ack from Andrew or Jan.

Wei.

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

Re: [PATCH v6 01/14] libxc: Rework extra module initialisation

[Wei Liu]

On Tue, Jul 12, 2016 at 03:42:38PM +0100, Anthony PERARD wrote:
> 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>

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

Re: [PATCH v6 02/14] libxc: Prepare a start info structure for hvmloader

[Wei Liu]

On Tue, Jul 12, 2016 at 03:42:39PM +0100, Anthony PERARD wrote:
> ... 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>

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

Re: [PATCH v6 05/14] libxl: Load guest BIOS from file

[Wei Liu]

On Tue, Jul 12, 2016 at 03:42:42PM +0100, Anthony PERARD wrote:
> 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>

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

Re: [PATCH v6 00/14] Load BIOS via toolstack instead of been embedded in hvmloader.

[Wei Liu]

I believe we only need a mechanical ack from Andrew and Jan for patch 6.
I'm happy to take care of this series once everything is ready.

Wei.

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

Re: [PATCH v6 00/14] Load BIOS via toolstack instead of been embedded in hvmloader.

[Wei Liu]

On Tue, Jul 12, 2016 at 04:03:21PM +0100, Wei Liu wrote:
> I believe we only need a mechanical ack from Andrew and Jan for patch 6.

Spoke too soon. Patch 8 and 9 would also need acks.

> I'm happy to take care of this series once everything is ready.
> 

This stays the same. :-)

Wei.

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

Re: [PATCH v6 06/14] xen: Move the hvm_start_info C representation from libxc to public/xen.h

[Andrew Cooper]

On 12/07/16 15:42, Anthony PERARD wrote:
> +#ifndef __XEN_PUBLIC_ARCH_X86_HVM_START_INFO_H__
> +#define __XEN_PUBLIC_ARCH_X86_HVM_START_INFO_H__
> +
> +/*
> + * C representation of the x86/HVM start info layout.
> + *
> + * The canonical definition of this layout resides in public/xen.h, this

You should also move the big comment block from public/xen.h to here,
along with the XEN_HVM_START_MAGIC_VALUE define.

There is no point having it split across two locations in the public
headers.

~Andrew

> + * is just a way to represent the layout described there using C types.
> + *
> + */
> +struct hvm_start_info {
> +    uint32_t magic;             /* Contains the magic value 0x336ec578       */
> +                                /* ("xEn3" with the 0x80 bit of the "E" set).*/
> +    uint32_t version;           /* Version of this structure.                */
> +    uint32_t flags;             /* SIF_xxx flags.                            */
> +    uint32_t nr_modules;        /* Number of modules passed to the kernel.   */
> +    uint64_t modlist_paddr;     /* Physical address of an array of           */
> +                                /* hvm_modlist_entry.                        */
> +    uint64_t cmdline_paddr;     /* Physical address of the command line.     */
> +    uint64_t rsdp_paddr;        /* Physical address of the RSDP ACPI data    */
> +                                /* structure.                                */
> +};
> +
> +struct hvm_modlist_entry {
> +    uint64_t paddr;             /* Physical address of the module.           */
> +    uint64_t size;              /* Size of the module in bytes.              */
> +    uint64_t cmdline_paddr;     /* Physical address of the command line.     */
> +    uint64_t reserved;
> +};
> +
> +#endif /* __XEN_PUBLIC_ARCH_X86_HVM_START_INFO_H__ */


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

Re: [PATCH v6 06/14] xen: Move the hvm_start_info C representation from libxc to public/xen.h

[Anthony PERARD]

On Tue, Jul 12, 2016 at 04:09:59PM +0100, Andrew Cooper wrote:
> On 12/07/16 15:42, Anthony PERARD wrote:
> > +#ifndef __XEN_PUBLIC_ARCH_X86_HVM_START_INFO_H__
> > +#define __XEN_PUBLIC_ARCH_X86_HVM_START_INFO_H__
> > +
> > +/*
> > + * C representation of the x86/HVM start info layout.
> > + *
> > + * The canonical definition of this layout resides in public/xen.h, this
> 
> You should also move the big comment block from public/xen.h to here,
> along with the XEN_HVM_START_MAGIC_VALUE define.

Is it fine to move the comment and the define even if there has been one
release of Xen with this in xen.h?

> There is no point having it split across two locations in the public
> headers.

-- 
Anthony PERARD

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

Re: [PATCH v6 06/14] xen: Move the hvm_start_info C representation from libxc to public/xen.h

[Andrew Cooper]

On 12/07/16 16:36, Anthony PERARD wrote:
> On Tue, Jul 12, 2016 at 04:09:59PM +0100, Andrew Cooper wrote:
>> On 12/07/16 15:42, Anthony PERARD wrote:
>>> +#ifndef __XEN_PUBLIC_ARCH_X86_HVM_START_INFO_H__
>>> +#define __XEN_PUBLIC_ARCH_X86_HVM_START_INFO_H__
>>> +
>>> +/*
>>> + * C representation of the x86/HVM start info layout.
>>> + *
>>> + * The canonical definition of this layout resides in public/xen.h, this
>> You should also move the big comment block from public/xen.h to here,
>> along with the XEN_HVM_START_MAGIC_VALUE define.
> Is it fine to move the comment and the define even if there has been one
> release of Xen with this in xen.h?

The comment, absolutely.  It is just a comment.

The define is more tricky to argue.

We currently expect people to copy&paste the public header files into
their own project, rather than linking to them, *and* insist on
maintaining API compatibility with further #ifdef'ary obfuscating the
structures and names.

This status-quo is ludicrous and needs to stop.

The chances of any out-of-tree users using XEN_HVM_START_MAGIC_VALUE is
minimal, and even if not 0, will be from their own local copy.

The chances of anyone wanting XEN_HVM_START_MAGIC_VALUE without the rest
of this new file is 0.


So I am going to go out on a limb and say yes to moving the define. 
Noone is going to notice or care, and we won't break anyone’s code by
doing so.

~Andrew

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

Re: [PATCH v6 06/14] xen: Move the hvm_start_info C representation from libxc to public/xen.h

[Anthony PERARD]

On Tue, Jul 12, 2016 at 05:07:35PM +0100, Andrew Cooper wrote:
> On 12/07/16 16:36, Anthony PERARD wrote:
> > On Tue, Jul 12, 2016 at 04:09:59PM +0100, Andrew Cooper wrote:
> >> On 12/07/16 15:42, Anthony PERARD wrote:
> >>> +#ifndef __XEN_PUBLIC_ARCH_X86_HVM_START_INFO_H__
> >>> +#define __XEN_PUBLIC_ARCH_X86_HVM_START_INFO_H__
> >>> +
> >>> +/*
> >>> + * C representation of the x86/HVM start info layout.
> >>> + *
> >>> + * The canonical definition of this layout resides in public/xen.h, this
> >> You should also move the big comment block from public/xen.h to here,
> >> along with the XEN_HVM_START_MAGIC_VALUE define.
> > Is it fine to move the comment and the define even if there has been one
> > release of Xen with this in xen.h?
> 
> The comment, absolutely.  It is just a comment.
> 
> The define is more tricky to argue.
> 
> We currently expect people to copy&paste the public header files into
> their own project, rather than linking to them, *and* insist on
> maintaining API compatibility with further #ifdef'ary obfuscating the
> structures and names.
> 
> This status-quo is ludicrous and needs to stop.
> 
> The chances of any out-of-tree users using XEN_HVM_START_MAGIC_VALUE is
> minimal, and even if not 0, will be from their own local copy.
> 
> The chances of anyone wanting XEN_HVM_START_MAGIC_VALUE without the rest
> of this new file is 0.
> 
> 
> So I am going to go out on a limb and say yes to moving the define. 
> Noone is going to notice or care, and we won't break anyone’s code by
> doing so.

Ok, I'll move everything, then.

Thanks,

-- 
Anthony PERARD

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