diff mbox

[Xen-devel,v2,07/17] libxl/arm: Construct ACPI GTDT table

Message ID 1466651824-6964-8-git-send-email-zhaoshenglong@huawei.com
State New
Headers show

Commit Message

Shannon Zhao June 23, 2016, 3:16 a.m. UTC
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(+)

Comments

Julien Grall June 23, 2016, 4:19 p.m. UTC | #1
Hi,

On 23/06/16 16:00, Stefano Stabellini wrote:
> On Thu, 23 Jun 2016, Shannon Zhao wrote:

[...]

>> +static void make_acpi_gtdt(libxl__gc *gc, struct xc_dom_image *dom)
>> +{
>> +    struct acpi_table_gtdt *gtdt;
>> +    size_t size = sizeof(*gtdt);
>> +
>> +    gtdt = libxl__zalloc(gc, size);
>> +
>> +    gtdt->non_secure_el1_interrupt = GUEST_TIMER_PHYS_NS_PPI;
>> +    gtdt->non_secure_el1_flags =
>> +                             (ACPI_LEVEL_SENSITIVE << ACPI_GTDT_INTERRUPT_MODE)
>> +                             |(ACPI_ACTIVE_LOW << ACPI_GTDT_INTERRUPT_POLARITY);
>> +    gtdt->virtual_timer_interrupt = GUEST_TIMER_VIRT_PPI;
>> +    gtdt->virtual_timer_flags =
>> +                             (ACPI_LEVEL_SENSITIVE << ACPI_GTDT_INTERRUPT_MODE)
>> +                             |(ACPI_ACTIVE_LOW << ACPI_GTDT_INTERRUPT_POLARITY);
>> +
>> +    make_acpi_header(&gtdt->header, "GTDT", size, 2);
>> +
>> +    acpitables[GTDT].table = gtdt;
>> +    acpitables[GTDT].size = size;
>> +    /* Align to 64bit. */
>> +    dom->acpitable_size += ROUNDUP(acpitables[GTDT].size, 3);
>> +}
>
> Many of this tables look pretty much static. Any reason why we can't
> define them and initialize them on an header somewhere like:
>
>      struct acpi_gtdt xen_acpi_gtdt {
>          .non_secure_el1_interrupt = GUEST_TIMER_PHYS_NS_PPI;
>          .non_secure_el1_flags = (ACPI_LEVEL_SENSITIVE << ACPI_GTDT_INTERRUPT_MODE) | (ACPI_ACTIVE_LOW << ACPI_GTDT_INTERRUPT_POLARITY);
>          .virtual_timer_interrupt = GUEST_TIMER_VIRT_PPI;
>          .virtual_timer_flags = (ACPI_LEVEL_SENSITIVE << ACPI_GTDT_INTERRUPT_MODE)|(ACPI_ACTIVE_LOW << ACPI_GTDT_INTERRUPT_POLARITY);
>      };
>
> it would make the code shorter and easier to read.

I agree with Stefano on this point.

>>   int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>>                           libxl__domain_build_state *state,
>>                           struct xc_dom_image *dom)
>> @@ -132,6 +159,7 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>>
>>       make_acpi_rsdp(gc, dom);
>>       make_acpi_xsdt(gc, dom);
>> +    make_acpi_gtdt(gc, dom);
>>
>>       return 0;
>>   }
>> --
>> 2.0.4
>>
>>
>

Regards,
Julien Grall June 23, 2016, 4:26 p.m. UTC | #2
Hi Shannon,

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?

>   enum {
>       RSDP,
>       XSDT,
> @@ -110,6 +113,30 @@ static void make_acpi_xsdt(libxl__gc *gc, struct xc_dom_image *dom)
>       dom->acpitable_size += ROUNDUP(acpitables[XSDT].size, 3);
>   }
>
> +static void make_acpi_gtdt(libxl__gc *gc, struct xc_dom_image *dom)
> +{
> +    struct acpi_table_gtdt *gtdt;
> +    size_t size = sizeof(*gtdt);
> +
> +    gtdt = libxl__zalloc(gc, size);
> +
> +    gtdt->non_secure_el1_interrupt = GUEST_TIMER_PHYS_NS_PPI;
> +    gtdt->non_secure_el1_flags =
> +                             (ACPI_LEVEL_SENSITIVE << ACPI_GTDT_INTERRUPT_MODE)
> +                             |(ACPI_ACTIVE_LOW << ACPI_GTDT_INTERRUPT_POLARITY);
> +    gtdt->virtual_timer_interrupt = GUEST_TIMER_VIRT_PPI;
> +    gtdt->virtual_timer_flags =
> +                             (ACPI_LEVEL_SENSITIVE << ACPI_GTDT_INTERRUPT_MODE)
> +                             |(ACPI_ACTIVE_LOW << ACPI_GTDT_INTERRUPT_POLARITY);
> +
> +    make_acpi_header(&gtdt->header, "GTDT", size, 2);
> +
> +    acpitables[GTDT].table = gtdt;
> +    acpitables[GTDT].size = size;
> +    /* Align to 64bit. */
> +    dom->acpitable_size += ROUNDUP(acpitables[GTDT].size, 3);

I am not sure how ROUNDUP(..., 3) will align to 64-bit.

> +}
> +
>   int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>                           libxl__domain_build_state *state,
>                           struct xc_dom_image *dom)
> @@ -132,6 +159,7 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>
>       make_acpi_rsdp(gc, dom);
>       make_acpi_xsdt(gc, dom);
> +    make_acpi_gtdt(gc, dom);
>
>       return 0;
>   }
>

