From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id 9B9131FF17C
for <inbox@lore.proxmox.com>; Wed, 19 Mar 2025 17:40:12 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id DA71FE9E3;
Wed, 19 Mar 2025 17:40:11 +0100 (CET)
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Wed, 19 Mar 2025 17:39:30 +0100
Message-Id: <20250319163930.294610-1-f.ebner@proxmox.com>
X-Mailer: git-send-email 2.39.5
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.168 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
KAM_LOTSOFHASH 0.25 Emails with lots of hash-like gibberish
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: [pve-devel] [PATCH qemu] revert hpet changes to fix performance
regression
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
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@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@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@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@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@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@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@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@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@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@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@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@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@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@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@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
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel