Re: [Xen-devel] [PATCH v4 2/4] libxl/x86: Build e820 map earlier for HVM/PVH guests

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Mar 20, 2018 at 09:50:50AM -0700, Maran Wilson wrote:
> From: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> 
> Since hvm_start_info has now been expanded to include memory map (i.e.
> e820) we need to know size of this map by the time we create
> dom->start_info_seg in alloc_magic_pages_hvm().
> 
> To do so we have to call libxl__arch_domain_construct_memmap() earlier,
> before xc_dom_build_image(). And since libxl__arch_domain_construct_memmap()
> is only used by for x86 we can make this call from x86's
> libxl__arch_domain_finalise_hw_description(), at the same time removing
> its NOP definition from ARM code and renaming and making it static in
> libxl_x86.c
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Just one nit.

> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 2e29b52..e83aeb9 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -698,6 +698,7 @@ static int set_vnuma_info(libxl__gc *gc, uint32_t domid,
>  }
>  
>  static int libxl__build_dom(libxl__gc *gc, uint32_t domid,
> +             libxl_domain_config *d_config,
>               libxl_domain_build_info *info, libxl__domain_build_state *state,
>               struct xc_dom_image *dom)
>  {
> @@ -733,7 +734,8 @@ static int libxl__build_dom(libxl__gc *gc, uint32_t domid,
>          LOGE(ERROR, "xc_dom_boot_mem_init failed");
>          goto out;
>      }
> -    if ( (ret = libxl__arch_domain_finalise_hw_description(gc, info, dom)) 
> != 0 ) {
> +    if ( (ret = libxl__arch_domain_finalise_hw_description(gc, domid, 
> d_config,
> +                                                           info, dom)) != 0 
> ) {
>          LOGE(ERROR, "libxl__arch_domain_finalise_hw_description failed");
>          goto out;
>      }
> @@ -758,7 +760,7 @@ out:
>      return ret != 0 ? ERROR_FAIL : 0;
>  }
>  
> -int libxl__build_pv(libxl__gc *gc, uint32_t domid,
> +int libxl__build_pv(libxl__gc *gc, uint32_t domid, libxl_domain_config 
> *d_config,
>               libxl_domain_build_info *info, libxl__domain_build_state *state)

If you pass libxl_domain_config as a parameter you no longer need to
pass libxl_domain_build_info, and you can do:

libxl_domain_build_info *const info = &d_config->b_info;

That would make the parameters of libxl__build_hvm and libxl__build_pv
the same :).

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel