On Sat, Jan 25, 2025 at 1:32 PM Zhao Liu <zhao1....@intel.com> wrote:
+// Register space for each timer block. (HPET_BASE isn't defined here.)
+const HPET_REG_SPACE_LEN: u64 = 0x400; // 1024 bytes
Use doc comments "///"...
+// Timer N FSB Interrupt Route Register (masked by 0x18)
+const HPET_TN_FSB_ROUTE_REG: u64 = 0x010;
... all the way to here.
+/// HPET Timer Abstraction
+#[repr(C)]
+#[derive(Debug, Default, qemu_api_macros::offsets)]
+pub struct HPETTimer {
+ /// timer N index within the timer block (HPETState)
+ #[doc(alias = "tn")]
+ index: usize,
+ qemu_timer: Option<Box<QEMUTimer>>,
+ /// timer block abstraction containing this timer
+ state: Option<NonNull<HPETState>>,
+
+ /// Memory-mapped, software visible timer registers
These "headings" cannot be doc comments, because they would be attached
to the field after. Same below:
- /// Hidden register state
+ // Hidden register state
- /// HPET block Registers: Memory-mapped, software visible registers
+ // HPET block Registers: Memory-mapped, software visible registers
- /// Internal state
+ // Internal state
+ fn get_state(&self) -> &HPETState {
+ // SAFETY:
+ // the pointer is convertible to a reference
+ unsafe { self.state.unwrap().as_ref() }
+ }
Note for the future: I looked briefly into why a NonNull<> is needed
and the reasons are two. First, the offset_of! replacement does not
support lifetime parameters. Second, it's messy to pass a &HPETState
to the &HPETTimer, because the HPETTimer still has to be written into
the HPETState. This may be worth revisiting when QEMU starts using
pinned-init is added, but for now is self-contained.
+ if let Some(timer) = self.qemu_timer.as_mut() {
+ timer.timer_mod(self.last);
+ }
I think this can be just "timer.unwrap().timer_mod(self.last)" (not
sure if it needs an as_ref() too).
Paolo
