[PATCH 0/6] sbsa-gwdt cleanup and fixes

[PATCH 0/6] sbsa-gwdt cleanup and fixes

[Igor Mammedov]

Series picks up dropped gwdt fixes from

  [PATCH v3 00/17] Add watchdog support to arm/virt board
  https://patchew.org/QEMU/20260624102830.1355552-1-imammedo@redhat.com/

patches were basically rewritten to make changes simpler/easier to reson about.

patch 6/6 is partial fix with open question:

sbsa-gwdt currently mantains WVC in since VM start ticks, which works fine under
TCG. However under KVM those system counter domains diverge and CNTPCT in guest
starts to return host's value. As result WCV reads/writes got mixed up with
QEMU still assuming VM timeframe while guest using host's one.
Question is how should we fix it?

Igor Mammedov (6):
  sbsa-gwdt: reduce code ident
  arm: gwdt: consolidate clear WS0 and WS1 on explicit refresh
  arm: gwdt: simplify WCV update condition
  sbsa-gwdt: rename sbsa_gwdt_update_timer() to
    sbsa_gwdt_wor_update_timer()
  sbsa-gwdt: don't arm timer for compare values above INT64_MAX
  sbsa-gwdt: reschedule timer on direct WCV load

 hw/watchdog/sbsa_gwdt.c | 81 +++++++++++++++++++++++++++--------------
 1 file changed, 54 insertions(+), 27 deletions(-)

-- 
2.52.0

[PATCH 1/6] sbsa-gwdt: reduce code ident

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
 hw/watchdog/sbsa_gwdt.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/hw/watchdog/sbsa_gwdt.c b/hw/watchdog/sbsa_gwdt.c
index 330a74798a..7cc5609b83 100644
--- a/hw/watchdog/sbsa_gwdt.c
+++ b/hw/watchdog/sbsa_gwdt.c
@@ -105,25 +105,27 @@ static void sbsa_gwdt_update_timer(SBSA_GWDTState *s, WdtRefreshType rtype)
 
     timer_del(s->timer);
 
-    if (s->wcs & SBSA_GWDT_WCS_EN) {
-        /*
-         * Extract the upper 16 bits from woru & 32 bits from worl
-         * registers to construct the 48 bit offset value
-         */
-        timeout = s->woru;
-        timeout <<= 32;
-        timeout |= s->worl;
-        timeout = muldiv64(timeout, NANOSECONDS_PER_SECOND, s->freq);
-        timeout += qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-
-        if ((rtype == EXPLICIT_REFRESH) || ((rtype == TIMEOUT_REFRESH) &&
-                (!(s->wcs & SBSA_GWDT_WCS_WS0)))) {
-            /* store the current timeout value into compare registers */
-            s->wcvu = timeout >> 32;
-            s->wcvl = timeout;
-        }
-        timer_mod(s->timer, timeout);
+    if (!(s->wcs & SBSA_GWDT_WCS_EN)) {
+        return;
+    }
+
+    /*
+     * Extract the upper 16 bits from woru & 32 bits from worl
+     * registers to construct the 48 bit offset value
+     */
+    timeout = s->woru;
+    timeout <<= 32;
+    timeout |= s->worl;
+    timeout = muldiv64(timeout, NANOSECONDS_PER_SECOND, s->freq);
+    timeout += qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+
+    if ((rtype == EXPLICIT_REFRESH) || ((rtype == TIMEOUT_REFRESH) &&
+            (!(s->wcs & SBSA_GWDT_WCS_WS0)))) {
+        /* store the current timeout value into compare registers */
+        s->wcvu = timeout >> 32;
+        s->wcvl = timeout;
     }
+    timer_mod(s->timer, timeout);
 }
 
 static void sbsa_gwdt_rwrite(void *opaque, hwaddr offset, uint64_t data,
-- 
2.52.0

Re: [PATCH 1/6] sbsa-gwdt: reduce code ident

[Peter Maydell]

On Wed, 2 Sept 2026 at 13:26, Igor Mammedov <imammedo@redhat.com> wrote:
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>

Subject should presumably be "indent"; otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 1/6] sbsa-gwdt: reduce code ident

[Igor Mammedov]

