Re: [PATCH 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq()

[Ilpo Järvinen]

On Tue, 12 May 2026, Jacques Nilo wrote:

> serial8250_handle_irq() captures a SysRq character into port->sysrq_ch
> inside serial8250_handle_irq_locked() via uart_prepare_sysrq_char()
> (reached from serial8250_read_char()). Dispatch of that captured
> character to handle_sysrq() is expected to happen at port-unlock time,
> through uart_unlock_and_check_sysrq[_irqrestore]().
> 
> After commit 8324a54f604d ("serial: 8250: Add
> serial8250_handle_irq_locked()") the function was reduced to a wrapper
> that takes the port lock via guard(uart_port_lock_irqsave) whose
> destructor is plain uart_port_unlock_irqrestore(). The sysrq-aware
> unlock helper is no longer called, so port->sysrq_ch is captured but
> never dispatched: BREAK + SysRq key is consumed silently.
> 
> This was the very condition Johan Hovold's 853a9ae29e978 ("serial:
> 8250: fix handle_irq locking", 2021) introduced
> uart_unlock_and_check_sysrq_irqrestore() to address.
> 
> Switch to the new guard(uart_port_lock_sysrq_irqsave), whose destructor
> is the sysrq-aware unlock helper, restoring the pre-split behaviour.
> Update the Context: comment on serial8250_handle_irq_locked() so future
> HW-specific 8250 wrappers know to use the same guard or the explicit
> sysrq-aware unlock.
> 
> Verified on RTL8196E with CONFIG_MAGIC_SYSRQ_SERIAL=y: BREAK + 'h' on
> the console UART produces the SysRq help dump in dmesg and the brk
> counter in /proc/tty/driver/serial increments correctly.
> 
> Fixes: 8324a54f604d ("serial: 8250: Add serial8250_handle_irq_locked()")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jacques Nilo <jnilo@xxxxxxx>
> ---
>  drivers/tty/serial/8250/8250_port.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index e4e6a53eb..64f3487e8 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1786,7 +1786,10 @@ static bool handle_rx_dma(struct uart_8250_port *up, unsigned int iir)
>  }
>  
>  /*
> - * Context: port's lock must be held by the caller.
> + * Context: port's lock must be held by the caller. The caller must
> + * release it via guard(uart_port_lock_sysrq_irqsave) or
> + * uart_unlock_and_check_sysrq_irqrestore(), which captures SysRq
> + * character on unlock.
>   */
>  void serial8250_handle_irq_locked(struct uart_port *port, unsigned int iir)
>  {
> @@ -1839,7 +1842,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
>  	if (iir & UART_IIR_NO_INT)
>  		return 0;
>  
> -	guard(uart_port_lock_irqsave)(port);
> +	guard(uart_port_lock_sysrq_irqsave)(port);
>  	serial8250_handle_irq_locked(port, iir);
>  
>  	return 1;
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>

-- 
 i.