Re: [Xen-devel] [PATCH v5 16/16] libxl/arm: Add the size of ACPI tables to maxmem

[Julien Grall <julien.grall@xxxxxxx>]

Hi Shannon,

On 02/09/16 03:55, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
>
> Here it adds the ACPI tables size to set the target maxmem to avoid
> providing less available memory for guest.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> ---
>  tools/libxl/libxl_arch.h        |  2 +-
>  tools/libxl/libxl_arm.c         | 18 +++++++++++++++++-
>  tools/libxl/libxl_arm.h         |  4 ++++
>  tools/libxl/libxl_arm_acpi.c    | 20 ++++++++++++++++++++
>  tools/libxl/libxl_arm_no_acpi.c |  6 ++++++
>  tools/libxl/libxl_dom.c         |  2 +-
>  tools/libxl/libxl_x86.c         |  2 +-
>  7 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
> index 337061f..d62fa4c 100644
> --- a/tools/libxl/libxl_arch.h
> +++ b/tools/libxl/libxl_arch.h
> @@ -30,7 +30,7 @@ int libxl__arch_domain_save_config(libxl__gc *gc,
>  /* arch specific internal domain creation function */
>  _hidden
>  int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
> -               uint32_t domid);
> +                              libxl__domain_build_state *state, uint32_t 
> domid);
>
>  /* setup arch specific hardware description, i.e. DTB on ARM */
>  _hidden
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index e73d65e..c7d4f65 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -101,8 +101,24 @@ int libxl__arch_domain_save_config(libxl__gc *gc,
>  }
>
>  int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
> -                              uint32_t domid)
> +                              libxl__domain_build_state *state, uint32_t domid)
>  {
> +    libxl_domain_build_info *const info = &d_config->b_info;
> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> +    int size;
> +
> +    /* Add the size of ACPI tables to maxmem if ACPI is enabled for guest. */
> +    if (libxl_defbool_val(info->acpi)) {
> +        size = libxl__get_acpi_size(gc, info, state);
> +        if (size < 0)
> +            return ERROR_FAIL;
> +        if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
> +                                LIBXL_MAXMEM_CONSTANT + (size + 1023) / 1024)) 
> {
I still have some concern about use info->target_memkb + 
LIBXL_MAXMEM_CONSTANT here. What if the generic code decide to change 
the computation?
We may forgot to replicate here. My suggestion on the previous version 
was to have in the common code
xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + 
LIBXL_MAXMEM_CONSTANT + libxl__arch_memory_constant());
Or a similar name.

> +            LOGE(ERROR, "Couldn't set max memory");
> +            return ERROR_FAIL;
> +        }
> +    }
> +
>      return 0;
>  }
Regards,


--
Julien Grall

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