qemu-rust.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: qemu-rust@nongnu.org
Subject: [PATCH 05/10] rust: qdev: add clock creation
Date: Fri, 17 Jan 2025 20:39:58 +0100	[thread overview]
Message-ID: <20250117194003.1173231-6-pbonzini@redhat.com> (raw)
In-Reply-To: <20250117194003.1173231-1-pbonzini@redhat.com>

Add a Rust version of qdev_init_clock_in, which can be used in
instance_init.  There are a couple differences with the C
version:

- in Rust the object keeps its own reference to the clock (in addition to
  the one embedded in the NamedClockList), and the reference is dropped
  automatically by instance_finalize(); this is encoded in the signature
  of DeviceClassMethods::init_clock_in, which makes the lifetime of the
  clock independent of that of the object it holds.  This goes unnoticed
  in the C version and is due to the existence of aliases.

- also, anything that happens during instance_init uses the pinned_init
  framework to operate on a partially initialized object, and is done
  through class methods (i.e. through DeviceClassMethods rather than
  DeviceMethods) because the device does not exist yet.  Therefore, Rust
  code *must* create clocks from instance_init, which is stricter than C.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs | 28 +++++-------
 rust/qemu-api/src/prelude.rs     |  2 +
 rust/qemu-api/src/qdev.rs        | 76 ++++++++++++++++++++++++++++++--
 rust/qemu-api/src/vmstate.rs     |  4 +-
 4 files changed, 87 insertions(+), 23 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index d8409f3d310..dfe199ad0ed 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -13,8 +13,8 @@
     c_str, impl_vmstate_forward,
     irq::InterruptSource,
     prelude::*,
