Re: [Xen-devel] [V1 PATCH] dom0 pvh: map foreign pfns in our p2m for toolstack

[David Vrabel <david.vrabel@citrix.com>]

On 27/05/14 11:43, Roger Pau Monné wrote:
> On 24/05/14 03:33, Mukesh Rathor wrote:
>> When running as dom0 in pvh mode, foreign pfns that are accessed must be
>> added to our p2m which is managed by xen. This is done via
>> XENMEM_add_to_physmap_range hypercall. This is needed for toolstack
>> building guests and mapping guest memory, xentrace mapping xen pages,
>> etc..

Thanks.

Applied to devel/for-linus-3.16, but see comments below.

>> +static int xlate_add_to_p2m(unsigned long lpfn, unsigned long fgmfn,
>> +			    unsigned int domid)

The preferred abbreviation is GFN not GMFN.  I fixed this up.

>> +{
>> +	int rc, err = 0;
>> +	xen_pfn_t gpfn = lpfn;
>> +	xen_ulong_t idx = fgmfn;
>> +
>> +	struct xen_add_to_physmap_range xatp = {
>> +		.domid = DOMID_SELF,
>> +		.foreign_domid = domid,
>> +		.size = 1,
>> +		.space = XENMAPSPACE_gmfn_foreign,
>> +	};
>> +	set_xen_guest_handle(xatp.idxs, &idx);
>> +	set_xen_guest_handle(xatp.gpfns, &gpfn);
>> +	set_xen_guest_handle(xatp.errs, &err);
>> +
>> +	rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
>> +	return rc;
> 
> Thanks for the patches, I see two problems with this approach, the first
> one is that you are completely ignoring the error in the variable "err",
> which means that you can end up with a pfn that Linux thinks it's valid,
> but it's not mapped to any mfn, so when you try to access it you will
> trigger the vioapic crash.

I spotted this and fixed this up by adding:

+       if (rc < 0)
+               return rc;
+       return err;

> The second one is that this seems extremely inefficient, you are issuing
> one hypercall for each memory page, when you could instead batch all the
> pages into a single hypercall and map them in one shot.

I agree, but the 3.16 merge window is nearly here so I've applied it
as-is.  Note that the privcmd driver calls this function once per page,
so the lack of batching doesn't really hurt here.

David