On 19/09/17 10:47, Paul Durrant wrote:
-----Original Message-----
From: Julien Grall [mailto:julien.gr...@arm.com]
Sent: 19 September 2017 10:40
To: Jan Beulich <jbeul...@suse.com>; Paul Durrant
<paul.durr...@citrix.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>; George Dunlap
<george.dun...@citrix.com>; Ian Jackson <ian.jack...@citrix.com>; Wei Liu
<wei.l...@citrix.com>; sstabell...@kernel.org; xen-devel@lists.xen.org; Tim
(Xen.org) <t...@xen.org>
Subject: Re: [Xen-devel] [PATCH v2] xen: grant-table: Simplify
get_paged_frame
Hi,
On 19/09/17 09:56, Jan Beulich wrote:
On 19.09.17 at 10:34, <paul.durr...@citrix.com> wrote:
I do wonder whether this function belongs in the grant table code though.
Getting the page from a (d, gfn) tuple is probably something that's
needed in
a few places and hence putting the code in common/memory.c (with
suitable
adjustment to the error values) would seem more appropriate.
That's been true from the very beginning of the existence of
the function, I think.
I am not sure how this function would fit in common/memory.c code. We
already have get_page_from_gfn to get a page from the tuple (d, gfn).
But that doesn't have the extra logic for populating and unsharing right?
Otherwise why would get_paged_frame() need to exist at all?
This code does not have the logic to unshare. It just denies it.
There are multiple places in grant-table which require similar check.
Hence why the helper is here to avoid duplicating the code in grant_table.c.
With this patch, this will contain specific check for grant mapping,
such as denying foreign mapping. Are we going to impose that for
get_paged_frame in common/memory.c?
This function adds more check that may not fit everyone. The only place
I could see potential usage is prepare_ring_for_helper. But what would
be a suitable name given?
I would leave the name alone and just move the code.
The name does not make sense in common/memory.c. Why would you allow
populating but not unsharing? This looks to me a decision that fits
grant-table and not necessarily the rest of the Xen.
So I still don't think this would be a suitable function in common/memory.c
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
