diff options
author | Paolo Bonzini <pbonzini@redhat.com> | 2025-03-04 20:48:05 +0100 |
---|---|---|
committer | Paolo Bonzini <pbonzini@redhat.com> | 2025-06-17 09:54:52 +0200 |
commit | 345bef46a1b6765185bfe1450cc147f5feb5d0e7 (patch) | |
tree | 856e788fc99d931afd1e256664139efc14445564 /rust/hw | |
parent | 8d394f6cf0b50a82758b651e81a18dac13e70e7d (diff) | |
download | qemu-345bef46a1b6765185bfe1450cc147f5feb5d0e7.zip qemu-345bef46a1b6765185bfe1450cc147f5feb5d0e7.tar.gz qemu-345bef46a1b6765185bfe1450cc147f5feb5d0e7.tar.bz2 |
rust: qom: change instance_init to take a ParentInit<>
This removes undefined behavior associated to writing to uninitialized
fields, and makes it possible to remove "unsafe" from the instance_init
implementation.
However, the init function itself is still unsafe, because it must promise
(as a sort as MaybeUninit::assume_init) that all fields have been
initialized.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Diffstat (limited to 'rust/hw')
-rw-r--r-- | rust/hw/char/pl011/src/device.rs | 34 | ||||
-rw-r--r-- | rust/hw/timer/hpet/src/device.rs | 16 |
2 files changed, 21 insertions, 29 deletions
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index be8387f..2d416cd 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 <manos.pitsidianakis@linaro.org> // 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<unsafe fn(&mut Self)> = Some(Self::init); + const INSTANCE_INIT: Option<unsafe fn(ParentInit<Self>)> = Some(Self::init); const INSTANCE_POST_INIT: Option<fn(&Self)> = Some(Self::post_init); const CLASS_INIT: fn(&mut Self::Class) = Self::Class::class_init::<Self>; } @@ -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<Self>) { static PL011_OPS: MemoryRegionOps<PL011State> = MemoryRegionOpsBuilder::<PL011State>::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 340ca1d..a281927 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<Self>) { static HPET_RAM_OPS: MemoryRegionOps<HPETState> = MemoryRegionOpsBuilder::<HPETState>::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::<MaybeUninit<Self>>()) }); + 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<unsafe fn(&mut Self)> = Some(Self::init); + const INSTANCE_INIT: Option<unsafe fn(ParentInit<Self>)> = Some(Self::init); const INSTANCE_POST_INIT: Option<fn(&Self)> = Some(Self::post_init); const CLASS_INIT: fn(&mut Self::Class) = Self::Class::class_init::<Self>; } |