Re: [PATCH 10/18] xenstored: use grant references instead of map_foreign_range

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

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