[PATCH v3 2/4] hw/char: sifive_uart: Sync txwm interrupt pending status after TX FIFO enqueue

frank.chang@sifive.com posted 4 patches 3 weeks, 5 days ago
Maintainers: Alistair Francis <Alistair.Francis@wdc.com>, Palmer Dabbelt <palmer@dabbelt.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
[PATCH v3 2/4] hw/char: sifive_uart: Sync txwm interrupt pending status after TX FIFO enqueue
Posted by frank.chang@sifive.com 3 weeks, 5 days ago
From: Frank Chang <frank.chang@sifive.com>

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 <frank.chang@sifive.com>
---
 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);
+    }
+
     sifive_uart_trigger_tx_fifo(s);
 }
 
-- 
2.43.0
Re: [PATCH v3 2/4] hw/char: sifive_uart: Sync txwm interrupt pending status after TX FIFO enqueue
Posted by Chao Liu 3 weeks, 3 days ago
On Thu, Mar 12, 2026 at 11:31:59AM +0800, frank.chang@sifive.com wrote:
> From: Frank Chang <frank.chang@sifive.com>
> 
> 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 <frank.chang@sifive.com>
> ---
>  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 <chao.liu.zevorn@gmail.com>

Thanks,
Chao
> +
>      sifive_uart_trigger_tx_fifo(s);
>  }
>  
> -- 
> 2.43.0
> 
>
Re: [PATCH v3 2/4] hw/char: sifive_uart: Sync txwm interrupt pending status after TX FIFO enqueue
Posted by Alistair Francis 3 weeks, 4 days ago
On Thu, Mar 12, 2026 at 1:32 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> 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 <frank.chang@sifive.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  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);
> +    }
> +
>      sifive_uart_trigger_tx_fifo(s);
>  }
>
> --
> 2.43.0
>
>