Regards,
Julien Grall June 23, 2016, 4:58 p.m. UTC | #3
On 23/06/16 17:26, Julien Grall wrote:
>>   enum {
>>       RSDP,
>>       XSDT,
>> @@ -110,6 +113,30 @@ static void make_acpi_xsdt(libxl__gc *gc, struct
>> xc_dom_image *dom)
>>       dom->acpitable_size += ROUNDUP(acpitables[XSDT].size, 3);
>>   }
>>
>> +static void make_acpi_gtdt(libxl__gc *gc, struct xc_dom_image *dom)
>> +{
>> +    struct acpi_table_gtdt *gtdt;
>> +    size_t size = sizeof(*gtdt);
>> +
>> +    gtdt = libxl__zalloc(gc, size);
>> +
>> +    gtdt->non_secure_el1_interrupt = GUEST_TIMER_PHYS_NS_PPI;
>> +    gtdt->non_secure_el1_flags =
>> +                             (ACPI_LEVEL_SENSITIVE <<
>> ACPI_GTDT_INTERRUPT_MODE)
>> +                             |(ACPI_ACTIVE_LOW <<
>> ACPI_GTDT_INTERRUPT_POLARITY);
>> +    gtdt->virtual_timer_interrupt = GUEST_TIMER_VIRT_PPI;
>> +    gtdt->virtual_timer_flags =
>> +                             (ACPI_LEVEL_SENSITIVE <<
>> ACPI_GTDT_INTERRUPT_MODE)
>> +                             |(ACPI_ACTIVE_LOW <<
>> ACPI_GTDT_INTERRUPT_POLARITY);
>> +
>> +    make_acpi_header(&gtdt->header, "GTDT", size, 2);
>> +
>> +    acpitables[GTDT].table = gtdt;
>> +    acpitables[GTDT].size = size;
>> +    /* Align to 64bit. */
>> +    dom->acpitable_size += ROUNDUP(acpitables[GTDT].size, 3);
>
> I am not sure how ROUNDUP(..., 3) will align to 64-bit.

Hmmm I found why. The ROUNDUP macro in libxl is taking an order which is 
odd.

Cheers,
Julien Grall June 27, 2016, 10:17 a.m. UTC | #4
Hi Shannon,

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

Wei, Ian, Stefano, do you have any opinions?

Regards,
Julien Grall June 29, 2016, 9:42 a.m. UTC | #5
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,
Shannon Zhao June 29, 2016, 1:41 p.m. UTC | #6
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.
Julien Grall July 6, 2016, 9:50 a.m. UTC | #7
Hi Stefano,

On 05/07/16 17:42, Stefano Stabellini wrote:
> On Mon, 27 Jun 2016, 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
>>
>> Wei, Ian, Stefano, do you have any opinions?
>
> I think you are right that we should avoid redefining
> ACPI_LEVEL_SENSITIVE and ACPI_ACTIVE_LOW. But I think if possible we
> should also avoid redefining ACPI_MACHINE_WIDTH and
> COMPILER_DEPENDENT_INT64. If possible, we should include the header file
> with those definitions too (acpi/platform/acenv.h ?).

acenv.h is including aclinux.h with contains a lot of hypervisor 
specific include. So we would need to rework the include to use it in 
the toolstack.

Or maybe this can be found in /usr/include/?
diff mbox

Patch

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
+
 enum {
     RSDP,
     XSDT,
@@ -110,6 +113,30 @@  static void make_acpi_xsdt(libxl__gc *gc, struct xc_dom_image *dom)
     dom->acpitable_size += ROUNDUP(acpitables[XSDT].size, 3);
 }
 
+static void make_acpi_gtdt(libxl__gc *gc, struct xc_dom_image *dom)
+{
+    struct acpi_table_gtdt *gtdt;
+    size_t size = sizeof(*gtdt);
+
+    gtdt = libxl__zalloc(gc, size);
+
+    gtdt->non_secure_el1_interrupt = GUEST_TIMER_PHYS_NS_PPI;
+    gtdt->non_secure_el1_flags =
+                             (ACPI_LEVEL_SENSITIVE << ACPI_GTDT_INTERRUPT_MODE)
+                             |(ACPI_ACTIVE_LOW << ACPI_GTDT_INTERRUPT_POLARITY);
+    gtdt->virtual_timer_interrupt = GUEST_TIMER_VIRT_PPI;
+    gtdt->virtual_timer_flags =
+                             (ACPI_LEVEL_SENSITIVE << ACPI_GTDT_INTERRUPT_MODE)
+                             |(ACPI_ACTIVE_LOW << ACPI_GTDT_INTERRUPT_POLARITY);
+
+    make_acpi_header(&gtdt->header, "GTDT", size, 2);
+
+    acpitables[GTDT].table = gtdt;
+    acpitables[GTDT].size = size;
+    /* Align to 64bit. */
+    dom->acpitable_size += ROUNDUP(acpitables[GTDT].size, 3);
+}
+
 int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
                         libxl__domain_build_state *state,
                         struct xc_dom_image *dom)
@@ -132,6 +159,7 @@  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
 
     make_acpi_rsdp(gc, dom);
     make_acpi_xsdt(gc, dom);
+    make_acpi_gtdt(gc, dom);
 
     return 0;
 }