Parameterize the implementation of ClassInitImpl so that it is possible to call up the chain of implementations, one superclass at a time starting at ClassInitImpl<Self::Class>.
In order to avoid having to implement (for example) ClassInitImpl<PL011Class>, also remove the dummy PL011Class and PL011LuminaryClass structs and specify the same ObjectType::Class as the superclass. In the future this default behavior can be handled by a procedural macro, by looking at the first field in the struct. Note that the new trait is safe: the calls are started by rust_class_init<>(), which is not public and can convert the class pointer to a Rust reference. Since CLASS_BASE_INIT applies to the type that is being defined, and only to it, move it to ObjectImpl. Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> --- rust/hw/char/pl011/src/device.rs | 19 +------ rust/qemu-api/src/definitions.rs | 85 +++++++++++++++++++++++-------- rust/qemu-api/src/device_class.rs | 50 +++++++----------- rust/qemu-api/src/sysbus.rs | 18 ++++++- rust/qemu-api/tests/tests.rs | 9 +--- 5 files changed, 101 insertions(+), 80 deletions(-) diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index 3e29442a625..d9e9f35f456 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -13,7 +13,6 @@ c_str, definitions::ObjectImpl, device_class::DeviceImpl, - impl_device_class, irq::InterruptSource, prelude::*, }; @@ -108,7 +107,7 @@ pub struct PL011State { } unsafe impl ObjectType for PL011State { - type Class = PL011Class; + type Class = <SysBusDevice as ObjectType>::Class; const TYPE_NAME: &'static CStr = crate::TYPE_PL011; } @@ -118,11 +117,6 @@ impl ObjectImpl for PL011State { const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init); } -#[repr(C)] -pub struct PL011Class { - _inner: [u8; 0], -} - impl DeviceImpl for PL011State { fn properties() -> &'static [Property] { &device_class::PL011_PROPERTIES @@ -134,8 +128,6 @@ fn vmsd() -> Option<&'static VMStateDescription> { const RESET: Option<fn(&mut Self)> = Some(Self::reset); } -impl_device_class!(PL011State); - impl PL011State { /// Initializes a pre-allocated, unitialized instance of `PL011State`. /// @@ -627,11 +619,6 @@ pub struct PL011Luminary { parent_obj: PL011State, } -#[repr(C)] -pub struct PL011LuminaryClass { - _inner: [u8; 0], -} - impl PL011Luminary { /// Initializes a pre-allocated, unitialized instance of `PL011Luminary`. /// @@ -646,7 +633,7 @@ unsafe fn init(&mut self) { } unsafe impl ObjectType for PL011Luminary { - type Class = PL011LuminaryClass; + type Class = <PL011State as ObjectType>::Class; const TYPE_NAME: &'static CStr = crate::TYPE_PL011_LUMINARY; } @@ -657,5 +644,3 @@ impl ObjectImpl for PL011Luminary { } impl DeviceImpl for PL011Luminary {} - -impl_device_class!(PL011Luminary); diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs index 1975ad91867..2429b9f53f0 100644 --- a/rust/qemu-api/src/definitions.rs +++ b/rust/qemu-api/src/definitions.rs @@ -33,6 +33,16 @@ unsafe { std::ptr::drop_in_place(obj.cast::<T>()) } } +unsafe extern "C" fn rust_class_init<T: ObjectType + ClassInitImpl<T::Class>>( + klass: *mut ObjectClass, + _data: *mut c_void, +) { + // SAFETY: klass is a T::Class, since rust_class_init<T> + // is called from QOM core as the class_init function + // for class T + T::class_init(unsafe { &mut *klass.cast::<T::Class>() }) +} + /// Trait exposed by all structs corresponding to QOM objects. /// /// # Safety @@ -45,7 +55,8 @@ /// - the first field of the struct must be of the instance struct corresponding /// to the superclass declared in the `TypeInfo` pub unsafe trait ObjectType: Sized { - /// The QOM class object corresponding to this struct. Not used yet. + /// The QOM class object corresponding to this struct. This is used + /// to automatically generate a `class_init` method. type Class; /// The name of the type, which can be passed to `object_new()` to @@ -54,7 +65,7 @@ pub unsafe trait ObjectType: Sized { } /// Trait a type must implement to be registered with QEMU. -pub trait ObjectImpl: ObjectType + ClassInitImpl { +pub trait ObjectImpl: ObjectType + ClassInitImpl<Self::Class> { /// The parent of the type. This should match the first field of /// the struct that implements `ObjectImpl`: type ParentType: ObjectType; @@ -74,6 +85,15 @@ pub trait ObjectImpl: ObjectType + ClassInitImpl { /// `INSTANCE_INIT` functions have been called. const INSTANCE_POST_INIT: Option<fn(&mut Self)> = None; + /// 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), + > = None; + const TYPE_INFO: TypeInfo = TypeInfo { name: Self::TYPE_NAME.as_ptr(), parent: Self::ParentType::TYPE_NAME.as_ptr(), @@ -90,37 +110,60 @@ pub trait ObjectImpl: ObjectType + ClassInitImpl { instance_finalize: Some(drop_object::<Self>), abstract_: Self::ABSTRACT, class_size: core::mem::size_of::<Self::Class>(), - class_init: <Self as ClassInitImpl>::CLASS_INIT, - class_base_init: <Self as ClassInitImpl>::CLASS_BASE_INIT, + class_init: Some(rust_class_init::<Self>), + class_base_init: Self::CLASS_BASE_INIT, class_data: core::ptr::null_mut(), interfaces: core::ptr::null_mut(), }; } -/// Trait used to fill in a class struct. +/// Internal trait used to automatically 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. +/// Each QOM type has one such class struct; this trait takes care of +/// initializing the `T` part of the class struct, for the type that +/// implements the trait. /// -/// The Rust implementation of methods will usually come from a trait -/// 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 +/// Each struct will implement this trait with `T` equal to each +/// superclass. For example, a device should implement at least +/// `ClassInitImpl<`[`DeviceClass`](crate::bindings::DeviceClass)`>`. +/// +/// Fortunately, this is almost never necessary. Instead, the Rust +/// implementation of methods will usually come from a trait like +/// [`ObjectImpl`] or [`DeviceImpl`](crate::device_class::DeviceImpl). +/// `ClassInitImpl` then can be provided by blanket implementations +/// that operate on all implementors of the `*Impl`* trait. For example: +/// +/// ```ignore +/// impl<T> ClassInitImpl<DeviceClass> for T +/// where +/// T: DeviceImpl, +/// ``` +/// +/// The only case in which a manual implementation of the trait is needed +/// is for interfaces (note that there is no Rust example yet for using +/// interfaces). In this case, unlike the C case, the Rust class _has_ +/// to define its own class struct `FooClass` to go together with +/// `ClassInitImpl<FooClass>`. +pub trait ClassInitImpl<T> { + /// Initialize `klass` to point to the virtual method implementations + /// for `Self`. 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<unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void)>; - - /// 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), - >; + /// + /// The virtual method implementations usually come from another + /// trait, for example [`DeviceImpl`](crate::device_class::DeviceImpl) + /// when `T` is [`DeviceClass`](crate::bindings::DeviceClass). + /// + /// On entry, `klass`'s parent class is initialized, while the other fields + /// are all zero; it is therefore assumed that all fields in `T` can be + /// zeroed, otherwise it would not be possible to provide the class as a + /// `&mut T`. TODO: add a bound of [`Zeroable`](crate::zeroable::Zeroable) + /// to T; this is more easily done once Zeroable does not require a manual + /// implementation (Rust 1.75.0). + fn class_init(klass: &mut T); } #[macro_export] diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs index 03d03feee83..ee9ae7eeb74 100644 --- a/rust/qemu-api/src/device_class.rs +++ b/rust/qemu-api/src/device_class.rs @@ -2,10 +2,11 @@ // Author(s): Manos Pitsidianakis <manos.pitsidiana...@linaro.org> // SPDX-License-Identifier: GPL-2.0-or-later -use std::{ffi::CStr, os::raw::c_void}; +use std::ffi::CStr; use crate::{ - bindings::{self, DeviceClass, DeviceState, Error, ObjectClass, Property, VMStateDescription}, + bindings::{self, DeviceClass, DeviceState, Error, Property, VMStateDescription}, + definitions::ClassInitImpl, prelude::*, zeroable::Zeroable, }; @@ -45,7 +46,7 @@ fn vmsd() -> Option<&'static VMStateDescription> { /// # Safety /// /// This function is only called through the QOM machinery and -/// the `impl_device_class!` macro. +/// used by the `ClassInitImpl<DeviceClass>` trait. /// 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. @@ -66,43 +67,26 @@ fn vmsd() -> Option<&'static VMStateDescription> { T::RESET.unwrap()(unsafe { &mut *state }); } -/// # Safety -/// -/// We expect the FFI user of this function to pass a valid pointer that -/// can be downcasted to type `DeviceClass`, because `T` implements -/// `DeviceImpl`. -pub unsafe extern "C" fn rust_device_class_init<T: DeviceImpl>( - klass: *mut ObjectClass, - _: *mut c_void, -) { - let mut dc = ::core::ptr::NonNull::new(klass.cast::<DeviceClass>()).unwrap(); - unsafe { - let dc = dc.as_mut(); +impl<T> ClassInitImpl<DeviceClass> for T +where + T: DeviceImpl, +{ + fn class_init(dc: &mut DeviceClass) { if <T as DeviceImpl>::REALIZE.is_some() { dc.realize = Some(rust_realize_fn::<T>); } if <T as DeviceImpl>::RESET.is_some() { - bindings::device_class_set_legacy_reset(dc, Some(rust_reset_fn::<T>)); + unsafe { + bindings::device_class_set_legacy_reset(dc, Some(rust_reset_fn::<T>)); + } } if let Some(vmsd) = <T as DeviceImpl>::vmsd() { dc.vmsd = vmsd; } - bindings::device_class_set_props(dc, <T as DeviceImpl>::properties().as_ptr()); - } -} - -#[macro_export] -macro_rules! impl_device_class { - ($type:ty) => { - impl $crate::definitions::ClassInitImpl for $type { - const CLASS_INIT: Option< - unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut ::std::os::raw::c_void), - > = Some($crate::device_class::rust_device_class_init::<$type>); - const CLASS_BASE_INIT: Option< - unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut ::std::os::raw::c_void), - > = None; + unsafe { + bindings::device_class_set_props(dc, <T as DeviceImpl>::properties().as_ptr()); } - }; + } } #[macro_export] @@ -147,8 +131,8 @@ macro_rules! declare_properties { }; } -unsafe impl ObjectType for bindings::DeviceState { - type Class = bindings::DeviceClass; +unsafe impl ObjectType for DeviceState { + type Class = DeviceClass; const TYPE_NAME: &'static CStr = unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_DEVICE) }; } diff --git a/rust/qemu-api/src/sysbus.rs b/rust/qemu-api/src/sysbus.rs index 5ee068541cf..5d15b317405 100644 --- a/rust/qemu-api/src/sysbus.rs +++ b/rust/qemu-api/src/sysbus.rs @@ -6,7 +6,13 @@ pub use bindings::{SysBusDevice, SysBusDeviceClass}; -use crate::{bindings, cell::bql_locked, irq::InterruptSource, prelude::*}; +use crate::{ + bindings::{self, DeviceClass}, + cell::bql_locked, + definitions::ClassInitImpl, + irq::InterruptSource, + prelude::*, +}; unsafe impl ObjectType for SysBusDevice { type Class = SysBusDeviceClass; @@ -14,6 +20,16 @@ unsafe impl ObjectType for SysBusDevice { unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_SYS_BUS_DEVICE) }; } +// TODO: add SysBusDeviceImpl +impl<T> ClassInitImpl<SysBusDeviceClass> for T +where + T: ClassInitImpl<DeviceClass>, +{ + fn class_init(sdc: &mut SysBusDeviceClass) { + <T as ClassInitImpl<DeviceClass>>::class_init(&mut sdc.parent_class); + } +} + impl SysBusDevice { /// Return `self` cast to a mutable pointer, for use in calls to C code. const fn as_mut_ptr(&self) -> *mut SysBusDevice { diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs index 2a72b0f9276..4ce1bd19247 100644 --- a/rust/qemu-api/tests/tests.rs +++ b/rust/qemu-api/tests/tests.rs @@ -31,11 +31,6 @@ pub struct DummyState { pub migrate_clock: bool, } - #[repr(C)] - pub struct DummyClass { - pub _parent: DeviceClass, - } - declare_properties! { DUMMY_PROPERTIES, define_property!( @@ -48,7 +43,7 @@ pub struct DummyClass { } unsafe impl ObjectType for DummyState { - type Class = DummyClass; + type Class = <DeviceState as ObjectType>::Class; const TYPE_NAME: &'static CStr = c_str!("dummy"); } @@ -66,8 +61,6 @@ fn vmsd() -> Option<&'static VMStateDescription> { } } - impl_device_class!(DummyState); - unsafe { module_call_init(module_init_type::MODULE_INIT_QOM); object_unref(object_new(DummyState::TYPE_NAME.as_ptr()).cast()); -- 2.47.1