[V1 PATCH] dom0 pvh linux support

[V1 PATCH] dom0 pvh linux support

[Mukesh Rathor]

Hi,

Attached please find patch for linux to support toolstack on pvh dom0.

thanks, 
Mukesh

[V1 PATCH] dom0 pvh: map foreign pfns in our p2m for toolstack

[Mukesh Rathor]

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..

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
 arch/x86/xen/mmu.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 112 insertions(+), 3 deletions(-)

diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 86e02ea..8efc066 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -2510,6 +2510,93 @@ void __init xen_hvm_init_mmu_ops(void)
 }
 #endif
 
+#ifdef CONFIG_XEN_PVH
+/*
+ * Map foreign gmfn, fgmfn, to local pfn, lpfn. This for the user space
+ * creating new guest on pvh dom0 and needing to map domU pages.
+ */
+static int xlate_add_to_p2m(unsigned long lpfn, unsigned long fgmfn,
+			    unsigned int domid)
+{
+	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;
+}
+
+static int xlate_remove_from_p2m(unsigned long spfn, int count)
+{
+	struct xen_remove_from_physmap xrp;
+	int i, rc;
+
+	for (i = 0; i < count; i++) {
+		xrp.domid = DOMID_SELF;
+		xrp.gpfn = spfn+i;
+		rc = HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrp);
+		if (rc)
+			break;
+	}
+	return rc;
+}
+
+struct xlate_remap_data {
+	unsigned long fgmfn; /* foreign domain's gmfn */
+	pgprot_t prot;
+	domid_t  domid;
+	int index;
+	struct page **pages;
+};
+
+static int xlate_map_pte_fn(pte_t *ptep, pgtable_t token, unsigned long addr,
+			    void *data)
+{
+	int rc;
+	struct xlate_remap_data *remap = data;
+	unsigned long pfn = page_to_pfn(remap->pages[remap->index++]);
+	pte_t pteval = pte_mkspecial(pfn_pte(pfn, remap->prot));
+
+	rc = xlate_add_to_p2m(pfn, remap->fgmfn, remap->domid);
+	if (rc)
+		return rc;
+	native_set_pte(ptep, pteval);
+
+	return 0;
+}
+
+static int xlate_remap_gmfn_range(struct vm_area_struct *vma,
+				  unsigned long addr, unsigned long mfn,
+				  int nr, pgprot_t prot, unsigned domid,
+				  struct page **pages)
+{
+	int err;
+	struct xlate_remap_data pvhdata;
+
+	BUG_ON(!pages);
+
+	pvhdata.fgmfn = mfn;
+	pvhdata.prot = prot;
+	pvhdata.domid = domid;
+	pvhdata.index = 0;
+	pvhdata.pages = pages;
+	err = apply_to_page_range(vma->vm_mm, addr, nr << PAGE_SHIFT,
+				  xlate_map_pte_fn, &pvhdata);
+	flush_tlb_all();
+	return err;
+}
+#endif
+
 #define REMAP_BATCH_SIZE 16
 
 struct remap_data {
@@ -2544,13 +2631,20 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
 	unsigned long range;
 	int err = 0;
 
-	if (xen_feature(XENFEAT_auto_translated_physmap))
-		return -EINVAL;
-
 	prot = __pgprot(pgprot_val(prot) | _PAGE_IOMAP);
 
 	BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_IO)) == (VM_PFNMAP | VM_IO)));
 
+	if (xen_feature(XENFEAT_auto_translated_physmap)) {
+#ifdef CONFIG_XEN_PVH
+		/* We need to update the local page tables and the xen HAP */
+		return xlate_remap_gmfn_range(vma, addr, mfn, nr, prot,
+					      domid, pages);
+#else
+		return -EINVAL;
+#endif
+	}
+
 	rmd.mfn = mfn;
 	rmd.prot = prot;
 
@@ -2588,6 +2682,21 @@ int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
 	if (!pages || !xen_feature(XENFEAT_auto_translated_physmap))
 		return 0;
 
+#ifdef CONFIG_XEN_PVH
+	while (numpgs--) {
+
+		/* The mmu has already cleaned up the process mmu resources at
+		 * this point (lookup_address will return NULL). */
+		unsigned long pfn = page_to_pfn(pages[numpgs]);
+
+		xlate_remove_from_p2m(pfn, 1);
+	}
+	/* We don't need to flush tlbs because as part of xlate_remove_from_p2m,
+	 * the hypervisor will do tlb flushes after removing the p2m entries
+	 * from the EPT/NPT */
+	return 0;
+#else
 	return -EINVAL;
+#endif
 }
 EXPORT_SYMBOL_GPL(xen_unmap_domain_mfn_range);
-- 
1.8.3.1

[V1 PATCH] dom0 pvh: map foreign pfns in our p2m for toolstack

[Mukesh Rathor]

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..

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
 arch/x86/xen/mmu.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 112 insertions(+), 3 deletions(-)

diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 86e02ea..8efc066 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -2510,6 +2510,93 @@ void __init xen_hvm_init_mmu_ops(void)
 }
 #endif
 
+#ifdef CONFIG_XEN_PVH
+/*
+ * Map foreign gmfn, fgmfn, to local pfn, lpfn. This for the user space
+ * creating new guest on pvh dom0 and needing to map domU pages.
+ */
+static int xlate_add_to_p2m(unsigned long lpfn, unsigned long fgmfn,
+			    unsigned int domid)
+{
+	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;
+}
+
+static int xlate_remove_from_p2m(unsigned long spfn, int count)
+{
+	struct xen_remove_from_physmap xrp;
+	int i, rc;
+
+	for (i = 0; i < count; i++) {
+		xrp.domid = DOMID_SELF;
+		xrp.gpfn = spfn+i;
+		rc = HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrp);
+		if (rc)
+			break;
+	}
+	return rc;
+}
+
+struct xlate_remap_data {
+	unsigned long fgmfn; /* foreign domain's gmfn */
+	pgprot_t prot;
+	domid_t  domid;
+	int index;
+	struct page **pages;
+};
+
+static int xlate_map_pte_fn(pte_t *ptep, pgtable_t token, unsigned long addr,
+			    void *data)
+{
+	int rc;
+	struct xlate_remap_data *remap = data;
+	unsigned long pfn = page_to_pfn(remap->pages[remap->index++]);
+	pte_t pteval = pte_mkspecial(pfn_pte(pfn, remap->prot));
+
+	rc = xlate_add_to_p2m(pfn, remap->fgmfn, remap->domid);
+	if (rc)
+		return rc;
+	native_set_pte(ptep, pteval);
+
+	return 0;
+}
+
+static int xlate_remap_gmfn_range(struct vm_area_struct *vma,
+				  unsigned long addr, unsigned long mfn,
+				  int nr, pgprot_t prot, unsigned domid,
+				  struct page **pages)
+{
+	int err;
+	struct xlate_remap_data pvhdata;
+
+	BUG_ON(!pages);
+
+	pvhdata.fgmfn = mfn;
+	pvhdata.prot = prot;
+	pvhdata.domid = domid;
+	pvhdata.index = 0;
+	pvhdata.pages = pages;
+	err = apply_to_page_range(vma->vm_mm, addr, nr << PAGE_SHIFT,
+				  xlate_map_pte_fn, &pvhdata);
+	flush_tlb_all();
+	return err;
+}
+#endif
+
 #define REMAP_BATCH_SIZE 16
 
 struct remap_data {
@@ -2544,13 +2631,20 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
 	unsigned long range;
 	int err = 0;
 
-	if (xen_feature(XENFEAT_auto_translated_physmap))
-		return -EINVAL;
-
 	prot = __pgprot(pgprot_val(prot) | _PAGE_IOMAP);
 
 	BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_IO)) == (VM_PFNMAP | VM_IO)));
 
+	if (xen_feature(XENFEAT_auto_translated_physmap)) {
+#ifdef CONFIG_XEN_PVH
+		/* We need to update the local page tables and the xen HAP */
+		return xlate_remap_gmfn_range(vma, addr, mfn, nr, prot,
+					      domid, pages);
+#else
+		return -EINVAL;
+#endif
+	}
+
 	rmd.mfn = mfn;
 	rmd.prot = prot;
 
@@ -2588,6 +2682,21 @@ int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
 	if (!pages || !xen_feature(XENFEAT_auto_translated_physmap))
 		return 0;
 
+#ifdef CONFIG_XEN_PVH
+	while (numpgs--) {
+
+		/* The mmu has already cleaned up the process mmu resources at
+		 * this point (lookup_address will return NULL). */
+		unsigned long pfn = page_to_pfn(pages[numpgs]);
+
+		xlate_remove_from_p2m(pfn, 1);
+	}
+	/* We don't need to flush tlbs because as part of xlate_remove_from_p2m,
+	 * the hypervisor will do tlb flushes after removing the p2m entries
+	 * from the EPT/NPT */
+	return 0;
+#else
 	return -EINVAL;
+#endif
 }
 EXPORT_SYMBOL_GPL(xen_unmap_domain_mfn_range);
-- 
1.8.3.1

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

[Roger Pau Monné]

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..
> 
> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> ---
>  arch/x86/xen/mmu.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 112 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
> index 86e02ea..8efc066 100644
> --- a/arch/x86/xen/mmu.c
> +++ b/arch/x86/xen/mmu.c
> @@ -2510,6 +2510,93 @@ void __init xen_hvm_init_mmu_ops(void)
>  }
>  #endif
>  
> +#ifdef CONFIG_XEN_PVH
> +/*
> + * Map foreign gmfn, fgmfn, to local pfn, lpfn. This for the user space
> + * creating new guest on pvh dom0 and needing to map domU pages.
> + */
> +static int xlate_add_to_p2m(unsigned long lpfn, unsigned long fgmfn,
> +			    unsigned int domid)
> +{
> +	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.

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.

Roger.

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

[Roger Pau Monné]

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..
> 
> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> ---
>  arch/x86/xen/mmu.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 112 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
> index 86e02ea..8efc066 100644
> --- a/arch/x86/xen/mmu.c
> +++ b/arch/x86/xen/mmu.c
> @@ -2510,6 +2510,93 @@ void __init xen_hvm_init_mmu_ops(void)
>  }
>  #endif
>  
> +#ifdef CONFIG_XEN_PVH
> +/*
> + * Map foreign gmfn, fgmfn, to local pfn, lpfn. This for the user space
> + * creating new guest on pvh dom0 and needing to map domU pages.
> + */
> +static int xlate_add_to_p2m(unsigned long lpfn, unsigned long fgmfn,
> +			    unsigned int domid)
> +{
> +	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.

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.

Roger.

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

[David Vrabel]

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

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

[David Vrabel]

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

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

[Mukesh Rathor]

On Tue, 27 May 2014 11:59:26 +0100
David Vrabel <david.vrabel@citrix.com> wrote:

> 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;

Thanks a lot.

> > 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.

Thanks again, pleasure working with maintainer like you!

Mukesh

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

[Mukesh Rathor]

On Tue, 27 May 2014 11:59:26 +0100
David Vrabel <david.vrabel@citrix.com> wrote:

> 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;

Thanks a lot.

> > 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.

Thanks again, pleasure working with maintainer like you!

Mukesh


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel