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

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

[Anthony PERARD]

Hi all,

Changes in V7:
  - There is one new patch at the end to fix the doc.
  - Patch 6 as been change.
  that's it.

  There is just a few missing ackes:
    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
   15 docs/misc/hvmlite: Point to the canonical definition of hvm_start_info

Thanks.

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

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 (15):
  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 to the public headers
  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/misc/hvmlite: Point to the canonical definition of hvm_start_info

 docs/man/xl.cfg.pod.5.in                     |   9 ++
 docs/misc/hvmlite.markdown                   |  20 +---
 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 |  99 +++++++++++++++++
 xen/include/public/xen.h                     |  46 --------
 23 files changed, 558 insertions(+), 318 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 v7 01/15] 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>
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__);
-- 
Anthony PERARD


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

[PATCH v7 02/15] 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>
Acked-by: Wei Liu <wei.liu2@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 v7 03/15] 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 v7 04/15] 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 v7 05/15] 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>
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 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 48a43ce..5cccdc8 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -942,6 +942,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 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 5347b69..991a1cc 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2320,6 +2320,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 v7 06/15] xen: Move the hvm_start_info C representation to the public headers

[Anthony PERARD]

Instead of having several representation of hvm_start_info in C, define
it in public/arch-x86/hvm/start_info.h so both libxc and hvmloader can
use it.

Also move the comment describing the binary format to be alongside the
C struct.

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

---
Changes in V7:
- also move the comment and define into arch-x86/hvm/start_info.h.

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 | 99 ++++++++++++++++++++++++++++
 xen/include/public/xen.h                     | 46 -------------
 4 files changed, 100 insertions(+), 77 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..002e3b7
--- /dev/null
+++ b/xen/include/public/arch-x86/hvm/start_info.h
@@ -0,0 +1,99 @@
+/*
+ * 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__
+
+/*
+ * 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.
+ *
+ *  0 +----------------+
+ *    | magic          | Contains the magic value XEN_HVM_START_MAGIC_VALUE
+ *    |                | ("xEn3" with the 0x80 bit of the "E" set).
+ *  4 +----------------+
+ *    | version        | Version of this structure. Current version is 0. New
+ *    |                | versions are guaranteed to be backwards-compatible.
+ *  8 +----------------+
+ *    | flags          | SIF_xxx flags.
+ * 12 +----------------+
+ *    | nr_modules     | Number of modules passed to the kernel.
+ * 16 +----------------+
+ *    | modlist_paddr  | Physical address of an array of modules
+ *    |                | (layout of the structure below).
+ * 24 +----------------+
+ *    | cmdline_paddr  | Physical address of the command line,
+ *    |                | a zero-terminated ASCII string.
+ * 32 +----------------+
+ *    | rsdp_paddr     | Physical address of the RSDP ACPI data structure.
+ * 40 +----------------+
+ *
+ * The layout of each entry in the module structure is the following:
+ *
+ *  0 +----------------+
+ *    | paddr          | Physical address of the module.
+ *  8 +----------------+
+ *    | size           | Size of the module in bytes.
+ * 16 +----------------+
+ *    | cmdline_paddr  | Physical address of the command line,
+ *    |                | a zero-terminated ASCII string.
+ * 24 +----------------+
+ *    | reserved       |
+ * 32 +----------------+
+ *
+ * The address and sizes are always a 64bit little endian unsigned integer.
+ *
+ * NB: Xen on x86 will always try to place all the data below the 4GiB
+ * boundary.
+ */
+#define XEN_HVM_START_MAGIC_VALUE 0x336ec578
+
+/*
+ * C representation of the x86/HVM start info layout.
+ *
+ * The canonical definition of this layout is abrove, 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__ */
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index 427e74f..41e415f 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -820,52 +820,6 @@ struct start_info {
 };
 typedef struct start_info start_info_t;
 
-/*
- * 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.
- *
- *  0 +----------------+
- *    | magic          | Contains the magic value XEN_HVM_START_MAGIC_VALUE
- *    |                | ("xEn3" with the 0x80 bit of the "E" set).
- *  4 +----------------+
- *    | version        | Version of this structure. Current version is 0. New
- *    |                | versions are guaranteed to be backwards-compatible.
- *  8 +----------------+
- *    | flags          | SIF_xxx flags.
- * 12 +----------------+
- *    | nr_modules     | Number of modules passed to the kernel.
- * 16 +----------------+
- *    | modlist_paddr  | Physical address of an array of modules
- *    |                | (layout of the structure below).
- * 24 +----------------+
- *    | cmdline_paddr  | Physical address of the command line,
- *    |                | a zero-terminated ASCII string.
- * 32 +----------------+
- *    | rsdp_paddr     | Physical address of the RSDP ACPI data structure.
- * 40 +----------------+
- *
- * The layout of each entry in the module structure is the following:
- *
- *  0 +----------------+
- *    | paddr          | Physical address of the module.
- *  8 +----------------+
- *    | size           | Size of the module in bytes.
- * 16 +----------------+
- *    | cmdline_paddr  | Physical address of the command line,
- *    |                | a zero-terminated ASCII string.
- * 24 +----------------+
- *    | reserved       |
- * 32 +----------------+
- *
- * The address and sizes are always a 64bit little endian unsigned integer.
- *
- * NB: Xen on x86 will always try to place all the data below the 4GiB
- * boundary.
- */
-#define XEN_HVM_START_MAGIC_VALUE 0x336ec578
-
 /* New console union for dom0 introduced in 0x00030203. */
 #if __XEN_INTERFACE_VERSION__ < 0x00030203
 #define console_mfn    console.domU.mfn
-- 
Anthony PERARD


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

[PATCH v7 07/15] 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 v7 08/15] 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 v7 09/15] 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 v7 10/15] 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 v7 11/15] 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 v7 12/15] 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 v7 13/15] 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 v7 14/15] 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

[PATCH v7 15/15] docs/misc/hvmlite: Point to the canonical definition of hvm_start_info

[Anthony PERARD]

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>
---
CC: Roger Pau Monne <roger.pau@citrix.com>

New patch in V7.
---
 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
-- 
Anthony PERARD


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

Re: [PATCH v7 06/15] xen: Move the hvm_start_info C representation to the public headers

[Andrew Cooper]

On 28/07/16 11:50, Anthony PERARD wrote:
> Instead of having several representation of hvm_start_info in C, define
> it in public/arch-x86/hvm/start_info.h so both libxc and hvmloader can
> use it.
>
> Also move the comment describing the binary format to be alongside the
> C struct.
>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v7 08/15] hvmloader: Locate the BIOS blob

[Andrew Cooper]

On 28/07/16 11:50, Anthony PERARD wrote:
> @@ -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);

This is an unnecessary change in behaviour.  Currently, 'bios' is never
NULL, and would never pass bogus information to bios_load.

As this is the new way of providing firmware, it should be a hard error
if get_module_entry(, "firmware") fails, at which point this logic can
collapse down quite a lot.

~Andrew

