diff mbox series

[15/26] rust: qom: split ObjectType from ObjectImpl trait

Message ID 20241209123717.99077-16-pbonzini@redhat.com (mailing list archive)
State New
Headers show
Series rust: bundle of prerequisites for HPET implementation | expand

Commit Message

Paolo Bonzini Dec. 9, 2024, 12:37 p.m. UTC
Define a separate trait for fields that also applies to classes that are
defined by C code.  This makes it possible to add metadata to core classes,
which has multiple uses:

- it makes it possible to access the parent struct's TYPE_* for types
  that are defined in Rust code, and to avoid repeating it in every subclass

- implementors of ObjectType will be allowed to implement the IsA<> trait and
  therefore to perform typesafe casts from one class to another.

- in the future, an ObjectType could be created with Foo::new() in a type-safe
  manner, without having to pass a TYPE_* constant.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs  | 17 ++++++++++++-----
 rust/qemu-api/src/definitions.rs  |  9 ++++++---
 rust/qemu-api/src/device_class.rs | 11 ++++++-----
 rust/qemu-api/src/prelude.rs      |  2 ++
 rust/qemu-api/src/sysbus.rs       | 10 ++++++++--
 rust/qemu-api/tests/tests.rs      | 12 +++++++++---
 6 files changed, 43 insertions(+), 18 deletions(-)

Comments

Zhao Liu Dec. 11, 2024, 8:41 a.m. UTC | #1
On Mon, Dec 09, 2024 at 01:37:06PM +0100, Paolo Bonzini wrote:
> Date: Mon,  9 Dec 2024 13:37:06 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 15/26] rust: qom: split ObjectType from ObjectImpl trait
> X-Mailer: git-send-email 2.47.1
> 
> Define a separate trait for fields that also applies to classes that are
> defined by C code.  This makes it possible to add metadata to core classes,
> which has multiple uses:
> 
> - it makes it possible to access the parent struct's TYPE_* for types
>   that are defined in Rust code, and to avoid repeating it in every subclass
> 
> - implementors of ObjectType will be allowed to implement the IsA<> trait and
>   therefore to perform typesafe casts from one class to another.
> 
> - in the future, an ObjectType could be created with Foo::new() in a type-safe
>   manner, without having to pass a TYPE_* constant.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/hw/char/pl011/src/device.rs  | 17 ++++++++++++-----
>  rust/qemu-api/src/definitions.rs  |  9 ++++++---
>  rust/qemu-api/src/device_class.rs | 11 ++++++-----
>  rust/qemu-api/src/prelude.rs      |  2 ++
>  rust/qemu-api/src/sysbus.rs       | 10 ++++++++--
>  rust/qemu-api/tests/tests.rs      | 12 +++++++++---
>  6 files changed, 43 insertions(+), 18 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
diff mbox series

Patch

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index b9f8fb134b5..0ab825b1ca4 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -12,9 +12,10 @@ 
     bindings::{self, *},
     c_str,
     definitions::ObjectImpl,
-    device_class::{DeviceImpl, TYPE_SYS_BUS_DEVICE},
+    device_class::DeviceImpl,
     impl_device_class,
     irq::InterruptSource,
+    prelude::*,
 };
 
 use crate::{
@@ -106,10 +107,13 @@  pub struct PL011State {
     device_id: DeviceId,
 }
 
-impl ObjectImpl for PL011State {
+unsafe impl ObjectType for PL011State {
     type Class = PL011Class;
     const TYPE_NAME: &'static CStr = crate::TYPE_PL011;
-    const PARENT_TYPE_NAME: Option<&'static CStr> = Some(TYPE_SYS_BUS_DEVICE);
+}
+
+impl ObjectImpl for PL011State {
+    const PARENT_TYPE_NAME: Option<&'static CStr> = Some(<SysBusDevice as ObjectType>::TYPE_NAME);
     const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
 }
 
@@ -640,10 +644,13 @@  unsafe fn init(&mut self) {
     }
 }
 
-impl ObjectImpl for PL011Luminary {
+unsafe impl ObjectType for PL011Luminary {
     type Class = PL011LuminaryClass;
     const TYPE_NAME: &'static CStr = crate::TYPE_PL011_LUMINARY;
-    const PARENT_TYPE_NAME: Option<&'static CStr> = Some(crate::TYPE_PL011);
+}
+
+impl ObjectImpl for PL011Luminary {
+    const PARENT_TYPE_NAME: Option<&'static CStr> = Some(<PL011State as ObjectType>::TYPE_NAME);
     const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
 }
 
diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs
index 078ba30d61a..1c412dbc876 100644
--- a/rust/qemu-api/src/definitions.rs
+++ b/rust/qemu-api/src/definitions.rs
@@ -33,7 +33,7 @@ 
     unsafe { std::ptr::drop_in_place(obj.cast::<T>()) }
 }
 
-/// Trait a type must implement to be registered with QEMU.
+/// Trait exposed by all structs corresponding to QOM objects.
 ///
 /// # Safety
 ///
@@ -43,15 +43,18 @@ 
 ///   automatic if the class is defined via `ObjectImpl`).
 ///
 /// - the first field of the struct must be of the instance struct corresponding
-///   to the superclass declared as `PARENT_TYPE_NAME`
-pub trait ObjectImpl: ClassInitImpl + Sized {
+///   to the superclass declared in the `TypeInfo`
+pub unsafe trait ObjectType: Sized {
     /// The QOM class object corresponding to this struct.  Not used yet.
     type Class;
 
     /// The name of the type, which can be passed to `object_new()` to
     /// generate an instance of this type.
     const TYPE_NAME: &'static CStr;
+}
 
+/// Trait a type must implement to be registered with QEMU.
+pub trait ObjectImpl: ObjectType + ClassInitImpl {
     /// The parent of the type.  This should match the first field of
     /// the struct that implements `ObjectImpl`:
     const PARENT_TYPE_NAME: Option<&'static CStr>;
diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
index f25904be4f6..03d03feee83 100644
--- a/rust/qemu-api/src/device_class.rs
+++ b/rust/qemu-api/src/device_class.rs
@@ -6,6 +6,7 @@ 
 
 use crate::{
     bindings::{self, DeviceClass, DeviceState, Error, ObjectClass, Property, VMStateDescription},
+    prelude::*,
     zeroable::Zeroable,
 };
 
@@ -146,8 +147,8 @@  macro_rules! declare_properties {
     };
 }
 
-// workaround until we can use --generate-cstr in bindgen.
-pub const TYPE_DEVICE: &CStr =
-    unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_DEVICE) };
-pub const TYPE_SYS_BUS_DEVICE: &CStr =
-    unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_SYS_BUS_DEVICE) };
+unsafe impl ObjectType for bindings::DeviceState {
+    type Class = bindings::DeviceClass;
+    const TYPE_NAME: &'static CStr =
+        unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_DEVICE) };
+}
diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs
index a39e228babf..1b8677b2d9a 100644
--- a/rust/qemu-api/src/prelude.rs
+++ b/rust/qemu-api/src/prelude.rs
@@ -6,3 +6,5 @@ 
 
 pub use crate::cell::BqlCell;
 pub use crate::cell::BqlRefCell;
+
+pub use crate::definitions::ObjectType;
diff --git a/rust/qemu-api/src/sysbus.rs b/rust/qemu-api/src/sysbus.rs
index 4e192c75898..5ee068541cf 100644
--- a/rust/qemu-api/src/sysbus.rs
+++ b/rust/qemu-api/src/sysbus.rs
@@ -2,11 +2,17 @@ 
 // Author(s): Paolo Bonzini <pbonzini@redhat.com>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use std::ptr::addr_of;
+use std::{ffi::CStr, ptr::addr_of};
 
 pub use bindings::{SysBusDevice, SysBusDeviceClass};
 
-use crate::{bindings, cell::bql_locked, irq::InterruptSource};
+use crate::{bindings, cell::bql_locked, irq::InterruptSource, prelude::*};
+
+unsafe impl ObjectType for SysBusDevice {
+    type Class = SysBusDeviceClass;
+    const TYPE_NAME: &'static CStr =
+        unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_SYS_BUS_DEVICE) };
+}
 
 impl SysBusDevice {
     /// Return `self` cast to a mutable pointer, for use in calls to C code.
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index b8b12a40422..1be03eb685c 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -8,8 +8,9 @@ 
     bindings::*,
     c_str, declare_properties, define_property,
     definitions::ObjectImpl,
-    device_class::{self, DeviceImpl},
+    device_class::DeviceImpl,
     impl_device_class,
+    prelude::*,
     zeroable::Zeroable,
 };
 
@@ -46,10 +47,15 @@  pub struct DummyClass {
             ),
     }
 
-    impl ObjectImpl for DummyState {
+    unsafe impl ObjectType for DummyState {
         type Class = DummyClass;
         const TYPE_NAME: &'static CStr = c_str!("dummy");
-        const PARENT_TYPE_NAME: Option<&'static CStr> = Some(device_class::TYPE_DEVICE);
+    }
+
+    impl ObjectImpl for DummyState {
+        const PARENT_TYPE_NAME: Option<&'static CStr> =
+            Some(<DeviceState as ObjectType>::TYPE_NAME);
+        const ABSTRACT: bool = false;
     }
 
     impl DeviceImpl for DummyState {