Re: [Xen-devel] [PATCH v2] x86/vm_event: toggle singlestep from vm_event response

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

On Tue, Jun 30, 2015 at 10:18 AM, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:

On 30/06/15 15:11, Tamas K Lengyel wrote:
> diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
> index 577e971..b8c3dde 100644
> --- a/xen/include/public/vm_event.h
> +++ b/xen/include/public/vm_event.h
> @@ -44,9 +44,15 @@
>   *  paused
>   * VCPU_PAUSED in a response signals to unpause the vCPU
>   */
> -#define VM_EVENT_FLAG_VCPU_PAUSED     (1 << 0)
> -/* Flags to aid debugging mem_event */
> -#define VM_EVENT_FLAG_FOREIGN         (1 << 1)
> +#define VM_EVENT_FLAG_VCPU_PAUSED       (1 << 0)
> +/* Flag to aid debugging mem_event */
> +#define VM_EVENT_FLAG_FOREIGN           (1 << 1)
> +/*
> + * Toggle singlestepping on vm_event response.
> + * Requires the vCPU to be paused already (synchronous events only).
> + * Only supported on Intel CPUs with MTF capability.

This sentence shouldn't be in the public API.  It is a limitation of the
current implementation, not of the API, and could be removed with
further development.

I disagree because there is no error condition returned if a user tries to use it on non-Intel hw, so the only option a user would have to figure out why it's not working is reading the Xen source. IMHO the public API should describe the limitations as that's what potential users will read first. When we have hardware other then Intel that supports something like this, we can remove the comment.

Tamas

 

With this removed, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

> + */
> +#define VM_EVENT_FLAG_TOGGLE_SINGLESTEP (1 << 2)
>
>  /*
>   * Reasons for the vm event request

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