linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path
@ 2020-04-30 16:14 Alper Nebi Yasak
  2020-04-30 16:14 ` [RFC PATCH v2 1/3] printk: Add function to set console to preferred console's driver Alper Nebi Yasak
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Alper Nebi Yasak @ 2020-04-30 16:14 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Petr Mladek, Sergey Senozhatsky
  Cc: linux-serial, Steven Rostedt, Alper Nebi Yasak, linux-arm-kernel,
	linux-kernel, Andrew Morton, Andy Shevchenko, Arvind Sankar,
	Benjamin Herrenschmidt, Daniel Vetter, David S. Miller,
	Eric Biggers, Feng Tang, Grzegorz Halat, Lukas Wunner,
	Nicolas Pitre, Sam Ravnborg

I recently experienced some trouble with setting up an encrypted-root
system, my Chromebook Plus (rk3399-gru-kevin, ARM64) would appear to
hang where it should have asked for an encryption passphrase; and I
eventually figured out that the kernel preferred the serial port
(inaccessible to me) over the built-in working display/keyboard and was
probably asking there.

Running plymouth in the initramfs solves that specific problem, but
both the documentation and tty-related kconfig descriptions imply that
/dev/console should be tty0 if graphics are working, CONFIG_VT_CONSOLE
is enabled and no explicit console argument is given in the kernel
commandline.

However, I'm seeing different behaviour on systems with SPCR (as in QEMU
aarch64 virtual machines) and/or a device-tree chosen stdout-path node
(as in most arm/arm64 devices). On these machines, depending on the
console argument, the contents of the /proc/consoles file are:

                    |     "console=tty0"    |    (no console arg)   |
  ------------------+-----------------------+-----------------------+
  QEMU VM           | tty0     -WU (EC p  ) | ttyAMA0  -W- (EC   a) |
  (w/ SPCR)         | ttyAMA0  -W- (E    a) |                       |
  ------------------+-----------------------+-----------------------+
  Chromebook Plus   | tty0     -WU (EC p  ) | ttyS2    -W- (EC p a) |
  (w/ stdout-path)  |                       | tty0     -WU (E     ) |
  ------------------+-----------------------+-----------------------+
  Chromebook Plus   | tty0     -WU (EC p  ) | tty0     -WU (EC p  ) |
  (w/o either)      |                       |                       |
  ------------------+-----------------------+-----------------------+

This patchset tries to ensure that VT is preferred in those conditions
even in the presence of firmware-mandated serial consoles. These should
cleanly apply onto next-20200430.

More discussion due to or about the console confusion on ARM64:
- My Debian bug report about the initramfs prompts [0]
- Fedora test issue arising from ARM64 QEMU machines having SPCR [1]
- Debian-installer discussion on what to do with multiple consoles [2]

[0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=952452
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1661288
[2] https://lists.debian.org/debian-boot/2019/01/msg00184.html

Changes in v2:
- Fix #elif to #else (Reported-by: kbuild test robot <lkp@intel.com>)
- Refresh dmesg outputs with/without earlycon for next-20200430
- Use the correct format when referencing a commit

Alper Nebi Yasak (3):
  printk: Add function to set console to preferred console's driver
  vt: Set as preferred console when a non-dummy backend is bound
  printk: Preset tty0 as a pseudo-preferred console

 drivers/tty/vt/vt.c     |  7 +++++
 include/linux/console.h |  1 +
 kernel/printk/printk.c  | 68 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 1/3] printk: Add function to set console to preferred console's driver
  2020-04-30 16:14 [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path Alper Nebi Yasak
@ 2020-04-30 16:14 ` Alper Nebi Yasak
  2020-04-30 16:46   ` Andy Shevchenko
                     ` (2 more replies)
  2020-04-30 16:14 ` [RFC PATCH v2 2/3] vt: Set as preferred console when a non-dummy backend is bound Alper Nebi Yasak
                   ` (4 subsequent siblings)
  5 siblings, 3 replies; 19+ messages in thread
From: Alper Nebi Yasak @ 2020-04-30 16:14 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Petr Mladek, Sergey Senozhatsky
  Cc: linux-serial, Steven Rostedt, Alper Nebi Yasak, linux-arm-kernel,
	linux-kernel, Andrew Morton, Andy Shevchenko, Arvind Sankar,
	Benjamin Herrenschmidt, David S. Miller, Feng Tang

Currently, add_preferred_console sets a preferred console, but doesn't
actually change /dev/console to match it. That part is handled within
register_device, where a newly registered console driver will be set as
/dev/console if it matches the preferred console.

However, if the relevant driver is already registered, the only way to
set it as /dev/console is by un-registering and re-registering it. An
example is the xenfb_make_preferred_console() function:

	console_lock();
	for_each_console(c) {
		if (!strcmp(c->name, "tty") && c->index == 0)
			break;
	}
	console_unlock();
	if (c) {
		unregister_console(c);
		c->flags |= CON_CONSDEV;
		c->flags &= ~CON_PRINTBUFFER; /* don't print again */
		register_console(c);
	}

The code above was introduced in commit 9e124fe16ff2 ("xen: Enable
console tty by default in domU if it's not a dummy"). In short, it's aim
is to set VT as the preferred console only after a working framebuffer
is registered and thus VT is not the dummy device.

This patch introduces an update_console_to_preferred function that
handles the necessary /dev/console change. With this change, the example
above can be replaced with:

	console_lock();
	add_preferred_console("tty", 0, NULL);
	update_console_to_preferred();
	console_unlock();

More importantly, these two calls can be moved to vt.c in order to bump
its priority when a non-dummy backend for it is introduced, solving that
problem in general.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>

---

Changes in v2:
- Use the correct format when referencing a commit

 include/linux/console.h |  1 +
 kernel/printk/printk.c  | 56 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/include/linux/console.h b/include/linux/console.h
index 75dd20650fbe..4b3fa34be245 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -172,6 +172,7 @@ enum con_flush_mode {
 };
 
 extern int add_preferred_console(char *name, int idx, char *options);
+extern int update_console_to_preferred(void);
 extern void register_console(struct console *);
 extern int unregister_console(struct console *);
 extern struct console *console_drivers;
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 6ede4a7222e6..efda422203e4 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2240,12 +2240,68 @@ __setup("console=", console_setup);
  * be used by arch-specific code either to override the user or more
  * commonly to provide a default console (ie from PROM variables) when
  * the user has not supplied one.
+ *
+ * Preferences set by this function don't take effect until the next
+ * time a matching driver for the preferred console is registered. If a
+ * matching driver was already registered, @update_console_to_preferred
+ * function can be used to set that as the preferred console driver.
  */
 int add_preferred_console(char *name, int idx, char *options)
 {
 	return __add_preferred_console(name, idx, options, NULL, false);
 }
 
+/**
+ * update_console_to_preferred - set console to the preferred console's driver.
+ *
+ * Updates console_drivers and CON_CONSDEV flags so that an already
+ * registered and enabled console driver matching the preferred console
+ * is used as /dev/console.
+ *
+ * Must be called within console_lock();.
+ */
+int update_console_to_preferred(void)
+{
+	struct console_cmdline *c = NULL;
+	struct console *con = NULL;
+	struct console *tmp = NULL;
+
+	if (preferred_console >= 0)
+		c = &console_cmdline[preferred_console];
+
+	if (!c || !c->name[0])
+		return 0;
+
+	for_each_console(con) {
+		if (!con->next || !(con->next->flags & CON_ENABLED))
+			continue;
+		if (strcmp(c->name, con->next->name) != 0)
+			continue;
+		if (con->next->index >= 0 &&
+		    con->next->index != c->index)
+			continue;
+		break;
+	}
+
+	if (!con)
+		return -ENODEV;
+
+	pr_info("switching to console [%s%d]\n",
+		con->next->name, con->next->index);
+
+	tmp = con->next;
+	con->next = con->next->next;
+	tmp->next = console_drivers;
+	console_drivers = tmp;
+
+	if (console_drivers->next)
+		console_drivers->next->flags &= ~CON_CONSDEV;
+	console_drivers->flags |= CON_CONSDEV;
+	has_preferred_console = true;
+
+	return 0;
+}
+
 bool console_suspend_enabled = true;
 EXPORT_SYMBOL(console_suspend_enabled);
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 2/3] vt: Set as preferred console when a non-dummy backend is bound
  2020-04-30 16:14 [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path Alper Nebi Yasak
  2020-04-30 16:14 ` [RFC PATCH v2 1/3] printk: Add function to set console to preferred console's driver Alper Nebi Yasak
@ 2020-04-30 16:14 ` Alper Nebi Yasak
  2020-04-30 16:14 ` [RFC PATCH v2 3/3] printk: Preset tty0 as a pseudo-preferred console Alper Nebi Yasak
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Alper Nebi Yasak @ 2020-04-30 16:14 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Petr Mladek, Sergey Senozhatsky
  Cc: linux-serial, Steven Rostedt, Alper Nebi Yasak, linux-arm-kernel,
	linux-kernel, Daniel Vetter, Eric Biggers, Grzegorz Halat,
	Lukas Wunner, Nicolas Pitre, Sam Ravnborg

When a machine's device-tree has a "chosen" node with a "stdout-path"
property that specified console is added as the preferred console by
of_console_check via an add_preferred_console call. The property is
quite common in kernel device-tree definitions. As far as I can tell, it
is set to provide a reasonable default value for earlycon, and the
(usually serial) console is set as preferred to avoid output going to
VT's dummy backend instead of a working console.

However, a chosen stdout-path property is included even in device-trees
of systems that are designed to be used with a built-in display, e.g.
several ARM Chromebooks. In these cases where CONFIG_VT_CONSOLE is
enabled and no console argument is given on the kernel commandline, tty0
is still registered (presumably based on the order of of_console_check
and vt's register_console calls) but ends up not being the preferred
console.

As a result, it is possible for early userspace prompts (encryption
passphrase requests, emergency shells) to end up in a console that the
user doesn't expect or even have access to.

This patch tries to set tty0 as the /dev/console whenever a non-dummy
backend tries to register as its default, unless the preferred console
was set from the kernel commandline arguments.

On a Samsung Chromebook Plus (Google Kevin, rk3399-gru-kevin.dts), boot
messages are still visible on the framebuffer without this patch, but it
isn't the preferred console due to the device-tree having a stdout-path
property (from rk3399-gru.dtsi):

	# Without earlycon:
	$ sudo dmesg | grep -i "console\|printk\|tty[0-9a-z]" | grep -v systemd
	[    0.001447] Console: colour dummy device 80x25
	[    0.001820] printk: console [tty0] enabled
	[    3.012303] ff1a0000.serial: ttyS2 at MMIO 0xff1a0000 (irq = 39, base_baud = 1500000) is a 16550A
	[    4.326549] printk: console [ttyS2] enabled
	[    5.521780] dw-apb-uart ff1a0000.serial: forbid DMA for kernel console
	[    6.359706] Console: switching to colour frame buffer device 300x100

	$ cat /proc/consoles
	ttyS2                -W- (EC p a)    4:66
	tty0                 -WU (E  p  )    4:7

	# With earlycon:
	$ sudo dmesg | grep -i "console\|printk\|tty[0-9a-z]" | grep -v systemd
	[    0.000000] printk: bootconsole [uart0] enabled
	[    0.010257] Console: colour dummy device 80x25
	[    0.015131] printk: console [tty0] enabled
	[    0.019625] printk: bootconsole [uart0] disabled
	[    3.034305] ff1a0000.serial: ttyS2 at MMIO 0xff1a0000 (irq = 39, base_baud = 1500000) is a 16550A
	[    4.367601] printk: console [ttyS2] enabled
	[    5.576519] dw-apb-uart ff1a0000.serial: forbid DMA for kernel console
	[    6.435612] Console: switching to colour frame buffer device 300x100

	$ cat /proc/consoles
	ttyS2                -W- (EC p a)    4:66
	tty0                 -WU (E     )    4:7

And on the same machine, with this patch:

	# Without earlycon:
	$ sudo dmesg | grep -i "console\|printk\|tty[0-9a-z]" | grep -v systemd
	[    0.001451] Console: colour dummy device 80x25
	[    0.001821] printk: console [tty0] enabled
	[    3.013821] ff1a0000.serial: ttyS2 at MMIO 0xff1a0000 (irq = 39, base_baud = 1500000) is a 16550A
	[    4.328053] printk: console [ttyS2] enabled
	[    5.509658] dw-apb-uart ff1a0000.serial: forbid DMA for kernel console
	[    6.309330] Console: switching to colour frame buffer device 300x100
	[    6.378862] printk: switching to console [tty0]

	$ cat /proc/consoles
	tty0                 -WU (EC p  )    4:7
	ttyS2                -W- (E  p a)    4:66

	# With earlycon:
	$ sudo dmesg | grep -i "console\|printk\|tty[0-9a-z]" | grep -v systemd
	[    0.000000] printk: bootconsole [uart0] enabled
	[    0.010259] Console: colour dummy device 80x25
	[    0.015135] printk: console [tty0] enabled
	[    0.019628] printk: bootconsole [uart0] disabled
	[    3.037677] ff1a0000.serial: ttyS2 at MMIO 0xff1a0000 (irq = 39, base_baud = 1500000) is a 16550A
	[    4.370985] printk: console [ttyS2] enabled
	[    5.549226] dw-apb-uart ff1a0000.serial: forbid DMA for kernel console
	[    6.364818] Console: switching to colour frame buffer device 300x100
	[    6.417589] printk: switching to console [tty0]

	$ cat /proc/consoles
	tty0                 -WU (EC    )    4:7
	ttyS2                -W- (E  p a)    4:66

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>

---

Changes in v2:
- Refresh dmesg outputs with/without earlycon for next-20200430

 drivers/tty/vt/vt.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index e5ffed795e4c..218171dca711 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3588,6 +3588,13 @@ static int do_bind_con_driver(const struct consw *csw, int first, int last,
 		pr_cont("to %s\n", desc);
 	}
 
+#ifdef CONFIG_VT_CONSOLE
+	if (!console_set_on_cmdline && deflt && conswitchp != &dummy_con) {
+		add_preferred_console("tty", 0, NULL);
+		update_console_to_preferred();
+	}
+#endif
+
 	retval = 0;
 err:
 	module_put(owner);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 3/3] printk: Preset tty0 as a pseudo-preferred console
  2020-04-30 16:14 [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path Alper Nebi Yasak
  2020-04-30 16:14 ` [RFC PATCH v2 1/3] printk: Add function to set console to preferred console's driver Alper Nebi Yasak
  2020-04-30 16:14 ` [RFC PATCH v2 2/3] vt: Set as preferred console when a non-dummy backend is bound Alper Nebi Yasak
@ 2020-04-30 16:14 ` Alper Nebi Yasak
  2020-04-30 16:44 ` [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path Andy Shevchenko
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Alper Nebi Yasak @ 2020-04-30 16:14 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Petr Mladek, Sergey Senozhatsky
  Cc: linux-serial, Steven Rostedt, Alper Nebi Yasak, linux-arm-kernel,
	linux-kernel

The ACPI SPCR (Serial Port Console Redirection) table from a machine's
firmware can specify a serial console which can be used for earlycon.
However, in at least ARM64 systems the same device is also set up as the
preferred console.  Presumably due to the order of acpi_parse_spcr and
VT's register_device calls, setting the specified console as the
preferred one can prevent registering VT as a console.

This might look appropriate for machines which do not have or need
working graphics and whose users most likely have access to a serial
port. However, the use of SPCR tables may not be limited these. For
example, ARM64 QEMU virtual machines include a SPCR table regardless of
the existence of a graphics output or even the nonexistence of a serial
console. Or server hardware which has a SPCR table can be repurposed
into a workstation with the addition of a graphics card. As a result,
boot messages and early userspace prompts can go to an unexpected
console.

This patch presets tty0 as a pseudo-preferred console at compile-time to
ensure that CONFIG_VT_CONSOLE always results in the VT console getting
registered. With this, VT can get registered, these other consoles are
preferred when VT is a dummy, but we can also bump up VTs preference
when working graphics are available.

Without this patch, an ARM64 QEMU virtual machine has roughly the
following order of console events and consoles:

	# Without earlycon:
	$ sudo dmesg | grep -i "console\|printk\|tty[0-9a-z]" | grep -v systemd
	[    0.000000] ACPI: SPCR: console: pl011,mmio,0x9000000,9600
	[    0.000250] Console: colour dummy device 80x25
	[    0.140955] ARMH0011:00: ttyAMA0 at MMIO 0x9000000 (irq = 4, base_baud = 0) is a SBSA
	[    0.317189] printk: console [ttyAMA0] enabled
	[    3.540272] Console: switching to colour frame buffer device 150x42

	$ cat /proc/consoles
	ttyAMA0              -W- (EC p a)  204:64

	# With earlycon:
	$ sudo dmesg | grep -i "console\|printk\|tty[0-9a-z]" | grep -v systemd
	[    0.000000] ACPI: SPCR: console: pl011,mmio,0x9000000,9600
	[    0.000000] printk: bootconsole [pl11] enabled
	[    0.002064] Console: colour dummy device 80x25
	[    0.324415] ARMH0011:00: ttyAMA0 at MMIO 0x9000000 (irq = 4, base_baud = 0) is a SBSA
	[    0.327047] printk: console [ttyAMA0] enabled
	[    0.329139] printk: bootconsole [pl11] disabled
	[    3.704937] Console: switching to colour frame buffer device 150x42

	$ cat /proc/consoles
	ttyAMA0              -W- (EC   a)  204:64

In addition, boot messages aren't printed to the framebuffer (as tty0 is
not registered). With this patch, boot messages are visible on the
framebuffer and the information above becomes:

	# Without earlycon:
	$ sudo dmesg | grep -i "console\|printk\|tty[0-9a-z]" | grep -v systemd
	[    0.000000] ACPI: SPCR: console: pl011,mmio,0x9000000,9600
	[    0.000058] Console: colour dummy device 80x25
	[    0.000266] printk: console [tty0] enabled
	[    0.144615] ARMH0011:00: ttyAMA0 at MMIO 0x9000000 (irq = 4, base_baud = 0) is a SBSA
	[    0.299031] printk: console [ttyAMA0] enabled
	[    3.320333] Console: switching to colour frame buffer device 150x42
	[    3.329386] printk: switching to console [tty0]

	$ cat /proc/consoles
	tty0                 -WU (EC p  )    4:7
	ttyAMA0              -W- (E  p a)  204:64

	# With earlycon:
	$ sudo dmesg | grep -i "console\|printk\|tty[0-9a-z]" | grep -v systemd
	[    0.000000] ACPI: SPCR: console: pl011,mmio,0x9000000,9600
	[    0.000000] printk: bootconsole [pl11] enabled
	[    0.002326] Console: colour dummy device 80x25
	[    0.003545] printk: console [tty0] enabled
	[    0.236824] ARMH0011:00: ttyAMA0 at MMIO 0x9000000 (irq = 4, base_baud = 0) is a SBSA
	[    0.239243] printk: console [ttyAMA0] enabled
	[    0.241324] printk: bootconsole [pl11] disabled
	[    3.715086] Console: switching to colour frame buffer device 150x42
	[    3.742923] printk: switching to console [tty0]

	$ cat /proc/consoles
	tty0                 -WU (EC p  )    4:7
	ttyAMA0              -W- (E    a)  204:64

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>

---

Changes in v2:
- Fix #elif to #else (Reported-by: kbuild test robot <lkp@intel.com>)
- Refresh dmesg outputs with/without earlycon for next-20200430

 kernel/printk/printk.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index efda422203e4..aa9f9f16860f 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -277,7 +277,19 @@ static struct console *exclusive_console;
 
 #define MAX_CMDLINECONSOLES 8
 
+/*
+ * The preferred_console and has_preferred_console variables are
+ * intentionally not modified to reflect this so that the first
+ * registered console is still used as the preferred console.
+ */
+#ifdef CONFIG_VT_CONSOLE
+static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES] = {
+	[0].name = "tty",
+	[0].index = 0,
+};
+#else
 static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
+#endif
 
 static int preferred_console = -1;
 static bool has_preferred_console;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path
  2020-04-30 16:14 [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path Alper Nebi Yasak
                   ` (2 preceding siblings ...)
  2020-04-30 16:14 ` [RFC PATCH v2 3/3] printk: Preset tty0 as a pseudo-preferred console Alper Nebi Yasak
@ 2020-04-30 16:44 ` Andy Shevchenko
  2020-04-30 19:32   ` Alper Nebi Yasak
  2020-05-01  1:30 ` Sergey Senozhatsky
  2020-05-13 14:37 ` Petr Mladek
  5 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2020-04-30 16:44 UTC (permalink / raw)
  To: Alper Nebi Yasak
  Cc: Greg Kroah-Hartman, Jiri Slaby, Petr Mladek, Sergey Senozhatsky,
	linux-serial, Steven Rostedt, linux-arm-kernel, linux-kernel,
	Andrew Morton, Arvind Sankar, Benjamin Herrenschmidt,
	Daniel Vetter, David S. Miller, Eric Biggers, Feng Tang,
	Grzegorz Halat, Lukas Wunner, Nicolas Pitre, Sam Ravnborg

On Thu, Apr 30, 2020 at 07:14:34PM +0300, Alper Nebi Yasak wrote:

First of all I see only cover letter and one out of 3 patches.

> I recently experienced some trouble with setting up an encrypted-root
> system, my Chromebook Plus (rk3399-gru-kevin, ARM64) would appear to
> hang where it should have asked for an encryption passphrase; and I
> eventually figured out that the kernel preferred the serial port
> (inaccessible to me) over the built-in working display/keyboard and was
> probably asking there.

"probably". Please, confirm that first.
Also, without command line it's hard to say what you have asked kernel to do.

> Running plymouth in the initramfs solves that specific problem, but
> both the documentation and tty-related kconfig descriptions imply that
> /dev/console should be tty0 if graphics are working, CONFIG_VT_CONSOLE
> is enabled and no explicit console argument is given in the kernel
> commandline.

What is plymouth?

> However, I'm seeing different behaviour on systems with SPCR (as in QEMU
> aarch64 virtual machines) and/or a device-tree chosen stdout-path node
> (as in most arm/arm64 devices). On these machines, depending on the
> console argument, the contents of the /proc/consoles file are:
> 
>                     |     "console=tty0"    |    (no console arg)   |
>   ------------------+-----------------------+-----------------------+
>   QEMU VM           | tty0     -WU (EC p  ) | ttyAMA0  -W- (EC   a) |
>   (w/ SPCR)         | ttyAMA0  -W- (E    a) |                       |
>   ------------------+-----------------------+-----------------------+
>   Chromebook Plus   | tty0     -WU (EC p  ) | ttyS2    -W- (EC p a) |
>   (w/ stdout-path)  |                       | tty0     -WU (E     ) |
>   ------------------+-----------------------+-----------------------+
>   Chromebook Plus   | tty0     -WU (EC p  ) | tty0     -WU (EC p  ) |
>   (w/o either)      |                       |                       |
>   ------------------+-----------------------+-----------------------+

either == SPCR or stdout-path?

> This patchset tries to ensure that VT is preferred in those conditions
> even in the presence of firmware-mandated serial consoles.

This sounds completely wrong. serial should be preferred over vt due to very
debugging on early stages and SPCR is exactly for that.

> These should
> cleanly apply onto next-20200430.
> 
> More discussion due to or about the console confusion on ARM64:
> - My Debian bug report about the initramfs prompts [0]
> - Fedora test issue arising from ARM64 QEMU machines having SPCR [1]
> - Debian-installer discussion on what to do with multiple consoles [2]

Maybe you should figure out the real root cause?

> [0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=952452
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1661288
> [2] https://lists.debian.org/debian-boot/2019/01/msg00184.html

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 1/3] printk: Add function to set console to preferred console's driver
  2020-04-30 16:14 ` [RFC PATCH v2 1/3] printk: Add function to set console to preferred console's driver Alper Nebi Yasak
@ 2020-04-30 16:46   ` Andy Shevchenko
  2020-05-01  1:44   ` Sergey Senozhatsky
  2020-05-13  5:35   ` Sergey Senozhatsky
  2 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2020-04-30 16:46 UTC (permalink / raw)
  To: Alper Nebi Yasak
  Cc: Greg Kroah-Hartman, Jiri Slaby, Petr Mladek, Sergey Senozhatsky,
	linux-serial, Steven Rostedt, linux-arm-kernel, linux-kernel,
	Andrew Morton, Arvind Sankar, Benjamin Herrenschmidt,
	David S. Miller, Feng Tang

On Thu, Apr 30, 2020 at 07:14:35PM +0300, Alper Nebi Yasak wrote:
> Currently, add_preferred_console sets a preferred console, but doesn't
> actually change /dev/console to match it. That part is handled within
> register_device, where a newly registered console driver will be set as
> /dev/console if it matches the preferred console.
> 
> However, if the relevant driver is already registered, the only way to
> set it as /dev/console is by un-registering and re-registering it. An
> example is the xenfb_make_preferred_console() function:
> 
> 	console_lock();
> 	for_each_console(c) {
> 		if (!strcmp(c->name, "tty") && c->index == 0)
> 			break;
> 	}
> 	console_unlock();
> 	if (c) {
> 		unregister_console(c);
> 		c->flags |= CON_CONSDEV;
> 		c->flags &= ~CON_PRINTBUFFER; /* don't print again */
> 		register_console(c);
> 	}
> 
> The code above was introduced in commit 9e124fe16ff2 ("xen: Enable
> console tty by default in domU if it's not a dummy"). In short, it's aim
> is to set VT as the preferred console only after a working framebuffer
> is registered and thus VT is not the dummy device.
> 
> This patch introduces an update_console_to_preferred function that
> handles the necessary /dev/console change. With this change, the example
> above can be replaced with:
> 
> 	console_lock();
> 	add_preferred_console("tty", 0, NULL);
> 	update_console_to_preferred();
> 	console_unlock();
> 
> More importantly, these two calls can be moved to vt.c in order to bump
> its priority when a non-dummy backend for it is introduced, solving that
> problem in general.

Even w/o looking into the code I believe it breaks more platforms than fixes
anything. It was not first time one tries to do something about preferred
consoles and it appeared to break working configurations.

I would wait for PRINTK maintainers to tell, but to me it sounds like papering
over the real issue which you don't understand (yet).

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path
  2020-04-30 16:44 ` [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path Andy Shevchenko
@ 2020-04-30 19:32   ` Alper Nebi Yasak
  0 siblings, 0 replies; 19+ messages in thread
From: Alper Nebi Yasak @ 2020-04-30 19:32 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Jiri Slaby, Petr Mladek, Sergey Senozhatsky,
	linux-serial, Steven Rostedt, linux-arm-kernel, linux-kernel,
	Andrew Morton, Arvind Sankar, Benjamin Herrenschmidt,
	Daniel Vetter, David S. Miller, Eric Biggers, Feng Tang,
	Grzegorz Halat, Lukas Wunner, Nicolas Pitre, Sam Ravnborg

On 30/04/2020 19:44, Andy Shevchenko wrote:
> First of all I see only cover letter and one out of 3 patches.

Apologies, the tool I've used to send the patches (U-Boot's patman)
Cc-ed the scripts/get_maintainer.pl output per-patch, instead of
per-series as I had assumed it would. This was the first time I tried
it, I'll keep that in mind.

Here are links to all four emails:

https://lore.kernel.org/linux-serial/20200430161438.17640-1-alpernebiyasak@gmail.com/
https://lore.kernel.org/linux-serial/20200430161438.17640-2-alpernebiyasak@gmail.com/
https://lore.kernel.org/linux-serial/20200430161438.17640-3-alpernebiyasak@gmail.com/
https://lore.kernel.org/linux-serial/20200430161438.17640-4-alpernebiyasak@gmail.com/

Or I can resend the last two patches to you, or resend all the parts to
everyone again.

>> eventually figured out that the kernel preferred the serial port
>> (inaccessible to me) over the built-in working display/keyboard and was
>> probably asking there.
> 
> "probably". Please, confirm that first.
> Also, without command line it's hard to say what you have asked kernel to do.

I was trying to boot a Debian userspace with cryptsetup, with the kernel
command line:

	root=/dev/mapper/sda3_crypt quiet splash

The Debian initramfs handles most of the work (the password prompt,
device mounts, etc.).

When I used the same kernel/initramfs/rootfs on a QEMU aarch64 VM, it
only prompted on the serial console instead the framebuffer. I'm
assuming the same thing happens on my hardware as well.

I can also ask the Debian initramfs to launch a shell by adding "break"
to the command line, which won't be printed on my device's screen unless
I also add "console=tty0". That shell also only appears on the serial
console on the QEMU aarch64 VM, unless I again add "console=tty0".

This is my primary computer and I'd prefer not dismantling it, so my
findings above are the best I believe I can do to confirm it now. I'm 
hoping other people would be interested in this, and would test more 
than I can.

>> Running plymouth in the initramfs solves that specific problem, but
> 
> What is plymouth?

Plymouth is a userspace program that's famous for showing a splash
animation during boot, but in this context: it handles user-interaction
that might need to happen while the initramfs is running, by printing
messages and prompts and reading user input to/from all consoles.

>>    ------------------+-----------------------+-----------------------+
>>    Chromebook Plus   | tty0     -WU (EC p  ) | tty0     -WU (EC p  ) |
>>    (w/o either)      |                       |                       |
>>    ------------------+-----------------------+-----------------------+
> 
> either == SPCR or stdout-path?

As in "When the device has no SPCR _and_ no chosen stdout-path".

>> This patchset tries to ensure that VT is preferred in those conditions
>> even in the presence of firmware-mandated serial consoles.
> 
> This sounds completely wrong. serial should be preferred over vt due to very
> debugging on early stages and SPCR is exactly for that.

I'm saying that from a userspace perspective, and the patches explicitly
try to switch to the vt only after a real framebuffer is initialized. So
if I did it right, it would still use SPCR/stdout-path's console during
the early stages. (I admit I haven't adjusted to talking within a kernel 
context yet).

In all honesty, I'm not sure if this is even considered a kernel bug,
let alone my patches a correct solution; hence the RFC PATCH as an
attempt at demonstrating this can be "fixed" in kernel.

> Maybe you should figure out the real root cause?

Thanks for the reply. Any ideas on what else could be causing this 
behaviour?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path
  2020-04-30 16:14 [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path Alper Nebi Yasak
                   ` (3 preceding siblings ...)
  2020-04-30 16:44 ` [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path Andy Shevchenko
@ 2020-05-01  1:30 ` Sergey Senozhatsky
  2020-05-01 11:08   ` Alper Nebi Yasak
  2020-05-13 14:37 ` Petr Mladek
  5 siblings, 1 reply; 19+ messages in thread
From: Sergey Senozhatsky @ 2020-05-01  1:30 UTC (permalink / raw)
  To: Alper Nebi Yasak
  Cc: Greg Kroah-Hartman, Jiri Slaby, Petr Mladek, Sergey Senozhatsky,
	linux-serial, Steven Rostedt, linux-arm-kernel, linux-kernel,
	Andrew Morton, Andy Shevchenko, Arvind Sankar,
	Benjamin Herrenschmidt, Daniel Vetter, David S. Miller,
	Eric Biggers, Feng Tang, Grzegorz Halat, Lukas Wunner,
	Nicolas Pitre, Sam Ravnborg

On (20/04/30 19:14), Alper Nebi Yasak wrote:
>                     |     "console=tty0"    |    (no console arg)   |
>   ------------------+-----------------------+-----------------------+
>   QEMU VM           | tty0     -WU (EC p  ) | ttyAMA0  -W- (EC   a) |
>   (w/ SPCR)         | ttyAMA0  -W- (E    a) |                       |
>   ------------------+-----------------------+-----------------------+
>   Chromebook Plus   | tty0     -WU (EC p  ) | ttyS2    -W- (EC p a) |
>   (w/ stdout-path)  |                       | tty0     -WU (E     ) |
>   ------------------+-----------------------+-----------------------+
>   Chromebook Plus   | tty0     -WU (EC p  ) | tty0     -WU (EC p  ) |
>   (w/o either)      |                       |                       |
>   ------------------+-----------------------+-----------------------+
> 
> This patchset tries to ensure that VT is preferred in those conditions
> even in the presence of firmware-mandated serial consoles. These should
> cleanly apply onto next-20200430.

Well, if there is a "mandated console", then why would we prefer
any other console?

	-ss

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 1/3] printk: Add function to set console to preferred console's driver
  2020-04-30 16:14 ` [RFC PATCH v2 1/3] printk: Add function to set console to preferred console's driver Alper Nebi Yasak
  2020-04-30 16:46   ` Andy Shevchenko
@ 2020-05-01  1:44   ` Sergey Senozhatsky
  2020-05-01 11:48     ` Alper Nebi Yasak
  2020-05-13  5:35   ` Sergey Senozhatsky
  2 siblings, 1 reply; 19+ messages in thread
From: Sergey Senozhatsky @ 2020-05-01  1:44 UTC (permalink / raw)
  To: Alper Nebi Yasak
  Cc: Greg Kroah-Hartman, Jiri Slaby, Petr Mladek, Sergey Senozhatsky,
	linux-serial, Steven Rostedt, linux-arm-kernel, linux-kernel,
	Andrew Morton, Andy Shevchenko, Arvind Sankar,
	Benjamin Herrenschmidt, David S. Miller, Feng Tang

On (20/04/30 19:14), Alper Nebi Yasak wrote:
> Currently, add_preferred_console sets a preferred console, but doesn't
> actually change /dev/console to match it. That part is handled within
> register_device, where a newly registered console driver will be set as
> /dev/console if it matches the preferred console.
> 
> However, if the relevant driver is already registered, the only way to
> set it as /dev/console is by un-registering and re-registering it.

Hmm. Preferred console selection is very fragile, there are too many
setups and workarounds that even minor tweaks introduce regressions
oftentimes.

We have, by the way, a pending patchset which changes the same
are - preferred console selection.

git://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git for-5.7-preferred-console

[..]
> An example is the xenfb_make_preferred_console() function:
> 
> 	console_lock();
> 	for_each_console(c) {
> 		if (!strcmp(c->name, "tty") && c->index == 0)
> 			break;
> 	}
> 	console_unlock();
> 	if (c) {
> 		unregister_console(c);
> 		c->flags |= CON_CONSDEV;
> 		c->flags &= ~CON_PRINTBUFFER; /* don't print again */
> 		register_console(c);
> 	}

I didn't know about this code.

> The code above was introduced in commit 9e124fe16ff2 ("xen: Enable
> console tty by default in domU if it's not a dummy"). In short, it's aim
> is to set VT as the preferred console only after a working framebuffer
> is registered and thus VT is not the dummy device.
> 
> This patch introduces an update_console_to_preferred function that
> handles the necessary /dev/console change. With this change, the example
> above can be replaced with:
> 
> 	console_lock();
> 	add_preferred_console("tty", 0, NULL);
> 	update_console_to_preferred();
> 	console_unlock();
> 
> More importantly, these two calls can be moved to vt.c in order to bump
> its priority when a non-dummy backend for it is introduced, solving that
> problem in general.

Let me take a look over the weekend.

	-ss

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path
  2020-05-01  1:30 ` Sergey Senozhatsky
@ 2020-05-01 11:08   ` Alper Nebi Yasak
  2020-05-01 13:16     ` Andy Shevchenko
  0 siblings, 1 reply; 19+ messages in thread
From: Alper Nebi Yasak @ 2020-05-01 11:08 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Greg Kroah-Hartman, Jiri Slaby, Petr Mladek, linux-serial,
	Steven Rostedt, linux-arm-kernel, linux-kernel, Andrew Morton,
	Andy Shevchenko, Arvind Sankar, Benjamin Herrenschmidt,
	Daniel Vetter, David S. Miller, Eric Biggers, Feng Tang,
	Grzegorz Halat, Lukas Wunner, Nicolas Pitre, Sam Ravnborg

On 01/05/2020 04:30, Sergey Senozhatsky wrote:> Well, if there is a "mandated console", then why would we prefer
> any other console?

From what I understand, the firmware provides serial console settings to
be used as the preferred _serial_ console (where it would be OK to
switch to graphical consoles later on) and the kernel currently
understands that such a console should be the preferred _system_ console
(always preferred over even graphical ones). By "mandated" I'm referring
to the kernel's current behavior, not to (in my understanding) the
firmware's intentions.

Even if the firmware/specifications is really asking the kernel to (tell
userspace programs to) always use the serial console instead of the
framebuffer console, while on e.g. a laptop-like device intended to be
used with a keyboard and display -- is that the correct thing to do?

From the userspace, under the conditions:

- CONFIG_VT_CONSOLE is enabled
- There is a working graphics adapter and a display
- There is no console argument given in the kernel command line

I expect that:

- tty0 is included in the /proc/consoles list [1]
- tty0 is the preferred console and /dev/console refers to it [2]

With SPCR both are false, and with stdout-path only the second is false.
Again, I'm OK with these being false during earlier stages until
graphics start working, but I'm arguing they should be true after then.

In the patches I tried to keep these serial consoles still enabled and
preferred during early stages of boot, by trying to switch to vt only
after a real working graphical backend for it is initialized.

I mean, if my expectations are unreasonable and the current kernel
behaviour is considered correct, these patches would be conceptually
wrong; so please tell me if I got anything right/wrong in all this.


[1] From the descripion of CONFIG_VT_CONSOLE:

> [...] If you answer Y here, a virtual terminal (the device used to
> interact with a physical terminal) can be used as system console.
> [...] you should say Y here unless you want the kernel messages be
> output only to a serial port [...]

and by "as a prerequisite of [2]"


[2] From the descripion of CONFIG_VT_CONSOLE:

> If you do say Y here, by default the currently visible virtual
> terminal (/dev/tty0) will be used as system console. You can change
> that with a kernel command line option such as "console=tty3" which
> would use the third virtual terminal as system console. [...]

I'm assuming "by default" here means "without console arguments"
regardless of firmware requests. This paragraph (with small changes) is
repeated on many other Kconfig descriptions (drivers/tty/serial/Kconfig,
drivers/tty/serial/8250/Kconfig, arch/sparc/Kconfig from grepping for
'/dev/tty0' on **/Kconfig).

From Documentation/admin-guide/serial-console.rst:

> You can specify multiple console= options on the kernel command line.
> [...]
> Note that you can only define one console per device type (serial, video).
>
> If no console device is specified, the first device found capable of
> acting as a system console will be used. At this time, the system
> first looks for a VGA card and then for a serial port. So if you don't
> have a VGA card in your system the first serial port will automatically
> become the console.

and later on:

> Note that if you boot without a ``console=`` option (or with
> ``console=/dev/tty0``), ``/dev/console`` is the same as ``/dev/tty0``.
> In that case everything will still work.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 1/3] printk: Add function to set console to preferred console's driver
  2020-05-01  1:44   ` Sergey Senozhatsky
@ 2020-05-01 11:48     ` Alper Nebi Yasak
  0 siblings, 0 replies; 19+ messages in thread
