[PATCH 0/5] ARM64: ACPI core updates

[PATCH 0/5] ARM64: ACPI core updates

[Lorenzo Pieralisi]

This patch series implements some refactoring/clean-ups of arm64 ACPI
core code implemented in Hanjun Guo's series:

http://www.spinics.net/lists/kernel/msg1953211.html

available at:

git://git.linaro.org/leg/acpi/acpi.git acpi-5.1-v11

Patch [1] is a proposal to move the arm64 GSI IRQ mapping layer to generic
ACPI code, in that the arm64 GSI layer is based on IRQ domains that are
generic data structures in the kernel that are not tied to an arch specific
implementation.

Patches [2-5] implement some fixes/clean-up of arm64 ACPI core code.

Tested on Juno v8 chip.

Lorenzo Pieralisi (5):
  ACPI: move arm64 GSI IRQ model to generic GSI IRQ layer
  ARM64: kernel: psci: factor out probe function
  ARM64: kernel: psci: let ACPI probe PSCI version
  ARM64: kernel: acpi: refactor ACPI tables init and checks
  ARM64: kernel: acpi: honour acpi=force command line parameter

 arch/arm64/Kconfig            |   1 +
 arch/arm64/include/asm/acpi.h |   3 -
 arch/arm64/kernel/acpi.c      | 167 +++++++++++++++++-------------------------
 arch/arm64/kernel/psci.c      |  54 +++++++++-----
 arch/arm64/kernel/setup.c     |   2 +-
 drivers/acpi/Kconfig          |   3 +
 drivers/acpi/Makefile         |   1 +
 drivers/acpi/gsi.c            | 105 ++++++++++++++++++++++++++
 drivers/irqchip/irq-gic.c     |   2 +
 9 files changed, 214 insertions(+), 124 deletions(-)
 create mode 100644 drivers/acpi/gsi.c

-- 
2.2.1

[PATCH 1/5] ACPI: move arm64 GSI IRQ model to generic GSI IRQ layer

[Lorenzo Pieralisi]

The code deployed to implement GSI linux IRQ numbers mapping on arm64 turns
out to be generic enough so that it can be moved to ACPI core code along
with its respective config option ACPI_GENERIC_GSI selectable on
architectures that can reuse the same code.

Current ACPI IRQ mapping code is not integrated in the kernel IRQ domain
infrastructure, in particular there is no way to look-up the
IRQ domain associated with a particular interrupt controller, so this
first version of GSI generic code carries out the GSI<->IRQ mapping relying
on the IRQ default domain which is supposed to be always set on a
specific architecture in case the domain structure passed to
irq_create/find_mapping() functions is missing.

This patch moves the arm64 acpi functions that implement the gsi mappings:

acpi_gsi_to_irq()
acpi_register_gsi()
acpi_unregister_gsi()

to ACPI core code. Since the generic GSI<->domain mapping is based on IRQ
domains, it can be extended as soon as a way to map an interrupt
controller to an IRQ domain is implemented for ACPI in the IRQ domain
layer.

x86 and ia64 code for GSI mappings cannot rely on the generic GSI
layer at present for legacy reasons, so they do not select the
ACPI_GENERIC_GSI config options and keep relying on their arch
specific GSI mapping layer.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Jiang Liu <jiang.liu@linux.intel.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/Kconfig        |   1 +
 arch/arm64/kernel/acpi.c  |  73 --------------------------------
 drivers/acpi/Kconfig      |   3 ++
 drivers/acpi/Makefile     |   1 +
 drivers/acpi/gsi.c        | 105 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/irqchip/irq-gic.c |   2 +
 6 files changed, 112 insertions(+), 73 deletions(-)
 create mode 100644 drivers/acpi/gsi.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index e5aa081..0659db3 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1,5 +1,6 @@
 config ARM64
 	def_bool y
+	select ACPI_GENERIC_GSI if ACPI
 	select ACPI_REDUCED_HARDWARE_ONLY if ACPI
 	select ARCH_BINFMT_ELF_RANDOMIZE_PIE
 	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index fe9d8f0..a70f714 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -76,12 +76,6 @@ static int __init dt_scan_depth1_nodes(unsigned long node,
 }
 
 /*
- * Since we're on ARM, the default interrupt routing model
- * clearly has to be GIC.
- */
-enum acpi_irq_model_id acpi_irq_model = ACPI_IRQ_MODEL_GIC;
-
-/*
  * __acpi_map_table() will be called before page_init(), so early_ioremap()
  * or early_memremap() should be called here to for ACPI table mapping.
  */
@@ -224,73 +218,6 @@ void __init acpi_init_cpus(void)
 	pr_info("%d CPUs enabled, %d CPUs total\n", enabled_cpus, total_cpus);
 }
 
-int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
-{
-	*irq = irq_find_mapping(NULL, gsi);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
-
-/*
- * success: return IRQ number (>0)
- * failure: return =< 0
- */
-int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity)
-{
-	unsigned int irq;
-	unsigned int irq_type;
-
-	/*
-	 * ACPI have no bindings to indicate SPI or PPI, so we
-	 * use different mappings from DT in ACPI.
-	 *
-	 * For FDT
-	 * PPI interrupt: in the range [0, 15];
-	 * SPI interrupt: in the range [0, 987];
-	 *
-	 * For ACPI, GSI should be unique so using
-	 * the hwirq directly for the mapping:
-	 * PPI interrupt: in the range [16, 31];
-	 * SPI interrupt: in the range [32, 1019];
-	 */
-
-	if (trigger == ACPI_EDGE_SENSITIVE &&
-				polarity == ACPI_ACTIVE_LOW)
-		irq_type = IRQ_TYPE_EDGE_FALLING;
-	else if (trigger == ACPI_EDGE_SENSITIVE &&
-				polarity == ACPI_ACTIVE_HIGH)
-		irq_type = IRQ_TYPE_EDGE_RISING;
-	else if (trigger == ACPI_LEVEL_SENSITIVE &&
-				polarity == ACPI_ACTIVE_LOW)
-		irq_type = IRQ_TYPE_LEVEL_LOW;
-	else if (trigger == ACPI_LEVEL_SENSITIVE &&
-				polarity == ACPI_ACTIVE_HIGH)
-		irq_type = IRQ_TYPE_LEVEL_HIGH;
-	else
-		irq_type = IRQ_TYPE_NONE;
-
-	/*
-	 * Since only one GIC is supported in ACPI 5.0, we can
-	 * create mapping refer to the default domain
-	 */
-	irq = irq_create_mapping(NULL, gsi);
-	if (!irq)
-		return irq;
-
-	/* Set irq type if specified and different than the current one */
-	if (irq_type != IRQ_TYPE_NONE &&
-		irq_type != irq_get_trigger_type(irq))
-		irq_set_irq_type(irq, irq_type);
-	return irq;
-}
-EXPORT_SYMBOL_GPL(acpi_register_gsi);
-
-void acpi_unregister_gsi(u32 gsi)
-{
-}
-EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
-
 static int __init acpi_parse_fadt(struct acpi_table_header *table)
 {
 	struct acpi_table_fadt *fadt = (struct acpi_table_fadt *)table;
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index a8f531e2..ab2cbb5 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -48,6 +48,9 @@ config ACPI_LEGACY_TABLES_LOOKUP
 config ARCH_MIGHT_HAVE_ACPI_PDC
 	bool
 
+config ACPI_GENERIC_GSI
+	bool
+
 config ACPI_SYSTEM_POWER_STATES_SUPPORT
 	bool
 
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index db153c6..8a063e2 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -56,6 +56,7 @@ ifdef CONFIG_ACPI_VIDEO
 acpi-y				+= video_detect.o
 endif
 acpi-y				+= acpi_lpat.o
+acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
 
 # These are (potentially) separate modules
 
diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
new file mode 100644
index 0000000..38208f2
--- /dev/null
+++ b/drivers/acpi/gsi.c
@@ -0,0 +1,105 @@
+/*
+ * ACPI GSI IRQ layer
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ * Author: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/acpi.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+
+enum acpi_irq_model_id acpi_irq_model;
+
+static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
+{
+	switch (polarity) {
+	case ACPI_ACTIVE_LOW:
+		return trigger == ACPI_EDGE_SENSITIVE ?
+		       IRQ_TYPE_EDGE_FALLING :
+		       IRQ_TYPE_LEVEL_LOW;
+	case ACPI_ACTIVE_HIGH:
+		return trigger == ACPI_EDGE_SENSITIVE ?
+		       IRQ_TYPE_EDGE_RISING :
+		       IRQ_TYPE_LEVEL_HIGH;
+	case ACPI_ACTIVE_BOTH:
+		if (trigger == ACPI_EDGE_SENSITIVE)
+			return IRQ_TYPE_EDGE_BOTH;
+	default:
+		return IRQ_TYPE_NONE;
+	}
+}
+
+/**
+ * acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI
+ * @gsi: GSI IRQ number to map
+ * @irq: pointer where linux IRQ number is stored
+ *
+ * irq location updated with irq value [>0 on success, 0 on failure]
+ *
+ * Returns: linux IRQ number on success (>0)
+ *          -EINVAL on failure
+ */
+int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
+{
+	/*
+	 * Only default domain is supported at present, always find
+	 * the mapping corresponding to default domain by passing NULL
+	 * as irq_domain parameter
+	 */
+	*irq = irq_find_mapping(NULL, gsi);
+	/*
+	 * *irq == 0 means no mapping, that should
+	 * be reported as a failure
+	 */
+	return (*irq > 0) ? *irq : -EINVAL;
+}
+EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
+
+/**
+ * acpi_register_gsi() - Map a GSI to a linux IRQ number
+ * @dev: device for which IRQ has to be mapped
+ * @gsi: GSI IRQ number
+ * @trigger: trigger type of the GSI number to be mapped
+ * @polarity: polarity of the GSI to be mapped
+ *
+ * Returns: a valid linux IRQ number on success
+ *          -EINVAL on failure
+ */
+int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
+		      int polarity)
+{
+	unsigned int irq;
+	unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
+
+	/*
+	 * There is no way at present to look-up the IRQ domain on ACPI,
+	 * hence always create mapping referring to the default domain
+	 * by passing NULL as irq_domain parameter
+	 */
+	irq = irq_create_mapping(NULL, gsi);
+	if (!irq)
+		return -EINVAL;
+
+	/* Set irq type if specified and different than the current one */
+	if (irq_type != IRQ_TYPE_NONE &&
+		irq_type != irq_get_trigger_type(irq))
+		irq_set_irq_type(irq, irq_type);
+	return irq;
+}
+EXPORT_SYMBOL_GPL(acpi_register_gsi);
+
+/**
+ * acpi_unregister_gsi() - Free a GSI<->linux IRQ number mapping
+ * @gsi: GSI IRQ number
+ */
+void acpi_unregister_gsi(u32 gsi)
+{
+	int irq = irq_find_mapping(NULL, gsi);
+
+	irq_dispose_mapping(irq);
+}
+EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d15a36a..f1efb53 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1189,6 +1189,8 @@ gic_v2_acpi_init(struct acpi_table_header *table)
 	 */
 	gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
 	irq_set_default_host(gic_data[0].domain);
