On Thu, Aug 07, 2025 at 03:57:17PM +0200, Paolo Bonzini wrote:
> Date: Thu, 7 Aug 2025 15:57:17 +0200
> From: Paolo Bonzini <pbonz...@redhat.com>
> Subject: Re: [RFC 16/26] memory: Make flatview_do_translate() return a
> pointer to MemoryRegionSection
>
> On 8/7/25 14:30, Zhao Liu wrote:
> > Rust side will use cell::Opaque<> to hide details of C structure, and
> > this could help avoid the direct operation on C memory from Rust side.
> >
> > Therefore, it's necessary to wrap a translation binding and make it only
> > return the pointer to MemoryRegionSection, instead of the copy.
> >
> > As the first step, make flatview_do_translate return a pointer to
> > MemoryRegionSection, so that we can build a wrapper based on it.
>
> Independent of Rust, doing the copy as late as possible is good, but make it
> return a "const MemoryRegionSection*" so that there's no risk of overwriting
> data.
Yes, const MemoryRegionSection* is helpful...
> Hopefully this does not show a bigger problem!
...then we will get `*const bindings::MemoryRegionSection` from
flatview_translate_section().
This is mainly about how to construct Opaque<T> from `*cont T`:
impl FlatView {
fn translate(
&self,
addr: GuestAddress,
len: GuestUsize,
is_write: bool,
) -> Option<(&MemoryRegionSection, MemoryRegionAddress, GuestUsize)> {
...
let ptr = unsafe {
flatview_translate_section(
self.as_mut_ptr(),
addr.raw_value(),
&mut raw_addr,
&mut remain,
is_write,
MEMTXATTRS_UNSPECIFIED,
)
};
...
------> // Note here, Opaque<>::from_raw() requires *mut T.
// And we can definitely convert *cont T to *mut T!
let s = unsafe { <FlatView as GuestMemory>::R::from_raw(ptr as *mut _)
};
...
}
But look closer to Opaque<>, it has 2 safe methods: as_mut_ptr() &
raw_get().
These 2 methods indicate that the T pointed by Qpaque<T> is mutable,
which has the conflict with the original `*const bindings::MemoryRegionSection`.
So from this point, it seems unsafe to use Qpaque<> on this case.
To address this, I think we need:
- rich comments about this MemoryRegionSection is actually immuatble.
- modify other C functions to accept `const *MemoryRegionSection` as
argument.
What do you think?
Thanks,
Zhao
