[PATCH v2 0/7] x86/boot: Enable earlyprintk for HS UARTs

[PATCH v2 0/7] x86/boot: Enable earlyprintk for HS UARTs

[Andy Shevchenko]

Newer Intel platforms sometimes have only HS UART enabled and
accessible via MMIO on high addresses.

For now, earlyprintk doesn't handle such cases and always rely
on the legacy serial port to be present.

This series fills the gap and enables earlyprintk on HS UARTs.

Since v2:
- rebase on top of v5.1-rc1
- parse 'mmio' keyword instead of port range check (Ingo)
- deduplicate code wherever it's possible (Ingo)

Andy Shevchenko (7):
  x86/boot: Convert early_serial_base to unsigned long
  x86/boot: Introduce helpers for serial I/O
  x86/boot: Split out parse_serial_port() helper for earlyprintk
  x86/boot: Allow longer parameter list for earlyprintk
  x86/boot: Add MMIO byte accessors
  x86/boot: Introduce MMIO accessors and their support in earlyprintk
  x86/boot: Support nocfg parameter for earlyprintk

 arch/x86/boot/boot.h                          |  16 ++-
 .../boot/compressed/early_serial_console.c    |   5 +-
 arch/x86/boot/compressed/misc.c               |   4 +-
 arch/x86/boot/compressed/misc.h               |   8 +-
 arch/x86/boot/early_serial_console.c          | 135 ++++++++++++++----
 arch/x86/boot/tty.c                           |   9 +-
 6 files changed, 140 insertions(+), 37 deletions(-)

-- 
2.20.1

[PATCH v2 1/7] x86/boot: Convert early_serial_base to unsigned long

[Andy Shevchenko]

As a preparatory of adding flexible serial I/O accessors, convert
early_serial_base to unsigned long to cover all possible bus addresses
on the system.

No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/boot/boot.h                            | 2 +-
 arch/x86/boot/compressed/early_serial_console.c | 2 +-
 arch/x86/boot/compressed/misc.h                 | 4 ++--
 arch/x86/boot/early_serial_console.c            | 6 +++---
 arch/x86/boot/tty.c                             | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index 32a09eb5c101..f41291903da7 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -299,7 +299,7 @@ int check_knl_erratum(void);
 int validate_cpu(void);
 
 /* early_serial_console.c */
-extern int early_serial_base;
+extern unsigned long early_serial_base;
 void console_init(void);
 
 /* edd.c */
diff --git a/arch/x86/boot/compressed/early_serial_console.c b/arch/x86/boot/compressed/early_serial_console.c
index 261e81fb9582..5e3a66478754 100644
--- a/arch/x86/boot/compressed/early_serial_console.c
+++ b/arch/x86/boot/compressed/early_serial_console.c
@@ -1,5 +1,5 @@
 #include "misc.h"
 
-int early_serial_base;
+unsigned long early_serial_base;
 
 #include "../early_serial_console.c"
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index fd13655e0f9b..6f2bdf813949 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -110,10 +110,10 @@ static inline void finalize_identity_maps(void)
 
 #ifdef CONFIG_EARLY_PRINTK
 /* early_serial_console.c */
-extern int early_serial_base;
+extern unsigned long early_serial_base;
 void console_init(void);
 #else
-static const int early_serial_base;
+static const unsigned long early_serial_base;
 static inline void console_init(void)
 { }
 #endif
diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
index 023bf1c3de8b..2b663beda582 100644
--- a/arch/x86/boot/early_serial_console.c
+++ b/arch/x86/boot/early_serial_console.c
@@ -23,7 +23,7 @@
 
 #define DEFAULT_BAUD 9600
 
-static void early_serial_init(int port, int baud)
+static void early_serial_init(unsigned long port, int baud)
 {
 	unsigned char c;
 	unsigned divisor;
@@ -48,7 +48,7 @@ static void parse_earlyprintk(void)
 	int baud = DEFAULT_BAUD;
 	char arg[32];
 	int pos = 0;
-	int port = 0;
+	unsigned long port = 0;
 
 	if (cmdline_find_option("earlyprintk", arg, sizeof(arg)) > 0) {
 		char *e;
@@ -118,7 +118,7 @@ static void parse_console_uart8250(void)
 {
 	char optstr[64], *options;
 	int baud = DEFAULT_BAUD;
-	int port = 0;
+	unsigned long port = 0;
 
 	/*
 	 * console=uart8250,io,0x3f8,115200n8
diff --git a/arch/x86/boot/tty.c b/arch/x86/boot/tty.c
index def2451f46ae..d1d34c6ca153 100644
--- a/arch/x86/boot/tty.c
+++ b/arch/x86/boot/tty.c
@@ -15,7 +15,7 @@
 
 #include "boot.h"
 
-int early_serial_base;
+unsigned long early_serial_base;
 
 #define XMTRDY          0x20
 
-- 
2.20.1

[PATCH v2 2/7] x86/boot: Introduce helpers for serial I/O

[Andy Shevchenko]

As preparatory to enable earlyprintk on non-standard ports on x86,
introduce serial_in() and serial_out() helpers to perform serial I/O.

No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/boot/boot.h                          |  2 +
 .../boot/compressed/early_serial_console.c    |  3 +
 arch/x86/boot/compressed/misc.c               |  4 +-
 arch/x86/boot/compressed/misc.h               |  4 ++
 arch/x86/boot/early_serial_console.c          | 63 +++++++++++++------
 arch/x86/boot/tty.c                           |  7 ++-
 6 files changed, 61 insertions(+), 22 deletions(-)

diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index f41291903da7..86641b2e83f8 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -299,6 +299,8 @@ int check_knl_erratum(void);
 int validate_cpu(void);
 
 /* early_serial_console.c */
+extern unsigned int (*serial_in)(unsigned long addr, int offset);
+extern void (*serial_out)(unsigned long addr, int offset, int value);
 extern unsigned long early_serial_base;
 void console_init(void);
 
diff --git a/arch/x86/boot/compressed/early_serial_console.c b/arch/x86/boot/compressed/early_serial_console.c
index 5e3a66478754..4b6269624b59 100644
--- a/arch/x86/boot/compressed/early_serial_console.c
+++ b/arch/x86/boot/compressed/early_serial_console.c
@@ -1,5 +1,8 @@
 #include "misc.h"
 
+unsigned int (*serial_in)(unsigned long addr, int offset);
+void (*serial_out)(unsigned long addr, int offset, int value);
+
 unsigned long early_serial_base;
 
 #include "../early_serial_console.c"
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index c0d6c560df69..75b3c8bb92ac 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -98,10 +98,10 @@ static void serial_putchar(int ch)
 {
 	unsigned timeout = 0xffff;
 
-	while ((inb(early_serial_base + LSR) & XMTRDY) == 0 && --timeout)
+	while ((serial_in(early_serial_base, LSR) & XMTRDY) == 0 && --timeout)
 		cpu_relax();
 
-	outb(ch, early_serial_base + TXR);
+	serial_out(early_serial_base, TXR, ch);
 }
 
 void __putstr(const char *s)
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 6f2bdf813949..2fd3ec89a70d 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -110,9 +110,13 @@ static inline void finalize_identity_maps(void)
 
 #ifdef CONFIG_EARLY_PRINTK
 /* early_serial_console.c */
+extern unsigned int (*serial_in)(unsigned long addr, int offset);
+extern void (*serial_out)(unsigned long addr, int offset, int value);
 extern unsigned long early_serial_base;
 void console_init(void);
 #else
+static unsigned int (*serial_in)(unsigned long addr, int offset);
+static void (*serial_out)(unsigned long addr, int offset, int value);
 static const unsigned long early_serial_base;
 static inline void console_init(void)
 { }
diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
index 2b663beda582..05d02cd4de99 100644
--- a/arch/x86/boot/early_serial_console.c
+++ b/arch/x86/boot/early_serial_console.c
@@ -23,22 +23,45 @@
 
 #define DEFAULT_BAUD 9600
 
-static void early_serial_init(unsigned long port, int baud)
+static unsigned int io_serial_in(unsigned long addr, int offset)
+{
+	return inb(addr + offset);
+}
+
+static void io_serial_out(unsigned long addr, int offset, int value)
+{
+	outb(value, addr + offset);
+}
+
+static void early_serial_configure(unsigned long port, int baud)
 {
 	unsigned char c;
 	unsigned divisor;
 
-	outb(0x3, port + LCR);	/* 8n1 */
-	outb(0, port + IER);	/* no interrupt */
-	outb(0, port + FCR);	/* no fifo */
-	outb(0x3, port + MCR);	/* DTR + RTS */
+	serial_out(port, LCR, 0x3);	/* 8n1 */
+	serial_out(port, IER, 0);	/* no interrupt */
+	serial_out(port, FCR, 0);	/* no fifo */
+	serial_out(port, MCR, 0x3);	/* DTR + RTS */
 
 	divisor	= 115200 / baud;
-	c = inb(port + LCR);
-	outb(c | DLAB, port + LCR);
-	outb(divisor & 0xff, port + DLL);
-	outb((divisor >> 8) & 0xff, port + DLH);
-	outb(c & ~DLAB, port + LCR);
+	c = serial_in(port, LCR);
+	serial_out(port, LCR, c | DLAB);
+	serial_out(port, DLL, divisor & 0xff);
+	serial_out(port, DLH, (divisor >> 8) & 0xff);
+	serial_out(port, LCR, c & ~DLAB);
+}
+
+/* Assign serial I/O accessors */
+static void early_serial_use_io_accessors(void)
+{
+	/* These will always be IO based ports */
+	serial_in = io_serial_in;
+	serial_out = io_serial_out;
+}
+
+static void early_serial_init(unsigned long port, int baud)
+{
+	early_serial_configure(port, baud);
 
 	early_serial_base = port;
 }
@@ -73,6 +96,7 @@ static void parse_earlyprintk(void)
 				port = DEFAULT_SERIAL_PORT;
 			else
 				pos = e - arg;
+			early_serial_use_io_accessors();
 		} else if (!strncmp(arg + pos, "ttyS", 4)) {
 			static const int bases[] = { 0x3f8, 0x2f8 };
 			int idx = 0;
@@ -84,6 +108,7 @@ static void parse_earlyprintk(void)
 				idx = 1;
 
 			port = bases[idx];
+			early_serial_use_io_accessors();
 		}
 
 		if (arg[pos] == ',')
@@ -104,11 +129,11 @@ static unsigned int probe_baud(int port)
 	unsigned char lcr, dll, dlh;
 	unsigned int quot;
 
-	lcr = inb(port + LCR);
-	outb(lcr | DLAB, port + LCR);
-	dll = inb(port + DLL);
-	dlh = inb(port + DLH);
-	outb(lcr, port + LCR);
+	lcr = serial_in(port, LCR);
+	serial_out(port, LCR, lcr | DLAB);
+	dll = serial_in(port, DLL);
+	dlh = serial_in(port, DLH);
+	serial_out(port, LCR, lcr);
 	quot = (dlh << 8) | dll;
 
 	return BASE_BAUD / quot;
@@ -129,11 +154,13 @@ static void parse_console_uart8250(void)
 
 	options = optstr;
 
-	if (!strncmp(options, "uart8250,io,", 12))
+	if (!strncmp(options, "uart8250,io,", 12)) {
 		port = simple_strtoull(options + 12, &options, 0);
-	else if (!strncmp(options, "uart,io,", 8))
+		early_serial_use_io_accessors();
+	} else if (!strncmp(options, "uart,io,", 8)) {
 		port = simple_strtoull(options + 8, &options, 0);
-	else
+		early_serial_use_io_accessors();
+	} else
 		return;
 
 	if (options && (options[0] == ','))
diff --git a/arch/x86/boot/tty.c b/arch/x86/boot/tty.c
index d1d34c6ca153..b6aa9db65cda 100644
--- a/arch/x86/boot/tty.c
+++ b/arch/x86/boot/tty.c
@@ -15,6 +15,9 @@
 
 #include "boot.h"
 
+unsigned int (*serial_in)(unsigned long addr, int offset);
+void (*serial_out)(unsigned long addr, int offset, int value);
+
 unsigned long early_serial_base;
 
 #define XMTRDY          0x20
@@ -31,10 +34,10 @@ static void __attribute__((section(".inittext"))) serial_putchar(int ch)
 {
 	unsigned timeout = 0xffff;
 
-	while ((inb(early_serial_base + LSR) & XMTRDY) == 0 && --timeout)
+	while ((serial_in(early_serial_base, LSR) & XMTRDY) == 0 && --timeout)
 		cpu_relax();
 
-	outb(ch, early_serial_base + TXR);
+	serial_out(early_serial_base, TXR, ch);
 }
 
 static void __attribute__((section(".inittext"))) bios_putchar(int ch)
-- 
2.20.1

[PATCH v2 3/7] x86/boot: Split out parse_serial_port() helper for earlyprintk

[Andy Shevchenko]

The newly introduced helper will be used later on to parse serial port
in different type of earlyprintk command line arguments.

No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/boot/early_serial_console.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
index 05d02cd4de99..85a227a21f95 100644
--- a/arch/x86/boot/early_serial_console.c
+++ b/arch/x86/boot/early_serial_console.c
@@ -66,6 +66,20 @@ static void early_serial_init(unsigned long port, int baud)
 	early_serial_base = port;
 }
 
+static unsigned long parse_serial_port(const char *arg, int off, int *pos)
+{
+	unsigned long port;
+	char *e;
+
+	port = simple_strtoull(arg + off, &e, 16);
+	if (port == 0 || arg + off == e)
+		port = DEFAULT_SERIAL_PORT;
+	else
+		*pos = e - arg;
+
+	return port;
+}
+
 static void parse_earlyprintk(void)
 {
 	int baud = DEFAULT_BAUD;
@@ -91,11 +105,7 @@ static void parse_earlyprintk(void)
 		 *	"ttyS0,115200"
 		 */
 		if (pos == 7 && !strncmp(arg + pos, "0x", 2)) {
-			port = simple_strtoull(arg + pos, &e, 16);
-			if (port == 0 || arg + pos == e)
-				port = DEFAULT_SERIAL_PORT;
-			else
-				pos = e - arg;
+			port = parse_serial_port(arg, pos + 0, &pos);
 			early_serial_use_io_accessors();
 		} else if (!strncmp(arg + pos, "ttyS", 4)) {
 			static const int bases[] = { 0x3f8, 0x2f8 };
-- 
2.20.1

[PATCH v2 4/7] x86/boot: Allow longer parameter list for earlyprintk

[Andy Shevchenko]

Allow longer parameter list, up to 64 characters, for earlyprintk
to support new coming parameters.

If the parsed string is longer than this buffer it will be cut up by
the buffer size.

No functional change intended.

Note, that 'console' case is already covered.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/boot/early_serial_console.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
index 85a227a21f95..98aee6c6e374 100644
--- a/arch/x86/boot/early_serial_console.c
+++ b/arch/x86/boot/early_serial_console.c
@@ -83,7 +83,7 @@ static unsigned long parse_serial_port(const char *arg, int off, int *pos)
 static void parse_earlyprintk(void)
 {
 	int baud = DEFAULT_BAUD;
-	char arg[32];
+	char arg[64];
 	int pos = 0;
 	unsigned long port = 0;
 
-- 
2.20.1

[PATCH v2 5/7] x86/boot: Add MMIO byte accessors

[Andy Shevchenko]

readb() and writeb() would help to access serial device
via MMIO address space.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/boot/boot.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index 86641b2e83f8..b2a272d3f36b 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -79,6 +79,18 @@ static inline void io_delay(void)
 	asm volatile("outb %%al,%0" : : "dN" (DELAY_PORT));
 }
 
+static inline u8 readb(const volatile void __iomem *addr)
+{
+	u8 v;
+	asm volatile("movb %1,%0" : "=q" (v) : "m" (*(volatile u8 __force *)addr));
+	return v;
+}
+
+static inline void writeb(u8 v, volatile void __iomem *addr)
+{
+	asm volatile("movb %0,%1" : : "q" (v), "m" (*(volatile u8 __force *)addr));
+}
+
 /* These functions are used to reference data in other segments. */
 
 static inline u16 ds(void)
-- 
2.20.1

[PATCH v2 6/7] x86/boot: Introduce MMIO accessors and their support in earlyprintk

[Andy Shevchenko]

If user supplied serial base address via kernel command line and
at the same time the word 'mmio' is present, use MMIO accessors.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/boot/early_serial_console.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
index 98aee6c6e374..cfc9e55cd68a 100644
--- a/arch/x86/boot/early_serial_console.c
+++ b/arch/x86/boot/early_serial_console.c
@@ -33,6 +33,20 @@ static void io_serial_out(unsigned long addr, int offset, int value)
 	outb(value, addr + offset);
 }
 
+static void mem8_serial_out(unsigned long addr, int offset, int value)
+{
+	u8 __iomem *vaddr = (u8 __iomem *)addr;
+	/* shift implied by pointer type */
+	writeb(value, vaddr + offset);
+}
+
+static unsigned int mem8_serial_in(unsigned long addr, int offset)
+{
+	u8 __iomem *vaddr = (u8 __iomem *)addr;
+	/* shift implied by pointer type */
+	return readb(vaddr + offset);
+}
+
 static void early_serial_configure(unsigned long port, int baud)
 {
 	unsigned char c;
@@ -59,6 +73,13 @@ static void early_serial_use_io_accessors(void)
 	serial_out = io_serial_out;
 }
 
+static void early_serial_use_mmio_accessors(void)
+{
+	/* It is memory mapped - assume 8-bit alignment */
+	serial_in = mem8_serial_in;
+	serial_out = mem8_serial_out;
+}
+
 static void early_serial_init(unsigned long port, int baud)
 {
 	early_serial_configure(port, baud);
@@ -101,12 +122,16 @@ static void parse_earlyprintk(void)
 		/*
 		 * make sure we have
 		 *	"serial,0x3f8,115200"
+		 *	"serial,mmio,0xff010180,115200"
 		 *	"serial,ttyS0,115200"
 		 *	"ttyS0,115200"
 		 */
 		if (pos == 7 && !strncmp(arg + pos, "0x", 2)) {
 			port = parse_serial_port(arg, pos + 0, &pos);
 			early_serial_use_io_accessors();
+		} else if (pos == 7 && !strncmp(arg + pos, "mmio,0x", 7)) {
+			port = parse_serial_port(arg, pos + 5, &pos);
+			early_serial_use_mmio_accessors();
 		} else if (!strncmp(arg + pos, "ttyS", 4)) {
 			static const int bases[] = { 0x3f8, 0x2f8 };
 			int idx = 0;
-- 
2.20.1

[PATCH v2 7/7] x86/boot: Support nocfg parameter for earlyprintk

[Andy Shevchenko]

If by BIOS or by other means the serial port is configured
user might want to skip reconfiguration in the boot code.

Add support of 'nocfg' parameter to earlyprintk.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/boot/early_serial_console.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
index cfc9e55cd68a..5b4a81bf5d0e 100644
--- a/arch/x86/boot/early_serial_console.c
+++ b/arch/x86/boot/early_serial_console.c
@@ -80,9 +80,10 @@ static void early_serial_use_mmio_accessors(void)
 	serial_out = mem8_serial_out;
 }
 
-static void early_serial_init(unsigned long port, int baud)
+static void early_serial_init(unsigned long port, int baud, bool configure)
 {
-	early_serial_configure(port, baud);
+	if (configure)
+		early_serial_configure(port, baud);
 
 	early_serial_base = port;
 }
@@ -101,12 +102,22 @@ static unsigned long parse_serial_port(const char *arg, int off, int *pos)
 	return port;
 }
 
+static bool parse_serial_configure(const char *arg, int off, int *pos)
+{
+	if (strncmp(arg + off, "nocfg", 5))
+		return true;
+
+	*pos = off + 5;
+	return false;
+}
+
 static void parse_earlyprintk(void)
 {
 	int baud = DEFAULT_BAUD;
 	char arg[64];
 	int pos = 0;
 	unsigned long port = 0;
+	bool configure = true;
 
 	if (cmdline_find_option("earlyprintk", arg, sizeof(arg)) > 0) {
 		char *e;
@@ -146,6 +157,11 @@ static void parse_earlyprintk(void)
 			early_serial_use_io_accessors();
 		}
 
+		if (arg[pos] == ',')
+			pos++;
+
+		configure = parse_serial_configure(arg, pos, &pos);
+
 		if (arg[pos] == ',')
 			pos++;
 
@@ -155,7 +171,7 @@ static void parse_earlyprintk(void)
 	}
 
 	if (port)
-		early_serial_init(port, baud);
+		early_serial_init(port, baud, configure);
 }
 
 #define BASE_BAUD (1843200/16)
@@ -179,6 +195,7 @@ static void parse_console_uart8250(void)
 	char optstr[64], *options;
 	int baud = DEFAULT_BAUD;
 	unsigned long port = 0;
+	bool configure = true;
 
 	/*
 	 * console=uart8250,io,0x3f8,115200n8
@@ -204,7 +221,7 @@ static void parse_console_uart8250(void)
 		baud = probe_baud(port);
 
 	if (port)
-		early_serial_init(port, baud);
+		early_serial_init(port, baud, configure);
 }
 
 void console_init(void)
-- 
2.20.1

Re: [PATCH v2 7/7] x86/boot: Support nocfg parameter for earlyprintk

[Randy Dunlap]

On 3/19/19 11:43 AM, Andy Shevchenko wrote:
> If by BIOS or by other means the serial port is configured
> user might want to skip reconfiguration in the boot code.
> 
> Add support of 'nocfg' parameter to earlyprintk.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---

Hi Andy,
Please add "nocfg" to Documentation/admin-guide/kernel-parameters.txt.


-- 
~Randy

Re: [PATCH v2 7/7] x86/boot: Support nocfg parameter for earlyprintk

[Andy Shevchenko]

On Wed, Mar 20, 2019 at 08:05:51AM -0700, Randy Dunlap wrote:
> On 3/19/19 11:43 AM, Andy Shevchenko wrote:
> > If by BIOS or by other means the serial port is configured
> > user might want to skip reconfiguration in the boot code.
> > 
> > Add support of 'nocfg' parameter to earlyprintk.

> Please add "nocfg" to Documentation/admin-guide/kernel-parameters.txt.

I think it's not a time right now to do so. This only does it for boot stage.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 7/7] x86/boot: Support nocfg parameter for earlyprintk

[Randy Dunlap]

On 3/20/19 8:17 AM, Andy Shevchenko wrote:
> On Wed, Mar 20, 2019 at 08:05:51AM -0700, Randy Dunlap wrote:
>> On 3/19/19 11:43 AM, Andy Shevchenko wrote:
>>> If by BIOS or by other means the serial port is configured
>>> user might want to skip reconfiguration in the boot code.
>>>
>>> Add support of 'nocfg' parameter to earlyprintk.
> 
>> Please add "nocfg" to Documentation/admin-guide/kernel-parameters.txt.
> 
> I think it's not a time right now to do so. This only does it for boot stage.
> 

OK.   Is there somewhere else that it could be added to Documentation/[x86/]?

thanks.
-- 
~Randy

Re: [PATCH v2 7/7] x86/boot: Support nocfg parameter for earlyprintk

[Andy Shevchenko]

On Wed, Mar 20, 2019 at 08:19:46AM -0700, Randy Dunlap wrote:
> On 3/20/19 8:17 AM, Andy Shevchenko wrote:
> > On Wed, Mar 20, 2019 at 08:05:51AM -0700, Randy Dunlap wrote:
> >> On 3/19/19 11:43 AM, Andy Shevchenko wrote:

> >>> Add support of 'nocfg' parameter to earlyprintk.
> > 
> >> Please add "nocfg" to Documentation/admin-guide/kernel-parameters.txt.
> > 
> > I think it's not a time right now to do so. This only does it for boot stage.

> OK.   Is there somewhere else that it could be added to Documentation/[x86/]?

I don't see the proper place.

I think this is can be added when it would be recognized by next stage of
earlyprintk.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 7/7] x86/boot: Support nocfg parameter for earlyprintk

[Borislav Petkov]

On Wed, Mar 20, 2019 at 06:15:43PM +0200, Andy Shevchenko wrote:
> I don't see the proper place.

I see Documentation/x86/earlyprintk.txt which kinda begs to collect
earlyprintk documentation...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH v2 7/7] x86/boot: Support nocfg parameter for earlyprintk

[Andy Shevchenko]

On Wed, Mar 20, 2019 at 6:32 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Mar 20, 2019 at 06:15:43PM +0200, Andy Shevchenko wrote:
> > I don't see the proper place.
>
> I see Documentation/x86/earlyprintk.txt which kinda begs to collect
> earlyprintk documentation...

It says only about USB, but okay, I may shift it as a chapter.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/7] x86/boot: Convert early_serial_base to unsigned long

[Borislav Petkov]

On Tue, Mar 19, 2019 at 09:43:19PM +0300, Andy Shevchenko wrote:
> As a preparatory of adding flexible serial I/O accessors, convert
> early_serial_base to unsigned long to cover all possible bus addresses
> on the system.
> 
> No functional change intended.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/boot/boot.h                            | 2 +-
>  arch/x86/boot/compressed/early_serial_console.c | 2 +-
>  arch/x86/boot/compressed/misc.h                 | 4 ++--
>  arch/x86/boot/early_serial_console.c            | 6 +++---
>  arch/x86/boot/tty.c                             | 2 +-
>  5 files changed, 8 insertions(+), 8 deletions(-)

...

> diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
> index 023bf1c3de8b..2b663beda582 100644
> --- a/arch/x86/boot/early_serial_console.c
> +++ b/arch/x86/boot/early_serial_console.c
> @@ -23,7 +23,7 @@
>  
>  #define DEFAULT_BAUD 9600
>  
> -static void early_serial_init(int port, int baud)
> +static void early_serial_init(unsigned long port, int baud)
>  {
>  	unsigned char c;
>  	unsigned divisor;
> @@ -48,7 +48,7 @@ static void parse_earlyprintk(void)
>  	int baud = DEFAULT_BAUD;
>  	char arg[32];
>  	int pos = 0;
> -	int port = 0;
> +	unsigned long port = 0;
>  
>  	if (cmdline_find_option("earlyprintk", arg, sizeof(arg)) > 0) {
>  		char *e;
> @@ -118,7 +118,7 @@ static void parse_console_uart8250(void)
>  {
>  	char optstr[64], *options;
>  	int baud = DEFAULT_BAUD;
> -	int port = 0;
> +	unsigned long port = 0;

Here and above:

Please sort function local variables declaration in a reverse christmas
tree order:

	<type A> longest_variable_name;
	<type B> shorter_var_name;
	<type C> even_shorter;
	<type D> i;

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 1/7] x86/boot: Convert early_serial_base to unsigned long

[Andy Shevchenko]

On Thu, Mar 28, 2019 at 09:28:23AM +0100, Borislav Petkov wrote:
> On Tue, Mar 19, 2019 at 09:43:19PM +0300, Andy Shevchenko wrote:
> > As a preparatory of adding flexible serial I/O accessors, convert
> > early_serial_base to unsigned long to cover all possible bus addresses
> > on the system.

> > @@ -48,7 +48,7 @@ static void parse_earlyprintk(void)
> >  	int baud = DEFAULT_BAUD;
> >  	char arg[32];
> >  	int pos = 0;
> > -	int port = 0;
> > +	unsigned long port = 0;
> >  
> >  	if (cmdline_find_option("earlyprintk", arg, sizeof(arg)) > 0) {
> >  		char *e;
> > @@ -118,7 +118,7 @@ static void parse_console_uart8250(void)
> >  {
> >  	char optstr[64], *options;
> >  	int baud = DEFAULT_BAUD;
> > -	int port = 0;
> > +	unsigned long port = 0;
> 
> Here and above:

Agree on above and will fix, but don't see how this different in the latter?

> 
> Please sort function local variables declaration in a reverse christmas
> tree order:
> 
> 	<type A> longest_variable_name;
> 	<type B> shorter_var_name;
> 	<type C> even_shorter;
> 	<type D> i;
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/7] x86/boot: Convert early_serial_base to unsigned long

[Borislav Petkov]

On Thu, Mar 28, 2019 at 11:09:38AM +0200, Andy Shevchenko wrote:
> Agree on above and will fix, but don't see how this different in the latter?

Ok, so only the above.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 2/7] x86/boot: Introduce helpers for serial I/O

[Borislav Petkov]

On Tue, Mar 19, 2019 at 09:43:20PM +0300, Andy Shevchenko wrote:
> As preparatory to enable earlyprintk on non-standard ports on x86,
> introduce serial_in() and serial_out() helpers to perform serial I/O.
> 
> No functional change intended.

...

> +/* Assign serial I/O accessors */
> +static void early_serial_use_io_accessors(void)
> +{
> +	/* These will always be IO based ports */
> +	serial_in = io_serial_in;
> +	serial_out = io_serial_out;
> +}

This and the early_serial_use_mmio_accessors() in a later patch is just
silly because you need to export and forward-declare those serial_in and
serial_out function pointers in a bunch of places.

Just define exactly *two* simple functions serial_in() and serial_out()
which have enough logic to decide whether to do serial IO or MMIO and
put all that logic in those functions. No need for all that function
pointer assignment "fun".

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 3/7] x86/boot: Split out parse_serial_port() helper for earlyprintk

[Borislav Petkov]

On Tue, Mar 19, 2019 at 09:43:21PM +0300, Andy Shevchenko wrote:
> The newly introduced helper will be used later on to parse serial port
> in different type of earlyprintk command line arguments.
> 
> No functional change intended.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/boot/early_serial_console.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
> index 05d02cd4de99..85a227a21f95 100644
> --- a/arch/x86/boot/early_serial_console.c
> +++ b/arch/x86/boot/early_serial_console.c
> @@ -66,6 +66,20 @@ static void early_serial_init(unsigned long port, int baud)
>  	early_serial_base = port;
>  }
>  
> +static unsigned long parse_serial_port(const char *arg, int off, int *pos)
> +{
> +	unsigned long port;
> +	char *e;
> +
> +	port = simple_strtoull(arg + off, &e, 16);

WARNING: simple_strtoull is obsolete, use kstrtoull instead
#41: FILE: arch/x86/boot/early_serial_console.c:74:
+       port = simple_strtoull(arg + off, &e, 16);

Please do a conversion patch ontop.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 7/7] x86/boot: Support nocfg parameter for earlyprintk

[Borislav Petkov]

On Tue, Mar 19, 2019 at 09:43:25PM +0300, Andy Shevchenko wrote:
> If by BIOS or by other means the serial port is configured
> user might want to skip reconfiguration in the boot code.

That needs more explanation: how can that happen? How is the user
supposed to know whether the serial port is configured in order to use
this cmdline param?

Why can't the kernel detect whether the serial port is configured and
not touch that configuration?

A lot of why's and head scratching...

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 2/7] x86/boot: Introduce helpers for serial I/O

[Andy Shevchenko]

On Thu, Mar 28, 2019 at 01:32:50PM +0100, Borislav Petkov wrote:
> On Tue, Mar 19, 2019 at 09:43:20PM +0300, Andy Shevchenko wrote:
> > As preparatory to enable earlyprintk on non-standard ports on x86,
> > introduce serial_in() and serial_out() helpers to perform serial I/O.
> > 
> > No functional change intended.
> 
> ...
> 
> > +/* Assign serial I/O accessors */
> > +static void early_serial_use_io_accessors(void)
> > +{
> > +	/* These will always be IO based ports */
> > +	serial_in = io_serial_in;
> > +	serial_out = io_serial_out;
> > +}
> 
> This and the early_serial_use_mmio_accessors() in a later patch is just
> silly because you need to export and forward-declare those serial_in and
> serial_out function pointers in a bunch of places.

This is the same approach as it's done in earlyprintk support in the main part
of the kernel. Besides the fact that many drivers do it in the same way.

And I wouldn't call 'few' a 'bunch'...

