* [PATCH v4] x86/hvm: Allow guest_request vm_events coming from userspace
@ 2017-08-04 11:32 Alexandru Isaila
2017-08-04 11:42 ` Andrew Cooper
2017-08-05 1:32 ` Tamas K Lengyel
0 siblings, 2 replies; 8+ messages in thread
From: Alexandru Isaila @ 2017-08-04 11:32 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, wei.liu2, rcojocaru, George.Dunlap, ian.jackson,
tim, tamas, jbeulich, andrew.cooper3, Alexandru Isaila
In some introspection usecases, an in-guest agent needs to communicate
with the external introspection agent. An existing mechanism is
HVMOP_guest_request_vm_event, but this is restricted to kernel usecases
like all other hypercalls.
Introduce a mechanism whereby the introspection agent can whitelist the
use of HVMOP_guest_request_vm_event directly from userspace.
Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
---
Changes since V3:
- Changed commit message
- Added new lines
- Indent the maximum space on the defines
- Chaned the name of the define/function name/struct member
from vmcall to event
---
tools/libxc/include/xenctrl.h | 1 +
tools/libxc/xc_monitor.c | 14 ++++++++++++++
xen/arch/x86/hvm/hypercall.c | 5 +++++
xen/common/monitor.c | 14 ++++++++++++++
xen/include/public/domctl.h | 21 +++++++++++----------
xen/include/xen/sched.h | 5 +++--
6 files changed, 48 insertions(+), 12 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index bde8313..90a056f 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2022,6 +2022,7 @@ int xc_monitor_descriptor_access(xc_interface *xch, domid_t domain_id,
bool enable);
int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
bool enable, bool sync);
+int xc_allow_guest_userspace_event(xc_interface *xch, domid_t domain_id, bool enable);
int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
bool enable, bool sync);
int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool enable);
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index b44ce93..6064c39 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -161,6 +161,20 @@ int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool enable,
return do_domctl(xch, &domctl);
}
+int xc_allow_guest_userspace_event(xc_interface *xch, domid_t domain_id, bool enable)
+{
+ DECLARE_DOMCTL;
+
+ domctl.cmd = XEN_DOMCTL_monitor_op;
+ domctl.domain = domain_id;
+ domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
+ : XEN_DOMCTL_MONITOR_OP_DISABLE;
+ domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT;
+
+ return do_domctl(xch, &domctl);
+}
+
+
int xc_monitor_emulate_each_rep(xc_interface *xch, domid_t domain_id,
bool enable)
{
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index e7238ce..8eb5f49 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -155,6 +155,11 @@ int hvm_hypercall(struct cpu_user_regs *regs)
/* Fallthrough to permission check. */
case 4:
case 2:
+ if ( currd->monitor.guest_request_userspace_event &&
+ eax == __HYPERVISOR_hvm_op &&
+ (mode == 8 ? regs->rdi : regs->ebx) == HVMOP_guest_request_vm_event )
+ break;
+
if ( unlikely(hvm_get_cpl(curr)) )
{
default:
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
index 451f42f..21a1457 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -79,6 +79,20 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
break;
}
+ case XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT:
+ {
+ bool_t old_status = d->monitor.guest_request_enabled;
+
+ if ( unlikely(old_status == requested_status) )
+ return -EEXIST;
+
+ domain_pause(d);
+ d->monitor.guest_request_sync = mop->u.guest_request.sync;
+ d->monitor.guest_request_userspace_event = requested_status;
+ domain_unpause(d);
+ break;
+ }
+
default:
/* Give arch-side the chance to handle this event */
return arch_monitor_domctl_event(d, mop);
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index ff39762..870495c 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1073,16 +1073,17 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
#define XEN_DOMCTL_MONITOR_OP_GET_CAPABILITIES 2
#define XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP 3
-#define XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG 0
-#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR 1
-#define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP 2
-#define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT 3
-#define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST 4
-#define XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION 5
-#define XEN_DOMCTL_MONITOR_EVENT_CPUID 6
-#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL 7
-#define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT 8
-#define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS 9
+#define XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG 0
+#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR 1
+#define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP 2
+#define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT 3
+#define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST 4
+#define XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION 5
+#define XEN_DOMCTL_MONITOR_EVENT_CPUID 6
+#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL 7
+#define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT 8
+#define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS 9
+#define XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT 10
struct xen_domctl_monitor_op {
uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 6673b27..898a132 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -480,8 +480,9 @@ struct domain
/* Common monitor options */
struct {
- unsigned int guest_request_enabled : 1;
- unsigned int guest_request_sync : 1;
+ unsigned int guest_request_enabled : 1;
+ unsigned int guest_request_sync : 1;
+ unsigned int guest_request_userspace_event : 1;
} monitor;
};
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4] x86/hvm: Allow guest_request vm_events coming from userspace
2017-08-04 11:32 [PATCH v4] x86/hvm: Allow guest_request vm_events coming from userspace Alexandru Isaila
@ 2017-08-04 11:42 ` Andrew Cooper
2017-08-05 1:32 ` Tamas K Lengyel
1 sibling, 0 replies; 8+ messages in thread
From: Andrew Cooper @ 2017-08-04 11:42 UTC (permalink / raw)
To: Alexandru Isaila, xen-devel
Cc: sstabellini, wei.liu2, rcojocaru, George.Dunlap, ian.jackson,
tim, tamas, jbeulich
On 04/08/17 12:32, Alexandru Isaila wrote:
> In some introspection usecases, an in-guest agent needs to communicate
> with the external introspection agent. An existing mechanism is
> HVMOP_guest_request_vm_event, but this is restricted to kernel usecases
> like all other hypercalls.
>
> Introduce a mechanism whereby the introspection agent can whitelist the
> use of HVMOP_guest_request_vm_event directly from userspace.
>
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, with one note to
whoever commits it...
> diff --git a/xen/common/monitor.c b/xen/common/monitor.c
> index 451f42f..21a1457 100644
> --- a/xen/common/monitor.c
> +++ b/xen/common/monitor.c
> @@ -79,6 +79,20 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
> break;
> }
>
> + case XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT:
> + {
> + bool_t old_status = d->monitor.guest_request_enabled;
bool.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] x86/hvm: Allow guest_request vm_events coming from userspace
2017-08-04 11:32 [PATCH v4] x86/hvm: Allow guest_request vm_events coming from userspace Alexandru Isaila
2017-08-04 11:42 ` Andrew Cooper
@ 2017-08-05 1:32 ` Tamas K Lengyel
2017-08-05 8:18 ` Razvan Cojocaru
` (2 more replies)
1 sibling, 3 replies; 8+ messages in thread
From: Tamas K Lengyel @ 2017-08-05 1:32 UTC (permalink / raw)
To: Alexandru Isaila
Cc: Stefano Stabellini, wei.liu2, Razvan Cojocaru, George Dunlap,
Ian Jackson, Tim Deegan, Xen-devel, Jan Beulich, Andrew Cooper
[-- Attachment #1.1: Type: text/plain, Size: 7350 bytes --]
On Fri, Aug 4, 2017 at 5:32 AM, Alexandru Isaila <aisaila@bitdefender.com>
wrote:
> In some introspection usecases, an in-guest agent needs to communicate
> with the external introspection agent. An existing mechanism is
> HVMOP_guest_request_vm_event, but this is restricted to kernel usecases
> like all other hypercalls.
>
> Introduce a mechanism whereby the introspection agent can whitelist the
> use of HVMOP_guest_request_vm_event directly from userspace.
>
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
>
> ---
> Changes since V3:
> - Changed commit message
> - Added new lines
> - Indent the maximum space on the defines
> - Chaned the name of the define/function name/struct member
> from vmcall to event
> ---
> tools/libxc/include/xenctrl.h | 1 +
> tools/libxc/xc_monitor.c | 14 ++++++++++++++
> xen/arch/x86/hvm/hypercall.c | 5 +++++
> xen/common/monitor.c | 14 ++++++++++++++
> xen/include/public/domctl.h | 21 +++++++++++----------
> xen/include/xen/sched.h | 5 +++--
> 6 files changed, 48 insertions(+), 12 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index bde8313..90a056f 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2022,6 +2022,7 @@ int xc_monitor_descriptor_access(xc_interface *xch,
> domid_t domain_id,
> bool enable);
> int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
> bool enable, bool sync);
> +int xc_allow_guest_userspace_event(xc_interface *xch, domid_t domain_id,
> bool enable);
> int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
> bool enable, bool sync);
> int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool enable);
> diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
> index b44ce93..6064c39 100644
> --- a/tools/libxc/xc_monitor.c
> +++ b/tools/libxc/xc_monitor.c
> @@ -161,6 +161,20 @@ int xc_monitor_guest_request(xc_interface *xch,
> domid_t domain_id, bool enable,
> return do_domctl(xch, &domctl);
> }
>
> +int xc_allow_guest_userspace_event(xc_interface *xch, domid_t domain_id,
> bool enable)
>
This function should be prefixed with "xc_monitor_" like all the rest of
the functions here.
> +{
> + DECLARE_DOMCTL;
> +
> + domctl.cmd = XEN_DOMCTL_monitor_op;
> + domctl.domain = domain_id;
> + domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
> + : XEN_DOMCTL_MONITOR_OP_DISABLE;
> + domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_GUEST
> _USERSPACE_EVENT;
> +
> + return do_domctl(xch, &domctl);
> +}
> +
> +
> int xc_monitor_emulate_each_rep(xc_interface *xch, domid_t domain_id,
> bool enable)
> {
> diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
> index e7238ce..8eb5f49 100644
> --- a/xen/arch/x86/hvm/hypercall.c
> +++ b/xen/arch/x86/hvm/hypercall.c
> @@ -155,6 +155,11 @@ int hvm_hypercall(struct cpu_user_regs *regs)
> /* Fallthrough to permission check. */
> case 4:
> case 2:
> + if ( currd->monitor.guest_request_userspace_event &&
+ eax == __HYPERVISOR_hvm_op &&
> + (mode == 8 ? regs->rdi : regs->ebx) ==
> HVMOP_guest_request_vm_event )
> + break;
> +
> if ( unlikely(hvm_get_cpl(curr)) )
> {
> default:
> diff --git a/xen/common/monitor.c b/xen/common/monitor.c
> index 451f42f..21a1457 100644
> --- a/xen/common/monitor.c
> +++ b/xen/common/monitor.c
> @@ -79,6 +79,20 @@ int monitor_domctl(struct domain *d, struct
> xen_domctl_monitor_op *mop)
> break;
> }
>
> + case XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT:
> + {
> + bool_t old_status = d->monitor.guest_request_enabled;
>
You are checking guest_request enabled here while later setting
guest_request_userspace_event. These are two separate monitor options,
adjust accordingly.
> +
> + if ( unlikely(old_status == requested_status) )
> + return -EEXIST;
> +
> + domain_pause(d);
> + d->monitor.guest_request_sync = mop->u.guest_request.sync;
>
You are setting guest_request_sync here which is a setting belonging to
guest_request_enabled. If you need sync/async option for the userspace
guest request it should be a separate bit. Since the toolstack side you add
in this patch never sets the sync option I assume that is not the case, so
remove this.
> + d->monitor.guest_request_userspace_event = requested_status;
> + domain_unpause(d);
> + break;
> + }
> +
> default:
> /* Give arch-side the chance to handle this event */
> return arch_monitor_domctl_event(d, mop);
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index ff39762..870495c 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1073,16 +1073,17 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
> #define XEN_DOMCTL_MONITOR_OP_GET_CAPABILITIES 2
> #define XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP 3
>
> -#define XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG 0
> -#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR 1
> -#define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP 2
> -#define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT 3
> -#define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST 4
> -#define XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION 5
> -#define XEN_DOMCTL_MONITOR_EVENT_CPUID 6
> -#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL 7
> -#define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT 8
> -#define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS 9
> +#define XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG
> 0
> +#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR
> 1
> +#define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP
> 2
> +#define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT
> 3
> +#define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST
> 4
> +#define XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION
> 5
> +#define XEN_DOMCTL_MONITOR_EVENT_CPUID
> 6
> +#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL
> 7
> +#define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT
> 8
> +#define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS
> 9
> +#define XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT
> 10
>
> struct xen_domctl_monitor_op {
> uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 6673b27..898a132 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -480,8 +480,9 @@ struct domain
>
> /* Common monitor options */
> struct {
> - unsigned int guest_request_enabled : 1;
> - unsigned int guest_request_sync : 1;
> + unsigned int guest_request_enabled
> : 1;
> + unsigned int guest_request_sync
> : 1;
> + unsigned int guest_request_userspace_event
> : 1;
>
This should be "guest_request_userspace_enabled". It also should not be in
xen/sched.h as on ARM there is no instruction trapping from userspace
directly to the hypervisor (AFAIK).
> } monitor;
> };
>
> --
> 2.7.4
>
Tamas
[-- Attachment #1.2: Type: text/html, Size: 10321 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] x86/hvm: Allow guest_request vm_events coming from userspace
2017-08-05 1:32 ` Tamas K Lengyel
@ 2017-08-05 8:18 ` Razvan Cojocaru
2017-08-05 23:18 ` Tamas K Lengyel
2017-08-07 8:38 ` Alexandru Stefan ISAILA
2017-08-07 9:03 ` Alexandru Stefan ISAILA
2 siblings, 1 reply; 8+ messages in thread
From: Razvan Cojocaru @ 2017-08-05 8:18 UTC (permalink / raw)
To: Tamas K Lengyel, Alexandru Isaila
Cc: Stefano Stabellini, wei.liu2, George Dunlap, Tim Deegan,
Ian Jackson, Xen-devel, Jan Beulich, Andrew Cooper
On 08/05/2017 04:32 AM, Tamas K Lengyel wrote:
>
>
> On Fri, Aug 4, 2017 at 5:32 AM, Alexandru Isaila
> <aisaila@bitdefender.com <mailto:aisaila@bitdefender.com>> wrote:
>
> In some introspection usecases, an in-guest agent needs to communicate
> with the external introspection agent. An existing mechanism is
> HVMOP_guest_request_vm_event, but this is restricted to kernel usecases
> like all other hypercalls.
>
> Introduce a mechanism whereby the introspection agent can whitelist the
> use of HVMOP_guest_request_vm_event directly from userspace.
>
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com
> <mailto:aisaila@bitdefender.com>>
>
> ---
> Changes since V3:
> - Changed commit message
> - Added new lines
> - Indent the maximum space on the defines
> - Chaned the name of the define/function name/struct member
> from vmcall to event
> ---
> tools/libxc/include/xenctrl.h | 1 +
> tools/libxc/xc_monitor.c | 14 ++++++++++++++
> xen/arch/x86/hvm/hypercall.c | 5 +++++
> xen/common/monitor.c | 14 ++++++++++++++
> xen/include/public/domctl.h | 21 +++++++++++----------
> xen/include/xen/sched.h | 5 +++--
> 6 files changed, 48 insertions(+), 12 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h
> b/tools/libxc/include/xenctrl.h
> index bde8313..90a056f 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2022,6 +2022,7 @@ int xc_monitor_descriptor_access(xc_interface
> *xch, domid_t domain_id,
> bool enable);
> int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
> bool enable, bool sync);
> +int xc_allow_guest_userspace_event(xc_interface *xch, domid_t
> domain_id, bool enable);
> int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
> bool enable, bool sync);
> int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool
> enable);
> diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
> index b44ce93..6064c39 100644
> --- a/tools/libxc/xc_monitor.c
> +++ b/tools/libxc/xc_monitor.c
> @@ -161,6 +161,20 @@ int xc_monitor_guest_request(xc_interface *xch,
> domid_t domain_id, bool enable,
> return do_domctl(xch, &domctl);
> }
>
> +int xc_allow_guest_userspace_event(xc_interface *xch, domid_t
> domain_id, bool enable)
>
>
> This function should be prefixed with "xc_monitor_" like all the rest of
> the functions here.
That one was my suggestion, as I thought xc_monitor_-prefixed functions
are meant to toggle monitoring somehow, whereas this function only
toggles userspace use of guest request VMCALLs.
But re-adding the prefix is fine - since toggling it only influences in
the end how a vm_event is sent, it is after all monitor-related.
Thanks for the review,
Razvan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] x86/hvm: Allow guest_request vm_events coming from userspace
2017-08-05 8:18 ` Razvan Cojocaru
@ 2017-08-05 23:18 ` Tamas K Lengyel
0 siblings, 0 replies; 8+ messages in thread
From: Tamas K Lengyel @ 2017-08-05 23:18 UTC (permalink / raw)
To: Razvan Cojocaru
Cc: Stefano Stabellini, wei.liu2, George Dunlap, Tim Deegan,
Ian Jackson, Xen-devel, Jan Beulich, Andrew Cooper,
Alexandru Isaila
[-- Attachment #1.1: Type: text/plain, Size: 3382 bytes --]
On Sat, Aug 5, 2017 at 2:18 AM, Razvan Cojocaru <rcojocaru@bitdefender.com>
wrote:
> On 08/05/2017 04:32 AM, Tamas K Lengyel wrote:
> >
> >
> > On Fri, Aug 4, 2017 at 5:32 AM, Alexandru Isaila
> > <aisaila@bitdefender.com <mailto:aisaila@bitdefender.com>> wrote:
> >
> > In some introspection usecases, an in-guest agent needs to
> communicate
> > with the external introspection agent. An existing mechanism is
> > HVMOP_guest_request_vm_event, but this is restricted to kernel
> usecases
> > like all other hypercalls.
> >
> > Introduce a mechanism whereby the introspection agent can whitelist
> the
> > use of HVMOP_guest_request_vm_event directly from userspace.
> >
> > Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com
> > <mailto:aisaila@bitdefender.com>>
> >
> > ---
> > Changes since V3:
> > - Changed commit message
> > - Added new lines
> > - Indent the maximum space on the defines
> > - Chaned the name of the define/function name/struct member
> > from vmcall to event
> > ---
> > tools/libxc/include/xenctrl.h | 1 +
> > tools/libxc/xc_monitor.c | 14 ++++++++++++++
> > xen/arch/x86/hvm/hypercall.c | 5 +++++
> > xen/common/monitor.c | 14 ++++++++++++++
> > xen/include/public/domctl.h | 21 +++++++++++----------
> > xen/include/xen/sched.h | 5 +++--
> > 6 files changed, 48 insertions(+), 12 deletions(-)
> >
> > diff --git a/tools/libxc/include/xenctrl.h
> > b/tools/libxc/include/xenctrl.h
> > index bde8313..90a056f 100644
> > --- a/tools/libxc/include/xenctrl.h
> > +++ b/tools/libxc/include/xenctrl.h
> > @@ -2022,6 +2022,7 @@ int xc_monitor_descriptor_access(xc_interface
> > *xch, domid_t domain_id,
> > bool enable);
> > int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
> > bool enable, bool sync);
> > +int xc_allow_guest_userspace_event(xc_interface *xch, domid_t
> > domain_id, bool enable);
> > int xc_monitor_debug_exceptions(xc_interface *xch, domid_t
> domain_id,
> > bool enable, bool sync);
> > int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool
> > enable);
> > diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
> > index b44ce93..6064c39 100644
> > --- a/tools/libxc/xc_monitor.c
> > +++ b/tools/libxc/xc_monitor.c
> > @@ -161,6 +161,20 @@ int xc_monitor_guest_request(xc_interface *xch,
> > domid_t domain_id, bool enable,
> > return do_domctl(xch, &domctl);
> > }
> >
> > +int xc_allow_guest_userspace_event(xc_interface *xch, domid_t
> > domain_id, bool enable)
> >
> >
> > This function should be prefixed with "xc_monitor_" like all the rest of
> > the functions here.
> That one was my suggestion, as I thought xc_monitor_-prefixed functions
> are meant to toggle monitoring somehow, whereas this function only
> toggles userspace use of guest request VMCALLs.
>
So it wasn't exactly clear whether this is just an option on the
pre-existing guest request monitor like sync or a completely new, separate
monitor option on its own. It looks to me like it is a separate option so
let's treat it as such.
Tamas
[-- Attachment #1.2: Type: text/html, Size: 4718 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] x86/hvm: Allow guest_request vm_events coming from userspace
2017-08-05 1:32 ` Tamas K Lengyel
2017-08-05 8:18 ` Razvan Cojocaru
@ 2017-08-07 8:38 ` Alexandru Stefan ISAILA
2017-08-07 9:06 ` Jan Beulich
2017-08-07 9:03 ` Alexandru Stefan ISAILA
2 siblings, 1 reply; 8+ messages in thread
From: Alexandru Stefan ISAILA @ 2017-08-07 8:38 UTC (permalink / raw)
To: Tamas K Lengyel
Cc: Stefano Stabellini, wei.liu2, Razvan Cojocaru, George Dunlap,
Ian Jackson, Tim Deegan, Xen-devel, Jan Beulich, Andrew Cooper
[-- Attachment #1.1: Type: text/plain, Size: 8162 bytes --]
________________________________
From: Tamas K Lengyel <tamas@tklengyel.com>
Sent: Saturday, August 5, 2017 4:32 AM
To: Alexandru Stefan ISAILA
Cc: Xen-devel; wei.liu2@citrix.com; Tim Deegan; Stefano Stabellini; Konrad Rzeszutek Wilk; Jan Beulich; Ian Jackson; George Dunlap; Andrew Cooper; Razvan Cojocaru
Subject: Re: [PATCH v4] x86/hvm: Allow guest_request vm_events coming from userspace
On Fri, Aug 4, 2017 at 5:32 AM, Alexandru Isaila <aisaila@bitdefender.com<mailto:aisaila@bitdefender.com>> wrote:
In some introspection usecases, an in-guest agent needs to communicate
with the external introspection agent. An existing mechanism is
HVMOP_guest_request_vm_event, but this is restricted to kernel usecases
like all other hypercalls.
Introduce a mechanism whereby the introspection agent can whitelist the
use of HVMOP_guest_request_vm_event directly from userspace.
Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com<mailto:aisaila@bitdefender.com>>
---
Changes since V3:
- Changed commit message
- Added new lines
- Indent the maximum space on the defines
- Chaned the name of the define/function name/struct member
from vmcall to event
---
tools/libxc/include/xenctrl.h | 1 +
tools/libxc/xc_monitor.c | 14 ++++++++++++++
xen/arch/x86/hvm/hypercall.c | 5 +++++
xen/common/monitor.c | 14 ++++++++++++++
xen/include/public/domctl.h | 21 +++++++++++----------
xen/include/xen/sched.h | 5 +++--
6 files changed, 48 insertions(+), 12 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index bde8313..90a056f 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2022,6 +2022,7 @@ int xc_monitor_descriptor_access(xc_interface *xch, domid_t domain_id,
bool enable);
int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
bool enable, bool sync);
+int xc_allow_guest_userspace_event(xc_interface *xch, domid_t domain_id, bool enable);
int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
bool enable, bool sync);
int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool enable);
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index b44ce93..6064c39 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -161,6 +161,20 @@ int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool enable,
return do_domctl(xch, &domctl);
}
+int xc_allow_guest_userspace_event(xc_interface *xch, domid_t domain_id, bool enable)
This function should be prefixed with "xc_monitor_" like all the rest of the functions here.
+{
+ DECLARE_DOMCTL;
+
+ domctl.cmd = XEN_DOMCTL_monitor_op;
+ domctl.domain = domain_id;
+ domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
+ : XEN_DOMCTL_MONITOR_OP_DISABLE;
+ domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT;
+
+ return do_domctl(xch, &domctl);
+}
+
+
int xc_monitor_emulate_each_rep(xc_interface *xch, domid_t domain_id,
bool enable)
{
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index e7238ce..8eb5f49 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -155,6 +155,11 @@ int hvm_hypercall(struct cpu_user_regs *regs)
/* Fallthrough to permission check. */
case 4:
case 2:
+ if ( currd->monitor.guest_request_userspace_event &&
+ eax == __HYPERVISOR_hvm_op &&
+ (mode == 8 ? regs->rdi : regs->ebx) == HVMOP_guest_request_vm_event )
+ break;
+
if ( unlikely(hvm_get_cpl(curr)) )
{
default:
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
index 451f42f..21a1457 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -79,6 +79,20 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
break;
}
+ case XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT:
+ {
+ bool_t old_status = d->monitor.guest_request_enabled;
You are checking guest_request enabled here while later setting guest_request_userspace_event. These are two separate monitor options, adjust accordingly.
+
+ if ( unlikely(old_status == requested_status) )
+ return -EEXIST;
+
+ domain_pause(d);
+ d->monitor.guest_request_sync = mop->u.guest_request.sync;
You are setting guest_request_sync here which is a setting belonging to guest_request_enabled. If you need sync/async option for the userspace guest request it should be a separate bit. Since the toolstack side you add in this patch never sets the sync option I assume that is not the case, so remove this.
+ d->monitor.guest_request_userspace_event = requested_status;
+ domain_unpause(d);
+ break;
+ }
+
default:
/* Give arch-side the chance to handle this event */
return arch_monitor_domctl_event(d, mop);
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index ff39762..870495c 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1073,16 +1073,17 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
#define XEN_DOMCTL_MONITOR_OP_GET_CAPABILITIES 2
#define XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP 3
-#define XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG 0
-#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR 1
-#define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP 2
-#define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT 3
-#define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST 4
-#define XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION 5
-#define XEN_DOMCTL_MONITOR_EVENT_CPUID 6
-#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL 7
-#define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT 8
-#define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS 9
+#define XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG 0
+#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR 1
+#define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP 2
+#define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT 3
+#define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST 4
+#define XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION 5
+#define XEN_DOMCTL_MONITOR_EVENT_CPUID 6
+#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL 7
+#define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT 8
+#define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS 9
+#define XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT 10
struct xen_domctl_monitor_op {
uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 6673b27..898a132 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -480,8 +480,9 @@ struct domain
/* Common monitor options */
struct {
- unsigned int guest_request_enabled : 1;
- unsigned int guest_request_sync : 1;
+ unsigned int guest_request_enabled : 1;
+ unsigned int guest_request_sync : 1;
+ unsigned int guest_request_userspace_event : 1;
This should be "guest_request_userspace_enabled". It also should not be in xen/sched.h as on ARM there is no instruction trapping from userspace directly to the hypervisor (AFAIK).
Is everyone ok with moving the bit in a x86 specific region?
} monitor;
};
--
2.7.4
Tamas
Regards,
Alex
________________________________
This email was scanned by Bitdefender
[-- Attachment #1.2: Type: text/html, Size: 14733 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4] x86/hvm: Allow guest_request vm_events coming from userspace
2017-08-05 1:32 ` Tamas K Lengyel
2017-08-05 8:18 ` Razvan Cojocaru
2017-08-07 8:38 ` Alexandru Stefan ISAILA
@ 2017-08-07 9:03 ` Alexandru Stefan ISAILA
2 siblings, 0 replies; 8+ messages in thread
From: Alexandru Stefan ISAILA @ 2017-08-07 9:03 UTC (permalink / raw)
To: tamas
Cc: tim, sstabellini, wei.liu2, rcojocaru, George.Dunlap,
andrew.cooper3, ian.jackson, xen-devel, jbeulich
[-- Attachment #1.1: Type: text/plain, Size: 7888 bytes --]
On Vi, 2017-08-04 at 19:32 -0600, Tamas K Lengyel wrote:
On Fri, Aug 4, 2017 at 5:32 AM, Alexandru Isaila <aisaila@bitdefender.com<mailto:aisaila@bitdefender.com>> wrote:
In some introspection usecases, an in-guest agent needs to communicate
with the external introspection agent. An existing mechanism is
HVMOP_guest_request_vm_event, but this is restricted to kernel usecases
like all other hypercalls.
Introduce a mechanism whereby the introspection agent can whitelist the
use of HVMOP_guest_request_vm_event directly from userspace.
Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com<mailto:aisaila@bitdefender.com>>
---
Changes since V3:
- Changed commit message
- Added new lines
- Indent the maximum space on the defines
- Chaned the name of the define/function name/struct member
from vmcall to event
---
tools/libxc/include/xenctrl.h | 1 +
tools/libxc/xc_monitor.c | 14 ++++++++++++++
xen/arch/x86/hvm/hypercall.c | 5 +++++
xen/common/monitor.c | 14 ++++++++++++++
xen/include/public/domctl.h | 21 +++++++++++----------
xen/include/xen/sched.h | 5 +++--
6 files changed, 48 insertions(+), 12 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index bde8313..90a056f 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2022,6 +2022,7 @@ int xc_monitor_descriptor_access(xc_interface *xch, domid_t domain_id,
bool enable);
int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
bool enable, bool sync);
+int xc_allow_guest_userspace_event(xc_interface *xch, domid_t domain_id, bool enable);
int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
bool enable, bool sync);
int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool enable);
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index b44ce93..6064c39 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -161,6 +161,20 @@ int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool enable,
return do_domctl(xch, &domctl);
}
+int xc_allow_guest_userspace_event(xc_interface *xch, domid_t domain_id, bool enable)
This function should be prefixed with "xc_monitor_" like all the rest of the functions here.
+{
+ DECLARE_DOMCTL;
+
+ domctl.cmd = XEN_DOMCTL_monitor_op;
+ domctl.domain = domain_id;
+ domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
+ : XEN_DOMCTL_MONITOR_OP_DISABLE;
+ domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT;
+
+ return do_domctl(xch, &domctl);
+}
+
+
int xc_monitor_emulate_each_rep(xc_interface *xch, domid_t domain_id,
bool enable)
{
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index e7238ce..8eb5f49 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -155,6 +155,11 @@ int hvm_hypercall(struct cpu_user_regs *regs)
/* Fallthrough to permission check. */
case 4:
case 2:
+ if ( currd->monitor.guest_request_userspace_event &&
+ eax == __HYPERVISOR_hvm_op &&
+ (mode == 8 ? regs->rdi : regs->ebx) == HVMOP_guest_request_vm_event )
+ break;
+
if ( unlikely(hvm_get_cpl(curr)) )
{
default:
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
index 451f42f..21a1457 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -79,6 +79,20 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
break;
}
+ case XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT:
+ {
+ bool_t old_status = d->monitor.guest_request_enabled;
You are checking guest_request enabled here while later setting guest_request_userspace_event. These are two separate monitor options, adjust accordingly.
+
+ if ( unlikely(old_status == requested_status) )
+ return -EEXIST;
+
+ domain_pause(d);
+ d->monitor.guest_request_sync = mop->u.guest_request.sync;
You are setting guest_request_sync here which is a setting belonging to guest_request_enabled. If you need sync/async option for the userspace guest request it should be a separate bit. Since the toolstack side you add in this patch never sets the sync option I assume that is not the case, so remove this.
+ d->monitor.guest_request_userspace_event = requested_status;
+ domain_unpause(d);
+ break;
+ }
+
default:
/* Give arch-side the chance to handle this event */
return arch_monitor_domctl_event(d, mop);
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index ff39762..870495c 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1073,16 +1073,17 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
#define XEN_DOMCTL_MONITOR_OP_GET_CAPABILITIES 2
#define XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP 3
-#define XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG 0
-#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR 1
-#define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP 2
-#define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT 3
-#define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST 4
-#define XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION 5
-#define XEN_DOMCTL_MONITOR_EVENT_CPUID 6
-#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL 7
-#define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT 8
-#define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS 9
+#define XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG 0
+#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR 1
+#define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP 2
+#define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT 3
+#define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST 4
+#define XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION 5
+#define XEN_DOMCTL_MONITOR_EVENT_CPUID 6
+#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL 7
+#define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT 8
+#define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS 9
+#define XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT 10
struct xen_domctl_monitor_op {
uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 6673b27..898a132 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -480,8 +480,9 @@ struct domain
/* Common monitor options */
struct {
- unsigned int guest_request_enabled : 1;
- unsigned int guest_request_sync : 1;
+ unsigned int guest_request_enabled : 1;
+ unsigned int guest_request_sync : 1;
+ unsigned int guest_request_userspace_event : 1;
This should be "guest_request_userspace_enabled". It also should not be in xen/sched.h as on ARM there is no instruction trapping from userspace directly to the hypervisor (AFAIK).
Is everyone ok with moving the bit in a x86 specific region?
} monitor;
};
--
2.7.4
Tamas
________________________________
This email was scanned by Bitdefender
Regards,
Alex
________________________________
This email was scanned by Bitdefender
[-- Attachment #1.2: Type: text/html, Size: 13703 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4] x86/hvm: Allow guest_request vm_events coming from userspace
2017-08-07 8:38 ` Alexandru Stefan ISAILA
@ 2017-08-07 9:06 ` Jan Beulich
0 siblings, 0 replies; 8+ messages in thread
From: Jan Beulich @ 2017-08-07 9:06 UTC (permalink / raw)
To: aisaila
Cc: tim, sstabellini, wei.liu2, rcojocaru, George.Dunlap,
andrew.cooper3, ian.jackson, xen-devel, tamas
>>> Alexandru Stefan ISAILA <aisaila@bitdefender.com> 08/07/17 10:38 AM >>>
Can you please properly use quoting in your replies (and stay away from
sending HTML mails, if at all possible)? In your reply below, it is impossible
to tell which parts of the mail are actually your comments.
Jan
>>>>>>>>>>>>>> quoted original mail <<<<<<<<<<<<<<<<<<<<
________________________________
From: Tamas K Lengyel <tamas@tklengyel.com>
Sent: Saturday, August 5, 2017 4:32 AM
To: Alexandru Stefan ISAILA
Cc: Xen-devel; wei.liu2@citrix.com; Tim Deegan; Stefano Stabellini; Konrad Rzeszutek Wilk; Jan Beulich; Ian Jackson; George Dunlap; Andrew Cooper; Razvan Cojocaru
Subject: Re: [PATCH v4] x86/hvm: Allow guest_request vm_events coming from userspace
On Fri, Aug 4, 2017 at 5:32 AM, Alexandru Isaila <aisaila@bitdefender.com<mailto:aisaila@bitdefender.com>> wrote:
In some introspection usecases, an in-guest agent needs to communicate
with the external introspection agent. An existing mechanism is
HVMOP_guest_request_vm_event, but this is restricted to kernel usecases
like all other hypercalls.
Introduce a mechanism whereby the introspection agent can whitelist the
use of HVMOP_guest_request_vm_event directly from userspace.
Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com<mailto:aisaila@bitdefender.com>>
---
Changes since V3:
- Changed commit message
- Added new lines
- Indent the maximum space on the defines
- Chaned the name of the define/function name/struct member
from vmcall to event
---
tools/libxc/include/xenctrl.h | 1 +
tools/libxc/xc_monitor.c | 14 ++++++++++++++
xen/arch/x86/hvm/hypercall.c | 5 +++++
xen/common/monitor.c | 14 ++++++++++++++
xen/include/public/domctl.h | 21 +++++++++++----------
xen/include/xen/sched.h | 5 +++--
6 files changed, 48 insertions(+), 12 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index bde8313..90a056f 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2022,6 +2022,7 @@ int xc_monitor_descriptor_access(xc_interface *xch, domid_t domain_id,
bool enable);
int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
bool enable, bool sync);
+int xc_allow_guest_userspace_event(xc_interface *xch, domid_t domain_id, bool enable);
int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
bool enable, bool sync);
int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool enable);
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index b44ce93..6064c39 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -161,6 +161,20 @@ int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool enable,
return do_domctl(xch, &domctl);
}
+int xc_allow_guest_userspace_event(xc_interface *xch, domid_t domain_id, bool enable)
This function should be prefixed with "xc_monitor_" like all the rest of the functions here.
+{
+ DECLARE_DOMCTL;
+
+ domctl.cmd = XEN_DOMCTL_monitor_op;
+ domctl.domain = domain_id;
+ domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
+ : XEN_DOMCTL_MONITOR_OP_DISABLE;
+ domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT;
+
+ return do_domctl(xch, &domctl);
+}
+
+
int xc_monitor_emulate_each_rep(xc_interface *xch, domid_t domain_id,
bool enable)
{
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index e7238ce..8eb5f49 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -155,6 +155,11 @@ int hvm_hypercall(struct cpu_user_regs *regs)
/* Fallthrough to permission check. */
case 4:
case 2:
+ if ( currd->monitor.guest_request_userspace_event &&
+ eax == __HYPERVISOR+ break;
+
if ( unlikely(hvm_get_cpl(curr)) )
{
default:
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
index 451f42f..21a1457 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -79,6 +79,20 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
break;
}
+ case XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT:
+ {
+ bool_t old_status = d->monitor.guest_request_enabled;
You are checking guest_request enabled here while later setting guest_request_userspace_event. These are two separate monitor options, adjust accordingly.
+
+ if ( unlikely(old_status == requested_status) )
+ return -EEXIST;
+
+ domain_pause(d);
+ d->monitor.guest_request_sync = mop->u.guest_request.sync;
You are setting guest_request_sync here which is a setting belonging to guest_request_enabled. If you need sync/async option for the userspace guest request it should be a separate bit. Since the toolstack side you add in this patch never sets the sync option I assume that is not the case, so remove this.
+ d->monitor.guest_request_userspace_event = requested_status;
+ domain_unpause(d);
+ break;
+ }
+
default:
/* Give arch-side the chance to handle this event */
return arch_monitor_domctl_event(d, mop);
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index ff39762..870495c 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1073,16 +1073,17 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
#define XEN_DOMCTL_MONITOR_OP_GET_CAPABILITIES 2
#define XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP 3
-#define XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG 0
-#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR 1
-#define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP 2
-#define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT 3
-#define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST 4
-#define XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION 5
-#define XEN_DOMCTL_MONITOR_EVENT_CPUID 6
-#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL 7
-#define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT 8
-#define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS 9
+#define XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG 0
+#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR 1
+#define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP 2
+#define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT 3
+#define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST 4
+#define XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION 5
+#define XEN_DOMCTL_MONITOR_EVENT_CPUID 6
+#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL 7
+#define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT 8
+#define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS 9
+#define XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT 10
struct xen_domctl_monitor_op {
uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 6673b27..898a132 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -480,8 +480,9 @@ struct domain
/* Common monitor options */
struct {
- unsigned int guest_request_enabled : 1;
- unsigned int guest_request_sync : 1;
+ unsigned int guest_request_enabled : 1;
+ unsigned int guest_request_sync : 1;
+ unsigned int guest_request_userspace_event : 1;
This should be "guest_request_userspace_enabled". It also should not be in xen/sched.h as on ARM there is no instruction trapping from userspace dirIs everyone ok with moving the bit in a x86 specific region?
} monitor;
};
--
2.7.4
Tamas
Regards,
Alex
________________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-08-07 9:06 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-04 11:32 [PATCH v4] x86/hvm: Allow guest_request vm_events coming from userspace Alexandru Isaila
2017-08-04 11:42 ` Andrew Cooper
2017-08-05 1:32 ` Tamas K Lengyel
2017-08-05 8:18 ` Razvan Cojocaru
2017-08-05 23:18 ` Tamas K Lengyel
2017-08-07 8:38 ` Alexandru Stefan ISAILA
2017-08-07 9:06 ` Jan Beulich
2017-08-07 9:03 ` Alexandru Stefan ISAILA
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.