[PATCH v3 0/4] Fix SiFive UART spurious IRQ issue and misc updates

[PATCH v3 0/4] Fix SiFive UART spurious IRQ issue and misc updates

[frank.chang]

From: Frank Chang <frank.chang@sifive.com>

This patch series fixes the spurious IRQ issue in the SiFive UART on Linux
introduced after commit [1], which changed character transmission from
synchronous to asynchronous.

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.

This issue has been observed on resource-constrained systems, 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.

The remaining patches contain miscellaneous updates, including
implementing txctrl.txen and rxctrl.rxen, update IRQ when rxctrl is
written and remove the unused ip variable.

[1] 53c1557b230986ab6320a58e1b2c26216ecd86d5

Changelog:
  v3:
    * Rebase to the latest master branch.
  v2:
    * Include txctrl.txen and rxctrl.rxen.

Frank Chang (4):
  hw/char: sifive_uart: Implement txctrl.txen and rxctrl.rxen
  hw/char: sifive_uart: Sync txwm interrupt pending status after TX FIFO
    enqueue
  hw/char: sifive_uart: Update IRQ when rxctrl is written
  hw/char: sifive_uart: Remove ip variable

 hw/char/sifive_uart.c         | 46 ++++++++++++++++++++++++++++-------
 include/hw/char/sifive_uart.h |  3 ++-
 2 files changed, 39 insertions(+), 10 deletions(-)

--
2.43.0

[PATCH v3 1/4] hw/char: sifive_uart: Implement txctrl.txen and rxctrl.rxen

[frank.chang]

From: Frank Chang <frank.chang@sifive.com>

Implement txctrl.txen and rxctrl.rxen as follows:

* txctrl.txen
  The txen bit controls whether the Tx channel is active. When cleared,
  transmission of Tx FIFO contents is suppressed, and the txd pin is
  driven high.

* rxctrl.rxen:
  The rxen bit controls whether the Rx channel is active. When cleared,
  the state of the rxd pin is ignored, and no characters will be
  enqueued into the Rx FIFO.

Therefore, the Tx FIFO should not be dequeued when txctrl.txen is
cleared, and the Rx FIFO should not be enqueued when rxctrl.rxen is
cleared.

Signed-off-by: Frank Chang <frank.chang@sifive.com>
---
 hw/char/sifive_uart.c         | 27 ++++++++++++++++++++-------
 include/hw/char/sifive_uart.h |  2 ++
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
index af17cf9a6ce..3ce6a4ee76a 100644
--- a/hw/char/sifive_uart.c
+++ b/hw/char/sifive_uart.c
@@ -78,6 +78,11 @@ static gboolean sifive_uart_xmit(void *do_not_use, GIOCondition cond,
         return G_SOURCE_REMOVE;
     }
 
+    /* Don't pop the FIFO if transmit is disabled. */
+    if (!SIFIVE_UART_TXEN(s->txctrl)) {
+        return G_SOURCE_REMOVE;
+    }
+
     /* Don't pop the FIFO in case the write fails */
     characters = fifo8_peek_bufptr(&s->tx_fifo,
                                    fifo8_num_used(&s->tx_fifo), &numptr);
@@ -106,11 +111,19 @@ static gboolean sifive_uart_xmit(void *do_not_use, GIOCondition cond,
     return G_SOURCE_REMOVE;
 }
 
-static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
-                                      int size)
+static void sifive_uart_trigger_tx_fifo(SiFiveUARTState *s)
 {
     uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 
+    if (!timer_pending(s->fifo_trigger_handle)) {
+        timer_mod(s->fifo_trigger_handle, current_time +
+            TX_INTERRUPT_TRIGGER_DELAY_NS);
+    }
+}
+
+static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
+                                      int size)
+{
     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");
@@ -124,10 +137,7 @@ static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
         s->txfifo |= SIFIVE_UART_TXFIFO_FULL;
     }
 
-    if (!timer_pending(s->fifo_trigger_handle)) {
-        timer_mod(s->fifo_trigger_handle, current_time +
-                      TX_INTERRUPT_TRIGGER_DELAY_NS);
-    }
+    sifive_uart_trigger_tx_fifo(s);
 }
 
 static uint64_t
