Re: [PATCH 14/26] rust: qom: move bridge for TypeInfo functions out of pl011

[Zhao Liu]

> 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()...

> - 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).

...in realize(), also need to handle the "timers" property to enable
the timers that property requires.

> - 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

make sense, thanks!

> >
> > > 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!)."

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?

> 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.

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).

> 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.

Good idea, yes, will do.

Thanks,
Zhao