Re: [Xen-devel] [PATCH V2 1/2] xen, libxc: Fine grained control of REP emulation optimizations
On Mon, 2015-09-21 at 16:31 +0300, Razvan Cojocaru wrote: > diff --git a/tools/libxc/include/xenctrl.h > b/tools/libxc/include/xenctrl.h > index 3482544..3bfa00b 100644 > --- a/tools/libxc/include/xenctrl.h > +++ b/tools/libxc/include/xenctrl.h > @@ -2428,6 +2428,18 @@ int xc_monitor_software_breakpoint(xc_interface > *xch, domid_t domain_id, > int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, > bool enable, bool sync); > > +/** > + * This function enables / disables emulation for each REP for a > + * REP-compatible instruction. > + * > + * @parm xch a handle to an open hypervisor interface. > + * @parm domain_id the domain id one wants to get the node affinity of. > + * @parm enable if 0 optimize when possible, else emulate each REP. > + * @return 0 on success, -1 on failure. > + */ > +int xc_monitor_emulate_each_rep(xc_interface *xch, domid_t domain_id, > +bool enable); > + > /*** > * Memory sharing operations. > * > diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c > index 065669c..b1705dd 100644 > --- a/tools/libxc/xc_monitor.c > +++ b/tools/libxc/xc_monitor.c > @@ -143,3 +143,16 @@ int xc_monitor_guest_request(xc_interface *xch, > domid_t domain_id, bool enable, > > return do_domctl(xch, ); > } > + > +int xc_monitor_emulate_each_rep(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 = XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP; > +domctl.u.monitor_op.event = enable; > + > +return do_domctl(xch, ); > +} This is a plausible binding of a hypercall interface so from the toolside if the hypervisor people are happy with the nderlying inteface: Acked-by: Ian Campbell___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V2 1/2] xen, libxc: Fine grained control of REP emulation optimizations
On 09/22/2015 06:39 PM, Jan Beulich wrote: On 22.09.15 at 17:28,wrote: >> On 09/22/2015 06:17 PM, Jan Beulich wrote: >> On 21.09.15 at 15:31, wrote: --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -514,7 +514,8 @@ static int hvmemul_virtual_to_linear( * being triggered for repeated writes to a whole page. */ *reps = min_t(unsigned long, *reps, - unlikely(current->domain->arch.mem_access_emulate_enabled) + unlikely(current->domain->arch.mem_access_emulate_enabled && + current->domain->arch.mem_access_emulate_each_rep) ? 1 : 4096); >>> >>> unlikely() should not wrap compound conditions, or else its effect of >>> eliminating mis-predicted branches from the fast path won't be >>> achieved. In the case here I wonder though whether you couldn't >>> simply test only ->arch.mem_access_emulate_each_rep. >> >> I'll unfold the unlikely(). >> >> Testing only ->arch.mem_access_emulate_each_rep is what I had done in >> the original patch, however on Andrew Cooper's suggestion I've now gated >> this on ->domain->arch.mem_access_emulate_enabled as well. >> >> Otherwise, somebody might set mem_access_emulate_each_rep via its >> xc_monitor_*() call, but then after calling xc_monitor_disable() it >> would still be in effect, even if the guest is no longer being monitored. >> >> If this is not a problem, I'm happy to check just >> ->arch.mem_access_emulate_each_rep. > > Or perhaps "disabled" should just clear that flag, also to not surprise > the next one to "enable"? Fair point, I'll do that. Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V2 1/2] xen, libxc: Fine grained control of REP emulation optimizations
>>> On 21.09.15 at 15:31,wrote: > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -514,7 +514,8 @@ static int hvmemul_virtual_to_linear( > * being triggered for repeated writes to a whole page. > */ > *reps = min_t(unsigned long, *reps, > - unlikely(current->domain->arch.mem_access_emulate_enabled) > + unlikely(current->domain->arch.mem_access_emulate_enabled > && > + current->domain->arch.mem_access_emulate_each_rep) > ? 1 : 4096); unlikely() should not wrap compound conditions, or else its effect of eliminating mis-predicted branches from the fast path won't be achieved. In the case here I wonder though whether you couldn't simply test only ->arch.mem_access_emulate_each_rep. > --- a/xen/arch/x86/monitor.c > +++ b/xen/arch/x86/monitor.c > @@ -79,6 +79,12 @@ int monitor_domctl(struct domain *d, struct > xen_domctl_monitor_op *mop) > return 0; > } > > +if ( mop->op == XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP ) > +{ > +d->arch.mem_access_emulate_each_rep = !!mop->event; > +return 0; > +} Considering that there's another "if(mop->op == ...)" right above this, these two together should become another switch(). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V2 1/2] xen, libxc: Fine grained control of REP emulation optimizations
On 09/22/2015 06:17 PM, Jan Beulich wrote: On 21.09.15 at 15:31,wrote: >> --- a/xen/arch/x86/hvm/emulate.c >> +++ b/xen/arch/x86/hvm/emulate.c >> @@ -514,7 +514,8 @@ static int hvmemul_virtual_to_linear( >> * being triggered for repeated writes to a whole page. >> */ >> *reps = min_t(unsigned long, *reps, >> - unlikely(current->domain->arch.mem_access_emulate_enabled) >> + unlikely(current->domain->arch.mem_access_emulate_enabled >> && >> + >> current->domain->arch.mem_access_emulate_each_rep) >> ? 1 : 4096); > > unlikely() should not wrap compound conditions, or else its effect of > eliminating mis-predicted branches from the fast path won't be > achieved. In the case here I wonder though whether you couldn't > simply test only ->arch.mem_access_emulate_each_rep. I'll unfold the unlikely(). Testing only ->arch.mem_access_emulate_each_rep is what I had done in the original patch, however on Andrew Cooper's suggestion I've now gated this on ->domain->arch.mem_access_emulate_enabled as well. Otherwise, somebody might set mem_access_emulate_each_rep via its xc_monitor_*() call, but then after calling xc_monitor_disable() it would still be in effect, even if the guest is no longer being monitored. If this is not a problem, I'm happy to check just ->arch.mem_access_emulate_each_rep. >> --- a/xen/arch/x86/monitor.c >> +++ b/xen/arch/x86/monitor.c >> @@ -79,6 +79,12 @@ int monitor_domctl(struct domain *d, struct >> xen_domctl_monitor_op *mop) >> return 0; >> } >> >> +if ( mop->op == XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP ) >> +{ >> +d->arch.mem_access_emulate_each_rep = !!mop->event; >> +return 0; >> +} > > Considering that there's another "if(mop->op == ...)" right above > this, these two together should become another switch(). Understood. Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V2 1/2] xen, libxc: Fine grained control of REP emulation optimizations
>>> On 22.09.15 at 17:28,wrote: > On 09/22/2015 06:17 PM, Jan Beulich wrote: > On 21.09.15 at 15:31, wrote: >>> --- a/xen/arch/x86/hvm/emulate.c >>> +++ b/xen/arch/x86/hvm/emulate.c >>> @@ -514,7 +514,8 @@ static int hvmemul_virtual_to_linear( >>> * being triggered for repeated writes to a whole page. >>> */ >>> *reps = min_t(unsigned long, *reps, >>> - >>> unlikely(current->domain->arch.mem_access_emulate_enabled) >>> + >>> unlikely(current->domain->arch.mem_access_emulate_enabled && >>> + >>> current->domain->arch.mem_access_emulate_each_rep) >>> ? 1 : 4096); >> >> unlikely() should not wrap compound conditions, or else its effect of >> eliminating mis-predicted branches from the fast path won't be >> achieved. In the case here I wonder though whether you couldn't >> simply test only ->arch.mem_access_emulate_each_rep. > > I'll unfold the unlikely(). > > Testing only ->arch.mem_access_emulate_each_rep is what I had done in > the original patch, however on Andrew Cooper's suggestion I've now gated > this on ->domain->arch.mem_access_emulate_enabled as well. > > Otherwise, somebody might set mem_access_emulate_each_rep via its > xc_monitor_*() call, but then after calling xc_monitor_disable() it > would still be in effect, even if the guest is no longer being monitored. > > If this is not a problem, I'm happy to check just > ->arch.mem_access_emulate_each_rep. Or perhaps "disabled" should just clear that flag, also to not surprise the next one to "enable"? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH V2 1/2] xen, libxc: Fine grained control of REP emulation optimizations
Previously, if vm_event emulation support was enabled, then REP optimizations were disabled when emulating REP-compatible instructions. This patch allows fine-tuning of this behaviour by providing a dedicated libxc helper function. Signed-off-by: Razvan Cojocaru--- Changes since V1: - Renamed the patch, since this is now a xc_monitor_ function, so XEN_DOMCTL_emulate_each_rep no longer applies. - As suggested by Andrew Cooper, the function is a no-op unless mem_access emulation is enabled. --- tools/libxc/include/xenctrl.h | 12 tools/libxc/xc_monitor.c | 13 + xen/arch/x86/hvm/emulate.c|3 ++- xen/arch/x86/monitor.c|6 ++ xen/include/asm-x86/domain.h |1 + xen/include/public/domctl.h |1 + 6 files changed, 35 insertions(+), 1 deletion(-) diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index 3482544..3bfa00b 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -2428,6 +2428,18 @@ int xc_monitor_software_breakpoint(xc_interface *xch, domid_t domain_id, int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool enable, bool sync); +/** + * This function enables / disables emulation for each REP for a + * REP-compatible instruction. + * + * @parm xch a handle to an open hypervisor interface. + * @parm domain_id the domain id one wants to get the node affinity of. + * @parm enable if 0 optimize when possible, else emulate each REP. + * @return 0 on success, -1 on failure. + */ +int xc_monitor_emulate_each_rep(xc_interface *xch, domid_t domain_id, +bool enable); + /*** * Memory sharing operations. * diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c index 065669c..b1705dd 100644 --- a/tools/libxc/xc_monitor.c +++ b/tools/libxc/xc_monitor.c @@ -143,3 +143,16 @@ int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool enable, return do_domctl(xch, ); } + +int xc_monitor_emulate_each_rep(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 = XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP; +domctl.u.monitor_op.event = enable; + +return do_domctl(xch, ); +} diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index 5934c72..c39a883 100644 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -514,7 +514,8 @@ static int hvmemul_virtual_to_linear( * being triggered for repeated writes to a whole page. */ *reps = min_t(unsigned long, *reps, - unlikely(current->domain->arch.mem_access_emulate_enabled) + unlikely(current->domain->arch.mem_access_emulate_enabled && + current->domain->arch.mem_access_emulate_each_rep) ? 1 : 4096); reg = hvmemul_get_seg_reg(seg, hvmemul_ctxt); diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c index 3d52135..3cb7519 100644 --- a/xen/arch/x86/monitor.c +++ b/xen/arch/x86/monitor.c @@ -79,6 +79,12 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop) return 0; } +if ( mop->op == XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP ) +{ +d->arch.mem_access_emulate_each_rep = !!mop->event; +return 0; +} + /* * Sanity check */ diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h index 59cf826..a088110 100644 --- a/xen/include/asm-x86/domain.h +++ b/xen/include/asm-x86/domain.h @@ -386,6 +386,7 @@ struct arch_domain /* Mem_access emulation control */ bool_t mem_access_emulate_enabled; +bool_t mem_access_emulate_each_rep; } __cacheline_aligned; #define has_arch_pdevs(d)(!list_empty(&(d)->arch.pdev_list)) diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index 794d4d5..ae241f2 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -1007,6 +1007,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t); #define XEN_DOMCTL_MONITOR_OP_ENABLE0 #define XEN_DOMCTL_MONITOR_OP_DISABLE 1 #define XEN_DOMCTL_MONITOR_OP_GET_CAPABILITIES 2 +#define XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP 3 #define XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG 0 #define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR1 -- 1.7.9.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel