From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH v4 06/21] arm/acpi: Parse FADT table and get PSCI flags Date: Tue, 23 Feb 2016 14:37:33 +0000 Message-ID: References: <1453540813-15764-1-git-send-email-zhaoshenglong@huawei.com> <1453540813-15764-7-git-send-email-zhaoshenglong@huawei.com> <56CC4CCC.2070004@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <56CC4CCC.2070004@huawei.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Shannon Zhao Cc: ian.campbell@citrix.com, Stefano Stabellini , peter.huangpeng@huawei.com, xen-devel@lists.xen.org, julien.grall@citrix.com, stefano.stabellini@citrix.com, shannon.zhao@linaro.org List-Id: xen-devel@lists.xenproject.org On Tue, 23 Feb 2016, Shannon Zhao wrote: > On 2016/1/27 23:41, Stefano Stabellini wrote: > > On Sat, 23 Jan 2016, Shannon Zhao wrote: > >> From: Shannon Zhao > >> > >> There are two flags: PSCI_COMPLIANT and PSCI_USE_HVC. When set, the > >> former signals to the OS that the hardware is PSCI compliant. The latter > >> selects the appropriate conduit for PSCI calls by toggling between > >> Hypervisor Calls (HVC) and Secure Monitor Calls (SMC). FADT table > >> contains such information, parse FADT to get the flags for furture > >> usage. > >> > >> Since STAO table and the GIC version are introduced by ACPI 6.0, we will > >> check the version and only parse FADT table with version >= 6.0. If > >> firmware provides ACPI tables with ACPI version less than 6.0, OS will > >> be messed up with those information, so disable ACPI if we get an FADT > >> table with version less than 6.0. > >> > >> Signed-off-by: Hanjun Guo > >> Signed-off-by: Naresh Bhat > >> Signed-off-by: Parth Dixit > >> Signed-off-by: Shannon Zhao > >> --- > >> V4: drop disable_acpi in acpi_parse_fadt > >> --- > >> xen/arch/arm/acpi/boot.c | 30 ++++++++++++++++++++++++++++++ > >> xen/arch/arm/acpi/lib.c | 12 ++++++++++++ > >> xen/include/asm-arm/acpi.h | 9 +++++++++ > >> 3 files changed, 51 insertions(+) > >> > >> diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c > >> index 1570f7e..6b33fbe 100644 > >> --- a/xen/arch/arm/acpi/boot.c > >> +++ b/xen/arch/arm/acpi/boot.c > >> @@ -27,9 +27,32 @@ > >> > >> #include > >> #include > >> +#include > >> +#include > >> +#include > >> > >> #include > >> > >> +static int __init acpi_parse_fadt(struct acpi_table_header *table) > >> +{ > >> + struct acpi_table_fadt *fadt = (struct acpi_table_fadt *)table; > >> + > >> + /* > >> + * Revision in table header is the FADT Major revision, and there > >> + * is a minor revision of FADT which was introduced by ACPI 6.0, > >> + * we only deal with ACPI 6.0 or newer revision to get GIC and SMP > >> + * boot protocol configuration data, or we will disable ACPI. > >> + */ > >> + if ( table->revision > 6 > >> + || (table->revision == 6 && fadt->minor_revision >= 0) ) > >> + return 0; > >> + > >> + printk("Unsupported FADT revision %d.%d, should be 6.0+, will disable ACPI\n", > >> + table->revision, fadt->minor_revision); > >> + > >> + return -EINVAL; > >> +} > >> + > >> /* > >> * acpi_boot_table_init() called from setup_arch(), always. > >> * 1. find RSDP and get its address, and then find XSDT > >> @@ -54,5 +77,12 @@ int __init acpi_boot_table_init(void) > >> return error; > >> } > >> > >> + if ( acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt) ) > >> + { > >> + /* disable ACPI if no FADT is found */ > >> + disable_acpi(); > >> + printk("Can't find FADT\n"); > >> + } > >> + > >> return 0; > >> } > >> diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c > >> index f817fe6..a30e4e6 100644 > >> --- a/xen/arch/arm/acpi/lib.c > >> +++ b/xen/arch/arm/acpi/lib.c > >> @@ -50,3 +50,15 @@ char *__acpi_map_table(paddr_t phys, unsigned long size) > >> > >> return ((char *) base + offset); > >> } > >> + > >> +/* 1 to indicate PSCI 0.2+ is implemented */ > >> +bool_t __init acpi_psci_present(void) > >> +{ > >> + return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_COMPLIANT; > >> +} > >> + > >> +/* 1 to indicate HVC is present instead of SMC as the PSCI conduit */ > >> +bool_t __init acpi_psci_hvc_present(void) > >> +{ > >> + return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_USE_HVC; > >> +} > > > > So far so good. > > > > > >> diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h > >> index 6a037c9..1ce88f8 100644 > >> --- a/xen/include/asm-arm/acpi.h > >> +++ b/xen/include/asm-arm/acpi.h > >> @@ -31,6 +31,15 @@ > >> #define ACPI_MAP_MEM_ATTR PAGE_HYPERVISOR > >> > >> extern bool_t acpi_disabled; > >> + > >> +#ifdef CONFIG_ACPI > >> +bool_t __init acpi_psci_present(void); > >> +bool_t __init acpi_psci_hvc_present(void); > >> +#else > >> +static inline bool_t acpi_psci_present(void) { return false; } > >> +static inline bool_t acpi_psci_hvc_present(void) {return false; } > >> +#endif /* CONFIG_ACPI */ > >> + > >> /* Basic configuration for ACPI */ > >> static inline void disable_acpi(void) > >> { > > > > I would prefer if we only defined each function once, outside the ifdef > > (no static inline needed). Then we could > > > > #ifdef CONFIG_ACPI > > extern bool_t acpi_disabled; > > #else > > #define acpi_disabled (1) > > #endif > > > Yes, we could do this to drop the #else (CONFIG_ACPI) case in some > places. But I think it still needs to stub out acpi_psci_present and > acpi_psci_hvc_present because they are used in some codes which are not > covered by #ifdef CONFIG_ACPI, see[1]. The file psci.c will be compiled > whether ACPI is enabled or not. > > [1] [PATCH v4 09/21] arm/acpi: Add ACPI support for SMP initialization Just to clarify, see the appended patch, generated on top of ACPI_XEN_ARM_V3. With that I can compile Xen both arm64 (ACPI) and arm32 (non-ACPI). diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 1a40323..0a2b64e 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -52,7 +52,9 @@ struct bootinfo __initdata bootinfo; struct cpuinfo_arm __read_mostly boot_cpu_data; +#ifdef CONFIG_ACPI bool_t __read_mostly acpi_disabled; +#endif #ifdef CONFIG_ARM_32 static unsigned long opt_xenheap_megabytes __initdata; @@ -101,7 +103,7 @@ static void __init parse_acpi_param(char *s) } else if ( !strcmp(s, "force") ) { - acpi_disabled = 0; + enable_acpi(); } } diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h index 628f3c8..5fe5a1d 100644 --- a/xen/include/asm-arm/acpi.h +++ b/xen/include/asm-arm/acpi.h @@ -29,7 +29,6 @@ #define COMPILER_DEPENDENT_INT64 long long #define COMPILER_DEPENDENT_UINT64 unsigned long long -extern bool_t acpi_disabled; /* Tables marked as reserved in efi table */ typedef enum { @@ -43,22 +42,14 @@ typedef enum { TBL_MMAX, } EFI_MEM_RES; -#ifdef CONFIG_ACPI bool_t __init acpi_psci_present(void); bool_t __init acpi_psci_hvc_present(void); void __init acpi_smp_init_cpus(void); unsigned int acpi_get_irq_type(u32 flags); unsigned int acpi_get_table_offset(struct membank tbl_add[], EFI_MEM_RES index); -#else -static inline bool_t acpi_psci_present(void) { return false; } -static inline bool_t acpi_psci_hvc_present(void) {return false; } -static inline void acpi_smp_init_cpus(void) { } -static inline unsigned int acpi_get_irq_type(u32 flags) { return 0; } -unsigned int acpi_get_table_offset(struct membank tbl_add[], EFI_MEM_RES index) -{ - return 0; -} -#endif /* CONFIG_ACPI */ + +#ifdef CONFIG_ACPI +extern bool_t acpi_disabled; /* Basic configuration for ACPI */ static inline void disable_acpi(void) @@ -66,6 +57,18 @@ static inline void disable_acpi(void) acpi_disabled = 1; } +static inline void enable_acpi(void) +{ + acpi_disabled = 0; +} +#else + +#define acpi_disabled (1) +#define disable_acpi(x) +#define enable_acpi(x) + +#endif /* CONFIG_ACPI */ + #define ACPI_GTDT_INTR_MASK ( ACPI_GTDT_INTERRUPT_MODE | ACPI_GTDT_INTERRUPT_POLARITY ) /* Triggering */