-    qdev::DeviceImpl,
-    qom::{ClassInitImpl, ObjectImpl, ParentField},
+    qdev::{Clock, ClockEvent, DeviceImpl},
+    qom::{ClassInitImpl, ObjectImpl, Owned, ParentField},
 };
 
 use crate::{
@@ -123,7 +123,7 @@ pub struct PL011State {
     #[doc(alias = "irq")]
     pub interrupts: [InterruptSource; IRQMASK.len()],
     #[doc(alias = "clk")]
-    pub clock: NonNull<Clock>,
+    pub clock: Owned<Clock>,
     #[doc(alias = "migrate_clk")]
     pub migrate_clock: bool,
 }
@@ -487,8 +487,6 @@ impl PL011State {
     /// location/instance. All its fields are expected to hold unitialized
     /// values with the sole exception of `parent_obj`.
     unsafe fn init(&mut self) {
-        const CLK_NAME: &CStr = c_str!("clk");
-
         // SAFETY:
         //
         // self and self.iomem are guaranteed to be valid at this point since callers
@@ -508,22 +506,16 @@ unsafe fn init(&mut self) {
 
         // SAFETY:
         //
-        // self.clock is not initialized at this point; but since `NonNull<_>` is Copy,
-        // we can overwrite the undefined value without side effects. This is
+        // self.clock is not initialized at this point; but since `NonNull<_>` is
+        // not Drop, we can overwrite the undefined value without side effects. This is
         // safe since all PL011State instances are created by QOM code which
         // calls this function to initialize the fields; therefore no code is
         // able to access an invalid self.clock value.
-        unsafe {
-            let dev: &mut DeviceState = self.upcast_mut();
-            self.clock = NonNull::new(qdev_init_clock_in(
-                dev,
-                CLK_NAME.as_ptr(),
-                None, /* pl011_clock_update */
-                addr_of_mut!(*self).cast::<c_void>(),
-                ClockEvent::ClockUpdate.0,
-            ))
-            .unwrap();
-        }
+        self.clock = self.init_clock_in("clk", &Self::clock_update, ClockEvent::ClockUpdate);
+    }
+
+    const fn clock_update(&self, _event: ClockEvent) {
+        /* pl011_trace_baudrate_change(s); */
     }
 
     fn post_init(&self) {
diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs
index 3df6a5c21ec..87e3ce90f26 100644
--- a/rust/qemu-api/src/prelude.rs
+++ b/rust/qemu-api/src/prelude.rs
@@ -7,6 +7,8 @@
 pub use crate::cell::BqlCell;
 pub use crate::cell::BqlRefCell;
 
+pub use crate::qdev::DeviceMethods;
+
 pub use crate::qom::IsA;
 pub use crate::qom::Object;
 pub use crate::qom::ObjectCast;
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 4658409bebb..f573712550e 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -4,14 +4,19 @@
 
 //! Bindings to create devices and access device functionality from Rust.
 
-use std::ffi::CStr;
+use std::{
+    ffi::{CStr, CString},
+    os::raw::c_void,
+};
 
-pub use bindings::{DeviceClass, DeviceState, Property};
+pub use bindings::{Clock, ClockEvent, DeviceClass, DeviceState, Property};
 
 use crate::{
     bindings::{self, Error},
+    callbacks::FnCall,
+    cell::bql_locked,
     prelude::*,
-    qom::{ClassInitImpl, ObjectClass},
+    qom::{ClassInitImpl, ObjectClass, Owned},
     vmstate::VMStateDescription,
 };
 
@@ -145,3 +150,68 @@ unsafe impl ObjectType for DeviceState {
         unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_DEVICE) };
 }
 qom_isa!(DeviceState: Object);
+
+pub trait DeviceMethods: ObjectDeref
+where
+    Self::Target: IsA<DeviceState>,
+{
+    #[inline]
+    fn init_clock_in<F: for<'a> FnCall<(&'a Self::Target, ClockEvent)>>(
+        &self,
+        name: &str,
+        _cb: &F,
+        events: ClockEvent,
+    ) -> Owned<Clock> {
+        fn do_init_clock_in(
+            dev: *mut DeviceState,
+            name: &str,
+            cb: Option<unsafe extern "C" fn(*mut c_void, ClockEvent)>,
+            events: ClockEvent,
+        ) -> Owned<Clock> {
+            assert!(bql_locked());
+
+            // SAFETY: the clock is heap allocated and does not have a reference, so
+            // Owned::from adds one.  the callback is disabled automatically
+            // when the clock is unparented, which happens before the device is
+            // finalized.
+            unsafe {
+                let cstr = CString::new(name).unwrap();
+                let clk = bindings::qdev_init_clock_in(
+                    dev,
+                    cstr.as_ptr(),
+                    cb,
+                    dev.cast::<c_void>(),
+                    events.0,
+                );
+
+                Owned::from(&*clk)
+            }
+        }
+
+        let cb: Option<unsafe extern "C" fn(*mut c_void, ClockEvent)> = if F::is_some() {
+            unsafe extern "C" fn rust_clock_cb<T, F: for<'a> FnCall<(&'a T, ClockEvent)>>(
+                opaque: *mut c_void,
+                event: ClockEvent,
+            ) {
+                // SAFETY: the opaque is "this", which is indeed a pointer to T
+                F::call((unsafe { &*(opaque.cast::<T>()) }, event))
+            }
+            Some(rust_clock_cb::<Self::Target, F>)
+        } else {
+            None
+        };
+
+        // SAFETY: self can be cast to DeviceState because its type has an
+        // IsA<DeviceState> bound.
+        do_init_clock_in(unsafe { self.as_mut_ptr() }, name, cb, events)
+    }
+}
+
+impl<R: ObjectDeref> DeviceMethods for R where R::Target: IsA<DeviceState> {}
+
+unsafe impl ObjectType for Clock {
+    type Class = ObjectClass;
+    const TYPE_NAME: &'static CStr =
+        unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_CLOCK) };
+}
+qom_isa!(Clock: Object);
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 11d21b8791c..164effc6553 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -470,11 +470,11 @@ macro_rules! vmstate_clock {
                 $crate::assert_field_type!(
                     $struct_name,
                     $field_name,
-                    core::ptr::NonNull<$crate::bindings::Clock>
+                    $crate::qom::Owned<$crate::bindings::Clock>
                 );
                 $crate::offset_of!($struct_name, $field_name)
             },
-            size: ::core::mem::size_of::<*const $crate::bindings::Clock>(),
+            size: ::core::mem::size_of::<*const $crate::qdev::Clock>(),
             flags: VMStateFlags(VMStateFlags::VMS_STRUCT.0 | VMStateFlags::VMS_POINTER.0),
             vmsd: unsafe { ::core::ptr::addr_of!($crate::bindings::vmstate_clock) },
             ..$crate::zeroable::Zeroable::ZERO
-- 
2.47.1



  parent reply	other threads:[~2025-01-17 19:40 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-17 19:39 [RFC PATCH 00/10] rust: remaining part of qdev bindings Paolo Bonzini
2025-01-17 19:39 ` [PATCH 01/10] rust: qemu-api: add sub-subclass to the integration tests Paolo Bonzini
2025-01-20 16:40   ` Zhao Liu
2025-01-17 19:39 ` [PATCH 02/10] rust: qom: add reference counting functionality Paolo Bonzini
2025-01-26 15:15   ` Zhao Liu
2025-01-29 10:03     ` Paolo Bonzini
2025-02-05  8:28       ` Zhao Liu
2025-01-27  7:57   ` Zhao Liu
2025-01-29 10:16     ` Paolo Bonzini
2025-02-05  9:13       ` Zhao Liu
2025-02-05  9:10         ` Paolo Bonzini
2025-02-05  9:40           ` Zhao Liu
2025-02-06  3:26   ` Zhao Liu
2025-01-17 19:39 ` [PATCH 03/10] rust: qom: add object creation functionality Paolo Bonzini
2025-02-06  7:49   ` Zhao Liu
2025-02-06  7:39     ` Paolo Bonzini
2025-01-17 19:39 ` [PATCH 04/10] rust: callbacks: allow passing optional callbacks as () Paolo Bonzini
2025-01-27  8:41   ` Zhao Liu
2025-01-17 19:39 ` Paolo Bonzini [this message]
2025-02-06  8:15   ` [PATCH 05/10] rust: qdev: add clock creation Zhao Liu
2025-01-17 19:39 ` [PATCH 06/10] rust: qom: allow initializing interface vtables Paolo Bonzini
2025-01-27 10:33   ` Zhao Liu
2025-01-17 19:40 ` [PATCH 07/10] rust: qdev: make ObjectImpl a supertrait of DeviceImpl Paolo Bonzini
2025-01-27  9:10   ` Zhao Liu
2025-02-06  8:37   ` Philippe Mathieu-Daudé
2025-01-17 19:40 ` [PATCH 08/10] rust: qdev: switch from legacy reset to Resettable Paolo Bonzini
2025-01-27 10:31   ` Zhao Liu
2025-01-27 18:01     ` Paolo Bonzini
2025-01-28  9:25       ` Zhao Liu
2025-02-06  8:31   ` Zhao Liu
2025-01-17 19:40 ` [PATCH 09/10] rust: bindings: add Sync markers to types referred to by MemoryRegionOps Paolo Bonzini
2025-01-27 10:58   ` Zhao Liu
2025-01-17 19:40 ` [PATCH 10/10] rust: bindings for MemoryRegionOps Paolo Bonzini
2025-01-27 12:12   ` Zhao Liu
2025-01-27 18:11     ` Paolo Bonzini
2025-02-06  9:15       ` Zhao Liu
2025-02-06  9:15         ` Paolo Bonzini
2025-02-06  8:39   ` Philippe Mathieu-Daudé
2025-02-06  8:46     ` Paolo Bonzini
2025-02-06 10:02       ` Philippe Mathieu-Daudé
2025-02-06 10:19         ` Paolo Bonzini
2025-02-10 10:38           ` Philippe Mathieu-Daudé
2025-01-24  2:46 ` [RFC PATCH 00/10] rust: remaining part of qdev bindings Zhao Liu

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=20250117194003.1173231-6-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-rust@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).