[PATCH 0/2] acpi, x86: Add SPCR table support

[PATCH 0/2] acpi, x86: Add SPCR table support

[Prarit Bhargava]

The SPCR (Serial Port Console Redirection) Table provides information
about the configuration of serial port.  This information can be used
to configure the early console.

SPCR support was added for arm64 and is made available across all arches
in this patchset.  The first patch adds a weak per-arch configuration function
and moves the SPCR code into ACPI.  The second patch adds support to x86.

The existing behaviour on arm64 is maintained.  If the SPCR exists the
earlycon and console are automatically configured.

The existing default behaviour on x86 is also maintained.  If no console or
earlycon parameter is defined and the SPCR exists , the serial port is not
configured.  If the earlycon parameter is used both the early console
and the console are configured using the data from the SPCR.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-pm at vger.kernel.org
Cc: linux-acpi at vger.kernel.org
Cc: linux-serial at vger.kernel.org
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86 at kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>

Prarit Bhargava (2):
  acpi, spcr: Make SPCR avialable to other architectures
  acpi, x86: Use SPCR table for earlycon on x86

 Documentation/admin-guide/kernel-parameters.txt |   9 +-
 arch/arm64/Kconfig                              |   1 -
 arch/arm64/kernel/acpi.c                        | 128 +++++++++++++++++-
 arch/x86/kernel/acpi/boot.c                     |   3 +
 drivers/acpi/Kconfig                            |  11 +-
 drivers/acpi/spcr.c                             | 171 ++++++------------------
 drivers/tty/serial/earlycon.c                   |  16 +--
 include/linux/acpi.h                            |  14 +-
 include/linux/serial_core.h                     |   2 -
 9 files changed, 195 insertions(+), 160 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Prarit Bhargava]

Other architectures can use SPCR to setup an early console or console but
the current code is ARM64 specific.

Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
function acpi_arch_setup_console() that can be used for arch-specific
setup.  Move SPCR initialization flag into ACPI code.  Update the
Documention on the use of the SPCR earlycon.

This patch does not change arm64 behaviour.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-pm at vger.kernel.org
Cc: linux-acpi at vger.kernel.org
Cc: linux-serial at vger.kernel.org
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86 at kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 arch/arm64/Kconfig                              |   1 -
 arch/arm64/kernel/acpi.c                        | 128 +++++++++++++++++-
 drivers/acpi/Kconfig                            |  11 +-
 drivers/acpi/spcr.c                             | 168 +++++-------------------
 drivers/tty/serial/earlycon.c                   |  16 +--
 include/linux/acpi.h                            |  11 +-
 include/linux/serial_core.h                     |   2 -
 8 files changed, 184 insertions(+), 159 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 6571fbfdb2a1..0d173289c67e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -914,9 +914,9 @@
 
 	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.
 
 		cdns,<addr>[,options]
 			Start an early, polled-mode console on a Cadence
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a93339f5178f..c6f91c85abf6 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -6,7 +6,6 @@ config ARM64
 	select ACPI_IORT if ACPI
 	select ACPI_REDUCED_HARDWARE_ONLY if ACPI
 	select ACPI_MCFG if ACPI
-	select ACPI_SPCR_TABLE if ACPI
 	select ARCH_CLOCKSOURCE_DATA
 	select ARCH_HAS_DEBUG_VIRTUAL
 	select ARCH_HAS_DEVMEM_IS_ALLOWED
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3162715ed78..ef74e1f3e468 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -25,7 +25,6 @@
 #include <linux/memblock.h>
 #include <linux/of_fdt.h>
 #include <linux/smp.h>
-#include <linux/serial_core.h>
 
 #include <asm/cputype.h>
 #include <asm/cpu_ops.h>
@@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
 	return ret;
 }
 
+/*
+ * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
+ * 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
+ * acpi_parse_spcr().
+ */
+bool qdf2400_e44_present;
+EXPORT_SYMBOL(qdf2400_e44_present);
+
+/*
+ * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
+ * Detect them by examining the OEM fields in the SPCR header, similar to PCI
+ * quirk detection in pci_mcfg.c.
+ */
+static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
+{
+	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
+		return false;
+
+	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
+		return true;
+
+	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
+			h->oem_revision == 1)
+		return true;
+
+	return false;
+}
+
+/*
+ * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
+ * register aligned to 32-bit. In addition, the BIOS also encoded the
+ * access width to be 8 bits. This function detects this errata condition.
+ */
+static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
+{
+	bool xgene_8250 = false;
+
+	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
+		return false;
+
+	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
+	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
+		return false;
+
+	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
+	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
+		xgene_8250 = true;
+
+	if (!memcmp(tb->header.oem_table_id, "ProLiant",
+	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
+		xgene_8250 = true;
+
+	return xgene_8250;
+}
+
+int acpi_arch_setup_console(struct acpi_table_spcr *table,
+			    char *opts, char *uart, char *iotype,
+			    int baud_rate, bool earlycon)
+{
+	if (table->header.revision < 2) {
+		pr_err("wrong table version\n");
+		return -ENOENT;
+	}
+
+	switch (table->interface_type) {
+	case ACPI_DBG2_ARM_SBSA_32BIT:
+		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
+		/* fall through */
+	case ACPI_DBG2_ARM_PL011:
+	case ACPI_DBG2_ARM_SBSA_GENERIC:
+	case ACPI_DBG2_BCM2835:
+		snprintf(uart, ACPI_SPCR_BUF_SIZE, "p1101");
+		break;
+	default:
+		if (strlen(uart) == 0)
+			return -ENOENT;
+	}
+
+	/*
+	 * If the E44 erratum is required, then we need to tell the pl011
+	 * driver to implement the work-around.
+	 *
+	 * The global variable is used by the probe function when it
+	 * creates the UARTs, whether or not they're used as a console.
+	 *
+	 * If the user specifies "traditional" earlycon, the qdf2400_e44
+	 * console name matches the EARLYCON_DECLARE() statement, and
+	 * SPCR is not used.  Parameter "earlycon" is false.
+	 *
+	 * If the user specifies "SPCR" earlycon, then we need to update
+	 * the console name so that it also says "qdf2400_e44".  Parameter
+	 * "earlycon" is true.
+	 *
+	 * For consistency, if we change the console name, then we do it
+	 * for everyone, not just earlycon.
+	 */
+	if (qdf2400_erratum_44_present(&table->header)) {
+		qdf2400_e44_present = true;
+		if (earlycon)
+			snprintf(uart, ACPI_SPCR_BUF_SIZE, "qdf2400_e44");
+	}
+
+	if (xgene_8250_erratum_present(table)) {
+		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
+
+		/* for xgene v1 and v2 we don't know the clock rate of the
+		 * UART so don't attempt to change to the baud rate state
+		 * in the table because driver cannot calculate the dividers
+		 */
+		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx", uart,
+			 iotype, table->serial_port.address);
+	} else {
+		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart,
+			 iotype, table->serial_port.address, baud_rate);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(acpi_arch_setup_console);
+
 /*
  * acpi_boot_table_init() called from setup_arch(), always.
  *	1. find RSDP and get its address, and then find XSDT
@@ -230,10 +351,11 @@ void __init acpi_boot_table_init(void)
 
 done:
 	if (acpi_disabled) {
-		if (earlycon_init_is_deferred)
+		if (console_acpi_spcr_enable)
 			early_init_dt_scan_chosen_stdout();
 	} else {
-		parse_spcr(earlycon_init_is_deferred);
+		/* Always enable the ACPI SPCR console */
+		acpi_parse_spcr(console_acpi_spcr_enable);
 		if (IS_ENABLED(CONFIG_ACPI_BGRT))
 			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
 	}
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 46505396869e..ae25159dd976 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -78,9 +78,6 @@ config ACPI_DEBUGGER_USER
 
 endif
 
-config ACPI_SPCR_TABLE
-	bool
-
 config ACPI_LPIT
 	bool
 	depends on X86_64
@@ -563,4 +560,12 @@ config TPS68470_PMIC_OPREGION
 	  region, which must be available before any of the devices
 	  using this, are probed.
 
+config ACPI_SPCR_TABLE
+	bool "ACPI Serial Port Console Redirection Support"
+	default y if ARM64
+	help
+	  Enable support for Serial Port Console Redirection (SPCR) Table.
+	  This table provides information about the configuration of the
+	  earlycon console.
+
 endif	# ACPI
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index 324b35bfe781..d8ad8fa67582 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -16,65 +16,19 @@
 #include <linux/kernel.h>
 #include <linux/serial_core.h>
 
-/*
- * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
- * 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().
- */
-bool qdf2400_e44_present;
-EXPORT_SYMBOL(qdf2400_e44_present);
-
-/*
- * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
- * Detect them by examining the OEM fields in the SPCR header, similiar to PCI
- * quirk detection in pci_mcfg.c.
- */
-static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
-{
-	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
-		return false;
-
-	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
-		return true;
-
-	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
-			h->oem_revision == 1)
-		return true;
-
-	return false;
-}
-
-/*
- * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
- * register aligned to 32-bit. In addition, the BIOS also encoded the
- * access width to be 8 bits. This function detects this errata condition.
- */
-static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
+int __weak acpi_arch_setup_console(struct acpi_table_spcr *table,
+				   char *opts, char *uart, char *iotype,
+				   int baud_rate, bool earlycon)
 {
-	bool xgene_8250 = false;
-
-	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
-		return false;
+	snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart, iotype,
+		 table->serial_port.address, baud_rate);
+	return 0;
 
-	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
-	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
-		return false;
-
-	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
-	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
-		xgene_8250 = true;
-
-	if (!memcmp(tb->header.oem_table_id, "ProLiant",
-	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
-		xgene_8250 = true;
-
-	return xgene_8250;
 }
 
+bool console_acpi_spcr_enable __initdata;
 /**
- * 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
  *
@@ -86,13 +40,13 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
  * 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 earlycon)
 {
-	static char opts[64];
+	static char opts[ACPI_SPCR_OPTS_SIZE];
+	static char uart[ACPI_SPCR_BUF_SIZE];
+	static char iotype[ACPI_SPCR_BUF_SIZE];
 	struct acpi_table_spcr *table;
 	acpi_status status;
-	char *uart;
-	char *iotype;
 	int baud_rate;
 	int err;
 
@@ -105,103 +59,54 @@ int __init parse_spcr(bool earlycon)
 	if (ACPI_FAILURE(status))
 		return -ENOENT;
 
-	if (table->header.revision < 2) {
+	switch (table->baud_rate) {
+	case 3:
+		baud_rate = 9600;
+		break;
+	case 4:
+		baud_rate = 19200;
+		break;
+	case 6:
+		baud_rate = 57600;
+		break;
+	case 7:
+		baud_rate = 115200;
+		break;
+	default:
 		err = -ENOENT;
-		pr_err("wrong table version\n");
 		goto done;
 	}
 
 	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
-		switch (ACPI_ACCESS_BIT_WIDTH((
-			table->serial_port.access_width))) {
+		u8 width = ACPI_ACCESS_BIT_WIDTH((
+					table->serial_port.access_width));
+		switch (width) {
 		default:
 			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
+			width = 8;
 		case 8:
-			iotype = "mmio";
-			break;
 		case 16:
-			iotype = "mmio16";
-			break;
 		case 32:
-			iotype = "mmio32";
+			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio%d", width);
 			break;
 		}
 	} else
-		iotype = "io";
+		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "io");
 
 	switch (table->interface_type) {
-	case ACPI_DBG2_ARM_SBSA_32BIT:
-		iotype = "mmio32";
-		/* fall through */
-	case ACPI_DBG2_ARM_PL011:
-	case ACPI_DBG2_ARM_SBSA_GENERIC:
-	case ACPI_DBG2_BCM2835:
-		uart = "pl011";
-		break;
 	case ACPI_DBG2_16550_COMPATIBLE:
 	case ACPI_DBG2_16550_SUBSET:
-		uart = "uart";
+		snprintf(uart, ACPI_SPCR_BUF_SIZE, "uart");
 		break;
 	default:
 		err = -ENOENT;
 		goto done;
 	}
 
