rust: hpet: fully initialize object during instance_init

The array of BqlRefCell<HPETTimer> is not initialized yet at the
end of instance_init.  In particular, the "state" field is NonNull
and therefore it is invalid to have it as zero bytes.

Note that MaybeUninit is necessary because assigning to self.timers[index]
would trigger Drop of the old value.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Paolo Bonzini 2025-04-15 13:13:19 +02:00
parent abf1832424
commit eb64a0c6ae

View file

@ -4,6 +4,7 @@
use std::{ use std::{
ffi::{c_int, c_void, CStr}, ffi::{c_int, c_void, CStr},
mem::MaybeUninit,
pin::Pin, pin::Pin,
ptr::{addr_of_mut, null_mut, NonNull}, ptr::{addr_of_mut, null_mut, NonNull},
slice::from_ref, slice::from_ref,
@ -25,6 +26,7 @@ use qemu_api::{
qom_isa, qom_isa,
sysbus::{SysBusDevice, SysBusDeviceImpl}, sysbus::{SysBusDevice, SysBusDeviceImpl},
timer::{Timer, CLOCK_VIRTUAL, NANOSECONDS_PER_SECOND}, timer::{Timer, CLOCK_VIRTUAL, NANOSECONDS_PER_SECOND},
uninit_field_mut,
vmstate::VMStateDescription, vmstate::VMStateDescription,
vmstate_fields, vmstate_of, vmstate_struct, vmstate_subsections, vmstate_validate, vmstate_fields, vmstate_of, vmstate_struct, vmstate_subsections, vmstate_validate,
zeroable::Zeroable, zeroable::Zeroable,
@ -212,13 +214,13 @@ pub struct HPETTimer {
} }
impl HPETTimer { impl HPETTimer {
fn init(&mut self, index: u8, state: &HPETState) { fn new(index: u8, state: *const HPETState) -> HPETTimer {
*self = HPETTimer { HPETTimer {
index, index,
// SAFETY: the HPETTimer will only be used after the timer // SAFETY: the HPETTimer will only be used after the timer
// is initialized below. // is initialized below.
qemu_timer: unsafe { Timer::new() }, qemu_timer: unsafe { Timer::new() },
state: NonNull::new((state as *const HPETState).cast_mut()).unwrap(), state: NonNull::new(state.cast_mut()).unwrap(),
config: 0, config: 0,
cmp: 0, cmp: 0,
fsb: 0, fsb: 0,
@ -226,19 +228,15 @@ impl HPETTimer {
period: 0, period: 0,
wrap_flag: 0, wrap_flag: 0,
last: 0, last: 0,
}; }
}
fn init_timer_with_cell(cell: &BqlRefCell<Self>) {
let mut timer = cell.borrow_mut();
// SAFETY: HPETTimer is only used as part of HPETState, which is // SAFETY: HPETTimer is only used as part of HPETState, which is
// always pinned. // always pinned.
let qemu_timer = unsafe { Pin::new_unchecked(&mut self.qemu_timer) }; let qemu_timer = unsafe { Pin::new_unchecked(&mut timer.qemu_timer) };
qemu_timer.init_full( qemu_timer.init_full(None, CLOCK_VIRTUAL, Timer::NS, 0, timer_handler, cell);
None,
CLOCK_VIRTUAL,
Timer::NS,
0,
timer_handler,
&state.timers[self.index as usize],
)
} }
fn get_state(&self) -> &HPETState { fn get_state(&self) -> &HPETState {
@ -607,9 +605,18 @@ impl HPETState {
} }
} }
fn init_timer(&self) { fn init_timers(this: &mut MaybeUninit<Self>) {
for (index, timer) in self.timers.iter().enumerate() { let state = this.as_ptr();
timer.borrow_mut().init(index.try_into().unwrap(), self); for index in 0..HPET_MAX_TIMERS {
let mut timer = uninit_field_mut!(*this, timers[index]);
// Initialize in two steps, to avoid calling Timer::init_full on a
// temporary that can be moved.
let timer = timer.write(BqlRefCell::new(HPETTimer::new(
index.try_into().unwrap(),
state,
)));
HPETTimer::init_timer_with_cell(timer);
} }
} }
@ -710,6 +717,8 @@ impl HPETState {
"hpet", "hpet",
HPET_REG_SPACE_LEN, HPET_REG_SPACE_LEN,
); );
Self::init_timers(unsafe { &mut *((self as *mut Self).cast::<MaybeUninit<Self>>()) });
} }
fn post_init(&self) { fn post_init(&self) {
@ -731,7 +740,6 @@ impl HPETState {
self.hpet_id.set(HPETFwConfig::assign_hpet_id()?); self.hpet_id.set(HPETFwConfig::assign_hpet_id()?);
self.init_timer();
// 64-bit General Capabilities and ID Register; LegacyReplacementRoute. // 64-bit General Capabilities and ID Register; LegacyReplacementRoute.
self.capability.set( self.capability.set(
HPET_CAP_REV_ID_VALUE << HPET_CAP_REV_ID_SHIFT | HPET_CAP_REV_ID_VALUE << HPET_CAP_REV_ID_SHIFT |