On Thu, Mar 12, 2026 at 11:31:59AM +0800, [email protected] wrote:
> From: Frank Chang <[email protected]>
> 
> Currently, the txwm interrupt pending status is only updated when the
> asynchronous transmit handler runs. This can cause the txwm interrupt
> state to become unsynchronized between the SiFive UART and the
> interrupt controller.
> 
> For example, when a txwm interrupt is raised, the corresponding APLIC
> pending bit is also set. However, if software later enqueues additional
> characters into the TX FIFO exceeding the transmit watermark, the
> APLIC pending bit may remain set because the txwm interrupt pending
> status is not updated at enqueue time.
> 
> This issue has been observed on resource-constrained machines, where
> Linux reports spurious IRQ errors. In these cases, the asynchronous
> transmit handler is unable to drain the TX FIFO quickly enough to update
> the txwm pending status before software reads the ip register, which
> derives the txwm pending state directly from the actual number of
> characters in the TX FIFO.
> 
> This commit fixes the issue by updating the txwm interrupt pending
> status immediately after enqueuing data into the TX FIFO, ensuring that
> the interrupt pending status between the SiFive UART and the interrupt
> controller remains synchronized.
> 
> Signed-off-by: Frank Chang <[email protected]>
> ---
>  hw/char/sifive_uart.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
> index 3ce6a4ee76a..ae71a15a2a4 100644
> --- a/hw/char/sifive_uart.c
> +++ b/hw/char/sifive_uart.c
> @@ -124,12 +124,20 @@ static void sifive_uart_trigger_tx_fifo(SiFiveUARTState 
> *s)
>  static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
>                                        int size)
>  {
> +    uint32_t txcnt = SIFIVE_UART_GET_TXCNT(s->txctrl);
> +    bool update_irq = false;
> +
>      if (size > fifo8_num_free(&s->tx_fifo)) {
>          size = fifo8_num_free(&s->tx_fifo);
>          qemu_log_mask(LOG_GUEST_ERROR, "sifive_uart: TX FIFO overflow.\n");
>      }
>  
>      if (size > 0) {
> +        if (fifo8_num_used(&s->tx_fifo) < txcnt &&
> +            (fifo8_num_used(&s->tx_fifo) + size) >= txcnt) {
> +            update_irq = true;
> +        }
> +
>          fifo8_push_all(&s->tx_fifo, buf, size);
>      }
>  
> @@ -137,6 +145,14 @@ static void sifive_uart_write_tx_fifo(SiFiveUARTState 
> *s, const uint8_t *buf,
>          s->txfifo |= SIFIVE_UART_TXFIFO_FULL;
>      }
>  
> +    /*
> +     * Update txwm interrupt pending status when the number of entries
> +     * in the transmit FIFO crosses or reaches the watermark.
> +     */
> +    if (update_irq) {
> +        sifive_uart_update_irq(s);
> +    }
The watermark crossing detection logic looks correct, and commit message
is clear and well-written.

Reviewed-by: Chao Liu <[email protected]>

Thanks,
Chao
> +
>      sifive_uart_trigger_tx_fifo(s);
>  }
>  
> -- 
> 2.43.0
> 
> 

Reply via email to