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

["Lengyel, Tamas" <tlengyel@xxxxxxxxxxx>]

On Jul 9, 2015 5:15 AM, "Ian Campbell" <ian.campbell@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxx>
> > ---
> > 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.

Ack.

>
> 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@xxxxxxxxxx>
>
> From glancing at the diffstats I think this is the only tools impact of
> this whole series, please correct me if I'm wrong.

It is, yes.
>
> Ian.
>

Thanks!

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel