All of lore.kernel.org
 help / color / mirror / Atom feed
* Some problems about xenpaging
@ 2011-09-23  9:35 Hongkaixing
  2011-09-23 14:34 ` Olaf Hering
  2011-09-23 15:59 ` George Dunlap
  0 siblings, 2 replies; 3+ messages in thread
From: Hongkaixing @ 2011-09-23  9:35 UTC (permalink / raw)
  To: olaf; +Cc: YangXiaowei, , , Eric Li(Zhentao), Yanqiangjun

Hi, Olaf

we have tested the xenpaging feature and found some problems.
(1) the test case like this : when we start a VM with POD enable, the xenpaging is started at the same time.
    this case will cause many problems ,finally, we fixed the BUG, the patch is attached below.

(2) there is a very serious problem. we have observed many VM crash examples, the error code is not always the same. 
     we guess there exists conflict between xenpaging and memory mapping. 
     for instance, if the Dom0 map the DomU's memory to its own space , then the memory of DomainU is paged out, when the Domain0 access this memory area, a panic is caused.
     And I really do not know whether the qemu device can perceive the memory modified by xenpaging? 

    thanks!

here is the patch to solve the pod problem
1) fix the p2m_pod_decrease_reservation() function, take care of the paging type
--- ./origin/xen/arch/x86/mm/p2m.c      2011-09-05 20:39:30.000000000 +0800
+++ ./b/xen/arch/x86/mm/p2m.c   2011-09-23 23:46:19.000000000 +0800
@@ -675,6 +675,23 @@
             BUG_ON(p2md->pod.entry_count < 0);
             pod--;
         }
+        else if ( steal_for_cache && p2m_is_paging(t) )
+        {
+            struct page_info *page;
+            /* alloc a new page to compensate the pod list */
+            page = alloc_domheap_page(d, 0);
+            if ( unlikely(page == NULL) )
+            {
+                goto out_entry_check;
+            }
+            set_p2m_entry(d, gpfn + i, _mfn(INVALID_MFN), 0, p2m_invalid);
+            p2m_mem_paging_drop_page(d, gpfn+i);
+            p2m_pod_cache_add(d, page, 0);
+            steal_for_cache =  ( p2md->pod.entry_count > p2md->pod.count );
+            nonpod--;
+            ram--;
+        }
+        /* for other ram types */
         else if ( steal_for_cache && p2m_is_ram(t) )
         {
             struct page_info *page;

2) fix the race between POD and xenpaging
   situation can be described as the follow
   
   xenpaging                  POD
  
 mfn = gfn_to_mfn()         mfn = gfn_to_mfn()
   check p2m type           check p2mt
                            p2m_lock
                            change p2m type
                            p2m_unlock
                            add to pod list
    p2m_lock()
    change p2m type
    p2m_unlock
    put_page()

the result is a page is added to the pod list,and then it's removed from the list by put_page.
my suggestion is to extend the range of the p2m lock to contain the p2m check.

in  p2m_mem_paging_nominate() function
@@ -2532,7 +2561,8 @@
     mfn_t mfn;
     int ret;
 
-    mfn = gfn_to_mfn(d, gfn, &p2mt);
+    p2m_lock(d->arch.p2m);
+    mfn = gfn_to_mfn_query(d, gfn, &p2mt);
 
     /* Check if mfn is valid */
     ret = -EINVAL;
@@ -2580,13 +2610,12 @@
         goto out;
 
     /* Fix p2m entry */
-    p2m_lock(d->arch.p2m);
     set_p2m_entry(d, gfn, mfn, 0, p2m_ram_paging_out);
-    p2m_unlock(d->arch.p2m);
 
     ret = 0;
 
  out:
+    p2m_unlock(d->arch.p2m);
     return ret;
 }
in 
@@ -2595,34 +2624,39 @@
     struct page_info *page;
     p2m_type_t p2mt;
     mfn_t mfn;
-
+    int ret;
+    p2m_lock(d->arch.p2m);
     /* Get mfn */
-    mfn = gfn_to_mfn(d, gfn, &p2mt);
+    mfn = gfn_to_mfn_query(d, gfn, &p2mt);
+    
+    ret = -EINVAL;
     if ( unlikely(!mfn_valid(mfn)) )
-        return -EINVAL;
+        goto out;
 
     if (p2mt != p2m_ram_paging_out)
      {
          printk("p2m_mem_paging_evict type %d\n", p2mt);
-               return -EINVAL;
+         goto out;
      }
     /* Get the page so it doesn't get modified under Xen's feet */
     page = mfn_to_page(mfn);
     if ( unlikely(!get_page(page, d)) )
