Re: [PATCH v4 1/3] x86/monitor: add get_capabilities to monitor_op domctl

[Ian Campbell <ian.campbell@citrix.com>]

On Wed, 2015-07-08 at 18:52 -0400, Tamas K Lengyel wrote:
> Add option to monitor_op domctl to determine the monitor capabilities of the
> system.
> 
> Signed-off-by: Tamas K Lengyel <tlengyel@novetta.com>
> ---
> v4: add inline function wrapper for is_singlestep_supported to hvm.h
> v3: move is_singlestep_supported into vmx
>     sanity check capabilities for each monitor_op enable/disable
>     fix comment typo
> v2: skipped
> ---
>  tools/libxc/include/xenctrl.h |  6 ++++++
>  tools/libxc/xc_monitor.c      | 21 +++++++++++++++++++++
>  xen/arch/x86/hvm/vmx/vmx.c    |  6 ++++++
>  xen/arch/x86/monitor.c        | 33 +++++++++++++++++++++++++++++++--
>  xen/common/domctl.c           |  2 ++
>  xen/include/asm-x86/hvm/hvm.h |  6 ++++++
>  xen/include/public/domctl.h   | 18 +++++++++++++++---
>  7 files changed, 87 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 71539a7..4fe347b 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2364,6 +2364,12 @@ int xc_mem_access_disable_emulate(xc_interface *xch, domid_t domain_id);
>  void *xc_monitor_enable(xc_interface *xch, domid_t domain_id, uint32_t *port);
>  int xc_monitor_disable(xc_interface *xch, domid_t domain_id);
>  int xc_monitor_resume(xc_interface *xch, domid_t domain_id);
> +/*
> + * Get a bitmap of supported monitor events in the form
> + * (1 << XEN_DOMCTL_MONITOR_EVENT_*).
> + */
> +int xc_monitor_get_capabilities(xc_interface *xch, domid_t domain_id,
> +                                uint32_t *capabilities);
>  int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
>                               uint16_t index, bool enable, bool sync,
>                               bool onchangeonly);
> diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
> index 63013de..3221bdd 100644
> --- a/tools/libxc/xc_monitor.c
> +++ b/tools/libxc/xc_monitor.c
> @@ -45,6 +45,27 @@ int xc_monitor_resume(xc_interface *xch, domid_t domain_id)
>                                 NULL);
>  }
>  
> +int xc_monitor_get_capabilities(xc_interface *xch, domid_t domain_id,
> +                                uint32_t *capabilities)
> +{
> +    int rc;
> +    DECLARE_DOMCTL;
> +
> +    if ( !capabilities )
> +        return -EINVAL;

Should be:
        errno = EINVAL;
        return -1;

Lots of libxc gets this wrong, but a) lets try not to make things worse
and b) do_domctl which was used below gets it right so this function is
effectively mixing error reporting schemes.

Other than that this looks like a valid wrapping of a hypercall, so with
the above fixed and if the hypervisor people are happy with the domctl
interface itself:
        Acked-by: Ian Campbell <ian.campbell@citrix.com>

>From glancing at the diffstats I think this is the only tools impact of
this whole series, please correct me if I'm wrong.

Ian.