From b2a4854508a02fc8a585890e0272c8ae5fbad5c1 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 28 Oct 2024 11:28:23 +0100 Subject: rust: qom: add default definitions for ObjectImpl Remove a bunch of duplicate const definitions. Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/definitions.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'rust/qemu-api/src/definitions.rs') diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 26597934bb..92b3c6f911 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -14,10 +14,10 @@ pub trait ObjectImpl { const TYPE_INFO: TypeInfo; const TYPE_NAME: &'static CStr; const PARENT_TYPE_NAME: Option<&'static CStr>; - const ABSTRACT: bool; - const INSTANCE_INIT: Option; - const INSTANCE_POST_INIT: Option; - const INSTANCE_FINALIZE: Option; + const ABSTRACT: bool = false; + const INSTANCE_INIT: Option = None; + const INSTANCE_POST_INIT: Option = None; + const INSTANCE_FINALIZE: Option = None; } pub trait Class { -- cgit 1.4.1 From 93ea0896eaa97adfcc664fa65b5b70e555a652ff Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 28 Oct 2024 13:05:43 +0100 Subject: rust: qom: rename Class trait to ClassInitImpl While at it, document it. Signed-off-by: Paolo Bonzini --- rust/hw/char/pl011/src/device.rs | 4 ++-- rust/qemu-api/src/definitions.rs | 25 ++++++++++++++++++++++--- rust/qemu-api/tests/tests.rs | 4 ++-- 3 files changed, 26 insertions(+), 7 deletions(-) (limited to 'rust/qemu-api/src/definitions.rs') diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index 3d173ae816..bd12067aaf 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -117,7 +117,7 @@ pub struct PL011Class { _inner: [u8; 0], } -impl qemu_api::definitions::Class for PL011Class { +impl qemu_api::definitions::ClassInitImpl for PL011Class { const CLASS_INIT: Option = Some(crate::device_class::pl011_class_init); const CLASS_BASE_INIT: Option< @@ -650,7 +650,7 @@ pub unsafe extern "C" fn pl011_luminary_init(obj: *mut Object) { } } -impl qemu_api::definitions::Class for PL011LuminaryClass { +impl qemu_api::definitions::ClassInitImpl for PL011LuminaryClass { const CLASS_INIT: Option = None; const CLASS_BASE_INIT: Option< diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 92b3c6f911..3291f4242c 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -20,8 +20,27 @@ pub trait ObjectImpl { const INSTANCE_FINALIZE: Option = None; } -pub trait Class { +/// Trait used to fill in a class struct. +/// +/// Each QOM class that has virtual methods describes them in a +/// _class struct_. Class structs include a parent field corresponding +/// to the vtable of the parent class, all the way up to [`ObjectClass`]. +/// Each QOM type has one such class struct. +/// +/// The Rust implementation of methods will usually come from a trait +/// like [`ObjectImpl`]. +pub trait ClassInitImpl { + /// Function that is called after all parent class initialization + /// has occurred. On entry, the virtual method pointers are set to + /// the default values coming from the parent classes; the function + /// can change them to override virtual methods of a parent class. const CLASS_INIT: Option; + + /// Called on descendent classes after all parent class initialization + /// has occurred, but before the class itself is initialized. This + /// is only useful if a class is not a leaf, and can be used to undo + /// the effects of copying the contents of the parent's class struct + /// to the descendants. const CLASS_BASE_INIT: Option< unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void), >; @@ -82,8 +101,8 @@ macro_rules! type_info { instance_finalize: <$t as $crate::definitions::ObjectImpl>::INSTANCE_FINALIZE, abstract_: <$t as $crate::definitions::ObjectImpl>::ABSTRACT, class_size: ::core::mem::size_of::<<$t as $crate::definitions::ObjectImpl>::Class>(), - class_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::Class>::CLASS_INIT, - class_base_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::Class>::CLASS_BASE_INIT, + class_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::ClassInitImpl>::CLASS_INIT, + class_base_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::ClassInitImpl>::CLASS_BASE_INIT, class_data: ::core::ptr::null_mut(), interfaces: ::core::ptr::null_mut(), }; diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs index f793ff26e5..704c63c846 100644 --- a/rust/qemu-api/tests/tests.rs +++ b/rust/qemu-api/tests/tests.rs @@ -7,7 +7,7 @@ use std::{ffi::CStr, os::raw::c_void}; use qemu_api::{ bindings::*, c_str, declare_properties, define_property, - definitions::{Class, ObjectImpl}, + definitions::{ClassInitImpl, ObjectImpl}, device_class, device_class_init, zeroable::Zeroable, }; @@ -60,7 +60,7 @@ fn test_device_decl_macros() { const PARENT_TYPE_NAME: Option<&'static CStr> = Some(device_class::TYPE_DEVICE); } - impl Class for DummyClass { + impl ClassInitImpl for DummyClass { const CLASS_INIT: Option = Some(dummy_class_init); const CLASS_BASE_INIT: Option< -- cgit 1.4.1 From 3701fb22dfd438993e76e158beb97683359d1dd9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 28 Oct 2024 11:47:12 +0100 Subject: rust: qom: convert type_info! macro to an associated const type_info! is only used in the definition of ObjectImpl::TYPE_INFO, and in fact in all of them. Pull type_info!'s definition into the ObjectImpl trait, thus simplifying the external interface of qemu_api::definitions. Signed-off-by: Paolo Bonzini --- rust/hw/char/pl011/src/device.rs | 6 ++--- rust/qemu-api/src/definitions.rs | 50 ++++++++++++++++++---------------------- rust/qemu-api/tests/tests.rs | 1 - 3 files changed, 24 insertions(+), 33 deletions(-) (limited to 'rust/qemu-api/src/definitions.rs') diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index bd12067aaf..bcb146c24d 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -106,7 +106,6 @@ pub struct PL011State { impl ObjectImpl for PL011State { type Class = PL011Class; - const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! { Self }; const TYPE_NAME: &'static CStr = crate::TYPE_PL011; const PARENT_TYPE_NAME: Option<&'static CStr> = Some(TYPE_SYS_BUS_DEVICE); const INSTANCE_INIT: Option = Some(pl011_init); @@ -149,7 +148,7 @@ impl PL011State { addr_of_mut!(*self).cast::(), &PL011_OPS, addr_of_mut!(*self).cast::(), - Self::TYPE_INFO.name, + Self::TYPE_NAME.as_ptr(), 0x1000, ); sysbus_init_mmio(sbd, addr_of_mut!(self.iomem)); @@ -598,7 +597,7 @@ pub unsafe extern "C" fn pl011_create( chr: *mut Chardev, ) -> *mut DeviceState { unsafe { - let dev: *mut DeviceState = qdev_new(PL011State::TYPE_INFO.name); + let dev: *mut DeviceState = qdev_new(PL011State::TYPE_NAME.as_ptr()); let sysbus: *mut SysBusDevice = dev.cast::(); qdev_prop_set_chr(dev, c_str!("chardev").as_ptr(), chr); @@ -660,7 +659,6 @@ impl qemu_api::definitions::ClassInitImpl for PL011LuminaryClass { impl ObjectImpl for PL011Luminary { type Class = PL011LuminaryClass; - const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! { Self }; const TYPE_NAME: &'static CStr = crate::TYPE_PL011_LUMINARY; const PARENT_TYPE_NAME: Option<&'static CStr> = Some(crate::TYPE_PL011); const INSTANCE_INIT: Option = Some(pl011_luminary_init); diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 3291f4242c..6ecfaf51b0 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -9,15 +9,34 @@ use std::{ffi::CStr, os::raw::c_void}; use crate::bindings::{Object, ObjectClass, TypeInfo}; /// Trait a type must implement to be registered with QEMU. -pub trait ObjectImpl { - type Class; - const TYPE_INFO: TypeInfo; +pub trait ObjectImpl: Sized { + type Class: ClassInitImpl; const TYPE_NAME: &'static CStr; const PARENT_TYPE_NAME: Option<&'static CStr>; const ABSTRACT: bool = false; const INSTANCE_INIT: Option = None; const INSTANCE_POST_INIT: Option = None; const INSTANCE_FINALIZE: Option = None; + + const TYPE_INFO: TypeInfo = TypeInfo { + name: Self::TYPE_NAME.as_ptr(), + parent: if let Some(pname) = Self::PARENT_TYPE_NAME { + pname.as_ptr() + } else { + core::ptr::null_mut() + }, + instance_size: core::mem::size_of::(), + instance_align: core::mem::align_of::(), + instance_init: Self::INSTANCE_INIT, + instance_post_init: Self::INSTANCE_POST_INIT, + instance_finalize: Self::INSTANCE_FINALIZE, + abstract_: Self::ABSTRACT, + class_size: core::mem::size_of::(), + class_init: ::CLASS_INIT, + class_base_init: ::CLASS_BASE_INIT, + class_data: core::ptr::null_mut(), + interfaces: core::ptr::null_mut(), + }; } /// Trait used to fill in a class struct. @@ -83,28 +102,3 @@ macro_rules! module_init { } }; } - -#[macro_export] -macro_rules! type_info { - ($t:ty) => { - $crate::bindings::TypeInfo { - name: <$t as $crate::definitions::ObjectImpl>::TYPE_NAME.as_ptr(), - parent: if let Some(pname) = <$t as $crate::definitions::ObjectImpl>::PARENT_TYPE_NAME { - pname.as_ptr() - } else { - ::core::ptr::null_mut() - }, - instance_size: ::core::mem::size_of::<$t>(), - instance_align: ::core::mem::align_of::<$t>(), - instance_init: <$t as $crate::definitions::ObjectImpl>::INSTANCE_INIT, - instance_post_init: <$t as $crate::definitions::ObjectImpl>::INSTANCE_POST_INIT, - instance_finalize: <$t as $crate::definitions::ObjectImpl>::INSTANCE_FINALIZE, - abstract_: <$t as $crate::definitions::ObjectImpl>::ABSTRACT, - class_size: ::core::mem::size_of::<<$t as $crate::definitions::ObjectImpl>::Class>(), - class_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::ClassInitImpl>::CLASS_INIT, - class_base_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::ClassInitImpl>::CLASS_BASE_INIT, - class_data: ::core::ptr::null_mut(), - interfaces: ::core::ptr::null_mut(), - }; - } -} diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs index 704c63c846..7f9df348b0 100644 --- a/rust/qemu-api/tests/tests.rs +++ b/rust/qemu-api/tests/tests.rs @@ -55,7 +55,6 @@ fn test_device_decl_macros() { impl ObjectImpl for DummyState { type Class = DummyClass; - const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! { Self }; const TYPE_NAME: &'static CStr = c_str!("dummy"); const PARENT_TYPE_NAME: Option<&'static CStr> = Some(device_class::TYPE_DEVICE); } -- cgit 1.4.1 From c6c4f3e0d90990c642523c087482eac3f42566c1 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 28 Oct 2024 14:42:23 +0100 Subject: rust: qom: move ClassInitImpl to the instance side Put all traits on the instance struct, which makes it possible to reuse class structs if no new virtual methods or class fields are added. This is almost always the case for devices (because they are leaf classes), which is the primary use case for Rust. This is also simpler: soon we will find the implemented methods without macros, and this removes the need to go from the class struct to the instance struct to find the implementation of the *Impl traits. Signed-off-by: Paolo Bonzini --- rust/hw/char/pl011/src/device.rs | 4 ++-- rust/qemu-api/src/definitions.rs | 8 ++++---- rust/qemu-api/tests/tests.rs | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) (limited to 'rust/qemu-api/src/definitions.rs') diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index bcb146c24d..2384d4bcb9 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -116,7 +116,7 @@ pub struct PL011Class { _inner: [u8; 0], } -impl qemu_api::definitions::ClassInitImpl for PL011Class { +impl qemu_api::definitions::ClassInitImpl for PL011State { const CLASS_INIT: Option = Some(crate::device_class::pl011_class_init); const CLASS_BASE_INIT: Option< @@ -649,7 +649,7 @@ pub unsafe extern "C" fn pl011_luminary_init(obj: *mut Object) { } } -impl qemu_api::definitions::ClassInitImpl for PL011LuminaryClass { +impl qemu_api::definitions::ClassInitImpl for PL011Luminary { const CLASS_INIT: Option = None; const CLASS_BASE_INIT: Option< diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 6ecfaf51b0..487712611f 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -9,8 +9,8 @@ use std::{ffi::CStr, os::raw::c_void}; use crate::bindings::{Object, ObjectClass, TypeInfo}; /// Trait a type must implement to be registered with QEMU. -pub trait ObjectImpl: Sized { - type Class: ClassInitImpl; +pub trait ObjectImpl: ClassInitImpl + Sized { + type Class; const TYPE_NAME: &'static CStr; const PARENT_TYPE_NAME: Option<&'static CStr>; const ABSTRACT: bool = false; @@ -32,8 +32,8 @@ pub trait ObjectImpl: Sized { instance_finalize: Self::INSTANCE_FINALIZE, abstract_: Self::ABSTRACT, class_size: core::mem::size_of::(), - class_init: ::CLASS_INIT, - class_base_init: ::CLASS_BASE_INIT, + class_init: ::CLASS_INIT, + class_base_init: ::CLASS_BASE_INIT, class_data: core::ptr::null_mut(), interfaces: core::ptr::null_mut(), }; diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs index 7f9df348b0..fd0c979121 100644 --- a/rust/qemu-api/tests/tests.rs +++ b/rust/qemu-api/tests/tests.rs @@ -59,7 +59,7 @@ fn test_device_decl_macros() { const PARENT_TYPE_NAME: Option<&'static CStr> = Some(device_class::TYPE_DEVICE); } - impl ClassInitImpl for DummyClass { + impl ClassInitImpl for DummyState { const CLASS_INIT: Option = Some(dummy_class_init); const CLASS_BASE_INIT: Option< -- cgit 1.4.1 From f75fb90ff2af75cd4405fe4c6ba0c0c38a120590 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 12 Nov 2024 17:08:07 +0100 Subject: rust: qdev: move bridge for realize and reset functions out of pl011 Allow the DeviceImpl trait to expose safe Rust functions. rust_device_class_init<> adds thunks around the functions in DeviceImpl. Reviewed-by: Zhao Liu Signed-off-by: Paolo Bonzini --- rust/hw/char/pl011/src/device.rs | 5 ++--- rust/hw/char/pl011/src/device_class.rs | 26 ------------------------ rust/qemu-api/src/definitions.rs | 2 +- rust/qemu-api/src/device_class.rs | 36 ++++++++++++++++++++++++++++------ 4 files changed, 33 insertions(+), 36 deletions(-) (limited to 'rust/qemu-api/src/definitions.rs') diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index 28b1924337..56403c3660 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -125,9 +125,8 @@ impl DeviceImpl for PL011State { fn vmsd() -> Option<&'static VMStateDescription> { Some(&device_class::VMSTATE_PL011) } - const REALIZE: Option = - Some(device_class::pl011_realize); - const RESET: Option = Some(device_class::pl011_reset); + const REALIZE: Option = Some(Self::realize); + const RESET: Option = Some(Self::reset); } impl_device_class!(PL011State); diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs index c61b6bb025..975c3d42be 100644 --- a/rust/hw/char/pl011/src/device_class.rs +++ b/rust/hw/char/pl011/src/device_class.rs @@ -92,29 +92,3 @@ qemu_api::declare_properties! { default = true ), } - -/// # Safety -/// -/// We expect the FFI user of this function to pass a valid pointer, that has -/// the same size as [`PL011State`]. We also expect the device is -/// readable/writeable from one thread at any time. -pub unsafe extern "C" fn pl011_realize(dev: *mut DeviceState, _errp: *mut *mut Error) { - unsafe { - assert!(!dev.is_null()); - let mut state = NonNull::new_unchecked(dev.cast::()); - state.as_mut().realize(); - } -} - -/// # Safety -/// -/// We expect the FFI user of this function to pass a valid pointer, that has -/// the same size as [`PL011State`]. We also expect the device is -/// readable/writeable from one thread at any time. -pub unsafe extern "C" fn pl011_reset(dev: *mut DeviceState) { - unsafe { - assert!(!dev.is_null()); - let mut state = NonNull::new_unchecked(dev.cast::()); - state.as_mut().reset(); - } -} diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 487712611f..0467e6290e 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -47,7 +47,7 @@ pub trait ObjectImpl: ClassInitImpl + Sized { /// Each QOM type has one such class struct. /// /// The Rust implementation of methods will usually come from a trait -/// like [`ObjectImpl`]. +/// like [`ObjectImpl`] or [`DeviceImpl`](crate::device_class::DeviceImpl). pub trait ClassInitImpl { /// Function that is called after all parent class initialization /// has occurred. On entry, the virtual method pointers are set to diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index f683f94f2a..f25904be4f 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -17,14 +17,14 @@ pub trait DeviceImpl { /// /// If not `None`, the parent class's `realize` method is overridden /// with the function pointed to by `REALIZE`. - const REALIZE: Option = None; + const REALIZE: Option = None; /// If not `None`, the parent class's `reset` method is overridden /// with the function pointed to by `RESET`. /// /// Rust does not yet support the three-phase reset protocol; this is /// usually okay for leaf classes. - const RESET: Option = None; + const RESET: Option = None; /// An array providing the properties that the user can set on the /// device. Not a `const` because referencing statics in constants @@ -41,6 +41,30 @@ pub trait DeviceImpl { } } +/// # Safety +/// +/// This function is only called through the QOM machinery and +/// the `impl_device_class!` macro. +/// We expect the FFI user of this function to pass a valid pointer that +/// can be downcasted to type `T`. We also expect the device is +/// readable/writeable from one thread at any time. +unsafe extern "C" fn rust_realize_fn(dev: *mut DeviceState, _errp: *mut *mut Error) { + assert!(!dev.is_null()); + let state = dev.cast::(); + T::REALIZE.unwrap()(unsafe { &mut *state }); +} + +/// # Safety +/// +/// We expect the FFI user of this function to pass a valid pointer that +/// can be downcasted to type `T`. We also expect the device is +/// readable/writeable from one thread at any time. +unsafe extern "C" fn rust_reset_fn(dev: *mut DeviceState) { + assert!(!dev.is_null()); + let state = dev.cast::(); + T::RESET.unwrap()(unsafe { &mut *state }); +} + /// # Safety /// /// We expect the FFI user of this function to pass a valid pointer that @@ -53,11 +77,11 @@ pub unsafe extern "C" fn rust_device_class_init( let mut dc = ::core::ptr::NonNull::new(klass.cast::()).unwrap(); unsafe { let dc = dc.as_mut(); - if let Some(realize_fn) = ::REALIZE { - dc.realize = Some(realize_fn); + if ::REALIZE.is_some() { + dc.realize = Some(rust_realize_fn::); } - if let Some(reset_fn) = ::RESET { - bindings::device_class_set_legacy_reset(dc, Some(reset_fn)); + if ::RESET.is_some() { + bindings::device_class_set_legacy_reset(dc, Some(rust_reset_fn::)); } if let Some(vmsd) = ::vmsd() { dc.vmsd = vmsd; -- cgit 1.4.1 From 1f9d52c9388d14c3f5a605543a8ef53dceaad5bb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 28 Oct 2024 10:45:55 +0100 Subject: rust: qom: move bridge for TypeInfo functions out of pl011 Allow the ObjectImpl trait to expose Rust functions that avoid raw pointers (though INSTANCE_INIT for example is still unsafe). ObjectImpl::TYPE_INFO adds thunks around the functions in ObjectImpl. While at it, document `TypeInfo`. Reviewed-by: Zhao Liu Signed-off-by: Paolo Bonzini --- rust/hw/char/pl011/src/device.rs | 40 ++++++++------------------ rust/qemu-api/src/definitions.rs | 61 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 69 insertions(+), 32 deletions(-) (limited to 'rust/qemu-api/src/definitions.rs') diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index 56403c3660..b9f8fb134b 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -110,7 +110,7 @@ impl ObjectImpl 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); - const INSTANCE_INIT: Option = Some(pl011_init); + const INSTANCE_INIT: Option = Some(Self::init); } #[repr(C)] @@ -615,19 +615,6 @@ pub unsafe extern "C" fn pl011_create( } } -/// # Safety -/// -/// We expect the FFI user of this function to pass a valid pointer, that has -/// the same size as [`PL011State`]. We also expect the device is -/// readable/writeable from one thread at any time. -pub unsafe extern "C" fn pl011_init(obj: *mut Object) { - unsafe { - debug_assert!(!obj.is_null()); - let mut state = NonNull::new_unchecked(obj.cast::()); - state.as_mut().init(); - } -} - #[repr(C)] #[derive(Debug, qemu_api_macros::Object)] /// PL011 Luminary device model. @@ -640,19 +627,16 @@ pub struct PL011LuminaryClass { _inner: [u8; 0], } -/// Initializes a pre-allocated, unitialized instance of `PL011Luminary`. -/// -/// # Safety -/// -/// We expect the FFI user of this function to pass a valid pointer, that has -/// the same size as [`PL011Luminary`]. We also expect the device is -/// readable/writeable from one thread at any time. -pub unsafe extern "C" fn pl011_luminary_init(obj: *mut Object) { - unsafe { - debug_assert!(!obj.is_null()); - let mut state = NonNull::new_unchecked(obj.cast::()); - let state = state.as_mut(); - state.parent_obj.device_id = DeviceId::Luminary; +impl PL011Luminary { + /// Initializes a pre-allocated, unitialized instance of `PL011Luminary`. + /// + /// # Safety + /// + /// We expect the FFI user of this function to pass a valid pointer, that + /// has the same size as [`PL011Luminary`]. We also expect the device is + /// readable/writeable from one thread at any time. + unsafe fn init(&mut self) { + self.parent_obj.device_id = DeviceId::Luminary; } } @@ -660,7 +644,7 @@ impl ObjectImpl 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); - const INSTANCE_INIT: Option = Some(pl011_luminary_init); + const INSTANCE_INIT: Option = Some(Self::init); } impl DeviceImpl for PL011Luminary {} diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 0467e6290e..f297075898 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -8,16 +8,63 @@ use std::{ffi::CStr, os::raw::c_void}; use crate::bindings::{Object, ObjectClass, TypeInfo}; +unsafe extern "C" fn rust_instance_init(obj: *mut Object) { + // SAFETY: obj is an instance of T, since rust_instance_init + // is called from QOM core as the instance_init function + // for class T + unsafe { T::INSTANCE_INIT.unwrap()(&mut *obj.cast::()) } +} + +unsafe extern "C" fn rust_instance_post_init(obj: *mut Object) { + // SAFETY: obj is an instance of T, since rust_instance_post_init + // is called from QOM core as the instance_post_init function + // for class T + // + // FIXME: it's not really guaranteed that there are no backpointers to + // obj; it's quite possible that they have been created by instance_init(). + // The receiver should be &self, not &mut self. + T::INSTANCE_POST_INIT.unwrap()(unsafe { &mut *obj.cast::() }) +} + /// Trait a type must implement to be registered with QEMU. +/// +/// # Safety +/// +/// - the struct must be `#[repr(C)]` +/// +/// - `Class` and `TYPE` must match the data in the `TypeInfo` (this is +/// 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 { + /// 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; + + /// The parent of the type. This should match the first field of + /// the struct that implements `ObjectImpl`: const PARENT_TYPE_NAME: Option<&'static CStr>; + + /// Whether the object can be instantiated const ABSTRACT: bool = false; - const INSTANCE_INIT: Option = None; - const INSTANCE_POST_INIT: Option = None; const INSTANCE_FINALIZE: Option = None; + /// Function that is called to initialize an object. The parent class will + /// have already been initialized so the type is only responsible for + /// initializing its own members. + /// + /// FIXME: The argument is not really a valid reference. `&mut + /// MaybeUninit` would be a better description. + const INSTANCE_INIT: Option = None; + + /// Function that is called to finish initialization of an object, once + /// `INSTANCE_INIT` functions have been called. + const INSTANCE_POST_INIT: Option = None; + const TYPE_INFO: TypeInfo = TypeInfo { name: Self::TYPE_NAME.as_ptr(), parent: if let Some(pname) = Self::PARENT_TYPE_NAME { @@ -27,8 +74,14 @@ pub trait ObjectImpl: ClassInitImpl + Sized { }, instance_size: core::mem::size_of::(), instance_align: core::mem::align_of::(), - instance_init: Self::INSTANCE_INIT, - instance_post_init: Self::INSTANCE_POST_INIT, + instance_init: match Self::INSTANCE_INIT { + None => None, + Some(_) => Some(rust_instance_init::), + }, + instance_post_init: match Self::INSTANCE_POST_INIT { + None => None, + Some(_) => Some(rust_instance_post_init::), + }, instance_finalize: Self::INSTANCE_FINALIZE, abstract_: Self::ABSTRACT, class_size: core::mem::size_of::(), -- cgit 1.4.1 From 7bd8e3ef63330e870cf4644d21c285cce35c703d Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 31 Oct 2024 09:56:15 +0100 Subject: rust: qom: split ObjectType from ObjectImpl trait 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. Reviewed-by: Zhao Liu Signed-off-by: Paolo Bonzini --- rust/hw/char/pl011/src/device.rs | 17 ++++++++++++----- rust/qemu-api/src/definitions.rs | 27 +++++++++++++++++++++------ 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 | 17 +++++++++-------- 6 files changed, 58 insertions(+), 26 deletions(-) (limited to 'rust/qemu-api/src/definitions.rs') diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index b9f8fb134b..0ab825b1ca 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -12,9 +12,10 @@ use qemu_api::{ 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(::TYPE_NAME); const INSTANCE_INIT: Option = Some(Self::init); } @@ -640,10 +644,13 @@ impl PL011Luminary { } } -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(::TYPE_NAME); const INSTANCE_INIT: Option = Some(Self::init); } diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index f297075898..b98a692678 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -26,25 +26,40 @@ unsafe extern "C" fn rust_instance_post_init(obj: *mut Object) { T::INSTANCE_POST_INIT.unwrap()(unsafe { &mut *obj.cast::() }) } -/// Trait a type must implement to be registered with QEMU. +/// Trait exposed by all structs corresponding to QOM objects. /// /// # Safety /// -/// - the struct must be `#[repr(C)]` +/// For classes declared in C: +/// +/// - `Class` and `TYPE` must match the data in the `TypeInfo`; +/// +/// - the first field of the struct must be of the instance type corresponding +/// to the superclass, as declared in the `TypeInfo` +/// +/// - likewise, the first field of the `Class` struct must be of the class type +/// corresponding to the superclass /// -/// - `Class` and `TYPE` must match the data in the `TypeInfo` (this is -/// automatic if the class is defined via `ObjectImpl`). +/// For classes declared in Rust and implementing [`ObjectImpl`]: +/// +/// - the struct must be `#[repr(C)]`; /// /// - 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, as declared in `ObjectImpl::PARENT_TYPE_NAME` +/// +/// - likewise, the first field of the `Class` must be of the class struct +/// corresponding to the superclass +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 f25904be4f..03d03feee8 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -6,6 +6,7 @@ use std::{ffi::CStr, os::raw::c_void}; 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 a39e228bab..1b8677b2d9 100644 --- a/rust/qemu-api/src/prelude.rs +++ b/rust/qemu-api/src/prelude.rs @@ -6,3 +6,5 @@ pub use crate::bitops::IntegerExt; 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 4e192c7589..5ee068541c 100644 --- a/rust/qemu-api/src/sysbus.rs +++ b/rust/qemu-api/src/sysbus.rs @@ -2,11 +2,17 @@ // Author(s): Paolo Bonzini // 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 b8b12a4042..1d027dd652 100644 --- a/rust/qemu-api/tests/tests.rs +++ b/rust/qemu-api/tests/tests.rs @@ -5,12 +5,8 @@ use std::ffi::CStr; use qemu_api::{ - bindings::*, - c_str, declare_properties, define_property, - definitions::ObjectImpl, - device_class::{self, DeviceImpl}, - impl_device_class, - zeroable::Zeroable, + bindings::*, c_str, declare_properties, define_property, definitions::ObjectImpl, + device_class::DeviceImpl, impl_device_class, prelude::*, zeroable::Zeroable, }; #[test] @@ -46,10 +42,15 @@ fn test_device_decl_macros() { ), } - 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(::TYPE_NAME); + const ABSTRACT: bool = false; } impl DeviceImpl for DummyState { -- cgit 1.4.1 From 166e8a1fd15bfa527b25fc15ca315e572c0556d2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Sun, 24 Nov 2024 18:51:34 +0100 Subject: rust: qom: change the parent type to an associated type Avoid duplicated code to retrieve the QOM type strings from the Rust type. Reviewed-by: Zhao Liu Signed-off-by: Paolo Bonzini --- rust/hw/char/pl011/src/device.rs | 6 ++++-- rust/qemu-api/src/definitions.rs | 12 ++++-------- rust/qemu-api/tests/tests.rs | 3 +-- 3 files changed, 9 insertions(+), 12 deletions(-) (limited to 'rust/qemu-api/src/definitions.rs') diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index 0ab825b1ca..3e29442a62 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -113,7 +113,8 @@ unsafe impl ObjectType for PL011State { } impl ObjectImpl for PL011State { - const PARENT_TYPE_NAME: Option<&'static CStr> = Some(::TYPE_NAME); + type ParentType = SysBusDevice; + const INSTANCE_INIT: Option = Some(Self::init); } @@ -650,7 +651,8 @@ unsafe impl ObjectType for PL011Luminary { } impl ObjectImpl for PL011Luminary { - const PARENT_TYPE_NAME: Option<&'static CStr> = Some(::TYPE_NAME); + type ParentType = PL011State; + const INSTANCE_INIT: Option = Some(Self::init); } diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index b98a692678..df91a2e31a 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -45,10 +45,10 @@ unsafe extern "C" fn rust_instance_post_init(obj: *mut Object) { /// - the struct must be `#[repr(C)]`; /// /// - the first field of the struct must be of the instance struct corresponding -/// to the superclass, as declared in `ObjectImpl::PARENT_TYPE_NAME` +/// to the superclass, which is `ObjectImpl::ParentType` /// /// - likewise, the first field of the `Class` must be of the class struct -/// corresponding to the superclass +/// corresponding to the superclass, which is `ObjectImpl::ParentType::Class`. pub unsafe trait ObjectType: Sized { /// The QOM class object corresponding to this struct. Not used yet. type Class; @@ -62,7 +62,7 @@ pub unsafe trait ObjectType: Sized { 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>; + type ParentType: ObjectType; /// Whether the object can be instantiated const ABSTRACT: bool = false; @@ -82,11 +82,7 @@ pub trait ObjectImpl: ObjectType + ClassInitImpl { const TYPE_INFO: TypeInfo = TypeInfo { name: Self::TYPE_NAME.as_ptr(), - parent: if let Some(pname) = Self::PARENT_TYPE_NAME { - pname.as_ptr() - } else { - core::ptr::null_mut() - }, + parent: Self::ParentType::TYPE_NAME.as_ptr(), instance_size: core::mem::size_of::(), instance_align: core::mem::align_of::(), instance_init: match Self::INSTANCE_INIT { diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs index 1d027dd652..278efe967f 100644 --- a/rust/qemu-api/tests/tests.rs +++ b/rust/qemu-api/tests/tests.rs @@ -48,8 +48,7 @@ fn test_device_decl_macros() { } impl ObjectImpl for DummyState { - const PARENT_TYPE_NAME: Option<&'static CStr> = - Some(::TYPE_NAME); + type ParentType = DeviceState; const ABSTRACT: bool = false; } -- cgit 1.4.1