Re: Re: [PATCH v2] xen/gntdev, gntalloc: Remove unneeded VM flags

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

On 03/09/2011 12:05 PM, Konrad Rzeszutek Wilk wrote:
>>> @@ -471,8 +483,6 @@ static int gntalloc_mmap(struct file *filp, struct vm_area_struct *vma)
>>>  	vma->vm_private_data = gref;
>>>  
>>>  	vma->vm_flags |= VM_RESERVED;
>>> -	vma->vm_flags |= VM_DONTCOPY;
>>> -	vma->vm_flags |= VM_PFNMAP | VM_PFN_AT_MMAP;
> 
> So the gntalloc driver can be used on PV. You remove the DONTCOPY,PFNMAP, and PFN_AT_MMAP
> .. while
> 
>>> -	vma->vm_flags |= VM_RESERVED|VM_DONTCOPY|VM_DONTEXPAND|VM_PFNMAP;
>>> +	vma->vm_flags |= VM_RESERVED|VM_DONTEXPAND;
>>> +
>>> +	if (use_ptemod)
>>> +		vma->vm_flags |= VM_DONTCOPY|VM_PFNMAP;
>>>  
> 
> Here (gntdev) you are more careful. Should we do the same check under PV
> for the gntalloc driver?
> 
> Have you tested this driver in PV <-> HVM env?
> 

I have not run this test case yet; however, I expect that it will work. The
reason that we need to be careful in gntdev on PV guests is because we are
not changing the PFN/MFN mapping on PV; instead, we change the application's
page tables to point to the other domain's memory. This means that the vma
cannot be copied without using another grant mapping hypercall; it also
requires special handling on unmap, which is the reason for gntdev's
dependency on the MMU notifier.

For gntalloc, this is not a concern - the pages are owned by the domain
using the gntalloc device, and can be mapped and unmapped in the same manner
as any other page of memory.

It is possible to use the same PFN/MFN mapping change in PV that we use in
HVM, which would allow PV guests to avoid treating gntdev memory specially.
I found problems with doing this the first time I tried, but I think they
were related to issues with updating the p2m and m2p maps that have since
been addressed. This change would effectively make "use_ptemod" always
false. It's slightly less efficient because the actual map requires two
hypercalls (one to update the p2m, and one to actually set up page tables)
but this doesn't seem like it would be an important issue.

-- 
Daniel De Graaf
National Security Agency