From mboxrd@z Thu Jan  1 00:00:00 1970
From: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Subject: Re: [PATCH 10/18] xenstored: use grant references
 instead of map_foreign_range
Date: Wed, 18 Jan 2012 13:18:53 -0500
Message-ID: <4F170D0D.8030304@tycho.nsa.gov>
References: <1326302490-19428-1-git-send-email-dgdegra@tycho.nsa.gov>
	<1326411330-7915-1-git-send-email-dgdegra@tycho.nsa.gov>
	<1326411330-7915-11-git-send-email-dgdegra@tycho.nsa.gov>
	<1326885330.14689.198.camel@zakaz.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <1326885330.14689.198.camel@zakaz.uk.xensource.com>
List-Unsubscribe: <http://lists.xensource.com/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: Alex Zeffertt <alex.zeffertt@eu.citrix.com>, "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>, Diego Ongaro <diego.ongaro@citrix.com>
List-Id: xen-devel@lists.xenproject.org

On 01/18/2012 06:15 AM, Ian Campbell wrote:
> On Thu, 2012-01-12 at 23:35 +0000, Daniel De Graaf wrote:
>> From: Alex Zeffertt <alex.zeffertt@eu.citrix.com>
>>
>> make xenstored use grantref rather than map_foreign_range (which can
>> only be used by privileged domains)
>>
>> This patch modifies the xenstore daemon to use xc_gnttab_map_grant_ref
>> instead of xc_map_foreign_range where available.
>>
>> Previous versions of this patch have been sent to xen-devel. See
>> http://lists.xensource.com/archives/html/xen-devel/2008-07/msg00610.html
>> http://lists.xensource.com/archives/html/xen-devel/2009-03/msg01492.html
>>
>> Signed-off-by: Diego Ongaro <diego.ongaro@citrix.com>
>> Signed-off-by: Alex Zeffertt <alex.zeffertt@eu.citrix.com>
>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>> ---
>>  tools/xenstore/xenstored_domain.c |   45 ++++++++++++++++++++++++++++++++-----
>>  1 files changed, 39 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
>> index 443af82..0b8353b 100644
>> --- a/tools/xenstore/xenstored_domain.c
>> +++ b/tools/xenstore/xenstored_domain.c
>> @@ -32,8 +32,10 @@
>>  #include "xenstored_watch.h"
>>  
>>  #include <xenctrl.h>
>> +#include <xen/grant_table.h>
>>  
>>  static xc_interface **xc_handle;
>> +static xc_gnttab **xcg_handle;
>>  static evtchn_port_t virq_port;
>>  
>>  xc_evtchn *xce_handle = NULL;
>> @@ -174,8 +176,12 @@ static int destroy_domain(void *_domain)
>>  			eprintf("> Unbinding port %i failed!\n", domain->port);
>>  	}
>>  
>> -	if (domain->interface)
>> -		munmap(domain->interface, getpagesize());
>> +	if (domain->interface) {
>> +		if (*xcg_handle >= 0 && domain->domid != 0)
> 
> Why the special case for domid 0 here? There seems to be no equivalent
> for the map case, including the one you add in patch 15/18.
 
dom0 is mapped by dom0_init()/xenbus_map() and must be unmapped with munmap,
which is not guaranteed to be the same as xc_gnttab_munmap. The map case
doesn't need to handle that.

> I think the map and unmap logic could usefully be made into helper
> functions.
> 
> Ian.

-- 
Daniel De Graaf
National Security Agency