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.
