Tools: Remove shared page from mem_event/access/paging interfaces

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

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.

This change also affects the x86/mm bits in the hypervisor that process the
mem_event setup domctl.

Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
Acked-by: Tim Deegan <tim@xen.org>
Acked-by: Olaf Hering <olaf@aepfle.de>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Tim Deegan <tim@xen.org>

diff --git a/tools/libxc/xc_mem_access.c b/tools/libxc/xc_mem_access.c
index 3dca355dc195623bcf9aaa9526c7e9ea95eb61b8..a3786025f5e106b865957276675c0a6296eb3f14 100644 (file)
--- a/tools/libxc/xc_mem_access.c
+++ b/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, void *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)

diff --git a/tools/libxc/xc_mem_event.c b/tools/libxc/xc_mem_event.c
index a568a52e7787c557cb1982e2c013fe604cbab1e8..fcca47c764658278b2dfa9b78920568fd7a2677a 100644 (file)
--- a/tools/libxc/xc_mem_event.c
+++ b/tools/libxc/xc_mem_event.c
@@ -24,19 +24,21 @@
 #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, void *ring_page)
 {
     DECLARE_DOMCTL;
+    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 = (unsigned long) ring_page;
     
-    return do_domctl(xch, &domctl);
+    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, 

diff --git a/tools/libxc/xc_mem_paging.c b/tools/libxc/xc_mem_paging.c
index c1330295e7919ca2f81392db609b888f6e049576..f8eaa870b33bf838a841ada0e89f04828d2f963d 100644 (file)
--- a/tools/libxc/xc_mem_paging.c
+++ b/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, void *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)

diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index 1b2659f063f3a33ab80e166adbb85912b1e5b665..823d47a2aceaea6c1792397b1d410e65470152b5 100644 (file)
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -1892,13 +1892,13 @@ int xc_tmem_restore_extra(xc_interface *xch, int dom, int fd);
  * 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, 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);
+                         uint32_t *port, void *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);
@@ -1908,7 +1908,7 @@ int xc_mem_paging_load(xc_interface *xch, domid_t domain_id,
                         unsigned long gfn, void *buffer);
 
 int xc_mem_access_enable(xc_interface *xch, domid_t domain_id,
-                        void *shared_page, void *ring_page);
+                         uint32_t *port, void *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);

diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index 033ff8f9624b5be9e5fd27b15e11f1255788211b..d297529e42fa447e39c29cd7829f596a471776cf 100644 (file)
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -98,7 +98,7 @@ typedef 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;
     spinlock_t ring_lock;
 } mem_event_t;
@@ -166,7 +166,7 @@ int xc_wait_for_event_or_timeout(xc_interface *xch, xc_evtchn *xce, unsigned lon
  err:
     return -errno;
 }
