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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 29/01/2021 14:59, Roger Pau Monné wrote:
> 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.

mmap() of 0 is an unconditional failure.  So sayeth POSIX.

For the size request, we don't mmap(), and a pointer of 0 is the signal
to Xen.

For the regular mapping, we support both NULL (let the kernel choose),
and non-NULL (I want my mapping here please), just like regular mmap().

~Andrew