-	switch (table->baud_rate) {
-	case 3:
-		baud_rate = 9600;
-		break;
-	case 4:
-		baud_rate = 19200;
-		break;
-	case 6:
-		baud_rate = 57600;
-		break;
-	case 7:
-		baud_rate = 115200;
-		break;
-	default:
-		err = -ENOENT;
+	err = acpi_arch_setup_console(table, opts, uart, iotype, baud_rate,
+				      earlycon);
+	if (err)
 		goto done;
-	}
-
-	/*
-	 * If the E44 erratum is required, then we need to tell the pl011
-	 * driver to implement the work-around.
-	 *
-	 * The global variable is used by the probe function when it
-	 * creates the UARTs, whether or not they're used as a console.
-	 *
-	 * If the user specifies "traditional" earlycon, the qdf2400_e44
-	 * console name matches the EARLYCON_DECLARE() statement, and
-	 * SPCR is not used.  Parameter "earlycon" is false.
-	 *
-	 * If the user specifies "SPCR" earlycon, then we need to update
-	 * the console name so that it also says "qdf2400_e44".  Parameter
-	 * "earlycon" is true.
-	 *
-	 * For consistency, if we change the console name, then we do it
-	 * for everyone, not just earlycon.
-	 */
-	if (qdf2400_erratum_44_present(&table->header)) {
-		qdf2400_e44_present = true;
-		if (earlycon)
-			uart = "qdf2400_e44";
-	}
-
-	if (xgene_8250_erratum_present(table)) {
-		iotype = "mmio32";
-
-		/* for xgene v1 and v2 we don't know the clock rate of the
-		 * UART so don't attempt to change to the baud rate state
-		 * in the table because driver cannot calculate the dividers
-		 */
-		snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
-			 table->serial_port.address);
-	} else {
-		snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
-			 table->serial_port.address, baud_rate);
-	}
 
 	pr_info("console: %s\n", opts);
 
@@ -209,7 +114,6 @@ int __init parse_spcr(bool earlycon)
 		setup_earlycon(opts);
 
 	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
-
 done:
 	acpi_put_table((struct acpi_table_header *)table);
 	return err;
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 4c8b80f1c688..b0a5964dd20f 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -196,26 +196,16 @@ int __init setup_earlycon(char *buf)
 	return -ENOENT;
 }
 
-/*
- * 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()
- */
-bool earlycon_init_is_deferred __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]) {
+		/* Do not start early console until ACPI has been enabled. */
 		if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
-			earlycon_init_is_deferred = true;
+			console_acpi_spcr_enable = true;
 			return 0;
 		} else if (!buf) {
 			return early_init_dt_scan_chosen_stdout();
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index dc1ebfeeb5ec..818c1053cc6f 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1242,9 +1242,16 @@ static inline bool acpi_has_watchdog(void) { return false; }
 
 #ifdef CONFIG_ACPI_SPCR_TABLE
 extern bool qdf2400_e44_present;
-int parse_spcr(bool earlycon);
+#define ACPI_SPCR_OPTS_SIZE 64
+#define ACPI_SPCR_BUF_SIZE 32
+extern bool console_acpi_spcr_enable __initdata;
+extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
+				   char *opts, char *uart, char *iotype,
+				   int baud_rate, bool earlycon);
+int acpi_parse_spcr(bool earlycon);
 #else
-static inline int parse_spcr(bool earlycon) { return 0; }
+static const bool console_acpi_spcr_enable;
+static inline int acpi_parse_spcr(bool earlycon) { return 0; }
 #endif
 
 #if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 37b044e78333..abfffb4b1c37 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -376,10 +376,8 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
 			     const char *options);
 
 #ifdef CONFIG_SERIAL_EARLYCON
-extern bool earlycon_init_is_deferred __initdata;
 int setup_earlycon(char *buf);
 #else
-static const bool earlycon_init_is_deferred;
 static inline int setup_earlycon(char *buf) { return 0; }
 #endif
 
-- 
2.15.0.rc0.39.g2f0e14e64

[PATCH 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Timur Tabi]

On Thu, Dec 7, 2017 at 11:29 AM, Prarit Bhargava <prarit@redhat.com> wrote:
> Other architectures can use SPCR to setup an early console or console but
> the current code is ARM64 specific.
>
> Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
> function acpi_arch_setup_console() that can be used for arch-specific
> setup.  Move SPCR initialization flag into ACPI code.  Update the
> Documention on the use of the SPCR earlycon.
>
> This patch does not change arm64 behaviour.

Let's hope so.  Our E44 erratum work-around has caused lots of
problems in the past, so the code is a bit fragile.

> +static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> +{
> +       if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
> +               return false;
> +
> +       if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> +               return true;
> +
> +       if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> +                       h->oem_revision == 1)
> +               return true;
> +
> +       return false;
> +}

Please give us a chance to test this patch before merging. We've had a
lot of problems with our E44 work-around, and we need to make sure
that qdf2400_erratum_44_present function is called before the pl011
driver is probed.

> +
> +/*
> + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> + * register aligned to 32-bit. In addition, the BIOS also encoded the
> + * access width to be 8 bits. This function detects this errata condition.
> + */
> +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> +{
> +       bool xgene_8250 = false;
> +
> +       if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> +               return false;
> +
> +       if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> +           memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
> +               return false;
> +
> +       if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> +           ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> +               xgene_8250 = true;
> +
> +       if (!memcmp(tb->header.oem_table_id, "ProLiant",
> +           ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> +               xgene_8250 = true;
> +
> +       return xgene_8250;
> +}

I suspect that this function has the same issues as
qdf2400_erratum_44_present().

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

So ACPI without SPCR has never been tested by us.  Making it optional
makes me a little nervous.  We'll have to evaluate this change.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Prarit Bhargava]

On 12/07/2017 01:43 PM, Timur Tabi wrote:
> On Thu, Dec 7, 2017 at 11:29 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>> Other architectures can use SPCR to setup an early console or console but
>> the current code is ARM64 specific.
>>
>> Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
>> function acpi_arch_setup_console() that can be used for arch-specific
>> setup.  Move SPCR initialization flag into ACPI code.  Update the
>> Documention on the use of the SPCR earlycon.
>>
>> This patch does not change arm64 behaviour.
> 
> Let's hope so.  Our E44 erratum work-around has caused lots of
> problems in the past, so the code is a bit fragile.
> 
>> +static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
>> +{
>> +       if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
>> +               return false;
>> +
>> +       if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
>> +               return true;
>> +
>> +       if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
>> +                       h->oem_revision == 1)
>> +               return true;
>> +
>> +       return false;
>> +}
> 
> Please give us a chance to test this patch before merging. We've had a
> lot of problems with our E44 work-around, and we need to make sure
> that qdf2400_erratum_44_present function is called before the pl011
> driver is probed.

Timor, do you know of a specific system that has this problem?  If so, I
could see if we have one @ Red Hat and test on it to verify the WAR.

> 
>> +
>> +/*
>> + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
>> + * register aligned to 32-bit. In addition, the BIOS also encoded the
>> + * access width to be 8 bits. This function detects this errata condition.
>> + */
>> +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
>> +{
>> +       bool xgene_8250 = false;
>> +
>> +       if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
>> +               return false;
>> +
>> +       if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
>> +           memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
>> +               return false;
>> +
>> +       if (!memcmp(tb->header.oem_table_id, "XGENESPC",
>> +           ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
>> +               xgene_8250 = true;
>> +
>> +       if (!memcmp(tb->header.oem_table_id, "ProLiant",
>> +           ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
>> +               xgene_8250 = true;
>> +
>> +       return xgene_8250;
>> +}
> 
> I suspect that this function has the same issues as
> qdf2400_erratum_44_present().
>

Ditto.

>> +config ACPI_SPCR_TABLE
>> +       bool "ACPI Serial Port Console Redirection Support"
>> +       default y if ARM64
>> +       help
>> +         Enable support for Serial Port Console Redirection (SPCR) Table.
>> +         This table provides information about the configuration of the
>> +         earlycon console.
>> +
> 
> So ACPI without SPCR has never been tested by us.  Making it optional
> makes me a little nervous.  We'll have to evaluate this change.
> 

It is 'y' for ARM64?  Maybe I have that wrong but I thought that would
always build it for ARM64.

P.

[PATCH 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Timur Tabi]

On 12/07/2017 01:05 PM, Prarit Bhargava wrote:
>> Please give us a chance to test this patch before merging. We've had a
>> lot of problems with our E44 work-around, and we need to make sure
>> that qdf2400_erratum_44_present function is called before the pl011
>> driver is probed.

> Timor, do you know of a specific system that has this problem?  If so, I
> could see if we have one @ Red Hat and test on it to verify the WAR.

Yes, Red Hat has our QDF2400 systems.  You need one with 1.x silicon. 
Contact anyone you know in the ARM server group.  Email me offline if 
you can't find anyone.

>>> + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
>>> + * register aligned to 32-bit. In addition, the BIOS also encoded the
>>> + * access width to be 8 bits. This function detects this errata condition.
>>> + */
>>> +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
>>> +{
>>> +       bool xgene_8250 = false;
>>> +
>>> +       if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
>>> +               return false;
>>> +
>>> +       if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
>>> +           memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
>>> +               return false;
>>> +
>>> +       if (!memcmp(tb->header.oem_table_id, "XGENESPC",
>>> +           ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
>>> +               xgene_8250 = true;
>>> +
>>> +       if (!memcmp(tb->header.oem_table_id, "ProLiant",
>>> +           ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
>>> +               xgene_8250 = true;
>>> +
>>> +       return xgene_8250;
>>> +}
>> I suspect that this function has the same issues as
>> qdf2400_erratum_44_present().
>>
> Ditto.

I don't know if Red Hat has any XGene systems.

> 
>>> +config ACPI_SPCR_TABLE
>>> +       bool "ACPI Serial Port Console Redirection Support"
>>> +       default y if ARM64
>>> +       help
>>> +         Enable support for Serial Port Console Redirection (SPCR) Table.
>>> +         This table provides information about the configuration of the
>>> +         earlycon console.
>>> +
>> So ACPI without SPCR has never been tested by us.  Making it optional
>> makes me a little nervous.  We'll have to evaluate this change.
>>
> It is 'y' for ARM64?  Maybe I have that wrong but I thought that would
> always build it for ARM64.

It was forced 't' for ARM64.  That is, if ACPI is enabled, SPCR is 
always enabled and always used.  With your patch, we would need to test 
our kernel with console=ttyAMA0,0xfeblabla,etc to make sure that it 
still works.

'default y' means that some can turn SPCR off, but keep ACPI still on.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Prarit Bhargava]

On 12/07/2017 01:43 PM, Timur Tabi wrote:
> On Thu, Dec 7, 2017 at 11:29 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>> Other architectures can use SPCR to setup an early console or console but
>> the current code is ARM64 specific.
>>
>> Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
>> function acpi_arch_setup_console() that can be used for arch-specific
>> setup.  Move SPCR initialization flag into ACPI code.  Update the
>> Documention on the use of the SPCR earlycon.
>>
>> This patch does not change arm64 behaviour.
> 
> Let's hope so.  Our E44 erratum work-around has caused lots of
> problems in the past, so the code is a bit fragile.

So it does look like I broke something :(  I'll fix this and do more testing.
One thing that I just learned (relative to x86) is that you have to test much
further and wider than on x86.

P.

> 
>> +static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
>> +{
>> +       if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
>> +               return false;
>> +
>> +       if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
>> +               return true;
>> +
>> +       if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
>> +                       h->oem_revision == 1)
>> +               return true;
>> +
>> +       return false;
>> +}
> 
> Please give us a chance to test this patch before merging. We've had a
> lot of problems with our E44 work-around, and we need to make sure
> that qdf2400_erratum_44_present function is called before the pl011
> driver is probed.
> 
>> +
>> +/*
>> + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
>> + * register aligned to 32-bit. In addition, the BIOS also encoded the
>> + * access width to be 8 bits. This function detects this errata condition.
>> + */
>> +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
>> +{
>> +       bool xgene_8250 = false;
>> +
>> +       if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
>> +               return false;
>> +
>> +       if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
>> +           memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
>> +               return false;
>> +
>> +       if (!memcmp(tb->header.oem_table_id, "XGENESPC",
>> +           ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
>> +               xgene_8250 = true;
>> +
>> +       if (!memcmp(tb->header.oem_table_id, "ProLiant",
>> +           ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
>> +               xgene_8250 = true;
>> +
>> +       return xgene_8250;
>> +}
> 
> I suspect that this function has the same issues as
> qdf2400_erratum_44_present().
> 
>> +config ACPI_SPCR_TABLE
>> +       bool "ACPI Serial Port Console Redirection Support"
>> +       default y if ARM64
>> +       help
>> +         Enable support for Serial Port Console Redirection (SPCR) Table.
>> +         This table provides information about the configuration of the
>> +         earlycon console.
>> +
> 
> So ACPI without SPCR has never been tested by us.  Making it optional
> makes me a little nervous.  We'll have to evaluate this change.
> 

[PATCH 2/2] acpi, x86: Use SPCR table for earlycon on x86

[Prarit Bhargava]

The ACPI SPCR code has been used to define an earlycon console for arm64
and can be used for x86.

Modify the ACPI SPCR parsing code to account for console behaviour
differences between arm64 and x86.  Initialize the SPCR code from
x86 ACPI initialization code.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>

Cc: linux-doc at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-pm at vger.kernel.org
Cc: linux-acpi at vger.kernel.org
Cc: linux-serial at vger.kernel.org
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86 at kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
 Documentation/admin-guide/kernel-parameters.txt | 3 +++
 arch/arm64/kernel/acpi.c                        | 2 +-
 arch/x86/kernel/acpi/boot.c                     | 3 +++
 drivers/acpi/Kconfig                            | 2 +-
 drivers/acpi/spcr.c                             | 5 +++--
 include/linux/acpi.h                            | 7 +++++--
 6 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 0d173289c67e..c7cc890a0e81 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -918,6 +918,9 @@
 			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
 			(xuartps) serial port at the specified address. Only
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index ef74e1f3e468..9d7842022d47 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -355,7 +355,7 @@ void __init acpi_boot_table_init(void)
 			early_init_dt_scan_chosen_stdout();
 	} else {
 		/* Always enable the ACPI SPCR console */
-		acpi_parse_spcr(console_acpi_spcr_enable);
+		acpi_parse_spcr(console_acpi_spcr_enable, true);
 		if (IS_ENABLED(CONFIG_ACPI_BGRT))
 			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
 	}
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index f4c463df8b08..4527ecbab4cb 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1626,6 +1626,9 @@ 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(console_acpi_spcr_enable, console_acpi_spcr_enable);
+
 	return 0;
 }
 
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index ae25159dd976..6bcddd149ca7 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -562,7 +562,7 @@ config TPS68470_PMIC_OPREGION
 
 config ACPI_SPCR_TABLE
 	bool "ACPI Serial Port Console Redirection Support"
-	default y if ARM64
+	default y if (X86 || ARM64)
 	help
 	  Enable support for Serial Port Console Redirection (SPCR) Table.
 	  This table provides information about the configuration of the
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index d8ad8fa67582..23abb1c207a0 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -40,7 +40,7 @@ bool console_acpi_spcr_enable __initdata;
  * from arch initialization code as soon as the DT/ACPI decision is made.
  *
  */
-int __init acpi_parse_spcr(bool earlycon)
+int __init acpi_parse_spcr(bool earlycon, bool enable_console)
 {
 	static char opts[ACPI_SPCR_OPTS_SIZE];
 	static char uart[ACPI_SPCR_BUF_SIZE];
@@ -113,7 +113,8 @@ int __init acpi_parse_spcr(bool earlycon)
 	if (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);
 done:
 	acpi_put_table((struct acpi_table_header *)table);
 	return err;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 818c1053cc6f..252d692f017f 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1248,10 +1248,13 @@ extern bool console_acpi_spcr_enable __initdata;
 extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
 				   char *opts, char *uart, char *iotype,
 				   int baud_rate, bool earlycon);
-int acpi_parse_spcr(bool earlycon);
+int acpi_parse_spcr(bool earlycon, bool enable_console);
 #else
 static const bool console_acpi_spcr_enable;
-static inline int acpi_parse_spcr(bool earlycon) { return 0; }
+static inline int acpi_parse_spcr(bool earlycon, bool enable_console)
+{
+	return 0;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)
-- 
2.15.0.rc0.39.g2f0e14e64

[PATCH 2/2] acpi, x86: Use SPCR table for earlycon on x86

[Timur Tabi]

On Thu, Dec 7, 2017 at 11:29 AM, Prarit Bhargava <prarit@redhat.com> wrote:
> -int __init acpi_parse_spcr(bool earlycon)
> +int __init acpi_parse_spcr(bool earlycon, bool enable_console)
>  {
>         static char opts[ACPI_SPCR_OPTS_SIZE];
>         static char uart[ACPI_SPCR_BUF_SIZE];
> @@ -113,7 +113,8 @@ int __init acpi_parse_spcr(bool earlycon)
>         if (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);

So if earlycon==true but enable_console==false, then you parse SPCR
and use it for the early console, but not the primary console?  I'm
not sure what that gives you.

[PATCH 2/2] acpi, x86: Use SPCR table for earlycon on x86

[Prarit Bhargava]

On 12/07/2017 01:46 PM, Timur Tabi wrote:
> On Thu, Dec 7, 2017 at 11:29 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>> -int __init acpi_parse_spcr(bool earlycon)
>> +int __init acpi_parse_spcr(bool earlycon, bool enable_console)
>>  {
>>         static char opts[ACPI_SPCR_OPTS_SIZE];
>>         static char uart[ACPI_SPCR_BUF_SIZE];
>> @@ -113,7 +113,8 @@ int __init acpi_parse_spcr(bool earlycon)
>>         if (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);
> 
> So if earlycon==true but enable_console==false, then you parse SPCR
> and use it for the early console, but not the primary console?  I'm
> not sure what that gives you.
> 

On arm64 the original SPCR code provides both early and primary consoles (as
you've pointed out).  IOW, if SPRC exists on arm64 a console will be
initialized.  That is still the case after my change.

The expected behaviour on x86 is that unless someone specifies 'earlycon' or
'console=' that no console should be initialized.  There are users (some IOT
boxes, etc.) that do NOT want a console.

The above code keeps the existing x86 behaviour.

P.

[PATCH 0/2] acpi, x86: Add SPCR table support

[Ingo Molnar]

* Prarit Bhargava <prarit@redhat.com> wrote:

> The SPCR (Serial Port Console Redirection) Table provides information
> about the configuration of serial port.  This information can be used
> to configure the early console.

s/about the configuration of serial port
 /about the configuration of the serial port

> SPCR support was added for arm64 and is made available across all arches
> in this patchset.  The first patch adds a weak per-arch configuration function
> and moves the SPCR code into ACPI.  The second patch adds support to x86.
> 
> The existing behaviour on arm64 is maintained.  If the SPCR exists the
> earlycon and console are automatically configured.

s/arm64
 /ARM64

which is easier to read and it's also the prevalent spelling:

 triton:~/tip> for N in $(git grep -ih arm64 arch/arm64/ | sed 's/[[:punct:]]/ /g'); do echo $N | grep -iw arm64; done | sort | uniq -c
     412 arm64
       1 Arm64
     854 ARM64

> The existing default behaviour on x86 is also maintained.  If no console or
> earlycon parameter is defined and the SPCR exists , the serial port is not
> configured.  If the earlycon parameter is used both the early console
> and the console are configured using the data from the SPCR.

s/exists , the
 /exists, the

But, the logic to not use the SPCR looks confusing to me.

The SPCR is only present if the user has explicitly configured a serial console 
for that machine, either in the firmware, or remotely via IPMI, correct? I.e. SPCR 
will not be spuriously present by default on systems that have a serial console 
but the user never expressed any interest for them, right?

If so then we should pick up that serial console configuration and activate it, 
regardless of any kernel boot options!

Thanks,

	Ingo

[PATCH 0/2] acpi, x86: Add SPCR table support

[Prarit Bhargava]

On 12/08/2017 01:29 AM, Ingo Molnar wrote:
> 
> * Prarit Bhargava <prarit@redhat.com> wrote:
> 
>> The SPCR (Serial Port Console Redirection) Table provides information
>> about the configuration of serial port.  This information can be used
>> to configure the early console.
> 
> s/about the configuration of serial port
>  /about the configuration of the serial port
> 
>> SPCR support was added for arm64 and is made available across all arches
>> in this patchset.  The first patch adds a weak per-arch configuration function
>> and moves the SPCR code into ACPI.  The second patch adds support to x86.
>>
>> The existing behaviour on arm64 is maintained.  If the SPCR exists the
>> earlycon and console are automatically configured.
> 
> s/arm64
>  /ARM64
> 
> which is easier to read and it's also the prevalent spelling:
> 
>  triton:~/tip> for N in $(git grep -ih arm64 arch/arm64/ | sed 's/[[:punct:]]/ /g'); do echo $N | grep -iw arm64; done | sort | uniq -c
>      412 arm64
>        1 Arm64
>      854 ARM64
> 
>> The existing default behaviour on x86 is also maintained.  If no console or
>> earlycon parameter is defined and the SPCR exists , the serial port is not
>> configured.  If the earlycon parameter is used both the early console
>> and the console are configured using the data from the SPCR.
> 
> s/exists , the
>  /exists, the
> 
> But, the logic to not use the SPCR looks confusing to me.
> 
> The SPCR is only present if the user has explicitly configured a serial console 
> for that machine, either in the firmware, or remotely via IPMI, correct? I.e. SPCR 
> will not be spuriously present by default on systems that have a serial console 
> but the user never expressed any interest for them, right?

If I disable "Serial Port Console Debug" in my BIOS I still see the SPCR configured:

[root at prarit-lab ~]# dmesg | grep SPCR
[    0.000000] ACPI: SPCR 0x0000000069031000 000050 (v01
00000000      00000000)

AFAICT the SPCR is always enabled on some systems.

> 
> If so then we should pick up that serial console configuration and activate it, 
> regardless of any kernel boot options!

I'm worried about someone who doesn't want a console on ttyS0 suddenly ending up
with one.  The SPCR could contain incorrect data, etc.

I originally wanted this on by default, but the chances of breaking someone's
setup seems significant doesn't it?  Maybe I'm being too cautious.  Anyone else
want to weigh in?  I'm not ignoring your idea Ingo, I'm just worried about being
yelled at by a user :) because I broke their default console setup.

P.

[PATCH 0/2] acpi, x86: Add SPCR table support

[Jeffrey Hugo]

On 12/8/2017 7:29 AM, Prarit Bhargava wrote:
> 
> 
> On 12/08/2017 01:29 AM, Ingo Molnar wrote:
>>
>> * Prarit Bhargava <prarit@redhat.com> wrote:
>>
>>> The SPCR (Serial Port Console Redirection) Table provides information
>>> about the configuration of serial port.  This information can be used
>>> to configure the early console.
>>
>> s/about the configuration of serial port
>>   /about the configuration of the serial port
>>
>>> SPCR support was added for arm64 and is made available across all arches
>>> in this patchset.  The first patch adds a weak per-arch configuration function
>>> and moves the SPCR code into ACPI.  The second patch adds support to x86.
>>>
>>> The existing behaviour on arm64 is maintained.  If the SPCR exists the
>>> earlycon and console are automatically configured.
>>
>> s/arm64
>>   /ARM64
>>
>> which is easier to read and it's also the prevalent spelling:
>>
>>   triton:~/tip> for N in $(git grep -ih arm64 arch/arm64/ | sed 's/[[:punct:]]/ /g'); do echo $N | grep -iw arm64; done | sort | uniq -c
>>       412 arm64
>>         1 Arm64
>>       854 ARM64
>>
>>> The existing default behaviour on x86 is also maintained.  If no console or
>>> earlycon parameter is defined and the SPCR exists , the serial port is not
>>> configured.  If the earlycon parameter is used both the early console
>>> and the console are configured using the data from the SPCR.
>>
>> s/exists , the
>>   /exists, the
>>
>> But, the logic to not use the SPCR looks confusing to me.
>>
>> The SPCR is only present if the user has explicitly configured a serial console
>> for that machine, either in the firmware, or remotely via IPMI, correct? I.e. SPCR
>> will not be spuriously present by default on systems that have a serial console
>> but the user never expressed any interest for them, right?
> 
> If I disable "Serial Port Console Debug" in my BIOS I still see the SPCR configured:
> 
> [root at prarit-lab ~]# dmesg | grep SPCR
> [    0.000000] ACPI: SPCR 0x0000000069031000 000050 (v01
> 00000000      00000000)
> 
> AFAICT the SPCR is always enabled on some systems.

"Serial Port Console Debug" sounds like DBG2 to me, although I am unsure 
of the specific platform you are referencing.

> 
>>
>> If so then we should pick up that serial console configuration and activate it,
>> regardless of any kernel boot options!
> 
> I'm worried about someone who doesn't want a console on ttyS0 suddenly ending up
> with one.  The SPCR could contain incorrect data, etc.
> 
> I originally wanted this on by default, but the chances of breaking someone's
> setup seems significant doesn't it?  Maybe I'm being too cautious.  Anyone else
> want to weigh in?  I'm not ignoring your idea Ingo, I'm just worried about being
> yelled at by a user :) because I broke their default console setup.
> 

SPCR is always present on QDF2400 (ARM64 platform).  Firmware will 
automatically update the SPCR table to point to the correct console (IE 
Serial-over-LAN or SOL if configured via IPMI).  Unless a user 
specifically chooses to override the SPCR via "console=", we expect that 
a console will be present based on the data in SPCR.

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 0/2] acpi, x86: Add SPCR table support

[Prarit Bhargava]

On 12/08/2017 10:31 AM, Jeffrey Hugo wrote:
> On 12/8/2017 7:29 AM, Prarit Bhargava wrote:
>>
>>
>> On 12/08/2017 01:29 AM, Ingo Molnar wrote:
>>>
>>> * Prarit Bhargava <prarit@redhat.com> wrote:
>>>
>>>> The SPCR (Serial Port Console Redirection) Table provides information
>>>> about the configuration of serial port.  This information can be used
>>>> to configure the early console.
>>>
>>> s/about the configuration of serial port
>>>   /about the configuration of the serial port
>>>
>>>> SPCR support was added for arm64 and is made available across all arches
>>>> in this patchset.  The first patch adds a weak per-arch configuration function
>>>> and moves the SPCR code into ACPI.  The second patch adds support to x86.
>>>>
>>>> The existing behaviour on arm64 is maintained.  If the SPCR exists the
>>>> earlycon and console are automatically configured.
>>>
>>> s/arm64
>>>   /ARM64
>>>
>>> which is easier to read and it's also the prevalent spelling:
>>>
>>>   triton:~/tip> for N in $(git grep -ih arm64 arch/arm64/ | sed
>>> 's/[[:punct:]]/ /g'); do echo $N | grep -iw arm64; done | sort | uniq -c
>>>       412 arm64
>>>         1 Arm64
>>>       854 ARM64
>>>
>>>> The existing default behaviour on x86 is also maintained.  If no console or
>>>> earlycon parameter is defined and the SPCR exists , the serial port is not
>>>> configured.  If the earlycon parameter is used both the early console
>>>> and the console are configured using the data from the SPCR.
>>>
>>> s/exists , the
>>>   /exists, the
>>>
>>> But, the logic to not use the SPCR looks confusing to me.
>>>
>>> The SPCR is only present if the user has explicitly configured a serial console
>>> for that machine, either in the firmware, or remotely via IPMI, correct? I.e.
>>> SPCR
>>> will not be spuriously present by default on systems that have a serial console
>>> but the user never expressed any interest for them, right?
>>
>> If I disable "Serial Port Console Debug" in my BIOS I still see the SPCR
>> configured:
>>
>> [root at prarit-lab ~]# dmesg | grep SPCR
>> [    0.000000] ACPI: SPCR 0x0000000069031000 000050 (v01
>> 00000000      00000000)
>>
>> AFAICT the SPCR is always enabled on some systems.
> 
> "Serial Port Console Debug" sounds like DBG2 to me, although I am unsure of the
> specific platform you are referencing.
> 
>>
>>>
>>> If so then we should pick up that serial console configuration and activate it,
>>> regardless of any kernel boot options!
>>
>> I'm worried about someone who doesn't want a console on ttyS0 suddenly ending up
>> with one.  The SPCR could contain incorrect data, etc.
>>
>> I originally wanted this on by default, but the chances of breaking someone's
>> setup seems significant doesn't it?  Maybe I'm being too cautious.  Anyone else
>> want to weigh in?  I'm not ignoring your idea Ingo, I'm just worried about being
>> yelled at by a user :) because I broke their default console setup.
>>
> 
> SPCR is always present on QDF2400 (ARM64 platform).  Firmware will automatically
> update the SPCR table to point to the correct console (IE Serial-over-LAN or SOL
> if configured via IPMI).  Unless a user specifically chooses to override the
> SPCR via "console=", we expect that a console will be present based on the data
> in SPCR.

Hey Jeffrey -- I think Ingo's & my conversation is x86-specific.  I am *NOT*
changing the behaviour on ARM64.

P.

> 

[PATCH 0/2] acpi, x86: Add SPCR table support

[Jon Masters]

On 12/08/2017 09:29 AM, Prarit Bhargava wrote:

> If I disable "Serial Port Console Debug" in my BIOS I still see the SPCR configured:
> 
> [root at prarit-lab ~]# dmesg | grep SPCR
> [    0.000000] ACPI: SPCR 0x0000000069031000 000050 (v01
> 00000000      00000000)
> 
> AFAICT the SPCR is always enabled on some systems.

It's part of some of the Windows design flows that it gets incorporated,
and it should always be present.

Jon (not talking about AArch64/ARM/arm/ARMv8-A/64-bit ARM/Go ARM Go).

[PATCH 0/2] acpi, x86: Add SPCR table support

[Ingo Molnar]

* Prarit Bhargava <prarit@redhat.com> wrote:

> If I disable "Serial Port Console Debug" in my BIOS I still see the SPCR configured:
> 
> [root at prarit-lab ~]# dmesg | grep SPCR
> [    0.000000] ACPI: SPCR 0x0000000069031000 000050 (v01
> 00000000      00000000)
> 
> AFAICT the SPCR is always enabled on some systems.

Fair enough.

> > If so then we should pick up that serial console configuration and activate it, 
> > regardless of any kernel boot options!
> 
> I'm worried about someone who doesn't want a console on ttyS0 suddenly ending up
> with one.  The SPCR could contain incorrect data, etc.

Yeah, that's not good.

Thanks,

	Ingo