+
+	acpi_irq_model = ACPI_IRQ_MODEL_GIC;
 	return 0;
 }
 #endif
-- 
2.2.1

[PATCH 2/5] ARM64: kernel: psci: factor out probe function

[Lorenzo Pieralisi]

PSCI v0.2+ versions provide a specific PSCI call (PSCI_VERSION) to
detect the PSCI version at run-time. Current PSCI v0.2 init code
carries out the version probing in the PSCI 0.2 DT init function,
but the version probing does not depend on DT so it can be factored out
in order to make it available to other boot mechanisms (ie ACPI) to
reuse. The psci_probe() probing function can be easily extended
to add detection and initialization of PSCI functions defined in
PSCI versions >0.2.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/kernel/psci.c | 50 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index d3c52ce..2caac44 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -303,25 +303,19 @@ static void __init psci_0_2_set_functions(void)
 }
 
 /*
- * PSCI Function IDs for v0.2+ are well defined so use
- * standard values.
+ * Probe function for PSCI firmware versions >= 0.2
  */
-static int __init psci_0_2_init(struct device_node *np)
+static int __init psci_probe(void)
 {
-	int err, ver;
-
-	err = get_set_conduit_method(np);
-
-	if (err)
-		goto out_put_node;
-
-	ver = psci_get_version();
+	int ver = psci_get_version();
 
 	if (ver == PSCI_RET_NOT_SUPPORTED) {
-		/* PSCI v0.2 mandates implementation of PSCI_ID_VERSION. */
+		/*
+		 * PSCI versions >=0.2 mandates implementation of
+		 * PSCI_VERSION.
+		 */
 		pr_err("PSCI firmware does not comply with the v0.2 spec.\n");
-		err = -EOPNOTSUPP;
-		goto out_put_node;
+		return -EOPNOTSUPP;
 	} else {
 		pr_info("PSCIv%d.%d detected in firmware.\n",
 				PSCI_VERSION_MAJOR(ver),
@@ -329,14 +323,38 @@ static int __init psci_0_2_init(struct device_node *np)
 
 		if (PSCI_VERSION_MAJOR(ver) == 0 &&
 				PSCI_VERSION_MINOR(ver) < 2) {
-			err = -EINVAL;
 			pr_err("Conflicting PSCI version detected.\n");
-			goto out_put_node;
+			return -EINVAL;
 		}
 	}
 
 	psci_0_2_set_functions();
 
+	return 0;
+}
+
+/*
+ * PSCI init function for PSCI versions >=0.2
+ *
+ * Probe based on PSCI PSCI_VERSION function
+ */
+static int __init psci_0_2_init(struct device_node *np)
+{
+	int err;
+
+	err = get_set_conduit_method(np);
+
+	if (err)
+		goto out_put_node;
+	/*
+	 * Starting with v0.2, the PSCI specification introduced a call
+	 * (PSCI_VERSION) that allows probing the firmware version, so
+	 * that PSCI function IDs and version specific initialization
+	 * can be carried out according to the specific version reported
+	 * by firmware
+	 */
+	err = psci_probe();
+
 out_put_node:
 	of_node_put(np);
 	return err;
-- 
2.2.1

[PATCH 3/5] ARM64: kernel: psci: let ACPI probe PSCI version

[Lorenzo Pieralisi]

PSCI v0.2+ allows the kernel to probe the PSCI firmware version.

This patch replaces the default initialization of PSCI v0.2+
functions with code that allows probing PSCI firmware version
and initializes PSCI functions accordingly.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/kernel/psci.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index 2caac44..ea18cb5 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -439,9 +439,7 @@ int __init psci_acpi_init(void)
 	else
 		invoke_psci_fn = __invoke_psci_fn_smc;
 
-	psci_0_2_set_functions();
-
-	return 0;
+	return psci_probe();
 }
 
 #ifdef CONFIG_SMP
-- 
2.2.1

[PATCH 4/5] ARM64: kernel: acpi: refactor ACPI tables init and checks

[Lorenzo Pieralisi]

Current ACPI init code on ARM64 relies on acpi_table_parse() API to
check if the FADT is present and to carry out sanity checks on that.

The handler passed to the acpi_table_parse() function and used to
carry out the parsing on the requested table returns a value that is
ignored by the acpi_table_parse() function, so it is not possible
to propagate errors back to the acpi_table_parse() caller through
the handler.

This forces ARM64 ACPI init code to have disable_acpi() calls scattered
all over the place that makes code unwieldy and not easy to follow.

This patch refactors the ARM64 ACPI init code, by creating a
self-contained function (ie acpi_fadt_sanity_check()) that carries
out the required checks on FADT and returns an adequate return value
to the caller. This allows creating a common error path that disables
ACPI and makes code more readable and easy to parse and change were
further checks FADT to be added in the future.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
---
 arch/arm64/kernel/acpi.c | 104 +++++++++++++++++++++++++++++++----------------
 1 file changed, 70 insertions(+), 34 deletions(-)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index a70f714..542f045 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -218,43 +218,60 @@ void __init acpi_init_cpus(void)
 	pr_info("%d CPUs enabled, %d CPUs total\n", enabled_cpus, total_cpus);
 }
 
