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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.07.15 at 02:43, <tlengyel@xxxxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -6431,6 +6431,11 @@ int hvm_debug_op(struct vcpu *v, int32_t op)
>      return rc;
>  }
>  
> +bool_t hvm_is_singlestep_supported(void)
> +{
> +    return cpu_has_monitor_trap_flag;
> +}

This being a VMX flag, it should be tested in VMX code, i.e. you'd
want to add a new hook to hvm_funcs.

> --- a/xen/arch/x86/monitor.c
> +++ b/xen/arch/x86/monitor.c
> @@ -42,6 +42,22 @@ int status_check(struct xen_domctl_monitor_op *mop, bool_t 
> status)
>      return 0;
>  }
>  
> +static inline
> +void get_capabilities(struct domain *d, struct xen_domctl_monitor_op *mop)
> +{
> +    mop->event = 0;
> +
> +    if ( !is_hvm_domain(d) || !cpu_has_vmx )
> +        return;
> +
> +    mop->event = (1 << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
> +                 (1 << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
> +                 (1 << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT);
> +
> +    if ( hvm_is_singlestep_supported() )
> +        mop->event |= (1 << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
> +}

And with this in mind I suppose the hook then would return a bit
mask to be or-ed in here, instead of just a boolean flag.

> @@ -51,6 +67,12 @@ int monitor_domctl(struct domain *d, struct 
> xen_domctl_monitor_op *mop)
>      if ( rc )
>          return rc;
>  
> +    if ( mop->op == XEN_DOMCTL_MONITOR_OP_GET_CAPABILITIES )
> +    {
> +        get_capabilities(d, mop);
> +        return 0;
> +    }
> +
>      /*
>       * At the moment only Intel HVM domains are supported. However, event
>       * delivery could be extended to AMD and PV domains.

Considering that capabilities could be domain specific (as is also
expressed by d being passed to get_capabilities()), I think the
conditional above should actually move past the one following this
comment.

> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -1167,6 +1167,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> u_domctl)
>              break;
>  
>          ret = monitor_domctl(d, &op->u.monitor_op);
> +        copyback = 1;
>          break;

Conditional upon ret == 0?

> @@ -1015,7 +1019,15 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
>  
>  struct xen_domctl_monitor_op {
>      uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
> -    uint32_t event; /* XEN_DOMCTL_MONITOR_EVENT_* */
> +
> +    /*
> +     * When used with ENABLE/DISABLE this has be set to

... has to be ... ?

Jan

> +     * the requested XEN_DOMCTL_MONITOR_EVENT_* value.
> +     * With GET_CAPABILITIES this field returns a bitmap of
> +     * events supported by the platform, in the format
> +     * (1 << XEN_DOMCTL_MONITOR_EVENT_*).
> +     */
> +    uint32_t event;
>  
>      /*
>       * Further options when issuing XEN_DOMCTL_MONITOR_OP_ENABLE.



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