| Message ID | 1501774150-11683-1-git-send-email-aisaila@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/08/17 16:29, Alexandru Isaila wrote: > Allow guest userspace code to request that a vm_event be sent out > via VMCALL. This functionality seems to be handy for a number of > Xen developers, as stated on the mailing list (thread "[Xen-devel] > HVMOP_guest_request_vm_event only works from guest in ring0"). > This is a use case in communication between a userspace application > in the guest and the introspection application in dom0. This ends up as the immutable description of the change in history, once committed. May I recommend: "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 V2: > -Added a new flag to enable the vm call from the guest > userspace > --- > tools/libxc/include/xenctrl.h | 2 ++ > tools/libxc/xc_monitor.c | 14 ++++++++++++++ > xen/arch/x86/hvm/hypercall.c | 8 ++++++++ > xen/common/monitor.c | 13 +++++++++++++ > xen/include/public/domctl.h | 21 +++++++++++---------- > xen/include/xen/sched.h | 5 +++-- > 6 files changed, 51 insertions(+), 12 deletions(-) > > diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h > index bde8313..eed60db 100644 > --- a/tools/libxc/include/xenctrl.h > +++ b/tools/libxc/include/xenctrl.h > @@ -2022,6 +2022,8 @@ 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_monitor_guest_userspace_vmcall(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..63c6320 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_vmcall(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_VMCALL; > + > + 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..c7fab4b 100644 > --- a/xen/arch/x86/hvm/hypercall.c > +++ b/xen/arch/x86/hvm/hypercall.c > @@ -152,9 +152,17 @@ int hvm_hypercall(struct cpu_user_regs *regs) > { > case 8: > eax = regs->rax; > + if ( currd->monitor.guest_request_userspace_vmcall && > + eax == __HYPERVISOR_hvm_op && > + regs->rdi == HVMOP_guest_request_vm_event ) > + break; Newline > /* Fallthrough to permission check. */ > case 4: > case 2: > + if ( mode != 8 && currd->monitor.guest_request_userspace_vmcall && > + eax == __HYPERVISOR_hvm_op && > + regs->ebx == HVMOP_guest_request_vm_event ) > + break; Newline > if ( unlikely(hvm_get_cpl(curr)) ) > { > default: > diff --git a/xen/common/monitor.c b/xen/common/monitor.c > index 451f42f..4011dc3 100644 > --- a/xen/common/monitor.c > +++ b/xen/common/monitor.c > @@ -78,6 +78,19 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop) > domain_unpause(d); > break; > } Newline > + case XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_VMCALL: > + { > + bool_t old_status = d->monitor.guest_request_enabled; and bool here. All can be fixed on commit, if there are no other comments/concerns. ~Andrew
>>> Alexandru Isaila <aisaila@bitdefender.com> 08/03/17 5:29 PM >>> >--- a/xen/arch/x86/hvm/hypercall.c >+++ b/xen/arch/x86/hvm/hypercall.c >@@ -152,9 +152,17 @@ int hvm_hypercall(struct cpu_user_regs *regs) >{ >case 8: >eax = regs->rax; >+ if ( currd->monitor.guest_request_userspace_vmcall && >+ eax == __HYPERVISOR_hvm_op && >+ regs->rdi == HVMOP_guest_request_vm_event ) >+ break; >/* Fallthrough to permission check. */ >case 4: >case 2: >+ if ( mode != 8 && currd->monitor.guest_request_userspace_vmcall && >+ eax == __HYPERVISOR_hvm_op && >+ regs->ebx == HVMOP_guest_request_vm_event ) >+ break; Let's limit ugliness and redundancy as much as possible: if ( currd->monitor.guest_request_userspace_vmcall && eax == __HYPERVISOR_hvm_op && (mode == 8 ? regs->rdi : regs->ebx) == HVMOP_guest_request_vm_event ) with the first half above dropped altogether. >--- 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 This isn't the first time I see this whole block being re-indented. I'd suggest to either increase indentation to a maximum (i.e. for the right side to be just below 80 cols) or to accept extremely long entries to stand out. +#define XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_VMCALL 10 I dislike the mention of VMCALL (which is an insn mnemonic after all) here, and I also think the name suggests broader access than is actually being granted. Realizing the redundancy I'd still think XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_EVENT would be better >--- 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_vmcall : 1; Same here then. Jan
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index bde8313..eed60db 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -2022,6 +2022,8 @@ 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_monitor_guest_userspace_vmcall(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..63c6320 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_vmcall(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_VMCALL; + + 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..c7fab4b 100644 --- a/xen/arch/x86/hvm/hypercall.c +++ b/xen/arch/x86/hvm/hypercall.c @@ -152,9 +152,17 @@ int hvm_hypercall(struct cpu_user_regs *regs) { case 8: eax = regs->rax; + if ( currd->monitor.guest_request_userspace_vmcall && + eax == __HYPERVISOR_hvm_op && + regs->rdi == HVMOP_guest_request_vm_event ) + break; /* Fallthrough to permission check. */ case 4: case 2: + if ( mode != 8 && currd->monitor.guest_request_userspace_vmcall && + eax == __HYPERVISOR_hvm_op && + 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..4011dc3 100644 --- a/xen/common/monitor.c +++ b/xen/common/monitor.c @@ -78,6 +78,19 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop) domain_unpause(d); break; } + case XEN_DOMCTL_MONITOR_EVENT_GUEST_USERSPACE_VMCALL: + { + 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_vmcall = requested_status; + domain_unpause(d); + break; + } default: /* Give arch-side the chance to handle this event */ diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index ff39762..e782517 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_VMCALL 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..11137b0 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_vmcall : 1; } monitor; };
Allow guest userspace code to request that a vm_event be sent out via VMCALL. This functionality seems to be handy for a number of Xen developers, as stated on the mailing list (thread "[Xen-devel] HVMOP_guest_request_vm_event only works from guest in ring0"). This is a use case in communication between a userspace application in the guest and the introspection application in dom0. Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com> --- Changes since V2: -Added a new flag to enable the vm call from the guest userspace --- tools/libxc/include/xenctrl.h | 2 ++ tools/libxc/xc_monitor.c | 14 ++++++++++++++ xen/arch/x86/hvm/hypercall.c | 8 ++++++++ xen/common/monitor.c | 13 +++++++++++++ xen/include/public/domctl.h | 21 +++++++++++---------- xen/include/xen/sched.h | 5 +++-- 6 files changed, 51 insertions(+), 12 deletions(-)