| Message ID | 20231216173409.1264655-1-michael@amarulasolutions.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tty: serial: kgdboc: Fix 8250_* kgd over serial | expand |
Hi Michael, kernel test robot noticed the following build errors: [auto build test ERROR on tty/tty-testing] [also build test ERROR on tty/tty-next tty/tty-linus linus/master v6.7-rc5 next-20231215] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Michael-Trimarchi/tty-serial-kgdboc-Fix-8250_-kgd-over-serial/20231217-013726 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing patch link: https://lore.kernel.org/r/20231216173409.1264655-1-michael%40amarulasolutions.com patch subject: [PATCH] tty: serial: kgdboc: Fix 8250_* kgd over serial config: i386-buildonly-randconfig-003-20231217 (https://download.01.org/0day-ci/archive/20231217/202312171302.vjOAqLOI-lkp@intel.com/config) compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231217/202312171302.vjOAqLOI-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202312171302.vjOAqLOI-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/tty/serial/serial_core.c:2636:26: error: expression is not assignable if (!port || port->type = PORT_UNKNOWN || !(port->ops->poll_get_char && port->ops->poll_put_char)) { ~~~~~~~~~~~~~~~~~~~ ^ 1 error generated. vim +2636 drivers/tty/serial/serial_core.c 2618 2619 static int uart_poll_init(struct tty_driver *driver, int line, char *options) 2620 { 2621 struct uart_driver *drv = driver->driver_state; 2622 struct uart_state *state = drv->state + line; 2623 enum uart_pm_state pm_state; 2624 struct tty_port *tport; 2625 struct uart_port *port; 2626 int baud = 9600; 2627 int bits = 8; 2628 int parity = 'n'; 2629 int flow = 'n'; 2630 int ret = 0; 2631 2632 tport = &state->port; 2633 mutex_lock(&tport->mutex); 2634 2635 port = uart_port_check(state); > 2636 if (!port || port->type = PORT_UNKNOWN || !(port->ops->poll_get_char && port->ops->poll_put_char)) { 2637 ret = -1; 2638 goto out; 2639 } 2640 2641 pm_state = state->pm_state; 2642 uart_change_pm(state, UART_PM_STATE_ON); 2643 2644 if (port->ops->poll_init) { 2645 /* 2646 * We don't set initialized as we only initialized the hw, 2647 * e.g. state->xmit is still uninitialized. 2648 */ 2649 if (!tty_port_initialized(tport)) 2650 ret = port->ops->poll_init(port); 2651 } 2652 2653 if (!ret && options) { 2654 uart_parse_options(options, &baud, &parity, &bits, &flow); 2655 console_list_lock(); 2656 ret = uart_set_options(port, NULL, baud, parity, bits, flow); 2657 console_list_unlock(); 2658 } 2659 out: 2660 if (ret) 2661 uart_change_pm(state, pm_state); 2662 mutex_unlock(&tport->mutex); 2663 return ret; 2664 } 2665
Hi, On Sun, Dec 17, 2023 at 11:34 PM Michael Trimarchi <michael@amarulasolutions.com> wrote: > > Check if port type is not PORT_UNKNOWN during poll_init. > The kgdboc calls the tty_find_polling_driver that check > if the serial is able to use poll_init. The poll_init calls > the uart uart_poll_init that try to configure the uart with the > selected boot parameters. The uart must be ready before setting > parameters. Seems that PORT_UNKNOWN is already used by other > functions in serial_core to detect uart status, so use the same > to avoid to use it in invalid state. > > The crash happen for instance in am62x architecture where the 8250 > register the platform driver after the 8250 core is initialized. > > Follow the report crash coming from KGDB > > Thread 2 received signal SIGSEGV, Segmentation fault. > [Switching to Thread 1] > _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584 > 584 __raw_writeb(value, PCI_IOBASE + addr); > (gdb) bt > > This section of the code is too early because in this case > the omap serial is not probed > > Thread 2 received signal SIGSEGV, Segmentation fault. > [Switching to Thread 1] > _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584 > 584 __raw_writeb(value, PCI_IOBASE + addr); > (gdb) bt > > Thread 2 received signal SIGSEGV, Segmentation fault. > [Switching to Thread 1] > _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584 > 584 __raw_writeb(value, PCI_IOBASE + addr); > (gdb) bt > 0 _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584 > 1 logic_outb (value=0 '\000', addr=18446739675637874689) at lib/logic_pio.c:299 > 2 0xffff80008082dfcc in io_serial_out (p=0x0, offset=16760830, value=0) at drivers/tty/serial/8250/8250_port.c:416 > 3 0xffff80008082fe34 in serial_port_out (value=<optimized out>, offset=<optimized out>, up=<optimized out>) > at ./include/linux/serial_core.h:677 > 4 serial8250_do_set_termios (port=0xffff8000828ee940 <serial8250_ports+1568>, termios=0xffff80008292b93c, old=0x0) > at drivers/tty/serial/8250/8250_port.c:2860 > 5 0xffff800080830064 in serial8250_set_termios (port=0xfffffbfffe800000, termios=0xffbffe, old=0x0) > at drivers/tty/serial/8250/8250_port.c:2912 > 6 0xffff80008082571c in uart_set_options (port=0xffff8000828ee940 <serial8250_ports+1568>, co=0x0, baud=115200, parity=110, bits=8, flow=110) > at drivers/tty/serial/serial_core.c:2285 > 7 0xffff800080828434 in uart_poll_init (driver=0xfffffbfffe800000, line=16760830, options=0xffff8000828f7506 <config+6> "115200n8") > at drivers/tty/serial/serial_core.c:2656 > 8 0xffff800080801690 in tty_find_polling_driver (name=0xffff8000828f7500 <config> "ttyS2,115200n8", line=0xffff80008292ba90) > at drivers/tty/tty_io.c:410 > 9 0xffff80008086c0b0 in configure_kgdboc () at drivers/tty/serial/kgdboc.c:194 > 10 0xffff80008086c1ec in kgdboc_probe (pdev=0xfffffbfffe800000) at drivers/tty/serial/kgdboc.c:249 > 11 0xffff8000808b399c in platform_probe (_dev=0xffff000000ebb810) at drivers/base/platform.c:1404 > 12 0xffff8000808b0b44 in call_driver_probe (drv=<optimized out>, dev=<optimized out>) at drivers/base/dd.c:579 > 13 really_probe (dev=0xffff000000ebb810, drv=0xffff80008277f138 <kgdboc_platform_driver+48>) at drivers/base/dd.c:658 > 14 0xffff8000808b0d2c in __driver_probe_device (drv=0xffff80008277f138 <kgdboc_platform_driver+48>, dev=0xffff000000ebb810) > at drivers/base/dd.c:800 > 15 0xffff8000808b0eb8 in driver_probe_device (drv=0xfffffbfffe800000, dev=0xffff000000ebb810) at drivers/base/dd.c:830 > 16 0xffff8000808b0ff4 in __device_attach_driver (drv=0xffff80008277f138 <kgdboc_platform_driver+48>, _data=0xffff80008292bc48) > at drivers/base/dd.c:958 > 17 0xffff8000808ae970 in bus_for_each_drv (bus=0xfffffbfffe800000, start=0x0, data=0xffff80008292bc48, > fn=0xffff8000808b0f3c <__device_attach_driver>) at drivers/base/bus.c:457 > 18 0xffff8000808b1408 in __device_attach (dev=0xffff000000ebb810, allow_async=true) at drivers/base/dd.c:1030 > 19 0xffff8000808b16d8 in device_initial_probe (dev=0xfffffbfffe800000) at drivers/base/dd.c:1079 > 20 0xffff8000808af9f4 in bus_probe_device (dev=0xffff000000ebb810) at drivers/base/bus.c:532 > 21 0xffff8000808ac77c in device_add (dev=0xfffffbfffe800000) at drivers/base/core.c:3625 > 22 0xffff8000808b3428 in platform_device_add (pdev=0xffff000000ebb800) at drivers/base/platform.c:716 > 23 0xffff800081b5dc0c in init_kgdboc () at drivers/tty/serial/kgdboc.c:292 > 24 0xffff800080014db0 in do_one_initcall (fn=0xffff800081b5dba4 <init_kgdboc>) at init/main.c:1236 > 25 0xffff800081b0114c in do_initcall_level (command_line=<optimized out>, level=<optimized out>) at init/main.c:1298 > 26 do_initcalls () at init/main.c:1314 > 27 do_basic_setup () at init/main.c:1333 > 28 kernel_init_freeable () at init/main.c:1551 > 29 0xffff8000810271ec in kernel_init (unused=0xfffffbfffe800000) at init/main.c:1441 > 30 0xffff800080015e80 in ret_from_fork () at arch/arm64/kernel/entry.S:857 > > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> > --- > v1 -> v2: > - fix if condition during submission > - improve a bit the commit message > RFC -> v1: > - refuse uart that has type PORT_UNKNOWN > > --- > drivers/tty/serial/serial_core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I'm not a total expert on this code, but this seems reasonable to me. One nit is ${SUBJECT} should probably be "kgdb over serial" instead of "kgd over serial" Reviewed-by: Douglas Anderson <dianders@chromium.org> > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c > index f1348a509552..9b7ed4aac77a 100644 > --- a/drivers/tty/serial/serial_core.c > +++ b/drivers/tty/serial/serial_core.c > @@ -2633,7 +2633,7 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options) > mutex_lock(&tport->mutex); > > port = uart_port_check(state); > - if (!port || !(port->ops->poll_get_char && port->ops->poll_put_char)) { > + if (!port || port->type == PORT_UNKNOWN || !(port->ops->poll_get_char && port->ops->poll_put_char)) { Another slight nit is that the above line feels a little long, clocking in at 110 columns. I know the 80 column limit isn't so firm these days, but if it were me I'd split it across 2 lines. -Doug
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index f1348a509552..aa07eb894a6e 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -2633,7 +2633,7 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options) mutex_lock(&tport->mutex); port = uart_port_check(state); - if (!port || !(port->ops->poll_get_char && port->ops->poll_put_char)) { + if (!port || port->type = PORT_UNKNOWN || !(port->ops->poll_get_char && port->ops->poll_put_char)) { ret = -1; goto out; }
Check if port type is not PORT_UNKNOWN in the serial driver. The kgdboc calls the tty_find_polling_driver that check if the serial is able to use poll_init. The poll_init calls the uart uart_poll_init that try to configure the uart with the selected parameters. The uart must be ready and we can check it using type as in other tty_io functions. The crash happen for instance in am62x architecture where the 8250 register the platform driver after the 8250 core is initialized. As shown by kdbg the iobase and membase is not configured Thread 2 received signal SIGSEGV, Segmentation fault. [Switching to Thread 1] _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584 584 __raw_writeb(value, PCI_IOBASE + addr); (gdb) bt This section of the code is too early because in this case the omap serial is not probed Thread 2 received signal SIGSEGV, Segmentation fault. [Switching to Thread 1] _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584 584 __raw_writeb(value, PCI_IOBASE + addr); (gdb) bt Thread 2 received signal SIGSEGV, Segmentation fault. [Switching to Thread 1] _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584 584 __raw_writeb(value, PCI_IOBASE + addr); (gdb) bt 0 _outb (addr=<optimized out>, value=<optimized out>) at ./include/asm-generic/io.h:584 1 logic_outb (value=0 '\000', addr=18446739675637874689) at lib/logic_pio.c:299 2 0xffff80008082dfcc in io_serial_out (p=0x0, offset=16760830, value=0) at drivers/tty/serial/8250/8250_port.c:416 3 0xffff80008082fe34 in serial_port_out (value=<optimized out>, offset=<optimized out>, up=<optimized out>) at ./include/linux/serial_core.h:677 4 serial8250_do_set_termios (port=0xffff8000828ee940 <serial8250_ports+1568>, termios=0xffff80008292b93c, old=0x0) at drivers/tty/serial/8250/8250_port.c:2860 5 0xffff800080830064 in serial8250_set_termios (port=0xfffffbfffe800000, termios=0xffbffe, old=0x0) at drivers/tty/serial/8250/8250_port.c:2912 6 0xffff80008082571c in uart_set_options (port=0xffff8000828ee940 <serial8250_ports+1568>, co=0x0, baud=115200, parity=110, bits=8, flow=110) at drivers/tty/serial/serial_core.c:2285 7 0xffff800080828434 in uart_poll_init (driver=0xfffffbfffe800000, line=16760830, options=0xffff8000828f7506 <config+6> "115200n8") at drivers/tty/serial/serial_core.c:2656 8 0xffff800080801690 in tty_find_polling_driver (name=0xffff8000828f7500 <config> "ttyS2,115200n8", line=0xffff80008292ba90) at drivers/tty/tty_io.c:410 9 0xffff80008086c0b0 in configure_kgdboc () at drivers/tty/serial/kgdboc.c:194 10 0xffff80008086c1ec in kgdboc_probe (pdev=0xfffffbfffe800000) at drivers/tty/serial/kgdboc.c:249 11 0xffff8000808b399c in platform_probe (_dev=0xffff000000ebb810) at drivers/base/platform.c:1404 12 0xffff8000808b0b44 in call_driver_probe (drv=<optimized out>, dev=<optimized out>) at drivers/base/dd.c:579 13 really_probe (dev=0xffff000000ebb810, drv=0xffff80008277f138 <kgdboc_platform_driver+48>) at drivers/base/dd.c:658 14 0xffff8000808b0d2c in __driver_probe_device (drv=0xffff80008277f138 <kgdboc_platform_driver+48>, dev=0xffff000000ebb810) at drivers/base/dd.c:800 15 0xffff8000808b0eb8 in driver_probe_device (drv=0xfffffbfffe800000, dev=0xffff000000ebb810) at drivers/base/dd.c:830 16 0xffff8000808b0ff4 in __device_attach_driver (drv=0xffff80008277f138 <kgdboc_platform_driver+48>, _data=0xffff80008292bc48) at drivers/base/dd.c:958 17 0xffff8000808ae970 in bus_for_each_drv (bus=0xfffffbfffe800000, start=0x0, data=0xffff80008292bc48, fn=0xffff8000808b0f3c <__device_attach_driver>) at drivers/base/bus.c:457 18 0xffff8000808b1408 in __device_attach (dev=0xffff000000ebb810, allow_async=true) at drivers/base/dd.c:1030 19 0xffff8000808b16d8 in device_initial_probe (dev=0xfffffbfffe800000) at drivers/base/dd.c:1079 20 0xffff8000808af9f4 in bus_probe_device (dev=0xffff000000ebb810) at drivers/base/bus.c:532 21 0xffff8000808ac77c in device_add (dev=0xfffffbfffe800000) at drivers/base/core.c:3625 22 0xffff8000808b3428 in platform_device_add (pdev=0xffff000000ebb800) at drivers/base/platform.c:716 23 0xffff800081b5dc0c in init_kgdboc () at drivers/tty/serial/kgdboc.c:292 24 0xffff800080014db0 in do_one_initcall (fn=0xffff800081b5dba4 <init_kgdboc>) at init/main.c:1236 25 0xffff800081b0114c in do_initcall_level (command_line=<optimized out>, level=<optimized out>) at init/main.c:1298 26 do_initcalls () at init/main.c:1314 27 do_basic_setup () at init/main.c:1333 28 kernel_init_freeable () at init/main.c:1551 29 0xffff8000810271ec in kernel_init (unused=0xfffffbfffe800000) at init/main.c:1441 30 0xffff800080015e80 in ret_from_fork () at arch/arm64/kernel/entry.S:857 Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> --- RFC -> v1: - refuse uart that has type PORT_UNKNOWN --- drivers/tty/serial/serial_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)