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

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1458 bytes --]

On Mon, Aug 03, 2026 at 11:26:29AM -0700, Connor Kite wrote:
> On Tue, Jul 28, 2026 at 10:59 AM Stefan Hajnoczi <stefanha@redhat.com> 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]