From: Alper Nebi Yasak @ 2020-05-01 11:48 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Greg Kroah-Hartman, Jiri Slaby, Petr Mladek, linux-serial,
	Steven Rostedt, linux-arm-kernel, linux-kernel, Andrew Morton,
	Andy Shevchenko, Arvind Sankar, Benjamin Herrenschmidt,
	David S. Miller, Feng Tang

On 01/05/2020 04:44, Sergey Senozhatsky wrote:
> Hmm. Preferred console selection is very fragile, there are too many
> setups and workarounds that even minor tweaks introduce regressions
> oftentimes.

All this would only execute on #ifdef VT_CONSOLE right now and I think
they can be gated further by a new Kconfig like VT_CONSOLE_PREFERRED, if
that'd make it better.

> We have, by the way, a pending patchset which changes the same
> are - preferred console selection.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git for-5.7-preferred-console

I was working on next-20200430 where that patchset is already included,
can confirm it doesn't solve this problem. I hope I've correctly avoided
breaking it by setting its "has_preferred_console" variable (thus this
patchset somewhat depends on that).

> Let me take a look over the weekend.

Thanks in advance.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path
  2020-05-01 11:08   ` Alper Nebi Yasak
@ 2020-05-01 13:16     ` Andy Shevchenko
  2020-05-01 15:07       ` Alper Nebi Yasak
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2020-05-01 13:16 UTC (permalink / raw)
  To: Alper Nebi Yasak
  Cc: Sergey Senozhatsky, Greg Kroah-Hartman, Jiri Slaby, Petr Mladek,
	open list:SERIAL DRIVERS, Steven Rostedt, linux-arm Mailing List,
	Linux Kernel Mailing List, Andrew Morton, Andy Shevchenko,
	Arvind Sankar, Benjamin Herrenschmidt, Daniel Vetter,
	David S. Miller, Eric Biggers, Feng Tang, Grzegorz Halat,
	Lukas Wunner, Nicolas Pitre, Sam Ravnborg

On Fri, May 1, 2020 at 2:11 PM Alper Nebi Yasak
<alpernebiyasak@gmail.com> wrote:
> On 01/05/2020 04:30, Sergey Senozhatsky wrote:

> I'm assuming "by default" here means "without console arguments"
> regardless of firmware requests. This paragraph (with small changes) is
> repeated on many other Kconfig descriptions (drivers/tty/serial/Kconfig,
> drivers/tty/serial/8250/Kconfig, arch/sparc/Kconfig from grepping for
> '/dev/tty0' on **/Kconfig).
>
> From Documentation/admin-guide/serial-console.rst:
>
> > You can specify multiple console= options on the kernel command line.
> > [...]
> > Note that you can only define one console per device type (serial, video).
> >
> > If no console device is specified, the first device found capable of
> > acting as a system console will be used. At this time, the system
> > first looks for a VGA card and then for a serial port. So if you don't
> > have a VGA card in your system the first serial port will automatically
> > become the console.
>
> and later on:
>
> > Note that if you boot without a ``console=`` option (or with
> > ``console=/dev/tty0``), ``/dev/console`` is the same as ``/dev/tty0``.
> > In that case everything will still work.

I'm wondering if behaviour is changed if you put console=tty1 instead
of console=tty0.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path
  2020-05-01 13:16     ` Andy Shevchenko
@ 2020-05-01 15:07       ` Alper Nebi Yasak
  0 siblings, 0 replies; 19+ messages in thread
From: Alper Nebi Yasak @ 2020-05-01 15:07 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Sergey Senozhatsky, Greg Kroah-Hartman, Jiri Slaby, Petr Mladek,
	open list:SERIAL DRIVERS, Steven Rostedt, linux-arm Mailing List,
	Linux Kernel Mailing List, Andrew Morton, Andy Shevchenko,
	Arvind Sankar, Benjamin Herrenschmidt, Daniel Vetter,
	David S. Miller, Eric Biggers, Feng Tang, Grzegorz Halat,
	Lukas Wunner, Nicolas Pitre, Sam Ravnborg



On 01/05/2020 16:16, Andy Shevchenko wrote:
> On Fri, May 1, 2020 at 2:11 PM Alper Nebi Yasak
> <alpernebiyasak@gmail.com> wrote:
>> I'm assuming "by default" here means "without console arguments"
>> regardless of firmware requests. This paragraph (with small changes) is
>> repeated on many other Kconfig descriptions (drivers/tty/serial/Kconfig,
>> drivers/tty/serial/8250/Kconfig, arch/sparc/Kconfig from grepping for
>> '/dev/tty0' on **/Kconfig).
>>
>>  From Documentation/admin-guide/serial-console.rst:
>>
>>> You can specify multiple console= options on the kernel command line.
>>> [...]
>>> Note that you can only define one console per device type (serial, video).
>>>
>>> If no console device is specified, the first device found capable of
>>> acting as a system console will be used. At this time, the system
>>> first looks for a VGA card and then for a serial port. So if you don't
>>> have a VGA card in your system the first serial port will automatically
>>> become the console.
>>
>> and later on:
>>
>>> Note that if you boot without a ``console=`` option (or with
>>> ``console=/dev/tty0``), ``/dev/console`` is the same as ``/dev/tty0``.
>>> In that case everything will still work.
> 
> I'm wondering if behaviour is changed if you put console=tty1 instead
> of console=tty0.

Just tested again with the QEMU aarch64 VM. Comparing console=tty1 and
console=tty0 cases: /proc/consoles has tty1 instead of tty0 (both also
has ttyAMA0), and `echo '/dev/console is here' >>/dev/console` goes to
vt1 instead of the currently visible vt. Same difference before and
after this patchset.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 1/3] printk: Add function to set console to preferred console's driver
  2020-04-30 16:14 ` [RFC PATCH v2 1/3] printk: Add function to set console to preferred console's driver Alper Nebi Yasak
  2020-04-30 16:46   ` Andy Shevchenko
  2020-05-01  1:44   ` Sergey Senozhatsky
@ 2020-05-13  5:35   ` Sergey Senozhatsky
  2020-05-24 10:01     ` Daniel Vetter
  2 siblings, 1 reply; 19+ messages in thread
From: Sergey Senozhatsky @ 2020-05-13  5:35 UTC (permalink / raw)
  To: Alper Nebi Yasak
  Cc: Greg Kroah-Hartman, Jiri Slaby, Petr Mladek, Sergey Senozhatsky,
	linux-serial, Steven Rostedt, linux-arm-kernel, linux-kernel,
	Andrew Morton, Andy Shevchenko, Arvind Sankar,
	Benjamin Herrenschmidt, David S. Miller, Feng Tang,
	Daniel Vetter

On (20/04/30 19:14), Alper Nebi Yasak wrote:
[..]
> +int update_console_to_preferred(void)
> +{
> +	struct console_cmdline *c = NULL;
> +	struct console *con = NULL;
> +	struct console *tmp = NULL;
> +
> +	if (preferred_console >= 0)
> +		c = &console_cmdline[preferred_console];
> +
> +	if (!c || !c->name[0])
> +		return 0;
> +
> +	for_each_console(con) {
> +		if (!con->next || !(con->next->flags & CON_ENABLED))
> +			continue;
> +		if (strcmp(c->name, con->next->name) != 0)
> +			continue;

This matches the consoles by exact name. Consoles can have aliases,
but matching by alias is rather complex and it has some side effects.

Let me Cc more people on this. VT has a console takeover logic,
I wonder if we can extend the takeover code somehow.

Daniel, any thoughts?

https://lore.kernel.org/lkml/20200430161438.17640-1-alpernebiyasak@gmail.com

	-ss

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path
  2020-04-30 16:14 [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path Alper Nebi Yasak
                   ` (4 preceding siblings ...)
  2020-05-01  1:30 ` Sergey Senozhatsky
@ 2020-05-13 14:37 ` Petr Mladek
  2020-05-13 22:22   ` Benjamin Herrenschmidt
  2020-05-15 19:27   ` Alper Nebi Yasak
  5 siblings, 2 replies; 19+ messages in thread
From: Petr Mladek @ 2020-05-13 14:37 UTC (permalink / raw)
  To: Alper Nebi Yasak
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky, linux-serial,
	Steven Rostedt, linux-arm-kernel, linux-kernel, Andrew Morton,
	Andy Shevchenko, Arvind Sankar, Benjamin Herrenschmidt,
	Daniel Vetter, David S. Miller, Eric Biggers, Feng Tang,
	Grzegorz Halat, Lukas Wunner, Nicolas Pitre, Sam Ravnborg

On Thu 2020-04-30 19:14:34, Alper Nebi Yasak wrote:
> I recently experienced some trouble with setting up an encrypted-root
> system, my Chromebook Plus (rk3399-gru-kevin, ARM64) would appear to
> hang where it should have asked for an encryption passphrase; and I
> eventually figured out that the kernel preferred the serial port
> (inaccessible to me) over the built-in working display/keyboard and was
> probably asking there.
> 
> Running plymouth in the initramfs solves that specific problem, but
> both the documentation and tty-related kconfig descriptions imply that
> /dev/console should be tty0 if graphics are working, CONFIG_VT_CONSOLE
> is enabled and no explicit console argument is given in the kernel
> commandline.
> 
> However, I'm seeing different behaviour on systems with SPCR (as in QEMU
> aarch64 virtual machines) and/or a device-tree chosen stdout-path node
> (as in most arm/arm64 devices). On these machines, depending on the
> console argument, the contents of the /proc/consoles file are:

I dug many times into the history of the console registration code.
The following table mostly confirms my expectations.


>                     |     "console=tty0"    |    (no console arg)   |
>   ------------------+-----------------------+-----------------------+
>   QEMU VM           | tty0     -WU (EC p  ) | ttyAMA0  -W- (EC   a) |
>   (w/ SPCR)         | ttyAMA0  -W- (E    a) |
>   |

The SPCR handling is inconsistent over architectures, see
https://lkml.kernel.org/r/20180830123849.26163-1-prarit@redhat.com

IMHO, arm developers decided that consoles defined by SPCR are always
enabled when existing.

In 1st column: tty0 is the preferred console because it is defined
on the commandline.

In 2nd column: tty0 is not enabled at all because another console was
defined by SPCR. Note that ttySX and ttyX consoles are registered only
as a fallback when there is no other console defined.

The following code is responsible for the fallback, see register_console()

	/*
	 *	See if we want to use this console driver. If we
	 *	didn't select a console we take the first one
	 *	that registers here.
	 */
	if (!has_preferred) {
		if (newcon->index < 0)
			newcon->index = 0;
		if (newcon->setup == NULL ||
		    newcon->setup(newcon, NULL) == 0) {
			newcon->flags |= CON_ENABLED;
			if (newcon->device) {
				newcon->flags |= CON_CONSDEV;
				has_preferred = true;
			}
		}
	}


>   ------------------+-----------------------+-----------------------+
>   Chromebook Plus   | tty0     -WU (EC p  ) | ttyS2    -W- (EC p a) |
>   (w/ stdout-path)  |                       | tty0     -WU (E     ) |

Hmm, of_console_check() explicitly ignores the console defined by
stdout-path when there is a console on the commandline. This explains
1st column.

I am not sure about 2nd column. My guess is that ttyX consoles are
tried first. tty0 is registered as a fallback because there is no
other console at the moment. ttyS2 is tried later and it is
registered because it is in stdout-patch and there is no console
in the command line. It is somehow consistent with  CONFIG_VT_CONSOLE
description.

Sadly, it is different logic than with SPCR :-(


>   ------------------+-----------------------+-----------------------+
>   Chromebook Plus   | tty0     -WU (EC p  ) | tty0     -WU (EC p  ) |
>   (w/o either)      |                       |                       |
>   ------------------+-----------------------+-----------------------+

This variant is easy and everyone would probably expect this.


Regarding the description of CONFIG_VT_CONSOLE option. I am afraid
that it was created and true only before SPCR and device tree support
was introduced.


Now, it is really sad that SPCR and device tree have different
behavior even across architectures. But I am afraid that we could
not change it without breaking many setups.

The only common rules are:

   + The last console on the command line should always be the
     preferred one when defined.

   + Consoles defined by the device (SPCR, device tree) are used
     when there is no commandline.

   + ttyX or ttySX are used as a fallback when nothing else is defined.


My suggestion is:

   + Fix SPCR setting or device tree of your device when the defaults
     are not as expected.

   + Use command line to force your value when the defaults are not
     as expected and you could not change them.


I am afraid that we could not fix your problem on the kernel side. It
would broke other setups that depend on the existing behavior.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path
  2020-05-13 14:37 ` Petr Mladek
@ 2020-05-13 22:22   ` Benjamin Herrenschmidt
  2020-05-15 19:27   ` Alper Nebi Yasak
  1 sibling, 0 replies; 19+ messages in thread
From: Benjamin Herrenschmidt @ 2020-05-13 22:22 UTC (permalink / raw)
  To: Petr Mladek, Alper Nebi Yasak
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky, linux-serial,
	Steven Rostedt, linux-arm-kernel, linux-kernel, Andrew Morton,
	Andy Shevchenko, Arvind Sankar, Daniel Vetter, David S. Miller,
	Eric Biggers, Feng Tang, Grzegorz Halat, Lukas Wunner,
	Nicolas Pitre, Sam Ravnborg

On Wed, 2020-05-13 at 16:37 +0200, Petr Mladek wrote:
> The only common rules are:
> 
>    + The last console on the command line should always be the
>      preferred one when defined.
> 
>    + Consoles defined by the device (SPCR, device tree) are used
>      when there is no commandline.

With the exception that on x86, SPCR is only used for early_con, we
don't do add_preferred_console() at all for it.

I sort-of understand why... the track record on BIOS quality out there
being what it is, I could see this causing a number of systems start
sending the console to a non-existent or non-wired serial port instead
of the tty/gpu because the BIOS leave SPCR set/enabled for no reason.

It may or may not be the case in practice but I don't see how we can
figure that out without either a large campain of data collection from
tons of systems (which will miss plenty) or just taking the chance &
breaking people and see who screams :-)

Cheers,
Ben.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path
  2020-05-13 14:37 ` Petr Mladek
  2020-05-13 22:22   ` Benjamin Herrenschmidt
@ 2020-05-15 19:27   ` Alper Nebi Yasak
  2020-05-25 13:04     ` Petr Mladek
  1 sibling, 1 reply; 19+ messages in thread
From: Alper Nebi Yasak @ 2020-05-15 19:27 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky, linux-serial,
	Steven Rostedt, linux-arm-kernel, linux-kernel, Andrew Morton,
	Andy Shevchenko, Arvind Sankar, Benjamin Herrenschmidt,
	Daniel Vetter, David S. Miller, Eric Biggers, Feng Tang,
	Grzegorz Halat, Lukas Wunner, Nicolas Pitre, Sam Ravnborg

On 13/05/2020 17:37, Petr Mladek wrote:
> On Thu 2020-04-30 19:14:34, Alper Nebi Yasak wrote:
>>                      |     "console=tty0"    |    (no console arg)   |
>>    ------------------+-----------------------+-----------------------+
>>    QEMU VM           | tty0     -WU (EC p  ) | ttyAMA0  -W- (EC   a) |
>>    (w/ SPCR)         | ttyAMA0  -W- (E    a) |
>>    |
> 
> The SPCR handling is inconsistent over architectures, see
> https://lkml.kernel.org/r/20180830123849.26163-1-prarit@redhat.com
> 
> IMHO, arm developers decided that consoles defined by SPCR are always
> enabled when existing.

I'm OK with those being enabled. Though, I hope "not registering tty0"
wasn't an explicit decision, but maybe an oversight/trade-off due to
assuming SPCR code will only run on servers without displays (where tty0
wouldn't matter). (I understand it might be too late to change that.)

So I'd want the 2nd column to be: tty0(EC) ttyAMA0(E) at best, and
ttyAMA0(EC) tty0(E) at worst.

> In 1st column: tty0 is the preferred console because it is defined
> on the commandline.
> 
> In 2nd column: tty0 is not enabled at all because another console was
> defined by SPCR. Note that ttySX and ttyX consoles are registered only
> as a fallback when there is no other console defined.
> 
> The following code is responsible for the fallback, see register_console()
> 
> 	/*
> 	 *	See if we want to use this console driver. If we
> 	 *	didn't select a console we take the first one
> 	 *	that registers here.
> 	 */
> 	if (!has_preferred) {
> 		if (newcon->index < 0)
> 			newcon->index = 0;
> 		if (newcon->setup == NULL ||
> 		    newcon->setup(newcon, NULL) == 0) {
> 			newcon->flags |= CON_ENABLED;
> 			if (newcon->device) {
> 				newcon->flags |= CON_CONSDEV;
> 				has_preferred = true;
> 			}
> 		}
> 	}
> 
> 
>>    ------------------+-----------------------+-----------------------+
>>    Chromebook Plus   | tty0     -WU (EC p  ) | ttyS2    -W- (EC p a) |
>>    (w/ stdout-path)  |                       | tty0     -WU (E     ) |
> 
> Hmm, of_console_check() explicitly ignores the console defined by
> stdout-path when there is a console on the commandline. This explains
> 1st column.
> 
> I am not sure about 2nd column. My guess is that ttyX consoles are
> tried first. tty0 is registered as a fallback because there is no
> other console at the moment. ttyS2 is tried later and it is
> registered because it is in stdout-patch and there is no console
> in the command line. It is somehow consistent with  CONFIG_VT_CONSOLE
> description.
> 
> Sadly, it is different logic than with SPCR :-(

I like the fact that this one has tty0. For example, Debian's installer
iterates over /proc/consoles and launches itself on all the consoles it
finds there, so it wouldn't launch on my chromebook's screen if tty0
wasn't included (just like it doesn't launch on a QEMU aarch64 VM's
framebuffer).

>>    ------------------+-----------------------+-----------------------+
>>    Chromebook Plus   | tty0     -WU (EC p  ) | tty0     -WU (EC p  ) |
>>    (w/o either)      |                       |                       |
>>    ------------------+-----------------------+-----------------------+
> 
> This variant is easy and everyone would probably expect this.

I think things run roughly in the following order (from what I can
decipher from kernel messages) and I think it matches your explanations:

|            ACPI SPCR            |      dt chosen stdout-path      |
+=================================+=================================+
| acpi_parse_spcr()               |                                 |
| -> add_preferred_console(uart0) |                                 |
|    (if not on x86)              |                                 |
+---------------------------------+---------------------------------+
|                        console_setup()                            |
|                        -> add_preferred_console(tty0)             |
|                           (if console=tty0)                       |
+---------------------------------+---------------------------------+
|                        register_console(vt)                       |
+---------------------------------+---------------------------------+
|                                 | of_console_check()              |
|                                 | -> add_preferred_console(uart2) |
|                                 |    (if no console arg)          |
+---------------------------------+---------------------------------+
|                        register_console(serial)                   |
+---------------------------------+---------------------------------+

> Regarding the description of CONFIG_VT_CONSOLE option. I am afraid
> that it was created and true only before SPCR and device tree support
> was introduced.

OK. Assuming these changes won't go any further, maybe I'll try
documenting the current behavior in relevant places.

> Now, it is really sad that SPCR and device tree have different
> behavior even across architectures. But I am afraid that we could
> not change it without breaking many setups.
> 
> The only common rules are:
> 
>     + The last console on the command line should always be the
>       preferred one when defined.
> 
>     + Consoles defined by the device (SPCR, device tree) are used
>       when there is no commandline.
> 
>     + ttyX or ttySX are used as a fallback when nothing else is defined.
> 
> 
> My suggestion is:
> 
>     + Fix SPCR setting or device tree of your device when the defaults
>       are not as expected.

Maybe I can get QEMU's SPCR use conditional on the existence a
framebuffer, and get distributions to remove stdout-path from certain
device-trees; but that would disable the serial console completely
(instead of having it enabled where tty0 is still preferred).

>     + Use command line to force your value when the defaults are not
>       as expected and you could not change them.

This works; but I'd have to know the machine's serial configuration in
advance to put it in the cmdline as "console=<serial> console=tty0", or
lose the serial console as in the above. (A "console=dt" like that
"console=spcr" patch you linked to would be useful here if it existed.)

Both seem imperfect in that sense, but tolerable.

> I am afraid that we could not fix your problem on the kernel side. It
> would broke other setups that depend on the existing behavior.
> 
> Best Regards,
> Petr

Thanks for the detailed reply.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 1/3] printk: Add function to set console to preferred console's driver
  2020-05-13  5:35   ` Sergey Senozhatsky
@ 2020-05-24 10:01     ` Daniel Vetter
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2020-05-24 10:01 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Alper Nebi Yasak, Greg Kroah-Hartman, Jiri Slaby, Petr Mladek,
	linux-serial, Steven Rostedt, Linux ARM,
	Linux Kernel Mailing List, Andrew Morton, Andy Shevchenko,
	Arvind Sankar, Benjamin Herrenschmidt, David S. Miller,
	Feng Tang

On Wed, May 13, 2020 at 7:35 AM Sergey Senozhatsky
<sergey.senozhatsky@gmail.com> wrote:
>
> On (20/04/30 19:14), Alper Nebi Yasak wrote:
> [..]
> > +int update_console_to_preferred(void)
> > +{
> > +     struct console_cmdline *c = NULL;
> > +     struct console *con = NULL;
> > +     struct console *tmp = NULL;
> > +
> > +     if (preferred_console >= 0)
> > +             c = &console_cmdline[preferred_console];
> > +
> > +     if (!c || !c->name[0])
> > +             return 0;
> > +
> > +     for_each_console(con) {
> > +             if (!con->next || !(con->next->flags & CON_ENABLED))
> > +                     continue;
> > +             if (strcmp(c->name, con->next->name) != 0)
> > +                     continue;
>
> This matches the consoles by exact name. Consoles can have aliases,
> but matching by alias is rather complex and it has some side effects.
>
> Let me Cc more people on this. VT has a console takeover logic,
> I wonder if we can extend the takeover code somehow.
>
> Daniel, any thoughts?

Apologies for late reply, but nope, no thoughts. I have some ideas for
the locking in the console subsystem, but that's just to untangle it
from gpu drivers as much as possible. Otherwise I'm trying to stay
away from it as far as I can :-)

Cheers, Daniel

>
> https://lore.kernel.org/lkml/20200430161438.17640-1-alpernebiyasak@gmail.com
>
>         -ss



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path
  2020-05-15 19:27   ` Alper Nebi Yasak
@ 2020-05-25 13:04     ` Petr Mladek
  0 siblings, 0 replies; 19+ messages in thread
From: Petr Mladek @ 2020-05-25 13:04 UTC (permalink / raw)
  To: Alper Nebi Yasak
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky, linux-serial,
	Steven Rostedt, linux-arm-kernel, linux-kernel, Andrew Morton,
	Andy Shevchenko, Arvind Sankar, Benjamin Herrenschmidt,
	Daniel Vetter, David S. Miller, Eric Biggers, Feng Tang,
	Grzegorz Halat, Lukas Wunner, Nicolas Pitre, Sam Ravnborg

On Fri 2020-05-15 22:27:02, Alper Nebi Yasak wrote:
> On 13/05/2020 17:37, Petr Mladek wrote:
> > On Thu 2020-04-30 19:14:34, Alper Nebi Yasak wrote:
> I think things run roughly in the following order (from what I can
> decipher from kernel messages) and I think it matches your explanations:
> 
> |            ACPI SPCR            |      dt chosen stdout-path      |
> +=================================+=================================+
> | acpi_parse_spcr()               |                                 |
> | -> add_preferred_console(uart0) |                                 |
> |    (if not on x86)              |                                 |
> +---------------------------------+---------------------------------+
> |                        console_setup()                            |
> |                        -> add_preferred_console(tty0)             |
> |                           (if console=tty0)                       |
> +---------------------------------+---------------------------------+
> |                        register_console(vt)                       |
> +---------------------------------+---------------------------------+
> |                                 | of_console_check()              |
> |                                 | -> add_preferred_console(uart2) |
> |                                 |    (if no console arg)          |
> +---------------------------------+---------------------------------+
> |                        register_console(serial)                   |
> +---------------------------------+---------------------------------+


I was first a bit confused by the above table. The order looks fine
but I was not sure about the indentation. I think that some more
details are needed to get the picture and context.

I see the following order in start_kernel():

1. Add spcr consoles: by acpi_parse_spcr() called from setup_arch().
2. Add and register early consoles: by parse_early_param()
3. Add normal consoles from command line: by parse_args()

4. Register tty console: by vty_init() called via long chain
   from fs_initcall(chr_dev_init). It seems to be init call
   in 5th round, see include/linux/init.h

5. Register other (serial) consoles are most likely registered from
   device_initcall() in 6th round, see include/linux/init.h.

The consoles defined by the device tree are not added directly.
Instead, the probe() callbacks checks whether such console is
selected in device tree by of_console_check() called from
uart_add_one_port().


> > My suggestion is:
> > 
> >     + Fix SPCR setting or device tree of your device when the defaults
> >       are not as expected.
> 
> Maybe I can get QEMU's SPCR use conditional on the existence a
> framebuffer, and get distributions to remove stdout-path from certain
> device-trees; but that would disable the serial console completely
> (instead of having it enabled where tty0 is still preferred).

I am afraid that this is a problem with many defaults. They might be
good enough for many people but others would want something else.

It might be acceptable to add consoles. But it might be a problem to
remove consoles or change the currently preferred one.

The only exception would be when most people are annoyed with the
current default. But this need to be discussed with people familiar
with the given architecture or device.


> >     + Use command line to force your value when the defaults are not
> >       as expected and you could not change them.
> 
> This works; but I'd have to know the machine's serial configuration in
> advance to put it in the cmdline as "console=<serial> console=tty0", or
> lose the serial console as in the above. (A "console=dt" like that
> "console=spcr" patch you linked to would be useful here if it existed.)

The generic parameters: console=tty, console=serial, console=dt, console=spcr
looks fine to me. IMHO, the only problem might be when a particular
serial console drive is not able to guess reasonable defaults for the
baud rate, etc.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2020-05-25 13:04 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-30 16:14 [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path Alper Nebi Yasak
2020-04-30 16:14 ` [RFC PATCH v2 1/3] printk: Add function to set console to preferred console's driver Alper Nebi Yasak
2020-04-30 16:46   ` Andy Shevchenko
2020-05-01  1:44   ` Sergey Senozhatsky
2020-05-01 11:48     ` Alper Nebi Yasak
2020-05-13  5:35   ` Sergey Senozhatsky
2020-05-24 10:01     ` Daniel Vetter
2020-04-30 16:14 ` [RFC PATCH v2 2/3] vt: Set as preferred console when a non-dummy backend is bound Alper Nebi Yasak
2020-04-30 16:14 ` [RFC PATCH v2 3/3] printk: Preset tty0 as a pseudo-preferred console Alper Nebi Yasak
2020-04-30 16:44 ` [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path Andy Shevchenko
2020-04-30 19:32   ` Alper Nebi Yasak
2020-05-01  1:30 ` Sergey Senozhatsky
2020-05-01 11:08   ` Alper Nebi Yasak
2020-05-01 13:16     ` Andy Shevchenko
2020-05-01 15:07       ` Alper Nebi Yasak
2020-05-13 14:37 ` Petr Mladek
2020-05-13 22:22   ` Benjamin Herrenschmidt
2020-05-15 19:27   ` Alper Nebi Yasak
2020-05-25 13:04     ` Petr Mladek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).