[PATCH v2] x86/early_printk: add MMIO-based UARTs

Denis Mukhin via B4 Relay posted 1 patch 9 months, 1 week ago
There is a newer version of this series
Documentation/admin-guide/kernel-parameters.txt |  4 +++
arch/x86/kernel/early_printk.c                  | 42 ++++++++++++++++++++++++-
2 files changed, 45 insertions(+), 1 deletion(-)
[PATCH v2] x86/early_printk: add MMIO-based UARTs
Posted by Denis Mukhin via B4 Relay 9 months, 1 week ago
From: Denis Mukhin <dmukhin@ford.com>

During the bring-up of an x86 board, the kernel was crashing before
reaching the platform's console driver because of a bug in the firmware,
leaving no trace of the boot progress.

It was discovered that the only available method to debug the kernel
boot process was via the platform's MMIO-based UART, as the board lacked
an I/O port-based UART, PCI UART, or functional video output.

Then it turned out that earlyprintk= does not have a knob to configure
the MMIO-mapped UART.

Extend the early printk facility to support platform MMIO-based UARTs
on x86 systems, enabling debugging during the system bring-up phase.

The command line syntax to enable platform MMIO-based UART is:
  earlyprintk=mmio,membase[,{nocfg|baudrate}][,keep]

Note, the change does not integrate MMIO-based UART support to:
  arch/x86/boot/early_serial_console.c

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes in v2:
- Fixed description of early_mmio_serial_init()
- Link to v1: https://lore.kernel.org/r/20250313-earlyprintk-v1-1-8f818d77a8dd@ford.com
---
 Documentation/admin-guide/kernel-parameters.txt |  4 +++
 arch/x86/kernel/early_printk.c                  | 42 ++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index fb8752b42ec8582b8750d7e014c4d76166fa2fc1..bee9ee18a506d019dc3d330268e3e1c83434ebba 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1414,11 +1414,15 @@
 			earlyprintk=pciserial[,force],bus:device.function[,baudrate]
 			earlyprintk=xdbc[xhciController#]
 			earlyprintk=bios
+			earlyprintk=mmio,membase[,{nocfg|baudrate}][,keep]
 
 			earlyprintk is useful when the kernel crashes before
 			the normal console is initialized. It is not enabled by
 			default because it has some cosmetic problems.
 
+			Use "nocfg" to skip UART configuration, assume
+			BIOS/firmware has configured UART correctly.
+
 			Append ",keep" to not disable it when the real console
 			takes over.
 
diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index 44f937015e1e25bf41532eb7e1031a6be32a6523..068729a53e87cafd27ea9de1781887dff01d5710 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -191,7 +191,6 @@ static __init void early_serial_init(char *s)
 	early_serial_hw_init(divisor);
 }
 
-#ifdef CONFIG_PCI
 static void mem32_serial_out(unsigned long addr, int offset, int value)
 {
 	u32 __iomem *vaddr = (u32 __iomem *)addr;
@@ -206,6 +205,42 @@ static unsigned int mem32_serial_in(unsigned long addr, int offset)
 	return readl(vaddr + offset);
 }
 
+/*
+ * early_mmio_serial_init() - Initialize MMIO-based early serial console.
+ * @s: MMIO-based serial specification.
+ */
+static __init void early_mmio_serial_init(char *s)
+{
+	unsigned long baudrate;
+	unsigned long membase;
+	char *e;
+
+	if (*s == ',')
+		s++;
+
+	if (!strncmp(s, "0x", 2)) {
+		membase = simple_strtoul(s, &e, 16);
+		early_serial_base = (unsigned long)early_ioremap(membase, PAGE_SIZE);
+		serial_in = mem32_serial_in;
+		serial_out = mem32_serial_out;
+
+		s += strcspn(s, ",");
+		if (*s == ',')
+			s++;
+	}
+
+	if (!strncmp(s, "nocfg", 5))
+		baudrate = 0;
+	else {
+		baudrate = simple_strtoul(s, &e, 0);
+		if (baudrate == 0 || s == e)
+			baudrate = DEFAULT_BAUD;
+	}
+	if (baudrate)
+		early_serial_hw_init(115200 / baudrate);
+}
+
+#ifdef CONFIG_PCI
 /*
  * early_pci_serial_init()
  *
@@ -352,6 +387,11 @@ static int __init setup_early_printk(char *buf)
 	keep = (strstr(buf, "keep") != NULL);
 
 	while (*buf != '\0') {
+		if (!strncmp(buf, "mmio", 4)) {
+			early_mmio_serial_init(buf + 4);
+			early_console_register(&early_serial_console, keep);
+			buf += 4;
+		}
 		if (!strncmp(buf, "serial", 6)) {
 			buf += 6;
 			early_serial_init(buf);

---
base-commit: 8aed61b8334e00f4fe5de9f2df1cd183dc328a9d
change-id: 20250313-earlyprintk-f68bcf10febc

Best regards,
-- 
Denis Mukhin <dmukhin@ford.com>
Re: [PATCH v2] x86/early_printk: add MMIO-based UARTs
Posted by Ingo Molnar 9 months ago
* Denis Mukhin via B4 Relay <devnull+dmukhin.ford.com@kernel.org> wrote:

> +	if (!strncmp(s, "nocfg", 5))
> +		baudrate = 0;
> +	else {
> +		baudrate = simple_strtoul(s, &e, 0);
> +		if (baudrate == 0 || s == e)
> +			baudrate = DEFAULT_BAUD;
> +	}

In standard kernel coding style we always balance curly braces and 
don't skip them in the single-statement case. Ie. the above should be:

	if (!strncmp(s, "nocfg", 5)) {
		baudrate = 0;
	} else {


> +	if (baudrate)
> +		early_serial_hw_init(115200 / baudrate);

Hm, I think that division will go poorly if 'baudrate' ends up being 0 
in the 'nocfg' case ... ;-)

Thanks,

	Ingo
Re: [PATCH v2] x86/early_printk: add MMIO-based UARTs
Posted by Denis Mukhin 9 months ago
Hi,

Thanks for taking a look!

On Monday, March 17th, 2025 at 12:58 AM, Ingo Molnar <mingo@kernel.org> wrote:

>
>
>
> * Denis Mukhin via B4 Relay devnull+dmukhin.ford.com@kernel.org wrote:
>
> > + if (!strncmp(s, "nocfg", 5))
> > + baudrate = 0;
> > + else {
> > + baudrate = simple_strtoul(s, &e, 0);
> > + if (baudrate == 0 || s == e)
> > + baudrate = DEFAULT_BAUD;
> > + }
>
>
> In standard kernel coding style we always balance curly braces and
> don't skip them in the single-statement case. Ie. the above should be:
>
> if (!strncmp(s, "nocfg", 5)) {
> baudrate = 0;
> } else {
>
> > + if (baudrate)
> > + early_serial_hw_init(115200 / baudrate);
>
>
> Hm, I think that division will go poorly if 'baudrate' ends up being 0
> in the 'nocfg' case ... ;-)

This patch has a guardrail:
  early_serial_hw_init(115200 / baudrate);
will not be called in case of baudrate is 0.

I can re-write code to avoid confusion.

>
> Thanks,
>
> Ingo

Thanks,
Denis
Re: [PATCH v2] x86/early_printk: add MMIO-based UARTs
Posted by Ingo Molnar 9 months ago
* Denis Mukhin <dmkhn@proton.me> wrote:

> Hi,
> 
> Thanks for taking a look!
> 
> On Monday, March 17th, 2025 at 12:58 AM, Ingo Molnar <mingo@kernel.org> wrote:
> 
> >
> >
> >
> > * Denis Mukhin via B4 Relay devnull+dmukhin.ford.com@kernel.org wrote:
> >
> > > + if (!strncmp(s, "nocfg", 5))
> > > + baudrate = 0;
> > > + else {
> > > + baudrate = simple_strtoul(s, &e, 0);
> > > + if (baudrate == 0 || s == e)
> > > + baudrate = DEFAULT_BAUD;
> > > + }
> >
> >
> > In standard kernel coding style we always balance curly braces and
> > don't skip them in the single-statement case. Ie. the above should be:
> >
> > if (!strncmp(s, "nocfg", 5)) {
> > baudrate = 0;
> > } else {
> >
> > > + if (baudrate)
> > > + early_serial_hw_init(115200 / baudrate);
> >
> >
> > Hm, I think that division will go poorly if 'baudrate' ends up being 0
> > in the 'nocfg' case ... ;-)
> 
> This patch has a guardrail:
>   early_serial_hw_init(115200 / baudrate);
> will not be called in case of baudrate is 0.

Ugh, I must have had very limited reading comprehension that day :-/

> I can re-write code to avoid confusion.

No need to rewrite, the code is clear enough, it's my fault. :-)

But please do send -v3 with the curly braces fix, and merged against 
the latest x86 tree at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master

because there's a pending commit that creates a conflict:

  306859de59e5 ("x86/early_printk: Harden early_serial")

... while the conflict looks simple enough, it would be best to also 
test it, etc.

Thanks,

	Ingo