Re: [Xen-devel] [PATCH v6] x86/hvm: Allow guest_request vm_events coming from userspace
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, &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..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
Re: [Xen-devel] [PATCH v6] x86/hvm: Allow guest_request vm_events coming from userspace
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
>>> 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
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
>>> 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
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
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
>>> 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