Re: [Xen-devel] [PATCH v4 2/7] xen: xsm: flask: introduce XENMAPSPACE_gmfn_share for memory sharing

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.02.18 at 08:15, <blackskygg@xxxxxxxxx> wrote:
> Hi Jan,
> 
> 2018-02-13 23:26 GMT+08:00 Jan Beulich <JBeulich@xxxxxxxx>:
>>>>> On 13.02.18 at 16:15, <blackskygg@xxxxxxxxx> wrote:
>>> I've updated the comments according to your previous suggestions,
>>> do they look good to you?
>>
>> The one in the public header is way too verbose. I specifically don't
>> see why you would need to spell out XSM privilege requirements
>> there. Please make new comments match existing ones in style and
>> verbosity if at all possible, while still conveying all necessary /
>> relevant information.
>>
> 
> I shortened it a little bit, and now it looks like:
> 
> #define XENMAPSPACE_gmfn_share   6 /* GMFN from another dom. Unlike
> gmfn_foreign,
>                                       if (c) tries to map pages from (t) into
>                                       (d), this doesn't require that (d) 
> itself
>                                       has the privilege to map the pages, but
>                                       instead requires that (c) has the
>                                       privilege to do so, as long as (d) and 
> (t)
>                                       are allowed to share memory pages.
>                                       This is XENMEM_add_to_physmap_batch 
> only,
>                                       and currently ARM only. */

Which leaves unclear what (c), (d), and (t) are. How about

"GMFN from another dom, XENMEM_add_to_physmap_batch (and
currently ARM) only. Other than XENMAPSPACE_gmfn_foreign this
<explain here what the difference is with a few simple words>."

(You can and should go into further detail in the commit message.)
Without this _properly_ explained, I'll continue to ask why you
can't simply make XENMAPSPACE_gmfn_foreign do what you want
(as it already takes two domid_t-s as input), by suitably adjusting
its XSM check(s).

You'd also need to adjust the comment on the foreign_domid
structure field, as it saying "gmfn_foreign" would otherwise become
stale with your change.

I don't, btw, like the ARM only part here - there's nothing
inherently wrong with the same operation being sensible on x86.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel