|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC 02/35] xen: arm64: ACPI: Support common ACPI drivers
Hi Parth, On 04/02/2015 14:01, parth.dixit@xxxxxxxxxx wrote: From: Naresh Bhat <naresh.bhat@xxxxxxxxxx> xen hypervisor will use ACPI for initialisation in the same manner that current x86/x86_64 ones do. Add the calls to initialise the ACPI tables and load devices using the xen/drivers/acpi subsytem. All changes in this patch are mostly unrelated, and most of them requires an explanation why we need to it. I would split this patch in small chunk. See below for others comment. #if defined(CONFIG_X86) && defined(HAS_ACPI) ? Also, this change seems to be related to the patch #1.I would make sense to create a separate patch which will disable the compilation of pmstate and adding this #ifdef. I remembered to have a discussion about this change with Naresh few month ago. __va should only be used when the memory is direct-mapped to Xen (i.e accessible directly). On ARM64, this only the case for the RAM. Can you confirm that ACPI will always reside to the RAM? Futhermore, the code of this function seems x86-specific. The low 1MB may not be mapped on ARM64. I would move the whole function (acpi_os_map_memory) per-architecture. +#endif } Why only x86? Linux seems to define it also for ARM64. acpi_status acpi_os_read_memory(acpi_physical_address phys_addr, u32 * value, u32 width) diff --git a/xen/drivers/acpi/utilities/utglobal.c b/xen/drivers/acpi/utilities/utglobal.c index 7dbc964..65c918e 100644 --- a/xen/drivers/acpi/utilities/utglobal.c +++ b/xen/drivers/acpi/utilities/utglobal.c @@ -43,6 +43,7 @@ #define DEFINE_ACPI_GLOBALS +#include <asm/system.h> Why it's necessary? #include <xen/config.h> #include <xen/init.h> #include <xen/lib.h> diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h new file mode 100644 index 0000000..f6284b5 --- /dev/null +++ b/xen/include/asm-arm/acpi.h @@ -0,0 +1,106 @@ +/* + * Copyright (C) 2014, Naresh Bhat <naresh.bhat@xxxxxxxxxx> + * + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + */ + +#ifndef _ASM_ARM64_ACPI_H +#define _ASM_ARM64_ACPI_H + +#include <xen/init.h> + +#define COMPILER_DEPENDENT_INT64 long long +#define COMPILER_DEPENDENT_UINT64 unsigned long long + +#define MAX_LOCAL_APIC 256 +#define MAX_IO_APICS 64 + +/* + * Calling conventions: + * + * ACPI_SYSTEM_XFACE - Interfaces to host OS (handlers, threads) + * ACPI_EXTERNAL_XFACE - External ACPI interfaces + * ACPI_INTERNAL_XFACE - Internal ACPI interfaces + * ACPI_INTERNAL_VAR_XFACE - Internal variable-parameter list interfaces + */ +#define ACPI_SYSTEM_XFACE +#define ACPI_EXTERNAL_XFACE +#define ACPI_INTERNAL_XFACE +#define ACPI_INTERNAL_VAR_XFACE + +/* Asm macros */ +#define ACPI_ASM_MACROS +#define BREAKPOINT3 It's never used on common code. +#define ACPI_DISABLE_IRQS() local_irq_disable() +#define ACPI_ENABLE_IRQS() local_irq_enable() +#define ACPI_FLUSH_CPU_CACHE() flush_cache_all() + +/* Blob handling macros */ +#define ACPI_BLOB_HEADER_SIZE 8 + +/* Basic configuration for ACPI */ +#ifdef CONFIG_ACPI +extern int acpi_disabled; +extern int acpi_noirq; +extern int acpi_pci_disabled; +extern int acpi_strict; I think it would be better to define common external variable in a common headers. Although I'm an ACPI maintainers... Can you document this structure? +extern struct acpi_arm_root acpi_arm_rsdp_info; This external variable is defined but seem to never be used in the whole series. +void arm_acpi_reserve_memory(void); + +/* Low-level suspend routine. */ +extern int (*acpi_suspend_lowlevel)(void); + +extern void prefill_possible_map(void); Those 3 functions are never implemented neither used. +#define acpi_wakeup_address (0) This macro is never used. Ditto for acpi_noirq_set and acpi_disable_pci +#else /* !CONFIG_ACPI */ +#define acpi_disabled 1 /* ACPI sometimes enabled on ARM */ +#define acpi_noirq 1 /* ACPI sometimes enabled on ARM */ +#define acpi_pci_disabled 1 /* ACPI PCI sometimes enabled on ARM */ +#define acpi_strict 1 /* no ACPI spec workarounds on ARM */ The comments are unclear to me here. +#endif + +#endif /*_ASM_ARM_ACPI_H*/ diff --git a/xen/include/asm-arm/arm64/page.h b/xen/include/asm-arm/arm64/page.h index 1fd416d..13b0ea1 100644 --- a/xen/include/asm-arm/arm64/page.h +++ b/xen/include/asm-arm/arm64/page.h @@ -1,6 +1,8 @@ #ifndef __ARM_ARM64_PAGE_H__ #define __ARM_ARM64_PAGE_H__ +#include <asm-arm/system.h> + Why? Regards, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |