Re: [PATCH 1/2] xen, libxc: Introduced XEN_DOMCTL_emulate_each_rep

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 15/09/15 10:19, Razvan Cojocaru wrote:
> 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 <rcojocaru@bitdefender.com>

This disables all rep optimisations by default, so on its own is
inappropriate.

I am also not sure that an individual domctl subop is appropriate.  Its
purpose is to undo a performance hit caused by introspection, so should
live as an introspection subop IMO.

> ---
>  tools/libxc/include/xenctrl.h    |   11 +++++++++++
>  tools/libxc/xc_domain.c          |   18 ++++++++++++++++++
>  xen/arch/x86/hvm/emulate.c       |    2 +-
>  xen/common/domctl.c              |    5 +++++
>  xen/include/asm-x86/hvm/domain.h |    1 +
>  xen/include/public/domctl.h      |    8 ++++++++
>  6 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 3482544..4ece851 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -643,6 +643,17 @@ int xc_domain_node_getaffinity(xc_interface *xch,
>                                 xc_nodemap_t nodemap);
>  
>  /**
> + * This function enables / disables emulation for each REP for a
> + * REP-compatible instruction.
> + *
> + * @parm xch a handle to an open hypervisor interface.
> + * @parm domid 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_domain_emulate_each_rep(xc_interface *xch, uint32_t domid, int enable);
> +
> +/**
>   * This function specifies the CPU affinity for a vcpu.
>   *
>   * There are two kinds of affinity. Soft affinity is on what CPUs a vcpu
> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> index e7278dd..19b2e46 100644
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -2555,6 +2555,24 @@ int xc_domain_soft_reset(xc_interface *xch,
>      domctl.domain = (domid_t)domid;
>      return do_domctl(xch, &domctl);
>  }
> +
> +int xc_domain_emulate_each_rep(xc_interface *xch, uint32_t domid, int enable)
> +{
> +    int ret = -1;
> +    DECLARE_DOMCTL;
> +
> +    domctl.cmd = XEN_DOMCTL_emulate_each_rep;
> +    domctl.domain = (domid_t)domid;
> +    domctl.u.emulate_each_rep.op = enable;
> +
> +    ret = do_domctl(xch, &domctl);
> +
> +    if ( ret == -ESRCH )
> +        errno = ENOENT;

Why do you override ESRCH?  ESRCH is the expected error for a missing
domain.

~Andrew