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

2017-08-25 Thread Jan Beulich 
>>> On 25.08.17 at 17:49,  wrote:
> On 08/25/2017 02:44 PM, Jan Beulich wrote:
> On 25.08.17 at 15:00,  wrote:
>>> On Vi, 2017-08-25 at 06:13 -0600, Jan Beulich wrote:
>
>>
>>>
>>> On 17.08.17 at 13:50,  wrote:
> --- 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;
> +d->arch.monitor.guest_request_userspace_enabled = mop-
>> u.guest_request.allow_userspace;
 This breaks the build on ARM.
>>> There are 2 solutions, I can move the case in x86/monitor.c in
>>> the arch_monitor_domctl_event function or I can make a arch specific
>>> function that does the assignment in the x86 case and does nothing in
>>> the arm case. What approach do you prefer?
>> 
>> That's a question to the maintainers of that code. What I care
>> about is that patches touching common code please are at least
>> build-checked on the other architecture before submission.
> 
> I don't think this is a reasonable requirement.  That's what push gates
> (and Travis) are for.

Hmm, I can see your way of thinking, but to me (doing an ARM
build test if I don't forget it before pushing) it's a waste of time
to apply a patch only to then find it breaks the build and needs
reverting.

Jan


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

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

2017-08-25 Thread George Dunlap 
On 08/25/2017 02:44 PM, Jan Beulich wrote:
 On 25.08.17 at 15:00,  wrote:
>> On Vi, 2017-08-25 at 06:13 -0600, Jan Beulich wrote:

>
>>
>> On 17.08.17 at 13:50,  wrote:
 --- 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;
 +d->arch.monitor.guest_request_userspace_enabled = mop-
> u.guest_request.allow_userspace;
>>> This breaks the build on ARM.
>> There are 2 solutions, I can move the case in x86/monitor.c in
>> the arch_monitor_domctl_event function or I can make a arch specific
>> function that does the assignment in the x86 case and does nothing in
>> the arm case. What approach do you prefer?
> 
> That's a question to the maintainers of that code. What I care
> about is that patches touching common code please are at least
> build-checked on the other architecture before submission.

I don't think this is a reasonable requirement.  That's what push gates
(and Travis) are for.

 -George

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

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

2017-08-25 Thread Tamas K Lengyel 
On Fri, Aug 25, 2017 at 7:44 AM, Jan Beulich  wrote:
 On 25.08.17 at 15:00,  wrote:
>> On Vi, 2017-08-25 at 06:13 -0600, Jan Beulich wrote:
>>> >
>>> > >
>>> > > >
>>> > > > On 17.08.17 at 13:50,  wrote:
>>> > --- 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;
>>> > +d->arch.monitor.guest_request_userspace_enabled = mop-
>>> > >u.guest_request.allow_userspace;
>>> This breaks the build on ARM.
>> There are 2 solutions, I can move the case in x86/monitor.c in
>> the arch_monitor_domctl_event function or I can make a arch specific
>> function that does the assignment in the x86 case and does nothing in
>> the arm case. What approach do you prefer?
>
> That's a question to the maintainers of that code. What I care
> about is that patches touching common code please are at least
> build-checked on the other architecture before submission.
>

Ough, yes, please build-check your code on both architectures before
sending them. As for which route to take I prefer in this case doing
the arch specific function that does the assignment when needed and is
empty when not. The function itself could probably be declared as
static inline too.

Tamas

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

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

2017-08-25 Thread Jan Beulich 
>>> On 25.08.17 at 15:00,  wrote:
> On Vi, 2017-08-25 at 06:13 -0600, Jan Beulich wrote:
>> >
>> > >
>> > > >
>> > > > On 17.08.17 at 13:50,  wrote:
>> > --- 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;
>> > +d->arch.monitor.guest_request_userspace_enabled = mop-
>> > >u.guest_request.allow_userspace;
>> This breaks the build on ARM.
> There are 2 solutions, I can move the case in x86/monitor.c in
> the arch_monitor_domctl_event function or I can make a arch specific
> function that does the assignment in the x86 case and does nothing in
> the arm case. What approach do you prefer?

That's a question to the maintainers of that code. What I care
about is that patches touching common code please are at least
build-checked on the other architecture before submission.

Jan


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

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

2017-08-25 Thread Alexandru Stefan ISAILA 
On Vi, 2017-08-25 at 06:13 -0600, Jan Beulich wrote:
> >
> > >
> > > >
> > > > On 17.08.17 at 13:50,  wrote:
> > --- 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;
> > +d->arch.monitor.guest_request_userspace_enabled = mop-
> > >u.guest_request.allow_userspace;
> This breaks the build on ARM.
There are 2 solutions, I can move the case in x86/monitor.c in
the arch_monitor_domctl_event function or I can make a arch specific
function that does the assignment in the x86 case and does nothing in
the arm case. What approach do you prefer?
>
> Jan
>
>
> 
> This email was scanned by Bitdefender

Thanks,
Alex


This email was scanned by Bitdefender
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

2017-08-25 Thread Jan Beulich 
>>> On 17.08.17 at 13:50,  wrote:
> --- 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;
> +d->arch.monitor.guest_request_userspace_enabled = 
> mop->u.guest_request.allow_userspace;

This breaks the build on ARM.

Jan


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

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

2017-08-22 Thread Wei Liu 
On Thu, Aug 17, 2017 at 02:50:19PM +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 
> 
> ---
> Changes since V5:
>   - Added the bool allow_userspace to the xc_monitor_guest_request
>function
> ---
>  tools/libxc/include/xenctrl.h |  2 +-
>  tools/libxc/xc_monitor.c  |  3 ++-

Acked-by: Wei Liu 

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

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

2017-08-18 Thread Tamas K Lengyel 
On Thu, Aug 17, 2017 at 5:50 AM, 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 

Acked-by: Tamas K Lengyel 

>
> ---
> Changes since V5:
> - Added the bool allow_userspace to the xc_monitor_guest_request
>  function
> ---
>  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-x86/domain.h  | 19 ++-
>  xen/include/public/domctl.h   |  1 +
>  6 files changed, 20 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, );
>  }
> 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..20463e0 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;
> +d->arch.monitor.guest_request_userspace_enabled = 
> mop->u.guest_request.allow_userspace;
>  domain_unpause(d);
>  break;
>  }
> 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

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

2017-08-17 Thread 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 

---
Changes since V5:
- Added the bool allow_userspace to the xc_monitor_guest_request
 function
---
 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-x86/domain.h  | 19 ++-
 xen/include/public/domctl.h   |  1 +
 6 files changed, 20 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, );
 }
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..20463e0 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;
+d->arch.monitor.guest_request_userspace_enabled = 
mop->u.guest_request.allow_userspace;
 domain_unpause(d);
 break;
 }
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