Re: [PATCH v4] tools/foreignmem: Support querying the size of a resource

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Jan 29, 2021 at 11:38:43AM +0100, Manuel Bouyer wrote:
> On Thu, Jan 28, 2021 at 12:01:52PM +0000, Andrew Cooper wrote:
> > With the Xen side of this interface (soon to be) fixed to return real sizes,
> > userspace needs to be able to make the query.
> > 
> > Introduce xenforeignmemory_resource_size() for the purpose, bumping the
> > library minor version.
> > 
> > Update both all osdep_xenforeignmemory_map_resource() implementations to
> > understand size requests, skip the mmap() operation, and copy back the
> > nr_frames field.
> > 
> > For NetBSD, also fix up the ioctl() error path to issue an unmap(), which 
> > was
> > overlooked by c/s 4a64e2bb39 "libs/foreignmemory: Implement on NetBSD".
> > [....]
> > diff --git a/tools/libs/foreignmemory/netbsd.c 
> > b/tools/libs/foreignmemory/netbsd.c
> > index d26566f601..4ae60aafdd 100644
> > --- a/tools/libs/foreignmemory/netbsd.c
> > +++ b/tools/libs/foreignmemory/netbsd.c
> > @@ -132,6 +132,10 @@ int osdep_xenforeignmemory_map_resource(
> >      };
> >      int rc;
> >  
> > +    if ( !fres->addr && !fres->nr_frames )
> > +        /* Request for resource size.  Skip mmap(). */
> > +        goto skip_mmap;
> > +
> >      fres->addr = mmap(fres->addr, fres->nr_frames << PAGE_SHIFT,
> >                        fres->prot, fres->flags | MAP_ANON | MAP_SHARED, -1, 
> > 0);
> 
> What happens if fres->addr is not NULL and nr_frames is 0 ?

mmap would return MAP_FAILED and errno == EINVAL in that case AFAICT
on Linux and FreeBSD. NetBSD mmap man page doesn't seem to mention
what happens in that case, so the comments below apply to Linux and
FreeBSD. Maybe we need to handle this differently for NetBSD?

> Is it supposed to happen ?

I think that's fine. Calling osdep_xenforeignmemory_map_resource with
nr_frames == 0 is pointing to a bug in the caller, so returning error
should be fine.

> Should we assert that fres->addr is NULL when
> nr_frames is 0 ? Or force fres->addr to NULL when nr_frames is 0 ?

Doesn't really matter, mmap will return EINVAL if nr_frames == 0
regardless of the value of addr.

Roger.