Re: [Xen-devel] [PATCH v2 03/17] libxl/arm: Add a configuration option for ARM DomU ACPI

[Julien Grall <julien.grall@xxxxxxx>]

On 13/07/2016 10:48, Shannon Zhao wrote:
> On 2016/7/13 17:20, Julien Grall wrote:
> > On 13/07/2016 08:54, Shannon Zhao wrote:
> > > On 2016/7/12 19:33, Wei Liu wrote:
> > > > On Tue, Jul 12, 2016 at 10:22:39AM +0100, Julien Grall wrote:
> > > > [...]
> > > > > > > Yeah, we can deprecate that field. But we need to take care to not
> > > > > > > break
> > > > > > > users of the old field.
> > > > > > Ok, what name would you suggest?
> > > > > I would suggest b_info->u.acpi
> > > > b_info->acpi would be more appropriate.
> > > >
> > > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > > > index ef614be..a57823d 100644
> > > > --- a/tools/libxl/libxl_types.idl
> > > > +++ b/tools/libxl/libxl_types.idl
> > > > @@ -494,11 +494,16 @@ 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),
> > > > +    ("acpi",             libxl_defbool),
> > > >      ("u", KeyedUnion(None, libxl_domain_type, "type",
> > > >                  [("hvm", Struct(None, [("firmware",         string),
> > > >                                         ("bios",
> > > > libxl_bios_type),
> > > >                                         ("pae",
> > > > libxl_defbool),
> > > >                                         ("apic",
> > > > libxl_defbool),
> > > > +                                       # The following acpi field is
> > > > +                                       # deprecated. Please use the
> > > > unified
> > > > +                                       # acpi field above which
> > > > works for both
> > > > +                                       # x86 and ARM.
> > > >                                         ("acpi",
> > > > libxl_defbool),
> > > >                                         ("acpi_s3",
> > > > libxl_defbool),
> > > >                                         ("acpi_s4",
> > > > libxl_defbool),
> > > >
> > > >
> > > > And then:
> > > >
> > > > 1. modify xl to set the new field.
> > > > 2. modify libxl to handle compatibility: user of the old field should
> > > >    continue to work.
> > > I found that the default value of acpi is true on x86. But we decided
> > > before it's should be false by default on ARM. How to deal with this?
> > > Julien, Stefano, can we make acpi true by default?
> > I already said that I am not in favor of making ACPI true by default at
> > least for ARM 32-bit guest.
> >
> > ARM 32-bit guest will not use ACPI, if we decide to enable it by
> > default, we will require the user to install iasl for nothing.
> >
> > IHMO, ACPI should be disabled by default for any ARM guests. Libxl can
> > take this decision easily.
> I know but here we want to unify the acpi option for x86 and ARM while
> on x86 it's true by default. What I want to ask is that how to
> distinguish x86 and ARM in libxl__domain_build_info_setdefault(), so we
> can assign acpi with different default value for x86 and ARM.
By using #ifdef in the code?

Regards,

--
Julien Grall

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