[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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |