[PATCH v10] x86/hvm: Allow guest_request vm_events coming from userspace

[PATCH v10] x86/hvm: Allow guest_request vm_events coming from userspace

[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>
Acked-by: Wei Liu <wei.liu2@citrix.com>

---
Changes since V9:
	- Changed allow_userspace type from uint8_t ro bool
	- Added Wei Liu's ack from v7

Note: Could not test on ARM, compiled both on arm and x86
---
 tools/libxc/include/xenctrl.h |  2 +-
 tools/libxc/xc_monitor.c      |  3 ++-
 xen/arch/x86/hvm/hypercall.c  |  5 +++++
 xen/common/monitor.c          |  1 +
 xen/include/asm-arm/monitor.h |  6 ++++++
 xen/include/asm-x86/domain.h  | 19 ++++++++++---------
 xen/include/asm-x86/monitor.h |  6 ++++++
 xen/include/public/domctl.h   |  1 +
 8 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index bde8313..a3d0929 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2021,7 +2021,7 @@ int xc_monitor_software_breakpoint(xc_interface *xch, domid_t domain_id,
 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);
+                             bool enable, bool sync, bool allow_userspace);
 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..a677820 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -147,7 +147,7 @@ int xc_monitor_descriptor_access(xc_interface *xch, domid_t domain_id,
 }
 
 int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool enable,
-                             bool sync)
+                             bool sync, bool allow_userspace)
 {
     DECLARE_DOMCTL;
 
@@ -157,6 +157,7 @@ int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool enable,
                                     : XEN_DOMCTL_MONITOR_OP_DISABLE;
     domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST;
     domctl.u.monitor_op.u.guest_request.sync = sync;
+    domctl.u.monitor_op.u.guest_request.allow_userspace = enable ? allow_userspace : false;
 
     return do_domctl(xch, &domctl);
 }
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index e7238ce..5742dd1 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->arch.monitor.guest_request_userspace_enabled &&
+            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..4c540e5 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -75,6 +75,7 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
         domain_pause(d);
         d->monitor.guest_request_sync = mop->u.guest_request.sync;
         d->monitor.guest_request_enabled = requested_status;
+        arch_monitor_allow_userspace(d, mop->u.guest_request.allow_userspace);
         domain_unpause(d);
         break;
     }
diff --git a/xen/include/asm-arm/monitor.h b/xen/include/asm-arm/monitor.h
index 1c4fea3..2bdad7d 100644
--- a/xen/include/asm-arm/monitor.h
+++ b/xen/include/asm-arm/monitor.h
@@ -26,6 +26,12 @@
 #include <public/domctl.h>
 
 static inline
+void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
+{
+    return;
+}
+
+static inline
 int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
 {
     /* No arch-specific monitor ops on ARM. */
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index c10522b..de02507 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -396,15 +396,16 @@ struct arch_domain
 
     /* Arch-specific monitor options */
     struct {
-        unsigned int write_ctrlreg_enabled       : 4;
-        unsigned int write_ctrlreg_sync          : 4;
-        unsigned int write_ctrlreg_onchangeonly  : 4;
-        unsigned int singlestep_enabled          : 1;
-        unsigned int software_breakpoint_enabled : 1;
-        unsigned int debug_exception_enabled     : 1;
-        unsigned int debug_exception_sync        : 1;
-        unsigned int cpuid_enabled               : 1;
-        unsigned int descriptor_access_enabled   : 1;
+        unsigned int write_ctrlreg_enabled                                 : 4;
+        unsigned int write_ctrlreg_sync                                    : 4;
+        unsigned int write_ctrlreg_onchangeonly                            : 4;
+        unsigned int singlestep_enabled                                    : 1;
+        unsigned int software_breakpoint_enabled                           : 1;
+        unsigned int debug_exception_enabled                               : 1;
+        unsigned int debug_exception_sync                                  : 1;
+        unsigned int cpuid_enabled                                         : 1;
+        unsigned int descriptor_access_enabled                             : 1;
+        unsigned int guest_request_userspace_enabled                       : 1;
         struct monitor_msr_bitmap *msr_bitmap;
         uint64_t write_ctrlreg_mask[4];
     } monitor;
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index c5c323b..765d0b4 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -33,6 +33,12 @@ struct monitor_msr_bitmap {
 };
 
 static inline
+void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
+{
+    d->arch.monitor.guest_request_userspace_enabled = allow_userspace;
+}
+
+static inline
 int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
 {
     int rc = 0;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index ff39762..5997c52 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1124,6 +1124,7 @@ struct xen_domctl_monitor_op {
         struct {
             /* Pause vCPU until response */
             uint8_t sync;
+            uint8_t allow_userspace;
         } guest_request;
 
         struct {
-- 
2.7.4


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

Re: [PATCH v10] x86/hvm: Allow guest_request vm_events coming from userspace

[Jan Beulich]

>>> On 29.08.17 at 11:23, <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>
> Acked-by: Wei Liu <wei.liu2@citrix.com>

For the pieces it applies to
Acked-by: Jan Beulich <jbeulich@suse.com>

However, as I keep looking at pieces which shouldn't really require
my attention, I've noticed one more cosmetic issue:

> --- a/xen/include/asm-arm/monitor.h
> +++ b/xen/include/asm-arm/monitor.h
> @@ -26,6 +26,12 @@
>  #include <public/domctl.h>
>  
>  static inline
> +void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
> +{
> +    return;
> +}

I don't see the point of the return statement here. But I'll leave
it to the ARM maintainers, and it would be easy to drop while
committing if no other issues are going to arise.

Jan


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

Re: [PATCH v10] x86/hvm: Allow guest_request vm_events coming from userspace

[Tamas K Lengyel]

On Tue, Aug 29, 2017 at 3:36 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 29.08.17 at 11:23, <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>
>> Acked-by: Wei Liu <wei.liu2@citrix.com>
>
> For the pieces it applies to
> Acked-by: Jan Beulich <jbeulich@suse.com>
>
> However, as I keep looking at pieces which shouldn't really require
> my attention, I've noticed one more cosmetic issue:
>
>> --- a/xen/include/asm-arm/monitor.h
>> +++ b/xen/include/asm-arm/monitor.h
>> @@ -26,6 +26,12 @@
>>  #include <public/domctl.h>
>>
>>  static inline
>> +void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
>> +{
>> +    return;
>> +}
>
> I don't see the point of the return statement here. But I'll leave
> it to the ARM maintainers, and it would be easy to drop while
> committing if no other issues are going to arise.
>

I would bet that a sane compiler wouldn't generate anything for that
with or without return in there but sure, if we can drop it then why
not.

Tamas

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

Re: [PATCH v10] x86/hvm: Allow guest_request vm_events coming from userspace

[Tamas K Lengyel]

On Tue, Aug 29, 2017 at 3:23 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>
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Tamas K Lengyel <tamas@tklengyel.com>

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