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 18:03, Julien Grall wrote:
> 
> 
> 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?
Maybe this could not work since CONFIG_ARM can not be accessed in libxl
in current codes. I'm not sure why it can't work. Wei, do you have any
suggestion?

Thanks,
-- 
Shannon


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