>>> On 14.02.18 at 08:15, <blacksk...@gmail.com> wrote: > Hi Jan, > > 2018-02-13 23:26 GMT+08:00 Jan Beulich <jbeul...@suse.com>: >>>>> On 13.02.18 at 16:15, <blacksk...@gmail.com> 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@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
