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

Stefan Hajnoczi <[email protected]> Tue, 4 Aug 2026 14:16:59 -0400

Newsgroups	gmane.comp.emulators.qemu.block,gmane.comp.emulators.qemu
Message-ID	<20260804181659.GB488200@fedora>

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.

signature.asc (application/pgp-signature, 488 B)