Re: [Xen-devel] [PATCH v4 06/21] arm/acpi: Parse FADT table and get PSCI flags

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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 <shannon.zhao@xxxxxxxxxx>
> >>
> >> 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 <hanjun.guo@xxxxxxxxxx>
> >> Signed-off-by: Naresh Bhat <naresh.bhat@xxxxxxxxxx>
> >> Signed-off-by: Parth Dixit <parth.dixit@xxxxxxxxxx>
> >> Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> >> ---
> >> 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 <xen/init.h>
> >>  #include <xen/acpi.h>
> >> +#include <xen/errno.h>
> >> +#include <acpi/actables.h>
> >> +#include <xen/mm.h>
> >>  
> >>  #include <asm/acpi.h>
> >>  
> >> +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 */

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel