Re: [Xen-devel] [PATCH 16/18] arm/altp2m: Extended libxl to activate altp2m on ARM.
On Mon, Jul 25, 2016 at 01:26:29PM +0200, Sergej Proskurin wrote: > > On 07/25/2016 12:08 PM, Wei Liu wrote: > > On Mon, Jul 25, 2016 at 10:49:41AM +0100, Julien Grall wrote: > >> Hello, > >> > >> On 25/07/16 10:04, Sergej Proskurin wrote: > >>> On 07/25/2016 10:32 AM, Wei Liu wrote: > On Sun, Jul 24, 2016 at 06:06:00PM +0200, Sergej Proskurin wrote: > >>> +xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M, > >>> + libxl_defbool_val(info->u.pv.altp2m)); > >>> +} > >>> +#endif > >>> + > >>> static void hvm_set_conf_params(xc_interface *handle, uint32_t domid, > >>> libxl_domain_build_info *const info) > >>> { > >>> @@ -433,6 +443,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t > >>> domid, > >>> return rc; > >>> #endif > >>> } > >>> +#if defined(__arm__) || defined(__aarch64__) > >>> +else /* info->type == LIBXL_DOMAIN_TYPE_PV */ > >>> +pv_set_conf_params(ctx->xch, domid, info); > >>> +#endif > >>> > >>> rc = libxl__arch_domain_create(gc, d_config, domid); > >>> > >>> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > >>> index ef614be..0a164f9 100644 > >>> --- a/tools/libxl/libxl_types.idl > >>> +++ b/tools/libxl/libxl_types.idl > >>> @@ -554,6 +554,7 @@ libxl_domain_build_info = > >>> Struct("domain_build_info",[ > >>> ("features", string, {'const': > >>> True}), > >>> # Use host's E820 for PCI > >>> passthrough. > >>> ("e820_host", libxl_defbool), > >>> + ("altp2m", libxl_defbool), > >> Why is this placed in PV instead of arch_arm? > >> > > The current implementation mirrors the x86's altp2m, where the altp2m > > field represents a property of HVM guests in b_info->u.hvm.altp2m. Since > > guests on ARM are marked as PV, I have placed the field altp2m into > > b_info->u.pv.altp2m. > > > > However, if you believe that it would make more sense to place altp2m > > into b_info->arch_arm.altp2m, I will try to adapt the affected fields. > > > OK. I don't think that one should be placed in u.pv because that union > is for x86. However it is also not suitable to just promote that to > a common field because x86 pv doesn't have altp2m support. > > I think arch_arm would be a better location. > > >>> Alright, I will move the field altp2m to arch_arm and adopt the > >>> initialization routines. > >> Would not it make more sense to introduce a generic field 'altp2m' (i.e > >> outside of hvm and arch_arm)? > >> > >> Otherwise, toolstack such as libvirt will need to have specific code to > >> handle ARM and x86. > >> > > Hmm... this is a compelling reason. > > > > After mulling over this issue a bit more, I think I'm fine with > > promoting altp2m to a common field. > > > > Sergej, this is a patch to get you started. Please make sure setting the > > old field still works. If I'm not clear enough, please ask. > > > > Alright, thank you. One question up front: Do we still need two > different LIBXL_HAVE macros then? > Yes, we still need one. Something like LIBXL_HAVE_COMMON_ALTP2M ? I'm bad at naming things though. > Also, concerning the dom configuration parameters: Do you think we > should use two different dom configuration parameters (the legacy > "altp2mhvm" and the new "altp2m") or shall we merge both into "altp2m" > as we will address only one common altp2m field in the struct? > I think using altp2m is fine. But altp2mhvm needs to continue to work because it is trivial to keep the old one. In any case, document the new parameter, deprecate the old one and document and the relationship between the two. Wei. > Best regards > ~Sergej > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 16/18] arm/altp2m: Extended libxl to activate altp2m on ARM.
On 07/25/2016 12:08 PM, Wei Liu wrote: > On Mon, Jul 25, 2016 at 10:49:41AM +0100, Julien Grall wrote: >> Hello, >> >> On 25/07/16 10:04, Sergej Proskurin wrote: >>> On 07/25/2016 10:32 AM, Wei Liu wrote: On Sun, Jul 24, 2016 at 06:06:00PM +0200, Sergej Proskurin wrote: >>> +xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M, >>> + libxl_defbool_val(info->u.pv.altp2m)); >>> +} >>> +#endif >>> + >>> static void hvm_set_conf_params(xc_interface *handle, uint32_t domid, >>> libxl_domain_build_info *const info) >>> { >>> @@ -433,6 +443,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, >>> return rc; >>> #endif >>> } >>> +#if defined(__arm__) || defined(__aarch64__) >>> +else /* info->type == LIBXL_DOMAIN_TYPE_PV */ >>> +pv_set_conf_params(ctx->xch, domid, info); >>> +#endif >>> >>> rc = libxl__arch_domain_create(gc, d_config, domid); >>> >>> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl >>> index ef614be..0a164f9 100644 >>> --- a/tools/libxl/libxl_types.idl >>> +++ b/tools/libxl/libxl_types.idl >>> @@ -554,6 +554,7 @@ libxl_domain_build_info = >>> Struct("domain_build_info",[ >>> ("features", string, {'const': >>> True}), >>> # Use host's E820 for PCI >>> passthrough. >>> ("e820_host", libxl_defbool), >>> + ("altp2m", libxl_defbool), >> Why is this placed in PV instead of arch_arm? >> > The current implementation mirrors the x86's altp2m, where the altp2m > field represents a property of HVM guests in b_info->u.hvm.altp2m. Since > guests on ARM are marked as PV, I have placed the field altp2m into > b_info->u.pv.altp2m. > > However, if you believe that it would make more sense to place altp2m > into b_info->arch_arm.altp2m, I will try to adapt the affected fields. > OK. I don't think that one should be placed in u.pv because that union is for x86. However it is also not suitable to just promote that to a common field because x86 pv doesn't have altp2m support. I think arch_arm would be a better location. >>> Alright, I will move the field altp2m to arch_arm and adopt the >>> initialization routines. >> Would not it make more sense to introduce a generic field 'altp2m' (i.e >> outside of hvm and arch_arm)? >> >> Otherwise, toolstack such as libvirt will need to have specific code to >> handle ARM and x86. >> > Hmm... this is a compelling reason. > > After mulling over this issue a bit more, I think I'm fine with > promoting altp2m to a common field. > > Sergej, this is a patch to get you started. Please make sure setting the > old field still works. If I'm not clear enough, please ask. > Alright, thank you. One question up front: Do we still need two different LIBXL_HAVE macros then? Also, concerning the dom configuration parameters: Do you think we should use two different dom configuration parameters (the legacy "altp2mhvm" and the new "altp2m") or shall we merge both into "altp2m" as we will address only one common altp2m field in the struct? Best regards ~Sergej ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 16/18] arm/altp2m: Extended libxl to activate altp2m on ARM.
On Mon, Jul 25, 2016 at 10:49:41AM +0100, Julien Grall wrote: > Hello, > > On 25/07/16 10:04, Sergej Proskurin wrote: > >On 07/25/2016 10:32 AM, Wei Liu wrote: > >>On Sun, Jul 24, 2016 at 06:06:00PM +0200, Sergej Proskurin wrote: > >+xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M, > >+ libxl_defbool_val(info->u.pv.altp2m)); > >+} > >+#endif > >+ > > static void hvm_set_conf_params(xc_interface *handle, uint32_t domid, > > libxl_domain_build_info *const info) > > { > >@@ -433,6 +443,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, > > return rc; > > #endif > > } > >+#if defined(__arm__) || defined(__aarch64__) > >+else /* info->type == LIBXL_DOMAIN_TYPE_PV */ > >+pv_set_conf_params(ctx->xch, domid, info); > >+#endif > > > > rc = libxl__arch_domain_create(gc, d_config, domid); > > > >diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > >index ef614be..0a164f9 100644 > >--- a/tools/libxl/libxl_types.idl > >+++ b/tools/libxl/libxl_types.idl > >@@ -554,6 +554,7 @@ libxl_domain_build_info = > >Struct("domain_build_info",[ > > ("features", string, {'const': > > True}), > > # Use host's E820 for PCI > > passthrough. > > ("e820_host", libxl_defbool), > >+ ("altp2m", libxl_defbool), > Why is this placed in PV instead of arch_arm? > > >>>The current implementation mirrors the x86's altp2m, where the altp2m > >>>field represents a property of HVM guests in b_info->u.hvm.altp2m. Since > >>>guests on ARM are marked as PV, I have placed the field altp2m into > >>>b_info->u.pv.altp2m. > >>> > >>>However, if you believe that it would make more sense to place altp2m > >>>into b_info->arch_arm.altp2m, I will try to adapt the affected fields. > >>> > >>OK. I don't think that one should be placed in u.pv because that union > >>is for x86. However it is also not suitable to just promote that to > >>a common field because x86 pv doesn't have altp2m support. > >> > >>I think arch_arm would be a better location. > >> > > > >Alright, I will move the field altp2m to arch_arm and adopt the > >initialization routines. > > Would not it make more sense to introduce a generic field 'altp2m' (i.e > outside of hvm and arch_arm)? > > Otherwise, toolstack such as libvirt will need to have specific code to > handle ARM and x86. > Hmm... this is a compelling reason. After mulling over this issue a bit more, I think I'm fine with promoting altp2m to a common field. Sergej, this is a patch to get you started. Please make sure setting the old field still works. If I'm not clear enough, please ask. Wei. ---8<--- diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index ef614be..81d3ae5 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -494,6 +494,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ # Note that the partial device tree should avoid to use the phandle # 65000 which is reserved by the toolstack. ("device_tree", string), +("altp2m", libxl_defbool), ("u", KeyedUnion(None, libxl_domain_type, "type", [("hvm", Struct(None, [("firmware", string), ("bios", libxl_bios_type), @@ -512,7 +513,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), + ("altp2m", libxl_defbool), # deprecated, please use common field ("smbios_firmware", string), ("acpi_firmware",string), ("hdtype", libxl_hdtype), ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 16/18] arm/altp2m: Extended libxl to activate altp2m on ARM.
Hello, On 25/07/16 10:04, Sergej Proskurin wrote: On 07/25/2016 10:32 AM, Wei Liu wrote: On Sun, Jul 24, 2016 at 06:06:00PM +0200, Sergej Proskurin wrote: +xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M, + libxl_defbool_val(info->u.pv.altp2m)); +} +#endif + static void hvm_set_conf_params(xc_interface *handle, uint32_t domid, libxl_domain_build_info *const info) { @@ -433,6 +443,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, return rc; #endif } +#if defined(__arm__) || defined(__aarch64__) +else /* info->type == LIBXL_DOMAIN_TYPE_PV */ +pv_set_conf_params(ctx->xch, domid, info); +#endif rc = libxl__arch_domain_create(gc, d_config, domid); diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index ef614be..0a164f9 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -554,6 +554,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("features", string, {'const': True}), # Use host's E820 for PCI passthrough. ("e820_host", libxl_defbool), + ("altp2m", libxl_defbool), Why is this placed in PV instead of arch_arm? The current implementation mirrors the x86's altp2m, where the altp2m field represents a property of HVM guests in b_info->u.hvm.altp2m. Since guests on ARM are marked as PV, I have placed the field altp2m into b_info->u.pv.altp2m. However, if you believe that it would make more sense to place altp2m into b_info->arch_arm.altp2m, I will try to adapt the affected fields. OK. I don't think that one should be placed in u.pv because that union is for x86. However it is also not suitable to just promote that to a common field because x86 pv doesn't have altp2m support. I think arch_arm would be a better location. Alright, I will move the field altp2m to arch_arm and adopt the initialization routines. Would not it make more sense to introduce a generic field 'altp2m' (i.e outside of hvm and arch_arm)? Otherwise, toolstack such as libvirt will need to have specific code to handle ARM and x86. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 16/18] arm/altp2m: Extended libxl to activate altp2m on ARM.
Hi Wei, On 07/25/2016 10:32 AM, Wei Liu wrote: > On Sun, Jul 24, 2016 at 06:06:00PM +0200, Sergej Proskurin wrote: >> Hi Wei, >> >> On 07/07/2016 06:27 PM, Wei Liu wrote: >>> On Mon, Jul 04, 2016 at 01:45:45PM +0200, Sergej Proskurin wrote: The current implementation allows to set the parameter HVM_PARAM_ALTP2M. This parameter allows further usage of altp2m on ARM. For this, we define an additional altp2m field for PV domains as part of the libxl_domain_type struct. This field can be set only on ARM systems through the "altp2m" switch in the domain's configuration file (i.e. set altp2m=1). Signed-off-by: Sergej Proskurin--- Cc: Ian Jackson Cc: Wei Liu --- tools/libxl/libxl_create.c | 1 + tools/libxl/libxl_dom.c | 14 ++ tools/libxl/libxl_types.idl | 1 + tools/libxl/xl_cmdimpl.c| 5 + 4 files changed, 21 insertions(+) diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 1b99472..40b5f61 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -400,6 +400,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, b_info->cmdline = b_info->u.pv.cmdline; b_info->u.pv.cmdline = NULL; } +libxl_defbool_setdefault(_info->u.pv.altp2m, false); break; default: LOG(ERROR, "invalid domain type %s in create info", diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index ec29060..ab023a2 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -277,6 +277,16 @@ err: } #endif +#if defined(__arm__) || defined(__aarch64__) +static void pv_set_conf_params(xc_interface *handle, uint32_t domid, + libxl_domain_build_info *const info) +{ +if ( libxl_defbool_val(info->u.pv.altp2m) ) >>> Coding style. >>> >> I am not sure which part does not fulfill Xen's coding style. Could you >> be more specific please? Thank you. >> > There is a CODING_STYLE file in libxl as well. The coding style in > tools/ is generally different from the coding style in hypervisor with > the exception of libxc. > > Sorry for being too terse on this. Do ask questions! :-) It's all good, no worries :) Thank you for the hint (it must be the indentation inside the if-clause brackets). I was still in the Xen coding style, when I wrote this part: Now, I know better, thank you :) +xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M, + libxl_defbool_val(info->u.pv.altp2m)); +} +#endif + static void hvm_set_conf_params(xc_interface *handle, uint32_t domid, libxl_domain_build_info *const info) { @@ -433,6 +443,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, return rc; #endif } +#if defined(__arm__) || defined(__aarch64__) +else /* info->type == LIBXL_DOMAIN_TYPE_PV */ +pv_set_conf_params(ctx->xch, domid, info); +#endif rc = libxl__arch_domain_create(gc, d_config, domid); diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index ef614be..0a164f9 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -554,6 +554,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("features", string, {'const': True}), # Use host's E820 for PCI passthrough. ("e820_host", libxl_defbool), + ("altp2m", libxl_defbool), >>> Why is this placed in PV instead of arch_arm? >>> >> The current implementation mirrors the x86's altp2m, where the altp2m >> field represents a property of HVM guests in b_info->u.hvm.altp2m. Since >> guests on ARM are marked as PV, I have placed the field altp2m into >> b_info->u.pv.altp2m. >> >> However, if you believe that it would make more sense to place altp2m >> into b_info->arch_arm.altp2m, I will try to adapt the affected fields. >> > OK. I don't think that one should be placed in u.pv because that union > is for x86. However it is also not suitable to just promote that to > a common field because x86 pv doesn't have altp2m support. > > I think arch_arm would be a better location. > Alright, I will move the field altp2m to arch_arm and adopt the initialization routines. >>> You would also need a LIBXL_HAVE macro in libxl.h for the new field. >>> >> There is already a LIBXL_HAVE_ALTP2M macro defined in libxl.h. Or did >> you mean using an explicit one for ARM? >> > Yes, we need a new one for ARM. > Ok. I will
Re: [Xen-devel] [PATCH 16/18] arm/altp2m: Extended libxl to activate altp2m on ARM.
On Sun, Jul 24, 2016 at 06:06:00PM +0200, Sergej Proskurin wrote: > Hi Wei, > > On 07/07/2016 06:27 PM, Wei Liu wrote: > > On Mon, Jul 04, 2016 at 01:45:45PM +0200, Sergej Proskurin wrote: > >> The current implementation allows to set the parameter HVM_PARAM_ALTP2M. > >> This parameter allows further usage of altp2m on ARM. For this, we > >> define an additional altp2m field for PV domains as part of the > >> libxl_domain_type struct. This field can be set only on ARM systems > >> through the "altp2m" switch in the domain's configuration file (i.e. > >> set altp2m=1). > >> > >> Signed-off-by: Sergej Proskurin> >> --- > >> Cc: Ian Jackson > >> Cc: Wei Liu > >> --- > >> tools/libxl/libxl_create.c | 1 + > >> tools/libxl/libxl_dom.c | 14 ++ > >> tools/libxl/libxl_types.idl | 1 + > >> tools/libxl/xl_cmdimpl.c| 5 + > >> 4 files changed, 21 insertions(+) > >> > >> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > >> index 1b99472..40b5f61 100644 > >> --- a/tools/libxl/libxl_create.c > >> +++ b/tools/libxl/libxl_create.c > >> @@ -400,6 +400,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, > >> b_info->cmdline = b_info->u.pv.cmdline; > >> b_info->u.pv.cmdline = NULL; > >> } > >> +libxl_defbool_setdefault(_info->u.pv.altp2m, false); > >> break; > >> default: > >> LOG(ERROR, "invalid domain type %s in create info", > >> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > >> index ec29060..ab023a2 100644 > >> --- a/tools/libxl/libxl_dom.c > >> +++ b/tools/libxl/libxl_dom.c > >> @@ -277,6 +277,16 @@ err: > >> } > >> #endif > >> > >> +#if defined(__arm__) || defined(__aarch64__) > >> +static void pv_set_conf_params(xc_interface *handle, uint32_t domid, > >> + libxl_domain_build_info *const info) > >> +{ > >> +if ( libxl_defbool_val(info->u.pv.altp2m) ) > > > > Coding style. > > > > I am not sure which part does not fulfill Xen's coding style. Could you > be more specific please? Thank you. > There is a CODING_STYLE file in libxl as well. The coding style in tools/ is generally different from the coding style in hypervisor with the exception of libxc. Sorry for being too terse on this. Do ask questions! :-) > >> +xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M, > >> + libxl_defbool_val(info->u.pv.altp2m)); > >> +} > >> +#endif > >> + > >> static void hvm_set_conf_params(xc_interface *handle, uint32_t domid, > >> libxl_domain_build_info *const info) > >> { > >> @@ -433,6 +443,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, > >> return rc; > >> #endif > >> } > >> +#if defined(__arm__) || defined(__aarch64__) > >> +else /* info->type == LIBXL_DOMAIN_TYPE_PV */ > >> +pv_set_conf_params(ctx->xch, domid, info); > >> +#endif > >> > >> rc = libxl__arch_domain_create(gc, d_config, domid); > >> > >> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > >> index ef614be..0a164f9 100644 > >> --- a/tools/libxl/libxl_types.idl > >> +++ b/tools/libxl/libxl_types.idl > >> @@ -554,6 +554,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ > >>("features", string, {'const': > >> True}), > >># Use host's E820 for PCI > >> passthrough. > >>("e820_host", libxl_defbool), > >> + ("altp2m", libxl_defbool), > > > > Why is this placed in PV instead of arch_arm? > > > > The current implementation mirrors the x86's altp2m, where the altp2m > field represents a property of HVM guests in b_info->u.hvm.altp2m. Since > guests on ARM are marked as PV, I have placed the field altp2m into > b_info->u.pv.altp2m. > > However, if you believe that it would make more sense to place altp2m > into b_info->arch_arm.altp2m, I will try to adapt the affected fields. > OK. I don't think that one should be placed in u.pv because that union is for x86. However it is also not suitable to just promote that to a common field because x86 pv doesn't have altp2m support. I think arch_arm would be a better location. > > You would also need a LIBXL_HAVE macro in libxl.h for the new field. > > > > There is already a LIBXL_HAVE_ALTP2M macro defined in libxl.h. Or did > you mean using an explicit one for ARM? > Yes, we need a new one for ARM. Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 16/18] arm/altp2m: Extended libxl to activate altp2m on ARM.
Hi Wei, On 07/07/2016 06:27 PM, Wei Liu wrote: > On Mon, Jul 04, 2016 at 01:45:45PM +0200, Sergej Proskurin wrote: >> The current implementation allows to set the parameter HVM_PARAM_ALTP2M. >> This parameter allows further usage of altp2m on ARM. For this, we >> define an additional altp2m field for PV domains as part of the >> libxl_domain_type struct. This field can be set only on ARM systems >> through the "altp2m" switch in the domain's configuration file (i.e. >> set altp2m=1). >> >> Signed-off-by: Sergej Proskurin>> --- >> Cc: Ian Jackson >> Cc: Wei Liu >> --- >> tools/libxl/libxl_create.c | 1 + >> tools/libxl/libxl_dom.c | 14 ++ >> tools/libxl/libxl_types.idl | 1 + >> tools/libxl/xl_cmdimpl.c| 5 + >> 4 files changed, 21 insertions(+) >> >> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c >> index 1b99472..40b5f61 100644 >> --- a/tools/libxl/libxl_create.c >> +++ b/tools/libxl/libxl_create.c >> @@ -400,6 +400,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, >> b_info->cmdline = b_info->u.pv.cmdline; >> b_info->u.pv.cmdline = NULL; >> } >> +libxl_defbool_setdefault(_info->u.pv.altp2m, false); >> break; >> default: >> LOG(ERROR, "invalid domain type %s in create info", >> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c >> index ec29060..ab023a2 100644 >> --- a/tools/libxl/libxl_dom.c >> +++ b/tools/libxl/libxl_dom.c >> @@ -277,6 +277,16 @@ err: >> } >> #endif >> >> +#if defined(__arm__) || defined(__aarch64__) >> +static void pv_set_conf_params(xc_interface *handle, uint32_t domid, >> + libxl_domain_build_info *const info) >> +{ >> +if ( libxl_defbool_val(info->u.pv.altp2m) ) > > Coding style. > I am not sure which part does not fulfill Xen's coding style. Could you be more specific please? Thank you. >> +xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M, >> + libxl_defbool_val(info->u.pv.altp2m)); >> +} >> +#endif >> + >> static void hvm_set_conf_params(xc_interface *handle, uint32_t domid, >> libxl_domain_build_info *const info) >> { >> @@ -433,6 +443,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, >> return rc; >> #endif >> } >> +#if defined(__arm__) || defined(__aarch64__) >> +else /* info->type == LIBXL_DOMAIN_TYPE_PV */ >> +pv_set_conf_params(ctx->xch, domid, info); >> +#endif >> >> rc = libxl__arch_domain_create(gc, d_config, domid); >> >> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl >> index ef614be..0a164f9 100644 >> --- a/tools/libxl/libxl_types.idl >> +++ b/tools/libxl/libxl_types.idl >> @@ -554,6 +554,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ >>("features", string, {'const': True}), >># Use host's E820 for PCI passthrough. >>("e820_host", libxl_defbool), >> + ("altp2m", libxl_defbool), > > Why is this placed in PV instead of arch_arm? > The current implementation mirrors the x86's altp2m, where the altp2m field represents a property of HVM guests in b_info->u.hvm.altp2m. Since guests on ARM are marked as PV, I have placed the field altp2m into b_info->u.pv.altp2m. However, if you believe that it would make more sense to place altp2m into b_info->arch_arm.altp2m, I will try to adapt the affected fields. > You would also need a LIBXL_HAVE macro in libxl.h for the new field. > There is already a LIBXL_HAVE_ALTP2M macro defined in libxl.h. Or did you mean using an explicit one for ARM? >>])), >> ("invalid", None), >> ], keyvar_init_val = "LIBXL_DOMAIN_TYPE_INVALID")), >> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c >> index 6459eec..12c6e48 100644 >> --- a/tools/libxl/xl_cmdimpl.c >> +++ b/tools/libxl/xl_cmdimpl.c >> @@ -1718,6 +1718,11 @@ static void parse_config_data(const char >> *config_source, >> exit(1); >> } >> >> +#if defined(__arm__) || defined(__aarch64__) >> +/* Enable altp2m for PV guests solely on ARM */ >> +xlu_cfg_get_defbool(config, "altp2m", _info->u.pv.altp2m, 0); >> +#endif >> + >> break; >> } >> default: >> -- >> 2.8.3 >> Thank you. Best regards, ~Sergej ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 16/18] arm/altp2m: Extended libxl to activate altp2m on ARM.
On Mon, Jul 04, 2016 at 01:45:45PM +0200, Sergej Proskurin wrote: > The current implementation allows to set the parameter HVM_PARAM_ALTP2M. > This parameter allows further usage of altp2m on ARM. For this, we > define an additional altp2m field for PV domains as part of the > libxl_domain_type struct. This field can be set only on ARM systems > through the "altp2m" switch in the domain's configuration file (i.e. > set altp2m=1). > > Signed-off-by: Sergej Proskurin> --- > Cc: Ian Jackson > Cc: Wei Liu > --- > tools/libxl/libxl_create.c | 1 + > tools/libxl/libxl_dom.c | 14 ++ > tools/libxl/libxl_types.idl | 1 + > tools/libxl/xl_cmdimpl.c| 5 + > 4 files changed, 21 insertions(+) > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > index 1b99472..40b5f61 100644 > --- a/tools/libxl/libxl_create.c > +++ b/tools/libxl/libxl_create.c > @@ -400,6 +400,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, > b_info->cmdline = b_info->u.pv.cmdline; > b_info->u.pv.cmdline = NULL; > } > +libxl_defbool_setdefault(_info->u.pv.altp2m, false); > break; > default: > LOG(ERROR, "invalid domain type %s in create info", > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index ec29060..ab023a2 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -277,6 +277,16 @@ err: > } > #endif > > +#if defined(__arm__) || defined(__aarch64__) > +static void pv_set_conf_params(xc_interface *handle, uint32_t domid, > + libxl_domain_build_info *const info) > +{ > +if ( libxl_defbool_val(info->u.pv.altp2m) ) Coding style. > +xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M, > + libxl_defbool_val(info->u.pv.altp2m)); > +} > +#endif > + > static void hvm_set_conf_params(xc_interface *handle, uint32_t domid, > libxl_domain_build_info *const info) > { > @@ -433,6 +443,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, > return rc; > #endif > } > +#if defined(__arm__) || defined(__aarch64__) > +else /* info->type == LIBXL_DOMAIN_TYPE_PV */ > +pv_set_conf_params(ctx->xch, domid, info); > +#endif > > rc = libxl__arch_domain_create(gc, d_config, domid); > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index ef614be..0a164f9 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -554,6 +554,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ >("features", string, {'const': True}), ># Use host's E820 for PCI passthrough. >("e820_host", libxl_defbool), > + ("altp2m", libxl_defbool), Why is this placed in PV instead of arch_arm? You would also need a LIBXL_HAVE macro in libxl.h for the new field. >])), > ("invalid", None), > ], keyvar_init_val = "LIBXL_DOMAIN_TYPE_INVALID")), > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 6459eec..12c6e48 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -1718,6 +1718,11 @@ static void parse_config_data(const char > *config_source, > exit(1); > } > > +#if defined(__arm__) || defined(__aarch64__) > +/* Enable altp2m for PV guests solely on ARM */ > +xlu_cfg_get_defbool(config, "altp2m", _info->u.pv.altp2m, 0); > +#endif > + > break; > } > default: > -- > 2.8.3 > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 16/18] arm/altp2m: Extended libxl to activate altp2m on ARM.
The current implementation allows to set the parameter HVM_PARAM_ALTP2M. This parameter allows further usage of altp2m on ARM. For this, we define an additional altp2m field for PV domains as part of the libxl_domain_type struct. This field can be set only on ARM systems through the "altp2m" switch in the domain's configuration file (i.e. set altp2m=1). Signed-off-by: Sergej Proskurin--- Cc: Ian Jackson Cc: Wei Liu --- tools/libxl/libxl_create.c | 1 + tools/libxl/libxl_dom.c | 14 ++ tools/libxl/libxl_types.idl | 1 + tools/libxl/xl_cmdimpl.c| 5 + 4 files changed, 21 insertions(+) diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 1b99472..40b5f61 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -400,6 +400,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, b_info->cmdline = b_info->u.pv.cmdline; b_info->u.pv.cmdline = NULL; } +libxl_defbool_setdefault(_info->u.pv.altp2m, false); break; default: LOG(ERROR, "invalid domain type %s in create info", diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index ec29060..ab023a2 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -277,6 +277,16 @@ err: } #endif +#if defined(__arm__) || defined(__aarch64__) +static void pv_set_conf_params(xc_interface *handle, uint32_t domid, + libxl_domain_build_info *const info) +{ +if ( libxl_defbool_val(info->u.pv.altp2m) ) +xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M, + libxl_defbool_val(info->u.pv.altp2m)); +} +#endif + static void hvm_set_conf_params(xc_interface *handle, uint32_t domid, libxl_domain_build_info *const info) { @@ -433,6 +443,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, return rc; #endif } +#if defined(__arm__) || defined(__aarch64__) +else /* info->type == LIBXL_DOMAIN_TYPE_PV */ +pv_set_conf_params(ctx->xch, domid, info); +#endif rc = libxl__arch_domain_create(gc, d_config, domid); diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index ef614be..0a164f9 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -554,6 +554,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("features", string, {'const': True}), # Use host's E820 for PCI passthrough. ("e820_host", libxl_defbool), + ("altp2m", libxl_defbool), ])), ("invalid", None), ], keyvar_init_val = "LIBXL_DOMAIN_TYPE_INVALID")), diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 6459eec..12c6e48 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -1718,6 +1718,11 @@ static void parse_config_data(const char *config_source, exit(1); } +#if defined(__arm__) || defined(__aarch64__) +/* Enable altp2m for PV guests solely on ARM */ +xlu_cfg_get_defbool(config, "altp2m", _info->u.pv.altp2m, 0); +#endif + break; } default: -- 2.8.3 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 16/18] arm/altp2m: Extended libxl to activate altp2m on ARM.
The current implementation allows to set the parameter HVM_PARAM_ALTP2M. This parameter allows further usage of altp2m on ARM. For this, we define an additional altp2m field for PV domains as part of the libxl_domain_type struct. This field can be set only on ARM systems through the "altp2m" switch in the domain's configuration file (i.e. set altp2m=1). Signed-off-by: Sergej Proskurin--- Cc: Ian Jackson Cc: Wei Liu --- tools/libxl/libxl_create.c | 1 + tools/libxl/libxl_dom.c | 14 ++ tools/libxl/libxl_types.idl | 1 + tools/libxl/xl_cmdimpl.c| 5 + 4 files changed, 21 insertions(+) diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 1b99472..40b5f61 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -400,6 +400,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, b_info->cmdline = b_info->u.pv.cmdline; b_info->u.pv.cmdline = NULL; } +libxl_defbool_setdefault(_info->u.pv.altp2m, false); break; default: LOG(ERROR, "invalid domain type %s in create info", diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index ec29060..ab023a2 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -277,6 +277,16 @@ err: } #endif +#if defined(__arm__) || defined(__aarch64__) +static void pv_set_conf_params(xc_interface *handle, uint32_t domid, + libxl_domain_build_info *const info) +{ +if ( libxl_defbool_val(info->u.pv.altp2m) ) +xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M, + libxl_defbool_val(info->u.pv.altp2m)); +} +#endif + static void hvm_set_conf_params(xc_interface *handle, uint32_t domid, libxl_domain_build_info *const info) { @@ -433,6 +443,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, return rc; #endif } +#if defined(__arm__) || defined(__aarch64__) +else /* info->type == LIBXL_DOMAIN_TYPE_PV */ +pv_set_conf_params(ctx->xch, domid, info); +#endif rc = libxl__arch_domain_create(gc, d_config, domid); diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index ef614be..0a164f9 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -554,6 +554,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("features", string, {'const': True}), # Use host's E820 for PCI passthrough. ("e820_host", libxl_defbool), + ("altp2m", libxl_defbool), ])), ("invalid", None), ], keyvar_init_val = "LIBXL_DOMAIN_TYPE_INVALID")), diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 6459eec..12c6e48 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -1718,6 +1718,11 @@ static void parse_config_data(const char *config_source, exit(1); } +#if defined(__arm__) || defined(__aarch64__) +/* Enable altp2m for PV guests solely on ARM */ +xlu_cfg_get_defbool(config, "altp2m", _info->u.pv.altp2m, 0); +#endif + break; } default: -- 2.8.3 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel