On Wed, Dec 11, 2024 at 5:38 PM Zhao Liu <zhao1....@intel.com> wrote:
>
> > Generally:
> >
> > - embedded objects will have to be initialized in instance_init unless they
> > are Options
>
> I see, at least for HPETTimer array, I need to prepare all of them in
> instance_init()...
>
> ...in realize(), also need to handle the "timers" property to enable
> the timers that property requires.
I see -- though, thinking more about it, since you have
fn init_timer(&mut self) {
let raw_ptr: *mut HPETState = self;
for i in 0..HPET_MAX_TIMERS {
let mut timer = self.get_timer(i).borrow_mut();
timer.init(i, raw_ptr).init_timer_with_state();
}
}
It seems to me that you can do everything in instance_init. Later on a
function like the above
will become something like
impl HPETTimer {
fn init_timer(hpet: NonNull<HPETState>, n: usize) -> impl PinInit<Self> {
pin_init!(&this in HPETTimer {
index: n,
qemu_timer <- Timer::init_ns(...),
state: hpet,
config: 0,
cmp: 0,
fsb: 0,
cmp64: 0,
period: 0,
wrap_flag: false,
last: 0,
}
}
}
But even now you can write something that takes a &mut self as the
first argument. It's undefined behavior but it's okay as long as we
have a path forward.
> > > > The way that this will become safe is to use the pinned_init crate from
> > > > Linux: instance_init returns the initialization as an "impl
> > > PinInit<Self>",
> > >
> > > Then do we need to place OBJECT in some suitable memory location (Box or
> > > something) for Rust implementation?
> > >
> >
> > Allocation is still done by the C code, so I am not sure I understand the
> > question. Rust code allocates QOM objects with object_new() so they are
> > malloc-ed.
>
> Sorry, I'm not familiar enough with this piece...I have the question
> because PinInit doc said "you will need a suitable memory location that
> can hold a T. This can be KBox<T>, Arc<T>, UniqueArc<T> or even the
> stack (see stack_pin_init!)."
Ah, I see. You can use __pinned_init directly on the memory that is
passed to rust_instance_init. See for example the implementation of
InPlaceWrite for Box
(https://docs.rs/pinned-init/latest/src/pinned_init/lib.rs.html#1307).
> I see that the core point is ensuring that structures cannot be moved.
> Given that object_new() on the C side ensures that the allocated object
> will not be moved, Rust side does not need to worry about pinning, correct?
Sort of... You still need to worry about it for two reasons:
1) if you have &mut Self you can move values out of the object using
e.g. mem::replace or mem::swap. Those would move the value in memory
and cause trouble (think of moving a QEMUTimer while it is pointed to
by the QEMUTimerList). This is solved by 1) using &Self all the time +
interior mutability 2) using pinned_init's "PinnedDrop" functionality,
because &Self can be used in QEMU-specific APIs but (obviously) not in
the built-in Drop trait.
2) right now marking something as pinned is an indirect way to tell
the compiler and miri that there are external references to it. For a
longer discussion you can read
https://crates.io/crates/pinned-aliasable or
https://gist.github.com/Darksonn/1567538f56af1a8038ecc3c664a42462.
Linux does this with a wrapper type similar to the one in pinned-aliasable:
/// Stores an opaque value.
///
/// This is meant to be used with FFI objects that are never
interpreted by Rust code.
#[repr(transparent)]
pub struct Opaque<T> {
value: UnsafeCell<MaybeUninit<T>>,
_pin: PhantomPinned,
}
It's on my todo list to introduce it in qemu_api::cell and (for
example) change qom::Object from
pub use bindings::Object
to
pub type Object = Opaque<bindings::Object>;
Or something like that.
> Vec seems to lack proper vmstate support. I understand that we need to
> modify VMSTATE_STRUCT_VARRAY_POINTER_* to introduce a variant for Vec.
>
> Since the array support is already available, I chose to use an array
> instead (although vmstate is disabled for now).
Yes, you're right.
Palo
