* [PATCH v5 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.
2016-02-17 7:33 [PATCH v5 0/2] Vm-events: move monitor vm-events code to common-side Corneliu ZUZU
2016-02-17 7:36 ` [PATCH v5 1/2] xen/arm: fix file comments Corneliu ZUZU
@ 2016-02-17 7:38 ` Corneliu ZUZU
2016-02-17 15:56 ` Tamas K Lengyel
2016-02-17 15:09 ` [PATCH v5 0/2] Vm-events: move monitor vm-events " Konrad Rzeszutek Wilk
2 siblings, 1 reply; 6+ messages in thread
From: Corneliu ZUZU @ 2016-02-17 7:38 UTC (permalink / raw)
To: xen-devel
Cc: Tamas K Lengyel, Keir Fraser, Ian Campbell, Razvan Cojocaru,
Andrew Cooper, Stefano Stabellini, Jan Beulich
This patch moves monitor_domctl to common-side.
Purpose: move what's common to common, prepare for implementation
of such vm-events on ARM.
* move get_capabilities to arch-side => arch_monitor_get_capabilities.
* add arch-side monitor op handling function => arch_monitor_domctl_op.
e.g. X86-side handles XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP op
* add arch-side monitor event handling function => arch_monitor_domctl_event.
e.g. X86-side handles XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR event enable/disable
* remove status_check
Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
Changed since v4:
* arch_monitor_domctl_event @ x86/monitor.c:
replaced !old_status w/ requested_status (equivalent but more readable)
---
MAINTAINERS | 1 +
xen/arch/x86/monitor.c | 153 +++++++++++-------------------------------
xen/common/Makefile | 1 +
xen/common/domctl.c | 2 +-
xen/common/monitor.c | 69 +++++++++++++++++++
xen/include/asm-arm/monitor.h | 30 +++++++--
xen/include/asm-x86/monitor.h | 53 +++++++++++++--
xen/include/xen/monitor.h | 30 +++++++++
8 files changed, 217 insertions(+), 122 deletions(-)
create mode 100644 xen/common/monitor.c
create mode 100644 xen/include/xen/monitor.h
diff --git a/MAINTAINERS b/MAINTAINERS
index f07384c..5cbb1dc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -355,6 +355,7 @@ M: Tamas K Lengyel <tamas@tklengyel.com>
S: Supported
F: xen/common/vm_event.c
F: xen/common/mem_access.c
+F: xen/common/monitor.c
F: xen/arch/x86/hvm/event.c
F: xen/arch/x86/monitor.c
F: xen/arch/*/vm_event.c
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 1d43880..a507edb 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -1,9 +1,10 @@
/*
* arch/x86/monitor.c
*
- * Architecture-specific monitor_op domctl handler.
+ * Arch-specific monitor_op domctl handler.
*
* Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com)
+ * Copyright (c) 2016, Bitdefender S.R.L.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public
@@ -18,87 +19,14 @@
* License along with this program; If not, see <http://www.gnu.org/licenses/>.
*/
-#include <xen/config.h>
-#include <xen/sched.h>
-#include <xen/mm.h>
-#include <asm/domain.h>
#include <asm/monitor.h>
-#include <public/domctl.h>
-#include <xsm/xsm.h>
+#include <public/vm_event.h>
-/*
- * Sanity check whether option is already enabled/disabled
- */
-static inline
-int status_check(struct xen_domctl_monitor_op *mop, bool_t status)
-{
- bool_t requested_status = (mop->op == XEN_DOMCTL_MONITOR_OP_ENABLE);
-
- if ( status == requested_status )
- return -EEXIST;
-
- return 0;
-}
-
-static inline uint32_t get_capabilities(struct domain *d)
-{
- uint32_t capabilities = 0;
-
- /*
- * At the moment only Intel HVM domains are supported. However, event
- * delivery could be extended to AMD and PV domains.
- */
- if ( !is_hvm_domain(d) || !cpu_has_vmx )
- return capabilities;
-
- capabilities = (1 << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
- (1 << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
- (1 << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
- (1 << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
-
- /* Since we know this is on VMX, we can just call the hvm func */
- if ( hvm_is_singlestep_supported() )
- capabilities |= (1 << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
-
- return capabilities;
-}
-
-int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
+int arch_monitor_domctl_event(struct domain *d,
+ struct xen_domctl_monitor_op *mop)
{
- int rc;
struct arch_domain *ad = &d->arch;
- uint32_t capabilities = get_capabilities(d);
-
- if ( current->domain == d ) /* no domain_pause() */
- return -EPERM;
-
- rc = xsm_vm_event_control(XSM_PRIV, d, mop->op, mop->event);
- if ( rc )
- return rc;
-
- switch ( mop->op )
- {
- case XEN_DOMCTL_MONITOR_OP_GET_CAPABILITIES:
- mop->event = capabilities;
- return 0;
-
- case XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP:
- domain_pause(d);
- ad->mem_access_emulate_each_rep = !!mop->event;
- domain_unpause(d);
- return 0;
- }
-
- /*
- * Sanity check
- */
- if ( mop->op != XEN_DOMCTL_MONITOR_OP_ENABLE &&
- mop->op != XEN_DOMCTL_MONITOR_OP_DISABLE )
- return -EOPNOTSUPP;
-
- /* Check if event type is available. */
- if ( !(capabilities & (1 << mop->event)) )
- return -EOPNOTSUPP;
+ bool_t requested_status = (XEN_DOMCTL_MONITOR_OP_ENABLE == mop->op);
switch ( mop->event )
{
@@ -106,13 +34,11 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
{
unsigned int ctrlreg_bitmask =
monitor_ctrlreg_bitmask(mop->u.mov_to_cr.index);
- bool_t status =
+ bool_t old_status =
!!(ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask);
- struct vcpu *v;
- rc = status_check(mop, status);
- if ( rc )
- return rc;
+ if ( unlikely(old_status == requested_status) )
+ return -EEXIST;
domain_pause(d);
@@ -126,15 +52,18 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
else
ad->monitor.write_ctrlreg_onchangeonly &= ~ctrlreg_bitmask;
- if ( !status )
+ if ( requested_status )
ad->monitor.write_ctrlreg_enabled |= ctrlreg_bitmask;
else
ad->monitor.write_ctrlreg_enabled &= ~ctrlreg_bitmask;
- if ( mop->u.mov_to_cr.index == VM_EVENT_X86_CR3 )
- /* Latches new CR3 mask through CR0 code */
+ if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index )
+ {
+ struct vcpu *v;
+ /* Latches new CR3 mask through CR0 code. */
for_each_vcpu ( d, v )
hvm_update_guest_cr(v, 0);
+ }
domain_unpause(d);
@@ -143,77 +72,75 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
case XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR:
{
- bool_t status = ad->monitor.mov_to_msr_enabled;
+ bool_t old_status = ad->monitor.mov_to_msr_enabled;
- rc = status_check(mop, status);
- if ( rc )
- return rc;
+ if ( unlikely(old_status == requested_status) )
+ return -EEXIST;
- if ( mop->op == XEN_DOMCTL_MONITOR_OP_ENABLE &&
- mop->u.mov_to_msr.extended_capture &&
+ if ( requested_status && mop->u.mov_to_msr.extended_capture &&
!hvm_enable_msr_exit_interception(d) )
return -EOPNOTSUPP;
domain_pause(d);
- if ( mop->op == XEN_DOMCTL_MONITOR_OP_ENABLE &&
- mop->u.mov_to_msr.extended_capture )
- ad->monitor.mov_to_msr_extended = 1;
+ if ( requested_status && mop->u.mov_to_msr.extended_capture )
+ ad->monitor.mov_to_msr_extended = 1;
else
ad->monitor.mov_to_msr_extended = 0;
- ad->monitor.mov_to_msr_enabled = !status;
+ ad->monitor.mov_to_msr_enabled = requested_status;
domain_unpause(d);
break;
}
case XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP:
{
- bool_t status = ad->monitor.singlestep_enabled;
+ bool_t old_status = ad->monitor.singlestep_enabled;
- rc = status_check(mop, status);
- if ( rc )
- return rc;
+ if ( unlikely(old_status == requested_status) )
+ return -EEXIST;
domain_pause(d);
- ad->monitor.singlestep_enabled = !status;
+ ad->monitor.singlestep_enabled = requested_status;
domain_unpause(d);
break;
}
case XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT:
{
- bool_t status = ad->monitor.software_breakpoint_enabled;
+ bool_t old_status = ad->monitor.software_breakpoint_enabled;
- rc = status_check(mop, status);
- if ( rc )
- return rc;
+ if ( unlikely(old_status == requested_status) )
+ return -EEXIST;
domain_pause(d);
- ad->monitor.software_breakpoint_enabled = !status;
+ ad->monitor.software_breakpoint_enabled = requested_status;
domain_unpause(d);
break;
}
case XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST:
{
- bool_t status = ad->monitor.guest_request_enabled;
+ bool_t old_status = ad->monitor.guest_request_enabled;
- rc = status_check(mop, status);
- if ( rc )
- return rc;
+ if ( unlikely(old_status == requested_status) )
+ return -EEXIST;
domain_pause(d);
ad->monitor.guest_request_sync = mop->u.guest_request.sync;
- ad->monitor.guest_request_enabled = !status;
+ ad->monitor.guest_request_enabled = requested_status;
domain_unpause(d);
break;
}
default:
+ /*
+ * Should not be reached unless arch_monitor_get_capabilities() is not
+ * properly implemented.
+ */
+ ASSERT_UNREACHABLE();
return -EOPNOTSUPP;
-
- };
+ }
return 0;
}
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 6e82b33..0d76efe 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -20,6 +20,7 @@ obj-y += lib.o
obj-y += lzo.o
obj-$(CONFIG_HAS_MEM_ACCESS) += mem_access.o
obj-y += memory.o
+obj-y += monitor.o
obj-y += multicall.o
obj-y += notifier.o
obj-y += page_alloc.o
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 22fa5d5..b34c0a1 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -25,11 +25,11 @@
#include <xen/paging.h>
#include <xen/hypercall.h>
#include <xen/vm_event.h>
+#include <xen/monitor.h>
#include <asm/current.h>
#include <asm/irq.h>
#include <asm/page.h>
#include <asm/p2m.h>
-#include <asm/monitor.h>
#include <public/domctl.h>
#include <xsm/xsm.h>
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
new file mode 100644
index 0000000..2a99a04
--- /dev/null
+++ b/xen/common/monitor.c
@@ -0,0 +1,69 @@
+/*
+ * xen/common/monitor.c
+ *
+ * Common monitor_op domctl handler.
+ *
+ * Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com)
+ * Copyright (c) 2016, Bitdefender S.R.L.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/monitor.h>
+#include <xen/sched.h>
+#include <xsm/xsm.h>
+#include <public/domctl.h>
+#include <asm/monitor.h>
+
+int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
+{
+ int rc;
+
+ if ( unlikely(current->domain == d) ) /* no domain_pause() */
+ return -EPERM;
+
+ rc = xsm_vm_event_control(XSM_PRIV, d, mop->op, mop->event);
+ if ( unlikely(rc) )
+ return rc;
+
+ switch ( mop->op )
+ {
+ case XEN_DOMCTL_MONITOR_OP_ENABLE:
+ case XEN_DOMCTL_MONITOR_OP_DISABLE:
+ /* Check if event type is available. */
+ /* sanity check: avoid left-shift undefined behavior */
+ if ( unlikely(mop->event > 31) )
+ return -EINVAL;
+ if ( unlikely(!(arch_monitor_get_capabilities(d) & (1U << mop->event))) )
+ return -EOPNOTSUPP;
+ /* Arch-side handles enable/disable ops. */
+ return arch_monitor_domctl_event(d, mop);
+
+ case XEN_DOMCTL_MONITOR_OP_GET_CAPABILITIES:
+ mop->event = arch_monitor_get_capabilities(d);
+ return 0;
+
+ default:
+ /* The monitor op is probably handled on the arch-side. */
+ return arch_monitor_domctl_op(d, mop);
+ }
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/monitor.h b/xen/include/asm-arm/monitor.h
index a3a9703..82a4a66 100644
--- a/xen/include/asm-arm/monitor.h
+++ b/xen/include/asm-arm/monitor.h
@@ -1,9 +1,10 @@
/*
* include/asm-arm/monitor.h
*
- * Architecture-specific monitor_op domctl handler.
+ * Arch-specific monitor_op domctl handler.
*
* Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com)
+ * Copyright (c) 2016, Bitdefender S.R.L.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public
@@ -24,10 +25,31 @@
#include <xen/sched.h>
#include <public/domctl.h>
+static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
+{
+ /* No monitor vm-events implemented on ARM. */
+ return 0;
+}
+
+static inline
+int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
+{
+ /* No arch-specific monitor ops on ARM. */
+ return -EOPNOTSUPP;
+}
+
static inline
-int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *op)
+int arch_monitor_domctl_event(struct domain *d,
+ struct xen_domctl_monitor_op *mop)
{
- return -ENOSYS;
+ /*
+ * No arch-specific monitor vm-events on ARM.
+ *
+ * Should not be reached unless arch_monitor_get_capabilities() is not
+ * properly implemented.
+ */
+ ASSERT_UNREACHABLE();
+ return -EOPNOTSUPP;
}
-#endif /* __ASM_X86_MONITOR_H__ */
+#endif /* __ASM_ARM_MONITOR_H__ */
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index 7c8280b..c789f71 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -1,9 +1,10 @@
/*
* include/asm-x86/monitor.h
*
- * Architecture-specific monitor_op domctl handler.
+ * Arch-specific monitor_op domctl handler.
*
* Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com)
+ * Copyright (c) 2016, Bitdefender S.R.L.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public
@@ -21,11 +22,55 @@
#ifndef __ASM_X86_MONITOR_H__
#define __ASM_X86_MONITOR_H__
-struct domain;
-struct xen_domctl_monitor_op;
+#include <xen/sched.h>
+#include <public/domctl.h>
+#include <asm/cpufeature.h>
+#include <asm/hvm/hvm.h>
#define monitor_ctrlreg_bitmask(ctrlreg_index) (1U << (ctrlreg_index))
-int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *op);
+static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
+{
+ uint32_t capabilities = 0;
+
+ /*
+ * At the moment only Intel HVM domains are supported. However, event
+ * delivery could be extended to AMD and PV domains.
+ */
+ if ( !is_hvm_domain(d) || !cpu_has_vmx )
+ return capabilities;
+
+ capabilities = (1 << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
+ (1 << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
+ (1 << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
+ (1 << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
+
+ /* Since we know this is on VMX, we can just call the hvm func */
+ if ( hvm_is_singlestep_supported() )
+ capabilities |= (1 << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
+
+ return capabilities;
+}
+
+static inline
+int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
+{
+ switch ( mop->op )
+ {
+ case XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP:
+ domain_pause(d);
+ d->arch.mem_access_emulate_each_rep = !!mop->event;
+ domain_unpause(d);
+ break;
+
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+int arch_monitor_domctl_event(struct domain *d,
+ struct xen_domctl_monitor_op *mop);
#endif /* __ASM_X86_MONITOR_H__ */
diff --git a/xen/include/xen/monitor.h b/xen/include/xen/monitor.h
new file mode 100644
index 0000000..7015e6d
--- /dev/null
+++ b/xen/include/xen/monitor.h
@@ -0,0 +1,30 @@
+/*
+ * include/xen/monitor.h
+ *
+ * Common monitor_op domctl handler.
+ *
+ * Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com)
+ * Copyright (c) 2016, Bitdefender S.R.L.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __XEN_MONITOR_H__
+#define __XEN_MONITOR_H__
+
+struct domain;
+struct xen_domctl_monitor_op;
+
+int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *op);
+
+#endif /* __XEN_MONITOR_H__ */
--
2.5.0
^ permalink raw reply related [flat|nested] 6+ messages in thread