[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v2 07/17] libxl/arm: Construct ACPI GTDT	table
 
 
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@xxxxxxxxxx>
Construct GTDT table with the interrupt information of timers.
Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
---
    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/?
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
 
 
    
     |