> 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> {
Thank you! I should pass NonNull type other than `*mut HPETState` for now :-)
> 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,
> }
> }
> }
That's the right and ideal way, and I like it.
> 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.
Yes, I agree. In the next version, I can follow your suggestion and put
these embedded items into instance_init(), to be better prepared for the
next step.
> > > > > 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).
Thank you! I understand your intention. A similar implementation would
also be quite natural in rust_instance_init.
> > 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
With your help and through our discussions, I have gained a clearer
understanding of this intention.
> 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.
Thanks for sharing!
> 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.
Yes, I agree with this idea. It's what we need.
Regards,
Zhao
