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

[Julien Grall <julien.grall@arm.com>]

Hi Shannon,

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.

Regards,

-- 
Julien Grall

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