From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel De Graaf Subject: Re: [PATCH 18/23] arch/x86: Add missing mem_sharing XSM hooks Date: Thu, 27 Sep 2012 12:11:49 -0400 Message-ID: <50647AC5.9080905@tycho.nsa.gov> References: <1347895425-17959-1-git-send-email-dgdegra@tycho.nsa.gov> <1347895425-17959-19-git-send-email-dgdegra@tycho.nsa.gov> <20120927130956.GE8831@ocelot.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20120927130956.GE8831@ocelot.phlegethon.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tim Deegan Cc: Joe Epstein , keir@xen.org, Jan Beulich , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 09/27/2012 09:09 AM, Tim Deegan wrote: > Cc'ing Joe, the author of the original check I'm talking about below. > > At 11:23 -0400 on 17 Sep (1347881020), Daniel De Graaf wrote: >> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c >> index 24e2d93..7062f02 100644 >> --- a/xen/arch/x86/domctl.c >> +++ b/xen/arch/x86/domctl.c >> @@ -1447,10 +1447,8 @@ long arch_do_domctl( >> d = rcu_lock_domain_by_id(domctl->domain); >> if ( d != NULL ) >> { >> - ret = xsm_mem_event(d); >> - if ( !ret ) >> - ret = mem_event_domctl(d, &domctl->u.mem_event_op, >> - guest_handle_cast(u_domctl, void)); >> + ret = mem_event_domctl(d, &domctl->u.mem_event_op, >> + guest_handle_cast(u_domctl, void)); >> rcu_unlock_domain(d); >> copy_to_guest(u_domctl, domctl, 1); >> } >> @@ -1506,7 +1504,7 @@ long arch_do_domctl( >> d = rcu_lock_domain_by_id(domctl->domain); >> if ( d != NULL ) >> { >> - ret = xsm_mem_event(d); >> + ret = xsm_mem_event_setup(d); >> if ( !ret ) { >> p2m = p2m_get_hostp2m(d); >> p2m->access_required = domctl->u.access_required.access_required; > > [...] > >> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h >> index 626a332..5fb0afe 100644 >> --- a/xen/include/xsm/dummy.h >> +++ b/xen/include/xsm/dummy.h >> @@ -551,16 +551,37 @@ static XSM_DEFAULT(int, hvm_inject_msi) (struct domain *d) >> return 0; >> } >> >> -static XSM_DEFAULT(int, mem_event) (struct domain *d) >> +static XSM_DEFAULT(int, mem_event_setup) (struct domain *d) >> { >> return 0; >> } > > I think this ought to be at least IS_PRIV_FOR. I can see the original > code allowed all callers to use it, but surely it ought to be only for > the tools. Since only the tools can actually set the mem-access rights > (and so this is pretty much a noop) I don't think this causes any > substantial problem but we might as well adjust it anyway. > > Tim. Because this is a domctl, it already requires IS_PRIV as checked by xsm_domctl (and was already checked before this series). -- Daniel De Graaf National Security Agency