diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index be8387f6f2..2d416cd9a3 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -2,7 +2,7 @@ // Author(s): Manos Pitsidianakis // SPDX-License-Identifier: GPL-2.0-or-later -use std::{ffi::CStr, mem::size_of, ptr::addr_of_mut}; +use std::{ffi::CStr, mem::size_of}; use qemu_api::{ chardev::{CharBackend, Chardev, Event}, @@ -11,9 +11,10 @@ use qemu_api::{ memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder}, prelude::*, qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType, ResettablePhasesImpl}, - qom::{ObjectImpl, Owned, ParentField}, + qom::{ObjectImpl, Owned, ParentField, ParentInit}, static_assert, sysbus::{SysBusDevice, SysBusDeviceImpl}, + uninit_field_mut, vmstate::VMStateDescription, }; @@ -163,7 +164,7 @@ impl PL011Impl for PL011State { impl ObjectImpl for PL011State { type ParentType = SysBusDevice; - const INSTANCE_INIT: Option = Some(Self::init); + const INSTANCE_INIT: Option)> = Some(Self::init); const INSTANCE_POST_INIT: Option = Some(Self::post_init); const CLASS_INIT: fn(&mut Self::Class) = Self::Class::class_init::; } @@ -488,7 +489,7 @@ impl PL011State { /// `PL011State` type. It must not be called more than once on the same /// location/instance. All its fields are expected to hold uninitialized /// values with the sole exception of `parent_obj`. - unsafe fn init(&mut self) { + unsafe fn init(mut this: ParentInit) { static PL011_OPS: MemoryRegionOps = MemoryRegionOpsBuilder::::new() .read(&PL011State::read) .write(&PL011State::write) @@ -496,28 +497,23 @@ impl PL011State { .impl_sizes(4, 4) .build(); - // SAFETY: - // - // self and self.iomem are guaranteed to be valid at this point since callers - // must make sure the `self` reference is valid. + // SAFETY: this and this.iomem are guaranteed to be valid at this point MemoryRegion::init_io( - unsafe { &mut *addr_of_mut!(self.iomem) }, - addr_of_mut!(*self), + &mut uninit_field_mut!(*this, iomem), &PL011_OPS, "pl011", 0x1000, ); - self.regs = Default::default(); + uninit_field_mut!(*this, regs).write(Default::default()); - // SAFETY: - // - // self.clock is not initialized at this point; but since `Owned<_>` is - // not Drop, we can overwrite the undefined value without side effects; - // it's not sound but, because for all PL011State instances are created - // by QOM code which calls this function to initialize the fields, at - // leastno code is able to access an invalid self.clock value. - self.clock = self.init_clock_in("clk", &Self::clock_update, ClockEvent::ClockUpdate); + let clock = DeviceState::init_clock_in( + &mut this, + "clk", + &Self::clock_update, + ClockEvent::ClockUpdate, + ); + uninit_field_mut!(*this, clock).write(clock); } const fn clock_update(&self, _event: ClockEvent) { diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs index 340ca1d355..a281927781 100644 --- a/rust/hw/timer/hpet/src/device.rs +++ b/rust/hw/timer/hpet/src/device.rs @@ -21,8 +21,8 @@ use qemu_api::{ hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder, MEMTXATTRS_UNSPECIFIED, }, prelude::*, - qdev::{DeviceImpl, DeviceMethods, DeviceState, Property, ResetType, ResettablePhasesImpl}, - qom::{ObjectImpl, ObjectType, ParentField}, + qdev::{DeviceImpl, DeviceState, Property, ResetType, ResettablePhasesImpl}, + qom::{ObjectImpl, ObjectType, ParentField, ParentInit}, qom_isa, sysbus::{SysBusDevice, SysBusDeviceImpl}, timer::{Timer, CLOCK_VIRTUAL, NANOSECONDS_PER_SECOND}, @@ -697,7 +697,7 @@ impl HPETState { .set(self.counter.get().deposit(shift, len, val)); } - unsafe fn init(&mut self) { + unsafe fn init(mut this: ParentInit) { static HPET_RAM_OPS: MemoryRegionOps = MemoryRegionOpsBuilder::::new() .read(&HPETState::read) @@ -707,18 +707,14 @@ impl HPETState { .impl_sizes(4, 8) .build(); - // SAFETY: - // self and self.iomem are guaranteed to be valid at this point since callers - // must make sure the `self` reference is valid. MemoryRegion::init_io( - unsafe { &mut *addr_of_mut!(self.iomem) }, - addr_of_mut!(*self), + &mut uninit_field_mut!(*this, iomem), &HPET_RAM_OPS, "hpet", HPET_REG_SPACE_LEN, ); - Self::init_timers(unsafe { &mut *((self as *mut Self).cast::>()) }); + Self::init_timers(&mut this); } fn post_init(&self) { @@ -900,7 +896,7 @@ unsafe impl ObjectType for HPETState { impl ObjectImpl for HPETState { type ParentType = SysBusDevice; - const INSTANCE_INIT: Option = Some(Self::init); + const INSTANCE_INIT: Option)> = Some(Self::init); const INSTANCE_POST_INIT: Option = Some(Self::post_init); const CLASS_INIT: fn(&mut Self::Class) = Self::Class::class_init::; } diff --git a/rust/qemu-api/src/memory.rs b/rust/qemu-api/src/memory.rs index 9ef2694bd6..e40fad6cf1 100644 --- a/rust/qemu-api/src/memory.rs +++ b/rust/qemu-api/src/memory.rs @@ -16,6 +16,7 @@ use crate::{ callbacks::FnCall, cell::Opaque, prelude::*, + uninit::MaybeUninitField, zeroable::Zeroable, }; @@ -147,7 +148,7 @@ impl MemoryRegion { #[inline(always)] unsafe fn do_init_io( slot: *mut bindings::MemoryRegion, - owner: *mut Object, + owner: *mut bindings::Object, ops: &'static bindings::MemoryRegionOps, name: &'static str, size: u64, @@ -156,7 +157,7 @@ impl MemoryRegion { let cstr = CString::new(name).unwrap(); memory_region_init_io( slot, - owner.cast::(), + owner, ops, owner.cast::(), cstr.as_ptr(), @@ -166,16 +167,15 @@ impl MemoryRegion { } pub fn init_io>( - &mut self, - owner: *mut T, + this: &mut MaybeUninitField<'_, T, Self>, ops: &'static MemoryRegionOps, name: &'static str, size: u64, ) { unsafe { Self::do_init_io( - self.0.as_mut_ptr(), - owner.cast::(), + this.as_mut_ptr().cast(), + MaybeUninitField::parent_mut(this).cast(), &ops.0, name, size, diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs index 0610959f46..36f02fb57d 100644 --- a/rust/qemu-api/src/qdev.rs +++ b/rust/qemu-api/src/qdev.rs @@ -19,7 +19,7 @@ use crate::{ error::{Error, Result}, irq::InterruptSource, prelude::*, - qom::{ObjectClass, ObjectImpl, Owned}, + qom::{ObjectClass, ObjectImpl, Owned, ParentInit}, vmstate::VMStateDescription, }; @@ -247,15 +247,9 @@ unsafe impl ObjectType for DeviceState { } qom_isa!(DeviceState: Object); -/// Trait for methods exposed by the [`DeviceState`] class. The methods can be -/// called on all objects that have the trait `IsA`. -/// -/// The trait should only be used through the blanket implementation, -/// which guarantees safety via `IsA`. -pub trait DeviceMethods: ObjectDeref -where - Self::Target: IsA, -{ +/// Initialization methods take a [`ParentInit`] and can be called as +/// associated functions. +impl DeviceState { /// Add an input clock named `name`. Invoke the callback with /// `self` as the first parameter for the events that are requested. /// @@ -266,12 +260,15 @@ where /// which Rust code has a reference to a child object) it would be /// possible for this function to return a `&Clock` too. #[inline] - fn init_clock_in FnCall<(&'a Self::Target, ClockEvent)>>( - &self, + pub fn init_clock_in FnCall<(&'a T, ClockEvent)>>( + this: &mut ParentInit, name: &str, _cb: &F, events: ClockEvent, - ) -> Owned { + ) -> Owned + where + T::ParentType: IsA, + { fn do_init_clock_in( dev: &DeviceState, name: &str, @@ -287,10 +284,10 @@ where unsafe { let cstr = CString::new(name).unwrap(); let clk = bindings::qdev_init_clock_in( - dev.as_mut_ptr(), + dev.0.as_mut_ptr(), cstr.as_ptr(), cb, - dev.as_void_ptr(), + dev.0.as_void_ptr(), events.0, ); @@ -307,12 +304,12 @@ where // SAFETY: the opaque is "this", which is indeed a pointer to T F::call((unsafe { &*(opaque.cast::()) }, event)) } - Some(rust_clock_cb::) + Some(rust_clock_cb::) } else { None }; - do_init_clock_in(self.upcast(), name, cb, events) + do_init_clock_in(unsafe { this.upcast_mut() }, name, cb, events) } /// Add an output clock named `name`. @@ -324,16 +321,30 @@ where /// which Rust code has a reference to a child object) it would be /// possible for this function to return a `&Clock` too. #[inline] - fn init_clock_out(&self, name: &str) -> Owned { + pub fn init_clock_out(this: &mut ParentInit, name: &str) -> Owned + where + T::ParentType: IsA, + { unsafe { let cstr = CString::new(name).unwrap(); - let clk = bindings::qdev_init_clock_out(self.upcast().as_mut_ptr(), cstr.as_ptr()); + let dev: &mut DeviceState = this.upcast_mut(); + let clk = bindings::qdev_init_clock_out(dev.0.as_mut_ptr(), cstr.as_ptr()); let clk: &Clock = Clock::from_raw(clk); Owned::from(clk) } } +} +/// Trait for methods exposed by the [`DeviceState`] class. The methods can be +/// called on all objects that have the trait `IsA`. +/// +/// The trait should only be used through the blanket implementation, +/// which guarantees safety via `IsA`. +pub trait DeviceMethods: ObjectDeref +where + Self::Target: IsA, +{ fn prop_set_chr(&self, propname: &str, chr: &Owned) { assert!(bql_locked()); let c_propname = CString::new(propname).unwrap(); diff --git a/rust/qemu-api/src/qom.rs b/rust/qemu-api/src/qom.rs index 04d102591d..e20ee014cb 100644 --- a/rust/qemu-api/src/qom.rs +++ b/rust/qemu-api/src/qom.rs @@ -382,12 +382,15 @@ impl DerefMut for ParentInit<'_, T> { } unsafe extern "C" fn rust_instance_init(obj: *mut bindings::Object) { - let mut state = NonNull::new(obj).unwrap().cast::(); + let mut state = NonNull::new(obj).unwrap().cast::>(); + // 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()(state.as_mut()); + ParentInit::with(state.as_mut(), |parent_init| { + T::INSTANCE_INIT.unwrap()(parent_init); + }); } } @@ -654,7 +657,7 @@ pub trait ObjectImpl: ObjectType + IsA { /// /// FIXME: The argument is not really a valid reference. `&mut /// MaybeUninit` would be a better description. - const INSTANCE_INIT: Option = None; + const INSTANCE_INIT: Option)> = None; /// Function that is called to finish initialization of an object, once /// `INSTANCE_INIT` functions have been called.