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

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Fri, Jul 10, 2015 at 1:52 AM, Ed White <edmund.h.white@xxxxxxxxx> wrote:
> The altp2mhvm and nestedhvm parameters are mutually
> exclusive and cannot be set together.
>
> Signed-off-by: Ed White <edmund.h.white@xxxxxxxxx>
>
> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> for the hypervisor 
> bits.
> ---
>  docs/man/xl.cfg.pod.5           | 12 ++++++++++++
>  tools/libxl/libxl.h             |  6 ++++++
>  tools/libxl/libxl_create.c      |  1 +
>  tools/libxl/libxl_dom.c         |  2 ++
>  tools/libxl/libxl_types.idl     |  1 +
>  tools/libxl/xl_cmdimpl.c        | 10 ++++++++++
>  xen/arch/x86/hvm/hvm.c          | 23 +++++++++++++++++++++--
>  xen/include/public/hvm/params.h |  5 ++++-
>  8 files changed, 57 insertions(+), 3 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.h b/tools/libxl/libxl.h
> index a1c5d15..17222e7 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -745,6 +745,12 @@ typedef struct libxl__ctx libxl_ctx;
>  #define LIBXL_HAVE_BUILDINFO_SERIAL_LIST 1
>
>  /*
> + * LIBXL_HAVE_ALTP2M
> + * If this is defined, then libxl supports alternate p2m functionality.
> + */
> +#define LIBXL_HAVE_ALTP2M 1
> +
> +/*
>   * LIBXL_HAVE_REMUS
>   * If this is defined, then libxl supports remus.
>   */
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index f366a09..418deee 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.altp2m,             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 bdc0465..2f1200e 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.altp2m));
>  }
>
>  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 e1632fa..fb641fe 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -440,6 +440,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>                                         ("mmio_hole_memkb",  MemKB),
>                                         ("timer_mode",       
> libxl_timer_mode),
>                                         ("nested_hvm",       libxl_defbool),
> +                                       ("altp2m",           libxl_defbool),
>                                         ("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..43cf6bf 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1500,6 +1500,16 @@ 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.altp2m, 0);
> +
> +        if (!libxl_defbool_is_default(b_info->u.hvm.nested_hvm) &&
> +            libxl_defbool_val(b_info->u.hvm.nested_hvm) &&
> +            !libxl_defbool_is_default(b_info->u.hvm.altp2m) &&
> +            libxl_defbool_val(b_info->u.hvm.altp2m)) {
> +            fprintf(stderr, "ERROR: nestedhvm and altp2mhvm cannot be used 
> together\n");
> +            exit(1);
> +        }
> +
>          xlu_cfg_replace_string(config, "smbios_firmware",
>                                 &b_info->u.hvm.smbios_firmware, 0);
>          xlu_cfg_replace_string(config, "acpi_firmware",
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 23cd507..6e59e68 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -5750,6 +5750,7 @@ static int hvm_allow_set_param(struct domain *d,
>      case HVM_PARAM_VIRIDIAN:
>      case HVM_PARAM_IOREQ_SERVER_PFN:
>      case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
> +    case HVM_PARAM_ALTP2MHVM:

Sorry I missed this -- when I was skimming the reviews of the previous
version, I assumed that when Wei asked "hvm" to be taken out because
it was redundant, it would include the HVM at the end of this
HVM_PARAM.  It seems fairly redundant to have HVM both at the
beginning and the end.  (Note that argument doesn't apply to
NESTEDHVM, because in that case, it's the HVM itself which is nested.)

(I also have an idea this may have been discussed before, but I can't
find the relevant conversation now, so let me know if I'm covering old
ground...)

 -George

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