* [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
* 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 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@lists.xen.org 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 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@lists.xen.org, 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
* [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 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 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 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 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).