Re: [PATCH 2/5] rust/hpet: move hpet_offset to HPETRegisters

[Zhao Liu]

> @@ -596,11 +598,11 @@ fn callback(&self, regs: &mut HPETRegisters) {
>              } else {
>                  tn_regs.cmp = tn_regs.cmp64;
>              }
> -            self.arm_timer(tn_regs, tn_regs.cmp64);
>          } else if tn_regs.wrap_flag != 0 {
>              tn_regs.wrap_flag = 0;
> -            self.arm_timer(tn_regs, tn_regs.cmp64);
>          }
> +        let next_tick = tn_regs.cmp64;
> +        self.arm_timer(regs, next_tick);

I didn't notice this in previous review... arming timer unconditionally
is wrong, since the code block is:

if tn_regs.is_periodic() && tn_regs.period != 0 {
    ...
    self.arm_timer(tn_regs, tn_regs.cmp64);
} else if tn_regs.wrap_flag != 0 {
    ...
    self.arm_timer(tn_regs, tn_regs.cmp64);
}

So one-shot mode (with wrap_flag == 0) shouldn't arm timer again,
(otherwise, the guest will hang).

---
diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs
index 19676af74bc6..179bd18e2045 100644
--- a/rust/hw/timer/hpet/src/device.rs
+++ b/rust/hw/timer/hpet/src/device.rs
@@ -586,23 +586,33 @@ fn callback(&self, regs: &mut HPETRegisters) {
         // Mutex<HPETRegisters>.
         assert!(bql::is_locked());

-        let cur_tick: u64 = regs.get_ticks();
-        let tn_regs = &mut regs.tn_regs[self.index as usize];
+        let next_tick = {
+            let cur_tick: u64 = regs.get_ticks();
+            let tn_regs = &mut regs.tn_regs[self.index as usize];

-        if tn_regs.is_periodic() && tn_regs.period != 0 {
-            while hpet_time_after(cur_tick, tn_regs.cmp64) {
-                tn_regs.cmp64 += tn_regs.period;
-            }
-            if tn_regs.is_32bit_mod() {
-                tn_regs.cmp = u64::from(tn_regs.cmp64 as u32); // truncate!
+            if tn_regs.is_periodic() && tn_regs.period != 0 {
+                while hpet_time_after(cur_tick, tn_regs.cmp64) {
+                    tn_regs.cmp64 += tn_regs.period;
+                }
+                if tn_regs.is_32bit_mod() {
+                    tn_regs.cmp = u64::from(tn_regs.cmp64 as u32); // truncate!
+                } else {
+                    tn_regs.cmp = tn_regs.cmp64;
+                }
+
+                Some(tn_regs.cmp64)
+            } else if tn_regs.wrap_flag != 0 {
+                tn_regs.wrap_flag = 0;
+
+                Some(tn_regs.cmp64)
             } else {
-                tn_regs.cmp = tn_regs.cmp64;
+                None
             }
-        } else if tn_regs.wrap_flag != 0 {
-            tn_regs.wrap_flag = 0;
+        };
+
+        if let Some(t) = next_tick {
+            self.arm_timer(regs, t);
         }
-        let next_tick = tn_regs.cmp64;
-        self.arm_timer(regs, next_tick);
         self.update_irq(regs, true);
     }