> Just define exactly *two* simple functions serial_in() and serial_out()
> which have enough logic to decide whether to do serial IO or MMIO and
> put all that logic in those functions. 

Any idea how it can be done?

> No need for all that function
> pointer assignment "fun".

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/7] x86/boot: Split out parse_serial_port() helper for earlyprintk

[Andy Shevchenko]

On Thu, Mar 28, 2019 at 01:35:04PM +0100, Borislav Petkov wrote:
> On Tue, Mar 19, 2019 at 09:43:21PM +0300, Andy Shevchenko wrote:
> > The newly introduced helper will be used later on to parse serial port
> > in different type of earlyprintk command line arguments.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  arch/x86/boot/early_serial_console.c | 20 +++++++++++++++-----
> >  1 file changed, 15 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
> > index 05d02cd4de99..85a227a21f95 100644
> > --- a/arch/x86/boot/early_serial_console.c
> > +++ b/arch/x86/boot/early_serial_console.c
> > @@ -66,6 +66,20 @@ static void early_serial_init(unsigned long port, int baud)
> >  	early_serial_base = port;
> >  }
> >  
> > +static unsigned long parse_serial_port(const char *arg, int off, int *pos)
> > +{
> > +	unsigned long port;
> > +	char *e;
> > +
> > +	port = simple_strtoull(arg + off, &e, 16);
> 
> WARNING: simple_strtoull is obsolete, use kstrtoull instead
> #41: FILE: arch/x86/boot/early_serial_console.c:74:
> +       port = simple_strtoull(arg + off, &e, 16);

kstrtox() simple can't cover _all_ simple_strtox() cases.

> Please do a conversion patch ontop.

I don't feel like uglifying code to satisfy a tool producing a silly warning.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 7/7] x86/boot: Support nocfg parameter for earlyprintk

[Andy Shevchenko]

On Thu, Mar 28, 2019 at 02:03:17PM +0100, Borislav Petkov wrote:
> On Tue, Mar 19, 2019 at 09:43:25PM +0300, Andy Shevchenko wrote:
> > If by BIOS or by other means the serial port is configured
> > user might want to skip reconfiguration in the boot code.
> 
> That needs more explanation: how can that happen? 

I didn't get what is 'that' here? You mean that serial interface becomes
configured before kernel even starts? See below.

> How is the user
> supposed to know whether the serial port is configured in order to use
> this cmdline param?

If the connection to the target is done via serial interface and
firmware and / or bootloader already did necessary configuration.

> Why can't the kernel detect whether the serial port is configured and
> not touch that configuration?

Because it can't at this stage. We simple can't afford to move all serial
drivers (even partially) to the _boot_ part of the kernel.

> A lot of why's and head scratching...

What other why's should be answered?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/7] x86/boot: Introduce helpers for serial I/O

[Borislav Petkov]

On Thu, Mar 28, 2019 at 03:11:40PM +0200, Andy Shevchenko wrote:
> Any idea how it can be done?

Like this?

static u8 serial_inb(unsigned long addr, int offset)
{
	if (early_serial_do_mmio) {
		asm volatile("movb %1,%0" : "=q" (v) : "m" (*(volatile u8 __force *)addr));

		return v;
	} else {
		 return inb(addr + offset);
	}
}

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 3/7] x86/boot: Split out parse_serial_port() helper for earlyprintk

[Borislav Petkov]

On Thu, Mar 28, 2019 at 03:15:57PM +0200, Andy Shevchenko wrote:
> kstrtox() simple can't cover _all_ simple_strtox() cases.

I see only one case here. Are you saying kstrtoull() cannot work
here?

> I don't feel like uglifying code to satisfy a tool producing a silly
warning.

Uglifying how?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 7/7] x86/boot: Support nocfg parameter for earlyprintk

[Borislav Petkov]

On Thu, Mar 28, 2019 at 03:21:53PM +0200, Andy Shevchenko wrote:
> If the connection to the target is done via serial interface and
> firmware and / or bootloader already did necessary configuration.

Again: how is the user supposed to know that? When a user sees "nocfg"'s
documentation, how is the user supposed to decide whether she needs
to supply nocfg or not?

Which exact setups will need to supply nocfg on the cmdline?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 3/7] x86/boot: Split out parse_serial_port() helper for earlyprintk

[Andy Shevchenko]

On Thu, Mar 28, 2019 at 02:39:07PM +0100, Borislav Petkov wrote:
> On Thu, Mar 28, 2019 at 03:15:57PM +0200, Andy Shevchenko wrote:
> > kstrtox() simple can't cover _all_ simple_strtox() cases.
> 
> I see only one case here. Are you saying kstrtoull() cannot work
> here?

Can, but with code uglifying to workaround kstrtox() restrictions.
simple_strtox() _just works_ as expected.

> > I don't feel like uglifying code to satisfy a tool producing a silly
> warning.
> 
> Uglifying how?

Requires buffer copying, finding digits there, making a proper termination. And
basically dropping the error check, because we don't care at this stage about
wrong input.

And we didn't start to talk about possible user expectation breakage and
basically breakage weird user cases.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/7] x86/boot: Introduce helpers for serial I/O

[Andy Shevchenko]

On Thu, Mar 28, 2019 at 02:28:55PM +0100, Borislav Petkov wrote:
> On Thu, Mar 28, 2019 at 03:11:40PM +0200, Andy Shevchenko wrote:
> > Any idea how it can be done?
> 
> Like this?
> 
> static u8 serial_inb(unsigned long addr, int offset)

It's not a generic serial_in()...

> {
> 	if (early_serial_do_mmio) {
> 		asm volatile("movb %1,%0" : "=q" (v) : "m" (*(volatile u8 __force *)addr));
> 
> 		return v;
> 	} else {
> 		 return inb(addr + offset);
> 	}
> }

What about mmio32 as next step?

Do we have to "fix" earlyprintk implementation in kernel to do the same?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 7/7] x86/boot: Support nocfg parameter for earlyprintk

[Andy Shevchenko]

On Thu, Mar 28, 2019 at 02:42:13PM +0100, Borislav Petkov wrote:
> On Thu, Mar 28, 2019 at 03:21:53PM +0200, Andy Shevchenko wrote:
> > If the connection to the target is done via serial interface and
> > firmware and / or bootloader already did necessary configuration.
> 
> Again: how is the user supposed to know that?

How one knows that she is typing on keyboard right now and not on touch screen?
I really don't get this. User either knows this beforehand or doesn't care.
Maybe she needs to read documentation on hardware at some point?

> When a user sees "nocfg"'s
> documentation, how is the user supposed to decide whether she needs
> to supply nocfg or not?

The problem nocfg tries to solve is a hard coded default baud rate and
impossibility to know at this stage all parameters of the serial interface.

> Which exact setups will need to supply nocfg on the cmdline?

If it's _non-standard_ serial interface (by meaning of I/O accessors and port
addresses), user may consider to use this options in case the interface to
communicate with kernel is serial one and its configuration is done by other
means (firmware / bootloader / EFI / etc).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/7] x86/boot: Split out parse_serial_port() helper for earlyprintk

[Borislav Petkov]

On Thu, Mar 28, 2019 at 03:50:15PM +0200, Andy Shevchenko wrote:
> Can, but with code uglifying to workaround kstrtox() restrictions.
> simple_strtox() _just works_ as expected.

simple_strtox() also has its problems.

> Requires buffer copying, finding digits there, making a proper termination. And

It needs a single kstrtoull() call to parse the port. And we have
kstrtoull() available in the compressed stage so pls do the conversion
ontop.

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 2/7] x86/boot: Introduce helpers for serial I/O

[Borislav Petkov]

On Thu, Mar 28, 2019 at 03:52:26PM +0200, Andy Shevchenko wrote:
> It's not a generic serial_in()...

Why?

> What about mmio32 as next step?

Next step where? Is it part of your patches?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 7/7] x86/boot: Support nocfg parameter for earlyprintk

[Borislav Petkov]

On Thu, Mar 28, 2019 at 04:00:39PM +0200, Andy Shevchenko wrote:
> If it's _non-standard_ serial interface (by meaning of I/O accessors and port
> addresses), user may consider to use this options in case the interface to
> communicate with kernel is serial one and its configuration is done by other
> means (firmware / bootloader / EFI / etc).

And is this non-standard interface out there in the wild or only at
Intel's labs?

Are there boxes like that out there?

If I go to the store and buy a box, how do I know whether that box would
need "nocfg"? Do I look in the BIOS or how do I find out I need to supply
"nocfg"?

> If by BIOS or by other means the serial port is configured

How do I find out that the serial port is configured?

And don't give me that "example" about the typing on keyboard or on
touch screen again.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 2/7] x86/boot: Introduce helpers for serial I/O

[Andy Shevchenko]

On Thu, Mar 28, 2019 at 03:05:08PM +0100, Borislav Petkov wrote:
> On Thu, Mar 28, 2019 at 03:52:26PM +0200, Andy Shevchenko wrote:
> > It's not a generic serial_in()...
> 
> Why?

By name and by what it's handling (only byte accesses to I/O or MMIO).

> > What about mmio32 as next step?
> 
> Next step where? Is it part of your patches?

Not this series, but we have hardware that does MMIO32 accesses and yet using
non-standard address.

My point is, that doing branch(es) on each I/O call is not good as simple
calling the proper (pre-defined) I/O accessor.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/7] x86/boot: Split out parse_serial_port() helper for earlyprintk

[Andy Shevchenko]

On Thu, Mar 28, 2019 at 03:03:29PM +0100, Borislav Petkov wrote:
> On Thu, Mar 28, 2019 at 03:50:15PM +0200, Andy Shevchenko wrote:
> > Can, but with code uglifying to workaround kstrtox() restrictions.
> > simple_strtox() _just works_ as expected.
> 
> simple_strtox() also has its problems.

Yes! And point here: we do not care.

> > Requires buffer copying, finding digits there, making a proper termination. And
> 
> It needs a single kstrtoull() call to parse the port. 

No, it does not. It needs more as I described. Have you investigated a topic?
Have you seen all possible combinations of input here?

> And we have
> kstrtoull() available in the compressed stage so pls do the conversion
> ontop.

If it's strong opinion, I consider this rather as NAK.
Thank you for review.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 7/7] x86/boot: Support nocfg parameter for earlyprintk

[Andy Shevchenko]

On Thu, Mar 28, 2019 at 03:13:07PM +0100, Borislav Petkov wrote:
> On Thu, Mar 28, 2019 at 04:00:39PM +0200, Andy Shevchenko wrote:
> > If it's _non-standard_ serial interface (by meaning of I/O accessors and port
> > addresses), user may consider to use this options in case the interface to
> > communicate with kernel is serial one and its configuration is done by other
> > means (firmware / bootloader / EFI / etc).
> 
> And is this non-standard interface out there in the wild or only at
> Intel's labs?

Of course in the wild.

> Are there boxes like that out there?

One at hand now is called Intel Edison.

> If I go to the store and buy a box, how do I know whether that box would
> need "nocfg"? Do I look in the BIOS or how do I find out I need to supply
> "nocfg"?

By reading some technical documentation I suppose.
What is your point here? earlyprintk is a mechanism for _debugging_, I suppose
the user should have slightly more knowledge than user.

> > If by BIOS or by other means the serial port is configured
> 
> How do I find out that the serial port is configured?

If you _already_ connected via it and get firmware / bootloader messages.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/7] x86/boot: Introduce helpers for serial I/O

[Borislav Petkov]

On Thu, Mar 28, 2019 at 04:20:01PM +0200, Andy Shevchenko wrote:
> By name and by what it's handling (only byte accesses to I/O or MMIO).

Your series does only byte accesses so you can use byte variant.

> My point is, that doing branch(es) on each I/O call is not good as simple
> calling the proper (pre-defined) I/O accessor.

What branches?

You do a branch and *then* you do an IO call.

What does "not good" even mean?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 3/7] x86/boot: Split out parse_serial_port() helper for earlyprintk

[Borislav Petkov]

On Thu, Mar 28, 2019 at 04:23:02PM +0200, Andy Shevchenko wrote:
> If it's strong opinion, I consider this rather as NAK.
> Thank you for review.

Your call.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 3/7] x86/boot: Split out parse_serial_port() helper for earlyprintk

[Andy Shevchenko]

On Thu, Mar 28, 2019 at 03:35:35PM +0100, Borislav Petkov wrote:
> On Thu, Mar 28, 2019 at 04:23:02PM +0200, Andy Shevchenko wrote:
> > If it's strong opinion, I consider this rather as NAK.
> > Thank you for review.
> 
> Your call.

Yes, as I said I won't uglify code to pursue kstrtox() whim.
Thanks again!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/7] x86/boot: Introduce helpers for serial I/O

[Andy Shevchenko]

On Thu, Mar 28, 2019 at 03:34:47PM +0100, Borislav Petkov wrote:
> On Thu, Mar 28, 2019 at 04:20:01PM +0200, Andy Shevchenko wrote:
> > By name and by what it's handling (only byte accesses to I/O or MMIO).
> 
> Your series does only byte accesses so you can use byte variant.
> 
> > My point is, that doing branch(es) on each I/O call is not good as simple
> > calling the proper (pre-defined) I/O accessor.
> 
> What branches?

Something like this at the end

if (mmio32)
	...
else if (mmio)
	...
else if (whatever)
	...
else
	...

> You do a branch and *then* you do an IO call.

Yes.

> What does "not good" even mean?

We still export global flag variable, on top of this we need an additional
branch which we can avoid for free.

-- 
With Best Regards,
Andy Shevchenko