linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: lorenzo.pieralisi@arm.com (Lorenzo Pieralisi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v10 18/21] ARM64 / ACPI: Select ACPI_REDUCED_HARDWARE_ONLY if ACPI is enabled on ARM64
Date: Fri, 13 Mar 2015 11:04:21 +0000	[thread overview]
Message-ID: <20150313110421.GA5229@red-moon> (raw)
In-Reply-To: <5502596D.6020901@huawei.com>

On Fri, Mar 13, 2015 at 03:28:45AM +0000, Hanjun Guo wrote:

[...]

> >  /*
> >   * acpi_boot_table_init() called from setup_arch(), always.
> >   *	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.
> >   */
> >  void __init acpi_boot_table_init(void)
> >  {
> > +	struct acpi_table_header *table;
> > +	struct acpi_table_fadt *fadt;
> > +	acpi_status status;
> > +	acpi_size tbl_size;
> > +
> >  	/*
> >  	 * Enable ACPI instead of device tree unless
> >  	 * - ACPI has been disabled explicitly (acpi=off), or
> > @@ -351,19 +318,52 @@ void __init acpi_boot_table_init(void)
> >  	    (!param_acpi_force && of_scan_flat_dt(dt_scan_depth1_nodes, NULL)))
> >  		return;
> >  
> > -	enable_acpi();
> > -
> >  	/* Initialize the ACPI boot-time table parser. */
> >  	if (acpi_table_init()) {
> 
> Since we disable ACPI in default, it is a bit strange for me to init all
> the ACPI tables and parse FADT when ACPI is disabled, could you
> put some comments here to clarify the purpose? other than that, it is looks
> good to me.

Ok, the purpose was to make things simpler, but I think that given
current code it is not 100% safe to init ACPI tables with
acpi_disabled == 1.

To me having to enable ACPI to parse the tables and check *if* ACPI tables
are there is a bit crazy, but I agree with you that given current code
it is safer.

Patch rewritten, here below, please have a look, test it and rework
bits as needed, I added comments where I thought they were needed but
please add to that if you feel it is worth it.

It should be easy to split, let me know if you want an incremental
version.

Thanks !
Lorenzo

---
 arch/arm64/kernel/acpi.c | 103 +++++++++++++++++++++++++++++++----------------
 1 file changed, 69 insertions(+), 34 deletions(-)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 5819ef7..e5ee4d4 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -291,43 +291,60 @@ void acpi_unregister_gsi(u32 gsi)
 }
 EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
 
-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((char *)table, tbl_size);
+	return ret;
 }
 
 /*
@@ -335,12 +352,17 @@ 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)
 {
+
 	/*
 	 * Enable ACPI instead of device tree unless
 	 * - ACPI has been disabled explicitly (acpi=off), or
@@ -351,19 +373,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;
+		pr_err("Failed to init ACPI tables\n");
+		goto init_failed;
 	}
 
-	if (acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt)) {
-		/* disable ACPI if no FADT is found */
-		disable_acpi();
-		pr_err("Can't find FADT\n");
-	}
+	/*
+	 * Check FADT presence and carry out FADT sanity checks
+	 */
+	if (acpi_fadt_sanity_check() < 0)
+		goto init_failed;
+
+	/*
+	 * ACPI tables initialized and FADT sanity checks passed, leave
+	 * ACPI enabled and carry on booting
+	 */
+	return;
+
+init_failed:
+	disable_acpi();
 }
 
 void __init acpi_gic_init(void)
-- 
1.9.1

  reply	other threads:[~2015-03-13 11:04 UTC|newest]

