All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] x86/hvm: Allow guest_request vm_events coming from userspace
@ 2017-08-03 15:29 Alexandru Isaila
  2017-08-03 15:48 ` Andrew Cooper
  2017-08-03 16:01 ` Jan Beulich
  0 siblings, 2 replies; 3+ messages in thread
From: Alexandru Isaila @ 2017-08-03 15:29 UTC (permalink / raw)
  To: xen-devel
  Cc: sstabellini, wei.liu2, rcojocaru, George.Dunlap, ian.jackson,
	tim, tamas, jbeulich, andrew.cooper3, Alexandru Isaila

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(-)

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;
 };
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH v3] x86/hvm: Allow guest_request vm_events coming from userspace
  2017-08-03 15:29 [PATCH v3] x86/hvm: Allow guest_request vm_events coming from userspace Alexandru Isaila
@ 2017-08-03 15:48 ` Andrew Cooper
  2017-08-03 16:01 ` Jan Beulich
  1 sibling, 0 replies; 3+ messages in thread
From: Andrew Cooper @ 2017-08-03 15:48 UTC (permalink / raw)
  To: Alexandru Isaila, xen-devel
  Cc: sstabellini, wei.liu2, rcojocaru, George.Dunlap, ian.jackson,
	tim, tamas, jbeulich

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH v3] x86/hvm: Allow guest_request vm_events coming from userspace
  2017-08-03 15:29 [PATCH v3] x86/hvm: Allow guest_request vm_events coming from userspace Alexandru Isaila
  2017-08-03 15:48 ` Andrew Cooper
@ 2017-08-03 16:01 ` Jan Beulich
  1 sibling, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2017-08-03 16:01 UTC (permalink / raw)
  To: aisaila
  Cc: tim, sstabellini, wei.liu2, rcojocaru, George.Dunlap,
	andrew.cooper3, ian.jackson, xen-devel, tamas

>>> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-08-03 16:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-03 15:29 [PATCH v3] x86/hvm: Allow guest_request vm_events coming from userspace Alexandru Isaila
2017-08-03 15:48 ` Andrew Cooper
2017-08-03 16:01 ` Jan Beulich

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.