Re: [PATCH v4 01/16] tools/libxl: Add an unified configuration option for ACPI

[Wei Liu <wei.liu2@citrix.com>]

On Tue, Aug 16, 2016 at 06:24:58PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Since the existing configuration option "u.hvm.acpi" is x86 specific and
> we want to reuse it on ARM as well, add a unified option "acpi" for
> x86 and ARM, and for ARM it's disabled by default.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  tools/libxl/libxl_create.c  | 9 ++++++++-
>  tools/libxl/libxl_dm.c      | 6 ++++--
>  tools/libxl/libxl_types.idl | 4 ++++
>  tools/libxl/xl_cmdimpl.c    | 2 +-
>  4 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 08822e3..3043b1f 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -215,6 +215,12 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>      if (!b_info->event_channels)
>          b_info->event_channels = 1023;
>  
> +#if defined(__arm__) || defined(__aarch64__)
> +    libxl_defbool_setdefault(&b_info->acpi, false);
> +#else
> +    libxl_defbool_setdefault(&b_info->acpi, true);
> +#endif
> +

I recently thought about how to handle the divergence between ARM and
x86. It would make sense to have an
libxl__arch_domain_build_info_acpi_setdefault here.

>      switch (b_info->type) {
>      case LIBXL_DOMAIN_TYPE_HVM:
>          if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
> @@ -454,7 +460,8 @@ int libxl__domain_build(libxl__gc *gc,
>          localents = libxl__calloc(gc, 9, sizeof(char *));
>          i = 0;
>          localents[i++] = "platform/acpi";
> -        localents[i++] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
> +        localents[i++] = (libxl_defbool_val(info->acpi) &&
> +                         libxl_defbool_val(info->u.hvm.acpi)) ? "1" : "0";

Please provide a function for this.

And the logic doesn't seem right. If the user sets u.hvm.acpi only,
(s)he should still have ACPI enabled.

Wei.

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