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
Description: PGP signature
