Re: [Xen-devel] [PATCH v3 12/13] x86/altp2m: Add altp2mhvm HVM domain parameter.

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Jul 01, 2015 at 11:09:36AM -0700, Ed White wrote:
> The altp2mhvm and nestedhvm parameters are mutually
> exclusive and cannot be set together.
> 
> Signed-off-by: Ed White <edmund.h.white@xxxxxxxxx>
> ---
>  docs/man/xl.cfg.pod.5           | 12 ++++++++++++
>  tools/libxl/libxl_create.c      |  1 +
>  tools/libxl/libxl_dom.c         |  2 ++
>  tools/libxl/libxl_types.idl     |  1 +
>  tools/libxl/xl_cmdimpl.c        |  8 ++++++++
>  xen/arch/x86/hvm/hvm.c          | 16 +++++++++++++++-
>  xen/include/public/hvm/params.h |  5 ++++-
>  7 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index a3e0e2e..18afd46 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1035,6 +1035,18 @@ enabled by default and you should usually omit it. It 
> may be necessary
>  to disable the HPET in order to improve compatibility with guest
>  Operating Systems (X86 only)
>  
> +=item B<altp2mhvm=BOOLEAN>
> +
> +Enables or disables hvm guest access to alternate-p2m capability.
> +Alternate-p2m allows a guest to manage multiple p2m guest physical
> +"memory views" (as opposed to a single p2m). This option is
> +disabled by default and is available only to hvm domains.
> +You may want this option if you want to access-control/isolate
> +access to specific guest physical memory pages accessed by
> +the guest, e.g. for HVM domain memory introspection or
> +for isolation/access-control of memory between components within
> +a single guest hvm domain.
> +
>  =item B<nestedhvm=BOOLEAN>
>  
>  Enable or disables guest access to hardware virtualisation features,
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 86384d2..35e322e 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -329,6 +329,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>          libxl_defbool_setdefault(&b_info->u.hvm.hpet,               true);
>          libxl_defbool_setdefault(&b_info->u.hvm.vpt_align,          true);
>          libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm,         false);
> +        libxl_defbool_setdefault(&b_info->u.hvm.altp2mhvm,          false);
>          libxl_defbool_setdefault(&b_info->u.hvm.usb,                false);
>          libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci,   true);
>  
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 600393d..b75f49b 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -300,6 +300,8 @@ static void hvm_set_conf_params(xc_interface *handle, 
> uint32_t domid,
>                      libxl_defbool_val(info->u.hvm.vpt_align));
>      xc_hvm_param_set(handle, domid, HVM_PARAM_NESTEDHVM,
>                      libxl_defbool_val(info->u.hvm.nested_hvm));
> +    xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2MHVM,
> +                    libxl_defbool_val(info->u.hvm.altp2mhvm));
>  }
>  
>  int libxl__build_pre(libxl__gc *gc, uint32_t domid,
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 23f27d4..66a89cf 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -437,6 +437,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>                                         ("mmio_hole_memkb",  MemKB),
>                                         ("timer_mode",       
> libxl_timer_mode),
>                                         ("nested_hvm",       libxl_defbool),
> +                                       ("altp2mhvm",        libxl_defbool),

It's redundant to have "hvm" in the name of this field. Calling it
"altp2m" would be fine IMHO.

>                                         ("smbios_firmware",  string),
>                                         ("acpi_firmware",    string),
>                                         ("nographic",        libxl_defbool),
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index c858068..ccb0de9 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1500,6 +1500,14 @@ static void parse_config_data(const char 
> *config_source,
>  
>          xlu_cfg_get_defbool(config, "nestedhvm", &b_info->u.hvm.nested_hvm, 
> 0);
>  
> +        xlu_cfg_get_defbool(config, "altp2mhvm", &b_info->u.hvm.altp2mhvm, 
> 0);
> +
> +        if (strcmp(libxl_defbool_to_string(b_info->u.hvm.nested_hvm), 
> "True") == 0 &&
> +            strcmp(libxl_defbool_to_string(b_info->u.hvm.altp2mhvm), "True") 
> == 0) {
> +            fprintf(stderr, "ERROR: nestedhvm and altp2mhvm cannot be used 
> together\n");

You can use libxl_defbool_val. Don't use strcmp.

> +            exit (1);

Coding style.

You also need to #define LIBXL_HAVE_XXX in libxl.h. See that file for
examples.

Wei.

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