-
+ 
 static void *init_page(void)
 {
     void *buffer;
@@ -214,14 +214,6 @@ xenaccess_t *xenaccess_init(xc_interface **xch_r, domid_t domain_id)
     /* Set domain id */
     xenaccess->mem_event.domain_id = domain_id;
 
-    /* Initialise shared page */
-    xenaccess->mem_event.shared_page = init_page();
-    if ( xenaccess->mem_event.shared_page == NULL )
-    {
-        ERROR("Error initialising shared page");
-        goto err;
-    }
-
     /* Initialise ring page */
     xenaccess->mem_event.ring_page = init_page();
     if ( xenaccess->mem_event.ring_page == NULL )
@@ -242,7 +234,7 @@ xenaccess_t *xenaccess_init(xc_interface **xch_r, domid_t domain_id)
 
     /* Initialise Xen */
     rc = xc_mem_access_enable(xenaccess->xc_handle, xenaccess->mem_event.domain_id,
-                             xenaccess->mem_event.shared_page,
+                             &xenaccess->mem_event.evtchn_port,
                              xenaccess->mem_event.ring_page);
     if ( rc != 0 )
     {
@@ -271,7 +263,7 @@ xenaccess_t *xenaccess_init(xc_interface **xch_r, domid_t domain_id)
     /* Bind event notification */
     rc = xc_evtchn_bind_interdomain(xenaccess->mem_event.xce_handle,
                                     xenaccess->mem_event.domain_id,
-                                    xenaccess->mem_event.shared_page->port);
+                                    xenaccess->mem_event.evtchn_port);
     if ( rc < 0 )
     {
         ERROR("Failed to bind event channel");
@@ -322,12 +314,6 @@ xenaccess_t *xenaccess_init(xc_interface **xch_r, domid_t domain_id)
  err:
     if ( xenaccess )
     {
-        if ( xenaccess->mem_event.shared_page )
-        {
-            munlock(xenaccess->mem_event.shared_page, PAGE_SIZE);
-            free(xenaccess->mem_event.shared_page);
-        }
-
         if ( xenaccess->mem_event.ring_page )
         {
             munlock(xenaccess->mem_event.ring_page, PAGE_SIZE);

diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
index c0f08e3c4c5990c667567b80066c9e9b0939583b..06ccdd996094d829c586d8f87bf749c461f7484a 100644 (file)
--- a/tools/xenpaging/xenpaging.c
+++ b/tools/xenpaging/xenpaging.c
@@ -337,14 +337,6 @@ static struct xenpaging *xenpaging_init(int argc, char *argv[])
         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();
     if ( paging->mem_event.ring_page == NULL )
@@ -361,7 +353,7 @@ static struct xenpaging *xenpaging_init(int argc, char *argv[])
     
     /* Initialise Xen */
     rc = xc_mem_paging_enable(xch, paging->mem_event.domain_id,
-                             paging->mem_event.shared_page,
+                             &paging->mem_event.evtchn_port, 
                              paging->mem_event.ring_page);
     if ( rc != 0 )
     {
@@ -393,7 +385,7 @@ static struct xenpaging *xenpaging_init(int argc, char *argv[])
     /* 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");
@@ -474,11 +466,6 @@ static struct xenpaging *xenpaging_init(int argc, char *argv[])
             munlock(paging->paging_buffer, PAGE_SIZE);
             free(paging->paging_buffer);
         }
-        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 )
         {

diff --git a/tools/xenpaging/xenpaging.h b/tools/xenpaging/xenpaging.h
index e348db63c5932d7b4fccf75f0ac504965bbeaff6..5a300c755c8d74ba3462178338b355383fc1d5d4 100644 (file)
--- a/tools/xenpaging/xenpaging.h
+++ b/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;
 };
 

diff --git a/xen/arch/x86/mm/mem_event.c b/xen/arch/x86/mm/mem_event.c
index 35bfedd8325a966f144f4732d60a105776ae13de..9a8b26b07d95982d91d61e71ddba699193983166 100644 (file)
--- a/xen/arch/x86/mm/mem_event.c
+++ b/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;
 
@@ -249,9 +225,6 @@ static int mem_event_disable(struct domain *d, struct mem_event_domain *med)
         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 )
         {

diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 3a4e9a734a065e5bffc0edecc83470ff84114dd6..941acc35909562d1214f57c66f21250063f68f56 100644 (file)
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -747,8 +747,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);

diff --git a/xen/include/public/mem_event.h b/xen/include/public/mem_event.h
index 29e125a5a7a3eab8c0fbb693500f1ae30dcc807b..5d0bd4c838d84d320a8ca95f494c6ce5f8c9b252 100644 (file)
--- a/xen/include/public/mem_event.h
+++ b/xen/include/public/mem_event.h
@@ -46,10 +46,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 {
     uint32_t flags;
     uint32_t vcpu_id;

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 3699929096e2fee116385a7126e8cb5a3effbe76..8c5931368da107d959be80a761ae52c57fd97264 100644 (file)
--- a/xen/include/xen/sched.h
+++ b/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 */