Re: [Xen-devel] [PATCH v3 01/16] libxc: Rework extra module initialisation

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Feb 25, 2016 at 02:55:59PM +0000, Anthony PERARD wrote:
> This patch use xc_dom_alloc_segment() to allocate the memory space for the
> ACPI modules and the SMBIOS modules. This is to replace the arbitrary
> placement of 1MB after the hvmloader image.
> 
> In later patches, while trying to load a firmware such as OVMF, the later
> could easily be loaded past the address 4MB (OVMF is a 2MB binary), but
> hvmloader use a range of memory from 4MB to 8MB to perform tests and in the
> process, clear the memory, before loading the modules.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

> ---
> New patch in V3.
> ---
>  tools/libxc/xc_dom_hvmloader.c | 124 
> +++++++++++------------------------------
>  1 file changed, 31 insertions(+), 93 deletions(-)
> 
> diff --git a/tools/libxc/xc_dom_hvmloader.c b/tools/libxc/xc_dom_hvmloader.c
> index 330d5e8..54e096c 100644
> --- a/tools/libxc/xc_dom_hvmloader.c
> +++ b/tools/libxc/xc_dom_hvmloader.c
> @@ -129,98 +129,45 @@ 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;

One thing you might want to take care is that the guest_addr_out is
actually within a reasonable range that hvmloader can access?


Wei.

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