-        return -EINVAL;
+        goto out;
 
     /* Decrement guest domain's ref count of the page */
     if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
         put_page(page);
 
     /* Remove mapping from p2m table */
-    p2m_lock(d->arch.p2m);
     set_p2m_entry(d, gfn, _mfn(PAGING_MFN), 0, p2m_ram_paged);
-    p2m_unlock(d->arch.p2m);
 
     /* Put the page back so it gets freed */
     put_page(page);
+    
+    ret = 0;
 
+out:
+    p2m_unlock(d->arch.p2m);
     return 0;
 }

3) fix the vmx_load_pdptrs() function in  vmx.c
in this situation the page directory table is paged out.
Although using mdelay() is a bad idea, it's better than making the xen crash  

void vmx_load_pdptrs(struct vcpu *v)
{
    unsigned long cr3 = v->arch.hvm_vcpu.guest_cr[3], mfn;
    uint64_t *guest_pdptrs;
    p2m_type_t p2mt;
    char *p;
    unsigned int try_count = 0;
    /* EPT needs to load PDPTRS into VMCS for PAE. */
    if ( !hvm_pae_enabled(v) || (v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
        return;
    if ( cr3 & 0x1fUL )
        goto crash;
    mfn = mfn_x(gfn_to_mfn(v->domain, cr3 >> PAGE_SHIFT, &p2mt));
    if ( !p2m_is_ram(p2mt) )
        goto crash;
+  if( p2m_is_paging(p2mt))
+  {
+        p2m_mem_paging_populate(v->domain, cr3 >> PAGE_SHIFT);
+        do
+        {
+             mdelay(1);
+             try_count ++;
+             if ( try_count > 65535 )
+             {
+                 goto crash;
+             }
+             mfn = mfn_x(gfn_to_mfn(v->domain, cr3 >> PAGE_SHIFT, &p2mt));
+        }while( !mfn_valid(mfn));
+    }
+
    p = map_domain_page(mfn);

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

* Re: Some problems about xenpaging
  2011-09-23  9:35 Some problems about xenpaging Hongkaixing
@ 2011-09-23 14:34 ` Olaf Hering
  2011-09-23 15:59 ` George Dunlap
  1 sibling, 0 replies; 3+ messages in thread
From: Olaf Hering @ 2011-09-23 14:34 UTC (permalink / raw)
  To: Hongkaixing, Tim Deegan, George Dunlap
  Cc: YangXiaowei, xen-devel, Eric Li(Zhentao), Yanqiangjun

On Fri, Sep 23, Hongkaixing wrote:

> Hi, Olaf

Hello Hongkaixing,

thanks for the feedback and the patches!

> we have tested the xenpaging feature and found some problems.
> (1) the test case like this : when we start a VM with POD enable, the xenpaging is started at the same time.
>     this case will cause many problems ,finally, we fixed the BUG, the patch is attached below.

The changes for p2m_pod_decrease_reservation() and
p2m_mem_paging_nominate() look ok to me, but I will leave the final word
to Tim and George.

If I understand the current logic in p2m_mem_paging_nominate()
correctly, if a gfn is p2m_populate_on_demand, then gfn_to_mfn() will
turn the gfn into p2m_ram_rw and let xenpaging write it to disk. That
does not look correct. With your change the gfn will not be nominated
because its not backed by RAM anyway.

The locking in the p2m_mem_paging_nominate(), p2m_mem_paging_evict() and
p2m_mem_paging_populate() can be improved. Using gfn_to_mfn_query()
inside the p2m_lock looks correct to me. Even p2m_mem_paging_resume()
could use the query variant and forward the type to p2m_ram_rw only if
the p2mt was p2m_ram_paging_in_start.

Please send proper patches for xen-unstable as described in the
MAINTAINERS file.

> (2) there is a very serious problem. we have observed many VM crash examples, the error code is not always the same. 
>      we guess there exists conflict between xenpaging and memory mapping. 
>      for instance, if the Dom0 map the DomU's memory to its own space , then the memory of DomainU is paged out, when the Domain0 access this memory area, a panic is caused.
>      And I really do not know whether the qemu device can perceive the memory modified by xenpaging? 

The issue is that gfn_to_mfn() returns the p2m_ram_paging* types and
expects the caller to retry until the gfn is accessible, instead of
calling p2m_mem_paging_populate() and going to sleep until the page is
available.
Thats still on my list of things to do.


Olaf


> here is the patch to solve the pod problem
> 1) fix the p2m_pod_decrease_reservation() function, take care of the paging type
> --- ./origin/xen/arch/x86/mm/p2m.c      2011-09-05 20:39:30.000000000 +0800
> +++ ./b/xen/arch/x86/mm/p2m.c   2011-09-23 23:46:19.000000000 +0800
> @@ -675,6 +675,23 @@
>              BUG_ON(p2md->pod.entry_count < 0);
>              pod--;
>          }
> +        else if ( steal_for_cache && p2m_is_paging(t) )
> +        {
> +            struct page_info *page;
> +            /* alloc a new page to compensate the pod list */
> +            page = alloc_domheap_page(d, 0);
> +            if ( unlikely(page == NULL) )
> +            {
> +                goto out_entry_check;
> +            }
> +            set_p2m_entry(d, gpfn + i, _mfn(INVALID_MFN), 0, p2m_invalid);
> +            p2m_mem_paging_drop_page(d, gpfn+i);
> +            p2m_pod_cache_add(d, page, 0);
> +            steal_for_cache =  ( p2md->pod.entry_count > p2md->pod.count );
> +            nonpod--;
> +            ram--;
> +        }
> +        /* for other ram types */
>          else if ( steal_for_cache && p2m_is_ram(t) )
>          {
>              struct page_info *page;
> 
> 2) fix the race between POD and xenpaging
>    situation can be described as the follow
>    
>    xenpaging                  POD
>   
>  mfn = gfn_to_mfn()         mfn = gfn_to_mfn()
>    check p2m type           check p2mt
>                             p2m_lock
>                             change p2m type
>                             p2m_unlock
>                             add to pod list
>     p2m_lock()
>     change p2m type
>     p2m_unlock
>     put_page()
> 
> the result is a page is added to the pod list,and then it's removed from the list by put_page.
> my suggestion is to extend the range of the p2m lock to contain the p2m check.
> 
> in  p2m_mem_paging_nominate() function
> @@ -2532,7 +2561,8 @@
>      mfn_t mfn;
>      int ret;
>  
> -    mfn = gfn_to_mfn(d, gfn, &p2mt);
> +    p2m_lock(d->arch.p2m);
> +    mfn = gfn_to_mfn_query(d, gfn, &p2mt);
>  
>      /* Check if mfn is valid */
>      ret = -EINVAL;
> @@ -2580,13 +2610,12 @@
>          goto out;
>  
>      /* Fix p2m entry */
> -    p2m_lock(d->arch.p2m);
>      set_p2m_entry(d, gfn, mfn, 0, p2m_ram_paging_out);
> -    p2m_unlock(d->arch.p2m);
>  
>      ret = 0;
>  
>   out:
> +    p2m_unlock(d->arch.p2m);
>      return ret;
>  }
> in 
> @@ -2595,34 +2624,39 @@
>      struct page_info *page;
>      p2m_type_t p2mt;
>      mfn_t mfn;
> -
> +    int ret;
> +    p2m_lock(d->arch.p2m);
>      /* Get mfn */
> -    mfn = gfn_to_mfn(d, gfn, &p2mt);
> +    mfn = gfn_to_mfn_query(d, gfn, &p2mt);
> +    
> +    ret = -EINVAL;
>      if ( unlikely(!mfn_valid(mfn)) )
> -        return -EINVAL;
> +        goto out;
>  
>      if (p2mt != p2m_ram_paging_out)
>       {
>           printk("p2m_mem_paging_evict type %d\n", p2mt);
> -               return -EINVAL;
> +         goto out;
>       }
>      /* Get the page so it doesn't get modified under Xen's feet */
>      page = mfn_to_page(mfn);
>      if ( unlikely(!get_page(page, d)) )
> -        return -EINVAL;
> +        goto out;
>  
>      /* Decrement guest domain's ref count of the page */
>      if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
>          put_page(page);
>  
>      /* Remove mapping from p2m table */
> -    p2m_lock(d->arch.p2m);
>      set_p2m_entry(d, gfn, _mfn(PAGING_MFN), 0, p2m_ram_paged);
> -    p2m_unlock(d->arch.p2m);
>  
>      /* Put the page back so it gets freed */
>      put_page(page);
> +    
> +    ret = 0;
>  
> +out:
> +    p2m_unlock(d->arch.p2m);
>      return 0;
>  }
> 
> 3) fix the vmx_load_pdptrs() function in  vmx.c
> in this situation the page directory table is paged out.
> Although using mdelay() is a bad idea, it's better than making the xen crash  
> 
> void vmx_load_pdptrs(struct vcpu *v)
> {
>     unsigned long cr3 = v->arch.hvm_vcpu.guest_cr[3], mfn;
>     uint64_t *guest_pdptrs;
>     p2m_type_t p2mt;
>     char *p;
>     unsigned int try_count = 0;
>     /* EPT needs to load PDPTRS into VMCS for PAE. */
>     if ( !hvm_pae_enabled(v) || (v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
>         return;
>     if ( cr3 & 0x1fUL )
>         goto crash;
>     mfn = mfn_x(gfn_to_mfn(v->domain, cr3 >> PAGE_SHIFT, &p2mt));
>     if ( !p2m_is_ram(p2mt) )
>         goto crash;
> +  if( p2m_is_paging(p2mt))
> +  {
> +        p2m_mem_paging_populate(v->domain, cr3 >> PAGE_SHIFT);
> +        do
> +        {
> +             mdelay(1);
> +             try_count ++;
> +             if ( try_count > 65535 )
> +             {
> +                 goto crash;
> +             }
> +             mfn = mfn_x(gfn_to_mfn(v->domain, cr3 >> PAGE_SHIFT, &p2mt));
> +        }while( !mfn_valid(mfn));
> +    }
> +
>     p = map_domain_page(mfn);

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

* Re: Some problems about xenpaging
  2011-09-23  9:35 Some problems about xenpaging Hongkaixing
  2011-09-23 14:34 ` Olaf Hering
@ 2011-09-23 15:59 ` George Dunlap
  1 sibling, 0 replies; 3+ messages in thread
From: George Dunlap @ 2011-09-23 15:59 UTC (permalink / raw)
  To: Hongkaixing; +Cc: YangXiaowei, olaf, , , Eric Li(Zhentao), Yanqiangjun

On Fri, Sep 23, 2011 at 10:35 AM, Hongkaixing <hongkaixing@huawei.com> wrote:
> Hi, Olaf
>
> we have tested the xenpaging feature and found some problems.
> (1) the test case like this : when we start a VM with POD enable, the xenpaging is started at the same time.
>    this case will cause many problems ,finally, we fixed the BUG, the patch is attached below.
>
> (2) there is a very serious problem. we have observed many VM crash examples, the error code is not always the same.
>     we guess there exists conflict between xenpaging and memory mapping.
>     for instance, if the Dom0 map the DomU's memory to its own space , then the memory of DomainU is paged out, when the Domain0 access this memory area, a panic is caused.
>     And I really do not know whether the qemu device can perceive the memory modified by xenpaging?
>
>    thanks!
>
> here is the patch to solve the pod problem
> 1) fix the p2m_pod_decrease_reservation() function, take care of the paging type
> --- ./origin/xen/arch/x86/mm/p2m.c      2011-09-05 20:39:30.000000000 +0800
> +++ ./b/xen/arch/x86/mm/p2m.c   2011-09-23 23:46:19.000000000 +0800

Kaixing,

Xie xie ni dui wo men shuo zhe xie shi.  (Dui bu qi wo de zhong wen bu
hao.)  As Olaf says, it would help us a lot if you could submit the
patches in a form that is easier for us to apply and to comment on:
namely, either having all the patches in separate files as
attachments, or having one e-mail per patch, with the patch being the
last thing in the e-mail.

There are some suggestions on how to post patches here:
http://wiki.xensource.com/xenwiki/SubmittingXenPatches.  My preferred
technique is to make a patch series using Mercurial Queues, and then
use the PatchBomb extension (mentioned in the link above) to mail them
as a series, by using "hg email -o"

 -George

> @@ -675,6 +675,23 @@
>             BUG_ON(p2md->pod.entry_count < 0);
>             pod--;
>         }
> +        else if ( steal_for_cache && p2m_is_paging(t) )
> +        {
> +            struct page_info *page;
> +            /* alloc a new page to compensate the pod list */
> +            page = alloc_domheap_page(d, 0);
> +            if ( unlikely(page == NULL) )
> +            {
> +                goto out_entry_check;
> +            }
> +            set_p2m_entry(d, gpfn + i, _mfn(INVALID_MFN), 0, p2m_invalid);
> +            p2m_mem_paging_drop_page(d, gpfn+i);
> +            p2m_pod_cache_add(d, page, 0);
> +            steal_for_cache =  ( p2md->pod.entry_count > p2md->pod.count );
> +            nonpod--;
> +            ram--;
> +        }
> +        /* for other ram types */
>         else if ( steal_for_cache && p2m_is_ram(t) )
>         {
>             struct page_info *page;
>
> 2) fix the race between POD and xenpaging
>   situation can be described as the follow
>
>   xenpaging                  POD
>
>  mfn = gfn_to_mfn()         mfn = gfn_to_mfn()
>   check p2m type           check p2mt
>                            p2m_lock
>                            change p2m type
>                            p2m_unlock
>                            add to pod list
>    p2m_lock()
>    change p2m type
>    p2m_unlock
>    put_page()
>
> the result is a page is added to the pod list,and then it's removed from the list by put_page.
> my suggestion is to extend the range of the p2m lock to contain the p2m check.
>
> in  p2m_mem_paging_nominate() function
> @@ -2532,7 +2561,8 @@
>     mfn_t mfn;
>     int ret;
>
> -    mfn = gfn_to_mfn(d, gfn, &p2mt);
> +    p2m_lock(d->arch.p2m);
> +    mfn = gfn_to_mfn_query(d, gfn, &p2mt);
>
>     /* Check if mfn is valid */
>     ret = -EINVAL;
> @@ -2580,13 +2610,12 @@
>         goto out;
>
>     /* Fix p2m entry */
> -    p2m_lock(d->arch.p2m);
>     set_p2m_entry(d, gfn, mfn, 0, p2m_ram_paging_out);
> -    p2m_unlock(d->arch.p2m);
>
>     ret = 0;
>
>  out:
> +    p2m_unlock(d->arch.p2m);
>     return ret;
>  }
> in
> @@ -2595,34 +2624,39 @@
>     struct page_info *page;
>     p2m_type_t p2mt;
>     mfn_t mfn;
> -
> +    int ret;
> +    p2m_lock(d->arch.p2m);
>     /* Get mfn */
> -    mfn = gfn_to_mfn(d, gfn, &p2mt);
> +    mfn = gfn_to_mfn_query(d, gfn, &p2mt);
> +
> +    ret = -EINVAL;
>     if ( unlikely(!mfn_valid(mfn)) )
> -        return -EINVAL;
> +        goto out;
>
>     if (p2mt != p2m_ram_paging_out)
>      {
>          printk("p2m_mem_paging_evict type %d\n", p2mt);
> -               return -EINVAL;
> +         goto out;
>      }
>     /* Get the page so it doesn't get modified under Xen's feet */
>     page = mfn_to_page(mfn);
>     if ( unlikely(!get_page(page, d)) )
> -        return -EINVAL;
> +        goto out;
>
>     /* Decrement guest domain's ref count of the page */
>     if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
>         put_page(page);
>
>     /* Remove mapping from p2m table */
> -    p2m_lock(d->arch.p2m);
>     set_p2m_entry(d, gfn, _mfn(PAGING_MFN), 0, p2m_ram_paged);
> -    p2m_unlock(d->arch.p2m);
>
>     /* Put the page back so it gets freed */
>     put_page(page);
> +
> +    ret = 0;
>
> +out:
> +    p2m_unlock(d->arch.p2m);
>     return 0;
>  }
>
> 3) fix the vmx_load_pdptrs() function in  vmx.c
> in this situation the page directory table is paged out.
> Although using mdelay() is a bad idea, it's better than making the xen crash
>
> void vmx_load_pdptrs(struct vcpu *v)
> {
>    unsigned long cr3 = v->arch.hvm_vcpu.guest_cr[3], mfn;
>    uint64_t *guest_pdptrs;
>    p2m_type_t p2mt;
>    char *p;
>    unsigned int try_count = 0;
>    /* EPT needs to load PDPTRS into VMCS for PAE. */
>    if ( !hvm_pae_enabled(v) || (v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
>        return;
>    if ( cr3 & 0x1fUL )
>        goto crash;
>    mfn = mfn_x(gfn_to_mfn(v->domain, cr3 >> PAGE_SHIFT, &p2mt));
>    if ( !p2m_is_ram(p2mt) )
>        goto crash;
> +  if( p2m_is_paging(p2mt))
> +  {
> +        p2m_mem_paging_populate(v->domain, cr3 >> PAGE_SHIFT);
> +        do
> +        {
> +             mdelay(1);
> +             try_count ++;
> +             if ( try_count > 65535 )
> +             {
> +                 goto crash;
> +             }
> +             mfn = mfn_x(gfn_to_mfn(v->domain, cr3 >> PAGE_SHIFT, &p2mt));
> +        }while( !mfn_valid(mfn));
> +    }
> +
>    p = map_domain_page(mfn);
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

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

end of thread, other threads:[~2011-09-23 15:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-23  9:35 Some problems about xenpaging Hongkaixing
2011-09-23 14:34 ` Olaf Hering
2011-09-23 15:59 ` George Dunlap

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.