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

[Alexandru Stefan ISAILA <aisaila@bitdefender.com>]

On Ma, 2017-08-08 at 12:27 +0100, Wei Liu wrote:
> 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@bitdefender.com>
> > 
> > ---
> > 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@citrix.com>
> 
> 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.
This indentation was a suggestion made by Jan Beulich on Patch V3.
> 
> ________________________
> This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel