Re: [PATCH v2 07/17] libxl/arm: Construct ACPI GTDT table

[Shannon Zhao <shannon.zhao@linaro.org>]

On 2016年06月29日 17:42, Julien Grall wrote:
> On 29/06/2016 10:29, Shannon Zhao wrote:
>>
>>
>> On 2016/6/27 18:17, Julien Grall wrote:
>>> On 27/06/16 02:44, Shannon Zhao wrote:
>>>> On 2016/6/24 0:26, Julien Grall wrote:
>>>>> On 23/06/16 04:16, Shannon Zhao wrote:
>>>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>>>
>>>>>> Construct GTDT table with the interrupt information of timers.
>>>>>>
>>>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>>>> ---
>>>>>>    tools/libxl/libxl_arm_acpi.c | 28 ++++++++++++++++++++++++++++
>>>>>>    1 file changed, 28 insertions(+)
>>>>>>
>>>>>> diff --git a/tools/libxl/libxl_arm_acpi.c
>>>>>> b/tools/libxl/libxl_arm_acpi.c
>>>>>> index d5ffedf..de863f4 100644
>>>>>> --- a/tools/libxl/libxl_arm_acpi.c
>>>>>> +++ b/tools/libxl/libxl_arm_acpi.c
>>>>>> @@ -39,6 +39,9 @@ typedef uint64_t u64;
>>>>>>    #define ACPI_BUILD_APPNAME6 "XenARM"
>>>>>>    #define ACPI_BUILD_APPNAME4 "Xen "
>>>>>>
>>>>>> +#define ACPI_LEVEL_SENSITIVE            (u8) 0x00
>>>>>> +#define ACPI_ACTIVE_LOW                 (u8) 0x01
>>>>>> +
>>>>>
>>>>> Why did not you include actypes.h rather than define these two
>>>>> defines?
>>>> If we include actypes.h, there will be some compiling errors.
>>>>
>>>> ../../xen/include/acpi/actypes.h:55:2: error: #error ACPI_MACHINE_WIDTH
>>>> not defined
>>>>   #error ACPI_MACHINE_WIDTH not defined
>>>>    ^
>>>> ../../xen/include/acpi/actypes.h:130:9: error: unknown type name
>>>> 'COMPILER_DEPENDENT_UINT64'
>>>>   typedef COMPILER_DEPENDENT_UINT64 UINT64;
>>>>           ^
>>>> ../../xen/include/acpi/actypes.h:131:9: error: unknown type name
>>>> 'COMPILER_DEPENDENT_INT64'
>>>>   typedef COMPILER_DEPENDENT_INT64 INT64;
>>>>           ^
>>>> ../../xen/include/acpi/actypes.h:202:2: error: #error unknown
>>>> ACPI_MACHINE_WIDTH
>>>>   #error unknown ACPI_MACHINE_WIDTH
>>>>    ^
>>>> ../../xen/include/acpi/actypes.h:207:9: error: unknown type name
>>>> 'acpi_native_uint'
>>>>   typedef acpi_native_uint acpi_size;
>>>>           ^
>>>> ../../xen/include/acpi/actypes.h:617:3: error: unknown type name
>>>> 'acpi_io_address'
>>>>     acpi_io_address pblk_address;
>>>>
>>>> Yeah, it maybe can be solved by defining ACPI_MACHINE_WIDTH and
>>>> COMPILER_DEPENDENT_INT64 here, but since we only needs
>>>> ACPI_LEVEL_SENSITIVE and ACPI_ACTIVE_LOW, I think it's ok to define
>>>> them
>>>> here.
>>>
>>> We should avoid to redefine value as much as possible. The 2 missing
>>> values are easy to define (see below) so there is no point to redefine
>>> in a less obvious way: no comment to explain what the values are for,
>>> and only a part of the set defined.
>>>
>>> #define ACPI_MACHINE_WIDTH BITS_PER_LONG
>>> #define COMPILER_DEPENDENT_INT64 int64_t
>>>
>> Actually not work. I add below codes but get compiling errors as well.
>> So it needs the BITS_PER_LONG but it exists asm-arm/config.h
> 
> Give a look to bitperlongs.h in /usr/include. Although the name is
> __BITS_PER_LONG.
Oh, Thanks. I'm wondering why other codes define own BITS_PER_LONG
rather than use __BITS_PER_LONG directly.

-- 
Shannon

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