Re: [PATCH XEN v5 07/23] tools: Refactor /dev/xen/gnt{dev, shr} wrappers into libxengnttab.

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 13/11/15 10:02, Ian Campbell wrote:
> On Wed, 2015-11-11 at 15:03 +0000, Ian Jackson wrote:
>> Ian Campbell writes ("[PATCH XEN v5 07/23] tools: Refactor
>> /dev/xen/gnt{dev,shr} wrappers into libxengnttab."):
>>> libxengnttab will provide a stable API and ABI for accessing the
>>> grant table devices.
>>>
>>> The functions are moved into the xengnt{tab,shr} namespace to make a
>>> clean break from libxc and avoid ambiguity regarding which interfaces
>>> are stable.
>>
>> I have reviewed the API.  Mostly this produced questions...
>
> Yes, thanks this is the sort of feedback I was looking for before we call
> this a stable interface.
>
> Some of my answers are of the "yes, we should decide that" variety.
>
> Daniel, I've added you because some of the questions below relate to the
> notification mechanism and to gntshr which I don't really understand (not
> that I really understand gnttab either).

Responses below.  I also agree that unifying the interfaces would be
better.

[...]
>>> +/**
>>> + * Memory maps a grant reference from one domain to a local address
>>> range.
>>> + * Mappings should be unmapped with xengnttab_munmap. If notify_offset
>>> or
>>> + * notify_port are not -1, this version will attempt to set up an
>>> unmap
>>> + * notification at the given offset and event channel. When the page
>>> is
>>> + * unmapped, the byte at the given offset will be zeroed and a wakeup
>>> will be
>>> + * sent to the given event channel.  Logs errors.
>>
>> What happens if the unmap notification cannot be set up ?
>>
>> Also "when the page is unmapped" makes it sound like you mean
>> xengnttab_munmap but actually I think this is when the grant is
>> withdrawn by the grantor ?
>>
>> If the grant is withdrawn by the grantor, does the page become
>> unuseable immediately ?  If so, how can anyone ever use this safely ?
>
> Daniel, could you answer these ones please.

This is intended to allow the kernel to send a close-request notification
when the application that allocated the grant page exits without calling
a proper shutdown (i.e. it crashes, calls _exit, calls execve, etc).

Without this signal, the kernel has no way to request that the mapper of
the page release it, and since Xen has no grant revocation mechanism, the
page will likely be tied up until the process on the other side is told to
release the page through some other method.

>>> +/*
>>> + * Creates and shares pages with another domain.
>>> + *
>> ...
>>> +void *xengntshr_share_pages(xengntshr_handle *xgs, uint32_t domid,
>>> +                            int count, uint32_t *refs, int writable);
>>
>> Can this fail ?  Can it partially succeed ?
>
> Daniel?

It can fail if you are out of pages to grant (there is a module parameter
that can be adjusted via sysfs for the maximum), or in the unlikely case
that the kernel itself is out of room in its grant mapping table (or if
the syscall itself encounters -ENOMEM).

>>> +/*
>>> + * Creates and shares a page with another domain, with unmap
>>> notification.
>>> + *
>>> + * @parm xgs a handle to an open grant sharing instance
>>> + * @parm domid the domain to share memory with
>>> + * @parm refs the grant reference of the pages (output)
>>> + * @parm writable true if the other domain can write to the page
>>> + * @parm notify_offset The byte offset in the page to use for unmap
>>> + *                     notification; -1 for none.
>>> + * @parm notify_port The event channel port to use for unmap notify,
>>> or -1
>>> + * @return local mapping of the page
>>> + */
>>> +void *xengntshr_share_page_notify(xengntshr_handle *xgs, uint32_t
>>> domid,
>>
>> What is this `unmap notification' ?
>
> Daniel?

As mentioned above, it is a way for the kernel to request for the other
side to unmap a page.  It is probably easiest to understand if looking
at the libvchan driver: one byte of the page is a "status" byte, and
when the side using xengntshr exits or dies, this byte is set to zero
and the event channel is notified.  When the peer is woken by the notify
and sees the state byte set to zero, it removes its own mappings so that
the shared pages can be freed.

>>
>>> +/*
>>> + * Unmaps the @count pages starting at @start_address, which were
>>> mapped by a
>>> + * call to xengntshr_share_*. Never logs.
>>
>> Linewrap in the comment.
>>
>>> + */
>>> +int xengntshr_munmap(xengntshr_handle *xgs, void *start_address,
>>> uint32_t count);
>>
>> What effect does this have on the peer ?
>
> Daniel?

If this removes the (final copy of the) mapping and a notify offset/port
is set, that processing happens.  Otherwise, the peer cannot tell when
this is called.

-- 
Daniel De Graaf
National Security Agency