RE: [PATCH 1/2] printk: nbcon: move locked_port flag to struct uart_port
From: Chang, Junxiao
Date: Wed Jan 24 2024 - 20:08:45 EST
> > > Console pointer in uart_port might be shared among multiple uart
> > > ports.
> >
> > I still want to investigate why the pointer is shared. This sounds
> > sloppy or dangerous.
> I have x86 a server box and PNP enumerates two UARTs (8250). Only one is wired up but both can be specified as console=.
> What do I need to do to reproduce this here? Using console= twice does not do the trick.
Issue could be reproduced with our hardware every time. My cmdline is: BOOT_IMAGE=(hd0,gpt2)/boot/bzImage-linux-intel-iot-lts-rt-6.6-kernel root=PARTLABEL=primary rootwait console=ttyS0,115200 console=tty0 init=/sbin/preinit-env console=ttyS4,115200n8 console=ttyS5,115200n8
If you would like to try any debug patch with my ADL hardware, please feel free to let me know.
For console pointer sharing issue, from code logic point of view, the call chain looks like:
serial8250_register_8250_port -> uart_add_one_port -> serial_ctrl_register_port -> serial_core_register_port -> serial_core_add_one_port
In API serial_core_add_one_port, uart_port's console pointer is assigned with driver's console pointer:
uport->cons = drv->cons;
Driver's console pointer points to static structure "univ8250_console" which is defined in 8250_core.c
That is, all 8250 serial devices' console pointer are same, they point to univ8250_console.
Below is debug log output:
sh-5.1# dmesg | grep @@@@
[ 1.687121] @@@@ univ8250_console_init univ8250_console address:ffffffff935df1e0
[ 3.419880] @@@@ serial8250_register_8250_port: name:ttyS4, cons pointer:ffffffff935df1e0
[ 3.534954] @@@@ serial8250_register_8250_port: name:ttyS5, cons pointer:ffffffff935df1e0
[ 11.971345] @@@@ serial8250_do_shutdown, curr thread:(agetty), name:ttyS0, line 0, cons pointer:ffffffff935df1e0
[ 11.971506] @@@@ serial8250_do_shutdown, curr thread:(agetty), name:ttyS4, line 4, cons pointer:ffffffff935df1e0
[ 11.971849] @@@@ serial8250_do_shutdown, curr thread:(agetty), name:ttyS4, line 4, cons pointer:ffffffff935df1e0
[ 11.983072] @@@@ serial8250_do_shutdown, curr thread:agetty, name:ttyS4, line 4, cons pointer:ffffffff935df1e0
[ 11.983595] @@@@ serial8250_do_shutdown, curr thread:(agetty), name:ttyS5, line 5, cons pointer:ffffffff935df1e0
[ 11.983895] @@@@ serial8250_do_shutdown, curr thread:(agetty), name:ttyS5, line 5, cons pointer:ffffffff935df1e0
[ 12.009632] @@@@ serial8250_do_shutdown, curr thread:agetty, name:ttyS5, line 5, cons pointer:ffffffff935df1e0
sh-5.1#
All cons pointers address are same "ffffffff935df1e0".
Debug patch:
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 30434718fad80..740fd7e133a28 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -755,6 +755,7 @@ static int __init univ8250_console_init(void)
serial8250_isa_init_ports();
register_console(&univ8250_console);
+ printk("@@@@ %s univ8250_console address:%lx\n", __func__, (unsigned long)(&univ8250_console));
return 0;
}
console_initcall(univ8250_console_init);
@@ -1181,6 +1182,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
if (ret)
goto err;
+ printk("@@@@ %s: name:%s, cons pointer:%lx\n", __func__, uart->port.name, (unsigned long)uart->port.cons);
ret = uart->port.line;
} else {
dev_info(uart->port.dev,
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 437a7d3d34cde..93f1e548d1301 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2510,6 +2510,7 @@ void serial8250_do_shutdown(struct uart_port *port)
struct uart_8250_port *up = up_to_u8250p(port);
unsigned long flags;
+ printk("@@@@ %s, curr thread:%s, name:%s, line %d, cons pointer:%lx\n", __func__, current->comm, port->name, port->line, (unsigned long)port->cons);
serial8250_rpm_get(up);
/*
* Disable interrupts from this port
Thanks,
Junxiao