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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Aug 08, 2017 at 11:27:35AM +0300, 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 <aisaila@xxxxxxxxxxxxxxx>
> 
> ---
> Changes since V4:
>       - Changed function mane from xc_allow_guest_userspace_event
>         to xc_monitor_guest_userspace_event
>       - Fixed guest_request_enabled check
>       - Delete the guest_request_sync
>       - Changed guest_request_userspace_event to
>         guest_request_userspace_enabled
>       - Moved guest_request_userspace_enabled flag from sched.h to
>         domain.h
> ---
>  tools/libxc/include/xenctrl.h |  1 +
>  tools/libxc/xc_monitor.c      | 14 ++++++++++++++
>  xen/arch/x86/hvm/hypercall.c  |  5 +++++
>  xen/common/monitor.c          | 13 +++++++++++++
>  xen/include/asm-x86/domain.h  | 19 ++++++++++---------
>  xen/include/public/domctl.h   | 21 +++++++++++----------
>  6 files changed, 54 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index bde8313..c72e12d 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_monitor_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..bd8cbcf 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_monitor_guest_userspace_event(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_EVENT;
> +
> +    return do_domctl(xch, &domctl);
> +}
> +
> +

For this bit:

Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>

Some nits below.

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

Indentation here and below seems rather excessive.

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