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

Re: [Xen-devel] [PATCH v2 1/3] mem_access: modifications to mem_event enable API.



On Sun, Aug 24, 2014 at 5:26 PM, Dushyant Behl <myselfdushyantbehl@xxxxxxxxx> wrote:

tools/libxc/:
1. Modified the API xc_mem_event_enable to initialize
   shared ring to communicate with the hypervisor along with enabling mem_event.
2. Added memset to clear the ring_page of any bogus input before enabling any events.
3. Replaced calls to deprecated function xc_map_foreign_batch with calls
   to xc_map_foreign_bulk. The function xc_map_foreign_bulk has a cleaner
   error reporting interface than xc_map_foreign_batch.

4. The API xc_mem_event_enable is now modified to return int rather than void *,
   this was done to synchronize this API's behaviour with other mem_event API's.

tools/tests/xen-access/: Updated code to use the new helper API.

tools/ocaml/libs/xc/xenctrl_stubs.c: Changed the name of a macro from RING_SIZE
to BUF_RING_SIZE because the earlier name collided with xen shared ring
deinitions in io/ring.h

Signed-off-by: Dushyant Behl <myselfdushyantbehl@xxxxxxxxx>
---
 tools/libxc/xc_mem_access.c         |  8 ++++--

 tools/libxc/xc_mem_event.c          | 55 +++++++++++++++++++++++++++----------
 tools/libxc/xc_private.h            | 10 +++++--
 tools/libxc/xenctrl.h               |  9 ++++--
 tools/ocaml/libs/xc/xenctrl_stubs.c |  6 ++--
 tools/tests/xen-access/xen-access.c | 17 ++++--------
 6 files changed, 69 insertions(+), 36 deletions(-)


diff --git a/tools/libxc/xc_mem_access.c b/tools/libxc/xc_mem_access.c
index 461f0e9..89050be 100644
--- a/tools/libxc/xc_mem_access.c
+++ b/tools/libxc/xc_mem_access.c
@@ -24,9 +24,13 @@
 #include "xc_private.h"
 #include <xen/memory.h>

-void *xc_mem_access_enable(xc_interface *xch, domid_t domain_id, uint32_t *port)
+int xc_mem_access_enable(xc_interface *xch, domid_t domain_id,
+                         uint32_t *port, void *ring_page,
+                         mem_event_back_ring_t *back_ring)

 {
-    return xc_mem_event_enable(xch, domain_id, HVM_PARAM_ACCESS_RING_PFN, port);
+    return xc_mem_event_enable(xch, domain_id,
+                               HVM_PARAM_ACCESS_RING_PFN,
+                               port, ring_page, back_ring);
 }

 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 faf1cc6..cdbeca8 100644
--- a/tools/libxc/xc_mem_event.c
+++ b/tools/libxc/xc_mem_event.c

@@ -22,6 +22,7 @@
  */

 #include "xc_private.h"
+#include <xen/mem_event.h>

 int xc_mem_event_control(xc_interface *xch, domid_t domain_id, unsigned int op,
                          unsigned int mode, uint32_t *port)
@@ -56,19 +57,27 @@ int xc_mem_event_memop(xc_interface *xch, domid_t domain_id,
     return do_memory_op(xch, mode, &meo, sizeof(meo));
 }

-void *xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
-                          uint32_t *port)

+/*
+ * Enables mem_event and initializes shared ring to communicate with hypervisor.
+ * Returns 0 if success and if failure returns -1 with

+ * errno properly set to indicate possible error.
+ * Param can be HVM_PARAM_PAGING/ACCESS/SHARING_RING_PFN
+ */

+int xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
+                        uint32_t *port, void *ring_page,

 

If the idea is to assign the mapped ring_page here (so it can be munmap-ed later), then this should be void **, shouldn't it?

 

+                        mem_event_back_ring_t *back_ring)

 {
-    void *ring_page = NULL;
     uint64_t pfn;
     xen_pfn_t ring_pfn, mmap_pfn;
     unsigned int op, mode;
-    int rc1, rc2, saved_errno;
+    int rc1, rc2, saved_errno, err;
+
+    ring_page = NULL;

     if ( !port )
     {
         errno = EINVAL;
-        return NULL;

+        return -1;

     }

     /* Pause the domain for ring page setup */
@@ -76,7 +85,7 @@ void *xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
     if ( rc1 != 0 )
     {
         PERROR("Unable to pause domain\n");
-        return NULL;

+        return -1;

     }

     /* Get the pfn of the ring page */

@@ -89,9 +98,9 @@ void *xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,

     ring_pfn = pfn;

 

Is ring_pfn (or mmap_pfn) needed anymore? The two were needed because foreign_batch would modify in place the passed pfn to signal error conditions. foreign_bulk treats the pfn as read-only, so I think we can get rid of either.

 

I remember the ARM build breaking in this area. So please do an ARM build after you take care of Andresâs comments.

 

Thanks,

Aravindh

 

_______________________________________________
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®.