diff mbox series

[2/9] vm_event: Define VM_EVENT type

Message ID 9cde4926b56fa05afffee270e5e28a3b9bd830d9.1559224640.git.ppircalabu@bitdefender.com (mailing list archive)
State Superseded
Headers show
Series Per vcpu vm_event channels | expand

Commit Message

Petre Ovidiu PIRCALABU May 30, 2019, 2:18 p.m. UTC
Define the type for each of the supported vm_event rings (paging,
monitor and sharing) and replace the ring param field with this type.

Replace XEN_DOMCTL_VM_EVENT_OP_ occurrences with their corresponding
XEN_VM_EVENT_TYPE_ counterpart.

Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
---
 tools/libxc/include/xenctrl.h |  1 +
 tools/libxc/xc_mem_paging.c   |  6 ++--
 tools/libxc/xc_monitor.c      |  6 ++--
 tools/libxc/xc_private.h      |  8 ++---
 tools/libxc/xc_vm_event.c     | 70 ++++++++++++++++++-------------------
 xen/common/vm_event.c         | 12 +++----
 xen/include/public/domctl.h   | 81 ++++++-------------------------------------
 xen/include/public/vm_event.h | 31 +++++++++++++++++
 8 files changed, 93 insertions(+), 122 deletions(-)

Comments

Andrew Cooper May 31, 2019, 11:26 p.m. UTC | #1
On 30/05/2019 07:18, Petre Pircalabu wrote:
> diff --git a/tools/libxc/xc_vm_event.c b/tools/libxc/xc_vm_event.c
> index ea10366..3b1018b 100644
> --- a/tools/libxc/xc_vm_event.c
> +++ b/tools/libxc/xc_vm_event.c
> @@ -23,29 +23,54 @@
>  #include "xc_private.h"
>  
>  int xc_vm_event_control(xc_interface *xch, uint32_t domain_id, unsigned int op,
> -                        unsigned int mode)
> +                        unsigned int type)
>  {
>      DECLARE_DOMCTL;
>  
>      domctl.cmd = XEN_DOMCTL_vm_event_op;
>      domctl.domain = domain_id;
>      domctl.u.vm_event_op.op = op;
> -    domctl.u.vm_event_op.mode = mode;
> +    domctl.u.vm_event_op.type = type;
>  
>      return do_domctl(xch, &domctl);
>  }
>  
> -void *xc_vm_event_enable(xc_interface *xch, uint32_t domain_id, int param,
> +static int xc_vm_event_ring_pfn_param(int type, int *param)
> +{
> +    if ( !param )
> +        return -EINVAL;
> +
> +    switch ( type )
> +    {
> +    case XEN_VM_EVENT_TYPE_PAGING:
> +        *param = HVM_PARAM_PAGING_RING_PFN;
> +        break;
> +
> +    case XEN_VM_EVENT_TYPE_MONITOR:
> +        *param = HVM_PARAM_MONITOR_RING_PFN;
> +        break;
> +
> +    case XEN_VM_EVENT_TYPE_SHARING:
> +        *param = HVM_PARAM_SHARING_RING_PFN;
> +        break;
> +
> +    default:
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}

This is an internal helper, so can reasonably be expected to not be
called with junk, and can do away with the param pointer.

Something like

static int xc_vm_event_ring_pfn_param(unsigned int type)
{
    switch ( type )
    {
        case XEN_VM_EVENT_TYPE_PAGING:
            return HVM_PARAM_PAGING_RING_PFN;
...
        default:
            return -EINVAL;
    }
}

will work fine because HVM_PARAM_* are all tiny unsigned integers in
practice.  It also has a more sensible API for the caller.

> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 19486d5..19281fa 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -769,80 +769,18 @@ struct xen_domctl_gdbsx_domstatus {
>   * VM event operations
>   */
>  
> -/* XEN_DOMCTL_vm_event_op */
> -
> -/*
> - * There are currently three rings available for VM events:
> - * sharing, monitor and paging. This hypercall allows one to
> - * control these rings (enable/disable), as well as to signal
> - * to the hypervisor to pull responses (resume) from the given
> - * ring.
> +/* XEN_DOMCTL_vm_event_op.

/*
 * XEN_DOMCTL_vm_event_op.
 *

please, seeing as you're adjusting the comment.

> + * Use for teardown/setup of helper<->hypervisor interface for paging,
> + * access and sharing.
>   */
>  #define XEN_VM_EVENT_ENABLE               0
>  #define XEN_VM_EVENT_DISABLE              1
>  #define XEN_VM_EVENT_RESUME               2
>  #define XEN_VM_EVENT_GET_VERSION          3
>  
> -/*
> - * Domain memory paging
> - * Page memory in and out.
> - * Domctl interface to set up and tear down the
> - * pager<->hypervisor interface. Use XENMEM_paging_op*
> - * to perform per-page operations.
> - *
> - * The XEN_VM_EVENT_PAGING_ENABLE domctl returns several
> - * non-standard error codes to indicate why paging could not be enabled:
> - * ENODEV - host lacks HAP support (EPT/NPT) or HAP is disabled in guest
> - * EMLINK - guest has iommu passthrough enabled
> - * EXDEV  - guest has PoD enabled
> - * EBUSY  - guest has or had paging enabled, ring buffer still active
> - */
> -#define XEN_DOMCTL_VM_EVENT_OP_PAGING            1
> -
> -/*
> - * Monitor helper.
> - *
> - * As with paging, use the domctl for teardown/setup of the
> - * helper<->hypervisor interface.
> - *
> - * The monitor interface can be used to register for various VM events. For
> - * example, there are HVM hypercalls to set the per-page access permissions
> - * of every page in a domain.  When one of these permissions--independent,
> - * read, write, and execute--is violated, the VCPU is paused and a memory event
> - * is sent with what happened. The memory event handler can then resume the
> - * VCPU and redo the access with a XEN_VM_EVENT_RESUME option.
> - *
> - * See public/vm_event.h for the list of available events that can be
> - * subscribed to via the monitor interface.
> - *
> - * The XEN_VM_EVENT_MONITOR_* domctls returns
> - * non-standard error codes to indicate why access could not be enabled:
> - * ENODEV - host lacks HAP support (EPT/NPT) or HAP is disabled in guest
> - * EBUSY  - guest has or had access enabled, ring buffer still active
> - *
> - */
> -#define XEN_DOMCTL_VM_EVENT_OP_MONITOR           2
> -
> -/*
> - * Sharing ENOMEM helper.
> - *
> - * As with paging, use the domctl for teardown/setup of the
> - * helper<->hypervisor interface.
> - *
> - * If setup, this ring is used to communicate failed allocations
> - * in the unshare path. XENMEM_sharing_op_resume is used to wake up
> - * vcpus that could not unshare.
> - *
> - * Note that shring can be turned on (as per the domctl below)
> - * *without* this ring being setup.
> - */
> -#define XEN_DOMCTL_VM_EVENT_OP_SHARING           3
> -
> -/* Use for teardown/setup of helper<->hypervisor interface for paging,
> - * access and sharing.*/
>  struct xen_domctl_vm_event_op {
> -    uint32_t       op;           /* XEN_VM_EVENT_* */
> -    uint32_t       mode;         /* XEN_DOMCTL_VM_EVENT_OP_* */
> +    uint32_t        op;           /* XEN_VM_EVENT_* */
> +    uint32_t        type;         /* XEN_VM_EVENT_TYPE_* */

Why did the vertical alignment change?

>  
>      union {
>          struct {
> @@ -857,7 +795,10 @@ struct xen_domctl_vm_event_op {
>   * Memory sharing operations
>   */
>  /* XEN_DOMCTL_mem_sharing_op.
> - * The CONTROL sub-domctl is used for bringup/teardown. */
> + * The CONTROL sub-domctl is used for bringup/teardown.
> + * Please note that mem sharing can be turned on *without* setting-up the
> + * correspondin ring
> + */

As a tangent, it can? how?  (I'm entirely prepared to believe that this
is how the code currently works, but I can't see how such a setup would
plausibly work.)

~Andrew
Jan Beulich June 3, 2019, 3:51 p.m. UTC | #2
>>> On 30.05.19 at 16:18, <ppircalabu@bitdefender.com> wrote:
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -38,7 +38,7 @@
>  #include "hvm/save.h"
>  #include "memory.h"
>  
> -#define XEN_DOMCTL_INTERFACE_VERSION 0x00000011
> +#define XEN_DOMCTL_INTERFACE_VERSION 0x00000012

I don't think such a bump is needed - afaict the interface remains
binary compatible.

Jan
Tamas K Lengyel June 3, 2019, 10:22 p.m. UTC | #3
> >  /* XEN_DOMCTL_mem_sharing_op.
> > - * The CONTROL sub-domctl is used for bringup/teardown. */
> > + * The CONTROL sub-domctl is used for bringup/teardown.
> > + * Please note that mem sharing can be turned on *without* setting-up the
> > + * correspondin ring
> > + */
>
> As a tangent, it can? how?  (I'm entirely prepared to believe that this
> is how the code currently works, but I can't see how such a setup would
> plausibly work.)

The vm_event ring for mem_sharing is only used to communicate
out-of-memory condition to an external listener. I think it's only
useful for logging since the listener wouldn't really be in a position
to try to "make space" for the faulting domain and it would get
destroyed after the message is sent. In any case, there isn't any
documentation for how it was intended to be used so this is just my
guess.

Tamas
Tamas K Lengyel June 3, 2019, 10:26 p.m. UTC | #4
On Mon, Jun 3, 2019 at 4:22 PM Tamas K Lengyel <tamas@tklengyel.com> wrote:
>
> > >  /* XEN_DOMCTL_mem_sharing_op.
> > > - * The CONTROL sub-domctl is used for bringup/teardown. */
> > > + * The CONTROL sub-domctl is used for bringup/teardown.
> > > + * Please note that mem sharing can be turned on *without* setting-up the
> > > + * correspondin ring
> > > + */
> >
> > As a tangent, it can? how?  (I'm entirely prepared to believe that this
> > is how the code currently works, but I can't see how such a setup would
> > plausibly work.)
>
> The vm_event ring for mem_sharing is only used to communicate
> out-of-memory condition to an external listener. I think it's only
> useful for logging since the listener wouldn't really be in a position
> to try to "make space" for the faulting domain and it would get
> destroyed after the message is sent. In any case, there isn't any
> documentation for how it was intended to be used so this is just my
> guess.

Actually, it seems the listener was intended to be able to try to
"make space" for the domain. How exactly is not clear but anyway, the
domain would get paused if there is a listener instead of just being
destroyed when there is an ENOMEM error while trying to deduplicate
shared pages.

Tamas
Petre Ovidiu PIRCALABU June 4, 2019, 10:12 a.m. UTC | #5
On Fri, 2019-05-31 at 16:26 -0700, Andrew Cooper wrote:
> On 30/05/2019 07:18, Petre Pircalabu wrote:
> > 
> 
> This is an internal helper, so can reasonably be expected to not be
> called with junk, and can do away with the param pointer.
> 
> Something like
> 
> static int xc_vm_event_ring_pfn_param(unsigned int type)
> {
>     switch ( type )
>     {
>         case XEN_VM_EVENT_TYPE_PAGING:
>             return HVM_PARAM_PAGING_RING_PFN;
> ...
>         default:
>             return -EINVAL;
>     }
> }
> 
> will work fine because HVM_PARAM_* are all tiny unsigned integers in
> practice.  It also has a more sensible API for the caller.

I think in the end it's best just to move the helper to
xc_vm_event_enable (inline the switch) as it is not use outside it.

> 
> > index 19486d5..19281fa 100644
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -769,80 +769,18 @@ struct xen_domctl_gdbsx_domstatus {
> >   * VM event operations
> >   */
> >  
> > 
> > -
> > -/* Use for teardown/setup of helper<->hypervisor interface for
> > paging,
> > - * access and sharing.*/
> >  struct xen_domctl_vm_event_op {
> > -    uint32_t       op;           /* XEN_VM_EVENT_* */
> > -    uint32_t       mode;         /* XEN_DOMCTL_VM_EVENT_OP_* */
> > +    uint32_t        op;           /* XEN_VM_EVENT_* */
> > +    uint32_t        type;         /* XEN_VM_EVENT_TYPE_* */
> 
> Why did the vertical alignment change?

The initial vertical alignment was not 4 spaces (I will revert it back
to the way it was in order to simplify the review  )
>
Andrew Cooper June 4, 2019, 4:09 p.m. UTC | #6
On 03/06/2019 23:26, Tamas K Lengyel wrote:
> On Mon, Jun 3, 2019 at 4:22 PM Tamas K Lengyel <tamas@tklengyel.com> wrote:
>>>>  /* XEN_DOMCTL_mem_sharing_op.
>>>> - * The CONTROL sub-domctl is used for bringup/teardown. */
>>>> + * The CONTROL sub-domctl is used for bringup/teardown.
>>>> + * Please note that mem sharing can be turned on *without* setting-up the
>>>> + * correspondin ring
>>>> + */
>>> As a tangent, it can? how?  (I'm entirely prepared to believe that this
>>> is how the code currently works, but I can't see how such a setup would
>>> plausibly work.)
>> The vm_event ring for mem_sharing is only used to communicate
>> out-of-memory condition to an external listener. I think it's only
>> useful for logging since the listener wouldn't really be in a position
>> to try to "make space" for the faulting domain and it would get
>> destroyed after the message is sent. In any case, there isn't any
>> documentation for how it was intended to be used so this is just my
>> guess.
> Actually, it seems the listener was intended to be able to try to
> "make space" for the domain. How exactly is not clear but anyway, the
> domain would get paused if there is a listener instead of just being
> destroyed when there is an ENOMEM error while trying to deduplicate
> shared pages.

I can't say I'm surprised by this...

~Andrew
diff mbox series

Patch

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 28fdbc0..943b933 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -46,6 +46,7 @@ 
 #include <xen/xsm/flask_op.h>
 #include <xen/kexec.h>
 #include <xen/platform.h>
+#include <xen/vm_event.h>
 
 #include "xentoollog.h"
 
diff --git a/tools/libxc/xc_mem_paging.c b/tools/libxc/xc_mem_paging.c
index 08468fb..37a8224 100644
--- a/tools/libxc/xc_mem_paging.c
+++ b/tools/libxc/xc_mem_paging.c
@@ -41,7 +41,7 @@  void *xc_mem_paging_enable(xc_interface *xch, uint32_t domain_id,
                            uint32_t *port)
 {
     return xc_vm_event_enable(xch, domain_id,
-                              XEN_DOMCTL_VM_EVENT_OP_PAGING,
+                              XEN_VM_EVENT_TYPE_PAGING,
                               port);
 }
 
@@ -49,14 +49,14 @@  int xc_mem_paging_disable(xc_interface *xch, uint32_t domain_id)
 {
     return xc_vm_event_control(xch, domain_id,
                                XEN_VM_EVENT_DISABLE,
-                               XEN_DOMCTL_VM_EVENT_OP_PAGING);
+                               XEN_VM_EVENT_TYPE_PAGING);
 }
 
 int xc_mem_paging_resume(xc_interface *xch, uint32_t domain_id)
 {
     return xc_vm_event_control(xch, domain_id,
                                XEN_VM_EVENT_RESUME,
-                               XEN_DOMCTL_VM_EVENT_OP_PAGING);
+                               XEN_VM_EVENT_TYPE_PAGING);
 }
 
 int xc_mem_paging_nominate(xc_interface *xch, uint32_t domain_id, uint64_t gfn)
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index d190c29..718fe8b 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -35,7 +35,7 @@  void *xc_monitor_enable(xc_interface *xch, uint32_t domain_id, uint32_t *port)
     }
 
     buffer = xc_vm_event_enable(xch, domain_id,
-                                HVM_PARAM_MONITOR_RING_PFN,
+                                XEN_VM_EVENT_TYPE_MONITOR,
                                 port);
     saved_errno = errno;
     if ( xc_domain_unpause(xch, domain_id) )
@@ -53,14 +53,14 @@  int xc_monitor_disable(xc_interface *xch, uint32_t domain_id)
 {
     return xc_vm_event_control(xch, domain_id,
                                XEN_VM_EVENT_DISABLE,
-                               XEN_DOMCTL_VM_EVENT_OP_MONITOR);
+                               XEN_VM_EVENT_TYPE_MONITOR);
 }
 
 int xc_monitor_resume(xc_interface *xch, uint32_t domain_id)
 {
     return xc_vm_event_control(xch, domain_id,
                                XEN_VM_EVENT_RESUME,
-                               XEN_DOMCTL_VM_EVENT_OP_MONITOR);
+                               XEN_VM_EVENT_TYPE_MONITOR);
 }
 
 int xc_monitor_get_capabilities(xc_interface *xch, uint32_t domain_id,
diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
index 663e78b..482451c 100644
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -412,12 +412,12 @@  int xc_ffs64(uint64_t x);
  * vm_event operations. Internal use only.
  */
 int xc_vm_event_control(xc_interface *xch, uint32_t domain_id, unsigned int op,
-                        unsigned int mode);
+                        unsigned int type);
 /*
- * Enables vm_event and returns the mapped ring page indicated by param.
- * param can be HVM_PARAM_PAGING/ACCESS/SHARING_RING_PFN
+ * Enables vm_event and returns the mapped ring page indicated by type.
+ * type can be XEN_VM_EVENT_TYPE_(PAGING/MONITOR/SHARING)
  */
-void *xc_vm_event_enable(xc_interface *xch, uint32_t domain_id, int param,
+void *xc_vm_event_enable(xc_interface *xch, uint32_t domain_id, int type,
                          uint32_t *port);
 
 int do_dm_op(xc_interface *xch, uint32_t domid, unsigned int nr_bufs, ...);
diff --git a/tools/libxc/xc_vm_event.c b/tools/libxc/xc_vm_event.c
index ea10366..3b1018b 100644
--- a/tools/libxc/xc_vm_event.c
+++ b/tools/libxc/xc_vm_event.c
@@ -23,29 +23,54 @@ 
 #include "xc_private.h"
 
 int xc_vm_event_control(xc_interface *xch, uint32_t domain_id, unsigned int op,
-                        unsigned int mode)
+                        unsigned int type)
 {
     DECLARE_DOMCTL;
 
     domctl.cmd = XEN_DOMCTL_vm_event_op;
     domctl.domain = domain_id;
     domctl.u.vm_event_op.op = op;
-    domctl.u.vm_event_op.mode = mode;
+    domctl.u.vm_event_op.type = type;
 
     return do_domctl(xch, &domctl);
 }
 
-void *xc_vm_event_enable(xc_interface *xch, uint32_t domain_id, int param,
+static int xc_vm_event_ring_pfn_param(int type, int *param)
+{
+    if ( !param )
+        return -EINVAL;
+
+    switch ( type )
+    {
+    case XEN_VM_EVENT_TYPE_PAGING:
+        *param = HVM_PARAM_PAGING_RING_PFN;
+        break;
+
+    case XEN_VM_EVENT_TYPE_MONITOR:
+        *param = HVM_PARAM_MONITOR_RING_PFN;
+        break;
+
+    case XEN_VM_EVENT_TYPE_SHARING:
+        *param = HVM_PARAM_SHARING_RING_PFN;
+        break;
+
+    default:
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+void *xc_vm_event_enable(xc_interface *xch, uint32_t domain_id, int type,
                          uint32_t *port)
 {
     void *ring_page = NULL;
     uint64_t pfn;
     xen_pfn_t ring_pfn, mmap_pfn;
-    unsigned int op, mode;
-    int rc;
+    int param, rc;
     DECLARE_DOMCTL;
 
-    if ( !port )
+    if ( !port || xc_vm_event_ring_pfn_param(type, &param) != 0 )
     {
         errno = EINVAL;
         return NULL;
@@ -83,37 +108,10 @@  void *xc_vm_event_enable(xc_interface *xch, uint32_t domain_id, int param,
         goto out;
     }
 
-    switch ( param )
-    {
-    case HVM_PARAM_PAGING_RING_PFN:
-        op = XEN_VM_EVENT_ENABLE;
-        mode = XEN_DOMCTL_VM_EVENT_OP_PAGING;
-        break;
-
-    case HVM_PARAM_MONITOR_RING_PFN:
-        op = XEN_VM_EVENT_ENABLE;
-        mode = XEN_DOMCTL_VM_EVENT_OP_MONITOR;
-        break;
-
-    case HVM_PARAM_SHARING_RING_PFN:
-        op = XEN_VM_EVENT_ENABLE;
-        mode = XEN_DOMCTL_VM_EVENT_OP_SHARING;
-        break;
-
-    /*
-     * This is for the outside chance that the HVM_PARAM is valid but is invalid
-     * as far as vm_event goes.
-     */
-    default:
-        errno = EINVAL;
-        rc = -1;
-        goto out;
-    }
-
     domctl.cmd = XEN_DOMCTL_vm_event_op;
     domctl.domain = domain_id;
-    domctl.u.vm_event_op.op = op;
-    domctl.u.vm_event_op.mode = mode;
+    domctl.u.vm_event_op.op = XEN_VM_EVENT_ENABLE;
+    domctl.u.vm_event_op.type = type;
 
     rc = do_domctl(xch, &domctl);
     if ( rc != 0 )
@@ -148,7 +146,7 @@  int xc_vm_event_get_version(xc_interface *xch)
     domctl.cmd = XEN_DOMCTL_vm_event_op;
     domctl.domain = DOMID_INVALID;
     domctl.u.vm_event_op.op = XEN_VM_EVENT_GET_VERSION;
-    domctl.u.vm_event_op.mode = XEN_DOMCTL_VM_EVENT_OP_MONITOR;
+    domctl.u.vm_event_op.type = XEN_VM_EVENT_TYPE_MONITOR;
 
     rc = do_domctl(xch, &domctl);
     if ( !rc )
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 6833c21..d7c5f22 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -371,7 +371,7 @@  static int vm_event_resume(struct domain *d, struct vm_event_domain *ved)
     vm_event_response_t rsp;
 
     /*
-     * vm_event_resume() runs in either XEN_DOMCTL_VM_EVENT_OP_*, or
+     * vm_event_resume() runs in either XEN_VM_EVENT_* domctls, or
      * EVTCHN_send context from the introspection consumer. Both contexts
      * are guaranteed not to be the subject of vm_event responses.
      * While we could ASSERT(v != current) for each VCPU in d in the loop
@@ -597,7 +597,7 @@  int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
     if ( unlikely(d == NULL) )
         return -ESRCH;
 
-    rc = xsm_vm_event_control(XSM_PRIV, d, vec->mode, vec->op);
+    rc = xsm_vm_event_control(XSM_PRIV, d, vec->type, vec->op);
     if ( rc )
         return rc;
 
@@ -624,10 +624,10 @@  int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
 
     rc = -ENOSYS;
 
-    switch ( vec->mode )
+    switch ( vec->type )
     {
 #ifdef CONFIG_HAS_MEM_PAGING
-    case XEN_DOMCTL_VM_EVENT_OP_PAGING:
+    case XEN_VM_EVENT_TYPE_PAGING:
     {
         rc = -EINVAL;
 
@@ -683,7 +683,7 @@  int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
     break;
 #endif
 
-    case XEN_DOMCTL_VM_EVENT_OP_MONITOR:
+    case XEN_VM_EVENT_TYPE_MONITOR:
     {
         rc = -EINVAL;
 
@@ -721,7 +721,7 @@  int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
     break;
 
 #ifdef CONFIG_HAS_MEM_SHARING
-    case XEN_DOMCTL_VM_EVENT_OP_SHARING:
+    case XEN_VM_EVENT_TYPE_SHARING:
     {
         rc = -EINVAL;
 
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 19486d5..19281fa 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -38,7 +38,7 @@ 
 #include "hvm/save.h"
 #include "memory.h"
 
-#define XEN_DOMCTL_INTERFACE_VERSION 0x00000011
+#define XEN_DOMCTL_INTERFACE_VERSION 0x00000012
 
 /*
  * NB. xen_domctl.domain is an IN/OUT parameter for this operation.
@@ -769,80 +769,18 @@  struct xen_domctl_gdbsx_domstatus {
  * VM event operations
  */
 
-/* XEN_DOMCTL_vm_event_op */
-
-/*
- * There are currently three rings available for VM events:
- * sharing, monitor and paging. This hypercall allows one to
- * control these rings (enable/disable), as well as to signal
- * to the hypervisor to pull responses (resume) from the given
- * ring.
+/* XEN_DOMCTL_vm_event_op.
+ * Use for teardown/setup of helper<->hypervisor interface for paging,
+ * access and sharing.
  */
 #define XEN_VM_EVENT_ENABLE               0
 #define XEN_VM_EVENT_DISABLE              1
 #define XEN_VM_EVENT_RESUME               2
 #define XEN_VM_EVENT_GET_VERSION          3
 
-/*
- * Domain memory paging
- * Page memory in and out.
- * Domctl interface to set up and tear down the
- * pager<->hypervisor interface. Use XENMEM_paging_op*
- * to perform per-page operations.
- *
- * The XEN_VM_EVENT_PAGING_ENABLE domctl returns several
- * non-standard error codes to indicate why paging could not be enabled:
- * ENODEV - host lacks HAP support (EPT/NPT) or HAP is disabled in guest
- * EMLINK - guest has iommu passthrough enabled
- * EXDEV  - guest has PoD enabled
- * EBUSY  - guest has or had paging enabled, ring buffer still active
- */
-#define XEN_DOMCTL_VM_EVENT_OP_PAGING            1
-
-/*
- * Monitor helper.
- *
- * As with paging, use the domctl for teardown/setup of the
- * helper<->hypervisor interface.
- *
- * The monitor interface can be used to register for various VM events. For
- * example, there are HVM hypercalls to set the per-page access permissions
- * of every page in a domain.  When one of these permissions--independent,
- * read, write, and execute--is violated, the VCPU is paused and a memory event
- * is sent with what happened. The memory event handler can then resume the
- * VCPU and redo the access with a XEN_VM_EVENT_RESUME option.
- *
- * See public/vm_event.h for the list of available events that can be
- * subscribed to via the monitor interface.
- *
- * The XEN_VM_EVENT_MONITOR_* domctls returns
- * non-standard error codes to indicate why access could not be enabled:
- * ENODEV - host lacks HAP support (EPT/NPT) or HAP is disabled in guest
- * EBUSY  - guest has or had access enabled, ring buffer still active
- *
- */
-#define XEN_DOMCTL_VM_EVENT_OP_MONITOR           2
-
-/*
- * Sharing ENOMEM helper.
- *
- * As with paging, use the domctl for teardown/setup of the
- * helper<->hypervisor interface.
- *
- * If setup, this ring is used to communicate failed allocations
- * in the unshare path. XENMEM_sharing_op_resume is used to wake up
- * vcpus that could not unshare.
- *
- * Note that shring can be turned on (as per the domctl below)
- * *without* this ring being setup.
- */
-#define XEN_DOMCTL_VM_EVENT_OP_SHARING           3
-
-/* Use for teardown/setup of helper<->hypervisor interface for paging,
- * access and sharing.*/
 struct xen_domctl_vm_event_op {
-    uint32_t       op;           /* XEN_VM_EVENT_* */
-    uint32_t       mode;         /* XEN_DOMCTL_VM_EVENT_OP_* */
+    uint32_t        op;           /* XEN_VM_EVENT_* */
+    uint32_t        type;         /* XEN_VM_EVENT_TYPE_* */
 
     union {
         struct {
@@ -857,7 +795,10 @@  struct xen_domctl_vm_event_op {
  * Memory sharing operations
  */
 /* XEN_DOMCTL_mem_sharing_op.
- * The CONTROL sub-domctl is used for bringup/teardown. */
+ * The CONTROL sub-domctl is used for bringup/teardown.
+ * Please note that mem sharing can be turned on *without* setting-up the
+ * correspondin ring
+ */
 #define XEN_DOMCTL_MEM_SHARING_CONTROL          0
 
 struct xen_domctl_mem_sharing_op {
@@ -1004,7 +945,7 @@  struct xen_domctl_psr_cmt_op {
  * Enable/disable monitoring various VM events.
  * This domctl configures what events will be reported to helper apps
  * via the ring buffer "MONITOR". The ring has to be first enabled
- * with the domctl XEN_DOMCTL_VM_EVENT_OP_MONITOR.
+ * with XEN_VM_EVENT_ENABLE.
  *
  * GET_CAPABILITIES can be used to determine which of these features is
  * available on a given platform.
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index 959083d..c48bc21 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -36,6 +36,37 @@ 
 #include "io/ring.h"
 
 /*
+ * There are currently three types of VM events.
+ */
+
+/*
+ * Domain memory paging
+ *
+ * Page memory in and out.
+ */
+#define XEN_VM_EVENT_TYPE_PAGING         1
+
+/*
+ * Monitor.
+ *
+ * The monitor interface can be used to register for various VM events. For
+ * example, there are HVM hypercalls to set the per-page access permissions
+ * of every page in a domain.  When one of these permissions--independent,
+ * read, write, and execute--is violated, the VCPU is paused and a memory event
+ * is sent with what happened. The memory event handler can then resume the
+ * VCPU and redo the access with a XEN_VM_EVENT_RESUME option.
+ */
+#define XEN_VM_EVENT_TYPE_MONITOR        2
+
+/*
+ * Sharing ENOMEM.
+ *
+ * Used to communicate failed allocations in the unshare path.
+ * XENMEM_sharing_op_resume is used to wake up vcpus that could not unshare.
+ */
+#define XEN_VM_EVENT_TYPE_SHARING        3
+
+/*
  * Memory event flags
  */