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

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

Hi Shannon,

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.

Regards,

-- 
Julien Grall

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