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

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

On Mon, Jul 6, 2015 at 11:26 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>> 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

There is no opportunity for that, the current API does not provide a mechanism to signal failure on things that were requested on the vm_event response. Creating such a mechanism is beyond the scope of this patch and I don't think it's necessary. IMHO the comment makes it clear that this will only work on Intel hardware which suffices for now.

Tamas

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