@@ -184,6 +194,9 @@ sifive_uart_write(void *opaque, hwaddr addr,
         return;
     case SIFIVE_UART_TXCTRL:
         s->txctrl = val64;
+        if (SIFIVE_UART_TXEN(s->txctrl) && !fifo8_is_empty(&s->tx_fifo)) {
+            sifive_uart_trigger_tx_fifo(s);
+        }
         return;
     case SIFIVE_UART_RXCTRL:
         s->rxctrl = val64;
@@ -231,7 +244,7 @@ static int sifive_uart_can_rx(void *opaque)
 {
     SiFiveUARTState *s = opaque;
 
-    return s->rx_fifo_len < sizeof(s->rx_fifo);
+    return SIFIVE_UART_RXEN(s->rxctrl) && (s->rx_fifo_len < sizeof(s->rx_fifo));
 }
 
 static void sifive_uart_event(void *opaque, QEMUChrEvent event)
diff --git a/include/hw/char/sifive_uart.h b/include/hw/char/sifive_uart.h
index 414564b0266..c78d9bd1fc6 100644
--- a/include/hw/char/sifive_uart.h
+++ b/include/hw/char/sifive_uart.h
@@ -51,6 +51,8 @@ enum {
 
 #define SIFIVE_UART_TXFIFO_FULL    0x80000000
 
+#define SIFIVE_UART_TXEN(txctrl)        (txctrl & 0x1)
+#define SIFIVE_UART_RXEN(rxctrl)        (rxctrl & 0x1)
 #define SIFIVE_UART_GET_TXCNT(txctrl)   ((txctrl >> 16) & 0x7)
 #define SIFIVE_UART_GET_RXCNT(rxctrl)   ((rxctrl >> 16) & 0x7)
 
-- 
2.43.0

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

[frank.chang]

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

[PATCH v3 3/4] hw/char: sifive_uart: Update IRQ when rxctrl is written

[frank.chang]

From: Frank Chang <frank.chang@sifive.com>

When rxctl is updated, we also need to check whether the IRQ should be
raised, as the user may activate the Rx channel or change the Rx FIFO
watermark level.

Signed-off-by: Frank Chang <frank.chang@sifive.com>
---
 hw/char/sifive_uart.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
index ae71a15a2a4..f255cca960d 100644
--- a/hw/char/sifive_uart.c
+++ b/hw/char/sifive_uart.c
@@ -216,6 +216,7 @@ sifive_uart_write(void *opaque, hwaddr addr,
         return;
     case SIFIVE_UART_RXCTRL:
         s->rxctrl = val64;
+        sifive_uart_update_irq(s);
         return;
     case SIFIVE_UART_DIV:
         s->div = val64;
-- 
2.43.0

[PATCH v3 4/4] hw/char: sifive_uart: Remove ip variable

[frank.chang]

From: Frank Chang <frank.chang@sifive.com>

The ip variable is no longer used in the code. Remove it from the
codebase.

Signed-off-by: Frank Chang <frank.chang@sifive.com>
---
 hw/char/sifive_uart.c         | 6 ++----
 include/hw/char/sifive_uart.h | 1 -
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
index f255cca960d..4c30fbf5685 100644
--- a/hw/char/sifive_uart.c
+++ b/hw/char/sifive_uart.c
@@ -285,7 +285,6 @@ static void sifive_uart_reset_enter(Object *obj, ResetType type)
 
     s->txfifo = 0;
     s->ie = 0;
-    s->ip = 0;
     s->txctrl = 0;
     s->rxctrl = 0;
     s->div = 0;
@@ -343,14 +342,13 @@ static void sifive_uart_reset_hold(Object *obj, ResetType type)
 
 static const VMStateDescription vmstate_sifive_uart = {
     .name = TYPE_SIFIVE_UART,
-    .version_id = 2,
-    .minimum_version_id = 2,
+    .version_id = 3,
+    .minimum_version_id = 3,
     .fields = (const VMStateField[]) {
         VMSTATE_UINT8_ARRAY(rx_fifo, SiFiveUARTState,
                             SIFIVE_UART_RX_FIFO_SIZE),
         VMSTATE_UINT8(rx_fifo_len, SiFiveUARTState),
         VMSTATE_UINT32(ie, SiFiveUARTState),
-        VMSTATE_UINT32(ip, SiFiveUARTState),
         VMSTATE_UINT32(txctrl, SiFiveUARTState),
         VMSTATE_UINT32(rxctrl, SiFiveUARTState),
         VMSTATE_UINT32(div, SiFiveUARTState),
diff --git a/include/hw/char/sifive_uart.h b/include/hw/char/sifive_uart.h
index c78d9bd1fc6..5f3b1327de0 100644
--- a/include/hw/char/sifive_uart.h
+++ b/include/hw/char/sifive_uart.h
@@ -73,7 +73,6 @@ struct SiFiveUARTState {
 
     uint32_t txfifo;
     uint32_t ie;
-    uint32_t ip;
     uint32_t txctrl;
     uint32_t rxctrl;
     uint32_t div;
-- 
2.43.0

Re: [PATCH v3 1/4] hw/char: sifive_uart: Implement txctrl.txen and rxctrl.rxen

[Alistair Francis]

On Thu, Mar 12, 2026 at 1:33 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> Implement txctrl.txen and rxctrl.rxen as follows:
>
> * txctrl.txen
>   The txen bit controls whether the Tx channel is active. When cleared,
>   transmission of Tx FIFO contents is suppressed, and the txd pin is
>   driven high.
>
> * rxctrl.rxen:
>   The rxen bit controls whether the Rx channel is active. When cleared,
>   the state of the rxd pin is ignored, and no characters will be
>   enqueued into the Rx FIFO.
>
> Therefore, the Tx FIFO should not be dequeued when txctrl.txen is
> cleared, and the Rx FIFO should not be enqueued when rxctrl.rxen is
> cleared.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>

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

Alistair

> ---
>  hw/char/sifive_uart.c         | 27 ++++++++++++++++++++-------
>  include/hw/char/sifive_uart.h |  2 ++
>  2 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
> index af17cf9a6ce..3ce6a4ee76a 100644
> --- a/hw/char/sifive_uart.c
> +++ b/hw/char/sifive_uart.c
> @@ -78,6 +78,11 @@ static gboolean sifive_uart_xmit(void *do_not_use, GIOCondition cond,
>          return G_SOURCE_REMOVE;
>      }
>
> +    /* Don't pop the FIFO if transmit is disabled. */
> +    if (!SIFIVE_UART_TXEN(s->txctrl)) {
> +        return G_SOURCE_REMOVE;
> +    }
> +
>      /* Don't pop the FIFO in case the write fails */
>      characters = fifo8_peek_bufptr(&s->tx_fifo,
>                                     fifo8_num_used(&s->tx_fifo), &numptr);
> @@ -106,11 +111,19 @@ static gboolean sifive_uart_xmit(void *do_not_use, GIOCondition cond,
>      return G_SOURCE_REMOVE;
>  }
>
> -static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
> -                                      int size)
> +static void sifive_uart_trigger_tx_fifo(SiFiveUARTState *s)
>  {
>      uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>
> +    if (!timer_pending(s->fifo_trigger_handle)) {
> +        timer_mod(s->fifo_trigger_handle, current_time +
> +            TX_INTERRUPT_TRIGGER_DELAY_NS);
> +    }
> +}
> +
> +static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
> +                                      int size)
> +{
>      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");
> @@ -124,10 +137,7 @@ static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
>          s->txfifo |= SIFIVE_UART_TXFIFO_FULL;
>      }
>
> -    if (!timer_pending(s->fifo_trigger_handle)) {
> -        timer_mod(s->fifo_trigger_handle, current_time +
> -                      TX_INTERRUPT_TRIGGER_DELAY_NS);
> -    }
> +    sifive_uart_trigger_tx_fifo(s);
>  }
>
>  static uint64_t
> @@ -184,6 +194,9 @@ sifive_uart_write(void *opaque, hwaddr addr,
>          return;
>      case SIFIVE_UART_TXCTRL:
>          s->txctrl = val64;
> +        if (SIFIVE_UART_TXEN(s->txctrl) && !fifo8_is_empty(&s->tx_fifo)) {
> +            sifive_uart_trigger_tx_fifo(s);
> +        }
>          return;
>      case SIFIVE_UART_RXCTRL:
>          s->rxctrl = val64;
> @@ -231,7 +244,7 @@ static int sifive_uart_can_rx(void *opaque)
>  {
>      SiFiveUARTState *s = opaque;
>
> -    return s->rx_fifo_len < sizeof(s->rx_fifo);
> +    return SIFIVE_UART_RXEN(s->rxctrl) && (s->rx_fifo_len < sizeof(s->rx_fifo));
>  }
>
>  static void sifive_uart_event(void *opaque, QEMUChrEvent event)
> diff --git a/include/hw/char/sifive_uart.h b/include/hw/char/sifive_uart.h
> index 414564b0266..c78d9bd1fc6 100644
> --- a/include/hw/char/sifive_uart.h
> +++ b/include/hw/char/sifive_uart.h
> @@ -51,6 +51,8 @@ enum {
>
>  #define SIFIVE_UART_TXFIFO_FULL    0x80000000
>
> +#define SIFIVE_UART_TXEN(txctrl)        (txctrl & 0x1)
> +#define SIFIVE_UART_RXEN(rxctrl)        (rxctrl & 0x1)
>  #define SIFIVE_UART_GET_TXCNT(txctrl)   ((txctrl >> 16) & 0x7)
>  #define SIFIVE_UART_GET_RXCNT(rxctrl)   ((rxctrl >> 16) & 0x7)
>
> --
> 2.43.0
>
>

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

[Alistair Francis]

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
>
>

Re: [PATCH v3 3/4] hw/char: sifive_uart: Update IRQ when rxctrl is written

[Alistair Francis]

On Thu, Mar 12, 2026 at 1:33 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> When rxctl is updated, we also need to check whether the IRQ should be
> raised, as the user may activate the Rx channel or change the Rx FIFO
> watermark level.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>

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

Alistair

> ---
>  hw/char/sifive_uart.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
> index ae71a15a2a4..f255cca960d 100644
> --- a/hw/char/sifive_uart.c
> +++ b/hw/char/sifive_uart.c
> @@ -216,6 +216,7 @@ sifive_uart_write(void *opaque, hwaddr addr,
>          return;
>      case SIFIVE_UART_RXCTRL:
>          s->rxctrl = val64;
> +        sifive_uart_update_irq(s);
>          return;
>      case SIFIVE_UART_DIV:
>          s->div = val64;
> --
> 2.43.0
>
>

Re: [PATCH v3 4/4] hw/char: sifive_uart: Remove ip variable

[Alistair Francis]

On Thu, Mar 12, 2026 at 1:33 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> The ip variable is no longer used in the code. Remove it from the
> codebase.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>

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

Alistair

> ---
>  hw/char/sifive_uart.c         | 6 ++----
>  include/hw/char/sifive_uart.h | 1 -
>  2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
> index f255cca960d..4c30fbf5685 100644
> --- a/hw/char/sifive_uart.c
> +++ b/hw/char/sifive_uart.c
> @@ -285,7 +285,6 @@ static void sifive_uart_reset_enter(Object *obj, ResetType type)
>
>      s->txfifo = 0;
>      s->ie = 0;
> -    s->ip = 0;
>      s->txctrl = 0;
>      s->rxctrl = 0;
>      s->div = 0;
> @@ -343,14 +342,13 @@ static void sifive_uart_reset_hold(Object *obj, ResetType type)
>
>  static const VMStateDescription vmstate_sifive_uart = {
>      .name = TYPE_SIFIVE_UART,
> -    .version_id = 2,
> -    .minimum_version_id = 2,
> +    .version_id = 3,
> +    .minimum_version_id = 3,
>      .fields = (const VMStateField[]) {
>          VMSTATE_UINT8_ARRAY(rx_fifo, SiFiveUARTState,
>                              SIFIVE_UART_RX_FIFO_SIZE),
>          VMSTATE_UINT8(rx_fifo_len, SiFiveUARTState),
>          VMSTATE_UINT32(ie, SiFiveUARTState),
> -        VMSTATE_UINT32(ip, SiFiveUARTState),
>          VMSTATE_UINT32(txctrl, SiFiveUARTState),
>          VMSTATE_UINT32(rxctrl, SiFiveUARTState),
>          VMSTATE_UINT32(div, SiFiveUARTState),
> diff --git a/include/hw/char/sifive_uart.h b/include/hw/char/sifive_uart.h
> index c78d9bd1fc6..5f3b1327de0 100644
> --- a/include/hw/char/sifive_uart.h
> +++ b/include/hw/char/sifive_uart.h
> @@ -73,7 +73,6 @@ struct SiFiveUARTState {
>
>      uint32_t txfifo;
>      uint32_t ie;
> -    uint32_t ip;
>      uint32_t txctrl;
>      uint32_t rxctrl;
>      uint32_t div;
> --
> 2.43.0
>
>

Re: [PATCH v3 0/4] Fix SiFive UART spurious IRQ issue and misc updates

[Alistair Francis]

On Thu, Mar 12, 2026 at 1:33 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> This patch series fixes the spurious IRQ issue in the SiFive UART on Linux
> introduced after commit [1], which changed character transmission from
> synchronous to asynchronous.
>
> 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.
>
> This issue has been observed on resource-constrained systems, 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.
>
> The remaining patches contain miscellaneous updates, including
> implementing txctrl.txen and rxctrl.rxen, update IRQ when rxctrl is
> written and remove the unused ip variable.
>
> [1] 53c1557b230986ab6320a58e1b2c26216ecd86d5
>
> Changelog:
>   v3:
>     * Rebase to the latest master branch.
>   v2:
>     * Include txctrl.txen and rxctrl.rxen.
>
> Frank Chang (4):
>   hw/char: sifive_uart: Implement txctrl.txen and rxctrl.rxen
>   hw/char: sifive_uart: Sync txwm interrupt pending status after TX FIFO
>     enqueue
>   hw/char: sifive_uart: Update IRQ when rxctrl is written
>   hw/char: sifive_uart: Remove ip variable

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  hw/char/sifive_uart.c         | 46 ++++++++++++++++++++++++++++-------
>  include/hw/char/sifive_uart.h |  3 ++-
>  2 files changed, 39 insertions(+), 10 deletions(-)
>
> --
> 2.43.0
>
>

Re: [PATCH v3 1/4] hw/char: sifive_uart: Implement txctrl.txen and rxctrl.rxen

[Chao Liu]

On Thu, Mar 12, 2026 at 11:31:58AM +0800, frank.chang@sifive.com wrote:
> From: Frank Chang <frank.chang@sifive.com>
> 
> Implement txctrl.txen and rxctrl.rxen as follows:
> 
> * txctrl.txen
>   The txen bit controls whether the Tx channel is active. When cleared,
>   transmission of Tx FIFO contents is suppressed, and the txd pin is
>   driven high.
> 
> * rxctrl.rxen:
>   The rxen bit controls whether the Rx channel is active. When cleared,
>   the state of the rxd pin is ignored, and no characters will be
>   enqueued into the Rx FIFO.
> 
> Therefore, the Tx FIFO should not be dequeued when txctrl.txen is
> cleared, and the Rx FIFO should not be enqueued when rxctrl.rxen is
> cleared.
> 
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> ---
>  hw/char/sifive_uart.c         | 27 ++++++++++++++++++++-------
>  include/hw/char/sifive_uart.h |  2 ++
>  2 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
> index af17cf9a6ce..3ce6a4ee76a 100644
> --- a/hw/char/sifive_uart.c
> +++ b/hw/char/sifive_uart.c
> @@ -78,6 +78,11 @@ static gboolean sifive_uart_xmit(void *do_not_use, GIOCondition cond,
>          return G_SOURCE_REMOVE;
>      }
>  
> +    /* Don't pop the FIFO if transmit is disabled. */
> +    if (!SIFIVE_UART_TXEN(s->txctrl)) {
> +        return G_SOURCE_REMOVE;
> +    }
> +
>      /* Don't pop the FIFO in case the write fails */
>      characters = fifo8_peek_bufptr(&s->tx_fifo,
>                                     fifo8_num_used(&s->tx_fifo), &numptr);
> @@ -106,11 +111,19 @@ static gboolean sifive_uart_xmit(void *do_not_use, GIOCondition cond,
>      return G_SOURCE_REMOVE;
>  }
>  
> -static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
> -                                      int size)
> +static void sifive_uart_trigger_tx_fifo(SiFiveUARTState *s)
>  {
>      uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>  
> +    if (!timer_pending(s->fifo_trigger_handle)) {
> +        timer_mod(s->fifo_trigger_handle, current_time +
> +            TX_INTERRUPT_TRIGGER_DELAY_NS);
> +    }
> +}
> +
> +static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
> +                                      int size)
> +{
>      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");
> @@ -124,10 +137,7 @@ static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
>          s->txfifo |= SIFIVE_UART_TXFIFO_FULL;
>      }
>  
> -    if (!timer_pending(s->fifo_trigger_handle)) {
> -        timer_mod(s->fifo_trigger_handle, current_time +
> -                      TX_INTERRUPT_TRIGGER_DELAY_NS);
> -    }
> +    sifive_uart_trigger_tx_fifo(s);
>  }
>  
>  static uint64_t
> @@ -184,6 +194,9 @@ sifive_uart_write(void *opaque, hwaddr addr,
>          return;
>      case SIFIVE_UART_TXCTRL:
>          s->txctrl = val64;
A sifive_uart_update_irq() call is also needed here.
When txctrl is written, the watermark (txcnt) may
change, or txen may toggle(you can see SiFive FE310-G002
datasheet), both of which can alter the txwm pending state.

Patch 3/4 adds this call for RXCTRL with the rationale "the
user may activate the Rx channel or change the Rx FIFO
watermark level" -- the same reasoning applies to TXCTRL.

Without this, there's a window where the IRQ line
doesn't reflect the actual pending state after a
txctrl write that changes the watermark.

Everything else looks good to me.

Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>

Thanks,
Chao

> +        if (SIFIVE_UART_TXEN(s->txctrl) && !fifo8_is_empty(&s->tx_fifo)) {
> +            sifive_uart_trigger_tx_fifo(s);
> +        }
>          return;
>      case SIFIVE_UART_RXCTRL:
>          s->rxctrl = val64;
> @@ -231,7 +244,7 @@ static int sifive_uart_can_rx(void *opaque)
>  {
>      SiFiveUARTState *s = opaque;
>  
> -    return s->rx_fifo_len < sizeof(s->rx_fifo);
> +    return SIFIVE_UART_RXEN(s->rxctrl) && (s->rx_fifo_len < sizeof(s->rx_fifo));
>  }
>  
>  static void sifive_uart_event(void *opaque, QEMUChrEvent event)
> diff --git a/include/hw/char/sifive_uart.h b/include/hw/char/sifive_uart.h
> index 414564b0266..c78d9bd1fc6 100644
> --- a/include/hw/char/sifive_uart.h
> +++ b/include/hw/char/sifive_uart.h
> @@ -51,6 +51,8 @@ enum {
>  
>  #define SIFIVE_UART_TXFIFO_FULL    0x80000000
>  
> +#define SIFIVE_UART_TXEN(txctrl)        (txctrl & 0x1)
> +#define SIFIVE_UART_RXEN(rxctrl)        (rxctrl & 0x1)
macro arguments should be parenthesized to avoid precedence surprises.

e.g. if the argument is ever an expression like (a | b), & binds tighter
than |. I suggest:

  #define SIFIVE_UART_TXEN(txctrl)   ((txctrl) & 0x1)
  #define SIFIVE_UART_RXEN(rxctrl)   ((rxctrl) & 0x1)

The existing GET_TXCNT / GET_RXCNT macros have the
same issue, so this could be a follow-up cleanup.

>  #define SIFIVE_UART_GET_TXCNT(txctrl)   ((txctrl >> 16) & 0x7)
>  #define SIFIVE_UART_GET_RXCNT(rxctrl)   ((rxctrl >> 16) & 0x7)
>  
> -- 
> 2.43.0
> 
> 

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

[Chao Liu]

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 3/4] hw/char: sifive_uart: Update IRQ when rxctrl is written

[Chao Liu]

On Thu, Mar 12, 2026 at 11:32:00AM +0800, frank.chang@sifive.com wrote:
> From: Frank Chang <frank.chang@sifive.com>
> 
> When rxctl is updated, we also need to check whether the IRQ should be
> raised, as the user may activate the Rx channel or change the Rx FIFO
> watermark level.
> 
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> ---
>  hw/char/sifive_uart.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
> index ae71a15a2a4..f255cca960d 100644
> --- a/hw/char/sifive_uart.c
> +++ b/hw/char/sifive_uart.c
> @@ -216,6 +216,7 @@ sifive_uart_write(void *opaque, hwaddr addr,
>          return;
>      case SIFIVE_UART_RXCTRL:
>          s->rxctrl = val64;
> +        sifive_uart_update_irq(s);
This is correct. As noted in the 1/4 review, the same treatment
should be applied to the SIFIVE_UART_TXCTRL case for consistency
-- the user may also change the Tx watermark level or toggle txen,
which affects the txwm pending state.

Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>

Thanks,
Chao
>          return;
>      case SIFIVE_UART_DIV:
>          s->div = val64;
> -- 
> 2.43.0
> 
> 

Re: [PATCH v3 4/4] hw/char: sifive_uart: Remove ip variable

[Chao Liu]

On Thu, Mar 12, 2026 at 11:32:01AM +0800, frank.chang@sifive.com wrote:
> From: Frank Chang <frank.chang@sifive.com>
> 
> The ip variable is no longer used in the code. Remove it from the
> codebase.
> 
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>

Thanks,
Chao
> ---
>  hw/char/sifive_uart.c         | 6 ++----
>  include/hw/char/sifive_uart.h | 1 -
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
> index f255cca960d..4c30fbf5685 100644
> --- a/hw/char/sifive_uart.c
> +++ b/hw/char/sifive_uart.c
> @@ -285,7 +285,6 @@ static void sifive_uart_reset_enter(Object *obj, ResetType type)
>  
>      s->txfifo = 0;
>      s->ie = 0;
> -    s->ip = 0;
>      s->txctrl = 0;
>      s->rxctrl = 0;
>      s->div = 0;
> @@ -343,14 +342,13 @@ static void sifive_uart_reset_hold(Object *obj, ResetType type)
>  
>  static const VMStateDescription vmstate_sifive_uart = {
>      .name = TYPE_SIFIVE_UART,
> -    .version_id = 2,
> -    .minimum_version_id = 2,
> +    .version_id = 3,
> +    .minimum_version_id = 3,
>      .fields = (const VMStateField[]) {
>          VMSTATE_UINT8_ARRAY(rx_fifo, SiFiveUARTState,
>                              SIFIVE_UART_RX_FIFO_SIZE),
>          VMSTATE_UINT8(rx_fifo_len, SiFiveUARTState),
>          VMSTATE_UINT32(ie, SiFiveUARTState),
> -        VMSTATE_UINT32(ip, SiFiveUARTState),
>          VMSTATE_UINT32(txctrl, SiFiveUARTState),
>          VMSTATE_UINT32(rxctrl, SiFiveUARTState),
>          VMSTATE_UINT32(div, SiFiveUARTState),
> diff --git a/include/hw/char/sifive_uart.h b/include/hw/char/sifive_uart.h
> index c78d9bd1fc6..5f3b1327de0 100644
> --- a/include/hw/char/sifive_uart.h
> +++ b/include/hw/char/sifive_uart.h
> @@ -73,7 +73,6 @@ struct SiFiveUARTState {
>  
>      uint32_t txfifo;
>      uint32_t ie;
> -    uint32_t ip;
>      uint32_t txctrl;
>      uint32_t rxctrl;
>      uint32_t div;
> -- 
> 2.43.0
> 
>