| Message ID | 20241210133448.3684593-3-tjarlama@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add a new command in kgdb for vmcoreinfo | expand |
On Tue, Dec 10, 2024 at 05:34:47AM -0800, Amal Raj T wrote: > From: Amal Raj T <amalrajt@meta.com> > > The current implementation of `poll_put_char` in the serial console driver > performs LF -> CRLF replacement, which can corrupt binary data. Since kdb > is the only user of `poll_put_char`, this patch moves the LF -> CRLF > replacement logic to kdb. This description only explains why it is safe to change uart_poll_put_char() but... > Link: https://lore.kernel.org/linux-debuggers/Zy093jVKPs9gSVx2@telecaster/ > > Signed-off-by: Amal Raj T <amalrajt@meta.com> > --- > drivers/tty/serial/serial_core.c | 4 ---- > kernel/debug/kdb/kdb_io.c | 2 ++ > 2 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c > index 74fa02b23772..ed18492b7f8f 100644 > --- a/drivers/tty/serial/serial_core.c > +++ b/drivers/tty/serial/serial_core.c > @@ -2142,8 +2142,6 @@ void uart_console_write(struct uart_port *port, const char *s, > unsigned int i; > > for (i = 0; i < count; i++, s++) { > - if (*s == '\n') > - putchar(port, '\r'); > putchar(port, *s); > } > } ... kgdb isn't the only user of uart_console_write() though, right? Daniel.
On Tue, Dec 10, 2024 at 3:16 PM Daniel Thompson <daniel@riscstar.com> wrote: > > On Tue, Dec 10, 2024 at 05:34:47AM -0800, Amal Raj T wrote: > > From: Amal Raj T <amalrajt@meta.com> > > > > The current implementation of `poll_put_char` in the serial console driver > > performs LF -> CRLF replacement, which can corrupt binary data. Since kdb > > is the only user of `poll_put_char`, this patch moves the LF -> CRLF > > replacement logic to kdb. > > This description only explains why it is safe to change > uart_poll_put_char() but... > > > > > Link: https://lore.kernel.org/linux-debuggers/Zy093jVKPs9gSVx2@telecaster/ > > > > Signed-off-by: Amal Raj T <amalrajt@meta.com> > > --- > > drivers/tty/serial/serial_core.c | 4 ---- > > kernel/debug/kdb/kdb_io.c | 2 ++ > > 2 files changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c > > index 74fa02b23772..ed18492b7f8f 100644 > > --- a/drivers/tty/serial/serial_core.c > > +++ b/drivers/tty/serial/serial_core.c > > @@ -2142,8 +2142,6 @@ void uart_console_write(struct uart_port *port, const char *s, > > unsigned int i; > > > > for (i = 0; i < count; i++, s++) { > > - if (*s == '\n') > > - putchar(port, '\r'); > > putchar(port, *s); > > } > > } > > ... kgdb isn't the only user of uart_console_write() though, right? Yes, the replacement should be only for uart_poll_put_char, fixed this in v2 > > > Daniel.
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 74fa02b23772..ed18492b7f8f 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -2142,8 +2142,6 @@ void uart_console_write(struct uart_port *port, const char *s, unsigned int i; for (i = 0; i < count; i++, s++) { - if (*s == '\n') - putchar(port, '\r'); putchar(port, *s); } } @@ -2738,8 +2736,6 @@ static void uart_poll_put_char(struct tty_driver *driver, int line, char ch) if (!port) return; - if (ch == '\n') - port->ops->poll_put_char(port, '\r'); port->ops->poll_put_char(port, ch); uart_port_deref(port); } diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c index 6a77f1c779c4..43a7c8ad741a 100644 --- a/kernel/debug/kdb/kdb_io.c +++ b/kernel/debug/kdb/kdb_io.c @@ -572,6 +572,8 @@ static void kdb_msg_write(const char *msg, int msg_len) len = msg_len; while (len--) { + if (*cp == '\n') + dbg_io_ops->write_char('\r'); dbg_io_ops->write_char(*cp); cp++; }