rust: timer: wrap QEMUTimer with Opaque<> and express pinning requirements

Timers must be pinned in memory, because modify() stores a pointer to them
in the TimerList.  To express this requirement, change init_full() to take
a pinned reference.  Because the only way to obtain a Timer is through
Timer::new(), which is unsafe, modify() can assume that the timer it got
was later initialized; and because the initialization takes a Pin<&mut
Timer> modify() can assume that the timer is 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 constructors provided by Opaque; and in fact Timer::new()
is also unsafe, because it leaves it to the caller to invoke init_full()
before modify().  Without a call to init_full(), modify() will cause a
NULL pointer dereference.

An alternative could be to combine new() + init_full() by returning a
pinned box; however, using a reference makes it easier to express
the requirement that the opaque outlives the timer.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

1 files changed, 36 insertions, 11 deletions

diff --git a/rust/qemu-api/src/timer.rs b/rust/qemu-api/src/timer.rs
index a593538917..f0b04ef95d 100644
--- a/rust/qemu-api/src/timer.rs
+++ b/rust/qemu-api/src/timer.rs
@@ -2,31 +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 _
+    /// Create a `Timer` struct without initializing it.
+    ///
+    /// # Safety
+    ///
+    /// The timer must be initialized before it is armed with
+    /// [`modify`](Self::modify).
+    pub unsafe fn new() -> Self {
+        // SAFETY: requirements relayed to callers of Timer::new
+        Self(unsafe { Opaque::zeroed() })
     }
 
+    /// Create a new timer with the given attributes.
     pub fn init_full<'timer, 'opaque: 'timer, T, F>(
-        &'timer mut self,
+        self: Pin<&'timer mut Self>,
         timer_list_group: Option<&TimerListGroup>,
         clk_type: ClockType,
         scale: u32,
@@ -51,7 +71,7 @@ impl Timer {
         // SAFETY: the opaque outlives the timer
         unsafe {
             timer_init_full(
-                self,
+                self.as_mut_ptr(),
                 if let Some(g) = timer_list_group {
                     g as *const TimerListGroup as *mut _
                 } else {
@@ -67,14 +87,19 @@ impl Timer {
     }
 
     pub fn modify(&self, expire_time: u64) {
+        // SAFETY: the only way to obtain a Timer safely is via methods that
+        // take a Pin<&mut Self>, therefore the timer is pinned
         unsafe { timer_mod(self.as_mut_ptr(), expire_time as i64) }
     }
 
     pub fn delete(&self) {
+        // SAFETY: the only way to obtain a Timer safely is via methods that
+        // take a Pin<&mut Self>, therefore the timer is pinned
         unsafe { timer_del(self.as_mut_ptr()) }
     }
 }
 
+// FIXME: use something like PinnedDrop from the pinned_init crate
 impl Drop for Timer {
     fn drop(&mut self) {
         self.delete()