On Sunday 19 March 2023 16:25:25 Tony Dinh wrote: > Hi Pali, > > On Sun, Mar 19, 2023 at 12:36 PM Pali Rohár <p...@kernel.org> wrote: > > > > On Saturday 18 March 2023 14:46:25 Tony Dinh wrote: > > > - When Netconsole is running, stdin/stdout/stderr are set to nc. Reset > > > stdin/stdout/stderr to serial (a sane deffault) before booting kernel. > > > > This can be a problematic. serial output does not have to be available > > for all devices. For example on Nokia N900 phone is available only > > lcd/vga display device. > > Here is a shortcoming of the current implementation of netconsole. > When it starts, the user sets the stdin/stdout/stderr envs to nc, and > we lose the previous state of the console. Especially with the > CONSOLE_MUX is not enabled. There is no way to set them back by > setting the envs in booting logic.
Cannot user use console mux and set stdout to both serial and nc at the same time? > It sounds like we need to implement internal envs to save the previous > state, and then restore it before shutting down the network and > booting the kernel. > I recall in previous Linux kernel versions many years ago Linux still > booted OK, but recently it can no longer boot with the stdio set to nc > (I've tested this failure case). Perhaps nc should _not_ be set > explicitly by the user. We might need a u-boot command to start > netconsole, and that would include checking if the serverip is > running? > > > > > Also this can break CONSOLE_MUX support when more devices are specified > > in stdin/stdout/stderr env variables. With CONSOLE_MUX, output is send > > to more than one device. User may set it to both serial and nc or also > > to other output (e.g. to lcd like on Nokia N900). > > > > So in my opinion, if "nc" is is going to be turned off then just "nc" > > string should be removed from env variable and let all other devices > > stay in env variables. > > > > Maybe you can use something like this? (taken from common/usb_kbd.c) > > > > #if CONFIG_IS_ENABLED(CONSOLE_MUX) > > if (iomux_replace_device(stdin, "nc", "nulldev")) > > return 1; > > #endif > > Thanks for pointing out that fact about CONSOLE_MUX. Yes I will > incorporate that as part of the logic. > > All the best, > Tony > > > > > > - Enable net_timeout when netconsole starts will give a better user > > > experience if netconsole server is not running. > > > > > > Signed-off-by: Tony Dinh <mibo...@gmail.com> > > > --- > > > > > > boot/bootm.c | 16 +++++++++++++++- > > > drivers/net/netconsole.c | 2 +- > > > 2 files changed, 16 insertions(+), 2 deletions(-) > > > > > > diff --git a/boot/bootm.c b/boot/bootm.c > > > index 2eec60ec7b..c4a3aaf1bd 100644 > > > --- a/boot/bootm.c > > > +++ b/boot/bootm.c > > > @@ -473,7 +473,21 @@ ulong bootm_disable_interrupts(void) > > > */ > > > iflag = disable_interrupts(); > > > #ifdef CONFIG_NETCONSOLE > > > - /* Stop the ethernet stack if NetConsole could have left it up */ > > > + /* > > > + * Make sure that the starting kernel message printed out. > > > + * Reset stdin/out/err back to serial and stop the ethernet > > > + * stack if NetConsole could have left it up > > > + */ > > > + char *s; > > > + int ret; > > > + > > > + s = env_get("stdout"); > > > + if (strcmp(s, "nc") == 0) { > > > + printf("\n\nStarting kernel ...\n"); > > > + ret = env_set("stdin", "serial"); > > > + ret = env_set("stdout", "serial"); > > > + ret = env_set("stderr", "serial"); > > > + } > > > eth_halt(); > > > #endif > > > > > > diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c > > > index 151bc55e07..2091014918 100644 > > > --- a/drivers/net/netconsole.c > > > +++ b/drivers/net/netconsole.c > > > @@ -20,7 +20,7 @@ static int input_size; /* char count in input buffer */ > > > static int input_offset; /* offset to valid chars in input buffer */ > > > static int input_recursion; > > > static int output_recursion; > > > -static int net_timeout; > > > +static int net_timeout = 1; > > > static uchar nc_ether[6]; /* server enet address */ > > > static struct in_addr nc_ip; /* server ip */ > > > static short nc_out_port; /* target output port */ > > > -- > > > 2.30.2 > > >