Re: [Xen-devel] [RFC PATCH v2 02/16] libxc: Load BIOS and ACPI table into guest memory

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Mon, 2015-10-26 at 16:03 +0000, Anthony PERARD wrote:

At first it seems like not loading the ACPI table already was an unnoticed
functional regression from the switch to HVM using the regular PV domain
builder. But then looking at that code I see there is an acpi_module field
which Roger added to the same xc_dom_image struct and appears to have added
code to support.

So I guess I am confused about how what you are adding here differs from
that, which at the least requires discussion in the commit message, but it
seems like either one or the other is misleadingly named now or they should
somehow be combined.

Ian.

> ... and prepare a cmdline for hvmloader with the order of the modules.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
> Âtools/libxc/include/xc_dom.hÂÂÂ|ÂÂ4 ++
> Âtools/libxc/xc_dom_hvmloader.c | 44 +++++++++++++++++----
> Âtools/libxc/xc_dom_x86.cÂÂÂÂÂÂÂ| 90
> ++++++++++++++++++++++++++++++++++++++++++
> Â3 files changed, 130 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> index 4939f76..c7003a4 100644
> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -203,6 +203,10 @@ struct xc_dom_image {
> Â
> ÂÂÂÂÂ/* Extra SMBIOS structures passed to HVMLOADER */
> ÂÂÂÂÂstruct xc_hvm_firmware_module smbios_module;
> +
> +ÂÂÂÂ/* BIOS as module */
> +ÂÂÂÂstruct xc_hvm_firmware_module bios_module;
> +ÂÂÂÂstruct xc_hvm_firmware_module acpi_table_module;
> Â};
> Â
> Â/* --- pluggable kernel loader ------------------------------------- */
> diff --git a/tools/libxc/xc_dom_hvmloader.c
> b/tools/libxc/xc_dom_hvmloader.c
> index 79a3b99..3987ed8 100644
> --- a/tools/libxc/xc_dom_hvmloader.c
> +++ b/tools/libxc/xc_dom_hvmloader.c
> @@ -129,6 +129,18 @@ static elf_errorstatus
> xc_dom_parse_hvm_kernel(struct xc_dom_image *dom)
> ÂÂÂÂÂreturn rc;
> Â}
> Â
> +static uint64_t module_init(struct xc_hvm_firmware_module *module,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂuint64_t mstart)
> +{
> +#define MODULE_ALIGN 1UL << 7
> +#define MKALIGN(x, a) (((uint64_t)(x) + (a) - 1) & ~(uint64_t)((a) - 1))
> +ÂÂÂÂif ( module->length != 0 ) {
> +ÂÂÂÂÂÂÂÂmodule->guest_addr_out = mstart;
> +ÂÂÂÂÂÂÂÂreturn MKALIGN(module->length, MODULE_ALIGN);
> +ÂÂÂÂ} else
> +ÂÂÂÂÂÂÂÂreturn 0;
> +}
> +
> Âstatic int modules_init(struct xc_dom_image *dom,
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂuint64_t vend, struct elf_binary *elf,
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂuint64_t *mstart_out, uint64_t *mend_out)
> @@ -136,33 +148,47 @@ static int modules_init(struct xc_dom_image *dom,
> Â#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;
> +ÂÂÂÂuint64_t total_len = 0, offset1 = 0, offset0;
> +ÂÂÂÂuint64_t mstart;
> Â
> -ÂÂÂÂif ( dom->acpi_module.length == 0 && dom->smbios_module.length == 0
> )
> -ÂÂÂÂÂÂÂÂreturn 0;
> +ÂÂÂÂ/* Want to place the modules 1Mb+change behind the loader image. */
> +ÂÂÂÂmstart = MKALIGN(elf->pend, MB_ALIGN) + (MB_ALIGN);
> Â
> ÂÂÂÂÂ/* 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);
> +ÂÂÂÂtotal_len += module_init(&dom->bios_module, mstart + total_len);
> +ÂÂÂÂtotal_len += module_init(&dom->acpi_table_module, mstart +
> total_len);
> +ÂÂÂÂoffset0 = total_len;
> +ÂÂÂÂ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);
> +ÂÂÂÂif ( total_len == 0 )
> +ÂÂÂÂÂÂÂÂreturn 0;
> +
> +ÂÂÂÂ*mstart_out = mstart;
> ÂÂÂÂÂ*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;
> +ÂÂÂÂÂÂÂÂdom->acpi_module.guest_addr_out = *mstart_out + offset0;
> ÂÂÂÂÂif ( dom->smbios_module.length != 0 )
> ÂÂÂÂÂÂÂÂÂdom->smbios_module.guest_addr_out = *mstart_out + offset1;
> Â
> ÂÂÂÂÂreturn 0;
> Â}
> Â
> +static void loadmodule(struct xc_hvm_firmware_module *module,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂuint8_t *dest, uint64_t mstart)
> +{
> +ÂÂÂÂif ( module->length != 0 )
> +ÂÂÂÂÂÂÂÂmemcpy(dest + (module->guest_addr_out - mstart),
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂmodule->data, module->length);
> +}
> +
> Âstatic int loadmodules(struct xc_dom_image *dom,
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂuint64_t mstart, uint64_t mend,
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂuint32_t domid)
> @@ -201,9 +227,11 @@ static int loadmodules(struct xc_dom_image *dom,
> ÂÂÂÂÂmemset(dest, 0, pages << PAGE_SHIFT);
> Â
> ÂÂÂÂÂ/* Load modules into range */
> +ÂÂÂÂloadmodule(&dom->bios_module, dest, mstart);
> +ÂÂÂÂloadmodule(&dom->acpi_table_module, dest, mstart);
> ÂÂÂÂÂif ( dom->acpi_module.length != 0 )
> ÂÂÂÂÂ{
> -ÂÂÂÂÂÂÂÂmemcpy(dest,
> +ÂÂÂÂÂÂÂÂmemcpy(dest + (dom->acpi_module.guest_addr_out - mstart),
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂdom->acpi_module.data,
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂdom->acpi_module.length);
> ÂÂÂÂÂ}
> diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> index c3bb7a3..2444cc2 100644
> --- a/tools/libxc/xc_dom_x86.c
> +++ b/tools/libxc/xc_dom_x86.c
> @@ -511,6 +511,27 @@ static void build_hvm_info(void *hvm_info_page,
> struct xc_dom_image *dom)
> ÂÂÂÂÂhvm_info->checksum = -sum;
> Â}
> Â
> +static void add_module_to_list(struct xc_hvm_firmware_module *module,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂconst char *name, char *cmdline, size_t
> n,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂstruct hvm_modlist_entry *modlist,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂsize_t modlist_size, int *module_nr)
> +{
> +ÂÂÂÂif ( module->length == 0 )
> +ÂÂÂÂÂÂÂÂreturn;
> +
> +ÂÂÂÂ/* assert(*module_nr < modlist_size); */
> +
> +ÂÂÂÂif ( *module_nr == 0 )
> +ÂÂÂÂÂÂÂÂstrcat(cmdline, "modules=");
> +ÂÂÂÂelse
> +ÂÂÂÂÂÂÂÂstrcat(cmdline, ",");
> +ÂÂÂÂstrcat(cmdline, name);
> +
> +ÂÂÂÂmodlist[*module_nr].paddr = module->guest_addr_out;
> +ÂÂÂÂmodlist[*module_nr].size = module->length;
> +ÂÂÂÂ(*module_nr)++;
> +}
> +
> Âstatic int alloc_magic_pages_hvm(struct xc_dom_image *dom)
> Â{
> ÂÂÂÂÂunsigned long i;
> @@ -627,6 +648,75 @@ static int alloc_magic_pages_hvm(struct xc_dom_image
> *dom)
> ÂÂÂÂÂ}
> ÂÂÂÂÂelse
> ÂÂÂÂÂ{
> +ÂÂÂÂÂÂÂÂstruct xc_dom_seg seg;
> +ÂÂÂÂÂÂÂÂstruct hvm_start_info *start_info;
> +ÂÂÂÂÂÂÂÂchar *cmdline;
> +ÂÂÂÂÂÂÂÂstruct hvm_modlist_entry *modlist;
> +ÂÂÂÂÂÂÂÂvoid *start_page;
> +ÂÂÂÂÂÂÂÂsize_t cmdline_size;
> +ÂÂÂÂÂÂÂÂsize_t start_info_size = sizeof(*start_info);
> +
> +ÂÂÂÂÂÂÂÂchar cmdline_new[MAX_GUEST_CMDLINE];
> +ÂÂÂÂÂÂÂÂint module_nr = 0;
> +
> +ÂÂÂÂÂÂÂÂstruct hvm_modlist_entry modlist_building[4];
> +
> +ÂÂÂÂÂÂÂÂcmdline_new[0] = '\0';
> +ÂÂÂÂÂÂÂÂadd_module_to_list(&dom->bios_module, "bios",
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂcmdline_new, MAX_GUEST_CMDLINE,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂmodlist_building,
> ARRAY_SIZE(modlist_building),
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ&module_nr);
> +ÂÂÂÂÂÂÂÂadd_module_to_list(&dom->acpi_table_module, "acpi_table",
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂcmdline_new, MAX_GUEST_CMDLINE,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂmodlist_building,
> ARRAY_SIZE(modlist_building),
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ&module_nr);
> +
> +ÂÂÂÂÂÂÂÂstart_info_size += sizeof(*modlist) * module_nr;
> +ÂÂÂÂÂÂÂÂcmdline_size = ROUNDUP(strlen(cmdline_new) + 1, 8);
> +ÂÂÂÂÂÂÂÂstart_info_size += cmdline_size;
> +
> +ÂÂÂÂÂÂÂÂrc = xc_dom_alloc_segment(dom, &seg, "HVMlite start info", 0,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂstart_info_size);
> +ÂÂÂÂÂÂÂÂif ( rc != 0 )
> +ÂÂÂÂÂÂÂÂ{
> +ÂÂÂÂÂÂÂÂÂÂÂÂDOMPRINTF("Unable to reserve memory for the start info");
> +ÂÂÂÂÂÂÂÂÂÂÂÂgoto out;
> +ÂÂÂÂÂÂÂÂ}
> +
> +ÂÂÂÂÂÂÂÂstart_page = xc_map_foreign_range(xch, domid, start_info_size,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂPROT_READ | PROT_WRITE,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂseg.pfn);
> +ÂÂÂÂÂÂÂÂif ( start_page == NULL )
> +ÂÂÂÂÂÂÂÂ{
> +ÂÂÂÂÂÂÂÂÂÂÂÂDOMPRINTF("Unable to map HVM start info page");
> +ÂÂÂÂÂÂÂÂÂÂÂÂgoto error_out;
> +ÂÂÂÂÂÂÂÂ}
> +
> +ÂÂÂÂÂÂÂÂstart_info = start_page;
> +ÂÂÂÂÂÂÂÂcmdline = start_page + sizeof(*start_info);
> +ÂÂÂÂÂÂÂÂmodlist = start_page + sizeof(*start_info) + cmdline_size;
> +
> +ÂÂÂÂÂÂÂÂif ( cmdline_size )
> +ÂÂÂÂÂÂÂÂ{
> +ÂÂÂÂÂÂÂÂÂÂÂÂstrncpy(cmdline, cmdline_new, MAX_GUEST_CMDLINE);
> +ÂÂÂÂÂÂÂÂÂÂÂÂcmdline[MAX_GUEST_CMDLINE - 1] = '\0';
> +ÂÂÂÂÂÂÂÂÂÂÂÂstart_info->cmdline_paddr = (seg.pfn << PAGE_SHIFT) +
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ((xen_pfn_t)cmdline -
> (xen_pfn_t)start_info);
> +ÂÂÂÂÂÂÂÂ}
> +
> +ÂÂÂÂÂÂÂÂstart_info->nr_modules = module_nr;
> +ÂÂÂÂÂÂÂÂif ( start_info->nr_modules != 0 ) {
> +ÂÂÂÂÂÂÂÂÂÂÂÂmemcpy(modlist, modlist_building, sizeof(*modlist) *
> module_nr);
> +ÂÂÂÂÂÂÂÂÂÂÂÂstart_info->modlist_paddr = (seg.pfn << PAGE_SHIFT) +
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ((xen_pfn_t)modlist -
> (xen_pfn_t)start_info);
> +ÂÂÂÂÂÂÂÂ}
> +
> +ÂÂÂÂÂÂÂÂstart_info->magic = HVM_START_MAGIC_VALUE;
> +
> +ÂÂÂÂÂÂÂÂmunmap(start_page, start_info_size);
> +
> +ÂÂÂÂÂÂÂÂdom->start_info_pfn = seg.pfn;
> +
> ÂÂÂÂÂÂÂÂÂ/*
> ÂÂÂÂÂÂÂÂÂÂ* Allocate and clear additional ioreq server pages. The default
> ÂÂÂÂÂÂÂÂÂÂ* server will use the IOREQ and BUFIOREQ special pages above.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel