The following issue was observed on Windows 10 21H2 x64+: when the domain state
is saved while all cores are executing the 'halt' instruction, and the memory
save takes a relatively long time (tens of seconds), the HPET counter may
overflow as follows:
counter  = 11243f3e4a
comparator = 910cb70f

In such cases, the fix implemented in commit
b144cf45d50b603c2909fc32c6abf7359f86f1aa does not work (because the 'diff' is
not negative), resulting in the guest VM becoming unresponsive for
approximately 30 seconds.

This patch adds an option to always adjust the HPET timer to fire immediately
after restore.
---
 docs/misc/xen-command-line.pandoc |  9 +++++++++
 xen/arch/x86/hvm/hpet.c           | 21 ++++++++++++++++-----
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index a75b6c9301..b28610918f 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1461,6 +1461,15 @@ HPET can be disabled by specifying `hpet=0`.
 
 Deprecated alternative of `hpet=broadcast`.
 
+### hpet_drift_fix (x86)
+> `= <boolean>`
+
+> Default: `false`
+
+Always set HPET timer to fire immediately after domain restore.
+This option can be used to fix unresponsive snapshots with modern x64 Windows
+systems (21H2+) which use non-periodic timers.
+
 ### hvm_debug (x86)
 > `= <integer>`
 
diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
index f0e5f877f4..5b9e38bd9b 100644
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -11,6 +11,7 @@
 #include <asm/current.h>
 #include <asm/hpet.h>
 #include <asm/mc146818rtc.h>
+#include <xen/param.h>
 #include <xen/sched.h>
 #include <xen/event.h>
 #include <xen/trace.h>
@@ -222,6 +223,9 @@ static void cf_check hpet_timer_fired(struct vcpu *v, void *data)
  * 1/(2^10) second, namely, 0.9765625 milliseconds */
 #define  HPET_TINY_TIME_SPAN  ((h->stime_freq >> 10) / STIME_PER_HPET_TICK)
 
