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

["Sahita, Ravi" <ravi.sahita@xxxxxxxxx>]

>From: dunlapg@xxxxxxxxx [mailto:dunlapg@xxxxxxxxx] On Behalf Of George
>Dunlap
>Sent: Friday, July 10, 2015 10:32 AM
>
>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...)


Wei has acked this today morning.

Ravi

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