Re: [Xen-devel] [PATCH 16/18] arm/altp2m: Extended libxl to activate altp2m on ARM.

[Julien Grall <julien.grall@xxxxxxx>]

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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel