From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mukesh Rathor Subject: Re: [V3 PATCH 7/9] pvh: change xsm_add_to_physmap Date: Wed, 27 Nov 2013 12:29:39 -0800 Message-ID: <20131127122939.50228bce@mantra.us.oracle.com> References: <1385519230-21132-1-git-send-email-mukesh.rathor@oracle.com> <1385519230-21132-8-git-send-email-mukesh.rathor@oracle.com> <529621E3.8090906@tycho.nsa.gov> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <529621E3.8090906@tycho.nsa.gov> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Daniel De Graaf Cc: george.dunlap@eu.citrix.com, Xen-devel@lists.xensource.com, tim@xen.org, keir.xen@gmail.com, JBeulich@suse.com List-Id: xen-devel@lists.xenproject.org On Wed, 27 Nov 2013 11:46:27 -0500 Daniel De Graaf wrote: > On 11/26/2013 09:27 PM, Mukesh Rathor wrote: > > In preparation for the next patch, we update xsm_add_to_physmap to > > allow for checking of foreign domain. Thus, the current domain must > > have the right to update the mappings of target domain with pages > > from foreign domain. > > > > Signed-off-by: Mukesh Rathor > > CC: dgdegra@tycho.nsa.gov > > --- > > xen/arch/x86/mm.c | 16 +++++++++++++--- > > xen/include/xsm/dummy.h | 10 ++++++++-- > > xen/include/xsm/xsm.h | 6 +++--- > > xen/xsm/flask/hooks.c | 10 ++++++++-- > > 4 files changed, 32 insertions(+), 10 deletions(-) > > The XSM changes look good; however, the calling code needs a bit of > tweaking. Currently, if domain 0 is specified as the foreign domain, > the check is skipped, and the check is also run unnecessarily when > foreign_domid is nonzero but the operation is not > XENMAPSPACE_gmfn_foreign. The locking in this version also implies a > potential TOCTOU bug, but which in reality is impossible to trigger > due to the existing RCU lock held on (d). I would suggest passing > the foreign struct domain instead of the domid, as below. > > An unrelated question about XENMAPSPACE_gmfn_foreign that came up > while looking at this: is the domain parameter (d) supposed to be > ignored here, with maps always modifying the current domain? I would > have expected this call to manipulate d's physmap, with the common > case being (d == current->domain). Right, that is the assumption at present that PVH dom0 is creating user domains. Perhaps a debug assert would be a good idea. I can add that. It's probably straightforward to just manipulate d's physmap, but I'd rather do it when it's really time for it, than to anticipate something I can't test right now. thanks for your input. Mukesh