* [PATCH v2 1/3] mem_access: modifications to mem_event enable API.
@ 2014-08-25 0:26 Dushyant Behl
2014-08-25 0:26 ` [PATCH v2 2/3] mem_event: Added new helper API to teardown mem event setup and unmap ring_page Dushyant Behl
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Dushyant Behl @ 2014-08-25 0:26 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, Stefano Stabellini, Ian Jackson,
Aravindh Puthiyaparambil (aravindp),
Andres Lagar Cavilla, Dushyant Behl, David Scott
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@gmail.com>
---
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,
+ 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;
mmap_pfn = pfn;
- ring_page = xc_map_foreign_batch(xch, domain_id, PROT_READ | PROT_WRITE,
- &mmap_pfn, 1);
- if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
+ ring_page = xc_map_foreign_bulk(xch, domain_id,
+ PROT_READ | PROT_WRITE, &mmap_pfn, &err, 1);
+ if ( err != 0 || ring_page == NULL )
{
/* Map failed, populate ring page */
rc1 = xc_domain_populate_physmap_exact(xch, domain_id, 1, 0, 0,
@@ -103,15 +112,23 @@ void *xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
}
mmap_pfn = ring_pfn;
- ring_page = xc_map_foreign_batch(xch, domain_id, PROT_READ | PROT_WRITE,
- &mmap_pfn, 1);
- if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
+ ring_page = xc_map_foreign_bulk(xch, domain_id, PROT_READ | PROT_WRITE,
+ &mmap_pfn, &err, 1);
+ if ( err != 0 || ring_page == NULL )
{
PERROR("Could not map the ring page\n");
+ rc1 = -1;
goto out;
}
}
+ /* Clear the ring page */
+ memset(ring_page, 0, PAGE_SIZE);
+
+ /* Initialise ring */
+ SHARED_RING_INIT((mem_event_sring_t *)ring_page);
+ BACK_RING_INIT(back_ring, (mem_event_sring_t *)ring_page, PAGE_SIZE);
+
switch ( param )
{
case HVM_PARAM_PAGING_RING_PFN:
@@ -149,7 +166,12 @@ void *xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
/* Remove the ring_pfn from the guest's physmap */
rc1 = xc_domain_decrease_reservation_exact(xch, domain_id, 1, 0, &ring_pfn);
if ( rc1 != 0 )
+ {
PERROR("Failed to remove ring page from guest physmap");
+ goto out;
+ }
+
+ rc1 = 0;
out:
saved_errno = errno;
@@ -165,11 +187,16 @@ void *xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
}
if ( ring_page )
- munmap(ring_page, XC_PAGE_SIZE);
+ {
+ rc2 = munmap(ring_page, XC_PAGE_SIZE);
+ if ( rc2 < 0 )
+ PERROR("Error while munmap of ring_page");
+ }
ring_page = NULL;
errno = saved_errno;
+ rc1 = -1;
}
- return ring_page;
+ return rc1;
}
diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
index c50a7c9..3d455e7 100644
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -32,6 +32,7 @@
#include "xenctrl.h"
#include "xenctrlosdep.h"
+#include <xen/mem_event.h>
#include <xen/sys/privcmd.h>
#if defined(HAVE_VALGRIND_MEMCHECK_H) && !defined(NDEBUG) && !defined(__MINIOS__)
@@ -377,10 +378,13 @@ int xc_mem_event_memop(xc_interface *xch, domid_t domain_id,
unsigned int op, unsigned int mode,
uint64_t gfn, void *buffer);
/*
- * Enables mem_event and returns the mapped ring page indicated by param.
+ * Enables mem_event and initializes shared ring to communicate with hypervisor
+ * and sets ring_page equal to mapped page.
+ * Returns 0 if success and if failure returns -1 with errno properly set.
* param can be HVM_PARAM_PAGING/ACCESS/SHARING_RING_PFN
*/
-void *xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
- uint32_t *port);
+int xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
+ uint32_t *port, void *ring_page,
+ mem_event_back_ring_t *back_ring);
#endif /* __XC_PRIVATE_H__ */
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index 1c5d0db..9d043d7 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -47,6 +47,7 @@
#include <xen/xsm/flask_op.h>
#include <xen/tmem.h>
#include <xen/kexec.h>
+#include <xen/mem_event.h>
#include "xentoollog.h"
@@ -2258,11 +2259,13 @@ int xc_mem_paging_load(xc_interface *xch, domid_t domain_id,
*/
/*
- * Enables mem_access and returns the mapped ring page.
- * Will return NULL on error.
+ * Enables mem_access and sets arg ring_page equal to mapped page.
+ * Will return 0 on success and -1 on error.
* Caller has to unmap this page when done.
*/
-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);
int xc_mem_access_disable(xc_interface *xch, domid_t domain_id);
int xc_mem_access_resume(xc_interface *xch, domid_t domain_id);
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
index f0810eb..beccb38 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -526,12 +526,12 @@ CAMLprim value stub_xc_evtchn_reset(value xch, value domid)
}
-#define RING_SIZE 32768
-static char ring[RING_SIZE];
+#define BUF_RING_SIZE 32768
+static char ring[BUF_RING_SIZE];
CAMLprim value stub_xc_readconsolering(value xch)
{
- unsigned int size = RING_SIZE - 1;
+ unsigned int size = BUF_RING_SIZE - 1;
char *ring_ptr = ring;
int retval;
diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index 090df5f..9242f86 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -245,11 +245,12 @@ xenaccess_t *xenaccess_init(xc_interface **xch_r, domid_t domain_id)
mem_event_ring_lock_init(&xenaccess->mem_event);
/* Enable mem_access */
- xenaccess->mem_event.ring_page =
- xc_mem_access_enable(xenaccess->xc_handle,
- xenaccess->mem_event.domain_id,
- &xenaccess->mem_event.evtchn_port);
- if ( xenaccess->mem_event.ring_page == NULL )
+ rc = xc_mem_access_enable(xenaccess->xc_handle,
+ xenaccess->mem_event.domain_id,
+ &xenaccess->mem_event.evtchn_port,
+ xenaccess->mem_event.ring_page,
+ &xenaccess->mem_event.back_ring);
+ if ( rc < 0 )
{
switch ( errno ) {
case EBUSY:
@@ -287,12 +288,6 @@ xenaccess_t *xenaccess_init(xc_interface **xch_r, domid_t domain_id)
evtchn_bind = 1;
xenaccess->mem_event.port = rc;
- /* Initialise ring */
- SHARED_RING_INIT((mem_event_sring_t *)xenaccess->mem_event.ring_page);
- BACK_RING_INIT(&xenaccess->mem_event.back_ring,
- (mem_event_sring_t *)xenaccess->mem_event.ring_page,
- XC_PAGE_SIZE);
-
/* Get domaininfo */
xenaccess->domain_info = malloc(sizeof(xc_domaininfo_t));
if ( xenaccess->domain_info == NULL )
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] mem_event: Added new helper API to teardown mem event setup and unmap ring_page.
2014-08-25 0:26 [PATCH v2 1/3] mem_access: modifications to mem_event enable API Dushyant Behl
@ 2014-08-25 0:26 ` Dushyant Behl
2014-08-25 15:47 ` Andres Lagar Cavilla
2014-08-25 22:53 ` Aravindh Puthiyaparambil (aravindp)
2014-08-25 0:26 ` [PATCH v2 3/3] xenpaging: updated code to use safer mem_event API's for setup and teardown Dushyant Behl
2014-08-25 15:45 ` [PATCH v2 1/3] mem_access: modifications to mem_event enable API Andres Lagar Cavilla
2 siblings, 2 replies; 10+ messages in thread
From: Dushyant Behl @ 2014-08-25 0:26 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, Stefano Stabellini, Ian Jackson,
Aravindh Puthiyaparambil (aravindp),
Andres Lagar Cavilla, Dushyant Behl, David Scott
tools/libxc/xc_mem_event.c: Added new generic API to teardown mem event setup,
the API supports hvm params PAGING, ACCESS and SHARING and also completes the
obvious job of unmapping ring_page.
tools/libxc/xc_mem_access.c: Modified mem_event_disable to use the new teardown API.
tools/tests/xen-access/: Updated code to use the new API's.
Signed-off-by: Dushyant Behl <myselfdushyantbehl@gmail.com>
---
tools/libxc/xc_mem_access.c | 9 +++----
tools/libxc/xc_mem_event.c | 53 +++++++++++++++++++++++++++++++++++++
tools/libxc/xc_private.h | 8 ++++++
tools/libxc/xenctrl.h | 5 ++--
tools/tests/xen-access/xen-access.c | 6 ++---
5 files changed, 69 insertions(+), 12 deletions(-)
diff --git a/tools/libxc/xc_mem_access.c b/tools/libxc/xc_mem_access.c
index 89050be..29835c3 100644
--- a/tools/libxc/xc_mem_access.c
+++ b/tools/libxc/xc_mem_access.c
@@ -33,12 +33,11 @@ int xc_mem_access_enable(xc_interface *xch, domid_t domain_id,
port, ring_page, back_ring);
}
-int xc_mem_access_disable(xc_interface *xch, domid_t domain_id)
+int xc_mem_access_disable(xc_interface *xch, domid_t domain_id, void *ring_page)
{
- return xc_mem_event_control(xch, domain_id,
- XEN_DOMCTL_MEM_EVENT_OP_ACCESS_DISABLE,
- XEN_DOMCTL_MEM_EVENT_OP_ACCESS,
- NULL);
+ return xc_mem_event_disable(xch, domain_id,
+ HVM_PARAM_ACCESS_RING_PFN,
+ ring_page);
}
int xc_mem_access_resume(xc_interface *xch, domid_t domain_id)
diff --git a/tools/libxc/xc_mem_event.c b/tools/libxc/xc_mem_event.c
index cdbeca8..b6ae7d0 100644
--- a/tools/libxc/xc_mem_event.c
+++ b/tools/libxc/xc_mem_event.c
@@ -200,3 +200,56 @@ int xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
return rc1;
}
+
+/*
+ * Disable mem_event.
+ * Returns 0 on success, if failure returns -1 with errno properly set.
+ * param can be HVM_PARAM_PAGING/ACCESS/SHARING_RING_PFN
+ */
+int xc_mem_event_disable(xc_interface *xch, domid_t domain_id,
+ int param, void *ring_page)
+{
+ int rc;
+ unsigned int op, mode;
+
+ switch ( param )
+ {
+ case HVM_PARAM_PAGING_RING_PFN:
+ op = XEN_DOMCTL_MEM_EVENT_OP_PAGING_DISABLE;
+ mode = XEN_DOMCTL_MEM_EVENT_OP_PAGING;
+ break;
+
+ case HVM_PARAM_ACCESS_RING_PFN:
+ op = XEN_DOMCTL_MEM_EVENT_OP_ACCESS_DISABLE;
+ mode = XEN_DOMCTL_MEM_EVENT_OP_ACCESS;
+ break;
+
+ case HVM_PARAM_SHARING_RING_PFN:
+ op = XEN_DOMCTL_MEM_EVENT_OP_SHARING_DISABLE;
+ mode = XEN_DOMCTL_MEM_EVENT_OP_SHARING;
+ break;
+
+ /*
+ * This is for the outside chance that the HVM_PARAM is valid but is invalid
+ * as far as mem_event goes.
+ */
+ default:
+ errno = EINVAL;
+ rc = -1;
+ goto out;
+ }
+
+ /* Remove the ring page. */
+ rc = munmap(ring_page, XC_PAGE_SIZE);
+ if ( rc < 0 )
+ PERROR("Error while munmap of ring_page");
+
+ ring_page = NULL;
+
+ rc = xc_mem_event_control(xch, domain_id, op, mode, NULL);
+ if ( rc != 0 )
+ PERROR("Failed to disable mem_event\n");
+
+ out:
+ return rc;
+}
diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
index 3d455e7..eb07f31 100644
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -387,4 +387,12 @@ int xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
uint32_t *port, void *ring_page,
mem_event_back_ring_t *back_ring);
+/*
+ * Disable mem_event.
+ * Returns 0 on success, if failure returns -1 with errno properly set.
+ * param can be HVM_PARAM_PAGING/ACCESS/SHARING_RING_PFN
+ */
+int xc_mem_event_disable(xc_interface *xch, domid_t domain_id,
+ int param, void *ring_page);
+
#endif /* __XC_PRIVATE_H__ */
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index 9d043d7..546e6f8 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -2259,14 +2259,13 @@ int xc_mem_paging_load(xc_interface *xch, domid_t domain_id,
*/
/*
- * Enables mem_access and sets arg ring_page equal to mapped page.
+ * Enables mem_access and sets arg ring page equal to mapped page.
* Will return 0 on success and -1 on error.
- * Caller has to unmap this page when done.
*/
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);
-int xc_mem_access_disable(xc_interface *xch, domid_t domain_id);
+int xc_mem_access_disable(xc_interface *xch, domid_t domain_id, void *ring_page);
int xc_mem_access_resume(xc_interface *xch, domid_t domain_id);
/*
diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index 9242f86..a4ef578 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -170,13 +170,11 @@ int xenaccess_teardown(xc_interface *xch, xenaccess_t *xenaccess)
return 0;
/* Tear down domain xenaccess in Xen */
- if ( xenaccess->mem_event.ring_page )
- munmap(xenaccess->mem_event.ring_page, XC_PAGE_SIZE);
-
if ( mem_access_enable )
{
rc = xc_mem_access_disable(xenaccess->xc_handle,
- xenaccess->mem_event.domain_id);
+ xenaccess->mem_event.domain_id,
+ xenaccess->mem_event.ring_page);
if ( rc != 0 )
{
ERROR("Error tearing down domain xenaccess in xen");
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] xenpaging: updated code to use safer mem_event API's for setup and teardown.
2014-08-25 0:26 [PATCH v2 1/3] mem_access: modifications to mem_event enable API Dushyant Behl
2014-08-25 0:26 ` [PATCH v2 2/3] mem_event: Added new helper API to teardown mem event setup and unmap ring_page Dushyant Behl
@ 2014-08-25 0:26 ` Dushyant Behl
2014-08-25 15:49 ` Andres Lagar Cavilla
2014-08-25 15:45 ` [PATCH v2 1/3] mem_access: modifications to mem_event enable API Andres Lagar Cavilla
2 siblings, 1 reply; 10+ messages in thread
From: Dushyant Behl @ 2014-08-25 0:26 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, Stefano Stabellini, Ian Jackson,
Aravindh Puthiyaparambil (aravindp),
Andres Lagar Cavilla, Dushyant Behl, David Scott
tools/libxc/xc_mem_paging.c: updated mem_paging enable and disable API's to use
the mem event enable and disable routines. The mem event API's take care of
security issues mentioned in XSA-99 and also provide more coarse grained behaviour.
tools/xenpaging/xenpaging.c: added calls to the new API's and removed the code
which duplicated the new API behaviour.
Signed-off-by: Dushyant Behl <myselfdushyantbehl@gmail.com>
---
tools/libxc/xc_mem_paging.c | 34 ++++++++++++++---------------
tools/libxc/xenctrl.h | 14 ++++++++++--
tools/xenpaging/xenpaging.c | 52 ++++++---------------------------------------
3 files changed, 36 insertions(+), 64 deletions(-)
diff --git a/tools/libxc/xc_mem_paging.c b/tools/libxc/xc_mem_paging.c
index 8aa7d4d..173df1e 100644
--- a/tools/libxc/xc_mem_paging.c
+++ b/tools/libxc/xc_mem_paging.c
@@ -23,28 +23,28 @@
#include "xc_private.h"
-
+/*
+ * Enables mem_paging and sets arg ring page equal to mapped page.
+ * Will return 0 on success and -1 on error.
+ */
int xc_mem_paging_enable(xc_interface *xch, domid_t domain_id,
- uint32_t *port)
+ uint32_t *port, void *ring_page,
+ mem_event_back_ring_t *back_ring)
{
- 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,
- port);
+ return xc_mem_event_enable(xch, domain_id,
+ HVM_PARAM_PAGING_RING_PFN,
+ port, ring_page, back_ring);
}
-int xc_mem_paging_disable(xc_interface *xch, domid_t domain_id)
+/*
+ * Disable mem_paging and unmap ring page.
+ * Will return 0 on success and -1 on error.
+ */
+int xc_mem_paging_disable(xc_interface *xch, domid_t domain_id, void *ring_page)
{
- return xc_mem_event_control(xch, domain_id,
- XEN_DOMCTL_MEM_EVENT_OP_PAGING_DISABLE,
- XEN_DOMCTL_MEM_EVENT_OP_PAGING,
- NULL);
+ return xc_mem_event_disable(xch, domain_id,
+ HVM_PARAM_ACCESS_RING_PFN,
+ ring_page);
}
int xc_mem_paging_nominate(xc_interface *xch, domid_t domain_id, unsigned long gfn)
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index 546e6f8..23ef496 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -2244,8 +2244,18 @@ int xc_tmem_restore_extra(xc_interface *xch, int dom, int fd);
* Hardware-Assisted Paging (i.e. Intel EPT, AMD NPT). Moreover, AMD NPT
* support is considered experimental.
*/
-int xc_mem_paging_enable(xc_interface *xch, domid_t domain_id, uint32_t *port);
-int xc_mem_paging_disable(xc_interface *xch, domid_t domain_id);
+/*
+ * Enables mem_paging and sets arg ring page equal to mapped page.
+ * Returns 0 on success and -1 on error.
+ */
+int xc_mem_paging_enable(xc_interface *xch, domid_t domain_id,
+ uint32_t *port, void *ring_page,
+ mem_event_back_ring_t *back_ring);
+/*
+ * Disables mem_paging and unmaps ring page.
+ * Returns 0 on success and -1 on error.
+ */
+int xc_mem_paging_disable(xc_interface *xch, domid_t domain_id, void *ring_page);
int xc_mem_paging_nominate(xc_interface *xch, domid_t domain_id,
unsigned long gfn);
int xc_mem_paging_evict(xc_interface *xch, domid_t domain_id, unsigned long gfn);
diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
index 82c1ee4..4a841bf 100644
--- a/tools/xenpaging/xenpaging.c
+++ b/tools/xenpaging/xenpaging.c
@@ -337,40 +337,12 @@ static struct xenpaging *xenpaging_init(int argc, char *argv[])
PERROR("Could not bind to xenpaging watch\n");
goto err;
}
-
- /* Map the ring page */
- xc_get_hvm_param(xch, paging->mem_event.domain_id,
- HVM_PARAM_PAGING_RING_PFN, &ring_pfn);
- mmap_pfn = ring_pfn;
- paging->mem_event.ring_page =
- xc_map_foreign_batch(xch, paging->mem_event.domain_id,
- PROT_READ | PROT_WRITE, &mmap_pfn, 1);
- if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
- {
- /* Map failed, populate ring page */
- rc = xc_domain_populate_physmap_exact(paging->xc_handle,
- paging->mem_event.domain_id,
- 1, 0, 0, &ring_pfn);
- if ( rc != 0 )
- {
- PERROR("Failed to populate ring gfn\n");
- goto err;
- }
-
- mmap_pfn = ring_pfn;
- paging->mem_event.ring_page =
- xc_map_foreign_batch(xch, paging->mem_event.domain_id,
- PROT_READ | PROT_WRITE, &mmap_pfn, 1);
- if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
- {
- PERROR("Could not map the ring page\n");
- goto err;
- }
- }
- /* Initialise Xen */
+ /* Enable mem paging and initialize shared ring to communicate with xen. */
rc = xc_mem_paging_enable(xch, paging->mem_event.domain_id,
- &paging->mem_event.evtchn_port);
+ &paging->mem_event.evtchn_port,
+ paging->mem_event.ring_page,
+ &paging->mem_event.back_ring);
if ( rc != 0 )
{
switch ( errno ) {
@@ -413,17 +385,6 @@ static struct xenpaging *xenpaging_init(int argc, char *argv[])
paging->mem_event.port = rc;
- /* Initialise ring */
- SHARED_RING_INIT((mem_event_sring_t *)paging->mem_event.ring_page);
- BACK_RING_INIT(&paging->mem_event.back_ring,
- (mem_event_sring_t *)paging->mem_event.ring_page,
- PAGE_SIZE);
-
- /* Now that the ring is set, remove it from the guest's physmap */
- if ( xc_domain_decrease_reservation_exact(xch,
- paging->mem_event.domain_id, 1, 0, &ring_pfn) )
- PERROR("Failed to remove ring from guest physmap");
-
/* Get max_pages from guest if not provided via cmdline */
if ( !paging->max_pages )
{
@@ -523,9 +484,10 @@ static void xenpaging_teardown(struct xenpaging *paging)
xs_unwatch(paging->xs_handle, "@releaseDomain", watch_token);
paging->xc_handle = NULL;
+
/* Tear down domain paging in Xen */
- munmap(paging->mem_event.ring_page, PAGE_SIZE);
- rc = xc_mem_paging_disable(xch, paging->mem_event.domain_id);
+ rc = xc_mem_paging_disable(xch, paging->mem_event.domain_id,
+ paging->mem_event.ring_page);
if ( rc != 0 )
{
PERROR("Error tearing down domain paging in xen");
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] mem_access: modifications to mem_event enable API.
2014-08-25 0:26 [PATCH v2 1/3] mem_access: modifications to mem_event enable API Dushyant Behl
2014-08-25 0:26 ` [PATCH v2 2/3] mem_event: Added new helper API to teardown mem event setup and unmap ring_page Dushyant Behl
2014-08-25 0:26 ` [PATCH v2 3/3] xenpaging: updated code to use safer mem_event API's for setup and teardown Dushyant Behl
@ 2014-08-25 15:45 ` Andres Lagar Cavilla
2014-08-25 22:38 ` Aravindh Puthiyaparambil (aravindp)
2014-09-04 22:40 ` Dushyant Behl
2 siblings, 2 replies; 10+ messages in thread
From: Andres Lagar Cavilla @ 2014-08-25 15:45 UTC (permalink / raw)
To: Dushyant Behl
Cc: Ian Campbell, Stefano Stabellini, Ian Jackson, Xen-devel,
Aravindh Puthiyaparambil (aravindp),
David Scott
[-- Attachment #1.1: Type: text/plain, Size: 11964 bytes --]
On Sun, Aug 24, 2014 at 5:26 PM, Dushyant Behl <myselfdushyantbehl@gmail.com
> 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@gmail.com>
> ---
> 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.
> mmap_pfn = pfn;
> - ring_page = xc_map_foreign_batch(xch, domain_id, PROT_READ |
> PROT_WRITE,
> - &mmap_pfn, 1);
> - if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
> + ring_page = xc_map_foreign_bulk(xch, domain_id,
> + PROT_READ | PROT_WRITE, &mmap_pfn,
> &err, 1);
> + if ( err != 0 || ring_page == NULL )
{
> /* Map failed, populate ring page */
> rc1 = xc_domain_populate_physmap_exact(xch, domain_id, 1, 0, 0,
> @@ -103,15 +112,23 @@ void *xc_mem_event_enable(xc_interface *xch, domid_t
> domain_id, int param,
> }
>
> mmap_pfn = ring_pfn;
> - ring_page = xc_map_foreign_batch(xch, domain_id, PROT_READ |
> PROT_WRITE,
> - &mmap_pfn, 1);
> - if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
> + ring_page = xc_map_foreign_bulk(xch, domain_id, PROT_READ |
> PROT_WRITE,
> + &mmap_pfn, &err, 1);
> + if ( err != 0 || ring_page == NULL )
> {
> PERROR("Could not map the ring page\n");
> + rc1 = -1;
>
If the convention will be to set errno (Ians?), then set errno here
errno = (err) ? : EINVAL;
or similar
> goto out;
> }
> }
>
> + /* Clear the ring page */
> + memset(ring_page, 0, PAGE_SIZE);
> +
> + /* Initialise ring */
> + SHARED_RING_INIT((mem_event_sring_t *)ring_page);
> + BACK_RING_INIT(back_ring, (mem_event_sring_t *)ring_page, PAGE_SIZE);
> +
> switch ( param )
> {
> case HVM_PARAM_PAGING_RING_PFN:
> @@ -149,7 +166,12 @@ void *xc_mem_event_enable(xc_interface *xch, domid_t
> domain_id, int param,
> /* Remove the ring_pfn from the guest's physmap */
> rc1 = xc_domain_decrease_reservation_exact(xch, domain_id, 1, 0,
> &ring_pfn);
> if ( rc1 != 0 )
> + {
> PERROR("Failed to remove ring page from guest physmap");
> + goto out;
> + }
> +
> + rc1 = 0;
>
> out:
> saved_errno = errno;
> @@ -165,11 +187,16 @@ void *xc_mem_event_enable(xc_interface *xch, domid_t
> domain_id, int param,
> }
>
> if ( ring_page )
> - munmap(ring_page, XC_PAGE_SIZE);
> + {
> + rc2 = munmap(ring_page, XC_PAGE_SIZE);
> + if ( rc2 < 0 )
> + PERROR("Error while munmap of ring_page");
> + }
> ring_page = NULL;
>
> errno = saved_errno;
> + rc1 = -1;
> }
>
> - return ring_page;
> + return rc1;
> }
> diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
> index c50a7c9..3d455e7 100644
> --- a/tools/libxc/xc_private.h
> +++ b/tools/libxc/xc_private.h
> @@ -32,6 +32,7 @@
> #include "xenctrl.h"
> #include "xenctrlosdep.h"
>
> +#include <xen/mem_event.h>
> #include <xen/sys/privcmd.h>
>
> #if defined(HAVE_VALGRIND_MEMCHECK_H) && !defined(NDEBUG) &&
> !defined(__MINIOS__)
> @@ -377,10 +378,13 @@ int xc_mem_event_memop(xc_interface *xch, domid_t
> domain_id,
> unsigned int op, unsigned int mode,
> uint64_t gfn, void *buffer);
> /*
> - * Enables mem_event and returns the mapped ring page indicated by param.
> + * Enables mem_event and initializes shared ring to communicate with
> hypervisor
> + * and sets ring_page equal to mapped page.
>
yeah, ring_page should be void **
> + * Returns 0 if success and if failure returns -1 with errno properly set.
> * param can be HVM_PARAM_PAGING/ACCESS/SHARING_RING_PFN
> */
> -void *xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
> - uint32_t *port);
> +int xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
> + uint32_t *port, void *ring_page,
> + mem_event_back_ring_t *back_ring);
>
> #endif /* __XC_PRIVATE_H__ */
> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> index 1c5d0db..9d043d7 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -47,6 +47,7 @@
> #include <xen/xsm/flask_op.h>
> #include <xen/tmem.h>
> #include <xen/kexec.h>
> +#include <xen/mem_event.h>
>
> #include "xentoollog.h"
>
> @@ -2258,11 +2259,13 @@ int xc_mem_paging_load(xc_interface *xch, domid_t
> domain_id,
> */
>
> /*
> - * Enables mem_access and returns the mapped ring page.
> - * Will return NULL on error.
> + * Enables mem_access and sets arg ring_page equal to mapped page.
> + * Will return 0 on success and -1 on error.
> * Caller has to unmap this page when done.
> */
> -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);
> int xc_mem_access_disable(xc_interface *xch, domid_t domain_id);
> int xc_mem_access_resume(xc_interface *xch, domid_t domain_id);
>
> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
> b/tools/ocaml/libs/xc/xenctrl_stubs.c
> index f0810eb..beccb38 100644
> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
> @@ -526,12 +526,12 @@ CAMLprim value stub_xc_evtchn_reset(value xch, value
> domid)
> }
>
>
> -#define RING_SIZE 32768
> -static char ring[RING_SIZE];
> +#define BUF_RING_SIZE 32768
> +static char ring[BUF_RING_SIZE];
>
> CAMLprim value stub_xc_readconsolering(value xch)
> {
> - unsigned int size = RING_SIZE - 1;
> + unsigned int size = BUF_RING_SIZE - 1;
> char *ring_ptr = ring;
> int retval;
>
> diff --git a/tools/tests/xen-access/xen-access.c
> b/tools/tests/xen-access/xen-access.c
> index 090df5f..9242f86 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -245,11 +245,12 @@ xenaccess_t *xenaccess_init(xc_interface **xch_r,
> domid_t domain_id)
> mem_event_ring_lock_init(&xenaccess->mem_event);
>
> /* Enable mem_access */
> - xenaccess->mem_event.ring_page =
> - xc_mem_access_enable(xenaccess->xc_handle,
> - xenaccess->mem_event.domain_id,
> - &xenaccess->mem_event.evtchn_port);
> - if ( xenaccess->mem_event.ring_page == NULL )
> + rc = xc_mem_access_enable(xenaccess->xc_handle,
> + xenaccess->mem_event.domain_id,
> + &xenaccess->mem_event.evtchn_port,
> + xenaccess->mem_event.ring_page,
> + &xenaccess->mem_event.back_ring);
> + if ( rc < 0 )
> {
> switch ( errno ) {
> case EBUSY:
> @@ -287,12 +288,6 @@ xenaccess_t *xenaccess_init(xc_interface **xch_r,
> domid_t domain_id)
> evtchn_bind = 1;
> xenaccess->mem_event.port = rc;
>
> - /* Initialise ring */
> - SHARED_RING_INIT((mem_event_sring_t *)xenaccess->mem_event.ring_page);
> - BACK_RING_INIT(&xenaccess->mem_event.back_ring,
> - (mem_event_sring_t *)xenaccess->mem_event.ring_page,
> - XC_PAGE_SIZE);
> -
> /* Get domaininfo */
> xenaccess->domain_info = malloc(sizeof(xc_domaininfo_t));
> if ( xenaccess->domain_info == NULL )
> --
> 1.9.1
>
>
[-- Attachment #1.2: Type: text/html, Size: 15580 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] mem_event: Added new helper API to teardown mem event setup and unmap ring_page.
2014-08-25 0:26 ` [PATCH v2 2/3] mem_event: Added new helper API to teardown mem event setup and unmap ring_page Dushyant Behl
@ 2014-08-25 15:47 ` Andres Lagar Cavilla
2014-08-25 22:53 ` Aravindh Puthiyaparambil (aravindp)
1 sibling, 0 replies; 10+ messages in thread
From: Andres Lagar Cavilla @ 2014-08-25 15:47 UTC (permalink / raw)
To: Dushyant Behl
Cc: Ian Campbell, Stefano Stabellini, Ian Jackson, Xen-devel,
Aravindh Puthiyaparambil (aravindp),
David Scott
[-- Attachment #1.1: Type: text/plain, Size: 6315 bytes --]
On Sun, Aug 24, 2014 at 5:26 PM, Dushyant Behl <myselfdushyantbehl@gmail.com
> wrote:
> tools/libxc/xc_mem_event.c: Added new generic API to teardown mem event
> setup,
> the API supports hvm params PAGING, ACCESS and SHARING and also completes
> the
> obvious job of unmapping ring_page.
>
> tools/libxc/xc_mem_access.c: Modified mem_event_disable to use the new
> teardown API.
>
> tools/tests/xen-access/: Updated code to use the new API's.
>
> Signed-off-by: Dushyant Behl <myselfdushyantbehl@gmail.com>
> ---
> tools/libxc/xc_mem_access.c | 9 +++----
> tools/libxc/xc_mem_event.c | 53
> +++++++++++++++++++++++++++++++++++++
> tools/libxc/xc_private.h | 8 ++++++
> tools/libxc/xenctrl.h | 5 ++--
> tools/tests/xen-access/xen-access.c | 6 ++---
> 5 files changed, 69 insertions(+), 12 deletions(-)
>
Reviewed-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
>
> diff --git a/tools/libxc/xc_mem_access.c b/tools/libxc/xc_mem_access.c
> index 89050be..29835c3 100644
> --- a/tools/libxc/xc_mem_access.c
> +++ b/tools/libxc/xc_mem_access.c
> @@ -33,12 +33,11 @@ int xc_mem_access_enable(xc_interface *xch, domid_t
> domain_id,
> port, ring_page, back_ring);
> }
>
> -int xc_mem_access_disable(xc_interface *xch, domid_t domain_id)
> +int xc_mem_access_disable(xc_interface *xch, domid_t domain_id, void
> *ring_page)
> {
> - return xc_mem_event_control(xch, domain_id,
> - XEN_DOMCTL_MEM_EVENT_OP_ACCESS_DISABLE,
> - XEN_DOMCTL_MEM_EVENT_OP_ACCESS,
> - NULL);
> + return xc_mem_event_disable(xch, domain_id,
> + HVM_PARAM_ACCESS_RING_PFN,
> + ring_page);
> }
>
> int xc_mem_access_resume(xc_interface *xch, domid_t domain_id)
> diff --git a/tools/libxc/xc_mem_event.c b/tools/libxc/xc_mem_event.c
> index cdbeca8..b6ae7d0 100644
> --- a/tools/libxc/xc_mem_event.c
> +++ b/tools/libxc/xc_mem_event.c
> @@ -200,3 +200,56 @@ int xc_mem_event_enable(xc_interface *xch, domid_t
> domain_id, int param,
>
> return rc1;
> }
> +
> +/*
> + * Disable mem_event.
> + * Returns 0 on success, if failure returns -1 with errno properly set.
> + * param can be HVM_PARAM_PAGING/ACCESS/SHARING_RING_PFN
> + */
> +int xc_mem_event_disable(xc_interface *xch, domid_t domain_id,
> + int param, void *ring_page)
> +{
> + int rc;
> + unsigned int op, mode;
> +
> + switch ( param )
> + {
> + case HVM_PARAM_PAGING_RING_PFN:
> + op = XEN_DOMCTL_MEM_EVENT_OP_PAGING_DISABLE;
> + mode = XEN_DOMCTL_MEM_EVENT_OP_PAGING;
> + break;
> +
> + case HVM_PARAM_ACCESS_RING_PFN:
> + op = XEN_DOMCTL_MEM_EVENT_OP_ACCESS_DISABLE;
> + mode = XEN_DOMCTL_MEM_EVENT_OP_ACCESS;
> + break;
> +
> + case HVM_PARAM_SHARING_RING_PFN:
> + op = XEN_DOMCTL_MEM_EVENT_OP_SHARING_DISABLE;
> + mode = XEN_DOMCTL_MEM_EVENT_OP_SHARING;
> + break;
> +
> + /*
> + * This is for the outside chance that the HVM_PARAM is valid but
> is invalid
> + * as far as mem_event goes.
> + */
> + default:
> + errno = EINVAL;
> + rc = -1;
> + goto out;
> + }
> +
> + /* Remove the ring page. */
> + rc = munmap(ring_page, XC_PAGE_SIZE);
> + if ( rc < 0 )
> + PERROR("Error while munmap of ring_page");
> +
> + ring_page = NULL;
> +
> + rc = xc_mem_event_control(xch, domain_id, op, mode, NULL);
> + if ( rc != 0 )
> + PERROR("Failed to disable mem_event\n");
> +
> + out:
> + return rc;
> +}
> diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
> index 3d455e7..eb07f31 100644
> --- a/tools/libxc/xc_private.h
> +++ b/tools/libxc/xc_private.h
> @@ -387,4 +387,12 @@ int xc_mem_event_enable(xc_interface *xch, domid_t
> domain_id, int param,
> uint32_t *port, void *ring_page,
> mem_event_back_ring_t *back_ring);
>
> +/*
> + * Disable mem_event.
> + * Returns 0 on success, if failure returns -1 with errno properly set.
> + * param can be HVM_PARAM_PAGING/ACCESS/SHARING_RING_PFN
> + */
> +int xc_mem_event_disable(xc_interface *xch, domid_t domain_id,
> + int param, void *ring_page);
> +
> #endif /* __XC_PRIVATE_H__ */
> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> index 9d043d7..546e6f8 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -2259,14 +2259,13 @@ int xc_mem_paging_load(xc_interface *xch, domid_t
> domain_id,
> */
>
> /*
> - * Enables mem_access and sets arg ring_page equal to mapped page.
> + * Enables mem_access and sets arg ring page equal to mapped page.
> * Will return 0 on success and -1 on error.
> - * Caller has to unmap this page when done.
> */
> 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);
> -int xc_mem_access_disable(xc_interface *xch, domid_t domain_id);
> +int xc_mem_access_disable(xc_interface *xch, domid_t domain_id, void
> *ring_page);
> int xc_mem_access_resume(xc_interface *xch, domid_t domain_id);
>
> /*
> diff --git a/tools/tests/xen-access/xen-access.c
> b/tools/tests/xen-access/xen-access.c
> index 9242f86..a4ef578 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -170,13 +170,11 @@ int xenaccess_teardown(xc_interface *xch,
> xenaccess_t *xenaccess)
> return 0;
>
> /* Tear down domain xenaccess in Xen */
> - if ( xenaccess->mem_event.ring_page )
> - munmap(xenaccess->mem_event.ring_page, XC_PAGE_SIZE);
> -
> if ( mem_access_enable )
> {
> rc = xc_mem_access_disable(xenaccess->xc_handle,
> - xenaccess->mem_event.domain_id);
> + xenaccess->mem_event.domain_id,
> + xenaccess->mem_event.ring_page);
> if ( rc != 0 )
> {
> ERROR("Error tearing down domain xenaccess in xen");
> --
> 1.9.1
>
>
[-- Attachment #1.2: Type: text/html, Size: 7890 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] xenpaging: updated code to use safer mem_event API's for setup and teardown.
2014-08-25 0:26 ` [PATCH v2 3/3] xenpaging: updated code to use safer mem_event API's for setup and teardown Dushyant Behl
@ 2014-08-25 15:49 ` Andres Lagar Cavilla
0 siblings, 0 replies; 10+ messages in thread
From: Andres Lagar Cavilla @ 2014-08-25 15:49 UTC (permalink / raw)
To: Dushyant Behl
Cc: Ian Campbell, Stefano Stabellini, Ian Jackson, Xen-devel,
Aravindh Puthiyaparambil (aravindp),
David Scott
[-- Attachment #1.1: Type: text/plain, Size: 7498 bytes --]
On Sun, Aug 24, 2014 at 5:26 PM, Dushyant Behl <myselfdushyantbehl@gmail.com
> wrote:
> tools/libxc/xc_mem_paging.c: updated mem_paging enable and disable API's
> to use
> the mem event enable and disable routines. The mem event API's take care of
> security issues mentioned in XSA-99 and also provide more coarse grained
> behaviour.
>
> tools/xenpaging/xenpaging.c: added calls to the new API's and removed the
> code
> which duplicated the new API behaviour.
>
> Signed-off-by: Dushyant Behl <myselfdushyantbehl@gmail.com>
> ---
> tools/libxc/xc_mem_paging.c | 34 ++++++++++++++---------------
> tools/libxc/xenctrl.h | 14 ++++++++++--
> tools/xenpaging/xenpaging.c | 52
> ++++++---------------------------------------
> 3 files changed, 36 insertions(+), 64 deletions(-)
>
Reviewed-by Andres Lagar-Cavilla <andres@lagarcavilla.org>
Worth repeating this is meant to serve multiple future in-tree users of
mempaging (e.g. lazy restore)
Andres
>
> diff --git a/tools/libxc/xc_mem_paging.c b/tools/libxc/xc_mem_paging.c
> index 8aa7d4d..173df1e 100644
> --- a/tools/libxc/xc_mem_paging.c
> +++ b/tools/libxc/xc_mem_paging.c
> @@ -23,28 +23,28 @@
>
> #include "xc_private.h"
>
> -
> +/*
> + * Enables mem_paging and sets arg ring page equal to mapped page.
> + * Will return 0 on success and -1 on error.
> + */
> int xc_mem_paging_enable(xc_interface *xch, domid_t domain_id,
> - uint32_t *port)
> + uint32_t *port, void *ring_page,
> + mem_event_back_ring_t *back_ring)
> {
> - 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,
> - port);
> + return xc_mem_event_enable(xch, domain_id,
> + HVM_PARAM_PAGING_RING_PFN,
> + port, ring_page, back_ring);
> }
>
> -int xc_mem_paging_disable(xc_interface *xch, domid_t domain_id)
> +/*
> + * Disable mem_paging and unmap ring page.
> + * Will return 0 on success and -1 on error.
> + */
> +int xc_mem_paging_disable(xc_interface *xch, domid_t domain_id, void
> *ring_page)
> {
> - return xc_mem_event_control(xch, domain_id,
> - XEN_DOMCTL_MEM_EVENT_OP_PAGING_DISABLE,
> - XEN_DOMCTL_MEM_EVENT_OP_PAGING,
> - NULL);
> + return xc_mem_event_disable(xch, domain_id,
> + HVM_PARAM_ACCESS_RING_PFN,
> + ring_page);
> }
>
> int xc_mem_paging_nominate(xc_interface *xch, domid_t domain_id, unsigned
> long gfn)
> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> index 546e6f8..23ef496 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -2244,8 +2244,18 @@ int xc_tmem_restore_extra(xc_interface *xch, int
> dom, int fd);
> * Hardware-Assisted Paging (i.e. Intel EPT, AMD NPT). Moreover, AMD NPT
> * support is considered experimental.
> */
> -int xc_mem_paging_enable(xc_interface *xch, domid_t domain_id, uint32_t
> *port);
> -int xc_mem_paging_disable(xc_interface *xch, domid_t domain_id);
> +/*
> + * Enables mem_paging and sets arg ring page equal to mapped page.
> + * Returns 0 on success and -1 on error.
> + */
> +int xc_mem_paging_enable(xc_interface *xch, domid_t domain_id,
> + uint32_t *port, void *ring_page,
> + mem_event_back_ring_t *back_ring);
> +/*
> + * Disables mem_paging and unmaps ring page.
> + * Returns 0 on success and -1 on error.
> + */
> +int xc_mem_paging_disable(xc_interface *xch, domid_t domain_id, void
> *ring_page);
> int xc_mem_paging_nominate(xc_interface *xch, domid_t domain_id,
> unsigned long gfn);
> int xc_mem_paging_evict(xc_interface *xch, domid_t domain_id, unsigned
> long gfn);
> diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
> index 82c1ee4..4a841bf 100644
> --- a/tools/xenpaging/xenpaging.c
> +++ b/tools/xenpaging/xenpaging.c
> @@ -337,40 +337,12 @@ static struct xenpaging *xenpaging_init(int argc,
> char *argv[])
> PERROR("Could not bind to xenpaging watch\n");
> goto err;
> }
> -
> - /* Map the ring page */
> - xc_get_hvm_param(xch, paging->mem_event.domain_id,
> - HVM_PARAM_PAGING_RING_PFN, &ring_pfn);
> - mmap_pfn = ring_pfn;
> - paging->mem_event.ring_page =
> - xc_map_foreign_batch(xch, paging->mem_event.domain_id,
> - PROT_READ | PROT_WRITE, &mmap_pfn, 1);
> - if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
> - {
> - /* Map failed, populate ring page */
> - rc = xc_domain_populate_physmap_exact(paging->xc_handle,
> - paging->mem_event.domain_id,
> - 1, 0, 0, &ring_pfn);
> - if ( rc != 0 )
> - {
> - PERROR("Failed to populate ring gfn\n");
> - goto err;
> - }
> -
> - mmap_pfn = ring_pfn;
> - paging->mem_event.ring_page =
> - xc_map_foreign_batch(xch, paging->mem_event.domain_id,
> - PROT_READ | PROT_WRITE, &mmap_pfn, 1);
> - if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
> - {
> - PERROR("Could not map the ring page\n");
> - goto err;
> - }
> - }
>
> - /* Initialise Xen */
> + /* Enable mem paging and initialize shared ring to communicate with
> xen. */
> rc = xc_mem_paging_enable(xch, paging->mem_event.domain_id,
> - &paging->mem_event.evtchn_port);
> + &paging->mem_event.evtchn_port,
> + paging->mem_event.ring_page,
> + &paging->mem_event.back_ring);
> if ( rc != 0 )
> {
> switch ( errno ) {
> @@ -413,17 +385,6 @@ static struct xenpaging *xenpaging_init(int argc,
> char *argv[])
>
> paging->mem_event.port = rc;
>
> - /* Initialise ring */
> - SHARED_RING_INIT((mem_event_sring_t *)paging->mem_event.ring_page);
> - BACK_RING_INIT(&paging->mem_event.back_ring,
> - (mem_event_sring_t *)paging->mem_event.ring_page,
> - PAGE_SIZE);
> -
> - /* Now that the ring is set, remove it from the guest's physmap */
> - if ( xc_domain_decrease_reservation_exact(xch,
> - paging->mem_event.domain_id, 1, 0, &ring_pfn) )
> - PERROR("Failed to remove ring from guest physmap");
> -
> /* Get max_pages from guest if not provided via cmdline */
> if ( !paging->max_pages )
> {
> @@ -523,9 +484,10 @@ static void xenpaging_teardown(struct xenpaging
> *paging)
> xs_unwatch(paging->xs_handle, "@releaseDomain", watch_token);
>
> paging->xc_handle = NULL;
> +
> /* Tear down domain paging in Xen */
> - munmap(paging->mem_event.ring_page, PAGE_SIZE);
> - rc = xc_mem_paging_disable(xch, paging->mem_event.domain_id);
> + rc = xc_mem_paging_disable(xch, paging->mem_event.domain_id,
> + paging->mem_event.ring_page);
> if ( rc != 0 )
> {
> PERROR("Error tearing down domain paging in xen");
> --
> 1.9.1
>
>
[-- Attachment #1.2: Type: text/html, Size: 9488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] mem_access: modifications to mem_event enable API.
2014-08-25 15:45 ` [PATCH v2 1/3] mem_access: modifications to mem_event enable API Andres Lagar Cavilla
@ 2014-08-25 22:38 ` Aravindh Puthiyaparambil (aravindp)
2014-09-04 22:40 ` Dushyant Behl
1 sibling, 0 replies; 10+ messages in thread
From: Aravindh Puthiyaparambil (aravindp) @ 2014-08-25 22:38 UTC (permalink / raw)
To: Andres Lagar Cavilla, Dushyant Behl
Cc: Ian Campbell, Stefano Stabellini, Ian Jackson, David Scott, Xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 4698 bytes --]
On Sun, Aug 24, 2014 at 5:26 PM, Dushyant Behl <myselfdushyantbehl@gmail.com<mailto:myselfdushyantbehl@gmail.com>> 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@gmail.com<mailto:myselfdushyantbehl@gmail.com>>
---
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
[-- Attachment #1.2: Type: text/html, Size: 10694 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] mem_event: Added new helper API to teardown mem event setup and unmap ring_page.
2014-08-25 0:26 ` [PATCH v2 2/3] mem_event: Added new helper API to teardown mem event setup and unmap ring_page Dushyant Behl
2014-08-25 15:47 ` Andres Lagar Cavilla
@ 2014-08-25 22:53 ` Aravindh Puthiyaparambil (aravindp)
2014-09-04 22:42 ` Dushyant Behl
1 sibling, 1 reply; 10+ messages in thread
From: Aravindh Puthiyaparambil (aravindp) @ 2014-08-25 22:53 UTC (permalink / raw)
To: Dushyant Behl, xen-devel
Cc: Andres Lagar Cavilla, Ian Campbell, Ian Jackson, David Scott,
Stefano Stabellini
>tools/libxc/xc_mem_event.c: Added new generic API to teardown mem
>event setup,
>the API supports hvm params PAGING, ACCESS and SHARING and also
>completes the
>obvious job of unmapping ring_page.
>
>tools/libxc/xc_mem_access.c: Modified mem_event_disable to use the new
>teardown API.
>
>tools/tests/xen-access/: Updated code to use the new API's.
>
>Signed-off-by: Dushyant Behl <myselfdushyantbehl@gmail.com>
>---
> tools/libxc/xc_mem_access.c | 9 +++----
> tools/libxc/xc_mem_event.c | 53
>+++++++++++++++++++++++++++++++++++++
> tools/libxc/xc_private.h | 8 ++++++
> tools/libxc/xenctrl.h | 5 ++--
> tools/tests/xen-access/xen-access.c | 6 ++---
> 5 files changed, 69 insertions(+), 12 deletions(-)
>
>diff --git a/tools/libxc/xc_mem_access.c b/tools/libxc/xc_mem_access.c
>index 89050be..29835c3 100644
>--- a/tools/libxc/xc_mem_access.c
>+++ b/tools/libxc/xc_mem_access.c
>@@ -33,12 +33,11 @@ int xc_mem_access_enable(xc_interface *xch,
>domid_t domain_id,
> port, ring_page, back_ring);
> }
>
>-int xc_mem_access_disable(xc_interface *xch, domid_t domain_id)
>+int xc_mem_access_disable(xc_interface *xch, domid_t domain_id, void
>*ring_page)
> {
>- return xc_mem_event_control(xch, domain_id,
>- XEN_DOMCTL_MEM_EVENT_OP_ACCESS_DISABLE,
>- XEN_DOMCTL_MEM_EVENT_OP_ACCESS,
>- NULL);
>+ return xc_mem_event_disable(xch, domain_id,
>+ HVM_PARAM_ACCESS_RING_PFN,
>+ ring_page);
> }
>
> int xc_mem_access_resume(xc_interface *xch, domid_t domain_id)
>diff --git a/tools/libxc/xc_mem_event.c b/tools/libxc/xc_mem_event.c
>index cdbeca8..b6ae7d0 100644
>--- a/tools/libxc/xc_mem_event.c
>+++ b/tools/libxc/xc_mem_event.c
>@@ -200,3 +200,56 @@ int xc_mem_event_enable(xc_interface *xch,
>domid_t domain_id, int param,
>
> return rc1;
> }
>+
>+/*
>+ * Disable mem_event.
>+ * Returns 0 on success, if failure returns -1 with errno properly set.
>+ * param can be HVM_PARAM_PAGING/ACCESS/SHARING_RING_PFN
>+ */
>+int xc_mem_event_disable(xc_interface *xch, domid_t domain_id,
>+ int param, void *ring_page)
>+{
>+ int rc;
>+ unsigned int op, mode;
>+
>+ switch ( param )
>+ {
>+ case HVM_PARAM_PAGING_RING_PFN:
>+ op = XEN_DOMCTL_MEM_EVENT_OP_PAGING_DISABLE;
>+ mode = XEN_DOMCTL_MEM_EVENT_OP_PAGING;
>+ break;
>+
>+ case HVM_PARAM_ACCESS_RING_PFN:
>+ op = XEN_DOMCTL_MEM_EVENT_OP_ACCESS_DISABLE;
>+ mode = XEN_DOMCTL_MEM_EVENT_OP_ACCESS;
>+ break;
>+
>+ case HVM_PARAM_SHARING_RING_PFN:
>+ op = XEN_DOMCTL_MEM_EVENT_OP_SHARING_DISABLE;
>+ mode = XEN_DOMCTL_MEM_EVENT_OP_SHARING;
>+ break;
>+
>+ /*
>+ * This is for the outside chance that the HVM_PARAM is valid but is
>invalid
>+ * as far as mem_event goes.
>+ */
>+ default:
>+ errno = EINVAL;
>+ rc = -1;
>+ goto out;
>+ }
Sorry, I should have caught this in my previous review. I think the Xen coding style places the case statement with the same indent as the switch.
Thanks,
Aravindh
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] mem_access: modifications to mem_event enable API.
2014-08-25 15:45 ` [PATCH v2 1/3] mem_access: modifications to mem_event enable API Andres Lagar Cavilla
2014-08-25 22:38 ` Aravindh Puthiyaparambil (aravindp)
@ 2014-09-04 22:40 ` Dushyant Behl
1 sibling, 0 replies; 10+ messages in thread
From: Dushyant Behl @ 2014-09-04 22:40 UTC (permalink / raw)
To: Andres Lagar Cavilla
Cc: Ian Campbell, Stefano Stabellini, Ian Jackson, Xen-devel,
Aravindh Puthiyaparambil (aravindp),
David Scott
[-- Attachment #1.1: Type: text/plain, Size: 12685 bytes --]
On Mon, Aug 25, 2014 at 9:15 PM, Andres Lagar Cavilla <
andres.lagarcavilla@gmail.com> wrote:
> On Sun, Aug 24, 2014 at 5:26 PM, Dushyant Behl <
> myselfdushyantbehl@gmail.com> 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@gmail.com>
>> ---
>> 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?
>
Yeah i also think the idea is correct to have this void **.
>
>
>> + 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.
>
>
>> mmap_pfn = pfn;
>> - ring_page = xc_map_foreign_batch(xch, domain_id, PROT_READ |
>> PROT_WRITE,
>> - &mmap_pfn, 1);
>> - if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
>> + ring_page = xc_map_foreign_bulk(xch, domain_id,
>> + PROT_READ | PROT_WRITE, &mmap_pfn,
>> &err, 1);
>> + if ( err != 0 || ring_page == NULL )
>
> {
>> /* Map failed, populate ring page */
>> rc1 = xc_domain_populate_physmap_exact(xch, domain_id, 1, 0, 0,
>> @@ -103,15 +112,23 @@ void *xc_mem_event_enable(xc_interface *xch,
>> domid_t domain_id, int param,
>> }
>>
>> mmap_pfn = ring_pfn;
>> - ring_page = xc_map_foreign_batch(xch, domain_id, PROT_READ |
>> PROT_WRITE,
>> - &mmap_pfn, 1);
>> - if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
>> + ring_page = xc_map_foreign_bulk(xch, domain_id, PROT_READ |
>> PROT_WRITE,
>> + &mmap_pfn, &err, 1);
>> + if ( err != 0 || ring_page == NULL )
>> {
>> PERROR("Could not map the ring page\n");
>> + rc1 = -1;
>>
>
> If the convention will be to set errno (Ians?), then set errno here
> errno = (err) ? : EINVAL;
> or similar
>
Yes, actually I missed it, errno should be set here.
I'll try to do an arm build with these changes and then send the next
version accordingly.
Thanks and Regards,
Dushyant Behl
>
>
>> goto out;
>> }
>> }
>>
>> + /* Clear the ring page */
>> + memset(ring_page, 0, PAGE_SIZE);
>> +
>> + /* Initialise ring */
>> + SHARED_RING_INIT((mem_event_sring_t *)ring_page);
>> + BACK_RING_INIT(back_ring, (mem_event_sring_t *)ring_page, PAGE_SIZE);
>> +
>> switch ( param )
>> {
>> case HVM_PARAM_PAGING_RING_PFN:
>> @@ -149,7 +166,12 @@ void *xc_mem_event_enable(xc_interface *xch, domid_t
>> domain_id, int param,
>> /* Remove the ring_pfn from the guest's physmap */
>> rc1 = xc_domain_decrease_reservation_exact(xch, domain_id, 1, 0,
>> &ring_pfn);
>> if ( rc1 != 0 )
>> + {
>> PERROR("Failed to remove ring page from guest physmap");
>> + goto out;
>> + }
>> +
>> + rc1 = 0;
>>
>> out:
>> saved_errno = errno;
>> @@ -165,11 +187,16 @@ void *xc_mem_event_enable(xc_interface *xch,
>> domid_t domain_id, int param,
>> }
>>
>> if ( ring_page )
>> - munmap(ring_page, XC_PAGE_SIZE);
>> + {
>> + rc2 = munmap(ring_page, XC_PAGE_SIZE);
>> + if ( rc2 < 0 )
>> + PERROR("Error while munmap of ring_page");
>> + }
>> ring_page = NULL;
>>
>> errno = saved_errno;
>> + rc1 = -1;
>> }
>>
>> - return ring_page;
>> + return rc1;
>> }
>> diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
>> index c50a7c9..3d455e7 100644
>> --- a/tools/libxc/xc_private.h
>> +++ b/tools/libxc/xc_private.h
>> @@ -32,6 +32,7 @@
>> #include "xenctrl.h"
>> #include "xenctrlosdep.h"
>>
>> +#include <xen/mem_event.h>
>> #include <xen/sys/privcmd.h>
>>
>> #if defined(HAVE_VALGRIND_MEMCHECK_H) && !defined(NDEBUG) &&
>> !defined(__MINIOS__)
>> @@ -377,10 +378,13 @@ int xc_mem_event_memop(xc_interface *xch, domid_t
>> domain_id,
>> unsigned int op, unsigned int mode,
>> uint64_t gfn, void *buffer);
>> /*
>> - * Enables mem_event and returns the mapped ring page indicated by param.
>> + * Enables mem_event and initializes shared ring to communicate with
>> hypervisor
>> + * and sets ring_page equal to mapped page.
>>
>
> yeah, ring_page should be void **
>
>
>> + * Returns 0 if success and if failure returns -1 with errno properly
>> set.
>> * param can be HVM_PARAM_PAGING/ACCESS/SHARING_RING_PFN
>> */
>> -void *xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int
>> param,
>> - uint32_t *port);
>> +int xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
>> + uint32_t *port, void *ring_page,
>> + mem_event_back_ring_t *back_ring);
>>
>> #endif /* __XC_PRIVATE_H__ */
>> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
>> index 1c5d0db..9d043d7 100644
>> --- a/tools/libxc/xenctrl.h
>> +++ b/tools/libxc/xenctrl.h
>> @@ -47,6 +47,7 @@
>> #include <xen/xsm/flask_op.h>
>> #include <xen/tmem.h>
>> #include <xen/kexec.h>
>> +#include <xen/mem_event.h>
>>
>> #include "xentoollog.h"
>>
>> @@ -2258,11 +2259,13 @@ int xc_mem_paging_load(xc_interface *xch, domid_t
>> domain_id,
>> */
>>
>> /*
>> - * Enables mem_access and returns the mapped ring page.
>> - * Will return NULL on error.
>> + * Enables mem_access and sets arg ring_page equal to mapped page.
>> + * Will return 0 on success and -1 on error.
>> * Caller has to unmap this page when done.
>> */
>> -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);
>> int xc_mem_access_disable(xc_interface *xch, domid_t domain_id);
>> int xc_mem_access_resume(xc_interface *xch, domid_t domain_id);
>>
>> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
>> b/tools/ocaml/libs/xc/xenctrl_stubs.c
>> index f0810eb..beccb38 100644
>> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
>> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
>> @@ -526,12 +526,12 @@ CAMLprim value stub_xc_evtchn_reset(value xch,
>> value domid)
>> }
>>
>>
>> -#define RING_SIZE 32768
>> -static char ring[RING_SIZE];
>> +#define BUF_RING_SIZE 32768
>> +static char ring[BUF_RING_SIZE];
>>
>> CAMLprim value stub_xc_readconsolering(value xch)
>> {
>> - unsigned int size = RING_SIZE - 1;
>> + unsigned int size = BUF_RING_SIZE - 1;
>> char *ring_ptr = ring;
>> int retval;
>>
>> diff --git a/tools/tests/xen-access/xen-access.c
>> b/tools/tests/xen-access/xen-access.c
>> index 090df5f..9242f86 100644
>> --- a/tools/tests/xen-access/xen-access.c
>> +++ b/tools/tests/xen-access/xen-access.c
>> @@ -245,11 +245,12 @@ xenaccess_t *xenaccess_init(xc_interface **xch_r,
>> domid_t domain_id)
>> mem_event_ring_lock_init(&xenaccess->mem_event);
>>
>> /* Enable mem_access */
>> - xenaccess->mem_event.ring_page =
>> - xc_mem_access_enable(xenaccess->xc_handle,
>> - xenaccess->mem_event.domain_id,
>> - &xenaccess->mem_event.evtchn_port);
>> - if ( xenaccess->mem_event.ring_page == NULL )
>> + rc = xc_mem_access_enable(xenaccess->xc_handle,
>> + xenaccess->mem_event.domain_id,
>> + &xenaccess->mem_event.evtchn_port,
>> + xenaccess->mem_event.ring_page,
>> + &xenaccess->mem_event.back_ring);
>> + if ( rc < 0 )
>> {
>> switch ( errno ) {
>> case EBUSY:
>> @@ -287,12 +288,6 @@ xenaccess_t *xenaccess_init(xc_interface **xch_r,
>> domid_t domain_id)
>> evtchn_bind = 1;
>> xenaccess->mem_event.port = rc;
>>
>> - /* Initialise ring */
>> - SHARED_RING_INIT((mem_event_sring_t
>> *)xenaccess->mem_event.ring_page);
>> - BACK_RING_INIT(&xenaccess->mem_event.back_ring,
>> - (mem_event_sring_t *)xenaccess->mem_event.ring_page,
>> - XC_PAGE_SIZE);
>> -
>> /* Get domaininfo */
>> xenaccess->domain_info = malloc(sizeof(xc_domaininfo_t));
>> if ( xenaccess->domain_info == NULL )
>> --
>> 1.9.1
>>
>>
>
[-- Attachment #1.2: Type: text/html, Size: 16649 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] mem_event: Added new helper API to teardown mem event setup and unmap ring_page.
2014-08-25 22:53 ` Aravindh Puthiyaparambil (aravindp)
@ 2014-09-04 22:42 ` Dushyant Behl
0 siblings, 0 replies; 10+ messages in thread
From: Dushyant Behl @ 2014-09-04 22:42 UTC (permalink / raw)
To: Aravindh Puthiyaparambil (aravindp)
Cc: Ian Campbell, Stefano Stabellini, Ian Jackson, xen-devel,
Andres Lagar Cavilla, David Scott
[-- Attachment #1.1: Type: text/plain, Size: 3641 bytes --]
On Tue, Aug 26, 2014 at 4:23 AM, Aravindh Puthiyaparambil (aravindp) <
aravindp@cisco.com> wrote:
> >tools/libxc/xc_mem_event.c: Added new generic API to teardown mem
> >event setup,
> >the API supports hvm params PAGING, ACCESS and SHARING and also
> >completes the
> >obvious job of unmapping ring_page.
> >
> >tools/libxc/xc_mem_access.c: Modified mem_event_disable to use the new
> >teardown API.
> >
> >tools/tests/xen-access/: Updated code to use the new API's.
> >
> >Signed-off-by: Dushyant Behl <myselfdushyantbehl@gmail.com>
> >---
> > tools/libxc/xc_mem_access.c | 9 +++----
> > tools/libxc/xc_mem_event.c | 53
> >+++++++++++++++++++++++++++++++++++++
> > tools/libxc/xc_private.h | 8 ++++++
> > tools/libxc/xenctrl.h | 5 ++--
> > tools/tests/xen-access/xen-access.c | 6 ++---
> > 5 files changed, 69 insertions(+), 12 deletions(-)
> >
> >diff --git a/tools/libxc/xc_mem_access.c b/tools/libxc/xc_mem_access.c
> >index 89050be..29835c3 100644
> >--- a/tools/libxc/xc_mem_access.c
> >+++ b/tools/libxc/xc_mem_access.c
> >@@ -33,12 +33,11 @@ int xc_mem_access_enable(xc_interface *xch,
> >domid_t domain_id,
> > port, ring_page, back_ring);
> > }
> >
> >-int xc_mem_access_disable(xc_interface *xch, domid_t domain_id)
> >+int xc_mem_access_disable(xc_interface *xch, domid_t domain_id, void
> >*ring_page)
> > {
> >- return xc_mem_event_control(xch, domain_id,
> >- XEN_DOMCTL_MEM_EVENT_OP_ACCESS_DISABLE,
> >- XEN_DOMCTL_MEM_EVENT_OP_ACCESS,
> >- NULL);
> >+ return xc_mem_event_disable(xch, domain_id,
> >+ HVM_PARAM_ACCESS_RING_PFN,
> >+ ring_page);
> > }
> >
> > int xc_mem_access_resume(xc_interface *xch, domid_t domain_id)
> >diff --git a/tools/libxc/xc_mem_event.c b/tools/libxc/xc_mem_event.c
> >index cdbeca8..b6ae7d0 100644
> >--- a/tools/libxc/xc_mem_event.c
> >+++ b/tools/libxc/xc_mem_event.c
> >@@ -200,3 +200,56 @@ int xc_mem_event_enable(xc_interface *xch,
> >domid_t domain_id, int param,
> >
> > return rc1;
> > }
> >+
> >+/*
> >+ * Disable mem_event.
> >+ * Returns 0 on success, if failure returns -1 with errno properly set.
> >+ * param can be HVM_PARAM_PAGING/ACCESS/SHARING_RING_PFN
> >+ */
> >+int xc_mem_event_disable(xc_interface *xch, domid_t domain_id,
> >+ int param, void *ring_page)
> >+{
> >+ int rc;
> >+ unsigned int op, mode;
> >+
> >+ switch ( param )
> >+ {
> >+ case HVM_PARAM_PAGING_RING_PFN:
> >+ op = XEN_DOMCTL_MEM_EVENT_OP_PAGING_DISABLE;
> >+ mode = XEN_DOMCTL_MEM_EVENT_OP_PAGING;
> >+ break;
> >+
> >+ case HVM_PARAM_ACCESS_RING_PFN:
> >+ op = XEN_DOMCTL_MEM_EVENT_OP_ACCESS_DISABLE;
> >+ mode = XEN_DOMCTL_MEM_EVENT_OP_ACCESS;
> >+ break;
> >+
> >+ case HVM_PARAM_SHARING_RING_PFN:
> >+ op = XEN_DOMCTL_MEM_EVENT_OP_SHARING_DISABLE;
> >+ mode = XEN_DOMCTL_MEM_EVENT_OP_SHARING;
> >+ break;
> >+
> >+ /*
> >+ * This is for the outside chance that the HVM_PARAM is valid
> but is
> >invalid
> >+ * as far as mem_event goes.
> >+ */
> >+ default:
> >+ errno = EINVAL;
> >+ rc = -1;
> >+ goto out;
> >+ }
>
> Sorry, I should have caught this in my previous review. I think the Xen
> coding style places the case statement with the same indent as the switch.
>
No problem, I'll update the patch.
Thanks,
Dushyant
[-- Attachment #1.2: Type: text/html, Size: 4905 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-09-04 22:42 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-25 0:26 [PATCH v2 1/3] mem_access: modifications to mem_event enable API Dushyant Behl
2014-08-25 0:26 ` [PATCH v2 2/3] mem_event: Added new helper API to teardown mem event setup and unmap ring_page Dushyant Behl
2014-08-25 15:47 ` Andres Lagar Cavilla
2014-08-25 22:53 ` Aravindh Puthiyaparambil (aravindp)
2014-09-04 22:42 ` Dushyant Behl
2014-08-25 0:26 ` [PATCH v2 3/3] xenpaging: updated code to use safer mem_event API's for setup and teardown Dushyant Behl
2014-08-25 15:49 ` Andres Lagar Cavilla
2014-08-25 15:45 ` [PATCH v2 1/3] mem_access: modifications to mem_event enable API Andres Lagar Cavilla
2014-08-25 22:38 ` Aravindh Puthiyaparambil (aravindp)
2014-09-04 22:40 ` Dushyant Behl
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.