Il mer 11 dic 2024, 08:41 Zhao Liu <zhao1....@intel.com> ha scritto: > 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. >
Yes that's pretty much it. (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(). > Generally: - embedded objects will have to be initialized in instance_init unless they are Options - sysbus_init_irq and sysbus_init_mmio can be called in both instance_init and instance_post_init for now, but they will have to be in post_init once the signature of init changes to return impl PinInit - if you don't need properties you can choose between post_init and realize, if you need properties you need to initialize in realize (and then, unlike C, you might need to explicitly allow the pre-realize state; for example using Option<&...> instead of just a reference; or Vec<> instead of an array). > > > 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. I discussed it with Manos some time ago and in principle you could use a Box (QOM supports custom freeing functions) but it's a bit complex because the freeing function would have to free the memory without dropping the contents of the Box (the drop is done by QOM via instance_finalize). If you want to allocate the HPETTimers at realize time, I think you can place them in a Vec. I think you won't need NonNull for this, but I am not 100% sure. Alternatively if you want to always prepare all MAX_TIMERS of them and then only use a subset, you can use an array. Either way, probably it makes sense for you to have an "fn timers_iter(&self) -> impl Iter<Item = &BqlRefCell<HPETTimer>>" in HPETState, or something like that. Then you can easily use for loops to walk all timers between 0 and self.num_timers-1. Paolo > 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 > >