On Tue, 8 Sep 2026 15:28:46 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Wed, 2 Sept 2026 at 13:26, Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > Reviewed-by: Eric Auger <eric.auger@redhat.com>  
> 
> Subject should presumably be "indent"; otherwise

yep, will fix it on respin if it happens

> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> thanks
> -- PMM
> 

[PATCH 2/6] arm: gwdt: consolidate clear WS0 and WS1 on explicit refresh

[Igor Mammedov]

Current impl had logic scattered all over the place and WCS was implicitly
cleared on WCS write.

Consolidate clear action in one place under EXPLICIT_REFRESH condition.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/watchdog/sbsa_gwdt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/watchdog/sbsa_gwdt.c b/hw/watchdog/sbsa_gwdt.c
index 7cc5609b83..2aa44573a7 100644
--- a/hw/watchdog/sbsa_gwdt.c
+++ b/hw/watchdog/sbsa_gwdt.c
@@ -105,6 +105,10 @@ static void sbsa_gwdt_update_timer(SBSA_GWDTState *s, WdtRefreshType rtype)
 
     timer_del(s->timer);
 
+    if (rtype == EXPLICIT_REFRESH) {
+        s->wcs &= ~(SBSA_GWDT_WCS_WS0 | SBSA_GWDT_WCS_WS1);
+    }
+
     if (!(s->wcs & SBSA_GWDT_WCS_EN)) {
         return;
     }
@@ -134,8 +138,6 @@ static void sbsa_gwdt_rwrite(void *opaque, hwaddr offset, uint64_t data,
 
     trace_sbsa_gwdt_refresh_write(offset, data);
     if (offset == SBSA_GWDT_WRR) {
-        s->wcs &= ~(SBSA_GWDT_WCS_WS0 | SBSA_GWDT_WCS_WS1);
-
         sbsa_gwdt_update_timer(s, EXPLICIT_REFRESH);
     } else {
         qemu_log_mask(LOG_GUEST_ERROR, "bad address in refresh frame write :"
@@ -157,14 +159,12 @@ static void sbsa_gwdt_write(void *opaque, hwaddr offset, uint64_t data,
 
     case SBSA_GWDT_WOR:
         s->worl = data;
-        s->wcs &= ~(SBSA_GWDT_WCS_WS0 | SBSA_GWDT_WCS_WS1);
         qemu_set_irq(s->irq, 0);
         sbsa_gwdt_update_timer(s, EXPLICIT_REFRESH);
         break;
 
     case SBSA_GWDT_WORU:
         s->woru = data & SBSA_GWDT_WOR_MASK;
-        s->wcs &= ~(SBSA_GWDT_WCS_WS0 | SBSA_GWDT_WCS_WS1);
         qemu_set_irq(s->irq, 0);
         sbsa_gwdt_update_timer(s, EXPLICIT_REFRESH);
         break;
-- 
2.52.0

Re: [PATCH 2/6] arm: gwdt: consolidate clear WS0 and WS1 on explicit refresh

[Peter Maydell]

On Wed, 2 Sept 2026 at 13:26, Igor Mammedov <imammedo@redhat.com> wrote:
>
> Current impl had logic scattered all over the place and WCS was implicitly
> cleared on WCS write.
>
> Consolidate clear action in one place under EXPLICIT_REFRESH condition.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---

Yes, this better matches the specification, which states that
WS0/WS1 remain asserted until an explicit refresh occurs,
and that writes to these 3 registers are all explicit refreshes.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[PATCH 3/6] arm: gwdt: simplify WCV update condition

[Igor Mammedov]

Current code implements SBSA 5.0, revision 1 of the generic watchdog.
Which requires WCV retain time when WS1 was triggered.

towards that effect sbsa_gwdt_update_timer(), used condition
   (rtype == TIMEOUT_REFRESH) && !(s->wcs & SBSA_GWDT_WCS_WS0)
to make sure that WCV won't be updated on WS1.

However that is dead code as sbsa_gwdt_timer_sysinterrupt() when
signalling WS1, never calls sbsa_gwdt_update_timer().

Delete dead condition and add assert to make sure that
sbsa_gwdt_update_timer() is never called unnoticed on WS1 path.

Above change leaves us with always true remaining if condition:
  (rtype == EXPLICIT_REFRESH) || (rtype == TIMEOUT_REFRESH)
drop it and update WCV unconditionaly.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/watchdog/sbsa_gwdt.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/watchdog/sbsa_gwdt.c b/hw/watchdog/sbsa_gwdt.c
index 2aa44573a7..a9bf3f8312 100644
--- a/hw/watchdog/sbsa_gwdt.c
+++ b/hw/watchdog/sbsa_gwdt.c
@@ -123,12 +123,12 @@ static void sbsa_gwdt_update_timer(SBSA_GWDTState *s, WdtRefreshType rtype)
     timeout = muldiv64(timeout, NANOSECONDS_PER_SECOND, s->freq);
     timeout += qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 
-    if ((rtype == EXPLICIT_REFRESH) || ((rtype == TIMEOUT_REFRESH) &&
-            (!(s->wcs & SBSA_GWDT_WCS_WS0)))) {
-        /* store the current timeout value into compare registers */
-        s->wcvu = timeout >> 32;
-        s->wcvl = timeout;
-    }
+    g_assert(!(s->wcs & SBSA_GWDT_WCS_WS1));
+
+    /* store the current timeout value into compare registers */
+    s->wcvu = timeout >> 32;
+    s->wcvl = timeout;
+
     timer_mod(s->timer, timeout);
 }
 
-- 
2.52.0

Re: [PATCH 3/6] arm: gwdt: simplify WCV update condition

[Peter Maydell]

On Wed, 2 Sept 2026 at 13:27, Igor Mammedov <imammedo@redhat.com> wrote:
>
> Current code implements SBSA 5.0, revision 1 of the generic watchdog.

The comment in the source code says "v6.0".

> Which requires WCV retain time when WS1 was triggered.
>
> towards that effect sbsa_gwdt_update_timer(), used condition
>    (rtype == TIMEOUT_REFRESH) && !(s->wcs & SBSA_GWDT_WCS_WS0)
> to make sure that WCV won't be updated on WS1.
>
> However that is dead code as sbsa_gwdt_timer_sysinterrupt() when
> signalling WS1, never calls sbsa_gwdt_update_timer().
>
> Delete dead condition and add assert to make sure that
> sbsa_gwdt_update_timer() is never called unnoticed on WS1 path.
>
> Above change leaves us with always true remaining if condition:
>   (rtype == EXPLICIT_REFRESH) || (rtype == TIMEOUT_REFRESH)
> drop it and update WCV unconditionaly.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/watchdog/sbsa_gwdt.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/watchdog/sbsa_gwdt.c b/hw/watchdog/sbsa_gwdt.c
> index 2aa44573a7..a9bf3f8312 100644
> --- a/hw/watchdog/sbsa_gwdt.c
> +++ b/hw/watchdog/sbsa_gwdt.c
> @@ -123,12 +123,12 @@ static void sbsa_gwdt_update_timer(SBSA_GWDTState *s, WdtRefreshType rtype)
>      timeout = muldiv64(timeout, NANOSECONDS_PER_SECOND, s->freq);
>      timeout += qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>
> -    if ((rtype == EXPLICIT_REFRESH) || ((rtype == TIMEOUT_REFRESH) &&
> -            (!(s->wcs & SBSA_GWDT_WCS_WS0)))) {
> -        /* store the current timeout value into compare registers */
> -        s->wcvu = timeout >> 32;
> -        s->wcvl = timeout;
> -    }
> +    g_assert(!(s->wcs & SBSA_GWDT_WCS_WS1));

I think this could use a comment to clarify, because it moves us
away from how the pseudocode in the spec is written.

    /*
     * We only call this function for an explicit refresh (which
     * clears WS0 and WS1) or after the first timeout setting WS0;
     * so we can always load the compare registers with the new
     * timeout value (which is not permitted when the second timeout
     * that sets WS1 happens).
     */
> +
> +    /* store the current timeout value into compare registers */
> +    s->wcvu = timeout >> 32;
> +    s->wcvl = timeout;
> +
>      timer_mod(s->timer, timeout);
>  }

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[PATCH 4/6] sbsa-gwdt: rename sbsa_gwdt_update_timer() to sbsa_gwdt_wor_update_timer()

[Igor Mammedov]

The timer update helper computes the timeout from the WOR (watchdog
offset register). Rename it to sbsa_gwdt_wor_update_timer() to make room
for a separate WCV-based update helper added by a following patch.

No functional change.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/watchdog/sbsa_gwdt.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/watchdog/sbsa_gwdt.c b/hw/watchdog/sbsa_gwdt.c
index a9bf3f8312..1211f1ca65 100644
--- a/hw/watchdog/sbsa_gwdt.c
+++ b/hw/watchdog/sbsa_gwdt.c
@@ -99,7 +99,7 @@ static uint64_t sbsa_gwdt_read(void *opaque, hwaddr addr, unsigned int size)
     return ret;
 }
 
-static void sbsa_gwdt_update_timer(SBSA_GWDTState *s, WdtRefreshType rtype)
+static void sbsa_gwdt_wor_update_timer(SBSA_GWDTState *s, WdtRefreshType rtype)
 {
     uint64_t timeout = 0;
 
@@ -138,7 +138,7 @@ static void sbsa_gwdt_rwrite(void *opaque, hwaddr offset, uint64_t data,
 
     trace_sbsa_gwdt_refresh_write(offset, data);
     if (offset == SBSA_GWDT_WRR) {
-        sbsa_gwdt_update_timer(s, EXPLICIT_REFRESH);
+        sbsa_gwdt_wor_update_timer(s, EXPLICIT_REFRESH);
     } else {
         qemu_log_mask(LOG_GUEST_ERROR, "bad address in refresh frame write :"
                         " 0x%x\n", (int)offset);
@@ -154,19 +154,19 @@ static void sbsa_gwdt_write(void *opaque, hwaddr offset, uint64_t data,
     case SBSA_GWDT_WCS:
         s->wcs = data & SBSA_GWDT_WCS_EN;
         qemu_set_irq(s->irq, 0);
-        sbsa_gwdt_update_timer(s, EXPLICIT_REFRESH);
+        sbsa_gwdt_wor_update_timer(s, EXPLICIT_REFRESH);
         break;
 
     case SBSA_GWDT_WOR:
         s->worl = data;
         qemu_set_irq(s->irq, 0);
-        sbsa_gwdt_update_timer(s, EXPLICIT_REFRESH);
+        sbsa_gwdt_wor_update_timer(s, EXPLICIT_REFRESH);
         break;
 
     case SBSA_GWDT_WORU:
         s->woru = data & SBSA_GWDT_WOR_MASK;
         qemu_set_irq(s->irq, 0);
-        sbsa_gwdt_update_timer(s, EXPLICIT_REFRESH);
+        sbsa_gwdt_wor_update_timer(s, EXPLICIT_REFRESH);
         break;
 
     case SBSA_GWDT_WCV:
@@ -205,7 +205,7 @@ static void sbsa_gwdt_timer_sysinterrupt(void *opaque)
     if (!(s->wcs & SBSA_GWDT_WCS_WS0)) {
         s->wcs |= SBSA_GWDT_WCS_WS0;
         trace_sbsa_gwdt_ws0_asserted();
-        sbsa_gwdt_update_timer(s, TIMEOUT_REFRESH);
+        sbsa_gwdt_wor_update_timer(s, TIMEOUT_REFRESH);
         qemu_set_irq(s->irq, 1);
     } else {
         s->wcs |= SBSA_GWDT_WCS_WS1;
-- 
2.52.0

Re: [PATCH 4/6] sbsa-gwdt: rename sbsa_gwdt_update_timer() to sbsa_gwdt_wor_update_timer()

[Peter Maydell]

On Wed, 2 Sept 2026 at 13:27, Igor Mammedov <imammedo@redhat.com> wrote:
>
> The timer update helper computes the timeout from the WOR (watchdog
> offset register). Rename it to sbsa_gwdt_wor_update_timer() to make room
> for a separate WCV-based update helper added by a following patch.
>
> No functional change.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[PATCH 5/6] sbsa-gwdt: don't arm timer for compare values above INT64_MAX

[Igor Mammedov]

QEMU timer subsystem uses int64_t, so a WCV value with bit 63 set
would overflow and cause the timer to fire immediately. The SBSA
spec defines WCV as an unsigned 64-bit compare value, so such
values are valid per spec and represent far-future deadlines
(however unpractical).

The current code is not affected: WCV writes don't reschedule the
timer yet, so a guest-supplied compare value never reaches the
timer API. The follow-up patch ("sbsa-gwdt: reschedule timer on
direct WCV load") changes that, at which point an out-of-range WCV
would overflow the timer. For example, Windows in GTDT mode writes
WCV in two 32-bit halves while the watchdog is running:

 sbsa-gwdt_control_write [0x8]  <- 0xffffffff    # WOR (~4 sec)
 sbsa-gwdt_control_write [0x0]  <- 0x1           # WCS enable
 sbsa-gwdt_control_write [0x14] <- 0xffffffff    # WCVU (intermediate)
 sbsa-gwdt_control_write [0x10] <- 0xa906ca28    # WCVL
 sbsa-gwdt_control_write [0x14] <- 0xecb1        # WCVU (final)

The intermediate WCVU write (0xffffffff) yields a WCV above
INT64_MAX; once WCV writes arm the timer, this overflows QEMU's
signed timer and fires immediately -- triggering WS0 => WS1 =>
reboot before the final WCVU write lands.

Add the guard first, before the WCV rescheduling patch, so the
tree stays bisectable. Instead of arming the timer with an
unsupported deadline, leave it disarmed. This reuses
hw/timer/sse-timer.c:sse_set_timer() (commit 0b8ceee822
"hw/timer/sse-timer: Model the SSE Subsystem System Timer").

Introduce a sbsa_gwdt_set_timer() helper for this and route the
WOR-based timeout through it.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/watchdog/sbsa_gwdt.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/watchdog/sbsa_gwdt.c b/hw/watchdog/sbsa_gwdt.c
index 1211f1ca65..0a1d981072 100644
--- a/hw/watchdog/sbsa_gwdt.c
+++ b/hw/watchdog/sbsa_gwdt.c
@@ -99,6 +99,22 @@ static uint64_t sbsa_gwdt_read(void *opaque, hwaddr addr, unsigned int size)
     return ret;
 }
 
+static void sbsa_gwdt_set_timer(SBSA_GWDTState *s, uint64_t deadline)
+{
+    /*
+     * WCV is an unsigned 64-bit compare value, but QEMUTimer stores the
+     * expiry as a signed int64_t. A deadline with bit 63 set would be seen
+     * as already expired and fire the watchdog immediately. Such a deadline
+     * is unreachable within any guest runtime, so treat it as "never" and
+     * leave the timer disarmed instead.
+     */
+    if (deadline <= INT64_MAX) {
+        timer_mod(s->timer, deadline);
+    } else {
+        timer_del(s->timer);
+    }
+}
+
 static void sbsa_gwdt_wor_update_timer(SBSA_GWDTState *s, WdtRefreshType rtype)
 {
     uint64_t timeout = 0;
@@ -129,7 +145,7 @@ static void sbsa_gwdt_wor_update_timer(SBSA_GWDTState *s, WdtRefreshType rtype)
     s->wcvu = timeout >> 32;
     s->wcvl = timeout;
 
-    timer_mod(s->timer, timeout);
+    sbsa_gwdt_set_timer(s, timeout);
 }
 
 static void sbsa_gwdt_rwrite(void *opaque, hwaddr offset, uint64_t data,
-- 
2.52.0

Re: [PATCH 5/6] sbsa-gwdt: don't arm timer for compare values above INT64_MAX

[Peter Maydell]

On Wed, 2 Sept 2026 at 13:27, Igor Mammedov <imammedo@redhat.com> wrote:
>
> QEMU timer subsystem uses int64_t, so a WCV value with bit 63 set
> would overflow and cause the timer to fire immediately. The SBSA
> spec defines WCV as an unsigned 64-bit compare value, so such
> values are valid per spec and represent far-future deadlines
> (however unpractical).
>
> The current code is not affected: WCV writes don't reschedule the
> timer yet, so a guest-supplied compare value never reaches the
> timer API. The follow-up patch ("sbsa-gwdt: reschedule timer on
> direct WCV load") changes that, at which point an out-of-range WCV
> would overflow the timer. For example, Windows in GTDT mode writes
> WCV in two 32-bit halves while the watchdog is running:
>
>  sbsa-gwdt_control_write [0x8]  <- 0xffffffff    # WOR (~4 sec)
>  sbsa-gwdt_control_write [0x0]  <- 0x1           # WCS enable
>  sbsa-gwdt_control_write [0x14] <- 0xffffffff    # WCVU (intermediate)
>  sbsa-gwdt_control_write [0x10] <- 0xa906ca28    # WCVL
>  sbsa-gwdt_control_write [0x14] <- 0xecb1        # WCVU (final)
>
> The intermediate WCVU write (0xffffffff) yields a WCV above
> INT64_MAX; once WCV writes arm the timer, this overflows QEMU's
> signed timer and fires immediately -- triggering WS0 => WS1 =>
> reboot before the final WCVU write lands.
>
> Add the guard first, before the WCV rescheduling patch, so the
> tree stays bisectable. Instead of arming the timer with an
> unsupported deadline, leave it disarmed. This reuses
> hw/timer/sse-timer.c:sse_set_timer() (commit 0b8ceee822
> "hw/timer/sse-timer: Model the SSE Subsystem System Timer").
>
> Introduce a sbsa_gwdt_set_timer() helper for this and route the
> WOR-based timeout through it.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[PATCH 6/6] sbsa-gwdt: reschedule timer on direct WCV load

[Igor Mammedov]

According to spec[1]:

 "The compare value can either be loaded directly or indirectly on an
  explicit refresh or timeout refresh"

QEMU accepts writes to WCV but doesn't reschedule the timer,
which it should per the spec pseudo code:

 "TimeoutRefresh = ( SystemCounter [63:0] > CompareValue [63:0])"

Fix it by updating the timer on WCV write.

partialy[2] fixes Windows in GTDT mode, which never issues a WRR refresh.
Instead, it programs WOR to ~4 sec, enables WCS, and immediately
writes a large absolute value into WCV to push the timeout far
into the future. Without this, QEMU ignores the WCV write,
the short WOR expires, and the guest reboots unexpectedly.

1) Arm® Server Base System Architecture 6.0
   Platform Design Document
   DEN0029D 6.0
   "A.2 Watchdog Operation"
2) under KVM system counter (CNTPCT) as read by guest apprears to
   produce host's value, so watchdog timeout effectively gets
   extended on host uptime offset. But that's a separate issue
   to be fixed.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/watchdog/sbsa_gwdt.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/watchdog/sbsa_gwdt.c b/hw/watchdog/sbsa_gwdt.c
index 0a1d981072..9ed109d6eb 100644
--- a/hw/watchdog/sbsa_gwdt.c
+++ b/hw/watchdog/sbsa_gwdt.c
@@ -148,6 +148,13 @@ static void sbsa_gwdt_wor_update_timer(SBSA_GWDTState *s, WdtRefreshType rtype)
     sbsa_gwdt_set_timer(s, timeout);
 }
 
+static void sbsa_gwdt_wcv_update_timer(SBSA_GWDTState *s)
+{
+    if (s->wcs & SBSA_GWDT_WCS_EN) {
+        sbsa_gwdt_set_timer(s, (uint64_t)s->wcvu << 32 | s->wcvl);
+    }
+}
+
 static void sbsa_gwdt_rwrite(void *opaque, hwaddr offset, uint64_t data,
                              unsigned size) {
     SBSA_GWDTState *s = SBSA_GWDT(opaque);
@@ -187,10 +194,12 @@ static void sbsa_gwdt_write(void *opaque, hwaddr offset, uint64_t data,
 
     case SBSA_GWDT_WCV:
         s->wcvl = data;
+        sbsa_gwdt_wcv_update_timer(s);
         break;
 
     case SBSA_GWDT_WCVU:
         s->wcvu = data;
+        sbsa_gwdt_wcv_update_timer(s);
         break;
 
     default:
-- 
2.52.0

Re: [PATCH 6/6] sbsa-gwdt: reschedule timer on direct WCV load

[Peter Maydell]

On Wed, 2 Sept 2026 at 13:27, Igor Mammedov <imammedo@redhat.com> wrote:
>
> According to spec[1]:
>
>  "The compare value can either be loaded directly or indirectly on an
>   explicit refresh or timeout refresh"
>
> QEMU accepts writes to WCV but doesn't reschedule the timer,
> which it should per the spec pseudo code:
>
>  "TimeoutRefresh = ( SystemCounter [63:0] > CompareValue [63:0])"
>
> Fix it by updating the timer on WCV write.
>
> partialy[2] fixes Windows in GTDT mode, which never issues a WRR refresh.
> Instead, it programs WOR to ~4 sec, enables WCS, and immediately
> writes a large absolute value into WCV to push the timeout far
> into the future. Without this, QEMU ignores the WCV write,
> the short WOR expires, and the guest reboots unexpectedly.
>
> 1) Arm® Server Base System Architecture 6.0
>    Platform Design Document
>    DEN0029D 6.0
>    "A.2 Watchdog Operation"
> 2) under KVM system counter (CNTPCT) as read by guest apprears to
>    produce host's value, so watchdog timeout effectively gets
>    extended on host uptime offset. But that's a separate issue
>    to be fixed.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 0/6] sbsa-gwdt cleanup and fixes

[Peter Maydell]

On Wed, 2 Sept 2026 at 13:26, Igor Mammedov <imammedo@redhat.com> wrote:
>
>
> Series picks up dropped gwdt fixes from
>
>   [PATCH v3 00/17] Add watchdog support to arm/virt board
>   https://patchew.org/QEMU/20260624102830.1355552-1-imammedo@redhat.com/
>
> patches were basically rewritten to make changes simpler/easier to reson about.
>
> patch 6/6 is partial fix with open question:
>
> sbsa-gwdt currently mantains WVC in since VM start ticks, which works fine under
> TCG. However under KVM those system counter domains diverge and CNTPCT in guest
> starts to return host's value. As result WCV reads/writes got mixed up with
> QEMU still assuming VM timeframe while guest using host's one.
> Question is how should we fix it?

Presumably we should be dealing in deltas from the
QEMU_CLOCK_VIRTUAL value?

Anyway, for this series, since the only thing I wanted to change
was expanding a comment, I'll apply this to target-arm.next and
make that tweak there.

-- PMM

Re: [PATCH 0/6] sbsa-gwdt cleanup and fixes

[Igor Mammedov]

On Tue, 8 Sep 2026 16:08:35 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Wed, 2 Sept 2026 at 13:26, Igor Mammedov <imammedo@redhat.com> wrote:
> >
> >
> > Series picks up dropped gwdt fixes from
> >
> >   [PATCH v3 00/17] Add watchdog support to arm/virt board
> >   https://patchew.org/QEMU/20260624102830.1355552-1-imammedo@redhat.com/
> >
> > patches were basically rewritten to make changes simpler/easier to reson about.
> >
> > patch 6/6 is partial fix with open question:
> >
> > sbsa-gwdt currently mantains WVC in since VM start ticks, which works fine under
> > TCG. However under KVM those system counter domains diverge and CNTPCT in guest
> > starts to return host's value. As result WCV reads/writes got mixed up with
> > QEMU still assuming VM timeframe while guest using host's one.
> > Question is how should we fix it?  
> 
> Presumably we should be dealing in deltas from the
> QEMU_CLOCK_VIRTUAL value?

deltas could works as workaround at cost of polluting sbsa-gwdt
device model with KVM quirk. but then real fun begins when we
bring in picture migration (even not counting CNTPCT jump),
WCV would either diverge or jump as well => more KVM quirks
in whatchdog code to deal with.

I'd think that we would want to track QEMU_CLOCK_VIRTUAL in WCV,
basically make CNTPCT = CNTVCT and treat as such everywhere.

What is lost on me is history/reasoning why CNTPCT tracks host value?
Why can't we make it track virtual clock instead (there is KVM_ARM_SET_COUNTER_OFFSET
that supposedly should do the job)?

> Anyway, for this series, since the only thing I wanted to change
> was expanding a comment, I'll apply this to target-arm.next and
> make that tweak there.

Thanks!
> 
> -- PMM
>