All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
  2009-05-20 14:44 [GIT] privcmd fixes --> working HVM Ian Campbell
@ 2009-05-20 14:45 ` Ian Campbell
  2009-05-20 19:33   ` Jeremy Fitzhardinge
  2009-05-20 21:13   ` Jeremy Fitzhardinge
  0 siblings, 2 replies; 11+ messages in thread
From: Ian Campbell @ 2009-05-20 14:45 UTC (permalink / raw)
  To: xen-devel; +Cc: Jeremy Fitzhardinge, Ian Campbell

On error IOCTL_PRIVCMD_MMAPBATCH is expected to set the top nibble of
the effected MFN and return 0. Currently it leaves the MFN unmodified
and returns the number of failures. Therefore:

- reimplement remap_domain_mfn_range() using direct
  HYPERVISOR_mmu_update() calls and small batches. The xen_set_domain_pte()
  interface does not report errors and since some failures are
  expected/normal using the multicall infrastructure is too noisy.
- return 0 as expected
- writeback the updated MFN list to mmapbatch->arr not over mmapbatch,
  smashing the caller's stack.
- remap_domain_mfn_range can be static.

With this change I am able to start an HVM domain.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
---
 drivers/xen/xenfs/privcmd.c |   56 +++++++++++++++++++++++++++++++-----------
 1 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/drivers/xen/xenfs/privcmd.c b/drivers/xen/xenfs/privcmd.c
index 263f622..110b062 100644
--- a/drivers/xen/xenfs/privcmd.c
+++ b/drivers/xen/xenfs/privcmd.c
@@ -31,14 +31,16 @@
 #include <xen/features.h>
 #include <xen/page.h>
 
+#define REMAP_BATCH_SIZE 16
+
 #ifndef HAVE_ARCH_PRIVCMD_MMAP
 static int privcmd_enforce_singleshot_mapping(struct vm_area_struct *vma);
 #endif
 
 struct remap_data {
 	unsigned long mfn;
-	unsigned domid;
 	pgprot_t prot;
+	struct mmu_update *mmu_update;
 };
 
 static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token,
@@ -47,17 +49,23 @@ static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token,
 	struct remap_data *rmd = data;
 	pte_t pte = pte_mkspecial(pfn_pte(rmd->mfn++, rmd->prot));
 
-	xen_set_domain_pte(ptep, pte, rmd->domid);
+	rmd->mmu_update->ptr = arbitrary_virt_to_machine(ptep).maddr;
+	rmd->mmu_update->val = pte_val_ma(pte);
+	rmd->mmu_update++;
 
 	return 0;
 }
 
-int remap_domain_mfn_range(struct vm_area_struct *vma, unsigned long addr,
-			   unsigned long mfn, unsigned long size,
-			   pgprot_t prot, unsigned domid)
+static int remap_domain_mfn_range(struct vm_area_struct *vma,
+				  unsigned long addr,
+				  unsigned long mfn, int nr,
+				  pgprot_t prot, unsigned domid)
 {
 	struct remap_data rmd;
-	int err;
+	struct mmu_update mmu_update[REMAP_BATCH_SIZE];
+	int batch;
+	unsigned long range;
+	int err = 0;
 
 	prot = __pgprot(pgprot_val(prot) | _PAGE_IOMAP);
 
@@ -65,10 +73,29 @@ int remap_domain_mfn_range(struct vm_area_struct *vma, unsigned long addr,
 
 	rmd.mfn = mfn;
 	rmd.prot = prot;
-	rmd.domid = domid;
 
-	err = apply_to_page_range(vma->vm_mm, addr, size,
-				  remap_area_mfn_pte_fn, &rmd);
+	while (nr) {
+		batch = min(REMAP_BATCH_SIZE, nr);
+		range = (unsigned long)batch << PAGE_SHIFT;
+
+		rmd.mmu_update = mmu_update;
+		err = apply_to_page_range(vma->vm_mm, addr, range,
+					  remap_area_mfn_pte_fn, &rmd);
+		if (err)
+			goto out;
+
+		err = -EFAULT;
+		if (HYPERVISOR_mmu_update(mmu_update, batch, NULL, domid) < 0)
+			goto out;
+
+		nr -= batch;
+		addr += range;
+	}
+
+	err = 0;
+out:
+
+	flush_tlb_all();
 
 	return err;
 }
@@ -157,7 +184,7 @@ static int traverse_pages(unsigned nelem, size_t size,
 {
 	void *pagedata;
 	unsigned pageidx;
-	int ret;
+	int ret = 0;
 
 	BUG_ON(size > PAGE_SIZE);
 
@@ -207,8 +234,7 @@ static int mmap_mfn_range(void *data, void *state)
 
 	rc = remap_domain_mfn_range(vma,
 				    msg->va & PAGE_MASK,
-				    msg->mfn,
-				    msg->npages << PAGE_SHIFT,
+				    msg->mfn, msg->npages,
 				    vma->vm_page_prot,
 				    st->domain);
 	if (rc < 0)
@@ -289,7 +315,7 @@ static int mmap_batch_fn(void *data, void *state)
 	struct mmap_batch_state *st = state;
 
 	if (remap_domain_mfn_range(st->vma, st->va & PAGE_MASK,
-				   *mfnp, PAGE_SIZE,
+				   *mfnp, 1,
 				   st->vma->vm_page_prot, st->domain) < 0) {
 		*mfnp |= 0xf0000000U;
 		st->err++;
@@ -361,9 +387,9 @@ static long privcmd_ioctl_mmap_batch(void __user *udata)
 	up_write(&mm->mmap_sem);
 
 	if (state.err > 0) {
-		ret = state.err;
+		ret = 0;
 
-		state.user = udata;
+		state.user = m.arr;
 		traverse_pages(m.num, sizeof(xen_pfn_t),
 			       &pagelist,
 			       mmap_return_errors, &state);
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
@ 2009-05-20 19:22 Boris Derzhavets
  0 siblings, 0 replies; 11+ messages in thread
From: Boris Derzhavets @ 2009-05-20 19:22 UTC (permalink / raw)
  To: xen-devel; +Cc: Jeremy Fitzhardinge, Ian Campbell


[-- Attachment #1.1: Type: text/plain, Size: 5364 bytes --]

How long it takes for your patches to get in 
git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git ?

Boris.

--- On Wed, 5/20/09, Ian Campbell <ian.campbell@citrix.com> wrote:

From: Ian Campbell <ian.campbell@citrix.com>
Subject: [Xen-devel] [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
To: xen-devel@lists.xensource.com
Cc: "Jeremy Fitzhardinge" <jeremy@goop.org>, "Ian Campbell" <ian.campbell@citrix.com>
Date: Wednesday, May 20, 2009, 10:45 AM

On error IOCTL_PRIVCMD_MMAPBATCH is expected to set the top nibble of
the effected MFN and return 0. Currently it leaves the MFN unmodified
and returns the number of failures. Therefore:

- reimplement remap_domain_mfn_range() using direct
  HYPERVISOR_mmu_update() calls and small batches. The xen_set_domain_pte()
  interface does not report errors and since some failures are
  expected/normal using the multicall infrastructure is too noisy.
- return 0 as expected
- writeback the updated MFN list to mmapbatch->arr not over mmapbatch,
  smashing the caller's stack.
- remap_domain_mfn_range can be static.

With this change I am able to start an HVM domain.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
---
 drivers/xen/xenfs/privcmd.c |   56 +++++++++++++++++++++++++++++++-----------
 1 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/drivers/xen/xenfs/privcmd.c b/drivers/xen/xenfs/privcmd.c
index 263f622..110b062 100644
--- a/drivers/xen/xenfs/privcmd.c
+++ b/drivers/xen/xenfs/privcmd.c
@@ -31,14 +31,16 @@
 #include <xen/features.h>
 #include <xen/page.h>
 
+#define REMAP_BATCH_SIZE 16
+
 #ifndef HAVE_ARCH_PRIVCMD_MMAP
 static int privcmd_enforce_singleshot_mapping(struct vm_area_struct *vma);
 #endif
 
 struct remap_data {
     unsigned long mfn;
-    unsigned domid;
     pgprot_t prot;
+    struct mmu_update *mmu_update;
 };
 
 static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token,
@@ -47,17 +49,23 @@ static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token,
     struct remap_data *rmd = data;
     pte_t pte = pte_mkspecial(pfn_pte(rmd->mfn++, rmd->prot));
 
-    xen_set_domain_pte(ptep, pte, rmd->domid);
+    rmd->mmu_update->ptr = arbitrary_virt_to_machine(ptep).maddr;
+    rmd->mmu_update->val = pte_val_ma(pte);
+    rmd->mmu_update++;
 
     return 0;
 }
 
-int remap_domain_mfn_range(struct vm_area_struct *vma, unsigned long addr,
-               unsigned long mfn, unsigned long size,
-               pgprot_t prot, unsigned domid)
+static int remap_domain_mfn_range(struct vm_area_struct *vma,
+                  unsigned long addr,
+                  unsigned long mfn, int nr,
+                  pgprot_t prot, unsigned domid)
 {
     struct remap_data rmd;
-    int err;
+    struct mmu_update mmu_update[REMAP_BATCH_SIZE];
+    int batch;
+    unsigned long range;
+    int err = 0;
 
     prot = __pgprot(pgprot_val(prot) | _PAGE_IOMAP);
 
@@ -65,10 +73,29 @@ int remap_domain_mfn_range(struct vm_area_struct *vma, unsigned long addr,
 
     rmd.mfn = mfn;
     rmd.prot = prot;
-    rmd.domid = domid;
 
-    err = apply_to_page_range(vma->vm_mm, addr, size,
-                  remap_area_mfn_pte_fn, &rmd);
+    while (nr) {
+        batch = min(REMAP_BATCH_SIZE, nr);
+        range = (unsigned long)batch << PAGE_SHIFT;
+
+        rmd.mmu_update = mmu_update;
+        err = apply_to_page_range(vma->vm_mm, addr, range,
+                      remap_area_mfn_pte_fn, &rmd);
+        if (err)
+            goto out;
+
+        err = -EFAULT;
+        if (HYPERVISOR_mmu_update(mmu_update, batch, NULL, domid) < 0)
+            goto out;
+
+        nr -= batch;
+        addr += range;
+    }
+
+    err = 0;
+out:
+
+    flush_tlb_all();
 
     return err;
 }
@@ -157,7 +184,7 @@ static int traverse_pages(unsigned nelem, size_t size,
 {
     void *pagedata;
     unsigned pageidx;
-    int ret;
+    int ret = 0;
 
     BUG_ON(size > PAGE_SIZE);
 
@@ -207,8 +234,7 @@ static int mmap_mfn_range(void *data, void *state)
 
     rc = remap_domain_mfn_range(vma,
                     msg->va & PAGE_MASK,
-                    msg->mfn,
-                    msg->npages << PAGE_SHIFT,
+                    msg->mfn, msg->npages,
                     vma->vm_page_prot,
                     st->domain);
     if (rc < 0)
@@ -289,7 +315,7 @@ static int mmap_batch_fn(void *data, void *state)
     struct mmap_batch_state *st = state;
 
     if (remap_domain_mfn_range(st->vma, st->va & PAGE_MASK,
-                   *mfnp, PAGE_SIZE,
+                   *mfnp, 1,
                    st->vma->vm_page_prot, st->domain) < 0) {
         *mfnp |= 0xf0000000U;
         st->err++;
@@ -361,9 +387,9 @@ static long privcmd_ioctl_mmap_batch(void __user *udata)
     up_write(&mm->mmap_sem);
 
     if (state.err > 0) {
-        ret = state.err;
+        ret = 0;
 
-        state.user = udata;
+        state.user = m.arr;
         traverse_pages(m.num, sizeof(xen_pfn_t),
                    &pagelist,
                    mmap_return_errors, &state);
-- 
1.5.6.5


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



      

[-- Attachment #1.2: Type: text/html, Size: 8941 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
  2009-05-20 14:45 ` [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting Ian Campbell
@ 2009-05-20 19:33   ` Jeremy Fitzhardinge
  2009-05-21  8:51     ` Ian Campbell
  2009-05-20 21:13   ` Jeremy Fitzhardinge
  1 sibling, 1 reply; 11+ messages in thread
From: Jeremy Fitzhardinge @ 2009-05-20 19:33 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell wrote:
> On error IOCTL_PRIVCMD_MMAPBATCH is expected to set the top nibble of
> the effected MFN and return 0. Currently it leaves the MFN unmodified
> and returns the number of failures. Therefore:
>
> - reimplement remap_domain_mfn_range() using direct
>   HYPERVISOR_mmu_update() calls and small batches. The xen_set_domain_pte()
>   interface does not report errors and since some failures are
>   expected/normal using the multicall infrastructure is too noisy.
>   
The noise is just for debugging; if failure is expected, then maybe we 
can extend it to be quiet about those cases.
> - return 0 as expected
> - writeback the updated MFN list to mmapbatch->arr not over mmapbatch,
>   smashing the caller's stack.
>   
Oops.
> - remap_domain_mfn_range can be static.
>
> With this change I am able to start an HVM domain.
>   

OK, good.  I've pulled this into xen-tip/dom0/xenfs.

Thanks,
    J

> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jeremy Fitzhardinge <jeremy@goop.org>
> ---
>  drivers/xen/xenfs/privcmd.c |   56 +++++++++++++++++++++++++++++++-----------
>  1 files changed, 41 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/xen/xenfs/privcmd.c b/drivers/xen/xenfs/privcmd.c
> index 263f622..110b062 100644
> --- a/drivers/xen/xenfs/privcmd.c
> +++ b/drivers/xen/xenfs/privcmd.c
> @@ -31,14 +31,16 @@
>  #include <xen/features.h>
>  #include <xen/page.h>
>  
> +#define REMAP_BATCH_SIZE 16
> +
>  #ifndef HAVE_ARCH_PRIVCMD_MMAP
>  static int privcmd_enforce_singleshot_mapping(struct vm_area_struct *vma);
>  #endif
>  
>  struct remap_data {
>  	unsigned long mfn;
> -	unsigned domid;
>  	pgprot_t prot;
> +	struct mmu_update *mmu_update;
>  };
>  
>  static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token,
> @@ -47,17 +49,23 @@ static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token,
>  	struct remap_data *rmd = data;
>  	pte_t pte = pte_mkspecial(pfn_pte(rmd->mfn++, rmd->prot));
>  
> -	xen_set_domain_pte(ptep, pte, rmd->domid);
> +	rmd->mmu_update->ptr = arbitrary_virt_to_machine(ptep).maddr;
> +	rmd->mmu_update->val = pte_val_ma(pte);
> +	rmd->mmu_update++;
>  
>  	return 0;
>  }
>  
> -int remap_domain_mfn_range(struct vm_area_struct *vma, unsigned long addr,
> -			   unsigned long mfn, unsigned long size,
> -			   pgprot_t prot, unsigned domid)
> +static int remap_domain_mfn_range(struct vm_area_struct *vma,
> +				  unsigned long addr,
> +				  unsigned long mfn, int nr,
> +				  pgprot_t prot, unsigned domid)
>  {
>  	struct remap_data rmd;
> -	int err;
> +	struct mmu_update mmu_update[REMAP_BATCH_SIZE];
> +	int batch;
> +	unsigned long range;
> +	int err = 0;
>  
>  	prot = __pgprot(pgprot_val(prot) | _PAGE_IOMAP);
>  
> @@ -65,10 +73,29 @@ int remap_domain_mfn_range(struct vm_area_struct *vma, unsigned long addr,
>  
>  	rmd.mfn = mfn;
>  	rmd.prot = prot;
> -	rmd.domid = domid;
>  
> -	err = apply_to_page_range(vma->vm_mm, addr, size,
> -				  remap_area_mfn_pte_fn, &rmd);
> +	while (nr) {
> +		batch = min(REMAP_BATCH_SIZE, nr);
> +		range = (unsigned long)batch << PAGE_SHIFT;
> +
> +		rmd.mmu_update = mmu_update;
> +		err = apply_to_page_range(vma->vm_mm, addr, range,
> +					  remap_area_mfn_pte_fn, &rmd);
> +		if (err)
> +			goto out;
> +
> +		err = -EFAULT;
> +		if (HYPERVISOR_mmu_update(mmu_update, batch, NULL, domid) < 0)
> +			goto out;
> +
> +		nr -= batch;
> +		addr += range;
> +	}
> +
> +	err = 0;
> +out:
> +
> +	flush_tlb_all();
>  
>  	return err;
>  }
> @@ -157,7 +184,7 @@ static int traverse_pages(unsigned nelem, size_t size,
>  {
>  	void *pagedata;
>  	unsigned pageidx;
> -	int ret;
> +	int ret = 0;
>  
>  	BUG_ON(size > PAGE_SIZE);
>  
> @@ -207,8 +234,7 @@ static int mmap_mfn_range(void *data, void *state)
>  
>  	rc = remap_domain_mfn_range(vma,
>  				    msg->va & PAGE_MASK,
> -				    msg->mfn,
> -				    msg->npages << PAGE_SHIFT,
> +				    msg->mfn, msg->npages,
>  				    vma->vm_page_prot,
>  				    st->domain);
>  	if (rc < 0)
> @@ -289,7 +315,7 @@ static int mmap_batch_fn(void *data, void *state)
>  	struct mmap_batch_state *st = state;
>  
>  	if (remap_domain_mfn_range(st->vma, st->va & PAGE_MASK,
> -				   *mfnp, PAGE_SIZE,
> +				   *mfnp, 1,
>  				   st->vma->vm_page_prot, st->domain) < 0) {
>  		*mfnp |= 0xf0000000U;
>  		st->err++;
> @@ -361,9 +387,9 @@ static long privcmd_ioctl_mmap_batch(void __user *udata)
>  	up_write(&mm->mmap_sem);
>  
>  	if (state.err > 0) {
> -		ret = state.err;
> +		ret = 0;
>  
> -		state.user = udata;
> +		state.user = m.arr;
>  		traverse_pages(m.num, sizeof(xen_pfn_t),
>  			       &pagelist,
>  			       mmap_return_errors, &state);
>   

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
  2009-05-20 14:45 ` [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting Ian Campbell
  2009-05-20 19:33   ` Jeremy Fitzhardinge
@ 2009-05-20 21:13   ` Jeremy Fitzhardinge
  1 sibling, 0 replies; 11+ messages in thread
From: Jeremy Fitzhardinge @ 2009-05-20 21:13 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell wrote:
> On error IOCTL_PRIVCMD_MMAPBATCH is expected to set the top nibble of
> the effected MFN and return 0. Currently it leaves the MFN unmodified
> and returns the number of failures. Therefore:
>
> - reimplement remap_domain_mfn_range() using direct
>   HYPERVISOR_mmu_update() calls and small batches. The xen_set_domain_pte()
>   interface does not report errors and since some failures are
>   expected/normal using the multicall infrastructure is too noisy.
> - return 0 as expected
> - writeback the updated MFN list to mmapbatch->arr not over mmapbatch,
>   smashing the caller's stack.
> - remap_domain_mfn_range can be static.
>
> With this change I am able to start an HVM domain.
>   

This breaks compiling xenfs as a module; neither flush_tlb_all or 
arbitrary_virt_to_machine are exported, I think.

    J

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
@ 2009-05-21  8:09 Boris Derzhavets
  2009-05-21 17:16 ` Jeremy Fitzhardinge
  0 siblings, 1 reply; 11+ messages in thread
From: Boris Derzhavets @ 2009-05-21  8:09 UTC (permalink / raw)
  To: Ian Campbell, Jeremy Fitzhardinge; +Cc: xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 1611 bytes --]

I've compiled 2.6.30-rc6-tip with xenfs built-in and successfully created and ran
Ubuntu 8.10 Server HVM and CentOS 5.3 HVM DomUs with traditional hvm-profiles.
Remote VNC connections to HVM DomUs might be unexpectedly frozen either broken after 5-10 minutes .

Boris.

--- On Wed, 5/20/09, Jeremy Fitzhardinge <jeremy@goop.org> wrote:

From: Jeremy Fitzhardinge <jeremy@goop.org>
Subject: Re: [Xen-devel] [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
To: "Ian Campbell" <ian.campbell@citrix.com>
Cc: xen-devel@lists.xensource.com
Date: Wednesday, May 20, 2009, 5:13 PM

Ian Campbell wrote:
> On error IOCTL_PRIVCMD_MMAPBATCH is expected to set the top nibble of
> the effected MFN and return 0. Currently it leaves the MFN unmodified
> and returns the number of failures. Therefore:
>
> - reimplement remap_domain_mfn_range() using direct
>   HYPERVISOR_mmu_update() calls and small batches. The xen_set_domain_pte()
>   interface does not report errors and since some failures are
>   expected/normal using the multicall infrastructure is too noisy.
> - return 0 as expected
> - writeback the updated MFN list to mmapbatch->arr not over mmapbatch,
>   smashing the caller's stack.
> - remap_domain_mfn_range can be static.
>
> With this change I am able to start an HVM domain.
>   

This breaks compiling xenfs as a module; neither flush_tlb_all or 
arbitrary_virt_to_machine are exported, I think.

    J

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



      

[-- Attachment #1.2: Type: text/html, Size: 2292 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
  2009-05-20 19:33   ` Jeremy Fitzhardinge
@ 2009-05-21  8:51     ` Ian Campbell
  2009-05-21  9:18       ` Ian Campbell
  2009-05-21 17:14       ` Jeremy Fitzhardinge
  0 siblings, 2 replies; 11+ messages in thread
From: Ian Campbell @ 2009-05-21  8:51 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: xen-devel@lists.xensource.com

On Wed, 2009-05-20 at 15:33 -0400, Jeremy Fitzhardinge wrote:
> Ian Campbell wrote:
> > On error IOCTL_PRIVCMD_MMAPBATCH is expected to set the top nibble of
> > the effected MFN and return 0. Currently it leaves the MFN unmodified
> > and returns the number of failures. Therefore:
> >
> > - reimplement remap_domain_mfn_range() using direct
> >   HYPERVISOR_mmu_update() calls and small batches. The xen_set_domain_pte()
> >   interface does not report errors and since some failures are
> >   expected/normal using the multicall infrastructure is too noisy.
> >   
> The noise is just for debugging; if failure is expected, then maybe we 
> can extend it to be quiet about those cases.

In this specific instance going directly at the mmu_udpate interface is
probably better since the propagation of errors from the multicall
infrastructure is tricky (well, currently non-existent). I don't think
multicalls would buy us anything here anyhow since mmu_update is batched
already.

On Wed, 2009-05-20 at 17:13 -0400, Jeremy Fitzhardinge wrote:
> This breaks compiling xenfs as a module; neither flush_tlb_all or 
> arbitrary_virt_to_machine are exported, I think.

Rather than exporting those I think moving remap_domain_mfn_range() to
core code (with xen_ on the front of the name) and exporting that would
be cleaner. Thoughts?

Ian.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
  2009-05-21  8:51     ` Ian Campbell
@ 2009-05-21  9:18       ` Ian Campbell
  2009-05-21 17:14       ` Jeremy Fitzhardinge
  1 sibling, 0 replies; 11+ messages in thread
From: Ian Campbell @ 2009-05-21  9:18 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Isaku, Yamahata, xen-devel@lists.xensource.com

On Thu, 2009-05-21 at 04:51 -0400, Ian Campbell wrote:
> 
> Rather than exporting those I think moving remap_domain_mfn_range() to
> core code (with xen_ on the front of the name) and exporting that
> would be cleaner. Thoughts?

The following changes since commit 0fccc3aa1ef9fe0753e1f5b4caf7803ae87dd59c:
  Ian Campbell (1):
        privcmd: MMAPBATCH: Fix error handling/reporting

are available in the git repository at:

  git://xenbits.xensource.com/people/ianc/linux-2.6.git for-jeremy/privcmd

Ian Campbell (1):
      xen/privcmd: move remap_domain_mfn_range() to core xen code and export.

 arch/x86/xen/mmu.c          |   66 +++++++++++++++++++++++++++++++++++
 drivers/xen/xenfs/privcmd.c |   81 ++++--------------------------------------
 include/xen/xen-ops.h       |    5 +++
 3 files changed, 79 insertions(+), 73 deletions(-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
  2009-05-21  8:51     ` Ian Campbell
  2009-05-21  9:18       ` Ian Campbell
@ 2009-05-21 17:14       ` Jeremy Fitzhardinge
  1 sibling, 0 replies; 11+ messages in thread
From: Jeremy Fitzhardinge @ 2009-05-21 17:14 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel@lists.xensource.com

Ian Campbell wrote:
>> The noise is just for debugging; if failure is expected, then maybe we 
>> can extend it to be quiet about those cases.
>>     
>
> In this specific instance going directly at the mmu_udpate interface is
> probably better since the propagation of errors from the multicall
> infrastructure is tricky (well, currently non-existent). I don't think
> multicalls would buy us anything here anyhow since mmu_update is batched
> already.
>   

Yeah.  The generic multicall stuff is (should be) tuned for the common 
case of no errors.  I think this is the first instance of something 
where we expect errors back.  The multicall path is fairly hot, and I 
suspect it's going to need some trimming when the real performance work 
starts, so keeping it low-feature is a good idea.

(Though we could make use of maybe-fail to deal with vmap aliases in 
pte-pinning...)

>> This breaks compiling xenfs as a module; neither flush_tlb_all or 
>> arbitrary_virt_to_machine are exported, I think.
>>     
>
> Rather than exporting those I think moving remap_domain_mfn_range() to
> core code (with xen_ on the front of the name) and exporting that would
> be cleaner. Thoughts?
>   

Yes, seems reasonable to me.  Though if its arch-neutral, drivers/xen 
would be better.

    J

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
  2009-05-21  8:09 [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting Boris Derzhavets
@ 2009-05-21 17:16 ` Jeremy Fitzhardinge
  0 siblings, 0 replies; 11+ messages in thread
From: Jeremy Fitzhardinge @ 2009-05-21 17:16 UTC (permalink / raw)
  To: Boris Derzhavets; +Cc: xen-devel, Ian Campbell

Boris Derzhavets wrote:
> I've compiled 2.6.30-rc6-tip with xenfs built-in and successfully 
> created and ran
> Ubuntu 8.10 Server HVM and CentOS 5.3 HVM DomUs with traditional 
> hvm-profiles.
> Remote VNC connections to HVM DomUs might be unexpectedly frozen 
> either broken after 5-10 minutes .
>

That's not new behaviour though, is it?  You reported VNC failures 
before.  Does disabling GSO help?

    J

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
@ 2009-05-22 18:07 Boris Derzhavets
  2009-05-22 18:22 ` Pasi Kärkkäinen
  0 siblings, 1 reply; 11+ messages in thread
From: Boris Derzhavets @ 2009-05-22 18:07 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: xen-devel, Ian Campbell


[-- Attachment #1.1: Type: text/plain, Size: 1544 bytes --]


 It's new behavior i might be disconnected from remote Xen 3.4 Host during
vnc session doing HVM install or just during running HVM via remote vnc
connection. To restore network i always have to reboot Xen Host itself.
I suspect hardware issues due to problems between cheap hub and
Marvel Yukon PC-E Gigabit Ethernet. To scp 4 GB file i always have to replug
network cable to PCI Ethernet ( RTL 8069SC or E8088 )

However, i attempted to switch off tso at HVM DomU:-

[root@FHVM ethtool-6]# /usr/local/sbin/ethtool -K eth1 tso off
Cannot set device tcp segmentation offload settings: Operation not supported
[root@FHVM ethtool-6]# /usr/local/sbin/ethtool -K eth1 tx off
Cannot set device tx csum settings: Operation not supported

Boris.

--- On Thu, 5/21/09, Jeremy Fitzhardinge <jeremy@goop.org> wrote:

From: Jeremy Fitzhardinge <jeremy@goop.org>
Subject: Re: [Xen-devel] [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
To: "Boris Derzhavets" <bderzhavets@yahoo.com>
Cc: "Ian Campbell" <ian.campbell@citrix.com>, xen-devel@lists.xensource.com
Date: Thursday, May 21, 2009, 1:16 PM

Boris Derzhavets wrote:
> I've compiled 2.6.30-rc6-tip with xenfs built-in and successfully created and ran
> Ubuntu 8.10 Server HVM and CentOS 5.3 HVM DomUs with traditional hvm-profiles.
> Remote VNC connections to HVM DomUs might be unexpectedly frozen either broken after 5-10 minutes .
> 

That's not new behaviour though, is it?  You reported VNC failures before.  Does disabling GSO help?

   J



      

[-- Attachment #1.2: Type: text/html, Size: 1951 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
  2009-05-22 18:07 Boris Derzhavets
@ 2009-05-22 18:22 ` Pasi Kärkkäinen
  0 siblings, 0 replies; 11+ messages in thread
From: Pasi Kärkkäinen @ 2009-05-22 18:22 UTC (permalink / raw)
  To: Boris Derzhavets; +Cc: Jeremy Fitzhardinge, xen-devel, Ian Campbell

On Fri, May 22, 2009 at 11:07:53AM -0700, Boris Derzhavets wrote:
> 
>  It's new behavior i might be disconnected from remote Xen 3.4 Host during
> vnc session doing HVM install or just during running HVM via remote vnc
> connection. To restore network i always have to reboot Xen Host itself.
> I suspect hardware issues due to problems between cheap hub and
> Marvel Yukon PC-E Gigabit Ethernet. To scp 4 GB file i always have to replug
> network cable to PCI Ethernet ( RTL 8069SC or E8088 )
>

Could be a driver bug in the dom0 kernel. Drivers for that NIC have been
really crappy..

-- Pasi
 
> However, i attempted to switch off tso at HVM DomU:-
> 
> [root@FHVM ethtool-6]# /usr/local/sbin/ethtool -K eth1 tso off
> Cannot set device tcp segmentation offload settings: Operation not supported
> [root@FHVM ethtool-6]# /usr/local/sbin/ethtool -K eth1 tx off
> Cannot set device tx csum settings: Operation not supported
>

Try switching to PV-on-HVM drivers for much better performance than the
default emulated vNICs.. if not using already.
 
-- Pasi

> Boris.
> 
> --- On Thu, 5/21/09, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> 
> From: Jeremy Fitzhardinge <jeremy@goop.org>
> Subject: Re: [Xen-devel] [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting
> To: "Boris Derzhavets" <bderzhavets@yahoo.com>
> Cc: "Ian Campbell" <ian.campbell@citrix.com>, xen-devel@lists.xensource.com
> Date: Thursday, May 21, 2009, 1:16 PM
> 
> Boris Derzhavets wrote:
> > I've compiled 2.6.30-rc6-tip with xenfs built-in and successfully created and ran
> > Ubuntu 8.10 Server HVM and CentOS 5.3 HVM DomUs with traditional hvm-profiles.
> > Remote VNC connections to HVM DomUs might be unexpectedly frozen either broken after 5-10 minutes .
> > 
> 
> That's not new behaviour though, is it?  You reported VNC failures before.  Does disabling GSO help?
> 
>    J
> 
> 
> 
>       
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2009-05-22 18:22 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-21  8:09 [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting Boris Derzhavets
2009-05-21 17:16 ` Jeremy Fitzhardinge
  -- strict thread matches above, loose matches on Subject: below --
2009-05-22 18:07 Boris Derzhavets
2009-05-22 18:22 ` Pasi Kärkkäinen
2009-05-20 19:22 Boris Derzhavets
2009-05-20 14:44 [GIT] privcmd fixes --> working HVM Ian Campbell
2009-05-20 14:45 ` [PATCH] privcmd: MMAPBATCH: Fix error handling/reporting Ian Campbell
2009-05-20 19:33   ` Jeremy Fitzhardinge
2009-05-21  8:51     ` Ian Campbell
2009-05-21  9:18       ` Ian Campbell
2009-05-21 17:14       ` Jeremy Fitzhardinge
2009-05-20 21:13   ` Jeremy Fitzhardinge

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.