Re: [Xen-devel] [PATCH v3 3/4] libxl: Store PVH guest's e820 map in xc_dom_image

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

On Thu, Mar 15, 2018 at 02:35:17PM -0700, Maran Wilson wrote:
> From: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> 
> We will later copy it to hvm_start_info.
> 
> (Also remove stale comment claming that xc_dom_image.start_info_seg is
> only used for HVMlite guests)
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> ---
>  tools/libxc/include/xc_dom.h | 8 +++++++-
>  tools/libxl/libxl_x86.c      | 6 ++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> index 491cad8..6ef68f9 100644
> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -99,7 +99,7 @@ struct xc_dom_image {
>      struct xc_dom_seg p2m_seg;
>      struct xc_dom_seg pgtables_seg;
>      struct xc_dom_seg devicetree_seg;
> -    struct xc_dom_seg start_info_seg; /* HVMlite only */
> +    struct xc_dom_seg start_info_seg;
>      xen_pfn_t start_info_pfn;
>      xen_pfn_t console_pfn;
>      xen_pfn_t xenstore_pfn;
> @@ -224,6 +224,12 @@ struct xc_dom_image {
>      /* Extra SMBIOS structures passed to HVMLOADER */
>      struct xc_hvm_firmware_module smbios_module;
>  
> +    /* PVH guests */
> +#if defined(__i386__) || defined(__x86_64__)
> +    struct e820entry *e820;
> +    unsigned int e820_entries;
> +#endif

Not sure whether it's worth protecting this with x86 guards,
xc_dom_image it's already a mix of ARM/x86 fields without much
protection.

> +
>      xen_pfn_t vuart_gfn;
>  };
>  
> diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
> index 3331cc5..0de278f 100644
> --- a/tools/libxl/libxl_x86.c
> +++ b/tools/libxl/libxl_x86.c
> @@ -600,6 +600,12 @@ int libxl__arch_domain_construct_memmap(libxl__gc *gc,
>          goto out;
>      }
>  
> +    if (d_config->b_info.type == LIBXL_DOMAIN_TYPE_PVH) {
> +        dom->e820 = libxl__malloc(gc, sizeof(struct e820entry) * 
> e820_entries);
> +        dom->e820_entries = e820_entries;
> +        memcpy(dom->e820,  e820, e820_entries * sizeof(*(dom->e820)));
                            ^ extra space.

Also could you please use sizeof(*(dom->e820)) or sizeof(struct
e820entry) consistently? (I think I prefer the latter).

Also you want this for both PVH and HVM, since hvmloader now also uses
the hvm_start_info structure to pass the BIOS blob as a module.

Thanks, Roger.

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