linux-kernel - Re: [PATCH 6/6] serial: 8250_dw: Ensure BUSY is deasserted

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aXjHZQnIFjfPabdU@smile.fi.intel.com>
Date: Tue, 27 Jan 2026 16:10:45 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	Petr Mladek <pmladek@...e.com>,
	John Ogness <john.ogness@...utronix.de>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jirislaby@...nel.org>,
	linux-serial <linux-serial@...r.kernel.org>,
	qianfan Zhao <qianfanguijin@....com>,
	Adriana Nicolae <adriana@...sta.com>,
	Markus Mayer <markus.mayer@...aro.org>,
	Tim Kryger <tim.kryger@...aro.org>,
	Matt Porter <matt.porter@...aro.org>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	Jamie Iles <jamie@...ieiles.com>,
	LKML <linux-kernel@...r.kernel.org>, stable@...r.kernel.org,
	"Bandal, Shankar" <shankar.bandal@...el.com>,
	"Murthy, Shanth" <shanth.murthy@...el.com>
Subject: Re: [PATCH 6/6] serial: 8250_dw: Ensure BUSY is deasserted

On Tue, Jan 27, 2026 at 03:35:27PM +0200, Ilpo Järvinen wrote:
> On Sat, 24 Jan 2026, Andy Shevchenko wrote:
> > On Fri, Jan 23, 2026 at 07:27:39PM +0200, Ilpo Järvinen wrote:

+Cc: printk people to check on printing from a serial driver routines.

...

> > > +	/* Prevent triggering interrupt from RBR filling */
> > > +	p->serial_out(p, UART_IER, 0);
> > 
> > Do we specifically use callbacks directly and not wrappers all over the change?
> 
> I guess it's just a habit, I suppose you meant using serial_port_in/out 
> instead. I can try to change those.

Not (only) me. Jiri updated this driver (and many others) to use callbacks.
That's why I added comments here and there about possible recursions.

...

> > > +	serial8250_fifo_wait_for_lsr_thre(up, p->fifosize);
> > > +	ndelay(p->frame_time);
> > 
> > Wouldn't be a problem on lowest baud rates (exempli gratia 110)?
> 
> Perhaps, but until somebody comes with an issue report related to 110, I'm 
> wondering if this really is worth trying to address. Any suggestion how is 
> welcome as well?

Polling work? Timer?

> > > +	retries = 4;	/* Arbitrary limit, 2 was always enough in tests */
> > > +	do {
> > > +		serial8250_clear_fifos(up);
> > > +		if (!(p->serial_in(p, usr_reg) & DW_UART_USR_BUSY))
> > > +			break;
> > > +		ndelay(p->frame_time);
> > > +	} while (--retries);
> > 
> > read_poll_timeout_atomic() ? I assume it can't be used due to small frame time?
> 
> Frame time is in nanoseconds yes. I did consider 
> read_poll_timeout_atomic() but it would have required nsec -> usec 
> conversion so I left this as it is.

Yeah with the same issue on low baud rates. So far I think we need to consider
9600 as commonly used by the old HW (which may be connected to a modern PC with
this new kernel running), so the frame time sounds like close to a millisecond.
And this can be met in real life.

Maybe put TODO/FIXME around these ndelay() calls?

> > > +	if (d->in_idle) {
> > 
> > > +		/*
> > > +		 * FIXME: this deadlocks if port->lock is already held
> > > +		 * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
> > > +		 */
> > 
> > Hmm... That FIXME should gone since we have non-blocking consoles, no?
> 
> No, lockdep still gets angry if printing is used while holding port's 
> lock.

Hmm... Let's ask PRINTK people about this. John, do we still have a gap
with nbcon? Or did I misunderstand the scope of its use?

> What would be possible though, is to mark the port's lock critical section 
> for print deferral (but it's outside the scope of this series). In case of 
> serial, it would be justified to use deferred printing (which is only 
> meant for special cases) because serial console and printing are related.
> 
> > > +		return;
> > > +	}

-- 
With Best Regards,
Andy Shevchenko