[PATCH 04/12] rust: timer: wrap QEMUTimer with Opaque<> and express pinning requirements

[Paolo Bonzini]

Timers must be pinned in memory, because modify() stores a pointer to them
in the TimerList.  To ensure this is the case, replace the separate new()
and init_full() with a single function that returns a pinned box.  Because
the only way to obtain a Timer is through Timer::new_full(), modify()
knows that the timer it got is also pinned.  In the future the pinning
requirement will be expressed through the pin_init crate instead.

Note that Timer is a bit different from other users of Opaque, in that
it is created in Rust code rather than C code.  This is why it has to
use the unsafe Opaque::new() function.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                    |  7 -----
 rust/hw/timer/hpet/src/hpet.rs | 23 ++++++++---------
 rust/qemu-api/src/timer.rs     | 47 ++++++++++++++++++++++------------
 3 files changed, 41 insertions(+), 36 deletions(-)

diff --git a/meson.build b/meson.build
index 0a2c61d2bfa..fc02e5fc763 100644
--- a/meson.build
+++ b/meson.build
@@ -4098,13 +4098,6 @@ if have_rust
   foreach enum : c_bitfields
     bindgen_args += ['--bitfield-enum', enum]
   endforeach
-  c_nocopy = [
-    'QEMUTimer',
-  ]
-  # Used to customize Drop trait
-  foreach struct : c_nocopy
-    bindgen_args += ['--no-copy', struct]
-  endforeach
 
   # TODO: Remove this comment when the clang/libclang mismatch issue is solved.
   #
diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
index be27eb0eff4..ce4b289d0c8 100644
--- a/rust/hw/timer/hpet/src/hpet.rs
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -4,6 +4,7 @@
 
 use std::{
     ffi::CStr,
+    pin::Pin,
     ptr::{addr_of_mut, null_mut, NonNull},
     slice::from_ref,
 };
@@ -156,7 +157,7 @@ pub struct HPETTimer {
     /// timer N index within the timer block (`HPETState`)
     #[doc(alias = "tn")]
     index: usize,
-    qemu_timer: Option<Box<Timer>>,
+    qemu_timer: Option<Pin<Box<Timer>>>,
     /// timer block abstraction containing this timer
     state: Option<NonNull<HPETState>>,
 
@@ -189,18 +190,14 @@ fn init(&mut self, index: usize, state_ptr: *mut 
HPETState) -> &mut Self {
     }
 
     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 = Some(Timer::new_full(
+            None,
+            CLOCK_VIRTUAL,
+            Timer::NS,
+            0,
+            timer_handler,
+            &self.get_state().timers[self.index],
+        ));
     }
 
     fn get_state(&self) -> &HPETState {
diff --git a/rust/qemu-api/src/timer.rs b/rust/qemu-api/src/timer.rs
index a593538917a..a8b2ab058b6 100644
--- a/rust/qemu-api/src/timer.rs
+++ b/rust/qemu-api/src/timer.rs
@@ -2,40 +2,51 @@
 // Author(s): Zhao Liu <zhai1.liu@intel.com>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use std::os::raw::{c_int, c_void};
+use std::{
+    os::raw::{c_int, c_void},
+    pin::Pin,
+};
 
 use crate::{
     bindings::{self, qemu_clock_get_ns, timer_del, timer_init_full, timer_mod, 
QEMUClockType},
     callbacks::FnCall,
+    cell::Opaque,
 };
 
-pub type Timer = bindings::QEMUTimer;
-pub type TimerListGroup = bindings::QEMUTimerListGroup;
+/// A safe wrapper around [`bindings::QEMUTimer`].
+#[repr(transparent)]
+#[derive(Debug, qemu_api_macros::Wrapper)]
+pub struct Timer(Opaque<bindings::QEMUTimer>);
+
+unsafe impl Send for Timer {}
+unsafe impl Sync for Timer {}
+
+#[repr(transparent)]
+#[derive(qemu_api_macros::Wrapper)]
+pub struct TimerListGroup(Opaque<bindings::QEMUTimerListGroup>);
+
+unsafe impl Send for TimerListGroup {}
+unsafe impl Sync for TimerListGroup {}
 
 impl Timer {
     pub const MS: u32 = bindings::SCALE_MS;
     pub const US: u32 = bindings::SCALE_US;
     pub const NS: u32 = bindings::SCALE_NS;
 
-    pub fn new() -> Self {
-        Default::default()
-    }
-
-    const fn as_mut_ptr(&self) -> *mut Self {
-        self as *const Timer as *mut _
-    }
-
-    pub fn init_full<'timer, 'opaque: 'timer, T, F>(
-        &'timer mut self,
+    pub fn new_full<'opaque, T, F>(
         timer_list_group: Option<&TimerListGroup>,
         clk_type: ClockType,
         scale: u32,
         attributes: u32,
         _cb: F,
         opaque: &'opaque T,
-    ) where
+    ) -> Pin<Box<Self>>
         F: for<'a> FnCall<(&'a T,)>,
     {
+        // SAFETY: returning a pinned box makes it certain that the object
+        // will not move when added to a TimerList with `modify()`.
+        let t = unsafe { Box::pin(Self(Opaque::new())) };
         let _: () = F::ASSERT_IS_SOME;
 
         /// timer expiration callback
@@ -51,7 +62,7 @@ pub fn init_full<'timer, 'opaque: 'timer, T, F>(
         // SAFETY: the opaque outlives the timer
         unsafe {
             timer_init_full(
-                self,
+                t.as_mut_ptr(),
                 if let Some(g) = timer_list_group {
                     g as *const TimerListGroup as *mut _
                 } else {
@@ -62,11 +73,14 @@ pub fn init_full<'timer, 'opaque: 'timer, T, F>(
                 attributes as c_int,
                 Some(timer_cb),
                 (opaque as *const T).cast::<c_void>() as *mut c_void,
-            )
+            );
         }
+        t
     }
 
     pub fn modify(&self, expire_time: u64) {
+        // SAFETY: the only way to obtain a Timer is via methods that return a
+        // Pin<Box<Self>>, therefore the timer is pinned
         unsafe { timer_mod(self.as_mut_ptr(), expire_time as i64) }
     }
 
@@ -75,6 +89,7 @@ pub fn delete(&self) {
     }
 }
 
+// FIXME: use something like PinnedDrop from the pinned_init crate
 impl Drop for Timer {
     fn drop(&mut self) {
         self.delete()
-- 
2.48.1