Skip to contentSkip to conversation

Update paging/sharing/access interfaces v2

PATCH In review Andres Lagar-Cavilla 5 participants 13 messages Feb 9, 2012 – Feb 10, 2012

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Update paging/sharing/access interfaces v2

i(Was switch from domctl to memops)
Changes from v1 posted Feb 2nd 2012

- Patches 1 & 2 Acked-by Tim Deegan on the hypervisor side
- Added patch 3 to clean up the enable domctl interface, based on discussion with Ian Campbell

Description from original post follows:

Per page operations in the paging, sharing, and access tracking subsystems are all implemented with domctls (e.g. a domctl to evict one page, or to share one page).

Under heavy load, the domctl path reveals a lack of scalability. The domctl lock serializes dom0's vcpus in the hypervisor. When performing thousands of per-page operations on dozens of domains, these vcpus will spin in the hypervisor. Beyond the aggressive locking, an added inefficiency of blocking vcpus in the domctl lock is that dom0 is prevented from re-scheduling any of its other work-starved processes.

We retain the domctl interface for setting up and tearing down paging/sharing/mem access for a domain. But we migrate all the per page operations to use the memory_op hypercalls (e.g XENMEM_*).

This is a backwards-incompatible ABI change. It's been floating on the list for a couple weeks now, with no nacks thus far.

 tools/libxc/xc_mem_access.c          |   12 +-
 tools/libxc/xc_mem_event.c           |   23 +++-
 tools/libxc/xc_mem_paging.c          |   44 ++++----
 tools/libxc/xc_memshr.c              |  182 ++++++++++++++++------------------
 tools/libxc/xenctrl.h                |    6 +-
 tools/memshr/interface.c             |    4 +-
 tools/tests/mem-sharing/memshrtool.c |    4 +-
 xen/arch/x86/domctl.c                |    1 -
 xen/arch/x86/mm/mem_access.c         |    7 +-
 xen/arch/x86/mm/mem_event.c          |   68 ++++++++++--
 xen/arch/x86/mm/mem_paging.c         |   13 +-
 xen/arch/x86/mm/mem_sharing.c        |   99 +++++++++++-------
 xen/arch/x86/x86_64/compat/mm.c      |   23 ++++
 xen/arch/x86/x86_64/mm.c             |   23 ++++
 xen/include/asm-x86/mem_access.h     |    3 +-
 xen/include/asm-x86/mem_event.h      |    2 +
 xen/include/asm-x86/mem_paging.h     |    3 +-
 xen/include/asm-x86/mem_sharing.h    |    3 +
 xen/include/public/domctl.h          |   90 +++-------------
 xen/include/public/memory.h          |   87 ++++++++++++++++
 tools/libxc/xc_memshr.c              |   11 ++
 tools/libxc/xenctrl.h                |    1 +
 tools/tests/mem-sharing/memshrtool.c |   11 ++
 xen/arch/x86/mm/mem_sharing.c        |   13 +-
 xen/arch/x86/x86_64/compat/mm.c      |    3 +
 xen/arch/x86/x86_64/mm.c             |    2 +
 xen/include/asm-x86/mem_sharing.h    |    3 +-
 xen/include/public/memory.h          |    3 +-
 tools/libxc/xc_mem_access.c          |   10 +-
 tools/libxc/xc_mem_event.c           |   16 ++-
 tools/libxc/xc_mem_paging.c          |   10 +-
 tools/libxc/xenctrl.h                |    7 +-
 tools/tests/xen-access/xen-access.c  |   56 +---------
 tools/xenpaging/xenpaging.c          |   33 +----
 tools/xenpaging/xenpaging.h          |    2 +-
 xen/arch/x86/mm/mem_event.c          |   33 +-----
 xen/include/public/domctl.h          |    4 +-
 xen/include/public/mem_event.h       |    4 -
 xen/include/xen/sched.h              |    2 -
 39 files changed, 514 insertions(+), 407 deletions(-)
#1 lore raw

PATCH 1/1 Use memops for mem paging, sharing, and access, instead of domctls

 tools/libxc/xc_mem_access.c          |   12 +-
 tools/libxc/xc_mem_event.c           |   23 +++-
 tools/libxc/xc_mem_paging.c          |   44 ++++----
 tools/libxc/xc_memshr.c              |  182 ++++++++++++++++------------------
 tools/libxc/xenctrl.h                |    6 +-
 tools/memshr/interface.c             |    4 +-
 tools/tests/mem-sharing/memshrtool.c |    4 +-
 xen/arch/x86/domctl.c                |    1 -
 xen/arch/x86/mm/mem_access.c         |    7 +-
 xen/arch/x86/mm/mem_event.c          |   68 ++++++++++--
 xen/arch/x86/mm/mem_paging.c         |   13 +-
 xen/arch/x86/mm/mem_sharing.c        |   99 +++++++++++-------
 xen/arch/x86/x86_64/compat/mm.c      |   23 ++++
 xen/arch/x86/x86_64/mm.c             |   23 ++++
 xen/include/asm-x86/mem_access.h     |    3 +-
 xen/include/asm-x86/mem_event.h      |    2 +
 xen/include/asm-x86/mem_paging.h     |    3 +-
 xen/include/asm-x86/mem_sharing.h    |    3 +
 xen/include/public/domctl.h          |   90 +++-------------
 xen/include/public/memory.h          |   87 ++++++++++++++++
 20 files changed, 421 insertions(+), 276 deletions(-)

Per page operations in the paging, sharing, and access tracking subsystems are all implemented with domctls (e.g. a domctl to evict one page, or to share one page).

Under heavy load, the domctl path reveals a lack of scalability. The domctl lock serializes dom0's vcpus in the hypervisor. When performing thousands of per-page operations on dozens of domains, these vcpus will spin in the hypervisor. Beyond the aggressive locking, an added inefficiency of blocking vcpus in the domctl lock is that dom0 is prevented from re-scheduling any of its other work-starved processes.

We retain the domctl interface for setting up and tearing down paging/sharing/mem access for a domain. But we migrate all the per page operations to use the memory_op hypercalls (e.g XENMEM_*).

tools/libxc/xc_mem_access.c +6 -6
diff -r 7bbe59cba812 -r e187e3e40055 tools/libxc/xc_mem_access.c
@@ -30,7 +30,7 @@ int xc_mem_access_enable(xc_interface *x
     return xc_mem_event_control(xch, domain_id,
                                 XEN_DOMCTL_MEM_EVENT_OP_ACCESS_ENABLE,
                                 XEN_DOMCTL_MEM_EVENT_OP_ACCESS,
-                                shared_page, ring_page, INVALID_MFN);
+                                shared_page, ring_page);
 }
 
 int xc_mem_access_disable(xc_interface *xch, domid_t domain_id)
@@ -38,15 +38,15 @@ int xc_mem_access_disable(xc_interface *
     return xc_mem_event_control(xch, domain_id,
                                 XEN_DOMCTL_MEM_EVENT_OP_ACCESS_DISABLE,
                                 XEN_DOMCTL_MEM_EVENT_OP_ACCESS,
-                                NULL, NULL, INVALID_MFN);
+                                NULL, NULL);
 }
 
 int xc_mem_access_resume(xc_interface *xch, domid_t domain_id, unsigned long gfn)
 {
-    return xc_mem_event_control(xch, domain_id,
-                                XEN_DOMCTL_MEM_EVENT_OP_ACCESS_RESUME,
-                                XEN_DOMCTL_MEM_EVENT_OP_ACCESS,
-                                NULL, NULL, gfn);
+    return xc_mem_event_memop(xch, domain_id,
+                                XENMEM_access_op_resume,
+                                XENMEM_access_op,
+                                gfn, NULL);
 }
 
 /*
tools/libxc/xc_mem_event.c +18 -5
diff -r 7bbe59cba812 -r e187e3e40055 tools/libxc/xc_mem_event.c
@@ -24,8 +24,7 @@
 #include "xc_private.h"
 
 int xc_mem_event_control(xc_interface *xch, domid_t domain_id, unsigned int op,
-                         unsigned int mode, void *page,
-                         void *ring_page, unsigned long gfn)
+                         unsigned int mode, void *page, void *ring_page)
 {
     DECLARE_DOMCTL;
 
@@ -34,11 +33,25 @@ int xc_mem_event_control(xc_interface *x
     domctl.u.mem_event_op.op = op;
     domctl.u.mem_event_op.mode = mode;
 
-    domctl.u.mem_event_op.u.shared_addr = (unsigned long)page;
+    domctl.u.mem_event_op.shared_addr = (unsigned long)page;
     domctl.u.mem_event_op.ring_addr = (unsigned long)ring_page;
-
-    domctl.u.mem_event_op.gfn = gfn;
     
     return do_domctl(xch, &domctl);
 }
 
+int xc_mem_event_memop(xc_interface *xch, domid_t domain_id, 
+                        unsigned int op, unsigned int mode,
+                        uint64_t gfn, void *buffer)
+{
+    xen_mem_event_op_t meo;
+
+    memset(&meo, 0, sizeof(meo));
+
+    meo.op      = op;
+    meo.domain  = domain_id;
+    meo.gfn     = gfn;
+    meo.buffer  = (unsigned long) buffer;
+
+    return do_memory_op(xch, mode, &meo, sizeof(meo));
+}
+
tools/libxc/xc_mem_paging.c +22 -22

80 lines not on this page · raw message

tools/libxc/xc_memshr.c +84 -98

265 lines not on this page · raw message

tools/libxc/xenctrl.h +4 -2
diff -r 7bbe59cba812 -r e187e3e40055 tools/libxc/xenctrl.h
@@ -1888,8 +1888,10 @@ int xc_tmem_restore_extra(xc_interface *
  * mem_event operations
  */
 int xc_mem_event_control(xc_interface *xch, domid_t domain_id, unsigned int op,
-                         unsigned int mode, void *shared_page,
-                          void *ring_page, unsigned long gfn);
+                         unsigned int mode, void *shared_page, void *ring_page);
+int xc_mem_event_memop(xc_interface *xch, domid_t domain_id, 
+                        unsigned int op, unsigned int mode,
+                        uint64_t gfn, void *buffer);
 
 int xc_mem_paging_enable(xc_interface *xch, domid_t domain_id,
                         void *shared_page, void *ring_page);
tools/memshr/interface.c +2 -2
diff -r 7bbe59cba812 -r e187e3e40055 tools/memshr/interface.c
@@ -186,12 +186,12 @@ int memshr_vbd_issue_ro_request(char *bu
            remove the relevant ones from the map */
         switch(ret)
         {
-            case XEN_DOMCTL_MEM_SHARING_S_HANDLE_INVALID:
+            case XENMEM_SHARING_OP_S_HANDLE_INVALID:
                 ret = blockshr_shrhnd_remove(memshr.blks, source_st, NULL);
                 if(ret) DPRINTF("Could not rm invl s_hnd: %u %"PRId64" %"PRId64"\n", 
                                     source_st.domain, source_st.frame, source_st.handle);
                 break;
-            case XEN_DOMCTL_MEM_SHARING_C_HANDLE_INVALID:
+            case XENMEM_SHARING_OP_C_HANDLE_INVALID:
                 ret = blockshr_shrhnd_remove(memshr.blks, client_st, NULL);
                 if(ret) DPRINTF("Could not rm invl c_hnd: %u %"PRId64" %"PRId64"\n", 
                                     client_st.domain, client_st.frame, client_st.handle);
tools/tests/mem-sharing/memshrtool.c +2 -2
diff -r 7bbe59cba812 -r e187e3e40055 tools/tests/mem-sharing/memshrtool.c
@@ -34,9 +34,9 @@ static int usage(const char* prog)
     int rc = f; \
     if ( rc < 0 ) { \
         printf("error executing %s: %s\n", #f, \
-                ((errno * -1) == XEN_DOMCTL_MEM_SHARING_C_HANDLE_INVALID) ? \
+                ((errno * -1) == XENMEM_SHARING_OP_S_HANDLE_INVALID) ? \
                 "problem with client handle" :\
-                ((errno * -1) == XEN_DOMCTL_MEM_SHARING_S_HANDLE_INVALID) ? \
+                ((errno * -1) == XENMEM_SHARING_OP_C_HANDLE_INVALID) ? \
                 "problem with source handle" : strerror(errno)); \
         return rc; \
     } \
xen/arch/x86/domctl.c +0 -1
diff -r 7bbe59cba812 -r e187e3e40055 xen/arch/x86/domctl.c
@@ -1463,7 +1463,6 @@ long arch_do_domctl(
             if ( !ret )
                 ret = mem_sharing_domctl(d, &domctl->u.mem_sharing_op);
             rcu_unlock_domain(d);
-            copy_to_guest(u_domctl, domctl, 1);
         } 
     }
     break;
xen/arch/x86/mm/mem_access.c +3 -4
diff -r 7bbe59cba812 -r e187e3e40055 xen/arch/x86/mm/mem_access.c
@@ -25,14 +25,13 @@
 #include <asm/mem_event.h>
 
 
-int mem_access_domctl(struct domain *d, xen_domctl_mem_event_op_t *mec,
-                      XEN_GUEST_HANDLE(void) u_domctl)
+int mem_access_memop(struct domain *d, xen_mem_event_op_t *meo)
 {
     int rc;
 
-    switch( mec->op )
+    switch( meo->op )
     {
-    case XEN_DOMCTL_MEM_EVENT_OP_ACCESS_RESUME:
+    case XENMEM_access_op_resume:
     {
         p2m_mem_access_resume(d);
         rc = 0;
xen/arch/x86/mm/mem_event.c +57 -11

101 lines not on this page · raw message

xen/arch/x86/mm/mem_paging.c +6 -7
diff -r 7bbe59cba812 -r e187e3e40055 xen/arch/x86/mm/mem_paging.c
@@ -25,33 +25,32 @@
 #include <asm/mem_event.h>
 
 
-int mem_paging_domctl(struct domain *d, xen_domctl_mem_event_op_t *mec,
-                      XEN_GUEST_HANDLE(void) u_domctl)
+int mem_paging_memop(struct domain *d, xen_mem_event_op_t *mec)
 {
     switch( mec->op )
     {
-    case XEN_DOMCTL_MEM_EVENT_OP_PAGING_NOMINATE:
+    case XENMEM_paging_op_nominate:
     {
         unsigned long gfn = mec->gfn;
         return p2m_mem_paging_nominate(d, gfn);
     }
     break;
 
-    case XEN_DOMCTL_MEM_EVENT_OP_PAGING_EVICT:
+    case XENMEM_paging_op_evict:
     {
         unsigned long gfn = mec->gfn;
         return p2m_mem_paging_evict(d, gfn);
     }
     break;
 
-    case XEN_DOMCTL_MEM_EVENT_OP_PAGING_PREP:
+    case XENMEM_paging_op_prep:
     {
         unsigned long gfn = mec->gfn;
-        return p2m_mem_paging_prep(d, gfn, mec->u.buffer);
+        return p2m_mem_paging_prep(d, gfn, mec->buffer);
     }
     break;
 
-    case XEN_DOMCTL_MEM_EVENT_OP_PAGING_RESUME:
+    case XENMEM_paging_op_resume:
     {
         p2m_mem_paging_resume(d);
         return 0;
xen/arch/x86/mm/mem_sharing.c +58 -41

254 lines not on this page · raw message

xen/arch/x86/x86_64/compat/mm.c +23 -0
diff -r 7bbe59cba812 -r e187e3e40055 xen/arch/x86/x86_64/compat/mm.c
@@ -2,6 +2,7 @@
 #include <xen/multicall.h>
 #include <compat/memory.h>
 #include <compat/xen.h>
+#include <asm/mem_event.h>
 
 int compat_set_gdt(XEN_GUEST_HANDLE(uint) frame_list, unsigned int entries)
 {
@@ -211,6 +212,28 @@ int compat_arch_memory_op(int op, XEN_GU
     case XENMEM_get_sharing_shared_pages:
         return mem_sharing_get_nr_shared_mfns();
 
+    case XENMEM_paging_op:
+    case XENMEM_access_op:
+    {
+        xen_mem_event_op_t meo;
+        if ( copy_from_guest(&meo, arg, 1) )
+            return -EFAULT;
+        rc = do_mem_event_op(op, meo.domain, (void *) &meo);
+        if ( !rc && copy_to_guest(arg, &meo, 1) )
+            return -EFAULT;
+        break;
+    }
+    case XENMEM_sharing_op:
+    {
+        xen_mem_sharing_op_t mso;
+        if ( copy_from_guest(&mso, arg, 1) )
+            return -EFAULT;
+        rc = do_mem_event_op(op, mso.domain, (void *) &mso);
+        if ( !rc && copy_to_guest(arg, &mso, 1) )
+            return -EFAULT;
+        break;
+    }
+
     default:
         rc = -ENOSYS;
         break;
xen/arch/x86/x86_64/mm.c +23 -0
diff -r 7bbe59cba812 -r e187e3e40055 xen/arch/x86/x86_64/mm.c
@@ -34,6 +34,7 @@
 #include <asm/msr.h>
 #include <asm/setup.h>
 #include <asm/numa.h>
+#include <asm/mem_event.h>
 #include <asm/mem_sharing.h>
 #include <public/memory.h>
 
@@ -1100,6 +1101,28 @@ long subarch_memory_op(int op, XEN_GUEST
     case XENMEM_get_sharing_shared_pages:
         return mem_sharing_get_nr_shared_mfns();
 
+    case XENMEM_paging_op:
+    case XENMEM_access_op:
+    {
+        xen_mem_event_op_t meo;
+        if ( copy_from_guest(&meo, arg, 1) )
+            return -EFAULT;
+        rc = do_mem_event_op(op, meo.domain, (void *) &meo);
+        if ( !rc && copy_to_guest(arg, &meo, 1) )
+            return -EFAULT;
+        break;
+    }
+    case XENMEM_sharing_op:
+    {
+        xen_mem_sharing_op_t mso;
+        if ( copy_from_guest(&mso, arg, 1) )
+            return -EFAULT;
+        rc = do_mem_event_op(op, mso.domain, (void *) &mso);
+        if ( !rc && copy_to_guest(arg, &mso, 1) )
+            return -EFAULT;
+        break;
+    }
+
     default:
         rc = -ENOSYS;
         break;
xen/include/asm-x86/mem_access.h +1 -2
diff -r 7bbe59cba812 -r e187e3e40055 xen/include/asm-x86/mem_access.h
@@ -21,8 +21,7 @@
  */
 
 
-int mem_access_domctl(struct domain *d, xen_domctl_mem_event_op_t *mec,
-                      XEN_GUEST_HANDLE(void) u_domctl);
+int mem_access_memop(struct domain *d, xen_mem_event_op_t *meo);
 int mem_access_send_req(struct domain *d, mem_event_request_t *req);
 
 
xen/include/asm-x86/mem_event.h +2 -0
diff -r 7bbe59cba812 -r e187e3e40055 xen/include/asm-x86/mem_event.h
@@ -42,6 +42,8 @@ void mem_event_put_request(struct domain
 int mem_event_get_response(struct domain *d, struct mem_event_domain *med,
                            mem_event_response_t *rsp);
 
+struct domain *get_mem_event_op_target(uint32_t domain, int *rc);
+int do_mem_event_op(int op, uint32_t domain, void *arg);
 int mem_event_domctl(struct domain *d, xen_domctl_mem_event_op_t *mec,
                      XEN_GUEST_HANDLE(void) u_domctl);
 
xen/include/asm-x86/mem_paging.h +1 -2
diff -r 7bbe59cba812 -r e187e3e40055 xen/include/asm-x86/mem_paging.h
@@ -21,8 +21,7 @@
  */
 
 
-int mem_paging_domctl(struct domain *d, xen_domctl_mem_event_op_t *mec,
-                      XEN_GUEST_HANDLE(void) u_domctl);
+int mem_paging_memop(struct domain *d, xen_mem_event_op_t *meo);
 
 
 /*
xen/include/asm-x86/mem_sharing.h +3 -0
diff -r 7bbe59cba812 -r e187e3e40055 xen/include/asm-x86/mem_sharing.h
@@ -23,6 +23,7 @@
 #define __MEM_SHARING_H__
 
 #include <public/domctl.h>
+#include <public/memory.h>
 
 /* Auditing of memory sharing code? */
 #define MEM_SHARING_AUDIT 0
@@ -56,6 +57,8 @@ int mem_sharing_unshare_page(struct doma
                              unsigned long gfn, 
                              uint16_t flags);
 int mem_sharing_sharing_resume(struct domain *d);
+int mem_sharing_memop(struct domain *d, 
+                       xen_mem_sharing_op_t *mec);
 int mem_sharing_domctl(struct domain *d, 
                        xen_domctl_mem_sharing_op_t *mec);
 void mem_sharing_init(void);
xen/include/public/domctl.h +19 -71

129 lines not on this page · raw message

xen/include/public/memory.h +87 -0

99 lines not on this page · raw message

#4 lore raw

PATCH 1/1 x86/mm: New sharing audit memop

In reply to Andres Lagar-Cavilla
 tools/libxc/xc_memshr.c              |  11 +++++++++++
 tools/libxc/xenctrl.h                |   1 +
 tools/tests/mem-sharing/memshrtool.c |  11 +++++++++++
 xen/arch/x86/mm/mem_sharing.c        |  13 ++++++-------
 xen/arch/x86/x86_64/compat/mm.c      |   3 +++
 xen/arch/x86/x86_64/mm.c             |   2 ++
 xen/include/asm-x86/mem_sharing.h    |   3 ++-
 xen/include/public/memory.h          |   1 +
 8 files changed, 37 insertions(+), 8 deletions(-)

Remove costly mem_sharting audits from the inline path, and instead make them callable as a memop.

Have the audit function return the number of errors detected.

Update memshrtool to be able to trigger audits.

Set sharing audits as enabled by default.

tools/libxc/xc_memshr.c +11 -0
diff -r e187e3e40055 -r 26217ae46e45 tools/libxc/xc_memshr.c
@@ -211,6 +211,17 @@ int xc_memshr_debug_gref(xc_interface *x
     return xc_memshr_memop(xch, domid, &mso);
 }
 
+int xc_memshr_audit(xc_interface *xch)
+{
+    xen_mem_sharing_op_t mso;
+
+    memset(&mso, 0, sizeof(mso));
+
+    mso.op = XENMEM_sharing_op_audit;
+
+    return do_memory_op(xch, XENMEM_sharing_op, &mso, sizeof(mso));
+}
+
 long xc_sharing_freed_pages(xc_interface *xch)
 {
     return do_memory_op(xch, XENMEM_get_sharing_freed_pages, NULL, 0);
tools/libxc/xenctrl.h +1 -0
diff -r e187e3e40055 -r 26217ae46e45 tools/libxc/xenctrl.h
@@ -1956,6 +1956,7 @@ int xc_memshr_debug_mfn(xc_interface *xc
 int xc_memshr_debug_gref(xc_interface *xch,
                          domid_t domid,
                          grant_ref_t gref);
+int xc_memshr_audit(xc_interface *xch);
 
 int xc_flask_load(xc_interface *xc_handle, char *buf, uint32_t size);
 int xc_flask_context_to_sid(xc_interface *xc_handle, char *buf, uint32_t size, uint32_t *sid);
tools/tests/mem-sharing/memshrtool.c +11 -0
diff -r e187e3e40055 -r 26217ae46e45 tools/tests/mem-sharing/memshrtool.c
@@ -27,6 +27,7 @@ static int usage(const char* prog)
     printf("  add-to-physmap <domid> <gfn> <source> <source-gfn> <source-handle>\n");
     printf("                          - Populate a page in a domain with a shared page.\n");
     printf("  debug-gfn <domid> <gfn> - Debug a particular domain and gfn.\n");
+    printf("  audit                   - Audit the sharing subsytem in Xen.\n");
     return 1;
 }
 
@@ -160,6 +161,16 @@ int main(int argc, const char** argv)
         gfn = strtol(argv[3], NULL, 0);
         R(xc_memshr_debug_gfn(xch, domid, gfn));
     }
+    else if( !strcasecmp(cmd, "audit") )
+    {
+        int rc = xc_memshr_audit(xch);
+        if ( rc < 0 )
+        {
+            printf("error executing xc_memshr_audit: %s\n", strerror(errno));
+            return rc;
+        }
+        printf("Audit returned %d errors.\n", rc);
+    }
 
     return 0;
 }
xen/arch/x86/mm/mem_sharing.c +6 -7
diff -r e187e3e40055 -r 26217ae46e45 xen/arch/x86/mm/mem_sharing.c
@@ -50,8 +50,6 @@ DEFINE_PER_CPU(pg_lock_data_t, __pld);
 
 #if MEM_SHARING_AUDIT
 
-static void mem_sharing_audit(void);
-
 static struct list_head shr_audit_list;
 static spinlock_t shr_audit_lock;
 DEFINE_RCU_READ_LOCK(shr_audit_read_lock);
@@ -81,7 +79,10 @@ static inline void audit_del_list(struct
 
 #else
 
-#define mem_sharing_audit() ((void)0)
+int mem_sharing_audit(void)
+{
+    return -ENOSYS;
+}
 
 #define audit_add_list(p)  ((void)0)
 static inline void audit_del_list(struct page_info *page)
@@ -212,7 +213,7 @@ static struct page_info* mem_sharing_loo
 }
 
 #if MEM_SHARING_AUDIT
-static void mem_sharing_audit(void)
+int mem_sharing_audit(void)
 {
     int errors = 0;
     unsigned long count_expected;
@@ -338,6 +339,7 @@ static void mem_sharing_audit(void)
         errors++;
     }
 
+    return errors;
 }
 #endif
 
@@ -914,7 +916,6 @@ int mem_sharing_unshare_page(struct doma
     gfn_info_t *gfn_info = NULL;
     struct list_head *le;
    
-    mem_sharing_audit();
     mfn = get_gfn(d, gfn, &p2mt);
     
     /* Has someone already unshared it? */
@@ -1178,8 +1179,6 @@ int mem_sharing_memop(struct domain *d, 
             break;
     }
 
-    mem_sharing_audit();
-
     return rc;
 }
 
xen/arch/x86/x86_64/compat/mm.c +3 -0
diff -r e187e3e40055 -r 26217ae46e45 xen/arch/x86/x86_64/compat/mm.c
@@ -3,6 +3,7 @@
 #include <compat/memory.h>
 #include <compat/xen.h>
 #include <asm/mem_event.h>
+#include <asm/mem_sharing.h>
 
 int compat_set_gdt(XEN_GUEST_HANDLE(uint) frame_list, unsigned int entries)
 {
@@ -228,6 +229,8 @@ int compat_arch_memory_op(int op, XEN_GU
         xen_mem_sharing_op_t mso;
         if ( copy_from_guest(&mso, arg, 1) )
             return -EFAULT;
+        if ( mso.op == XENMEM_sharing_op_audit )
+            return mem_sharing_audit(); 
         rc = do_mem_event_op(op, mso.domain, (void *) &mso);
         if ( !rc && copy_to_guest(arg, &mso, 1) )
             return -EFAULT;
xen/arch/x86/x86_64/mm.c +2 -0
diff -r e187e3e40055 -r 26217ae46e45 xen/arch/x86/x86_64/mm.c
@@ -1117,6 +1117,8 @@ long subarch_memory_op(int op, XEN_GUEST
         xen_mem_sharing_op_t mso;
         if ( copy_from_guest(&mso, arg, 1) )
             return -EFAULT;
+        if ( mso.op == XENMEM_sharing_op_audit )
+            return mem_sharing_audit(); 
         rc = do_mem_event_op(op, mso.domain, (void *) &mso);
         if ( !rc && copy_to_guest(arg, &mso, 1) )
             return -EFAULT;
xen/include/asm-x86/mem_sharing.h +2 -1
diff -r e187e3e40055 -r 26217ae46e45 xen/include/asm-x86/mem_sharing.h
@@ -26,7 +26,7 @@
 #include <public/memory.h>
 
 /* Auditing of memory sharing code? */
-#define MEM_SHARING_AUDIT 0
+#define MEM_SHARING_AUDIT 1
 
 typedef uint64_t shr_handle_t; 
 
@@ -61,6 +61,7 @@ int mem_sharing_memop(struct domain *d,
                        xen_mem_sharing_op_t *mec);
 int mem_sharing_domctl(struct domain *d, 
                        xen_domctl_mem_sharing_op_t *mec);
+int mem_sharing_audit(void);
 void mem_sharing_init(void);
 
 #else 
xen/include/public/memory.h +1 -0
diff -r e187e3e40055 -r 26217ae46e45 xen/include/public/memory.h
@@ -349,6 +349,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_event_op
 #define XENMEM_sharing_op_debug_mfn         5
 #define XENMEM_sharing_op_debug_gref        6
 #define XENMEM_sharing_op_add_physmap       7
+#define XENMEM_sharing_op_audit             8
 
 #define XENMEM_SHARING_OP_S_HANDLE_INVALID  (-10)
 #define XENMEM_SHARING_OP_C_HANDLE_INVALID  (-9)
#6 lore raw

PATCH 1/1 Tools: Sanitize mem_event/access/paging interfaces

In reply to Andres Lagar-Cavilla
 tools/libxc/xc_mem_access.c         |  10 +++++-
 tools/libxc/xc_mem_event.c          |  16 +++++++---
 tools/libxc/xc_mem_paging.c         |  10 +++++-
 tools/libxc/xenctrl.h               |   7 ++-
 tools/tests/xen-access/xen-access.c |  56 +++++-------------------------------
 tools/xenpaging/xenpaging.c         |  33 ++++++---------------
 tools/xenpaging/xenpaging.h         |   2 +-
 xen/arch/x86/mm/mem_event.c         |  33 +-------------------
 xen/include/public/domctl.h         |   4 +-
 xen/include/public/mem_event.h      |   4 --
 xen/include/xen/sched.h             |   2 -
 11 files changed, 56 insertions(+), 121 deletions(-)

Don't use the superfluous shared page, return the event channel directly as part of the domctl struct, instead.

Use hypercall buffers to manage the ring page.

In-tree consumers (xenpaging, xen-access) updated. This is an ABI/API change, so please voice any concerns.

Known pending issues:
- pager could die and its ring page could be used by some other process, yet Xen retains the mapping to it.
- use a saner interface for the paging_load buffer.

tools/libxc/xc_mem_access.c +8 -2
diff -r 26217ae46e45 -r 6325dd552671 tools/libxc/xc_mem_access.c
@@ -25,12 +25,18 @@
 
 
 int xc_mem_access_enable(xc_interface *xch, domid_t domain_id,
-                        void *shared_page, void *ring_page)
+                         uint32_t *port, xc_hypercall_buffer_t *ring_page)
 {
+    if ( !port )
+    {
+        errno = -EINVAL;
+        return -1;
+    }
+
     return xc_mem_event_control(xch, domain_id,
                                 XEN_DOMCTL_MEM_EVENT_OP_ACCESS_ENABLE,
                                 XEN_DOMCTL_MEM_EVENT_OP_ACCESS,
-                                shared_page, ring_page);
+                                port, ring_page);
 }
 
 int xc_mem_access_disable(xc_interface *xch, domid_t domain_id)
tools/libxc/xc_mem_event.c +11 -5
diff -r 26217ae46e45 -r 6325dd552671 tools/libxc/xc_mem_event.c
@@ -24,19 +24,25 @@
 #include "xc_private.h"
 
 int xc_mem_event_control(xc_interface *xch, domid_t domain_id, unsigned int op,
-                         unsigned int mode, void *page, void *ring_page)
+                         unsigned int mode, uint32_t *port,
+                         xc_hypercall_buffer_t *ring_page)
 {
     DECLARE_DOMCTL;
+    DECLARE_HYPERCALL_BUFFER_ARGUMENT(ring_page);
+    int rc;
 
     domctl.cmd = XEN_DOMCTL_mem_event_op;
     domctl.domain = domain_id;
     domctl.u.mem_event_op.op = op;
     domctl.u.mem_event_op.mode = mode;
-
-    domctl.u.mem_event_op.shared_addr = (unsigned long)page;
-    domctl.u.mem_event_op.ring_addr = (unsigned long)ring_page;
+    domctl.u.mem_event_op.ring_addr = (ring_page) ?
+        (unsigned long long) HYPERCALL_BUFFER_AS_ARG(ring_page) : 0;
     
-    return do_domctl(xch, &domctl);
+    errno = 0;
+    rc = do_domctl(xch, &domctl);
+    if ( !rc && port )
+        *port = domctl.u.mem_event_op.port;
+    return rc;
 }
 
 int xc_mem_event_memop(xc_interface *xch, domid_t domain_id, 
tools/libxc/xc_mem_paging.c +8 -2
diff -r 26217ae46e45 -r 6325dd552671 tools/libxc/xc_mem_paging.c
@@ -25,12 +25,18 @@
 
 
 int xc_mem_paging_enable(xc_interface *xch, domid_t domain_id,
-                        void *shared_page, void *ring_page)
+                         uint32_t *port, xc_hypercall_buffer_t *ring_page)
 {
+    if ( !port )
+    {
+        errno = -EINVAL;
+        return -1;
+    }
+        
     return xc_mem_event_control(xch, domain_id,
                                 XEN_DOMCTL_MEM_EVENT_OP_PAGING_ENABLE,
                                 XEN_DOMCTL_MEM_EVENT_OP_PAGING,
-                                shared_page, ring_page);
+                                port, ring_page);
 }
 
 int xc_mem_paging_disable(xc_interface *xch, domid_t domain_id)
tools/libxc/xenctrl.h +4 -3
diff -r 26217ae46e45 -r 6325dd552671 tools/libxc/xenctrl.h
@@ -1888,13 +1888,14 @@ int xc_tmem_restore_extra(xc_interface *
  * mem_event operations
  */
 int xc_mem_event_control(xc_interface *xch, domid_t domain_id, unsigned int op,
-                         unsigned int mode, void *shared_page, void *ring_page);
+                         unsigned int mode, uint32_t *port,
+                         xc_hypercall_buffer_t *ring_page);
 int xc_mem_event_memop(xc_interface *xch, domid_t domain_id, 
                         unsigned int op, unsigned int mode,
                         uint64_t gfn, void *buffer);
 
 int xc_mem_paging_enable(xc_interface *xch, domid_t domain_id,
-                        void *shared_page, void *ring_page);
+                         uint32_t *port, xc_hypercall_buffer_t *ring_page);
 int xc_mem_paging_disable(xc_interface *xch, domid_t domain_id);
 int xc_mem_paging_nominate(xc_interface *xch, domid_t domain_id,
                            unsigned long gfn);
@@ -1906,7 +1907,7 @@ int xc_mem_paging_resume(xc_interface *x
                          unsigned long gfn);
 
 int xc_mem_access_enable(xc_interface *xch, domid_t domain_id,
-                        void *shared_page, void *ring_page);
+                         uint32_t *port, xc_hypercall_buffer_t *ring_page);
 int xc_mem_access_disable(xc_interface *xch, domid_t domain_id);
 int xc_mem_access_resume(xc_interface *xch, domid_t domain_id,
                          unsigned long gfn);
tools/tests/xen-access/xen-access.c +9 -47

100 lines not on this page · raw message

tools/xenpaging/xenpaging.c +10 -23
diff -r 26217ae46e45 -r 6325dd552671 tools/xenpaging/xenpaging.c
@@ -285,6 +285,7 @@ static struct xenpaging *xenpaging_init(
     xentoollog_logger *dbg = NULL;
     char *p;
     int rc;
+    DECLARE_HYPERCALL_BUFFER(void, ring_page);
 
     /* Allocate memory */
     paging = calloc(1, sizeof(struct xenpaging));
@@ -341,16 +342,9 @@ static struct xenpaging *xenpaging_init(
         goto err;
     }
 
-    /* Initialise shared page */
-    paging->mem_event.shared_page = init_page();
-    if ( paging->mem_event.shared_page == NULL )
-    {
-        PERROR("Error initialising shared page");
-        goto err;
-    }
-
     /* Initialise ring page */
-    paging->mem_event.ring_page = init_page();
+    paging->mem_event.ring_page = 
+        xc_hypercall_buffer_alloc_pages(paging->xc_handle, ring_page, 1);
     if ( paging->mem_event.ring_page == NULL )
     {
         PERROR("Error initialising ring page");
@@ -365,8 +359,8 @@ static struct xenpaging *xenpaging_init(
     
     /* Initialise Xen */
     rc = xc_mem_paging_enable(xch, paging->mem_event.domain_id,
-                             paging->mem_event.shared_page,
-                             paging->mem_event.ring_page);
+                             &paging->mem_event.evtchn_port, 
+                             HYPERCALL_BUFFER(ring_page));
     if ( rc != 0 )
     {
         switch ( errno ) {
@@ -397,7 +391,7 @@ static struct xenpaging *xenpaging_init(
     /* Bind event notification */
     rc = xc_evtchn_bind_interdomain(paging->mem_event.xce_handle,
                                     paging->mem_event.domain_id,
-                                    paging->mem_event.shared_page->port);
+                                    paging->mem_event.evtchn_port);
     if ( rc < 0 )
     {
         PERROR("Failed to bind event channel");
@@ -452,19 +446,12 @@ static struct xenpaging *xenpaging_init(
     {
         if ( paging->xs_handle )
             xs_close(paging->xs_handle);
+
+        if ( paging->mem_event.ring_page )
+            xc_hypercall_buffer_free_pages(xch, ring_page, 1);
+
         if ( xch )
             xc_interface_close(xch);
-        if ( paging->mem_event.shared_page )
-        {
-            munlock(paging->mem_event.shared_page, PAGE_SIZE);
-            free(paging->mem_event.shared_page);
-        }
-
-        if ( paging->mem_event.ring_page )
-        {
-            munlock(paging->mem_event.ring_page, PAGE_SIZE);
-            free(paging->mem_event.ring_page);
-        }
 
         free(dom_path);
         free(watch_target_tot_pages);
tools/xenpaging/xenpaging.h +1 -1
diff -r 26217ae46e45 -r 6325dd552671 tools/xenpaging/xenpaging.h
@@ -36,7 +36,7 @@ struct mem_event {
     xc_evtchn *xce_handle;
     int port;
     mem_event_back_ring_t back_ring;
-    mem_event_shared_page_t *shared_page;
+    uint32_t evtchn_port;
     void *ring_page;
 };
 
xen/arch/x86/mm/mem_event.c +3 -30
diff -r 26217ae46e45 -r 6325dd552671 xen/arch/x86/mm/mem_event.c
@@ -50,12 +50,10 @@ static int mem_event_enable(
     struct domain *dom_mem_event = current->domain;
     struct vcpu *v = current;
     unsigned long ring_addr = mec->ring_addr;
-    unsigned long shared_addr = mec->shared_addr;
     l1_pgentry_t l1e;
-    unsigned long shared_gfn = 0, ring_gfn = 0; /* gcc ... */
+    unsigned long ring_gfn = 0; /* gcc ... */
     p2m_type_t p2mt;
     mfn_t ring_mfn;
-    mfn_t shared_mfn;
 
     /* Only one helper at a time. If the helper crashed,
      * the ring is in an undefined state and so is the guest.
@@ -66,10 +64,6 @@ static int mem_event_enable(
     /* Get MFN of ring page */
     guest_get_eff_l1e(v, ring_addr, &l1e);
     ring_gfn = l1e_get_pfn(l1e);
-    /* We're grabbing these two in an order that could deadlock
-     * dom0 if 1. it were an hvm 2. there were two concurrent
-     * enables 3. the two gfn's in each enable criss-crossed
-     * 2MB regions. Duly noted.... */
     ring_mfn = get_gfn(dom_mem_event, ring_gfn, &p2mt);
 
     if ( unlikely(!mfn_valid(mfn_x(ring_mfn))) )
@@ -80,23 +74,9 @@ static int mem_event_enable(
 
     mem_event_ring_lock_init(med);
 
-    /* Get MFN of shared page */
-    guest_get_eff_l1e(v, shared_addr, &l1e);
-    shared_gfn = l1e_get_pfn(l1e);
-    shared_mfn = get_gfn(dom_mem_event, shared_gfn, &p2mt);
-
-    if ( unlikely(!mfn_valid(mfn_x(shared_mfn))) )
-    {
-        put_gfn(dom_mem_event, ring_gfn);
-        put_gfn(dom_mem_event, shared_gfn);
-        return -EINVAL;
-    }
-
-    /* Map ring and shared pages */
+    /* Map ring page */
     med->ring_page = map_domain_page(mfn_x(ring_mfn));
-    med->shared_page = map_domain_page(mfn_x(shared_mfn));
     put_gfn(dom_mem_event, ring_gfn);
-    put_gfn(dom_mem_event, shared_gfn); 
 
     /* Set the number of currently blocked vCPUs to 0. */
     med->blocked = 0;
@@ -108,8 +88,7 @@ static int mem_event_enable(
     if ( rc < 0 )
         goto err;
 
-    ((mem_event_shared_page_t *)med->shared_page)->port = rc;
-    med->xen_port = rc;
+    med->xen_port = mec->port = rc;
 
     /* Prepare ring buffer */
     FRONT_RING_INIT(&med->front_ring,
@@ -125,9 +104,6 @@ static int mem_event_enable(
     return 0;
 
  err:
-    unmap_domain_page(med->shared_page);
-    med->shared_page = NULL;
-
     unmap_domain_page(med->ring_page);
     med->ring_page = NULL;
 
@@ -246,9 +222,6 @@ static int mem_event_disable(struct doma
         unmap_domain_page(med->ring_page);
         med->ring_page = NULL;
 
-        unmap_domain_page(med->shared_page);
-        med->shared_page = NULL;
-
         /* Unblock all vCPUs */
         for_each_vcpu ( d, v )
         {
xen/include/public/domctl.h +2 -2
diff -r 26217ae46e45 -r 6325dd552671 xen/include/public/domctl.h
@@ -746,8 +746,8 @@ struct xen_domctl_mem_event_op {
     uint32_t       op;           /* XEN_DOMCTL_MEM_EVENT_OP_*_* */
     uint32_t       mode;         /* XEN_DOMCTL_MEM_EVENT_OP_* */
 
-    uint64_aligned_t shared_addr;  /* IN:  Virtual address of shared page */
-    uint64_aligned_t ring_addr;    /* IN:  Virtual address of ring page */
+    uint32_t port;              /* OUT: event channel for ring */
+    uint64_aligned_t ring_addr; /* IN:  Virtual address of ring page */
 };
 typedef struct xen_domctl_mem_event_op xen_domctl_mem_event_op_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_mem_event_op_t);
xen/include/public/mem_event.h +0 -4
diff -r 26217ae46e45 -r 6325dd552671 xen/include/public/mem_event.h
@@ -51,10 +51,6 @@
 #define MEM_EVENT_REASON_INT3        5    /* int3 was hit: gla/gfn are RIP */
 #define MEM_EVENT_REASON_SINGLESTEP  6    /* single step was invoked: gla/gfn are RIP */
 
-typedef struct mem_event_shared_page {
-    uint32_t port;
-} mem_event_shared_page_t;
-
 typedef struct mem_event_st {
     uint16_t type;
     uint16_t flags;
xen/include/xen/sched.h +0 -2
diff -r 26217ae46e45 -r 6325dd552671 xen/include/xen/sched.h
@@ -190,8 +190,6 @@ struct mem_event_domain
     /* The ring has 64 entries */
     unsigned char foreign_producers;
     unsigned char target_producers;
-    /* shared page */
-    mem_event_shared_page_t *shared_page;
     /* shared ring page */
     void *ring_page;
     /* front-end ring */
Acked-by on v1 1/1
#5 lore raw

Re: [PATCH 2 of 3] x86/mm: New sharing audit memop

In reply to Andres Lagar-Cavilla

Andres Lagar-Cavilla writes ("[Xen-devel] [PATCH 2 of 3] x86/mm: New sharing audit memop"):

Quoted 2 lines

Remove costly mem_sharting audits from the inline path, and instead make them callable as a memop.

For the tools parts,

But a hypervisor maintainer should have the final say.

Ian.

Acked-by on v1 1/1
#2 lore raw

Re: [PATCH 1 of 3] Use memops for mem paging, sharing, and access, instead of domctls

In reply to Andres Lagar-Cavilla

Andres Lagar-Cavilla writes ("[Xen-devel] [PATCH 1 of 3] Use memops for mem paging, sharing, and access, instead of domctls"):

Quoted 3 lines

We retain the domctl interface for setting up and tearing down paging/sharing/mem access for a domain. But we migrate all the per page operations to use the memory_op hypercalls (e.g XENMEM_*).

The tools part of this looks sensible (and pretty formulaic) to me.

It'll need a hypervisor maintainer to approve the innards.

Ian.

#3 lore raw

Re: [PATCH 1 of 3] Use memops for mem paging, sharing, and access, instead of domctls

Quoted 11 lines

Andres Lagar-Cavilla writes ("[Xen-devel] [PATCH 1 of 3] Use memops for mem paging, sharing, and access, instead of domctls"):

Quoted 3 lines

We retain the domctl interface for setting up and tearing down paging/sharing/mem access for a domain. But we migrate all the per page operations to use the memory_op hypercalls (e.g XENMEM_*).

The tools part of this looks sensible (and pretty formulaic) to me.

It'll need a hypervisor maintainer to approve the innards.

Both this and the next patch have been acked by Tim Deegan on the hypervisor side.

However, I have not tested these in isolation. I've always used them preceded by the "locking p2m series" posted earlier. I would suggest that you hold off applying this until the locking p2m series gets applied. I don't think there should be any problems, but I'm being conservative here.

Thanks!
Andres

Quoted 3 lines

Ian.

#7 lore raw
In reply to Andres Lagar-Cavilla
Quoted 24 lines · Andres Lagar-Cavilla

i(Was switch from domctl to memops)
Changes from v1 posted Feb 2nd 2012

- Patches 1 & 2 Acked-by Tim Deegan on the hypervisor side
- Added patch 3 to clean up the enable domctl interface, based on discussion with Ian Campbell

Description from original post follows:

Per page operations in the paging, sharing, and access tracking subsystems are all implemented with domctls (e.g. a domctl to evict one page, or to share one page).

Under heavy load, the domctl path reveals a lack of scalability. The domctl lock serializes dom0's vcpus in the hypervisor. When performing thousands of per-page operations on dozens of domains, these vcpus will spin in the hypervisor. Beyond the aggressive locking, an added inefficiency of blocking vcpus in the domctl lock is that dom0 is prevented from re-scheduling any of its other work-starved processes.

We retain the domctl interface for setting up and tearing down paging/sharing/mem access for a domain. But we migrate all the per page operations to use the memory_op hypercalls (e.g XENMEM_*).

Quoted 5 lines

This is a backwards-incompatible ABI change. It's been floating on the list for a couple weeks now, with no nacks thus far.

Applied 1 and 2; thanks.

I'll leave patch 3 for others to comment -- I know there are out-of-tree users of the mem-access interface, and changing the hypercalls is less disruptive than changing the libxc interface.

Tim.

#8 lore raw
Quoted 35 lines
Quoted 34 lines · Andres Lagar-Cavilla

i(Was switch from domctl to memops)
Changes from v1 posted Feb 2nd 2012

- Patches 1 & 2 Acked-by Tim Deegan on the hypervisor side
- Added patch 3 to clean up the enable domctl interface, based on discussion with Ian Campbell

Description from original post follows:

Per page operations in the paging, sharing, and access tracking subsystems are
all implemented with domctls (e.g. a domctl to evict one page, or to share one
page).

Under heavy load, the domctl path reveals a lack of scalability. The domctl
lock serializes dom0's vcpus in the hypervisor. When performing thousands of
per-page operations on dozens of domains, these vcpus will spin in the hypervisor. Beyond the aggressive locking, an added inefficiency of blocking
vcpus in the domctl lock is that dom0 is prevented from re-scheduling any of
its other work-starved processes.

We retain the domctl interface for setting up and tearing down paging/sharing/mem access for a domain. But we migrate all the per page operations to use the memory_op hypercalls (e.g XENMEM_*).

This is a backwards-incompatible ABI change. It's been floating on the list for
a couple weeks now, with no nacks thus far.

Quoted 8 lines
Quoted 2 lines

Applied 1 and 2; thanks.

I'll leave patch 3 for others to comment -- I know there are out-of-tree users of the mem-access interface, and changing the hypercalls is less disruptive than changing the libxc interface.

Makes a lot of sense. Thanks.

I don't view this change as sine qua-non, yet, "it would be nice if"...

Is there a timeout mechanism if out-of-tree consumers are not on the ball?

Actually, this hiatus allows me to float a perhaps cleaner way to map the
ring: the known problem is that the pager may die abruptly, and Xen is still posting events to a page now belonging to some other dom0 process. This is dealt with in the qemu-dm case by stuffing the ring in an unused pfn (presumably somewhere in the mmio hole?)

Would that work? Is there a policy for parceling out these "magic pfn's"?

Andres

Quoted 3 lines

Tim.

#9 lore raw
Quoted 1 line · Andres Lagar-Cavilla

Is there a timeout mechanism if out-of-tree consumers are not on the ball?

I would guess they need an adjustment for a step from 4.1 to 4.2. Wasnt there already enough churn in the source in that area? And the (old) SONAME is gone as well, so they need to be recompiled anyway.

Quoted 7 lines

Actually, this hiatus allows me to float a perhaps cleaner way to map the
ring: the known problem is that the pager may die abruptly, and Xen is still posting events to a page now belonging to some other dom0 process. This is dealt with in the qemu-dm case by stuffing the ring in an unused pfn (presumably somewhere in the mmio hole?)

Would that work? Is there a policy for parceling out these "magic pfn's"?

I was just thinking about this issue. The bug is that the ring lives in dom0, the page should belong to domU and should be destroyed along with it. And ring users in dom0 should request (and maybe initially allocate and setup) a certain gfn belonging to domU.

Olaf

#10 lore raw
Quoted 6 lines · Olaf Hering
Quoted 6 lines

Actually, this hiatus allows me to float a perhaps cleaner way to map the
ring: the known problem is that the pager may die abruptly, and Xen is still posting events to a page now belonging to some other dom0 process. This is dealt with in the qemu-dm case by stuffing the ring in an unused pfn (presumably somewhere in the mmio hole?)

Quoted 6 lines
Quoted 1 line

Would that work? Is there a policy for parceling out these "magic pfn's"?

I was just thinking about this issue. The bug is that the ring lives in dom0, the page should belong to domU and should be destroyed along with it. And ring users in dom0 should request (and maybe initially allocate and setup) a certain gfn belonging to domU.

Yes indeed. The gpfn could be allocated by the domain builder, or by hvmloader (which might be too late!).

-- Keir

#11 lore raw
Quoted 14 lines · Olaf Hering
Quoted 13 lines
Quoted 12 lines

Actually, this hiatus allows me to float a perhaps cleaner way to map the
ring: the known problem is that the pager may die abruptly, and Xen is still posting events to a page now belonging to some other dom0 process.
This is dealt with in the qemu-dm case by stuffing the ring in an unused
pfn (presumably somewhere in the mmio hole?)

Would that work? Is there a policy for parceling out these "magic pfn's"?

Quoted 7 lines
Quoted 4 lines

I was just thinking about this issue. The bug is that the ring lives in dom0, the page should belong to domU and should be destroyed along with it. And ring users in dom0 should request (and maybe initially allocate and setup) a certain gfn belonging to domU.

Yes indeed. The gpfn could be allocated by the domain builder, or by hvmloader (which might be too late!).

Well, let's say the domain builder reserves three gpfns (paging, access, sharing). When helpers for each come up, the enable domctl allocates the actual frame. Or, we could have the frames allocated up front, it's not terrible wastage -- the enable domctl would init the ring.

My question was more along the lines of how to choose which guest pfns to reserve for this.

Andres

Quoted 6 lines

-- Keir

#12 lore raw
Quoted 8 lines · Andres Lagar-Cavilla
Quoted 7 lines
Quoted 4 lines

I was just thinking about this issue. The bug is that the ring lives in dom0, the page should belong to domU and should be destroyed along with it. And ring users in dom0 should request (and maybe initially allocate and setup) a certain gfn belonging to domU.

Yes indeed. The gpfn could be allocated by the domain builder, or by hvmloader (which might be too late!).

Quoted 7 lines

Well, let's say the domain builder reserves three gpfns (paging, access, sharing). When helpers for each come up, the enable domctl allocates the actual frame. Or, we could have the frames allocated up front, it's not terrible wastage -- the enable domctl would init the ring.

My question was more along the lines of how to choose which guest pfns to reserve for this.

Domain builder (xc_hvm_build.c) would be the sensible place to reserve, and push that info down to Xen in some way. Then allocate the pages during the enable domctl, via alloc_xenheap_page() (for a few arcane reasons, it's better to use this than alloc_domheap_page() for this particular purpose).

-- Keir