qemu-rust
[Top][All Lists]
Advanced

[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: Paolo Bonzini
Subject: Re: [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of pl011
Date: Wed, 11 Dec 2024 10:11:59 +0100



Il mer 11 dic 2024, 08:41 Zhao Liu <zhao1.liu@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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]