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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.06.15 at 16:40, <tlengyel@xxxxxxxxxxx> wrote:
> 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.

FWIW I agree with Andrew, and if on non-Intel hardware there's
no error (or other indication) being returned, that's actually an
issue to be fixed imo.

Jan


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