On Fri, Aug 4, 2017 at 5:32 AM, 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 > > --- > 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