acpi, spcr: Make SPCR available to x86

Note on testing: I've tested this on several ARM64 and x86 boxes (only earlycon, console=ttyS0,115200, and with both options), verified that functionality on ARM64 has not changed (ie, CONFIG_ACPI_SPCR_TABLE is always =y), and verified functionality when !CONFIG_ACPI_SPCR_TABLE on x86.

P.

----8<----

SPCR is currently only enabled or ARM64 and x86 can use SPCR to setup an early console.

General fixes include updating Documentation & Kconfig (for x86), updating comments, and changing parse_spcr() to acpi_parse_spcr(), and earlycon_init_is_deferred to earlycon_acpi_spcr_enable to be more descriptive.

On x86, many systems have a valid SPCR table but the table version is not 2 so the table version check must be a warning.

On ARM64 when the kernel parameter earlycon is used both the early console and console are enabled. On x86, only the earlycon should be enabled by by default. Modify acpi_parse_spcr() to allow options for initializing the early console and console separately.

Signed-off-by Prarit Bhargava <[email protected]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Cc [email protected]

Cc [email protected]

Cc Bhupesh Sharma <[email protected]>

Cc Lv Zheng <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc [email protected]

Cc Jonathan Corbet <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc "Rafael J. Wysocki" [email protected]>

Cc Timur Tabi <[email protected]>

Cc [email protected]

Cc [email protected]

---
 Documentation/admin-guide/kernel-parameters.txt |  9 +++++---
 arch/arm64/kernel/acpi.c                        |  4 ++--
 arch/x86/kernel/acpi/boot.c                     |  3 +++
 drivers/acpi/Kconfig                            |  7 +++++-
 drivers/acpi/spcr.c                             | 29 +++++++++++++------------
 drivers/tty/serial/earlycon.c                   | 15 +++++--------
 include/linux/acpi.h                            |  7 ++++--
 include/linux/serial_core.h                     |  4 ++--
 8 files changed, 44 insertions(+), 34 deletions(-)

Note on testing: I've tested this on several ARM64 and x86 boxes (only earlycon, console=ttyS0,115200, and with both options), verified that functionality on ARM64 has not changed (ie, CONFIG_ACPI_SPCR_TABLE is always =y), and verified functionality when !CONFIG_ACPI_SPCR_TABLE on x86.

P.

----8<----

SPCR is currently only enabled or ARM64 and x86 can use SPCR to setup an early console.

General fixes include updating Documentation & Kconfig (for x86), updating comments, and changing parse_spcr() to acpi_parse_spcr(), and earlycon_init_is_deferred to earlycon_acpi_spcr_enable to be more descriptive.

On x86, many systems have a valid SPCR table but the table version is not 2 so the table version check must be a warning.

On ARM64 when the kernel parameter earlycon is used both the early console and console are enabled. On x86, only the earlycon should be enabled by by default. Modify acpi_parse_spcr() to allow options for initializing the early console and console separately.

Signed-off-by Prarit Bhargava <[email protected]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Cc [email protected]

Cc [email protected]

Cc Bhupesh Sharma <[email protected]>

Cc Lv Zheng <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc [email protected]

Cc Jonathan Corbet <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc "Rafael J. Wysocki" [email protected]>

Cc Timur Tabi <[email protected]>

Cc [email protected]

Cc [email protected]

---
 Documentation/admin-guide/kernel-parameters.txt |  9 +++++---
 arch/arm64/kernel/acpi.c                        |  4 ++--
 arch/x86/kernel/acpi/boot.c                     |  3 +++
 drivers/acpi/Kconfig                            |  7 +++++-
 drivers/acpi/spcr.c                             | 29 +++++++++++++------------
 drivers/tty/serial/earlycon.c                   | 15 +++++--------
 include/linux/acpi.h                            |  7 ++++--
 include/linux/serial_core.h                     |  4 ++--
 8 files changed, 44 insertions(+), 34 deletions(-)

Documentation/admin-guide/kernel-parameters.txt +6 -3

index 46b26bfee27b..6f519230ed34 100644
@@ -915,9 +915,12 @@
 
 	earlycon=	[KNL] Output early console device and options.
 
-			When used with no options, the early console is
-			determined by the stdout-path property in device
-			tree's chosen node.
+			[ARM64] The early console is determined by the
+			stdout-path property in device tree's chosen node,
+			or determined by the ACPI SPCR table.
+
+			[X86] When used with no options the early console is
+			determined by the ACPI SPCR table.
 
 		cdns,<addr>[,options]
 			Start an early, polled-mode console on a Cadence

arch/arm64/kernel/acpi.c +2 -2

index b3162715ed78..2aa5609def27 100644
@@ -230,10 +230,10 @@ void __init acpi_boot_table_init(void)
 
 done:
 	if (acpi_disabled) {
-		if (earlycon_init_is_deferred)
+		if (earlycon_acpi_spcr_enable)
 			early_init_dt_scan_chosen_stdout();
 	} else {
-		parse_spcr(earlycon_init_is_deferred);
+		acpi_parse_spcr(earlycon_acpi_spcr_enable, true);
 		if (IS_ENABLED(CONFIG_ACPI_BGRT))
 			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
 	}

arch/x86/kernel/acpi/boot.c +3 -0

index f4c463df8b08..0bf6a58f7a6d 100644
@@ -36,6 +36,7 @@
 #include <linux/ioport.h>
 #include <linux/pci.h>
 #include <linux/efi-bgrt.h>
+#include <linux/serial_core.h>
 
 #include <asm/e820/api.h>
 #include <asm/irqdomain.h>

@@ -1626,6 +1627,8 @@ int __init acpi_boot_init(void)
 	if (!acpi_noirq)
 		x86_init.pci.init = pci_acpi_init;
 
+	/* Do not enable ACPI SPCR console by default */
+	acpi_parse_spcr(earlycon_acpi_spcr_enable, false);
 	return 0;
 }
 

drivers/acpi/Kconfig +6 -1

index 46505396869e..ddcb5f40e8ee 100644
@@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
 endif
 
 config ACPI_SPCR_TABLE
-	bool
+	bool "ACPI Serial Port Console Redirection Support"
+	default y if X86
+	help
+	  Enable support for Serial Port Console Redirection (SPCR) Table.
+	  This table provides information about the configuration of the
+	  earlycon console.
 
 config ACPI_LPIT
 	bool

drivers/acpi/spcr.c +15 -14

index 324b35bfe781..89e97d21a89c 100644
@@ -21,7 +21,7 @@
  * occasionally getting stuck as 1. To avoid the potential for a hang, check
  * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
  * implementations, so only do so if an affected platform is detected in
- * parse_spcr().
+ * acpi_parse_spcr().
  */
 bool qdf2400_e44_present;
 EXPORT_SYMBOL(qdf2400_e44_present);

@@ -74,19 +74,21 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
 }
 
 /**
- * parse_spcr() - parse ACPI SPCR table and add preferred console
+ * acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
  *
- * @earlycon: set up earlycon for the console specified by the table
+ * @enable_earlycon: set up earlycon for the console specified by the table
+ * @enable_console: setup the console specified by the table.
  *
  * For the architectures with support for ACPI, CONFIG_ACPI_SPCR_TABLE may be
  * defined to parse ACPI SPCR table.  As a result of the parsing preferred
- * console is registered and if @earlycon is true, earlycon is set up.
+ * console is registered and if @enable_earlycon is true, earlycon is set up.
+ * If @enable_console is true the system console is also configured.
  *
  * When CONFIG_ACPI_SPCR_TABLE is defined, this function should be called
  * from arch initialization code as soon as the DT/ACPI decision is made.
  *
  */
-int __init parse_spcr(bool earlycon)
+int __init acpi_parse_spcr(bool enable_earlycon, bool enable_console)
 {
 	static char opts[64];
 	struct acpi_table_spcr *table;

@@ -105,11 +107,8 @@ int __init parse_spcr(bool earlycon)
 	if (ACPI_FAILURE(status))
 		return -ENOENT;
 
-	if (table->header.revision < 2) {
-		err = -ENOENT;
-		pr_err("wrong table version\n");
-		goto done;
-	}
+	if (table->header.revision < 2)
+		pr_info("SPCR table version %d\n", table->header.revision);
 
 	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
 		switch (ACPI_ACCESS_BIT_WIDTH((

@@ -185,7 +184,7 @@ int __init parse_spcr(bool earlycon)
 	 */
 	if (qdf2400_erratum_44_present(&table->header)) {
 		qdf2400_e44_present = true;
-		if (earlycon)
+		if (enable_earlycon)
 			uart = "qdf2400_e44";
 	}
 

@@ -205,11 +204,13 @@ int __init parse_spcr(bool earlycon)
 
 	pr_info("console: %s\n", opts);
 
-	if (earlycon)
+	if (enable_earlycon)
 		setup_earlycon(opts);
 
-	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
-
+	if (enable_console)
+		err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
+	else
+		err = 0;
 done:
 	acpi_put_table((struct acpi_table_header *)table);
 	return err;

drivers/tty/serial/earlycon.c +5 -10

index 4c8b80f1c688..870e84fb6e39 100644
@@ -197,25 +197,20 @@ int __init setup_earlycon(char *buf)
 }
 
 /*
- * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
- * command line does not start DT earlycon immediately, instead it defers
- * starting it until DT/ACPI decision is made.  At that time if ACPI is enabled
- * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
+ * This defers the initialization of the early console until after ACPI has
+ * been initialized.
  */
-bool earlycon_init_is_deferred __initdata;
+bool earlycon_acpi_spcr_enable __initdata;
 
 /* early_param wrapper for setup_earlycon() */
 static int __init param_setup_earlycon(char *buf)
 {
 	int err;
 
-	/*
-	 * Just 'earlycon' is a valid param for devicetree earlycons;
-	 * don't generate a warning from parse_early_params() in that case
-	 */
+	/* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
 	if (!buf || !buf[0]) {
 		if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
-			earlycon_init_is_deferred = true;
+			earlycon_acpi_spcr_enable = true;
 			return 0;
 		} else if (!buf) {
 			return early_init_dt_scan_chosen_stdout();

include/linux/acpi.h +5 -2

index dc1ebfeeb5ec..9aac8f0ebe23 100644
@@ -1242,9 +1242,12 @@ static inline void acpi_table_upgrade(void) { }
 
 #ifdef CONFIG_ACPI_SPCR_TABLE
 extern bool qdf2400_e44_present;
-int parse_spcr(bool earlycon);
+int acpi_parse_spcr(bool enable_earlycon, bool enable_console);
 #else
-static inline int parse_spcr(bool earlycon) { return 0; }
+static inline int acpi_parse_spcr(bool enable_earlycon, bool enable_console)
+{
+	return 0;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)

include/linux/serial_core.h +0 -0

index 37b044e78333..aefd0e5115da 100644
@@ -376,10 +376,10 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
 			     const char *options);
 

arch/arm64/kernel/acpi.c void __init acpi_boot_table_init(void) · line 233

Quoted 3 lines · Prarit Bhargava

      if (acpi_disabled) {

arch/arm64/kernel/acpi.c +1 -1

-        if (earlycon_init_is_deferred)
+        if (earlycon_acpi_spcr_enable)

This patch works for me, so I can ACK it, but first you might want to rename earlycon_acpi_spcr_enable, because these two lines don't make much sense.

"If ACPI is disabled, and ACPI SCPR is enabled, then ...."

If ACPI is disabled, then how can a variable called "earlycon_acpi_spcr_enable" be true?

Would it make more sense to rename it to earlycon_spcr_enable?

drivers/acpi/Kconfig config ACPI_DEBUGGER_USER · line 87

Quoted 7 lines · Prarit Bhargava

config ACPI_SPCR_TABLE

drivers/acpi/Kconfig +6 -0

+	bool "ACPI Serial Port Console Redirection Support"
+	default y if X86
+	help
+	  Enable support for Serial Port Console Redirection (SPCR) Table.
+	  This table provides information about the configuration of the
+	  earlycon console.

I just got asked this by oldconfig, which left me rather puzzled, WTF does this do?

ACPI SPCR is used by a vendor to define the serial console for a system. If SPCR exists a user can add kernel parameter "earlycon" (no extra kernel parameters) and the console will work out-of-the-box.

Quoted 3 lines · Prarit Bhargava

ACPI SPCR is used by a vendor to define the serial console for a system. If SPCR exists a user can add kernel parameter "earlycon" (no extra kernel parameters) and the console will work out-of-the-box.

"earlycon" is needed only for an *early* console. You don't need any command line parameters for a normal console to work with SPCR.

That's true on ARM64. I specifically did not enable the console by default on x86. There are users who do not want a console active during boot & runtime.

Quoted 11 lines · Peter Zijlstra

drivers/acpi/Kconfig config ACPI_DEBUGGER_USER · line 87

Quoted 7 lines · Prarit Bhargava

config ACPI_SPCR_TABLE

drivers/acpi/Kconfig +6 -0

+	bool "ACPI Serial Port Console Redirection Support"
+	default y if X86
+	help
+	  Enable support for Serial Port Console Redirection (SPCR) Table.
+	  This table provides information about the configuration of the
+	  earlycon console.

I just got asked this by oldconfig, which left me rather puzzled, WTF does this do?

Odd -- I thought I had taken care of that :( My apologies Peter.

ACPI SPCR is used by a vendor to define the serial console for a system. If SPCR exists a user can add kernel parameter "earlycon" (no extra kernel parameters) and the console will work out-of-the-box.

The serial console configuration varies from vendor to vendor. This takes the guess-work out of defining a serial console.

Quoted 21 lines · Prarit Bhargava

Quoted 11 lines · Peter Zijlstra

Review

Quoted 7 lines · Prarit Bhargava

config ACPI_SPCR_TABLE

(quoted diff) +6 -0

+	bool "ACPI Serial Port Console Redirection Support"
+	default y if X86
+	help
+	  Enable support for Serial Port Console Redirection (SPCR) Table.
+	  This table provides information about the configuration of the
+	  earlycon console.

I just got asked this by oldconfig, which left me rather puzzled, WTF does this do?

Odd -- I thought I had taken care of that :( My apologies Peter.

ACPI SPCR is used by a vendor to define the serial console for a system. If SPCR exists a user can add kernel parameter "earlycon" (no extra kernel parameters) and the console will work out-of-the-box.

Quoted 2 lines

The serial console configuration varies from vendor to vendor. This takes the guess-work out of defining a serial console.

But when I specify "earlyprintk=serial,ttyS0,115200" this SPCR crud will not interfere?

Quoted 3 lines

But when I specify "earlyprintk=serial,ttyS0,115200" this SPCR crud will not interfere?

I tested "earlyprintk=serial,ttyS0,115200" on a system which is known to have a functional console with "console=ttyS0,1152008N1" both with and without CONFIG_ACPI_SPCR_TABLE enabled.

Quoted 4 lines · Prarit Bhargava

Quoted 3 lines

But when I specify "earlyprintk=serial,ttyS0,115200" this SPCR crud will not interfere?

Quoted 3 lines

I tested "earlyprintk=serial,ttyS0,115200" on a system which is known to have a functional console with "console=ttyS0,1152008N1" both with and without CONFIG_ACPI_SPCR_TABLE enabled.

OK, but my point was that SPCR will never override explicit earlyprintk stuff. There is absolutely nothing worse than breaking working setups.