qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Subject: [PULL 06/25] rust: hpet: embed Timer without the Option and Box indirection
Date: Sun,  9 Mar 2025 11:31:00 +0100	[thread overview]
Message-ID: <20250309103120.1116448-7-pbonzini@redhat.com> (raw)
In-Reply-To: <20250309103120.1116448-1-pbonzini@redhat.com>

This simplifies things for migration, since Option<Box<QEMUTimer>> does not
implement VMState.

This also shows a soundness issue because Timer::new() will leave a NULL
timer list pointer, which can then be dereferenced by Timer::modify().  It
will be fixed shortly.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/timer/hpet/src/hpet.rs | 59 ++++++++++++++++------------------
 1 file changed, 28 insertions(+), 31 deletions(-)

diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
index be27eb0eff4..02c81ae048f 100644
--- a/rust/hw/timer/hpet/src/hpet.rs
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -151,14 +151,14 @@ fn timer_handler(timer_cell: &BqlRefCell<HPETTimer>) {
 
 /// HPET Timer Abstraction
 #[repr(C)]
-#[derive(Debug, Default, qemu_api_macros::offsets)]
+#[derive(Debug, qemu_api_macros::offsets)]
 pub struct HPETTimer {
     /// timer N index within the timer block (`HPETState`)
     #[doc(alias = "tn")]
     index: usize,
-    qemu_timer: Option<Box<Timer>>,
+    qemu_timer: Timer,
     /// timer block abstraction containing this timer
-    state: Option<NonNull<HPETState>>,
+    state: NonNull<HPETState>,
 
     // Memory-mapped, software visible timer registers
     /// Timer N Configuration and Capability Register
@@ -181,32 +181,34 @@ pub struct HPETTimer {
 }
 
 impl HPETTimer {
-    fn init(&mut self, index: usize, state_ptr: *mut HPETState) -> &mut Self {
-        *self = HPETTimer::default();
-        self.index = index;
-        self.state = NonNull::new(state_ptr);
-        self
-    }
+    fn init(&mut self, index: usize, state: &HPETState) {
+        *self = HPETTimer {
+            index,
+            qemu_timer: Timer::new(),
+            state: NonNull::new(state as *const _ as *mut _).unwrap(),
+            config: 0,
+            cmp: 0,
+            fsb: 0,
+            cmp64: 0,
+            period: 0,
+            wrap_flag: 0,
+            last: 0,
+        };
 
-    fn init_timer_with_state(&mut self) {
-        self.qemu_timer = Some(Box::new({
-            let mut t = Timer::new();
-            t.init_full(
-                None,
-                CLOCK_VIRTUAL,
-                Timer::NS,
-                0,
-                timer_handler,
-                &self.get_state().timers[self.index],
-            );
-            t
-        }));
+        self.qemu_timer.init_full(
+            None,
+            CLOCK_VIRTUAL,
+            Timer::NS,
+            0,
+            timer_handler,
+            &state.timers[self.index],
+        )
     }
 
     fn get_state(&self) -> &HPETState {
         // SAFETY:
         // the pointer is convertible to a reference
-        unsafe { self.state.unwrap().as_ref() }
+        unsafe { self.state.as_ref() }
     }
 
     fn is_int_active(&self) -> bool {
@@ -330,7 +332,7 @@ fn arm_timer(&mut self, tick: u64) {
         }
 
         self.last = ns;
-        self.qemu_timer.as_ref().unwrap().modify(self.last);
+        self.qemu_timer.modify(self.last);
     }
 
     fn set_timer(&mut self) {
@@ -353,7 +355,7 @@ fn set_timer(&mut self) {
     fn del_timer(&mut self) {
         // Just remove the timer from the timer_list without destroying
         // this timer instance.
-        self.qemu_timer.as_ref().unwrap().delete();
+        self.qemu_timer.delete();
 
         if self.is_int_active() {
             // For level-triggered interrupt, this leaves interrupt status
@@ -581,13 +583,8 @@ fn handle_legacy_irq(&self, irq: u32, level: u32) {
     }
 
     fn init_timer(&self) {
-        let raw_ptr: *mut HPETState = self as *const HPETState as *mut HPETState;
-
         for (index, timer) in self.timers.iter().enumerate() {
-            timer
-                .borrow_mut()
-                .init(index, raw_ptr)
-                .init_timer_with_state();
+            timer.borrow_mut().init(index, self);
         }
     }
 
-- 
2.48.1



  parent reply	other threads:[~2025-03-09 10:33 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-09 10:30 [PULL 00/25] (Mostly) Rust patches for QEMU 10.0 soft freeze Paolo Bonzini
2025-03-09 10:30 ` [PULL 01/25] chardev: express dependency on io/ Paolo Bonzini
2025-03-09 10:30 ` [PULL 02/25] scripts: dump stdin on meson-buildoptions error Paolo Bonzini
2025-03-09 10:30 ` [PULL 03/25] rust: cell: add wrapper for FFI types Paolo Bonzini
2025-03-09 10:30 ` [PULL 04/25] rust: qemu_api_macros: add Wrapper derive macro Paolo Bonzini
2025-03-09 10:30 ` [PULL 05/25] rust: vmstate: add std::pin::Pin as transparent wrapper Paolo Bonzini
2025-03-09 10:31 ` Paolo Bonzini [this message]
2025-03-09 10:31 ` [PULL 07/25] rust: timer: wrap QEMUTimer with Opaque<> and express pinning requirements Paolo Bonzini
2025-03-09 10:31 ` [PULL 08/25] rust: irq: wrap IRQState with Opaque<> Paolo Bonzini
2025-03-09 10:31 ` [PULL 09/25] rust: qom: wrap Object " Paolo Bonzini
2025-03-09 10:31 ` [PULL 10/25] rust: qdev: wrap Clock and DeviceState " Paolo Bonzini
2025-03-09 10:31 ` [PULL 11/25] rust: hpet: do not access fields of SysBusDevice Paolo Bonzini
2025-03-09 10:31 ` [PULL 12/25] rust: sysbus: wrap SysBusDevice with Opaque<> Paolo Bonzini
2025-03-09 10:31 ` [PULL 13/25] rust: memory: wrap MemoryRegion " Paolo Bonzini
2025-03-09 10:31 ` [PULL 14/25] rust: chardev: wrap Chardev " Paolo Bonzini
2025-03-09 10:31 ` [PULL 15/25] rust: bindings: remove more unnecessary Send/Sync impls Paolo Bonzini
2025-03-09 10:31 ` [PULL 16/25] rust: chardev: provide basic bindings to character devices Paolo Bonzini
2025-03-09 10:31 ` [PULL 17/25] rust: pl011: move register definitions out of lib.rs Paolo Bonzini
2025-03-09 10:31 ` [PULL 18/25] rust: pl011: clean up visibilities of callbacks Paolo Bonzini
2025-03-09 10:31 ` [PULL 19/25] rust: pl011: switch to safe chardev operation Paolo Bonzini
2025-03-19 19:25   ` Peter Maydell
2025-03-19 20:51     ` Paolo Bonzini
2025-03-20 10:39       ` Peter Maydell
2025-03-09 10:31 ` [PULL 20/25] rust: pl011: pass around registers::Data Paolo Bonzini
2025-03-09 10:31 ` [PULL 21/25] rust: hpet: decode HPET registers into enums Paolo Bonzini
2025-03-21 12:03   ` Peter Maydell
2025-03-09 10:31 ` [PULL 22/25] rust: cell: add full example of declaring a SysBusDevice Paolo Bonzini
2025-03-09 10:31 ` [PULL 23/25] rust: qom: remove operations on &mut Paolo Bonzini
2025-03-09 10:31 ` [PULL 24/25] meson.build: default to -gsplit-dwarf for debug info Paolo Bonzini
2025-03-09 10:31 ` [PULL 25/25] rust: pl011: Allow NULL chardev argument to pl011_create() Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250309103120.1116448-7-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).