> +    }
>      else
>      {
>          BUG_ON(bios->bios_address + bios->image_size >
>


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

Re: [PATCH v7 09/15] hvmloader: Check modules whereabouts in perform_tests

[Andrew Cooper]

On 28/07/16 11:50, Anthony PERARD wrote:
> 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>

This is a loosing battle of overlap checks, and they are far less useful
than they used to be if they are conditionally skipped.

I would just drop tests.c entirely.  This was one longterm goal of mine
with XTF, and it is a far more appropriate way to get tests done,
especially as it is almost integrated into OSSTest now.

~Andrew

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

Re: [PATCH v7 10/15] hvmloader: Load SeaBIOS from hvm_start_info modules

[Andrew Cooper]

On 28/07/16 11:50, Anthony PERARD wrote:
> ... 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>

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

Re: [PATCH v7 07/15] hvmloader: Grab the hvm_start_info pointer

[Andrew Cooper]

[-- Attachment #1.1: Type: text/plain, Size: 263 bytes --]

On 28/07/16 11:50, Anthony PERARD wrote:
> 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>

[-- Attachment #1.2: Type: text/html, Size: 1032 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [PATCH v7 11/15] hvmloader: Load OVMF from modules

[Andrew Cooper]

On 28/07/16 11:50, Anthony PERARD wrote:
> ... 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>

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

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

[Boris Ostrovsky]

On 07/28/2016 06:49 AM, Anthony PERARD wrote:
> Hi all,
>
> Changes in V7:
>    - There is one new patch at the end to fix the doc.
>    - Patch 6 as been change.
>    that's it.
>
>    There is just a few missing ackes:
>      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
>     15 docs/misc/hvmlite: Point to the canonical definition of hvm_start_info
>
> Thanks.
>
> A git tree can be found here:
> git://xenbits.xen.org/people/aperard/xen-unstable.git
> tag: hvmloader-with-separated-bios-v7


I am unable to build this:

libxl_paths.c: In function ‘libxl__seabios_path’:
libxl_paths.c:40: error: ‘SEABIOS_PATH’ undeclared (first use in this 
function)
libxl_paths.c:40: error: (Each undeclared identifier is reported only once
libxl_paths.c:40: error: for each function it appears in.)
libxl_paths.c: In function ‘libxl__ovmf_path’:
libxl_paths.c:45: error: ‘OVMF_PATH’ undeclared (first use in this function)

IIUIC these two are supposed to be generated into tools/config.h but 
they were not for me. I haven't looked any further yet.

-boris



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

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

[Wei Liu]

On Fri, Jul 29, 2016 at 01:28:02AM -0400, Boris Ostrovsky wrote:
> 
> 
> On 07/28/2016 06:49 AM, Anthony PERARD wrote:
> >Hi all,
> >
> >Changes in V7:
> >   - There is one new patch at the end to fix the doc.
> >   - Patch 6 as been change.
> >   that's it.
> >
> >   There is just a few missing ackes:
> >     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
> >    15 docs/misc/hvmlite: Point to the canonical definition of hvm_start_info
> >
> >Thanks.
> >
> >A git tree can be found here:
> >git://xenbits.xen.org/people/aperard/xen-unstable.git
> >tag: hvmloader-with-separated-bios-v7
> 
> 
> I am unable to build this:
> 
> libxl_paths.c: In function ‘libxl__seabios_path’:
> libxl_paths.c:40: error: ‘SEABIOS_PATH’ undeclared (first use in this
> function)
> libxl_paths.c:40: error: (Each undeclared identifier is reported only once
> libxl_paths.c:40: error: for each function it appears in.)
> libxl_paths.c: In function ‘libxl__ovmf_path’:
> libxl_paths.c:45: error: ‘OVMF_PATH’ undeclared (first use in this function)
> 
> IIUIC these two are supposed to be generated into tools/config.h but they
> were not for me. I haven't looked any further yet.
> 
> -boris
> 

You need to run ./autogen.sh. Anthony didn't commit the changes to
./configure.

Wei.

> 

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

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

[Boris Ostrovsky]

On 07/29/2016 04:29 AM, Wei Liu wrote:
> On Fri, Jul 29, 2016 at 01:28:02AM -0400, Boris Ostrovsky wrote:
>>
>> On 07/28/2016 06:49 AM, Anthony PERARD wrote:
>>> Hi all,
>>>
>>> Changes in V7:
>>>   - There is one new patch at the end to fix the doc.
>>>   - Patch 6 as been change.
>>>   that's it.
>>>
>>>   There is just a few missing ackes:
>>>     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
>>>    15 docs/misc/hvmlite: Point to the canonical definition of hvm_start_info
>>>
>>> Thanks.
>>>
>>> A git tree can be found here:
>>> git://xenbits.xen.org/people/aperard/xen-unstable.git
>>> tag: hvmloader-with-separated-bios-v7
>>
>> I am unable to build this:
>>
>> libxl_paths.c: In function ‘libxl__seabios_path’:
>> libxl_paths.c:40: error: ‘SEABIOS_PATH’ undeclared (first use in this
>> function)
>> libxl_paths.c:40: error: (Each undeclared identifier is reported only once
>> libxl_paths.c:40: error: for each function it appears in.)
>> libxl_paths.c: In function ‘libxl__ovmf_path’:
>> libxl_paths.c:45: error: ‘OVMF_PATH’ undeclared (first use in this function)
>>
>> IIUIC these two are supposed to be generated into tools/config.h but they
>> were not for me. I haven't looked any further yet.
>>
>> -boris
>>
> You need to run ./autogen.sh. Anthony didn't commit the changes to
> ./configure.


Yes, that did the trick. Except for the machine on which I actually
wanted it to run:

root@ovs104> ./autogen.sh
configure.ac:4: error: Autoconf version 2.67 or higher is required
configure.ac:4: the top level
autom4te: /usr/bin/m4 failed with exit status: 63
root@ovs104> autoconf --version
autoconf (GNU Autoconf) 2.63
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv2+: GNU GPL version 2 or later
<http://gnu.org/licenses/old-licenses/gpl-2.0.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by David J. MacKenzie and Akim Demaille.
root@ovs104>

Is 2.67 really a pre-req?

-boris


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

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

[Wei Liu]

On Fri, Jul 29, 2016 at 10:36:02AM -0400, Boris Ostrovsky wrote:
> On 07/29/2016 04:29 AM, Wei Liu wrote:
> > On Fri, Jul 29, 2016 at 01:28:02AM -0400, Boris Ostrovsky wrote:
> >>
> >> On 07/28/2016 06:49 AM, Anthony PERARD wrote:
> >>> Hi all,
> >>>
> >>> Changes in V7:
> >>>   - There is one new patch at the end to fix the doc.
> >>>   - Patch 6 as been change.
> >>>   that's it.
> >>>
> >>>   There is just a few missing ackes:
> >>>     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
> >>>    15 docs/misc/hvmlite: Point to the canonical definition of hvm_start_info
> >>>
> >>> Thanks.
> >>>
> >>> A git tree can be found here:
> >>> git://xenbits.xen.org/people/aperard/xen-unstable.git
> >>> tag: hvmloader-with-separated-bios-v7
> >>
> >> I am unable to build this:
> >>
> >> libxl_paths.c: In function ‘libxl__seabios_path’:
> >> libxl_paths.c:40: error: ‘SEABIOS_PATH’ undeclared (first use in this
> >> function)
> >> libxl_paths.c:40: error: (Each undeclared identifier is reported only once
> >> libxl_paths.c:40: error: for each function it appears in.)
> >> libxl_paths.c: In function ‘libxl__ovmf_path’:
> >> libxl_paths.c:45: error: ‘OVMF_PATH’ undeclared (first use in this function)
> >>
> >> IIUIC these two are supposed to be generated into tools/config.h but they
> >> were not for me. I haven't looked any further yet.
> >>
> >> -boris
> >>
> > You need to run ./autogen.sh. Anthony didn't commit the changes to
> > ./configure.
> 
> 
> Yes, that did the trick. Except for the machine on which I actually
> wanted it to run:
> 
> root@ovs104> ./autogen.sh
> configure.ac:4: error: Autoconf version 2.67 or higher is required
> configure.ac:4: the top level
> autom4te: /usr/bin/m4 failed with exit status: 63
> root@ovs104> autoconf --version
> autoconf (GNU Autoconf) 2.63
> Copyright (C) 2008 Free Software Foundation, Inc.
> License GPLv2+: GNU GPL version 2 or later
> <http://gnu.org/licenses/old-licenses/gpl-2.0.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
> 
> Written by David J. MacKenzie and Akim Demaille.
> root@ovs104>
> 
> Is 2.67 really a pre-req?
> 

It is not written down. But I guess there is a reason for it.

Do you need me or Anthony to generate that for you?

Wei.

> -boris
> 

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

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

[Boris Ostrovsky]

On 07/29/2016 10:50 AM, Wei Liu wrote:
>
>>> You need to run ./autogen.sh. Anthony didn't commit the changes to
>>> ./configure.
>>
>> Yes, that did the trick. Except for the machine on which I actually
>> wanted it to run:
>>
>> root@ovs104> ./autogen.sh
>> configure.ac:4: error: Autoconf version 2.67 or higher is required
>> configure.ac:4: the top level
>> autom4te: /usr/bin/m4 failed with exit status: 63
>> root@ovs104> autoconf --version
>> autoconf (GNU Autoconf) 2.63
>> Copyright (C) 2008 Free Software Foundation, Inc.
>> License GPLv2+: GNU GPL version 2 or later
>> <http://gnu.org/licenses/old-licenses/gpl-2.0.html>
>> This is free software: you are free to change and redistribute it.
>> There is NO WARRANTY, to the extent permitted by law.
>>
>> Written by David J. MacKenzie and Akim Demaille.
>> root@ovs104>
>>
>> Is 2.67 really a pre-req?
>>
> It is not written down. But I guess there is a reason for it.
>
> Do you need me or Anthony to generate that for you?

No, I am good --- I can build on a system with software from this decade.

-boris


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

Re: [PATCH v7 15/15] docs/misc/hvmlite: Point to the canonical definition of hvm_start_info

[Jan Beulich]

>>> On 28.07.16 at 12:50, <anthony.perard@citrix.com> wrote:
> 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>


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

Re: [PATCH v7 08/15] hvmloader: Locate the BIOS blob

[Anthony PERARD]

On Thu, Jul 28, 2016 at 02:44:24PM +0100, Andrew Cooper wrote:
> On 28/07/16 11:50, Anthony PERARD wrote:
> > @@ -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);
> 
> This is an unnecessary change in behaviour.  Currently, 'bios' is never
> NULL, and would never pass bogus information to bios_load.
> 
> As this is the new way of providing firmware, it should be a hard error
> if get_module_entry(, "firmware") fails, at which point this logic can
> collapse down quite a lot.

At this point in the patch series, the module is not used yet, and the
seabios loader does not have a bios_load function. Also I've change the
logic again in "hvmloader: bios->bios_load() now needs to be defined".

Also, I've left ROMBIOS embedded in hvmloader, because it comes with
VGABIOS and Etherboot, so it would be a bit more complicated.

-- 
Anthony PERARD

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

Re: [PATCH v7 09/15] hvmloader: Check modules whereabouts in perform_tests

[Anthony PERARD]

On Thu, Jul 28, 2016 at 03:08:29PM +0100, Andrew Cooper wrote:
> On 28/07/16 11:50, Anthony PERARD wrote:
> > 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>
> 
> This is a loosing battle of overlap checks, and they are far less useful
> than they used to be if they are conditionally skipped.

The tests may be skipped if OVMF is used as firmware, otherwise it does
not really happen.

> I would just drop tests.c entirely.  This was one longterm goal of mine
> with XTF, and it is a far more appropriate way to get tests done,
> especially as it is almost integrated into OSSTest now.

I guess I can drop tests.c and this patch if I have to send another
version of the patch series.

-- 
Anthony PERARD

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

Re: [PATCH v7 06/15] xen: Move the hvm_start_info C representation to the public headers

[Jan Beulich]

>>> On 28.07.16 at 12:50, <anthony.perard@citrix.com> wrote:
> Instead of having several representation of hvm_start_info in C, define
> it in public/arch-x86/hvm/start_info.h so both libxc and hvmloader can
> use it.
> 
> Also move the comment describing the binary format to be alongside the
> C struct.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

with a couple of cosmetic notes (which could be addressed while
committing if no other need for re-sending arises):

> +/*
> + * C representation of the x86/HVM start info layout.
> + *
> + * The canonical definition of this layout is abrove, this is just a way to

above

> + * represent the layout described there using C types.
> + *

Stray blank comment line.

Jan


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

Re: [PATCH v7 05/15] libxl: Load guest BIOS from file

[Shannon Zhao]

On 2016/7/28 18:50, 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>
> 
> ---
> 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 48a43ce..5cccdc8 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -942,6 +942,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 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 5347b69..991a1cc 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -2320,6 +2320,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;
> +}
> +
Hi, I got bellow errors when compiling on ARM.

libxl_paths.c: In function 'libxl__seabios_path':
libxl_paths.c:40:12: error: 'SEABIOS_PATH' undeclared (first use in this
function)
     return SEABIOS_PATH;
            ^
libxl_paths.c:40:12: note: each undeclared identifier is reported only
once for each function it appears in
libxl_paths.c: In function 'libxl__ovmf_path':
libxl_paths.c:45:12: error: 'OVMF_PATH' undeclared (first use in this
function)
     return OVMF_PATH;
            ^
libxl_paths.c: In function 'libxl__seabios_path':
libxl_paths.c:41:1: error: control reaches end of non-void function
[-Werror=return-type]
 }
 ^
libxl_paths.c: In function 'libxl__ovmf_path':
libxl_paths.c:46:1: error: control reaches end of non-void function
[-Werror=return-type]
 }
 ^
cc1: all warnings being treated as errors

Thanks,
-- 
Shannon


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

Re: [PATCH v7 05/15] libxl: Load guest BIOS from file

[Wei Liu]

On Mon, Aug 15, 2016 at 05:04:20PM +0800, Shannon Zhao wrote:
[...]
> >  
> > +const char *libxl__seabios_path(void)
> > +{
> > +    return SEABIOS_PATH;
> > +}
> > +
> > +const char *libxl__ovmf_path(void)
> > +{
> > +    return OVMF_PATH;
> > +}
> > +
> Hi, I got bellow errors when compiling on ARM.
> 
> libxl_paths.c: In function 'libxl__seabios_path':
> libxl_paths.c:40:12: error: 'SEABIOS_PATH' undeclared (first use in this
> function)
>      return SEABIOS_PATH;
>             ^
> libxl_paths.c:40:12: note: each undeclared identifier is reported only
> once for each function it appears in
> libxl_paths.c: In function 'libxl__ovmf_path':
> libxl_paths.c:45:12: error: 'OVMF_PATH' undeclared (first use in this
> function)
>      return OVMF_PATH;
>             ^
> libxl_paths.c: In function 'libxl__seabios_path':
> libxl_paths.c:41:1: error: control reaches end of non-void function
> [-Werror=return-type]
>  }
>  ^
> libxl_paths.c: In function 'libxl__ovmf_path':
> libxl_paths.c:46:1: error: control reaches end of non-void function
> [-Werror=return-type]
>  }
>  ^
> cc1: all warnings being treated as errors
> 

Have you tried to run ./autogen.sh in xen.git top-level directory?

Wei.

> Thanks,
> -- 
> Shannon
> 

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

Re: [PATCH v7 05/15] libxl: Load guest BIOS from file

[Shannon Zhao]

On 2016/8/15 17:09, Wei Liu wrote:
> On Mon, Aug 15, 2016 at 05:04:20PM +0800, Shannon Zhao wrote:
> [...]
>>> > >  
>>> > > +const char *libxl__seabios_path(void)
>>> > > +{
>>> > > +    return SEABIOS_PATH;
>>> > > +}
>>> > > +
>>> > > +const char *libxl__ovmf_path(void)
>>> > > +{
>>> > > +    return OVMF_PATH;
>>> > > +}
>>> > > +
>> > Hi, I got bellow errors when compiling on ARM.
>> > 
>> > libxl_paths.c: In function 'libxl__seabios_path':
>> > libxl_paths.c:40:12: error: 'SEABIOS_PATH' undeclared (first use in this
>> > function)
>> >      return SEABIOS_PATH;
>> >             ^
>> > libxl_paths.c:40:12: note: each undeclared identifier is reported only
>> > once for each function it appears in
>> > libxl_paths.c: In function 'libxl__ovmf_path':
>> > libxl_paths.c:45:12: error: 'OVMF_PATH' undeclared (first use in this
>> > function)
>> >      return OVMF_PATH;
>> >             ^
>> > libxl_paths.c: In function 'libxl__seabios_path':
>> > libxl_paths.c:41:1: error: control reaches end of non-void function
>> > [-Werror=return-type]
>> >  }
>> >  ^
>> > libxl_paths.c: In function 'libxl__ovmf_path':
>> > libxl_paths.c:46:1: error: control reaches end of non-void function
>> > [-Werror=return-type]
>> >  }
>> >  ^
>> > cc1: all warnings being treated as errors
>> > 
> Have you tried to run ./autogen.sh in xen.git top-level directory?
Eh, I only ran ./configure and make dist-tools as before. ./autogen.sh
makes it work.

Thanks,
-- 
Shannon


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

Re: [PATCH v7 05/15] libxl: Load guest BIOS from file

[Wei Liu]

On Mon, Aug 15, 2016 at 05:14:43PM +0800, Shannon Zhao wrote:
> 
> 
> On 2016/8/15 17:09, Wei Liu wrote:
> > On Mon, Aug 15, 2016 at 05:04:20PM +0800, Shannon Zhao wrote:
> > [...]
> >>> > >  
> >>> > > +const char *libxl__seabios_path(void)
> >>> > > +{
> >>> > > +    return SEABIOS_PATH;
> >>> > > +}
> >>> > > +
> >>> > > +const char *libxl__ovmf_path(void)
> >>> > > +{
> >>> > > +    return OVMF_PATH;
> >>> > > +}
> >>> > > +
> >> > Hi, I got bellow errors when compiling on ARM.
> >> > 
> >> > libxl_paths.c: In function 'libxl__seabios_path':
> >> > libxl_paths.c:40:12: error: 'SEABIOS_PATH' undeclared (first use in this
> >> > function)
> >> >      return SEABIOS_PATH;
> >> >             ^
> >> > libxl_paths.c:40:12: note: each undeclared identifier is reported only
> >> > once for each function it appears in
> >> > libxl_paths.c: In function 'libxl__ovmf_path':
> >> > libxl_paths.c:45:12: error: 'OVMF_PATH' undeclared (first use in this
> >> > function)
> >> >      return OVMF_PATH;
> >> >             ^
> >> > libxl_paths.c: In function 'libxl__seabios_path':
> >> > libxl_paths.c:41:1: error: control reaches end of non-void function
> >> > [-Werror=return-type]
> >> >  }
> >> >  ^
> >> > libxl_paths.c: In function 'libxl__ovmf_path':
> >> > libxl_paths.c:46:1: error: control reaches end of non-void function
> >> > [-Werror=return-type]
> >> >  }
> >> >  ^
> >> > cc1: all warnings being treated as errors
> >> > 
> > Have you tried to run ./autogen.sh in xen.git top-level directory?
> Eh, I only ran ./configure and make dist-tools as before. ./autogen.sh
> makes it work.
> 

Good to hear that solved your problem.

Anthony didn't commit the changes to configure that's why you saw those
errors.

Wei.

> Thanks,
> -- 
> Shannon
> 

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