Re: [Xen-devel] [PATCH v3] x86/hvm: Allow guest_request vm_events coming from userspace

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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@xxxxxxxxxxxxxxx>
>
> ---
> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel