[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH] [RFC] x86/domctl: Fix getpageframeinfo* handling.



In tree, there is one single caller of XEN_DOMCTL_getpageframeinfo3
(xc_get_pfn_type_batch()), and no callers of the older variants.

getpageframeinfo3 and getpageframeinfo2 are compatible if the parameter
contents are considered to be unsigned long; a compat guest calling
getpageframeinfo3 falls through into the getpageframeinfo2 handler.

However, getpageframeinfo3 and getpageframeinfo2 have different algorithms for
calculating the eventual frame type, which means that a toolstack will get
different answers depending on whether it is compat or not, which is a problem
for all possible uses.

Rewrite getpageframeinfo3 such that the code block can handle both regular and
compat guests, and use the original getpageframeinfo3 algorithm for frame
time, which is more complete.

Remove getpageframeinfo2 and getpageframeinfo1, as they are unused and
obsolete.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Keir Fraser <keir@xxxxxxx>
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>

---
This is RFC because, while I like the consolidation of the compat and
non-compat codepaths, I am not totally happy with encouraging the general use
of raw_copy_to/from_guest().

Another option could be to introduce copy_ulong_to/from_guest(), but that does
not play nicely with the guest handle typing.
---
 xen/arch/x86/domctl.c               |  306 +++++++++--------------------------
 xen/include/public/domctl.h         |   27 +---
 xen/xsm/flask/hooks.c               |    2 -
 xen/xsm/flask/policy/access_vectors |    2 +-
 4 files changed, 78 insertions(+), 259 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 20cdccb..5c62849 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -91,237 +91,6 @@ long arch_do_domctl(
         break;
     }
 
-    case XEN_DOMCTL_getpageframeinfo:
-    {
-        struct page_info *page;
-        unsigned long mfn = domctl->u.getpageframeinfo.gmfn;
-
-        ret = -EINVAL;
-        if ( unlikely(!mfn_valid(mfn)) )
-            break;
-
-        page = mfn_to_page(mfn);
-
-        if ( likely(get_page(page, d)) )
-        {
-            ret = 0;
-
-            domctl->u.getpageframeinfo.type = XEN_DOMCTL_PFINFO_NOTAB;
-
-            if ( (page->u.inuse.type_info & PGT_count_mask) != 0 )
-            {
-                switch ( page->u.inuse.type_info & PGT_type_mask )
-                {
-                case PGT_l1_page_table:
-                    domctl->u.getpageframeinfo.type = XEN_DOMCTL_PFINFO_L1TAB;
-                    break;
-                case PGT_l2_page_table:
-                    domctl->u.getpageframeinfo.type = XEN_DOMCTL_PFINFO_L2TAB;
-                    break;
-                case PGT_l3_page_table:
-                    domctl->u.getpageframeinfo.type = XEN_DOMCTL_PFINFO_L3TAB;
-                    break;
-                case PGT_l4_page_table:
-                    domctl->u.getpageframeinfo.type = XEN_DOMCTL_PFINFO_L4TAB;
-                    break;
-                }
-            }
-
-            put_page(page);
-        }
-
-        copyback = 1;
-        break;
-    }
-
-    case XEN_DOMCTL_getpageframeinfo3:
-        if ( !has_32bit_shinfo(currd) )
-        {
-            unsigned int n, j;
-            unsigned int num = domctl->u.getpageframeinfo3.num;
-            struct page_info *page;
-            xen_pfn_t *arr;
-
-            if ( unlikely(num > 1024) ||
-                 unlikely(num != domctl->u.getpageframeinfo3.num) )
-            {
-                ret = -E2BIG;
-                break;
-            }
-
-            page = alloc_domheap_page(currd, MEMF_no_owner);
-            if ( !page )
-            {
-                ret = -ENOMEM;
-                break;
-            }
-            arr = __map_domain_page(page);
-
-            for ( n = ret = 0; n < num; )
-            {
-                unsigned int k = min_t(unsigned int, num - n,
-                                       PAGE_SIZE / sizeof(*arr));
-
-                if ( copy_from_guest_offset(arr,
-                                            domctl->u.getpageframeinfo3.array,
-                                            n, k) )
-                {
-                    ret = -EFAULT;
-                    break;
-                }
-
-                for ( j = 0; j < k; j++ )
-                {
-                    unsigned long type = 0;
-                    p2m_type_t t;
-
-                    page = get_page_from_gfn(d, arr[j], &t, P2M_ALLOC);
-
-                    if ( unlikely(!page) ||
-                         unlikely(is_xen_heap_page(page)) )
-                    {
-                        if ( p2m_is_broken(t) )
-                            type = XEN_DOMCTL_PFINFO_BROKEN;
-                        else
-                            type = XEN_DOMCTL_PFINFO_XTAB;
-                    }
-                    else
-                    {
-                        switch( page->u.inuse.type_info & PGT_type_mask )
-                        {
-                        case PGT_l1_page_table:
-                            type = XEN_DOMCTL_PFINFO_L1TAB;
-                            break;
-                        case PGT_l2_page_table:
-                            type = XEN_DOMCTL_PFINFO_L2TAB;
-                            break;
-                        case PGT_l3_page_table:
-                            type = XEN_DOMCTL_PFINFO_L3TAB;
-                            break;
-                        case PGT_l4_page_table:
-                            type = XEN_DOMCTL_PFINFO_L4TAB;
-                            break;
-                        }
-
-                        if ( page->u.inuse.type_info & PGT_pinned )
-                            type |= XEN_DOMCTL_PFINFO_LPINTAB;
-
-                        if ( page->count_info & PGC_broken )
-                            type = XEN_DOMCTL_PFINFO_BROKEN;
-                    }
-
-                    if ( page )
-                        put_page(page);
-                    arr[j] = type;
-                }
-
-                if ( copy_to_guest_offset(domctl->u.getpageframeinfo3.array,
-                                          n, arr, k) )
-                {
-                    ret = -EFAULT;
-                    break;
-                }
-
-                n += k;
-            }
-
-            page = mfn_to_page(domain_page_map_to_mfn(arr));
-            unmap_domain_page(arr);
-            free_domheap_page(page);
-
-            break;
-        }
-        /* fall thru */
-    case XEN_DOMCTL_getpageframeinfo2:
-    {
-        int n,j;
-        int num = domctl->u.getpageframeinfo2.num;
-        uint32_t *arr32;
-
-        if ( unlikely(num > 1024) )
-        {
-            ret = -E2BIG;
-            break;
-        }
-
-        arr32 = alloc_xenheap_page();
-        if ( !arr32 )
-        {
-            ret = -ENOMEM;
-            break;
-        }
-
-        ret = 0;
-        for ( n = 0; n < num; )
-        {
-            int k = PAGE_SIZE / 4;
-            if ( (num - n) < k )
-                k = num - n;
-
-            if ( copy_from_guest_offset(arr32,
-                                        domctl->u.getpageframeinfo2.array,
-                                        n, k) )
-            {
-                ret = -EFAULT;
-                break;
-            }
-
-            for ( j = 0; j < k; j++ )
-            {
-                struct page_info *page;
-                unsigned long gfn = arr32[j];
-
-                page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
-
-                if ( domctl->cmd == XEN_DOMCTL_getpageframeinfo3)
-                    arr32[j] = 0;
-
-                if ( unlikely(!page) ||
-                     unlikely(is_xen_heap_page(page)) )
-                    arr32[j] |= XEN_DOMCTL_PFINFO_XTAB;
-                else
-                {
-                    unsigned long type = 0;
-
-                    switch( page->u.inuse.type_info & PGT_type_mask )
-                    {
-                    case PGT_l1_page_table:
-                        type = XEN_DOMCTL_PFINFO_L1TAB;
-                        break;
-                    case PGT_l2_page_table:
-                        type = XEN_DOMCTL_PFINFO_L2TAB;
-                        break;
-                    case PGT_l3_page_table:
-                        type = XEN_DOMCTL_PFINFO_L3TAB;
-                        break;
-                    case PGT_l4_page_table:
-                        type = XEN_DOMCTL_PFINFO_L4TAB;
-                        break;
-                    }
-
-                    if ( page->u.inuse.type_info & PGT_pinned )
-                        type |= XEN_DOMCTL_PFINFO_LPINTAB;
-                    arr32[j] |= type;
-                }
-
-                if ( page )
-                    put_page(page);
-            }
-
-            if ( copy_to_guest_offset(domctl->u.getpageframeinfo2.array,
-                                      n, arr32, k) )
-            {
-                ret = -EFAULT;
-                break;
-            }
-
-            n += k;
-        }
-
-        free_xenheap_page(arr32);
-        break;
-    }
-
     case XEN_DOMCTL_getmemlist:
     {
         unsigned long max_pfns = domctl->u.getmemlist.max_pfns;
@@ -378,6 +147,81 @@ long arch_do_domctl(
         break;
     }
 
+    case XEN_DOMCTL_getpageframeinfo3:
+    {
+        unsigned int num = domctl->u.getpageframeinfo3.num;
+
+        /* Games to allow this code block to handle a compat guest. */
+        void * __user guest_handle = domctl->u.getpageframeinfo3.array.p;
+        unsigned int width = has_32bit_shinfo(currd) ? 32 : 64;
+
+        if ( unlikely(num > 1024) ||
+             unlikely(num != domctl->u.getpageframeinfo3.num) )
+        {
+            ret = -E2BIG;
+            break;
+        }
+
+        for ( i = 0; i < num; ++i )
+        {
+            unsigned long gfn = 0, type = 0;
+            struct page_info *page;
+            p2m_type_t t;
+
+            if ( raw_copy_from_guest(&gfn, guest_handle + (i * width), width) )
+            {
+                ret = -EFAULT;
+                break;
+            }
+
+            page = get_page_from_gfn(d, gfn, &t, P2M_ALLOC);
+
+            if ( unlikely(!page) ||
+                 unlikely(is_xen_heap_page(page)) )
+            {
+                if ( p2m_is_broken(t) )
+                    type = XEN_DOMCTL_PFINFO_BROKEN;
+                else
+                    type = XEN_DOMCTL_PFINFO_XTAB;
+            }
+            else
+            {
+                switch( page->u.inuse.type_info & PGT_type_mask )
+                {
+                case PGT_l1_page_table:
+                    type = XEN_DOMCTL_PFINFO_L1TAB;
+                    break;
+                case PGT_l2_page_table:
+                    type = XEN_DOMCTL_PFINFO_L2TAB;
+                    break;
+                case PGT_l3_page_table:
+                    type = XEN_DOMCTL_PFINFO_L3TAB;
+                    break;
+                case PGT_l4_page_table:
+                    type = XEN_DOMCTL_PFINFO_L4TAB;
+                    break;
+                }
+
+                if ( page->u.inuse.type_info & PGT_pinned )
+                    type |= XEN_DOMCTL_PFINFO_LPINTAB;
+
+                if ( page->count_info & PGC_broken )
+                    type = XEN_DOMCTL_PFINFO_BROKEN;
+            }
+
+            if ( page )
+                put_page(page);
+
+            if ( __raw_copy_to_guest(guest_handle + (i * width), &type, width) 
)
+            {
+                ret = -EFAULT;
+                break;
+            }
+        }
+
+        break;
+    }
+
     case XEN_DOMCTL_hypercall_init:
     {
         unsigned long gmfn = domctl->u.hypercall_init.gmfn;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 0c0ea4a..5d15f2a 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -149,27 +149,6 @@ struct xen_domctl_getmemlist {
 #define XEN_DOMCTL_PFINFO_BROKEN  (0xdU<<28) /* broken page */
 #define XEN_DOMCTL_PFINFO_LTAB_MASK (0xfU<<28)
 
-struct xen_domctl_getpageframeinfo {
-    /* IN variables. */
-    uint64_aligned_t gmfn; /* GMFN to query */
-    /* OUT variables. */
-    /* Is the page PINNED to a type? */
-    uint32_t type;         /* see above type defs */
-};
-typedef struct xen_domctl_getpageframeinfo xen_domctl_getpageframeinfo_t;
-DEFINE_XEN_GUEST_HANDLE(xen_domctl_getpageframeinfo_t);
-
-
-/* XEN_DOMCTL_getpageframeinfo2 */
-struct xen_domctl_getpageframeinfo2 {
-    /* IN variables. */
-    uint64_aligned_t num;
-    /* IN/OUT variables. */
-    XEN_GUEST_HANDLE_64(uint32) array;
-};
-typedef struct xen_domctl_getpageframeinfo2 xen_domctl_getpageframeinfo2_t;
-DEFINE_XEN_GUEST_HANDLE(xen_domctl_getpageframeinfo2_t);
-
 /* XEN_DOMCTL_getpageframeinfo3 */
 struct xen_domctl_getpageframeinfo3 {
     /* IN variables. */
@@ -1073,8 +1052,8 @@ struct xen_domctl {
 #define XEN_DOMCTL_unpausedomain                  4
 #define XEN_DOMCTL_getdomaininfo                  5
 #define XEN_DOMCTL_getmemlist                     6
-#define XEN_DOMCTL_getpageframeinfo               7
-#define XEN_DOMCTL_getpageframeinfo2              8
+/* #define XEN_DOMCTL_getpageframeinfo            7 Obsolete - use 
getpageframeinfo3 */
+/* #define XEN_DOMCTL_getpageframeinfo2           8 Obsolete - use 
getpageframeinfo3 */
 #define XEN_DOMCTL_setvcpuaffinity                9
 #define XEN_DOMCTL_shadow_op                     10
 #define XEN_DOMCTL_max_mem                       11
@@ -1150,8 +1129,6 @@ struct xen_domctl {
         struct xen_domctl_createdomain      createdomain;
         struct xen_domctl_getdomaininfo     getdomaininfo;
         struct xen_domctl_getmemlist        getmemlist;
-        struct xen_domctl_getpageframeinfo  getpageframeinfo;
-        struct xen_domctl_getpageframeinfo2 getpageframeinfo2;
         struct xen_domctl_getpageframeinfo3 getpageframeinfo3;
         struct xen_domctl_nodeaffinity      nodeaffinity;
         struct xen_domctl_vcpuaffinity      vcpuaffinity;
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 11b7453..6e37d29 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -644,8 +644,6 @@ static int flask_domctl(struct domain *d, int cmd)
     case XEN_DOMCTL_setdebugging:
         return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__SETDEBUGGING);
 
-    case XEN_DOMCTL_getpageframeinfo:
-    case XEN_DOMCTL_getpageframeinfo2:
     case XEN_DOMCTL_getpageframeinfo3:
         return current_has_perm(d, SECCLASS_MMU, MMU__PAGEINFO);
 
diff --git a/xen/xsm/flask/policy/access_vectors 
b/xen/xsm/flask/policy/access_vectors
index ea556df..68284d5 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -339,7 +339,7 @@ class mmu
 #  source = domain making the hypercall
 #  target = domain whose pages are being mapped
     map_write
-# XEN_DOMCTL_getpageframeinfo*
+# XEN_DOMCTL_getpageframeinfo3
     pageinfo
 # XEN_DOMCTL_getmemlist
     pagelist
-- 
1.7.10.4


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.