-static int __init acpi_parse_fadt(struct acpi_table_header *table)
+/*
+ * acpi_fadt_sanity_check() - Check FADT presence and carry out sanity
+ *			      checks on it
+ *
+ * Return 0 on success,  <0 on failure
+ */
+static int __init acpi_fadt_sanity_check(void)
 {
-	struct acpi_table_fadt *fadt = (struct acpi_table_fadt *)table;
+	struct acpi_table_header *table;
+	struct acpi_table_fadt *fadt;
+	acpi_status status;
+	acpi_size tbl_size;
+	int ret = 0;
+
+	/*
+	 * FADT is required on arm64; retrieve it to check its presence
+	 * and carry out revision and ACPI HW reduced compliancy tests
+	 */
+	status = acpi_get_table_with_size(ACPI_SIG_FADT, 0, &table, &tbl_size);
+	if (ACPI_FAILURE(status)) {
+		const char *msg = acpi_format_exception(status);
+
+		pr_err("Failed to get FADT table, %s\n", msg);
+		return -ENODEV;
+	}
+
+	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 5.1,
 	 * we only deal with ACPI 5.1 or newer revision to get GIC and SMP
-	 * boot protocol configuration data, or we will disable ACPI.
+	 * boot protocol configuration data.
 	 */
-	if (table->revision > 5 ||
-	    (table->revision == 5 && fadt->minor_revision >= 1)) {
-		if (!acpi_gbl_reduced_hardware) {
-			pr_err("Not hardware reduced ACPI mode, will not be supported\n");
-			goto disable_acpi;
-		}
-
-		/*
-		 * ACPI 5.1 only has two explicit methods to boot up SMP,
-		 * PSCI and Parking protocol, but the Parking protocol is
-		 * only specified for ARMv7 now, so make PSCI as the only
-		 * way for the SMP boot protocol before some updates for
-		 * the Parking protocol spec.
-		 */
-		if (acpi_psci_present())
-			return 0;
-
-		pr_warn("No PSCI support, will not bring up secondary CPUs\n");
-		return -EOPNOTSUPP;
+	if (table->revision < 5 ||
+	   (table->revision == 5 && fadt->minor_revision < 1)) {
+		pr_err("Unsupported FADT revision %d.%d, should be 5.1+\n",
+		       table->revision, fadt->minor_revision);
+		ret = -EINVAL;
+		goto out;
 	}
 
-	pr_warn("Unsupported FADT revision %d.%d, should be 5.1+, will disable ACPI\n",
-		table->revision, fadt->minor_revision);
+	if (!(fadt->flags & ACPI_FADT_HW_REDUCED)) {
+		pr_err("FADT not ACPI hardware reduced compliant\n");
+		ret = -EINVAL;
+	}
 
-disable_acpi:
-	disable_acpi();
-	return -EINVAL;
+out:
+	/*
+	 * acpi_get_table_with_size() creates FADT table mapping that
+	 * should be released after parsing and before resuming boot
+	 */
+	early_acpi_os_unmap_memory(table, tbl_size);
+	return ret;
 }
 
 /*
@@ -262,12 +279,18 @@ disable_acpi:
  *	1. find RSDP and get its address, and then find XSDT
  *	2. extract all tables and checksums them all
  *	3. check ACPI FADT revision
+ *	4. check ACPI FADT HW reduced flag
  *
  * We can parse ACPI boot-time tables such as MADT after
  * this function is called.
+ *
+ * ACPI is enabled on return if ACPI tables initialized and sanity checks
+ * passed, disabled otherwise
  */
 void __init acpi_boot_table_init(void)
 {
+	int err;
+
 	/*
 	 * Enable ACPI instead of device tree unless
 	 * - ACPI has been disabled explicitly (acpi=off), or
@@ -278,19 +301,32 @@ void __init acpi_boot_table_init(void)
 	    (!param_acpi_force && of_scan_flat_dt(dt_scan_depth1_nodes, NULL)))
 		return;
 
+	/*
+	 * ACPI is disabled at this point. Enable it in order to parse
+	 * the ACPI tables and carry out sanity checks
+	 */
 	enable_acpi();
 
 	/* Initialize the ACPI boot-time table parser. */
-	if (acpi_table_init()) {
-		disable_acpi();
-		return;
+	err = acpi_table_init();
+	if (err) {
+		pr_err("Failed to init ACPI tables\n");
+		goto out;
 	}
 
-	if (acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt)) {
-		/* disable ACPI if no FADT is found */
+	/*
+	 * Check FADT presence and carry out FADT sanity checks
+	 */
+	err = acpi_fadt_sanity_check();
+
+out:
+	/*
+	 * If ACPI tables are initialized and FADT sanity checks passed,
+	 * leave ACPI enabled and carry on booting; otherwise disable ACPI
+	 * on initialization error.
+	 */
+	if (err)
 		disable_acpi();
-		pr_err("Can't find FADT\n");
-	}
 }
 
 void __init acpi_gic_init(void)
-- 
2.2.1

[PATCH 5/5] ARM64: kernel: acpi: honour acpi=force command line parameter

[Lorenzo Pieralisi]

If acpi=force is passed on the command line, it forces ACPI to be
the only available boot method, hence it must be left enabled even
if the initialization and sanity checks on ACPI tables fails.

This patch refactors ACPI initialization to prevent disabling ACPI
if acpi=force is passed on the command line.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/acpi.h | 3 ---
 arch/arm64/kernel/acpi.c      | 6 ++++--
 arch/arm64/kernel/setup.c     | 2 +-
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index e1a8965..59c05d8 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -38,7 +38,6 @@ typedef u64 phys_cpuid_t;
 extern int acpi_disabled;
 extern int acpi_noirq;
 extern int acpi_pci_disabled;
-extern bool param_acpi_force;
 
 /* 1 to indicate PSCI 0.2+ is implemented */
 static inline bool acpi_psci_present(void)
@@ -92,8 +91,6 @@ void __init acpi_init_cpus(void);
 static inline bool acpi_psci_present(void) { return false; }
 static inline bool acpi_psci_use_hvc(void) { return false; }
 static inline void acpi_init_cpus(void) { }
-
-#define param_acpi_force false
 #endif /* CONFIG_ACPI */
 
 #endif /*_ASM_ACPI_H*/
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 542f045..789994a 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -43,7 +43,7 @@ static int enabled_cpus;
 static bool bootcpu_valid  __initdata;
 
 static bool param_acpi_off __initdata;
-bool param_acpi_force __initdata;
+static bool param_acpi_force __initdata;
 
 static int __init parse_acpi(char *arg)
 {
@@ -324,8 +324,10 @@ out:
 	 * If ACPI tables are initialized and FADT sanity checks passed,
 	 * leave ACPI enabled and carry on booting; otherwise disable ACPI
 	 * on initialization error.
+	 * If acpi=force was passed on the command line it forces ACPI
+	 * to be enabled even if its initialization failed.
 	 */
-	if (err)
+	if (err && !param_acpi_force)
 		disable_acpi();
 }
 
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index d60b1ad..b278311 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -390,7 +390,7 @@ void __init setup_arch(char **cmdline_p)
 
 	early_ioremap_reset();
 
-	if (acpi_disabled  && !param_acpi_force) {
+	if (acpi_disabled) {
 		unflatten_device_tree();
 		psci_dt_init();
 		cpu_read_bootcpu_ops();
-- 
2.2.1

Re: [PATCH 5/5] ARM64: kernel: acpi: honour acpi=force command line parameter

[Ard Biesheuvel]

On 24 March 2015 at 18:58, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> If acpi=force is passed on the command line, it forces ACPI to be
> the only available boot method, hence it must be left enabled even
> if the initialization and sanity checks on ACPI tables fails.
>
> This patch refactors ACPI initialization to prevent disabling ACPI
> if acpi=force is passed on the command line.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> ---

Yes, much better

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>  arch/arm64/include/asm/acpi.h | 3 ---
>  arch/arm64/kernel/acpi.c      | 6 ++++--
>  arch/arm64/kernel/setup.c     | 2 +-
>  3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index e1a8965..59c05d8 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -38,7 +38,6 @@ typedef u64 phys_cpuid_t;
>  extern int acpi_disabled;
>  extern int acpi_noirq;
>  extern int acpi_pci_disabled;
> -extern bool param_acpi_force;
>
>  /* 1 to indicate PSCI 0.2+ is implemented */
>  static inline bool acpi_psci_present(void)
> @@ -92,8 +91,6 @@ void __init acpi_init_cpus(void);
>  static inline bool acpi_psci_present(void) { return false; }
>  static inline bool acpi_psci_use_hvc(void) { return false; }
>  static inline void acpi_init_cpus(void) { }
> -
> -#define param_acpi_force false
>  #endif /* CONFIG_ACPI */
>
>  #endif /*_ASM_ACPI_H*/
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index 542f045..789994a 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -43,7 +43,7 @@ static int enabled_cpus;
>  static bool bootcpu_valid  __initdata;
>
>  static bool param_acpi_off __initdata;
> -bool param_acpi_force __initdata;
> +static bool param_acpi_force __initdata;
>
>  static int __init parse_acpi(char *arg)
>  {
> @@ -324,8 +324,10 @@ out:
>          * If ACPI tables are initialized and FADT sanity checks passed,
>          * leave ACPI enabled and carry on booting; otherwise disable ACPI
>          * on initialization error.
> +        * If acpi=force was passed on the command line it forces ACPI
> +        * to be enabled even if its initialization failed.
>          */
> -       if (err)
> +       if (err && !param_acpi_force)
>                 disable_acpi();
>  }
>
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index d60b1ad..b278311 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -390,7 +390,7 @@ void __init setup_arch(char **cmdline_p)
>
>         early_ioremap_reset();
>
> -       if (acpi_disabled  && !param_acpi_force) {
> +       if (acpi_disabled) {
>                 unflatten_device_tree();
>                 psci_dt_init();
>                 cpu_read_bootcpu_ops();
> --
> 2.2.1
>

Re: [PATCH 1/5] ACPI: move arm64 GSI IRQ model to generic GSI IRQ layer

[Marc Zyngier]

On 24/03/15 17:58, Lorenzo Pieralisi wrote:
> The code deployed to implement GSI linux IRQ numbers mapping on arm64 turns
> out to be generic enough so that it can be moved to ACPI core code along
> with its respective config option ACPI_GENERIC_GSI selectable on
> architectures that can reuse the same code.
> 
> Current ACPI IRQ mapping code is not integrated in the kernel IRQ domain
> infrastructure, in particular there is no way to look-up the
> IRQ domain associated with a particular interrupt controller, so this
> first version of GSI generic code carries out the GSI<->IRQ mapping relying
> on the IRQ default domain which is supposed to be always set on a
> specific architecture in case the domain structure passed to
> irq_create/find_mapping() functions is missing.
> 
> This patch moves the arm64 acpi functions that implement the gsi mappings:
> 
> acpi_gsi_to_irq()
> acpi_register_gsi()
> acpi_unregister_gsi()
> 
> to ACPI core code. Since the generic GSI<->domain mapping is based on IRQ
> domains, it can be extended as soon as a way to map an interrupt
> controller to an IRQ domain is implemented for ACPI in the IRQ domain
> layer.
> 
> x86 and ia64 code for GSI mappings cannot rely on the generic GSI
> layer at present for legacy reasons, so they do not select the
> ACPI_GENERIC_GSI config options and keep relying on their arch
> specific GSI mapping layer.

This looks like the right thing to do. I definitely like the sanity
checking that has been added here.

FWIW: Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.

> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Jiang Liu <jiang.liu@linux.intel.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/Kconfig        |   1 +
>  arch/arm64/kernel/acpi.c  |  73 --------------------------------
>  drivers/acpi/Kconfig      |   3 ++
>  drivers/acpi/Makefile     |   1 +
>  drivers/acpi/gsi.c        | 105 ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/irqchip/irq-gic.c |   2 +
>  6 files changed, 112 insertions(+), 73 deletions(-)
>  create mode 100644 drivers/acpi/gsi.c
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index e5aa081..0659db3 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1,5 +1,6 @@
>  config ARM64
>  	def_bool y
> +	select ACPI_GENERIC_GSI if ACPI
>  	select ACPI_REDUCED_HARDWARE_ONLY if ACPI
>  	select ARCH_BINFMT_ELF_RANDOMIZE_PIE
>  	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index fe9d8f0..a70f714 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -76,12 +76,6 @@ static int __init dt_scan_depth1_nodes(unsigned long node,
>  }
>  
>  /*
> - * Since we're on ARM, the default interrupt routing model
> - * clearly has to be GIC.
> - */
> -enum acpi_irq_model_id acpi_irq_model = ACPI_IRQ_MODEL_GIC;
> -
> -/*
>   * __acpi_map_table() will be called before page_init(), so early_ioremap()
>   * or early_memremap() should be called here to for ACPI table mapping.
>   */
> @@ -224,73 +218,6 @@ void __init acpi_init_cpus(void)
>  	pr_info("%d CPUs enabled, %d CPUs total\n", enabled_cpus, total_cpus);
>  }
>  
> -int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
> -{
> -	*irq = irq_find_mapping(NULL, gsi);
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
> -
> -/*
> - * success: return IRQ number (>0)
> - * failure: return =< 0
> - */
> -int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity)
> -{
> -	unsigned int irq;
> -	unsigned int irq_type;
> -
> -	/*
> -	 * ACPI have no bindings to indicate SPI or PPI, so we
> -	 * use different mappings from DT in ACPI.
> -	 *
> -	 * For FDT
> -	 * PPI interrupt: in the range [0, 15];
> -	 * SPI interrupt: in the range [0, 987];
> -	 *
> -	 * For ACPI, GSI should be unique so using
> -	 * the hwirq directly for the mapping:
> -	 * PPI interrupt: in the range [16, 31];
> -	 * SPI interrupt: in the range [32, 1019];
> -	 */
> -
> -	if (trigger == ACPI_EDGE_SENSITIVE &&
> -				polarity == ACPI_ACTIVE_LOW)
> -		irq_type = IRQ_TYPE_EDGE_FALLING;
> -	else if (trigger == ACPI_EDGE_SENSITIVE &&
> -				polarity == ACPI_ACTIVE_HIGH)
> -		irq_type = IRQ_TYPE_EDGE_RISING;
> -	else if (trigger == ACPI_LEVEL_SENSITIVE &&
> -				polarity == ACPI_ACTIVE_LOW)
> -		irq_type = IRQ_TYPE_LEVEL_LOW;
> -	else if (trigger == ACPI_LEVEL_SENSITIVE &&
> -				polarity == ACPI_ACTIVE_HIGH)
> -		irq_type = IRQ_TYPE_LEVEL_HIGH;
> -	else
> -		irq_type = IRQ_TYPE_NONE;
> -
> -	/*
> -	 * Since only one GIC is supported in ACPI 5.0, we can
> -	 * create mapping refer to the default domain
> -	 */
> -	irq = irq_create_mapping(NULL, gsi);
> -	if (!irq)
> -		return irq;
> -
> -	/* Set irq type if specified and different than the current one */
> -	if (irq_type != IRQ_TYPE_NONE &&
> -		irq_type != irq_get_trigger_type(irq))
> -		irq_set_irq_type(irq, irq_type);
> -	return irq;
> -}
> -EXPORT_SYMBOL_GPL(acpi_register_gsi);
> -
> -void acpi_unregister_gsi(u32 gsi)
> -{
> -}
> -EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
> -
>  static int __init acpi_parse_fadt(struct acpi_table_header *table)
>  {
>  	struct acpi_table_fadt *fadt = (struct acpi_table_fadt *)table;
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index a8f531e2..ab2cbb5 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -48,6 +48,9 @@ config ACPI_LEGACY_TABLES_LOOKUP
>  config ARCH_MIGHT_HAVE_ACPI_PDC
>  	bool
>  
> +config ACPI_GENERIC_GSI
> +	bool
> +
>  config ACPI_SYSTEM_POWER_STATES_SUPPORT
>  	bool
>  
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index db153c6..8a063e2 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -56,6 +56,7 @@ ifdef CONFIG_ACPI_VIDEO
>  acpi-y				+= video_detect.o
>  endif
>  acpi-y				+= acpi_lpat.o
> +acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
>  
>  # These are (potentially) separate modules
>  
> diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
> new file mode 100644
> index 0000000..38208f2
> --- /dev/null
> +++ b/drivers/acpi/gsi.c
> @@ -0,0 +1,105 @@
> +/*
> + * ACPI GSI IRQ layer
> + *
> + * Copyright (C) 2015 ARM Ltd.
> + * Author: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/acpi.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +
> +enum acpi_irq_model_id acpi_irq_model;
> +
> +static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
> +{
> +	switch (polarity) {
> +	case ACPI_ACTIVE_LOW:
> +		return trigger == ACPI_EDGE_SENSITIVE ?
> +		       IRQ_TYPE_EDGE_FALLING :
> +		       IRQ_TYPE_LEVEL_LOW;
> +	case ACPI_ACTIVE_HIGH:
> +		return trigger == ACPI_EDGE_SENSITIVE ?
> +		       IRQ_TYPE_EDGE_RISING :
> +		       IRQ_TYPE_LEVEL_HIGH;
> +	case ACPI_ACTIVE_BOTH:
> +		if (trigger == ACPI_EDGE_SENSITIVE)
> +			return IRQ_TYPE_EDGE_BOTH;
> +	default:
> +		return IRQ_TYPE_NONE;
> +	}
> +}
> +
> +/**
> + * acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI
> + * @gsi: GSI IRQ number to map
> + * @irq: pointer where linux IRQ number is stored
> + *
> + * irq location updated with irq value [>0 on success, 0 on failure]
> + *
> + * Returns: linux IRQ number on success (>0)
> + *          -EINVAL on failure
> + */
> +int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
> +{
> +	/*
> +	 * Only default domain is supported at present, always find
> +	 * the mapping corresponding to default domain by passing NULL
> +	 * as irq_domain parameter
> +	 */
> +	*irq = irq_find_mapping(NULL, gsi);
> +	/*
> +	 * *irq == 0 means no mapping, that should
> +	 * be reported as a failure
> +	 */
> +	return (*irq > 0) ? *irq : -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
> +
> +/**
> + * acpi_register_gsi() - Map a GSI to a linux IRQ number
> + * @dev: device for which IRQ has to be mapped
> + * @gsi: GSI IRQ number
> + * @trigger: trigger type of the GSI number to be mapped
> + * @polarity: polarity of the GSI to be mapped
> + *
> + * Returns: a valid linux IRQ number on success
> + *          -EINVAL on failure
> + */
> +int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
> +		      int polarity)
> +{
> +	unsigned int irq;
> +	unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
> +
> +	/*
> +	 * There is no way at present to look-up the IRQ domain on ACPI,
> +	 * hence always create mapping referring to the default domain
> +	 * by passing NULL as irq_domain parameter
> +	 */
> +	irq = irq_create_mapping(NULL, gsi);
> +	if (!irq)
> +		return -EINVAL;
> +
> +	/* Set irq type if specified and different than the current one */
> +	if (irq_type != IRQ_TYPE_NONE &&
> +		irq_type != irq_get_trigger_type(irq))
> +		irq_set_irq_type(irq, irq_type);
> +	return irq;
> +}
> +EXPORT_SYMBOL_GPL(acpi_register_gsi);
> +
> +/**
> + * acpi_unregister_gsi() - Free a GSI<->linux IRQ number mapping
> + * @gsi: GSI IRQ number
> + */
> +void acpi_unregister_gsi(u32 gsi)
> +{
> +	int irq = irq_find_mapping(NULL, gsi);
> +
> +	irq_dispose_mapping(irq);
> +}
> +EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index d15a36a..f1efb53 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1189,6 +1189,8 @@ gic_v2_acpi_init(struct acpi_table_header *table)
>  	 */
>  	gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
>  	irq_set_default_host(gic_data[0].domain);
> +
> +	acpi_irq_model = ACPI_IRQ_MODEL_GIC;
>  	return 0;
>  }
>  #endif
> 


-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 1/5] ACPI: move arm64 GSI IRQ model to generic GSI IRQ layer

[Hanjun Guo]

On 2015/3/25 1:58, Lorenzo Pieralisi wrote:
> The code deployed to implement GSI linux IRQ numbers mapping on arm64 turns
> out to be generic enough so that it can be moved to ACPI core code along
> with its respective config option ACPI_GENERIC_GSI selectable on
> architectures that can reuse the same code.
>
> Current ACPI IRQ mapping code is not integrated in the kernel IRQ domain
> infrastructure, in particular there is no way to look-up the
> IRQ domain associated with a particular interrupt controller, so this
> first version of GSI generic code carries out the GSI<->IRQ mapping relying
> on the IRQ default domain which is supposed to be always set on a
> specific architecture in case the domain structure passed to
> irq_create/find_mapping() functions is missing.
>
> This patch moves the arm64 acpi functions that implement the gsi mappings:
>
> acpi_gsi_to_irq()
> acpi_register_gsi()
> acpi_unregister_gsi()
>
> to ACPI core code. Since the generic GSI<->domain mapping is based on IRQ
> domains, it can be extended as soon as a way to map an interrupt
> controller to an IRQ domain is implemented for ACPI in the IRQ domain
> layer.
>
> x86 and ia64 code for GSI mappings cannot rely on the generic GSI
> layer at present for legacy reasons, so they do not select the
> ACPI_GENERIC_GSI config options and keep relying on their arch
> specific GSI mapping layer.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Jiang Liu <jiang.liu@linux.intel.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---

Acked-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

Re: [PATCH 2/5] ARM64: kernel: psci: factor out probe function

[Hanjun Guo]

On 2015/3/25 1:58, Lorenzo Pieralisi wrote:
> PSCI v0.2+ versions provide a specific PSCI call (PSCI_VERSION) to
> detect the PSCI version at run-time. Current PSCI v0.2 init code
> carries out the version probing in the PSCI 0.2 DT init function,
> but the version probing does not depend on DT so it can be factored out
> in order to make it available to other boot mechanisms (ie ACPI) to
> reuse. The psci_probe() probing function can be easily extended
> to add detection and initialization of PSCI functions defined in
> PSCI versions >0.2.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---

This cleanup looks pretty fine to me,

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

Re: [PATCH 3/5] ARM64: kernel: psci: let ACPI probe PSCI version

[Hanjun Guo]

On 2015/3/25 1:58, Lorenzo Pieralisi wrote:
> PSCI v0.2+ allows the kernel to probe the PSCI firmware version.
>
> This patch replaces the default initialization of PSCI v0.2+
> functions with code that allows probing PSCI firmware version
> and initializes PSCI functions accordingly.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

>  arch/arm64/kernel/psci.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
> index 2caac44..ea18cb5 100644
> --- a/arch/arm64/kernel/psci.c
> +++ b/arch/arm64/kernel/psci.c
> @@ -439,9 +439,7 @@ int __init psci_acpi_init(void)
>  	else
>  		invoke_psci_fn = __invoke_psci_fn_smc;
>  
> -	psci_0_2_set_functions();
> -
> -	return 0;
> +	return psci_probe();
>  }
>  
>  #ifdef CONFIG_SMP

Re: [PATCH 4/5] ARM64: kernel: acpi: refactor ACPI tables init and checks

[Hanjun Guo]

On 2015/3/25 1:58, Lorenzo Pieralisi wrote:
> Current ACPI init code on ARM64 relies on acpi_table_parse() API to
> check if the FADT is present and to carry out sanity checks on that.
>
> The handler passed to the acpi_table_parse() function and used to
> carry out the parsing on the requested table returns a value that is
> ignored by the acpi_table_parse() function, so it is not possible
> to propagate errors back to the acpi_table_parse() caller through
> the handler.
>
> This forces ARM64 ACPI init code to have disable_acpi() calls scattered
> all over the place that makes code unwieldy and not easy to follow.
>
> This patch refactors the ARM64 ACPI init code, by creating a
> self-contained function (ie acpi_fadt_sanity_check()) that carries
> out the required checks on FADT and returns an adequate return value
> to the caller. This allows creating a common error path that disables
> ACPI and makes code more readable and easy to parse and change were
> further checks FADT to be added in the future.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> ---

I already reviewed this patch set, and it is fine to me

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

>  arch/arm64/kernel/acpi.c | 104 +++++++++++++++++++++++++++++++----------------
>  1 file changed, 70 insertions(+), 34 deletions(-)
>
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index a70f714..542f045 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -218,43 +218,60 @@ void __init acpi_init_cpus(void)
>  	pr_info("%d CPUs enabled, %d CPUs total\n", enabled_cpus, total_cpus);
>  }
>  
> -static int __init acpi_parse_fadt(struct acpi_table_header *table)
> +/*
> + * acpi_fadt_sanity_check() - Check FADT presence and carry out sanity
> + *			      checks on it
> + *
> + * Return 0 on success,  <0 on failure
> + */
> +static int __init acpi_fadt_sanity_check(void)
>  {
> -	struct acpi_table_fadt *fadt = (struct acpi_table_fadt *)table;
> +	struct acpi_table_header *table;
> +	struct acpi_table_fadt *fadt;
> +	acpi_status status;
> +	acpi_size tbl_size;
> +	int ret = 0;
> +
> +	/*
> +	 * FADT is required on arm64; retrieve it to check its presence
> +	 * and carry out revision and ACPI HW reduced compliancy tests
> +	 */
> +	status = acpi_get_table_with_size(ACPI_SIG_FADT, 0, &table, &tbl_size);
> +	if (ACPI_FAILURE(status)) {
> +		const char *msg = acpi_format_exception(status);
> +
> +		pr_err("Failed to get FADT table, %s\n", msg);
> +		return -ENODEV;
> +	}
> +
> +	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 5.1,
>  	 * we only deal with ACPI 5.1 or newer revision to get GIC and SMP
> -	 * boot protocol configuration data, or we will disable ACPI.
> +	 * boot protocol configuration data.
>  	 */
> -	if (table->revision > 5 ||
> -	    (table->revision == 5 && fadt->minor_revision >= 1)) {
> -		if (!acpi_gbl_reduced_hardware) {
> -			pr_err("Not hardware reduced ACPI mode, will not be supported\n");
> -			goto disable_acpi;
> -		}
> -
> -		/*
> -		 * ACPI 5.1 only has two explicit methods to boot up SMP,
> -		 * PSCI and Parking protocol, but the Parking protocol is
> -		 * only specified for ARMv7 now, so make PSCI as the only
> -		 * way for the SMP boot protocol before some updates for
> -		 * the Parking protocol spec.
> -		 */
> -		if (acpi_psci_present())
> -			return 0;
> -
> -		pr_warn("No PSCI support, will not bring up secondary CPUs\n");
> -		return -EOPNOTSUPP;
> +	if (table->revision < 5 ||
> +	   (table->revision == 5 && fadt->minor_revision < 1)) {
> +		pr_err("Unsupported FADT revision %d.%d, should be 5.1+\n",
> +		       table->revision, fadt->minor_revision);
> +		ret = -EINVAL;
> +		goto out;
>  	}
>  
> -	pr_warn("Unsupported FADT revision %d.%d, should be 5.1+, will disable ACPI\n",
> -		table->revision, fadt->minor_revision);
> +	if (!(fadt->flags & ACPI_FADT_HW_REDUCED)) {
> +		pr_err("FADT not ACPI hardware reduced compliant\n");
> +		ret = -EINVAL;
> +	}
>  
> -disable_acpi:
> -	disable_acpi();
> -	return -EINVAL;
> +out:
> +	/*
> +	 * acpi_get_table_with_size() creates FADT table mapping that
> +	 * should be released after parsing and before resuming boot
> +	 */
> +	early_acpi_os_unmap_memory(table, tbl_size);
> +	return ret;
>  }
>  
>  /*
> @@ -262,12 +279,18 @@ disable_acpi:
>   *	1. find RSDP and get its address, and then find XSDT
>   *	2. extract all tables and checksums them all
>   *	3. check ACPI FADT revision
> + *	4. check ACPI FADT HW reduced flag
>   *
>   * We can parse ACPI boot-time tables such as MADT after
>   * this function is called.
> + *
> + * ACPI is enabled on return if ACPI tables initialized and sanity checks
> + * passed, disabled otherwise
>   */
>  void __init acpi_boot_table_init(void)
>  {
> +	int err;
> +
>  	/*
>  	 * Enable ACPI instead of device tree unless
>  	 * - ACPI has been disabled explicitly (acpi=off), or
> @@ -278,19 +301,32 @@ void __init acpi_boot_table_init(void)
>  	    (!param_acpi_force && of_scan_flat_dt(dt_scan_depth1_nodes, NULL)))
>  		return;
>  
> +	/*
> +	 * ACPI is disabled at this point. Enable it in order to parse
> +	 * the ACPI tables and carry out sanity checks
> +	 */
>  	enable_acpi();
>  
>  	/* Initialize the ACPI boot-time table parser. */
> -	if (acpi_table_init()) {
> -		disable_acpi();
> -		return;
> +	err = acpi_table_init();
> +	if (err) {
> +		pr_err("Failed to init ACPI tables\n");
> +		goto out;
>  	}
>  
> -	if (acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt)) {
> -		/* disable ACPI if no FADT is found */
> +	/*
> +	 * Check FADT presence and carry out FADT sanity checks
> +	 */
> +	err = acpi_fadt_sanity_check();
> +
> +out:
> +	/*
> +	 * If ACPI tables are initialized and FADT sanity checks passed,
> +	 * leave ACPI enabled and carry on booting; otherwise disable ACPI
> +	 * on initialization error.
> +	 */
> +	if (err)
>  		disable_acpi();
> -		pr_err("Can't find FADT\n");
> -	}
>  }
>  
>  void __init acpi_gic_init(void)

Re: [PATCH 5/5] ARM64: kernel: acpi: honour acpi=force command line parameter

[Hanjun Guo]

On 2015/3/25 1:58, Lorenzo Pieralisi wrote:
> If acpi=force is passed on the command line, it forces ACPI to be
> the only available boot method, hence it must be left enabled even
> if the initialization and sanity checks on ACPI tables fails.
>
> This patch refactors ACPI initialization to prevent disabling ACPI
> if acpi=force is passed on the command line.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> ---

Although it will introduce some useless table parsing for some
bad ACPI tables, it still obey the policy for acpi=force, and it makes
param_acpi_force much self-contained, so

Acked-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun


>  arch/arm64/include/asm/acpi.h | 3 ---
>  arch/arm64/kernel/acpi.c      | 6 ++++--
>  arch/arm64/kernel/setup.c     | 2 +-
>  3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index e1a8965..59c05d8 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -38,7 +38,6 @@ typedef u64 phys_cpuid_t;
>  extern int acpi_disabled;
>  extern int acpi_noirq;
>  extern int acpi_pci_disabled;
> -extern bool param_acpi_force;
>  
>  /* 1 to indicate PSCI 0.2+ is implemented */
>  static inline bool acpi_psci_present(void)
> @@ -92,8 +91,6 @@ void __init acpi_init_cpus(void);
>  static inline bool acpi_psci_present(void) { return false; }
>  static inline bool acpi_psci_use_hvc(void) { return false; }
>  static inline void acpi_init_cpus(void) { }
> -
> -#define param_acpi_force false
>  #endif /* CONFIG_ACPI */
>  
>  #endif /*_ASM_ACPI_H*/
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index 542f045..789994a 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -43,7 +43,7 @@ static int enabled_cpus;
>  static bool bootcpu_valid  __initdata;
>  
>  static bool param_acpi_off __initdata;
> -bool param_acpi_force __initdata;
> +static bool param_acpi_force __initdata;
>  
>  static int __init parse_acpi(char *arg)
>  {
> @@ -324,8 +324,10 @@ out:
>  	 * If ACPI tables are initialized and FADT sanity checks passed,
>  	 * leave ACPI enabled and carry on booting; otherwise disable ACPI
>  	 * on initialization error.
> +	 * If acpi=force was passed on the command line it forces ACPI
> +	 * to be enabled even if its initialization failed.
>  	 */
> -	if (err)
> +	if (err && !param_acpi_force)
>  		disable_acpi();
>  }
>  
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index d60b1ad..b278311 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -390,7 +390,7 @@ void __init setup_arch(char **cmdline_p)
>  
>  	early_ioremap_reset();
>  
> -	if (acpi_disabled  && !param_acpi_force) {
> +	if (acpi_disabled) {
>  		unflatten_device_tree();
>  		psci_dt_init();
>  		cpu_read_bootcpu_ops();

Re: [PATCH 4/5] ARM64: kernel: acpi: refactor ACPI tables init and checks

[Will Deacon]

On Tue, Mar 24, 2015 at 05:58:54PM +0000, Lorenzo Pieralisi wrote:
> Current ACPI init code on ARM64 relies on acpi_table_parse() API to
> check if the FADT is present and to carry out sanity checks on that.
> 
> The handler passed to the acpi_table_parse() function and used to
> carry out the parsing on the requested table returns a value that is
> ignored by the acpi_table_parse() function, so it is not possible
> to propagate errors back to the acpi_table_parse() caller through
> the handler.
> 
> This forces ARM64 ACPI init code to have disable_acpi() calls scattered
> all over the place that makes code unwieldy and not easy to follow.
> 
> This patch refactors the ARM64 ACPI init code, by creating a
> self-contained function (ie acpi_fadt_sanity_check()) that carries
> out the required checks on FADT and returns an adequate return value
> to the caller. This allows creating a common error path that disables
> ACPI and makes code more readable and easy to parse and change were
> further checks FADT to be added in the future.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> ---
>  arch/arm64/kernel/acpi.c | 104 +++++++++++++++++++++++++++++++----------------
>  1 file changed, 70 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index a70f714..542f045 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -218,43 +218,60 @@ void __init acpi_init_cpus(void)
>  	pr_info("%d CPUs enabled, %d CPUs total\n", enabled_cpus, total_cpus);
>  }
>  
> -static int __init acpi_parse_fadt(struct acpi_table_header *table)
> +/*
> + * acpi_fadt_sanity_check() - Check FADT presence and carry out sanity
> + *			      checks on it
> + *
> + * Return 0 on success,  <0 on failure
> + */
> +static int __init acpi_fadt_sanity_check(void)
>  {
> -	struct acpi_table_fadt *fadt = (struct acpi_table_fadt *)table;
> +	struct acpi_table_header *table;
> +	struct acpi_table_fadt *fadt;
> +	acpi_status status;
> +	acpi_size tbl_size;
> +	int ret = 0;
> +
> +	/*
> +	 * FADT is required on arm64; retrieve it to check its presence
> +	 * and carry out revision and ACPI HW reduced compliancy tests
> +	 */
> +	status = acpi_get_table_with_size(ACPI_SIG_FADT, 0, &table, &tbl_size);
> +	if (ACPI_FAILURE(status)) {
> +		const char *msg = acpi_format_exception(status);
> +
> +		pr_err("Failed to get FADT table, %s\n", msg);
> +		return -ENODEV;
> +	}
> +
> +	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 5.1,
>  	 * we only deal with ACPI 5.1 or newer revision to get GIC and SMP
> -	 * boot protocol configuration data, or we will disable ACPI.
> +	 * boot protocol configuration data.
>  	 */
> -	if (table->revision > 5 ||
> -	    (table->revision == 5 && fadt->minor_revision >= 1)) {
> -		if (!acpi_gbl_reduced_hardware) {
> -			pr_err("Not hardware reduced ACPI mode, will not be supported\n");
> -			goto disable_acpi;
> -		}
> -
> -		/*
> -		 * ACPI 5.1 only has two explicit methods to boot up SMP,
> -		 * PSCI and Parking protocol, but the Parking protocol is
> -		 * only specified for ARMv7 now, so make PSCI as the only
> -		 * way for the SMP boot protocol before some updates for
> -		 * the Parking protocol spec.
> -		 */
> -		if (acpi_psci_present())
> -			return 0;
> -
> -		pr_warn("No PSCI support, will not bring up secondary CPUs\n");
> -		return -EOPNOTSUPP;
> +	if (table->revision < 5 ||
> +	   (table->revision == 5 && fadt->minor_revision < 1)) {
> +		pr_err("Unsupported FADT revision %d.%d, should be 5.1+\n",
> +		       table->revision, fadt->minor_revision);
> +		ret = -EINVAL;
> +		goto out;
>  	}
>  
> -	pr_warn("Unsupported FADT revision %d.%d, should be 5.1+, will disable ACPI\n",
> -		table->revision, fadt->minor_revision);
> +	if (!(fadt->flags & ACPI_FADT_HW_REDUCED)) {
> +		pr_err("FADT not ACPI hardware reduced compliant\n");
> +		ret = -EINVAL;
> +	}
>  
> -disable_acpi:
> -	disable_acpi();
> -	return -EINVAL;
> +out:
> +	/*
> +	 * acpi_get_table_with_size() creates FADT table mapping that
> +	 * should be released after parsing and before resuming boot
> +	 */
> +	early_acpi_os_unmap_memory(table, tbl_size);
> +	return ret;
>  }
>  
>  /*
> @@ -262,12 +279,18 @@ disable_acpi:
>   *	1. find RSDP and get its address, and then find XSDT
>   *	2. extract all tables and checksums them all
>   *	3. check ACPI FADT revision
> + *	4. check ACPI FADT HW reduced flag
>   *
>   * We can parse ACPI boot-time tables such as MADT after
>   * this function is called.
> + *
> + * ACPI is enabled on return if ACPI tables initialized and sanity checks
> + * passed, disabled otherwise
>   */
>  void __init acpi_boot_table_init(void)
>  {
> +	int err;
> +
>  	/*
>  	 * Enable ACPI instead of device tree unless
>  	 * - ACPI has been disabled explicitly (acpi=off), or
> @@ -278,19 +301,32 @@ void __init acpi_boot_table_init(void)
>  	    (!param_acpi_force && of_scan_flat_dt(dt_scan_depth1_nodes, NULL)))
>  		return;
>  
> +	/*
> +	 * ACPI is disabled at this point. Enable it in order to parse
> +	 * the ACPI tables and carry out sanity checks
> +	 */
>  	enable_acpi();
>  
>  	/* Initialize the ACPI boot-time table parser. */
> -	if (acpi_table_init()) {
> -		disable_acpi();
> -		return;
> +	err = acpi_table_init();
> +	if (err) {
> +		pr_err("Failed to init ACPI tables\n");
> +		goto out;
>  	}
>  
> -	if (acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt)) {
> -		/* disable ACPI if no FADT is found */
> +	/*
> +	 * Check FADT presence and carry out FADT sanity checks
> +	 */
> +	err = acpi_fadt_sanity_check();
> +
> +out:
> +	/*
> +	 * If ACPI tables are initialized and FADT sanity checks passed,
> +	 * leave ACPI enabled and carry on booting; otherwise disable ACPI
> +	 * on initialization error.
> +	 */
> +	if (err)
>  		disable_acpi();
> -		pr_err("Can't find FADT\n");
> -	}

Could you rewrite most of this as:

  enable_acpi();
  if (acpi_table_init() || acpi_fadt_sanity_check()) {
	pr_err("Failed to init ACPI tables\n");
	disable_acpi();
  }

I think it reads a bit better without the out: label.

Will

Re: [PATCH 4/5] ARM64: kernel: acpi: refactor ACPI tables init and checks

[Lorenzo Pieralisi]

On Wed, Mar 25, 2015 at 02:26:23PM +0000, Will Deacon wrote:
> On Tue, Mar 24, 2015 at 05:58:54PM +0000, Lorenzo Pieralisi wrote:

[...]

> >  void __init acpi_boot_table_init(void)
> >  {
> > +	int err;
> > +
> >  	/*
> >  	 * Enable ACPI instead of device tree unless
> >  	 * - ACPI has been disabled explicitly (acpi=off), or
> > @@ -278,19 +301,32 @@ void __init acpi_boot_table_init(void)
> >  	    (!param_acpi_force && of_scan_flat_dt(dt_scan_depth1_nodes, NULL)))
> >  		return;
> >  
> > +	/*
> > +	 * ACPI is disabled at this point. Enable it in order to parse
> > +	 * the ACPI tables and carry out sanity checks
> > +	 */
> >  	enable_acpi();
> >  
> >  	/* Initialize the ACPI boot-time table parser. */
> > -	if (acpi_table_init()) {
> > -		disable_acpi();
> > -		return;
> > +	err = acpi_table_init();
> > +	if (err) {
> > +		pr_err("Failed to init ACPI tables\n");
> > +		goto out;
> >  	}
> >  
> > -	if (acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt)) {
> > -		/* disable ACPI if no FADT is found */
> > +	/*
> > +	 * Check FADT presence and carry out FADT sanity checks
> > +	 */
> > +	err = acpi_fadt_sanity_check();
> > +
> > +out:
> > +	/*
> > +	 * If ACPI tables are initialized and FADT sanity checks passed,
> > +	 * leave ACPI enabled and carry on booting; otherwise disable ACPI
> > +	 * on initialization error.
> > +	 */
> > +	if (err)
> >  		disable_acpi();
> > -		pr_err("Can't find FADT\n");
> > -	}
> 
> Could you rewrite most of this as:
> 
>   enable_acpi();
>   if (acpi_table_init() || acpi_fadt_sanity_check()) {
> 	pr_err("Failed to init ACPI tables\n");
> 	disable_acpi();
>   }
> 
> I think it reads a bit better without the out: label.

Yes it makes sense acpi_fadt_sanity_check() already spits some errors
but there is not really a point in jumping around.

Here is the updated patch (please note this requires updating patch 5
too, I will send it inline shortly), thanks.

-- >8 --
Subject: [PATCH] ARM64: kernel: acpi: refactor ACPI tables init and checks

Current ACPI init code on ARM64 relies on acpi_table_parse() API to
check if the FADT is present and to carry out sanity checks on that.

The handler passed to the acpi_table_parse() function and used to
carry out the parsing on the requested table returns a value that is
ignored by the acpi_table_parse() function, so it is not possible
to propagate errors back to the acpi_table_parse() caller through
the handler.

This forces ARM64 ACPI init code to have disable_acpi() calls scattered
all over the place that makes code unwieldy and not easy to follow.

This patch refactors the ARM64 ACPI init code, by creating a
self-contained function (ie acpi_fadt_sanity_check()) that carries
out the required checks on FADT and returns an adequate return value
to the caller. This allows creating a common error path that disables
ACPI and makes code more readable and easy to parse and change were
further checks FADT to be added in the future.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
---
 arch/arm64/kernel/acpi.c | 95 ++++++++++++++++++++++++++++++------------------
 1 file changed, 59 insertions(+), 36 deletions(-)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index a70f714..172b7c9 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -218,43 +218,60 @@ void __init acpi_init_cpus(void)
 	pr_info("%d CPUs enabled, %d CPUs total\n", enabled_cpus, total_cpus);
 }
 
-static int __init acpi_parse_fadt(struct acpi_table_header *table)
+/*
+ * acpi_fadt_sanity_check() - Check FADT presence and carry out sanity
+ *			      checks on it
+ *
+ * Return 0 on success,  <0 on failure
+ */
+static int __init acpi_fadt_sanity_check(void)
 {
-	struct acpi_table_fadt *fadt = (struct acpi_table_fadt *)table;
+	struct acpi_table_header *table;
+	struct acpi_table_fadt *fadt;
+	acpi_status status;
+	acpi_size tbl_size;
+	int ret = 0;
+
+	/*
+	 * FADT is required on arm64; retrieve it to check its presence
+	 * and carry out revision and ACPI HW reduced compliancy tests
+	 */
+	status = acpi_get_table_with_size(ACPI_SIG_FADT, 0, &table, &tbl_size);
+	if (ACPI_FAILURE(status)) {
+		const char *msg = acpi_format_exception(status);
+
+		pr_err("Failed to get FADT table, %s\n", msg);
+		return -ENODEV;
+	}
+
+	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 5.1,
 	 * we only deal with ACPI 5.1 or newer revision to get GIC and SMP
-	 * boot protocol configuration data, or we will disable ACPI.
+	 * boot protocol configuration data.
 	 */
-	if (table->revision > 5 ||
-	    (table->revision == 5 && fadt->minor_revision >= 1)) {
-		if (!acpi_gbl_reduced_hardware) {
-			pr_err("Not hardware reduced ACPI mode, will not be supported\n");
-			goto disable_acpi;
-		}
-
-		/*
-		 * ACPI 5.1 only has two explicit methods to boot up SMP,
-		 * PSCI and Parking protocol, but the Parking protocol is
-		 * only specified for ARMv7 now, so make PSCI as the only
-		 * way for the SMP boot protocol before some updates for
-		 * the Parking protocol spec.
-		 */
-		if (acpi_psci_present())
-			return 0;
-
-		pr_warn("No PSCI support, will not bring up secondary CPUs\n");
-		return -EOPNOTSUPP;
+	if (table->revision < 5 ||
+	   (table->revision == 5 && fadt->minor_revision < 1)) {
+		pr_err("Unsupported FADT revision %d.%d, should be 5.1+\n",
+		       table->revision, fadt->minor_revision);
+		ret = -EINVAL;
+		goto out;
 	}
 
-	pr_warn("Unsupported FADT revision %d.%d, should be 5.1+, will disable ACPI\n",
-		table->revision, fadt->minor_revision);
+	if (!(fadt->flags & ACPI_FADT_HW_REDUCED)) {
+		pr_err("FADT not ACPI hardware reduced compliant\n");
+		ret = -EINVAL;
+	}
 
-disable_acpi:
-	disable_acpi();
-	return -EINVAL;
+out:
+	/*
+	 * acpi_get_table_with_size() creates FADT table mapping that
+	 * should be released after parsing and before resuming boot
+	 */
+	early_acpi_os_unmap_memory(table, tbl_size);
+	return ret;
 }
 
 /*
@@ -262,9 +279,13 @@ disable_acpi:
  *	1. find RSDP and get its address, and then find XSDT
  *	2. extract all tables and checksums them all
  *	3. check ACPI FADT revision
+ *	4. check ACPI FADT HW reduced flag
  *
  * We can parse ACPI boot-time tables such as MADT after
  * this function is called.
+ *
+ * ACPI is enabled on return if ACPI tables initialized and sanity checks
+ * passed, disabled otherwise
  */
 void __init acpi_boot_table_init(void)
 {
@@ -278,18 +299,20 @@ void __init acpi_boot_table_init(void)
 	    (!param_acpi_force && of_scan_flat_dt(dt_scan_depth1_nodes, NULL)))
 		return;
 
+	/*
+	 * ACPI is disabled at this point. Enable it in order to parse
+	 * the ACPI tables and carry out sanity checks
+	 */
 	enable_acpi();
 
-	/* Initialize the ACPI boot-time table parser. */
-	if (acpi_table_init()) {
-		disable_acpi();
-		return;
-	}
-
-	if (acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt)) {
-		/* disable ACPI if no FADT is found */
+	/*
+	 * If ACPI tables are initialized and FADT sanity checks passed,
+	 * leave ACPI enabled and carry on booting; otherwise disable ACPI
+	 * on initialization error.
+	 */
+	if (acpi_table_init() || acpi_fadt_sanity_check()) {
+		pr_err("Failed to init ACPI tables\n");
 		disable_acpi();
-		pr_err("Can't find FADT\n");
 	}
 }
 
-- 
2.2.1

Re: [PATCH 5/5] ARM64: kernel: acpi: honour acpi=force command line parameter

[Lorenzo Pieralisi]

Hi Will,

On Wed, Mar 25, 2015 at 01:56:47PM +0000, Hanjun Guo wrote:
> On 2015/3/25 1:58, Lorenzo Pieralisi wrote:
> > If acpi=force is passed on the command line, it forces ACPI to be
> > the only available boot method, hence it must be left enabled even
> > if the initialization and sanity checks on ACPI tables fails.
> >
> > This patch refactors ACPI initialization to prevent disabling ACPI
> > if acpi=force is passed on the command line.
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > ---

As mentioned, here is the update version for this patch:

-- >8 --
Subject: [PATCH] ARM64: kernel: acpi: honour acpi=force command line parameter

If acpi=force is passed on the command line, it forces ACPI to be
the only available boot method, hence it must be left enabled even
if the initialization and sanity checks on ACPI tables fails.

This patch refactors ACPI initialization to prevent disabling ACPI
if acpi=force is passed on the command line.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/acpi.h |  3 ---
 arch/arm64/kernel/acpi.c      | 16 ++++++++++++----
 arch/arm64/kernel/setup.c     |  2 +-
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index e1a8965..59c05d8 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -38,7 +38,6 @@ typedef u64 phys_cpuid_t;
 extern int acpi_disabled;
 extern int acpi_noirq;
 extern int acpi_pci_disabled;
-extern bool param_acpi_force;
 
 /* 1 to indicate PSCI 0.2+ is implemented */
 static inline bool acpi_psci_present(void)
@@ -92,8 +91,6 @@ void __init acpi_init_cpus(void);
 static inline bool acpi_psci_present(void) { return false; }
 static inline bool acpi_psci_use_hvc(void) { return false; }
 static inline void acpi_init_cpus(void) { }
-
-#define param_acpi_force false
 #endif /* CONFIG_ACPI */
 
 #endif /*_ASM_ACPI_H*/
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 172b7c9..cd60329 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -43,7 +43,7 @@ static int enabled_cpus;
 static bool bootcpu_valid  __initdata;
 
 static bool param_acpi_off __initdata;
-bool param_acpi_force __initdata;
+static bool param_acpi_force __initdata;
 
 static int __init parse_acpi(char *arg)
 {
@@ -284,8 +284,13 @@ out:
  * We can parse ACPI boot-time tables such as MADT after
  * this function is called.
  *
- * ACPI is enabled on return if ACPI tables initialized and sanity checks
- * passed, disabled otherwise
+ * On return ACPI is enabled if either:
+ *
+ * - ACPI tables are initialized and sanity checks passed
+ * - acpi=force was passed in the command line and ACPI was not disabled
+ *   explicitly through acpi=off command line parameter
+ *
+ * ACPI is disabled on function return otherwise
  */
 void __init acpi_boot_table_init(void)
 {
@@ -309,10 +314,13 @@ void __init acpi_boot_table_init(void)
 	 * If ACPI tables are initialized and FADT sanity checks passed,
 	 * leave ACPI enabled and carry on booting; otherwise disable ACPI
 	 * on initialization error.
+	 * If acpi=force was passed on the command line it forces ACPI
+	 * to be enabled even if its initialization failed.
 	 */
 	if (acpi_table_init() || acpi_fadt_sanity_check()) {
 		pr_err("Failed to init ACPI tables\n");
-		disable_acpi();
+		if (!param_acpi_force)
+			disable_acpi();
 	}
 }
 
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index d60b1ad..b278311 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -390,7 +390,7 @@ void __init setup_arch(char **cmdline_p)
 
 	early_ioremap_reset();
 
-	if (acpi_disabled  && !param_acpi_force) {
+	if (acpi_disabled) {
 		unflatten_device_tree();
 		psci_dt_init();
 		cpu_read_bootcpu_ops();
-- 
2.2.1

Re: [PATCH 1/5] ACPI: move arm64 GSI IRQ model to generic GSI IRQ layer

[Lorenzo Pieralisi]

On Wed, Mar 25, 2015 at 01:20:44PM +0000, Marc Zyngier wrote:
> On 24/03/15 17:58, Lorenzo Pieralisi wrote:
> > The code deployed to implement GSI linux IRQ numbers mapping on arm64 turns
> > out to be generic enough so that it can be moved to ACPI core code along
> > with its respective config option ACPI_GENERIC_GSI selectable on
> > architectures that can reuse the same code.
> > 
> > Current ACPI IRQ mapping code is not integrated in the kernel IRQ domain
> > infrastructure, in particular there is no way to look-up the
> > IRQ domain associated with a particular interrupt controller, so this
> > first version of GSI generic code carries out the GSI<->IRQ mapping relying
> > on the IRQ default domain which is supposed to be always set on a
> > specific architecture in case the domain structure passed to
> > irq_create/find_mapping() functions is missing.
> > 
> > This patch moves the arm64 acpi functions that implement the gsi mappings:
> > 
> > acpi_gsi_to_irq()
> > acpi_register_gsi()
> > acpi_unregister_gsi()
> > 
> > to ACPI core code. Since the generic GSI<->domain mapping is based on IRQ
> > domains, it can be extended as soon as a way to map an interrupt
> > controller to an IRQ domain is implemented for ACPI in the IRQ domain
> > layer.
> > 
> > x86 and ia64 code for GSI mappings cannot rely on the generic GSI
> > layer at present for legacy reasons, so they do not select the
> > ACPI_GENERIC_GSI config options and keep relying on their arch
> > specific GSI mapping layer.
> 
> This looks like the right thing to do. I definitely like the sanity
> checking that has been added here.
> 
> FWIW: Acked-by: Marc Zyngier <marc.zyngier@arm.com>

Thank you, I think it makes it easier to add stacked IRQ domains support
later (ie sooner) and by code inspection I suspect x86 can make some use
of this code too with a bit of refactoring.

Lorenzo

Re: [PATCH 5/5] ARM64: kernel: acpi: honour acpi=force command line parameter

[Catalin Marinas]

On Tue, Mar 24, 2015 at 05:58:55PM +0000, Lorenzo Pieralisi wrote:
> If acpi=force is passed on the command line, it forces ACPI to be
> the only available boot method, hence it must be left enabled even
> if the initialization and sanity checks on ACPI tables fails.
> 
> This patch refactors ACPI initialization to prevent disabling ACPI
> if acpi=force is passed on the command line.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>

Better, indeed.