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

[Shannon Zhao <zhaoshenglong@xxxxxxxxxx>]

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.

Thanks,
-- 
Shannon


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