All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Ogness <john.ogness@linutronix.de>
To: Yunhui Cui <cuiyunhui@bytedance.com>,
	gregkh@linuxfoundation.org, jirislaby@kernel.org,
	pmladek@suse.com, arnd@arndb.de,
	andriy.shevchenko@linux.intel.com, namcao@linutronix.de,
	benjamin.larsson@genexis.eu, schnelle@linux.ibm.com,
	matt.porter@linaro.org, tim.kryger@linaro.org,
	markus.mayer@linaro.org, heikki.krogerus@linux.intel.com,
	linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org
Cc: Yunhui Cui <cuiyunhui@bytedance.com>
Subject: Re: [PATCH v2] serial: 8250: fix panic due to PSLVERR
Date: Tue, 08 Apr 2025 14:34:24 +0206	[thread overview]
Message-ID: <84bjt6rg5z.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <20250408061200.76148-1-cuiyunhui@bytedance.com>

On 2025-04-08, Yunhui Cui <cuiyunhui@bytedance.com> wrote:
> When the PSLVERR_RESP_EN parameter is set to 1, the device generates
> an error response if an attempt is made to read an empty RBR (Receive
> Buffer Register) while the FIFO is enabled.
>
> In serial8250_do_startup(), calling serial_port_out(port, UART_LCR,
> UART_LCR_WLEN8) triggers dw8250_check_lcr(), which invokes
> dw8250_force_idle() and serial8250_clear_and_reinit_fifos(). The latter
> function enables the FIFO via serial_out(p, UART_FCR, p->fcr).
> Execution proceeds to the dont_test_tx_en label:
> ...
> serial_port_in(port, UART_RX);
> This satisfies the PSLVERR trigger condition.
>
> Because another CPU(e.g., using printk()) is accessing the UART (UART
> is busy), the current CPU fails the check (value & ~UART_LCR_SPAR) ==
> (lcr & ~UART_LCR_SPAR), causing it to enter dw8250_force_idle().
>
> To resolve this issue, relevant serial_port_out() operations should be
> placed in a critical section, and UART_RX data should only be read
> when the UART_LSR DR bit is set.

The UART_LSR_DR check still has a race condition if the console is in
RS485 mode and !SER_RS485_RX_DURING_TX. It seems DW supports this mode
as there is code in dw8250_rs485_config() that sets DW_UART_TCR
differently for this.

In this mode, serial8250_console_write() will call the callback
->rs485_stop_tx(), which for DW is serial8250_em485_stop_tx(). And this
calls serial8250_clear_and_reinit_fifos().

To really close this race, all UART_RX reads would need to be under the
port lock. Most of them already are.

John Ogness

  parent reply	other threads:[~2025-04-08 12:28 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-08  6:12 [PATCH v2] serial: 8250: fix panic due to PSLVERR Yunhui Cui
2025-04-08  6:23 ` Greg KH
2025-04-08  6:23 ` Greg KH
2025-04-08 12:28 ` John Ogness [this message]
2025-04-10 12:00   ` [External] " yunhui cui

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=84bjt6rg5z.fsf@jogness.linutronix.de \
    --to=john.ogness@linutronix.de \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=arnd@arndb.de \
    --cc=benjamin.larsson@genexis.eu \
    --cc=cuiyunhui@bytedance.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=markus.mayer@linaro.org \
    --cc=matt.porter@linaro.org \
    --cc=namcao@linutronix.de \
    --cc=pmladek@suse.com \
    --cc=schnelle@linux.ibm.com \
    --cc=tim.kryger@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.