+bool hpet_drift_fix;
+boolean_param("hpet_drift_fix", hpet_drift_fix);
+
 static void hpet_set_timer(HPETState *h, unsigned int tn,
                            uint64_t guest_time)
 {
@@ -268,11 +272,18 @@ static void hpet_set_timer(HPETState *h, unsigned int tn,
      * are restoring after migrate, treat any wrap as past since the value
      * is unlikely to be 'small'.
      */
-    if ( (int64_t)diff < 0 )
-        diff = (timer_is_32bit(h, tn) &&
-                vhpet_domain(h)->creation_finished &&
-                (-diff > HPET_TINY_TIME_SPAN))
-            ? (uint32_t)diff : 0;
+    if (hpet_drift_fix && !vhpet_domain(h)->creation_finished)
+    {
+        diff = 0;
+    }
+    else
+    {
+        if ( (int64_t)diff < 0 )
+            diff = (timer_is_32bit(h, tn) &&
+                    vhpet_domain(h)->creation_finished &&
+                    (-diff > HPET_TINY_TIME_SPAN))
+                ? (uint32_t)diff : 0;
+    }
 
     destroy_periodic_time(&h->pt[tn]);
     if ( (tn <= 1) && (h->hpet.config & HPET_CFG_LEGACY) )
-- 
2.42.1

On Wed, Aug 27, 2025 at 09:01:08AM +0300, Vyacheslav Legoshin wrote:
> The following issue was observed on Windows 10 21H2 x64+: when the domain state
> is saved while all cores are executing the 'halt' instruction,

I assume this when executing `xl save` or equivalent command from a
different toolstack?

IIRC in that case the guest would be paused while the memory dump to
disk is done, and hence guest vCPUs won't be executing the `halt`
instruction, they wouldn't be executing at all.

> and the memory
> save takes a relatively long time (tens of seconds), the HPET counter may
> overflow as follows:
> counter  = 11243f3e4a
> comparator = 910cb70f
> 
> In such cases, the fix implemented in commit
> b144cf45d50b603c2909fc32c6abf7359f86f1aa does not work (because the 'diff' is
> not negative), resulting in the guest VM becoming unresponsive for
> approximately 30 seconds.
> 
> This patch adds an option to always adjust the HPET timer to fire immediately
> after restore.

What happens if the guest is left running after the save?  I assume
that using `xl save -c <domain>` would leave the domain in a similarly
wedged state, and your proposed workaround won't be effective there,
since there's no restoring process?  Or that's not the case there
because Xen is still keeping track of the internal timer, and would
set an interrupt as pending anyway?

Should we maybe store the last guest time at context save, and check
against that to see whether comparators should have triggered, instead
of playing this games?

Thanks, Roger.

On 27.08.2025 08:01, Vyacheslav Legoshin wrote:
> The following issue was observed on Windows 10 21H2 x64+: when the domain state
> is saved while all cores are executing the 'halt' instruction, and the memory
> save takes a relatively long time (tens of seconds), the HPET counter may
> overflow as follows:
> counter  = 11243f3e4a
> comparator = 910cb70f
> 
> In such cases, the fix implemented in commit
> b144cf45d50b603c2909fc32c6abf7359f86f1aa does not work (because the 'diff' is
> not negative), resulting in the guest VM becoming unresponsive for
> approximately 30 seconds.
> 
> This patch adds an option to always adjust the HPET timer to fire immediately
> after restore.

Thanks for the patch, but issues already start here: There's no Signed-off-by:.

> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1461,6 +1461,15 @@ HPET can be disabled by specifying `hpet=0`.
>  
>  Deprecated alternative of `hpet=broadcast`.
>  
> +### hpet_drift_fix (x86)
> +> `= <boolean>`
> +
> +> Default: `false`
> +
> +Always set HPET timer to fire immediately after domain restore.
> +This option can be used to fix unresponsive snapshots with modern x64 Windows
> +systems (21H2+) which use non-periodic timers.

I'm not convinced making this a global option is appropriate. If an option is
needed, it would better be a per-domain setting. Whether an option is needed
in the first place is tbd.

And then, if a global option was used, then please with dashes in favor of
underscores in its name.

> --- a/xen/arch/x86/hvm/hpet.c
> +++ b/xen/arch/x86/hvm/hpet.c
> @@ -11,6 +11,7 @@
>  #include <asm/current.h>
>  #include <asm/hpet.h>
>  #include <asm/mc146818rtc.h>
> +#include <xen/param.h>
>  #include <xen/sched.h>
>  #include <xen/event.h>
>  #include <xen/trace.h>
> @@ -222,6 +223,9 @@ static void cf_check hpet_timer_fired(struct vcpu *v, void *data)
>   * 1/(2^10) second, namely, 0.9765625 milliseconds */
>  #define  HPET_TINY_TIME_SPAN  ((h->stime_freq >> 10) / STIME_PER_HPET_TICK)
>  
> +bool hpet_drift_fix;

static and __ro_after_init.

> @@ -268,11 +272,18 @@ static void hpet_set_timer(HPETState *h, unsigned int tn,
>       * are restoring after migrate, treat any wrap as past since the value
>       * is unlikely to be 'small'.
>       */
> -    if ( (int64_t)diff < 0 )
> -        diff = (timer_is_32bit(h, tn) &&
> -                vhpet_domain(h)->creation_finished &&
> -                (-diff > HPET_TINY_TIME_SPAN))
> -            ? (uint32_t)diff : 0;
> +    if (hpet_drift_fix && !vhpet_domain(h)->creation_finished)

Nit (style): Missing blanks (see e.g. the other if() you're altering).

> +    {
> +        diff = 0;
> +    }

No real need for figure braces here.

The comment ahead of the construct also wants amending / updating.

> +    else
> +    {
> +        if ( (int64_t)diff < 0 )

"else if()" please, reducing the diff quite a bit.

Jan