Thread overview: 89+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-11 12:39 [PATCH v10 00/21] Introduce ACPI for ARM64 based on ACPI 5.1 Hanjun Guo
2015-03-11 12:39 ` [PATCH v10 01/21] ACPI / table: Use pr_debug() instead of pr_info() for MADT table scanning Hanjun Guo
2015-03-11 12:39 ` [PATCH v10 02/21] ACPI: add arm64 to the platforms that use ioremap Hanjun Guo
2015-03-11 12:39 ` [PATCH v10 03/21] ARM64: allow late use of early_ioremap Hanjun Guo
2015-03-11 12:39 ` [PATCH v10 04/21] ARM64 / ACPI: Get RSDP and ACPI boot-time tables Hanjun Guo
2015-03-11 12:39 ` [PATCH v10 05/21] ACPI: fix acpi_os_ioremap for arm64 Hanjun Guo
2015-03-11 12:39 ` [PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP Hanjun Guo
2015-03-12  9:32   ` Lorenzo Pieralisi
2015-03-12 22:57   ` Rafael J. Wysocki
2015-03-13  3:31     ` Hanjun Guo
2015-03-11 12:39 ` [PATCH v10 07/21] ARM64 / ACPI: Introduce PCI stub functions for ACPI Hanjun Guo
2015-03-11 12:39 ` [PATCH v10 08/21] ARM64 / ACPI: Introduce early_param "acpi=" to enable/disable ACPI Hanjun Guo
2015-03-18 11:35   ` Lorenzo Pieralisi
2015-03-18 20:07     ` Ard Biesheuvel
2015-03-19  2:30       ` Hanjun Guo
2015-03-19 10:04       ` Lorenzo Pieralisi
2015-03-11 12:39 ` [PATCH v10 09/21] ARM64 / ACPI: If we chose to boot from acpi then disable FDT Hanjun Guo
2015-03-18 16:52   ` Catalin Marinas
2015-03-11 12:39 ` [PATCH v10 10/21] ARM64 / ACPI: Get PSCI flags in FADT for PSCI init Hanjun Guo
2015-03-13 14:51   ` Lorenzo Pieralisi
2015-03-16 11:45     ` Hanjun Guo
2015-03-16 18:41       ` Lorenzo Pieralisi
2015-03-11 12:39 ` [PATCH v10 11/21] ACPI / table: Print GIC information when MADT is parsed Hanjun Guo
2015-03-11 12:39 ` [PATCH v10 12/21] ARM64 / ACPI: Parse MADT for SMP initialization Hanjun Guo
2015-03-11 12:39 ` [PATCH v10 13/21] ACPI / processor: Introduce phys_cpuid_t for CPU hardware ID Hanjun Guo
2015-03-12  9:51   ` Lorenzo Pieralisi
2015-03-12 10:16     ` Hanjun Guo
2015-03-11 12:39 ` [PATCH v10 14/21] ACPI / processor: Make it possible to get CPU hardware ID via GICC Hanjun Guo
2015-03-12 15:41   ` Lorenzo Pieralisi
2015-03-12 23:02   ` Rafael J. Wysocki
2015-03-11 12:39 ` [PATCH v10 15/21] ARM64 / ACPI: Introduce ACPI_IRQ_MODEL_GIC and register device's gsi Hanjun Guo
2015-03-18 18:41   ` Will Deacon
2015-03-19  3:45     ` Hanjun Guo
2015-03-19 10:12       ` Lorenzo Pieralisi
2015-03-19 19:37         ` Will Deacon
2015-03-20 13:07           ` Hanjun Guo
2015-03-20 14:25             ` Lorenzo Pieralisi
2015-03-21 21:38           ` Lorenzo Pieralisi
2015-03-11 12:39 ` [PATCH v10 16/21] irqchip: Add GICv2 specific ACPI boot support Hanjun Guo
     [not found]   ` <CACxGe6uWwts6X=Yc2ioBdQizXkF1_YgoNNOsREWirk2MFBVDHg@mail.gmail.com>
2015-03-11 23:11     ` Jason Cooper
2015-03-12  1:46       ` Hanjun Guo
2015-03-12  5:12         ` Jason Cooper
2015-03-12  7:31           ` Hanjun Guo
2015-03-13 17:15             ` Jason Cooper
2015-03-14  8:47               ` Grant Likely
2015-03-14 11:43                 ` Catalin Marinas
2015-03-12 10:14       ` Marc Zyngier
2015-03-14 18:44   ` Jason Cooper
2015-03-11 12:39 ` [PATCH v10 17/21] clocksource / arch_timer: Parse GTDT to initialize arch timer Hanjun Guo
2015-03-18 18:34   ` Will Deacon
2015-03-20 13:49   ` Daniel Lezcano
2015-03-11 12:39 ` [PATCH v10 18/21] ARM64 / ACPI: Select ACPI_REDUCED_HARDWARE_ONLY if ACPI is enabled on ARM64 Hanjun Guo
2015-03-12 18:21   ` Lorenzo Pieralisi
2015-03-13  3:28     ` Hanjun Guo
2015-03-13 11:04       ` Lorenzo Pieralisi [this message]
2015-03-16 11:33         ` Hanjun Guo
2015-03-17 12:50           ` Lorenzo Pieralisi
2015-03-18  9:18           ` Lorenzo Pieralisi
2015-03-18 15:06             ` Rafael J. Wysocki
2015-03-19  1:16               ` Hanjun Guo
2015-03-11 12:39 ` [PATCH v10 19/21] ARM64 / ACPI: Enable ARM64 in Kconfig Hanjun Guo
2015-03-11 12:39 ` [PATCH v10 20/21] Documentation: ACPI for ARM64 Hanjun Guo
2015-03-11 12:39 ` [PATCH v10 21/21] ARM64 / ACPI: additions of ACPI documentation for arm64 Hanjun Guo
2015-03-12 13:26 ` [PATCH v10 00/21] Introduce ACPI for ARM64 based on ACPI 5.1 Timur Tabi
2015-03-16  5:07   ` Suthikulpanit, Suravee
2015-03-18 19:05 ` Will Deacon
2015-03-18 19:09   ` Will Deacon
2015-03-19  4:09   ` Hanjun Guo
2015-03-19 10:17     ` Lorenzo Pieralisi
2015-03-19 19:39       ` Will Deacon
2015-03-24 22:02         ` Grant Likely
2015-03-25 11:24           ` Will Deacon
2015-03-25 11:54             ` Rafael J. Wysocki
2015-03-25 11:38               ` Will Deacon
2015-03-25 12:16                 ` Rafael J. Wysocki
2015-03-28 12:34                 ` Grant Likely
2015-03-26 10:24           ` Lorenzo Pieralisi
2015-03-20 18:54     ` Will Deacon
2015-03-21  3:17       ` Hanjun Guo
2015-03-21  7:03         ` Hanjun Guo
     [not found]           ` <CAFoFrHatzS3MwGVeOPPjY1R1sfBRYnJjgbQjvfzi6xS+XYD14g@mail.gmail.com>
2015-03-22 21:05             ` Julien Grall
2015-03-22 21:49               ` Rafael J. Wysocki
2015-03-22 21:32                 ` Julien Grall
2015-03-22 22:11                   ` Rafael J. Wysocki
2015-03-23  1:37                     ` Hanjun Guo
2015-03-23 18:39                       ` Stefano Stabellini
2015-03-23 18:32         ` Stefano Stabellini
2015-03-24 13:46           ` Hanjun Guo
2015-03-20 13:18 ` Mark Salter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150313110421.GA5229@red-moon \
    --to=lorenzo.pieralisi@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).