[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of p
From: |
Zhao Liu |
Subject: |
Re: [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of pl011 |
Date: |
Wed, 11 Dec 2024 15:59:33 +0800 |
> > > - const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)>
> > > = Some(pl011_init);
> > > + const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
> >
> > No need to keep `unsafe` here?
>
> Right now instance_init is called with only the parent initialized, and the
> remaining memory zeroed; its purpose is to prepare things for
> instance_post_init which can then be safe (it's also kind of wrong for
> instance_post_init to receive a &mut Self, because instance_init will create
> other pointers to the object, for example in a MemoryRegion's "parent"
> field).
Thank you for explanation. It makes a lot of sense.
> The right thing to do would be to have an argument of type &mut
> MaybeUninit<Self>. Then the caller would do something like
>
> let maybe_uninit = obj as *mut MaybeUninit<Self>;
> unsafe {
> Self::INSTANCE_INIT(&mut *maybe_uninit);
> maybe_uninit.assume_init_mut();
> }
>
> Note however that INSTANCE_INIT would still be unsafe, because its safety
> promise is that it prepares things for the caller's assume_init_mut().
Yes, I feel that this approach more clearly explains the purpose of QOM
init.
And since we are talking about the safety of INSTANCE_INIT, I think we
should add some safety guidelines here, like:
* Proper Initialization of pointers and references
* Explicit initialization of Non-Zero Fields
* In placed memory region is correctly initialized.
(Or do you have any additional or clearer guidelines?)
This could be the reference when adding SAFETY comment for the device's
own `unsafe` init(). :-)
And this is also a good explanation to distinguish between initialization
in init() and realize(). For example, HPET attempts to initialize the
timer array in realize().
> 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?
> and then instance_post_init can run with a &self. Until then, however,
> instance_init has to remain unsafe.
Thanks for sharing! It's a very reasonable direction.
Regards,
Zhao
- Re: [PATCH 09/26] rust: qom: convert type_info! macro to an associated const, (continued)
- [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of pl011, Paolo Bonzini, 2024/12/09
- Re: [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of pl011, Zhao Liu, 2024/12/10
- Re: [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of pl011, Paolo Bonzini, 2024/12/10
- Re: [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of pl011,
Zhao Liu <=
- Re: [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of pl011, Paolo Bonzini, 2024/12/11
- Re: [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of pl011, Zhao Liu, 2024/12/11
- Re: [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of pl011, Paolo Bonzini, 2024/12/12
- Re: [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of pl011, Zhao Liu, 2024/12/13
Re: [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of pl011, Zhao Liu, 2024/12/10
[PATCH 15/26] rust: qom: split ObjectType from ObjectImpl trait, Paolo Bonzini, 2024/12/09
[PATCH 11/26] rust: qdev: move device_class_init! body to generic function, ClassInitImpl implementation to macro, Paolo Bonzini, 2024/12/09
[PATCH 16/26] rust: qom: change the parent type to an associated type, Paolo Bonzini, 2024/12/09