Re: [PATCH RFC 11/15] hw/virtio/vhost-user: create isolation region

On Tue, Aug 4, 2026 at 11:17 AM Stefan Hajnoczi <[email protected]> wrote:
>
> On Mon, Aug 03, 2026 at 11:26:29AM -0700, Connor Kite wrote:
> > On Tue, Jul 28, 2026 at 10:59 AM Stefan Hajnoczi <[email protected]> wrote:
> > > >
> > > > +typedef struct IsolationRegion {
> > > > +    uint64_t base_addr;
> > > > +    uint64_t vring_base_addr;
> > > > +    uint64_t size;
> > > > +    int iso_fd;
> > > > +} IsolationRegion;
> > >
> > > The purpose of the base_addr and vring_base_addr fields is not obvious.
> > > I suggest adjusting the types, names, and adding comments to make the
> > > purpose clearer:
> > >
> > >   typedef struct {
> > >       void *mem;           /* mapped shared memory */
> > >       size_t size;
> > >       uint64_t vring_iova;
> > >       int fd;              /* shared memory fd */
> > >   } IsolationRegion;
> > >
> >
> > Agreed on adding clarification.  Is there a reason to change from uint64_t
> > to void * for referencing the shared memory region address?  Most uses of
> > that variable expect uint64_t currently.  Here's what I am currently thinking:
>
> The shared memory region address is a C pointer where the shared memory
> is mmapped and the type for an arbitrary C pointer is void *. The casts
> needed around qemu_memfd_alloc() and qemu_memfd_free() suggest that
> uint64_t is not the natural type for this.
>
> Keeping this as a C pointer helps differentiate the address from an
> IOVA, making the code clearer.
>
> Also, with the comments about base_addr below addressed, the
> hwaddr/uint64_t uses will be reduced.

Got it!  I will change here and plan to default to void * when representing host
addresses going forward.