[pve-devel] [PATCH qemu] revert hpet changes to fix performance regression

Fiona Ebner f.ebner at proxmox.com
Wed Mar 19 17:39:30 CET 2025


As reported in the community forum [0][1], QEMU processes for Linux
guests would consume more CPU on the host after an update to QEMU 9.2.
The issue was reproduced and bisecting pointed to QEMU commit
f0ccf77078 ("hpet: fix and cleanup persistence of interrupt status").

Some quick experimentation suggests that in particular the last part
 is responsible for the issue:
> - the timer must be kept running even if not enabled, in
>   order to set the ISR flag, so writes to HPET_TN_CFG must
>   not call hpet_del_timer()

Users confirmed that setting the hpet=off machine flag works around
the issue[0]. For Windows (7 or later) guests, the flag is already
disabled, because of issues in the past [2].

Upstream suggested reverting the relevant patches for now [3], because
other issues were reported too. All except commit 5895879aca ("hpet:
remove unnecessary variable "index"") are actually dependent on each
other for cleanly reverting f0ccf77078, and while not strictly
required, that one was reverted too for completeness.

[0]: https://forum.proxmox.com/threads/163694/
[1]: https://forum.proxmox.com/threads/161849/post-756793
[2]: https://lists.proxmox.com/pipermail/pve-devel/2012-December/004958.html
[3]: https://lore.kernel.org/qemu-devel/CABgObfaKJ5NFVKmYLFmu4C0iZZLJJtcWksLCzyA0tBoz0koZ4A@mail.gmail.com/

Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
---
 ...void-timer-storms-on-periodic-timers.patch |  50 ++++
 ...e-full-64-bit-target-value-of-the-co.patch | 203 +++++++++++++
 ...-hpet-accept-64-bit-reads-and-writes.patch | 281 ++++++++++++++++++
 ...e-read-only-bits-directly-in-new_val.patch |  64 ++++
 ...et-remove-unnecessary-variable-index.patch |  68 +++++
 ...re-high-bits-of-comparator-in-32-bit.patch |  40 +++
 ...and-cleanup-persistence-of-interrupt.patch | 120 ++++++++
 debian/patches/series                         |   7 +
 8 files changed, 833 insertions(+)
 create mode 100644 debian/patches/pve/0051-Revert-hpet-avoid-timer-storms-on-periodic-timers.patch
 create mode 100644 debian/patches/pve/0052-Revert-hpet-store-full-64-bit-target-value-of-the-co.patch
 create mode 100644 debian/patches/pve/0053-Revert-hpet-accept-64-bit-reads-and-writes.patch
 create mode 100644 debian/patches/pve/0054-Revert-hpet-place-read-only-bits-directly-in-new_val.patch
 create mode 100644 debian/patches/pve/0055-Revert-hpet-remove-unnecessary-variable-index.patch
 create mode 100644 debian/patches/pve/0056-Revert-hpet-ignore-high-bits-of-comparator-in-32-bit.patch
 create mode 100644 debian/patches/pve/0057-Revert-hpet-fix-and-cleanup-persistence-of-interrupt.patch

diff --git a/debian/patches/pve/0051-Revert-hpet-avoid-timer-storms-on-periodic-timers.patch b/debian/patches/pve/0051-Revert-hpet-avoid-timer-storms-on-periodic-timers.patch
new file mode 100644
index 0000000..3ff1ee4
--- /dev/null
+++ b/debian/patches/pve/0051-Revert-hpet-avoid-timer-storms-on-periodic-timers.patch
@@ -0,0 +1,50 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Fiona Ebner <f.ebner at proxmox.com>
+Date: Wed, 19 Mar 2025 17:31:05 +0100
+Subject: [PATCH] Revert "hpet: avoid timer storms on periodic timers"
+
+This reverts commit 7c912ffb59e8137091894d767433e65c3df8b0bf.
+
+Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
+---
+ hw/timer/hpet.c | 13 ++-----------
+ 1 file changed, 2 insertions(+), 11 deletions(-)
+
+diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
+index 5399f1b2a3..8ccc421cbb 100644
+--- a/hw/timer/hpet.c
++++ b/hw/timer/hpet.c
+@@ -59,7 +59,6 @@ typedef struct HPETTimer {  /* timers */
+     uint8_t wrap_flag;      /* timer pop will indicate wrap for one-shot 32-bit
+                              * mode. Next pop will be actual timer expiration.
+                              */
+-    uint64_t last;          /* last value armed, to avoid timer storms */
+ } HPETTimer;
+ 
+ struct HPETState {
+@@ -267,7 +266,6 @@ static int hpet_post_load(void *opaque, int version_id)
+     for (i = 0; i < s->num_timers; i++) {
+         HPETTimer *t = &s->timer[i];
+         t->cmp64 = hpet_calculate_cmp64(t, s->hpet_counter, t->cmp);
+-        t->last = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - NANOSECONDS_PER_SECOND;
+     }
+     /* Recalculate the offset between the main counter and guest time */
+     if (!s->hpet_offset_saved) {
+@@ -366,15 +364,8 @@ static const VMStateDescription vmstate_hpet = {
+ 
+ static void hpet_arm(HPETTimer *t, uint64_t tick)
+ {
+-    uint64_t ns = hpet_get_ns(t->state, tick);
+-
+-    /* Clamp period to reasonable min value (1 us) */
+-    if (timer_is_periodic(t) && ns - t->last < 1000) {
+-        ns = t->last + 1000;
+-    }
+-
+-    t->last = ns;
+-    timer_mod(t->qemu_timer, ns);
++    /* FIXME: Clamp period to reasonable min value? */
++    timer_mod(t->qemu_timer, hpet_get_ns(t->state, tick));
+ }
+ 
+ /*
diff --git a/debian/patches/pve/0052-Revert-hpet-store-full-64-bit-target-value-of-the-co.patch b/debian/patches/pve/0052-Revert-hpet-store-full-64-bit-target-value-of-the-co.patch
new file mode 100644
index 0000000..482940c
--- /dev/null
+++ b/debian/patches/pve/0052-Revert-hpet-store-full-64-bit-target-value-of-the-co.patch
@@ -0,0 +1,203 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Fiona Ebner <f.ebner at proxmox.com>
+Date: Wed, 19 Mar 2025 17:31:08 +0100
+Subject: [PATCH] Revert "hpet: store full 64-bit target value of the counter"
+
+This reverts commit 242d665396407f83a6acbffc804882eeb21cfdad.
+
+Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
+---
+ hw/timer/hpet.c | 111 +++++++++++++++++++++++++++---------------------
+ 1 file changed, 62 insertions(+), 49 deletions(-)
+
+diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
+index 8ccc421cbb..415a9433f1 100644
+--- a/hw/timer/hpet.c
++++ b/hw/timer/hpet.c
+@@ -54,7 +54,6 @@ typedef struct HPETTimer {  /* timers */
+     uint64_t cmp;           /* comparator */
+     uint64_t fsb;           /* FSB route */
+     /* Hidden register state */
+-    uint64_t cmp64;         /* comparator (extended to counter width) */
+     uint64_t period;        /* Last value written to comparator */
+     uint8_t wrap_flag;      /* timer pop will indicate wrap for one-shot 32-bit
+                              * mode. Next pop will be actual timer expiration.
+@@ -116,6 +115,11 @@ static uint32_t timer_enabled(HPETTimer *t)
+ }
+ 
+ static uint32_t hpet_time_after(uint64_t a, uint64_t b)
++{
++    return ((int32_t)(b - a) < 0);
++}
++
++static uint32_t hpet_time_after64(uint64_t a, uint64_t b)
+ {
+     return ((int64_t)(b - a) < 0);
+ }
+@@ -152,32 +156,27 @@ static uint64_t hpet_get_ticks(HPETState *s)
+     return ns_to_ticks(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + s->hpet_offset);
+ }
+ 
+-static uint64_t hpet_get_ns(HPETState *s, uint64_t tick)
+-{
+-    return ticks_to_ns(tick) - s->hpet_offset;
+-}
+-
+ /*
+- * calculate next value of the general counter that matches the
+- * target (either entirely, or the low 32-bit only depending on
+- * the timer mode).
++ * calculate diff between comparator value and current ticks
+  */
+-static uint64_t hpet_calculate_cmp64(HPETTimer *t, uint64_t cur_tick, uint64_t target)
++static inline uint64_t hpet_calculate_diff(HPETTimer *t, uint64_t current)
+ {
++
+     if (t->config & HPET_TN_32BIT) {
+-        uint64_t result = deposit64(cur_tick, 0, 32, target);
+-        if (result < cur_tick) {
+-            result += 0x100000000ULL;
+-        }
+-        return result;
++        uint32_t diff, cmp;
++
++        cmp = (uint32_t)t->cmp;
++        diff = cmp - (uint32_t)current;
++        diff = (int32_t)diff > 0 ? diff : (uint32_t)1;
++        return (uint64_t)diff;
+     } else {
+-        return target;
+-    }
+-}
++        uint64_t diff, cmp;
+ 
+-static uint64_t hpet_next_wrap(uint64_t cur_tick)
+-{
+-    return (cur_tick | 0xffffffffU) + 1;
++        cmp = t->cmp;
++        diff = cmp - current;
++        diff = (int64_t)diff > 0 ? diff : (uint64_t)1;
++        return diff;
++    }
+ }
+ 
+ static void update_irq(struct HPETTimer *timer, int set)
+@@ -261,12 +260,7 @@ static bool hpet_validate_num_timers(void *opaque, int version_id)
+ static int hpet_post_load(void *opaque, int version_id)
+ {
+     HPETState *s = opaque;
+-    int i;
+ 
+-    for (i = 0; i < s->num_timers; i++) {
+-        HPETTimer *t = &s->timer[i];
+-        t->cmp64 = hpet_calculate_cmp64(t, s->hpet_counter, t->cmp);
+-    }
+     /* Recalculate the offset between the main counter and guest time */
+     if (!s->hpet_offset_saved) {
+         s->hpet_offset = ticks_to_ns(s->hpet_counter)
+@@ -362,10 +356,14 @@ static const VMStateDescription vmstate_hpet = {
+     }
+ };
+ 
+-static void hpet_arm(HPETTimer *t, uint64_t tick)
++static void hpet_arm(HPETTimer *t, uint64_t ticks)
+ {
+-    /* FIXME: Clamp period to reasonable min value? */
+-    timer_mod(t->qemu_timer, hpet_get_ns(t->state, tick));
++    if (ticks < ns_to_ticks(INT64_MAX / 2)) {
++        timer_mod(t->qemu_timer,
++                  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + ticks_to_ns(ticks));
++    } else {
++        timer_del(t->qemu_timer);
++    }
+ }
+ 
+ /*
+@@ -374,44 +372,54 @@ static void hpet_arm(HPETTimer *t, uint64_t tick)
+ static void hpet_timer(void *opaque)
+ {
+     HPETTimer *t = opaque;
++    uint64_t diff;
++
+     uint64_t period = t->period;
+     uint64_t cur_tick = hpet_get_ticks(t->state);
+ 
+     if (timer_is_periodic(t) && period != 0) {
+-        while (hpet_time_after(cur_tick, t->cmp64)) {
+-            t->cmp64 += period;
+-        }
+         if (t->config & HPET_TN_32BIT) {
+-            t->cmp = (uint32_t)t->cmp64;
++            while (hpet_time_after(cur_tick, t->cmp)) {
++                t->cmp = (uint32_t)(t->cmp + t->period);
++            }
+         } else {
+-            t->cmp = t->cmp64;
++            while (hpet_time_after64(cur_tick, t->cmp)) {
++                t->cmp += period;
++            }
++        }
++        diff = hpet_calculate_diff(t, cur_tick);
++        hpet_arm(t, diff);
++    } else if (t->config & HPET_TN_32BIT && !timer_is_periodic(t)) {
++        if (t->wrap_flag) {
++            diff = hpet_calculate_diff(t, cur_tick);
++            hpet_arm(t, diff);
++            t->wrap_flag = 0;
+         }
+-        hpet_arm(t, t->cmp64);
+-    } else if (t->wrap_flag) {
+-        t->wrap_flag = 0;
+-        hpet_arm(t, t->cmp64);
+     }
+     update_irq(t, 1);
+ }
+ 
+ static void hpet_set_timer(HPETTimer *t)
+ {
++    uint64_t diff;
++    uint32_t wrap_diff;  /* how many ticks until we wrap? */
+     uint64_t cur_tick = hpet_get_ticks(t->state);
+ 
++    /* whenever new timer is being set up, make sure wrap_flag is 0 */
+     t->wrap_flag = 0;
+-    t->cmp64 = hpet_calculate_cmp64(t, cur_tick, t->cmp);
+-    if (t->config & HPET_TN_32BIT) {
+-
+-        /* hpet spec says in one-shot 32-bit mode, generate an interrupt when
+-         * counter wraps in addition to an interrupt with comparator match.
+-         */
+-        if (!timer_is_periodic(t) && t->cmp64 > hpet_next_wrap(cur_tick)) {
++    diff = hpet_calculate_diff(t, cur_tick);
++
++    /* hpet spec says in one-shot 32-bit mode, generate an interrupt when
++     * counter wraps in addition to an interrupt with comparator match.
++     */
++    if (t->config & HPET_TN_32BIT && !timer_is_periodic(t)) {
++        wrap_diff = 0xffffffff - (uint32_t)cur_tick;
++        if (wrap_diff < (uint32_t)diff) {
++            diff = wrap_diff;
+             t->wrap_flag = 1;
+-            hpet_arm(t, hpet_next_wrap(cur_tick));
+-            return;
+         }
+     }
+-    hpet_arm(t, t->cmp64);
++    hpet_arm(t, diff);
+ }
+ 
+ static void hpet_del_timer(HPETTimer *t)
+@@ -542,7 +550,12 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+                 timer->cmp = deposit64(timer->cmp, shift, len, value);
+             }
+             if (timer_is_periodic(timer)) {
+-                timer->period = deposit64(timer->period, shift, len, value);
++                /*
++                 * FIXME: Clamp period to reasonable min value?
++                 * Clamp period to reasonable max value
++                 */
++                new_val = deposit64(timer->period, shift, len, value);
++                timer->period = MIN(new_val, (timer->config & HPET_TN_32BIT ? ~0u : ~0ull) >> 1);
+             }
+             timer->config &= ~HPET_TN_SETVAL;
+             if (hpet_enabled(s)) {
diff --git a/debian/patches/pve/0053-Revert-hpet-accept-64-bit-reads-and-writes.patch b/debian/patches/pve/0053-Revert-hpet-accept-64-bit-reads-and-writes.patch
new file mode 100644
index 0000000..b47b823
--- /dev/null
+++ b/debian/patches/pve/0053-Revert-hpet-accept-64-bit-reads-and-writes.patch
@@ -0,0 +1,281 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Fiona Ebner <f.ebner at proxmox.com>
+Date: Wed, 19 Mar 2025 17:31:09 +0100
+Subject: [PATCH] Revert "hpet: accept 64-bit reads and writes"
+
+This reverts commit c2366567378dd8fb89329816003801f54e30e6f3.
+
+Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
+---
+ hw/timer/hpet.c       | 137 +++++++++++++++++++++++++++++-------------
+ hw/timer/trace-events |   3 +-
+ 2 files changed, 96 insertions(+), 44 deletions(-)
+
+diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
+index 415a9433f1..e1ac877759 100644
+--- a/hw/timer/hpet.c
++++ b/hw/timer/hpet.c
+@@ -437,7 +437,6 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr,
+                               unsigned size)
+ {
+     HPETState *s = opaque;
+-    int shift = (addr & 4) * 8;
+     uint64_t cur_tick;
+ 
+     trace_hpet_ram_read(addr);
+@@ -452,33 +451,52 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr,
+             return 0;
+         }
+ 
+-        switch (addr & 0x18) {
+-        case HPET_TN_CFG: // including interrupt capabilities
+-            return timer->config >> shift;
++        switch ((addr - 0x100) % 0x20) {
++        case HPET_TN_CFG:
++            return timer->config;
++        case HPET_TN_CFG + 4: // Interrupt capabilities
++            return timer->config >> 32;
+         case HPET_TN_CMP: // comparator register
+-            return timer->cmp >> shift;
++            return timer->cmp;
++        case HPET_TN_CMP + 4:
++            return timer->cmp >> 32;
+         case HPET_TN_ROUTE:
+-            return timer->fsb >> shift;
++            return timer->fsb;
++        case HPET_TN_ROUTE + 4:
++            return timer->fsb >> 32;
+         default:
+             trace_hpet_ram_read_invalid();
+             break;
+         }
+     } else {
+-        switch (addr & ~4) {
+-        case HPET_ID: // including HPET_PERIOD
+-            return s->capability >> shift;
++        switch (addr) {
++        case HPET_ID:
++            return s->capability;
++        case HPET_PERIOD:
++            return s->capability >> 32;
+         case HPET_CFG:
+-            return s->config >> shift;
++            return s->config;
++        case HPET_CFG + 4:
++            trace_hpet_invalid_hpet_cfg(4);
++            return 0;
+         case HPET_COUNTER:
+             if (hpet_enabled(s)) {
+                 cur_tick = hpet_get_ticks(s);
+             } else {
+                 cur_tick = s->hpet_counter;
+             }
+-            trace_hpet_ram_read_reading_counter(addr & 4, cur_tick);
+-            return cur_tick >> shift;
++            trace_hpet_ram_read_reading_counter(0, cur_tick);
++            return cur_tick;
++        case HPET_COUNTER + 4:
++            if (hpet_enabled(s)) {
++                cur_tick = hpet_get_ticks(s);
++            } else {
++                cur_tick = s->hpet_counter;
++            }
++            trace_hpet_ram_read_reading_counter(4, cur_tick);
++            return cur_tick >> 32;
+         case HPET_STATUS:
+-            return s->isr >> shift;
++            return s->isr;
+         default:
+             trace_hpet_ram_read_invalid();
+             break;
+@@ -492,11 +510,11 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+ {
+     int i;
+     HPETState *s = opaque;
+-    int shift = (addr & 4) * 8;
+-    int len = MIN(size * 8, 64 - shift);
+     uint64_t old_val, new_val, cleared;
+ 
+     trace_hpet_ram_write(addr, value);
++    old_val = hpet_ram_read(opaque, addr, 4);
++    new_val = value;
+ 
+     /*address range of all TN regs*/
+     if (addr >= 0x100 && addr <= 0x3ff) {
+@@ -508,12 +526,9 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+             trace_hpet_timer_id_out_of_range(timer_id);
+             return;
+         }
+-        switch (addr & 0x18) {
++        switch ((addr - 0x100) % 0x20) {
+         case HPET_TN_CFG:
+-            trace_hpet_ram_write_tn_cfg(addr & 4);
+-            old_val = timer->config;
+-            new_val = deposit64(old_val, shift, len, value);
+-            new_val = hpet_fixup_reg(new_val, old_val, HPET_TN_CFG_WRITE_MASK);
++            trace_hpet_ram_write_tn_cfg();
+             if (deactivating_bit(old_val, new_val, HPET_TN_TYPE_LEVEL)) {
+                 /*
+                  * Do this before changing timer->config; otherwise, if
+@@ -521,7 +536,8 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+                  */
+                 update_irq(timer, 0);
+             }
+-            timer->config = new_val;
++            new_val = hpet_fixup_reg(new_val, old_val, HPET_TN_CFG_WRITE_MASK);
++            timer->config = (timer->config & 0xffffffff00000000ULL) | new_val;
+             if (activating_bit(old_val, new_val, HPET_TN_ENABLE)
+                 && (s->isr & (1 << timer_id))) {
+                 update_irq(timer, 1);
+@@ -534,28 +550,56 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+                 hpet_set_timer(timer);
+             }
+             break;
++        case HPET_TN_CFG + 4: // Interrupt capabilities
++            trace_hpet_ram_write_invalid_tn_cfg(4);
++            break;
+         case HPET_TN_CMP: // comparator register
++            trace_hpet_ram_write_tn_cmp(0);
+             if (timer->config & HPET_TN_32BIT) {
+-                /* High 32-bits are zero, leave them untouched.  */
+-                if (shift) {
+-                    trace_hpet_ram_write_invalid_tn_cmp();
+-                    break;
++                new_val = (uint32_t)new_val;
++            }
++            if (!timer_is_periodic(timer)
++                || (timer->config & HPET_TN_SETVAL)) {
++                timer->cmp = (timer->cmp & 0xffffffff00000000ULL) | new_val;
++            }
++            if (timer_is_periodic(timer)) {
++                /*
++                 * FIXME: Clamp period to reasonable min value?
++                 * Clamp period to reasonable max value
++                 */
++                if (timer->config & HPET_TN_32BIT) {
++                    new_val = MIN(new_val, ~0u >> 1);
+                 }
+-                len = 64;
+-                value = (uint32_t) value;
++                timer->period =
++                    (timer->period & 0xffffffff00000000ULL) | new_val;
++            }
++            /*
++             * FIXME: on a 64-bit write, HPET_TN_SETVAL should apply to the
++             * high bits part as well.
++             */
++            timer->config &= ~HPET_TN_SETVAL;
++            if (hpet_enabled(s)) {
++                hpet_set_timer(timer);
+             }
+-            trace_hpet_ram_write_tn_cmp(addr & 4);
++            break;
++        case HPET_TN_CMP + 4: // comparator register high order
++            if (timer->config & HPET_TN_32BIT) {
++                trace_hpet_ram_write_invalid_tn_cmp();
++                break;
++            }
++            trace_hpet_ram_write_tn_cmp(4);
+             if (!timer_is_periodic(timer)
+                 || (timer->config & HPET_TN_SETVAL)) {
+-                timer->cmp = deposit64(timer->cmp, shift, len, value);
++                timer->cmp = (timer->cmp & 0xffffffffULL) | new_val << 32;
+             }
+             if (timer_is_periodic(timer)) {
+                 /*
+                  * FIXME: Clamp period to reasonable min value?
+                  * Clamp period to reasonable max value
+                  */
+-                new_val = deposit64(timer->period, shift, len, value);
+-                timer->period = MIN(new_val, (timer->config & HPET_TN_32BIT ? ~0u : ~0ull) >> 1);
++                new_val = MIN(new_val, ~0u >> 1);
++                timer->period =
++                    (timer->period & 0xffffffffULL) | new_val << 32;
+             }
+             timer->config &= ~HPET_TN_SETVAL;
+             if (hpet_enabled(s)) {
+@@ -563,7 +607,10 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+             }
+             break;
+         case HPET_TN_ROUTE:
+-            timer->fsb = deposit64(timer->fsb, shift, len, value);
++            timer->fsb = (timer->fsb & 0xffffffff00000000ULL) | new_val;
++            break;
++        case HPET_TN_ROUTE + 4:
++            timer->fsb = (new_val << 32) | (timer->fsb & 0xffffffff);
+             break;
+         default:
+             trace_hpet_ram_write_invalid();
+@@ -571,14 +618,12 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+         }
+         return;
+     } else {
+-        switch (addr & ~4) {
++        switch (addr) {
+         case HPET_ID:
+             return;
+         case HPET_CFG:
+-            old_val = s->config;
+-            new_val = deposit64(old_val, shift, len, value);
+             new_val = hpet_fixup_reg(new_val, old_val, HPET_CFG_WRITE_MASK);
+-            s->config = new_val;
++            s->config = (s->config & 0xffffffff00000000ULL) | new_val;
+             if (activating_bit(old_val, new_val, HPET_CFG_ENABLE)) {
+                 /* Enable main counter and interrupt generation. */
+                 s->hpet_offset =
+@@ -608,8 +653,10 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+                 qemu_set_irq(s->irqs[RTC_ISA_IRQ], s->rtc_irq_level);
+             }
+             break;
++        case HPET_CFG + 4:
++            trace_hpet_invalid_hpet_cfg(4);
++            break;
+         case HPET_STATUS:
+-            new_val = value << shift;
+             cleared = new_val & s->isr;
+             for (i = 0; i < s->num_timers; i++) {
+                 if (cleared & (1 << i)) {
+@@ -621,7 +668,15 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+             if (hpet_enabled(s)) {
+                 trace_hpet_ram_write_counter_write_while_enabled();
+             }
+-            s->hpet_counter = deposit64(s->hpet_counter, shift, len, value);
++            s->hpet_counter =
++                (s->hpet_counter & 0xffffffff00000000ULL) | value;
++            trace_hpet_ram_write_counter_written(0, value, s->hpet_counter);
++            break;
++        case HPET_COUNTER + 4:
++            trace_hpet_ram_write_counter_write_while_enabled();
++            s->hpet_counter =
++                (s->hpet_counter & 0xffffffffULL) | (((uint64_t)value) << 32);
++            trace_hpet_ram_write_counter_written(4, value, s->hpet_counter);
+             break;
+         default:
+             trace_hpet_ram_write_invalid();
+@@ -635,11 +690,7 @@ static const MemoryRegionOps hpet_ram_ops = {
+     .write = hpet_ram_write,
+     .valid = {
+         .min_access_size = 4,
+-        .max_access_size = 8,
+-    },
+-    .impl = {
+-        .min_access_size = 4,
+-        .max_access_size = 8,
++        .max_access_size = 4,
+     },
+     .endianness = DEVICE_NATIVE_ENDIAN,
+ };
+diff --git a/hw/timer/trace-events b/hw/timer/trace-events
+index 5cfc369fba..219747df2f 100644
+--- a/hw/timer/trace-events
++++ b/hw/timer/trace-events
+@@ -114,7 +114,8 @@ hpet_ram_read_reading_counter(uint8_t reg_off, uint64_t cur_tick) "reading count
+ hpet_ram_read_invalid(void) "invalid hpet_ram_readl"
+ hpet_ram_write(uint64_t addr, uint64_t value) "enter hpet_ram_writel at 0x%" PRIx64 " = 0x%" PRIx64
+ hpet_ram_write_timer_id(uint64_t timer_id) "hpet_ram_writel timer_id = 0x%" PRIx64
+-hpet_ram_write_tn_cfg(uint8_t reg_off) "hpet_ram_writel HPET_TN_CFG + %" PRIu8
++hpet_ram_write_tn_cfg(void) "hpet_ram_writel HPET_TN_CFG"
++hpet_ram_write_invalid_tn_cfg(uint8_t reg_off) "invalid HPET_TN_CFG + %" PRIu8 " write"
+ hpet_ram_write_tn_cmp(uint8_t reg_off) "hpet_ram_writel HPET_TN_CMP + %" PRIu8
+ hpet_ram_write_invalid_tn_cmp(void) "invalid HPET_TN_CMP + 4 write"
+ hpet_ram_write_invalid(void) "invalid hpet_ram_writel"
diff --git a/debian/patches/pve/0054-Revert-hpet-place-read-only-bits-directly-in-new_val.patch b/debian/patches/pve/0054-Revert-hpet-place-read-only-bits-directly-in-new_val.patch
new file mode 100644
index 0000000..85b0989
--- /dev/null
+++ b/debian/patches/pve/0054-Revert-hpet-place-read-only-bits-directly-in-new_val.patch
@@ -0,0 +1,64 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Fiona Ebner <f.ebner at proxmox.com>
+Date: Wed, 19 Mar 2025 17:31:10 +0100
+Subject: [PATCH] Revert "hpet: place read-only bits directly in "new_val""
+
+This reverts commit ba88935b0fac2588b0a739f810b58dfabf7f92c8.
+
+Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
+---
+ hw/timer/hpet.c | 15 ++++++++-------
+ 1 file changed, 8 insertions(+), 7 deletions(-)
+
+diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
+index e1ac877759..b12bbaf10d 100644
+--- a/hw/timer/hpet.c
++++ b/hw/timer/hpet.c
+@@ -510,7 +510,7 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+ {
+     int i;
+     HPETState *s = opaque;
+-    uint64_t old_val, new_val, cleared;
++    uint64_t old_val, new_val, val;
+ 
+     trace_hpet_ram_write(addr, value);
+     old_val = hpet_ram_read(opaque, addr, 4);
+@@ -536,12 +536,13 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+                  */
+                 update_irq(timer, 0);
+             }
+-            new_val = hpet_fixup_reg(new_val, old_val, HPET_TN_CFG_WRITE_MASK);
+-            timer->config = (timer->config & 0xffffffff00000000ULL) | new_val;
++            val = hpet_fixup_reg(new_val, old_val, HPET_TN_CFG_WRITE_MASK);
++            timer->config = (timer->config & 0xffffffff00000000ULL) | val;
+             if (activating_bit(old_val, new_val, HPET_TN_ENABLE)
+                 && (s->isr & (1 << timer_id))) {
+                 update_irq(timer, 1);
+             }
++
+             if (new_val & HPET_TN_32BIT) {
+                 timer->cmp = (uint32_t)timer->cmp;
+                 timer->period = (uint32_t)timer->period;
+@@ -622,8 +623,8 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+         case HPET_ID:
+             return;
+         case HPET_CFG:
+-            new_val = hpet_fixup_reg(new_val, old_val, HPET_CFG_WRITE_MASK);
+-            s->config = (s->config & 0xffffffff00000000ULL) | new_val;
++            val = hpet_fixup_reg(new_val, old_val, HPET_CFG_WRITE_MASK);
++            s->config = (s->config & 0xffffffff00000000ULL) | val;
+             if (activating_bit(old_val, new_val, HPET_CFG_ENABLE)) {
+                 /* Enable main counter and interrupt generation. */
+                 s->hpet_offset =
+@@ -657,9 +658,9 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+             trace_hpet_invalid_hpet_cfg(4);
+             break;
+         case HPET_STATUS:
+-            cleared = new_val & s->isr;
++            val = new_val & s->isr;
+             for (i = 0; i < s->num_timers; i++) {
+-                if (cleared & (1 << i)) {
++                if (val & (1 << i)) {
+                     update_irq(&s->timer[i], 0);
+                 }
+             }
diff --git a/debian/patches/pve/0055-Revert-hpet-remove-unnecessary-variable-index.patch b/debian/patches/pve/0055-Revert-hpet-remove-unnecessary-variable-index.patch
new file mode 100644
index 0000000..c8aa6ad
--- /dev/null
+++ b/debian/patches/pve/0055-Revert-hpet-remove-unnecessary-variable-index.patch
@@ -0,0 +1,68 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Fiona Ebner <f.ebner at proxmox.com>
+Date: Wed, 19 Mar 2025 17:31:11 +0100
+Subject: [PATCH] Revert "hpet: remove unnecessary variable "index""
+
+This reverts commit 5895879aca252f4ebb2d1078eaf836c61ec54e9b.
+
+Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
+---
+ hw/timer/hpet.c | 15 ++++++++-------
+ 1 file changed, 8 insertions(+), 7 deletions(-)
+
+diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
+index b12bbaf10d..6f83d88516 100644
+--- a/hw/timer/hpet.c
++++ b/hw/timer/hpet.c
+@@ -437,12 +437,12 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr,
+                               unsigned size)
+ {
+     HPETState *s = opaque;
+-    uint64_t cur_tick;
++    uint64_t cur_tick, index;
+ 
+     trace_hpet_ram_read(addr);
+-
++    index = addr;
+     /*address range of all TN regs*/
+-    if (addr >= 0x100 && addr <= 0x3ff) {
++    if (index >= 0x100 && index <= 0x3ff) {
+         uint8_t timer_id = (addr - 0x100) / 0x20;
+         HPETTimer *timer = &s->timer[timer_id];
+ 
+@@ -469,7 +469,7 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr,
+             break;
+         }
+     } else {
+-        switch (addr) {
++        switch (index) {
+         case HPET_ID:
+             return s->capability;
+         case HPET_PERIOD:
+@@ -510,14 +510,15 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+ {
+     int i;
+     HPETState *s = opaque;
+-    uint64_t old_val, new_val, val;
++    uint64_t old_val, new_val, val, index;
+ 
+     trace_hpet_ram_write(addr, value);
++    index = addr;
+     old_val = hpet_ram_read(opaque, addr, 4);
+     new_val = value;
+ 
+     /*address range of all TN regs*/
+-    if (addr >= 0x100 && addr <= 0x3ff) {
++    if (index >= 0x100 && index <= 0x3ff) {
+         uint8_t timer_id = (addr - 0x100) / 0x20;
+         HPETTimer *timer = &s->timer[timer_id];
+ 
+@@ -619,7 +620,7 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+         }
+         return;
+     } else {
+-        switch (addr) {
++        switch (index) {
+         case HPET_ID:
+             return;
+         case HPET_CFG:
diff --git a/debian/patches/pve/0056-Revert-hpet-ignore-high-bits-of-comparator-in-32-bit.patch b/debian/patches/pve/0056-Revert-hpet-ignore-high-bits-of-comparator-in-32-bit.patch
new file mode 100644
index 0000000..0fde8dd
--- /dev/null
+++ b/debian/patches/pve/0056-Revert-hpet-ignore-high-bits-of-comparator-in-32-bit.patch
@@ -0,0 +1,40 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Fiona Ebner <f.ebner at proxmox.com>
+Date: Wed, 19 Mar 2025 17:31:12 +0100
+Subject: [PATCH] Revert "hpet: ignore high bits of comparator in 32-bit mode"
+
+This reverts commit 9eb7fad3546a89ee7cf0e90f5b1daccf89725cea.
+
+Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
+---
+ hw/timer/hpet.c       | 4 ----
+ hw/timer/trace-events | 1 -
+ 2 files changed, 5 deletions(-)
+
+diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
+index 6f83d88516..509986c0a9 100644
+--- a/hw/timer/hpet.c
++++ b/hw/timer/hpet.c
+@@ -585,10 +585,6 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+             }
+             break;
+         case HPET_TN_CMP + 4: // comparator register high order
+-            if (timer->config & HPET_TN_32BIT) {
+-                trace_hpet_ram_write_invalid_tn_cmp();
+-                break;
+-            }
+             trace_hpet_ram_write_tn_cmp(4);
+             if (!timer_is_periodic(timer)
+                 || (timer->config & HPET_TN_SETVAL)) {
+diff --git a/hw/timer/trace-events b/hw/timer/trace-events
+index 219747df2f..b86870fb22 100644
+--- a/hw/timer/trace-events
++++ b/hw/timer/trace-events
+@@ -117,7 +117,6 @@ hpet_ram_write_timer_id(uint64_t timer_id) "hpet_ram_writel timer_id = 0x%" PRIx
+ hpet_ram_write_tn_cfg(void) "hpet_ram_writel HPET_TN_CFG"
+ hpet_ram_write_invalid_tn_cfg(uint8_t reg_off) "invalid HPET_TN_CFG + %" PRIu8 " write"
+ hpet_ram_write_tn_cmp(uint8_t reg_off) "hpet_ram_writel HPET_TN_CMP + %" PRIu8
+-hpet_ram_write_invalid_tn_cmp(void) "invalid HPET_TN_CMP + 4 write"
+ hpet_ram_write_invalid(void) "invalid hpet_ram_writel"
+ hpet_ram_write_counter_write_while_enabled(void) "Writing counter while HPET enabled!"
+ hpet_ram_write_counter_written(uint8_t reg_off, uint64_t value, uint64_t counter) "HPET counter + %" PRIu8 "written. crt = 0x%" PRIx64 " -> 0x%" PRIx64
diff --git a/debian/patches/pve/0057-Revert-hpet-fix-and-cleanup-persistence-of-interrupt.patch b/debian/patches/pve/0057-Revert-hpet-fix-and-cleanup-persistence-of-interrupt.patch
new file mode 100644
index 0000000..0c58bae
--- /dev/null
+++ b/debian/patches/pve/0057-Revert-hpet-fix-and-cleanup-persistence-of-interrupt.patch
@@ -0,0 +1,120 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Fiona Ebner <f.ebner at proxmox.com>
+Date: Wed, 19 Mar 2025 17:31:13 +0100
+Subject: [PATCH] Revert "hpet: fix and cleanup persistence of interrupt
+ status"
+
+This reverts commit f0ccf770789e48b7a73497b465fdc892d28c1339.
+
+Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
+---
+ hw/timer/hpet.c | 60 ++++++++++++++++---------------------------------
+ 1 file changed, 19 insertions(+), 41 deletions(-)
+
+diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
+index 509986c0a9..402cc960f0 100644
+--- a/hw/timer/hpet.c
++++ b/hw/timer/hpet.c
+@@ -196,31 +196,21 @@ static void update_irq(struct HPETTimer *timer, int set)
+     }
+     s = timer->state;
+     mask = 1 << timer->tn;
+-
+-    if (set && (timer->config & HPET_TN_TYPE_LEVEL)) {
+-        /*
+-         * If HPET_TN_ENABLE bit is 0, "the timer will still operate and
+-         * generate appropriate status bits, but will not cause an interrupt"
+-         */
+-        s->isr |= mask;
+-    } else {
++    if (!set || !timer_enabled(timer) || !hpet_enabled(timer->state)) {
+         s->isr &= ~mask;
+-    }
+-
+-    if (set && timer_enabled(timer) && hpet_enabled(s)) {
+-        if (timer_fsb_route(timer)) {
+-            address_space_stl_le(&address_space_memory, timer->fsb >> 32,
+-                                 timer->fsb & 0xffffffff, MEMTXATTRS_UNSPECIFIED,
+-                                 NULL);
+-        } else if (timer->config & HPET_TN_TYPE_LEVEL) {
+-            qemu_irq_raise(s->irqs[route]);
+-        } else {
+-            qemu_irq_pulse(s->irqs[route]);
+-        }
+-    } else {
+         if (!timer_fsb_route(timer)) {
+             qemu_irq_lower(s->irqs[route]);
+         }
++    } else if (timer_fsb_route(timer)) {
++        address_space_stl_le(&address_space_memory, timer->fsb >> 32,
++                             timer->fsb & 0xffffffff, MEMTXATTRS_UNSPECIFIED,
++                             NULL);
++    } else if (timer->config & HPET_TN_TYPE_LEVEL) {
++        s->isr |= mask;
++        qemu_irq_raise(s->irqs[route]);
++    } else {
++        s->isr &= ~mask;
++        qemu_irq_pulse(s->irqs[route]);
+     }
+ }
+ 
+@@ -424,13 +414,8 @@ static void hpet_set_timer(HPETTimer *t)
+ 
+ static void hpet_del_timer(HPETTimer *t)
+ {
+-    HPETState *s = t->state;
+     timer_del(t->qemu_timer);
+-
+-    if (s->isr & (1 << t->tn)) {
+-        /* For level-triggered interrupt, this leaves ISR set but lowers irq.  */
+-        update_irq(t, 1);
+-    }
++    update_irq(t, 0);
+ }
+ 
+ static uint64_t hpet_ram_read(void *opaque, hwaddr addr,
+@@ -530,26 +515,20 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+         switch ((addr - 0x100) % 0x20) {
+         case HPET_TN_CFG:
+             trace_hpet_ram_write_tn_cfg();
+-            if (deactivating_bit(old_val, new_val, HPET_TN_TYPE_LEVEL)) {
+-                /*
+-                 * Do this before changing timer->config; otherwise, if
+-                 * HPET_TN_FSB is set, update_irq will not lower the qemu_irq.
+-                 */
++            if (activating_bit(old_val, new_val, HPET_TN_FSB_ENABLE)) {
+                 update_irq(timer, 0);
+             }
+             val = hpet_fixup_reg(new_val, old_val, HPET_TN_CFG_WRITE_MASK);
+             timer->config = (timer->config & 0xffffffff00000000ULL) | val;
+-            if (activating_bit(old_val, new_val, HPET_TN_ENABLE)
+-                && (s->isr & (1 << timer_id))) {
+-                update_irq(timer, 1);
+-            }
+-
+             if (new_val & HPET_TN_32BIT) {
+                 timer->cmp = (uint32_t)timer->cmp;
+                 timer->period = (uint32_t)timer->period;
+             }
+-            if (hpet_enabled(s)) {
++            if (activating_bit(old_val, new_val, HPET_TN_ENABLE) &&
++                hpet_enabled(s)) {
+                 hpet_set_timer(timer);
++            } else if (deactivating_bit(old_val, new_val, HPET_TN_ENABLE)) {
++                hpet_del_timer(timer);
+             }
+             break;
+         case HPET_TN_CFG + 4: // Interrupt capabilities
+@@ -627,10 +606,9 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
+                 s->hpet_offset =
+                     ticks_to_ns(s->hpet_counter) - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+                 for (i = 0; i < s->num_timers; i++) {
+-                    if (timer_enabled(&s->timer[i]) && (s->isr & (1 << i))) {
+-                        update_irq(&s->timer[i], 1);
++                    if ((&s->timer[i])->cmp != ~0ULL) {
++                        hpet_set_timer(&s->timer[i]);
+                     }
+-                    hpet_set_timer(&s->timer[i]);
+                 }
+             } else if (deactivating_bit(old_val, new_val, HPET_CFG_ENABLE)) {
+                 /* Halt main counter and disable interrupt generation. */
diff --git a/debian/patches/series b/debian/patches/series
index b780c1f..c077e88 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -65,3 +65,10 @@ pve/0047-PVE-backup-factor-out-setting-up-snapshot-access-for.patch
 pve/0048-PVE-backup-save-device-name-in-device-info-structure.patch
 pve/0049-PVE-backup-include-device-name-in-error-when-setting.patch
 pve/0050-adapt-machine-version-deprecation-for-Proxmox-VE.patch
+pve/0051-Revert-hpet-avoid-timer-storms-on-periodic-timers.patch
+pve/0052-Revert-hpet-store-full-64-bit-target-value-of-the-co.patch
+pve/0053-Revert-hpet-accept-64-bit-reads-and-writes.patch
+pve/0054-Revert-hpet-place-read-only-bits-directly-in-new_val.patch
+pve/0055-Revert-hpet-remove-unnecessary-variable-index.patch
+pve/0056-Revert-hpet-ignore-high-bits-of-comparator-in-32-bit.patch
+pve/0057-Revert-hpet-fix-and-cleanup-persistence-of-interrupt.patch
-- 
2.39.5





More information about the pve-devel mailing list