From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH v4 12/15] x86/altp2m: Add altp2mhvm HVM domain parameter. Date: Tue, 14 Jul 2015 12:50:01 +0100 Message-ID: <55A4F769.8070205@eu.citrix.com> References: <1436489553-6300-1-git-send-email-edmund.h.white@intel.com> <1436489553-6300-13-git-send-email-edmund.h.white@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Sahita, Ravi" , "White, Edmund H" Cc: Wei Liu , Tim Deegan , Ian Jackson , "xen-devel@lists.xen.org" , Jan Beulich , Andrew Cooper , "tlengyel@novetta.com" , Daniel De Graaf List-Id: xen-devel@lists.xenproject.org On 07/10/2015 11:12 PM, Sahita, Ravi wrote: >> From: dunlapg@gmail.com [mailto:dunlapg@gmail.com] On Behalf Of George >> Dunlap >> Sent: Friday, July 10, 2015 10:32 AM >> >> On Fri, Jul 10, 2015 at 1:52 AM, Ed White >> wrote: >>> The altp2mhvm and nestedhvm parameters are mutually exclusive and >>> cannot be set together. >>> >>> Signed-off-by: Ed White >>> >>> Reviewed-by: Andrew Cooper 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 >>> + >>> +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 >>> >>> 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. That means he has no objections, but it doesn't mean nobody else has objections. :-) -George