WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH 04 of 17] x86/mm/p2m: merge gfn_to_mfn_unshare with o

To: <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH 04 of 17] x86/mm/p2m: merge gfn_to_mfn_unshare with other gfn_to_mfn paths
From: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Date: Thu, 2 Jun 2011 13:20:14 +0100
Delivery-date: Thu, 02 Jun 2011 05:28:09 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <patchbomb.1307017210@xxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <patchbomb.1307017210@xxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mercurial-patchbomb/1.8.3
# HG changeset patch
# User Tim Deegan <Tim.Deegan@xxxxxxxxxx>
# Date 1307017012 -3600
# Node ID 0d3e0a571fdddc873d1ff3750d15db6fc58fff8e
# Parent  a7d8612c9ba14ae6efbf420e213c983902433942
x86/mm/p2m: merge gfn_to_mfn_unshare with other gfn_to_mfn paths.

gfn_to_mfn_unshare() had its own function despite all other lookup types
being handled in one place. Merge it into _gfn_to_mfn_type(), so that it
gets the benefit of broken-page protection, for example, and tidy its
interfaces up to fit.

The unsharing code still has a lot of bugs, e.g.
 - failure to alloc for unshare on a foreign lookup still BUG()s,
 - at least one race condition in unshare-and-retry
 - p2m_* lookup types should probably be flags, not enum
but it's cleaner and will make later p2m cleanups easier.

Signed-off-by: Tim Deegan <Tim.Deegan@xxxxxxxxxx>

diff -r a7d8612c9ba1 -r 0d3e0a571fdd xen/arch/x86/hvm/emulate.c
--- a/xen/arch/x86/hvm/emulate.c        Thu Jun 02 13:16:52 2011 +0100
+++ b/xen/arch/x86/hvm/emulate.c        Thu Jun 02 13:16:52 2011 +0100
@@ -63,7 +63,7 @@ static int hvmemul_do_io(
     int rc;
 
     /* Check for paged out page */
-    ram_mfn = gfn_to_mfn_unshare(p2m, ram_gfn, &p2mt, 0);
+    ram_mfn = gfn_to_mfn_unshare(p2m, ram_gfn, &p2mt);
     if ( p2m_is_paging(p2mt) )
     {
         p2m_mem_paging_populate(p2m, ram_gfn);
diff -r a7d8612c9ba1 -r 0d3e0a571fdd xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c    Thu Jun 02 13:16:52 2011 +0100
+++ b/xen/arch/x86/hvm/hvm.c    Thu Jun 02 13:16:52 2011 +0100
@@ -352,7 +352,7 @@ static int hvm_set_ioreq_page(
     unsigned long mfn;
     void *va;
 
-    mfn = mfn_x(gfn_to_mfn_unshare(p2m, gmfn, &p2mt, 0));
+    mfn = mfn_x(gfn_to_mfn_unshare(p2m, gmfn, &p2mt));
     if ( !p2m_is_ram(p2mt) )
         return -EINVAL;
     if ( p2m_is_paging(p2mt) )
@@ -1767,7 +1767,7 @@ static void *__hvm_map_guest_frame(unsig
     struct p2m_domain *p2m = p2m_get_hostp2m(current->domain);
 
     mfn = mfn_x(writable
-                ? gfn_to_mfn_unshare(p2m, gfn, &p2mt, 0)
+                ? gfn_to_mfn_unshare(p2m, gfn, &p2mt)
                 : gfn_to_mfn(p2m, gfn, &p2mt));
     if ( (p2m_is_shared(p2mt) && writable) || !p2m_is_ram(p2mt) )
         return NULL;
@@ -2229,7 +2229,7 @@ static enum hvm_copy_result __hvm_copy(
             gfn = addr >> PAGE_SHIFT;
         }
 
-        mfn = mfn_x(gfn_to_mfn_unshare(p2m, gfn, &p2mt, 0));
+        mfn = mfn_x(gfn_to_mfn_unshare(p2m, gfn, &p2mt));
 
         if ( p2m_is_paging(p2mt) )
         {
@@ -3724,7 +3724,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
         rc = -EINVAL;
         if ( is_hvm_domain(d) )
         {
-            gfn_to_mfn_unshare(p2m_get_hostp2m(d), a.pfn, &t, 0);
+            gfn_to_mfn_unshare(p2m_get_hostp2m(d), a.pfn, &t);
             if ( p2m_is_mmio(t) )
                 a.mem_type =  HVMMEM_mmio_dm;
             else if ( p2m_is_readonly(t) )
@@ -3779,7 +3779,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
             p2m_type_t t;
             p2m_type_t nt;
             mfn_t mfn;
-            mfn = gfn_to_mfn_unshare(p2m, pfn, &t, 0);
+            mfn = gfn_to_mfn_unshare(p2m, pfn, &t);
             if ( p2m_is_paging(t) )
             {
                 p2m_mem_paging_populate(p2m, pfn);
@@ -3877,7 +3877,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
             mfn_t mfn;
             int success;
 
-            mfn = gfn_to_mfn_unshare(p2m, pfn, &t, 0);
+            mfn = gfn_to_mfn_unshare(p2m, pfn, &t);
 
             p2m_lock(p2m);
             success = p2m->set_entry(p2m, pfn, mfn, 0, t, 
memaccess[a.hvmmem_access]);
diff -r a7d8612c9ba1 -r 0d3e0a571fdd xen/arch/x86/mm.c
--- a/xen/arch/x86/mm.c Thu Jun 02 13:16:52 2011 +0100
+++ b/xen/arch/x86/mm.c Thu Jun 02 13:16:52 2011 +0100
@@ -4653,7 +4653,7 @@ long arch_memory_op(int op, XEN_GUEST_HA
             p2m_type_t p2mt;
 
             xatp.idx = mfn_x(gfn_to_mfn_unshare(p2m_get_hostp2m(d),
-                                                xatp.idx, &p2mt, 0));
+                                                xatp.idx, &p2mt));
             /* If the page is still shared, exit early */
             if ( p2m_is_shared(p2mt) )
             {
diff -r a7d8612c9ba1 -r 0d3e0a571fdd xen/arch/x86/mm/guest_walk.c
--- a/xen/arch/x86/mm/guest_walk.c      Thu Jun 02 13:16:52 2011 +0100
+++ b/xen/arch/x86/mm/guest_walk.c      Thu Jun 02 13:16:52 2011 +0100
@@ -93,7 +93,7 @@ static inline void *map_domain_gfn(struc
                                    uint32_t *rc) 
 {
     /* Translate the gfn, unsharing if shared */
-    *mfn = gfn_to_mfn_unshare(p2m, gfn_x(gfn), p2mt, 0);
+    *mfn = gfn_to_mfn_unshare(p2m, gfn_x(gfn), p2mt);
     if ( p2m_is_paging(*p2mt) )
     {
         p2m_mem_paging_populate(p2m, gfn_x(gfn));
diff -r a7d8612c9ba1 -r 0d3e0a571fdd xen/arch/x86/mm/hap/guest_walk.c
--- a/xen/arch/x86/mm/hap/guest_walk.c  Thu Jun 02 13:16:52 2011 +0100
+++ b/xen/arch/x86/mm/hap/guest_walk.c  Thu Jun 02 13:16:52 2011 +0100
@@ -57,7 +57,7 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PA
     walk_t gw;
 
     /* Get the top-level table's MFN */
-    top_mfn = gfn_to_mfn_unshare(p2m, cr3 >> PAGE_SHIFT, &p2mt, 0);
+    top_mfn = gfn_to_mfn_unshare(p2m, cr3 >> PAGE_SHIFT, &p2mt);
     if ( p2m_is_paging(p2mt) )
     {
         p2m_mem_paging_populate(p2m, cr3 >> PAGE_SHIFT);
@@ -89,7 +89,7 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PA
     if ( missing == 0 )
     {
         gfn_t gfn = guest_l1e_get_gfn(gw.l1e);
-        gfn_to_mfn_unshare(p2m, gfn_x(gfn), &p2mt, 0);
+        gfn_to_mfn_unshare(p2m, gfn_x(gfn), &p2mt);
         if ( p2m_is_paging(p2mt) )
         {
             p2m_mem_paging_populate(p2m, gfn_x(gfn));
diff -r a7d8612c9ba1 -r 0d3e0a571fdd xen/arch/x86/mm/mem_sharing.c
--- a/xen/arch/x86/mm/mem_sharing.c     Thu Jun 02 13:16:52 2011 +0100
+++ b/xen/arch/x86/mm/mem_sharing.c     Thu Jun 02 13:16:52 2011 +0100
@@ -294,8 +294,7 @@ static void mem_sharing_audit(void)
 
 
 static struct page_info* mem_sharing_alloc_page(struct domain *d, 
-                                                unsigned long gfn,
-                                                int must_succeed)
+                                                unsigned long gfn)
 {
     struct page_info* page;
     struct vcpu *v = current;
@@ -307,21 +306,20 @@ static struct page_info* mem_sharing_all
     memset(&req, 0, sizeof(req));
     req.type = MEM_EVENT_TYPE_SHARED;
 
-    if(must_succeed) 
+    if ( v->domain != d )
     {
-        /* We do not support 'must_succeed' any more. External operations such
-         * as grant table mappings may fail with OOM condition! 
-         */
-        BUG();
+        /* XXX This path needs some attention.  For now, just fail foreign 
+         * XXX requests to unshare if there's no memory.  This replaces 
+         * XXX old code that BUG()ed here; the callers now BUG()
+         * XXX elewhere. */
+        gdprintk(XENLOG_ERR, 
+                 "Failed alloc on unshare path for foreign (%d) lookup\n",
+                 d->domain_id);
+        return page;
     }
-    else
-    {
-        /* All foreign attempts to unshare pages should be handled through
-         * 'must_succeed' case. */
-        ASSERT(v->domain->domain_id == d->domain_id);
-        vcpu_pause_nosync(v);
-        req.flags |= MEM_EVENT_FLAG_VCPU_PAUSED;
-    }
+
+    vcpu_pause_nosync(v);
+    req.flags |= MEM_EVENT_FLAG_VCPU_PAUSED;
 
     /* XXX: Need to reserve a request, not just check the ring! */
     if(mem_event_check_ring(d)) return page;
@@ -692,8 +690,7 @@ gfn_found:
     if(ret == 0) goto private_page_found;
         
     old_page = page;
-    page = mem_sharing_alloc_page(d, gfn, flags & MEM_SHARING_MUST_SUCCEED);
-    BUG_ON(!page && (flags & MEM_SHARING_MUST_SUCCEED));
+    page = mem_sharing_alloc_page(d, gfn);
     if(!page) 
     {
         /* We've failed to obtain memory for private page. Need to re-add the
diff -r a7d8612c9ba1 -r 0d3e0a571fdd xen/common/grant_table.c
--- a/xen/common/grant_table.c  Thu Jun 02 13:16:52 2011 +0100
+++ b/xen/common/grant_table.c  Thu Jun 02 13:16:52 2011 +0100
@@ -110,7 +110,8 @@ static unsigned inline int max_nr_maptra
 #define gfn_to_mfn_private(_d, _gfn) ({                     \
     p2m_type_t __p2mt;                                      \
     unsigned long __x;                                      \
-    __x = mfn_x(gfn_to_mfn_unshare(p2m_get_hostp2m(_d), _gfn, &__p2mt, 1));  \
+    __x = mfn_x(gfn_to_mfn_unshare(p2m_get_hostp2m(_d), _gfn, &__p2mt));  \
+    BUG_ON(p2m_is_shared(__p2mt)); /* XXX fixme */          \
     if ( !p2m_is_valid(__p2mt) )                            \
         __x = INVALID_MFN;                                  \
     __x; })
@@ -153,7 +154,12 @@ static int __get_paged_frame(unsigned lo
     if ( readonly )
         mfn = gfn_to_mfn(p2m, gfn, &p2mt);
     else
-        mfn = gfn_to_mfn_unshare(p2m, gfn, &p2mt, 1);
+    {
+        mfn = gfn_to_mfn_unshare(p2m, gfn, &p2mt);
+        BUG_ON(p2m_is_shared(p2mt));
+        /* XXX Here, and above in gfn_to_mfn_private, need to handle
+         * XXX failure to unshare. */
+    }
 
     if ( p2m_is_valid(p2mt) ) {
         *frame = mfn_x(mfn);
diff -r a7d8612c9ba1 -r 0d3e0a571fdd xen/common/memory.c
--- a/xen/common/memory.c       Thu Jun 02 13:16:52 2011 +0100
+++ b/xen/common/memory.c       Thu Jun 02 13:16:52 2011 +0100
@@ -363,7 +363,7 @@ static long memory_exchange(XEN_GUEST_HA
                 p2m_type_t p2mt;
 
                 /* Shared pages cannot be exchanged */
-                mfn = mfn_x(gfn_to_mfn_unshare(p2m_get_hostp2m(d), gmfn + k, 
&p2mt, 0));
+                mfn = mfn_x(gfn_to_mfn_unshare(p2m_get_hostp2m(d), gmfn + k, 
&p2mt));
                 if ( p2m_is_shared(p2mt) )
                 {
                     rc = -ENOMEM;
diff -r a7d8612c9ba1 -r 0d3e0a571fdd xen/include/asm-x86/mem_sharing.h
--- a/xen/include/asm-x86/mem_sharing.h Thu Jun 02 13:16:52 2011 +0100
+++ b/xen/include/asm-x86/mem_sharing.h Thu Jun 02 13:16:52 2011 +0100
@@ -34,7 +34,6 @@ int mem_sharing_nominate_page(struct p2m
                               unsigned long gfn,
                               int expected_refcnt,
                               shr_handle_t *phandle);
-#define MEM_SHARING_MUST_SUCCEED      (1<<0)
 #define MEM_SHARING_DESTROY_GFN       (1<<1)
 int mem_sharing_unshare_page(struct p2m_domain *p2m, 
                              unsigned long gfn, 
diff -r a7d8612c9ba1 -r 0d3e0a571fdd xen/include/asm-x86/p2m.h
--- a/xen/include/asm-x86/p2m.h Thu Jun 02 13:16:52 2011 +0100
+++ b/xen/include/asm-x86/p2m.h Thu Jun 02 13:16:52 2011 +0100
@@ -112,9 +112,10 @@ typedef enum {
 } p2m_access_t;
 
 typedef enum {
-    p2m_query = 0,              /* Do not populate a PoD entries      */
-    p2m_alloc = 1,              /* Automatically populate PoD entries */
-    p2m_guest = 2,              /* Guest demand-fault; implies alloc  */
+    p2m_query,              /* Do not populate a PoD entries      */
+    p2m_alloc,              /* Automatically populate PoD entries */
+    p2m_unshare,            /* Break c-o-w sharing; implies alloc */
+    p2m_guest,              /* Guest demand-fault; implies alloc  */
 } p2m_query_t;
 
 /* We use bitmaps and maks to handle groups of types */
@@ -367,6 +368,14 @@ gfn_to_mfn_type_p2m(struct p2m_domain *p
     mfn_t mfn = p2m->get_entry(p2m, gfn, t, a, q);
 
 #ifdef __x86_64__
+    if ( q == p2m_unshare && p2m_is_shared(*t) )
+    {
+        mem_sharing_unshare_page(p2m, gfn, 0);
+        mfn = p2m->get_entry(p2m, gfn, t, a, q);
+    }
+#endif
+
+#ifdef __x86_64__
     if (unlikely((p2m_is_broken(*t))))
     {
         /* Return invalid_mfn to avoid caller's access */
@@ -401,32 +410,7 @@ static inline mfn_t _gfn_to_mfn_type(str
 #define gfn_to_mfn(p2m, g, t) _gfn_to_mfn_type((p2m), (g), (t), p2m_alloc)
 #define gfn_to_mfn_query(p2m, g, t) _gfn_to_mfn_type((p2m), (g), (t), 
p2m_query)
 #define gfn_to_mfn_guest(p2m, g, t) _gfn_to_mfn_type((p2m), (g), (t), 
p2m_guest)
-
-static inline mfn_t gfn_to_mfn_unshare(struct p2m_domain *p2m,
-                                       unsigned long gfn,
-                                       p2m_type_t *p2mt,
-                                       int must_succeed)
-{
-    mfn_t mfn;
-
-    mfn = gfn_to_mfn(p2m, gfn, p2mt);
-#ifdef __x86_64__
-    if ( p2m_is_shared(*p2mt) )
-    {
-        if ( mem_sharing_unshare_page(p2m, gfn,
-                                      must_succeed 
-                                      ? MEM_SHARING_MUST_SUCCEED : 0) )
-        {
-            BUG_ON(must_succeed);
-            return mfn;
-        }
-        mfn = gfn_to_mfn(p2m, gfn, p2mt);
-    }
-#endif
-
-    return mfn;
-}
-
+#define gfn_to_mfn_unshare(p, g, t) _gfn_to_mfn_type((p), (g), (t), 
p2m_unshare)
 
 /* Compatibility function exporting the old untyped interface */
 static inline unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn)

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