Sorry, when I revisit this patch, I have more thoughts..

> -/// A safe wrapper around [`bindings::MemoryRegion`].  Compared to the
> -/// underlying C struct it is marked as pinned because the QOM tree
> -/// contains a pointer to it.
> -pub struct MemoryRegion {
> -    inner: bindings::MemoryRegion,
> -    _pin: PhantomPinned,
> -}
> +/// A safe wrapper around [`bindings::MemoryRegion`].
> +#[repr(transparent)]
> +#[derive(qemu_api_macros::Wrapper)]
> +pub struct MemoryRegion(Opaque<bindings::MemoryRegion>);

Would MemoryRegionOps also need to be wrapped into Opaque<>?

Currently I understand it's not necessary, because init_io() requires
MemoryRegionOps reference to be static.

    pub fn init_io<T: IsA<Object>>(
        &mut self,
        owner: *mut T,
        ops: &'static MemoryRegionOps<T>,
        name: &'static str,
        size: u64,
    )

But I wonder whether it is the most ideal implementation... or to remove
the static limitation of MemoryRegionOps and consider pin+Opaque instead?

Another thought is for init_io, it's also necessary to ensure MemoryRegion
is pinned, because callbacks also need to access the pointer of MemoryRegion,
just like you did for timer, e.g.,

     pub fn init_io<T: IsA<Object>>(
-        &mut self,
+        self: Pin<&mut Self>,
         owner: *mut T,
         ops: &'static MemoryRegionOps<T>,
         name: &'static str,
         size: u64,
     ) {

(Just disscussion without clarifying the difficulty), if you agree with
this, then I think this would be a main pattern for callback, i.e.,
accepting a pin parameter. Perhaps an ideal option would be to be able
to add the pin limit to the FnCall.

Thanks,
Zhao


Reply via email to