Re: kern/55286: Kernel messages cause loss of serial console input

[Andreas Gustafsson <gson%gson.org@localhost>]

The following reply was made to PR kern/55286; it has been noted by GNATS.

From: Andreas Gustafsson <gson%gson.org@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/55286: Kernel messages cause loss of serial console input
Date: Sat, 23 May 2020 21:09:22 +0300

 I had a closer look at the code, and the issue is definitely caused
 by the read-ahead code in com.c.  Quoting a comment in com.c:
 
  * The read-ahead code is so that you can detect pending in-band
  * cn_magic in polled mode while doing output rather than having to
  * wait until the kernel decides it needs input.
 
 So, if I understand this correctly, this code is intended to support a
 nonstandard way of breaking into DDB (as opposed to the standard break
 condition) and will only work if the kernel happens to be printing
 something in polled mode at the time (!).
 
 To me this seems like a bad idea from the start, and I would prefer to
 simply remove this code rather than try to fix it.  Or does someone
 think it actually has some value?
 
 I have confirmed that after disabling the read-ahead using the patch
 below, the manual test procedure in the PR no longer reproduces the
 problem.
 
 Index: src/sys/dev/ic/com.c
 ===================================================================
 RCS file: /bracket/repo/src/sys/dev/ic/com.c,v
 retrieving revision 1.358
 diff -u -r1.358 com.c
 --- src/sys/dev/ic/com.c	1 May 2020 07:27:51 -0000	1.358
 +++ src/sys/dev/ic/com.c	23 May 2020 12:33:23 -0000
 @@ -2402,17 +2402,18 @@
  com_common_putc(dev_t dev, struct com_regs *regsp, int c)
  {
  	int s = splserial();
 -	int cin, stat, timo;
 +	int timo;
  
 +#if 0
  	if (com_readaheadcount < MAX_READAHEAD
  	     && ISSET(stat = CSR_READ_1(regsp, COM_REG_LSR), LSR_RXRDY)) {
 -		int cn_trapped = 0;
 +		int cin, stat, cn_trapped = 0;
  		cin = CSR_READ_1(regsp, COM_REG_RXDATA);
  		stat = CSR_READ_1(regsp, COM_REG_IIR);
  		cn_check_magic(dev, cin, com_cnm_state);
  		com_readahead[com_readaheadcount++] = cin;
  	}
 -
 +#endif
  	/* wait for any pending transmission to finish */
  	timo = 150000;
  	while (!ISSET(CSR_READ_1(regsp, COM_REG_LSR), LSR_TXRDY) && --timo)
 
 -- 
 Andreas Gustafsson, gson%gson.org@localhost