Re: [Xen-devel] [PATCH V2 2/2] xen: Introduce VM_EVENT_FLAG_SET_REGISTERS

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Tue, Sep 22, 2015 at 9:19 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>> On 21.09.15 at 15:31, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> A previous version of this patch dealing with support for skipping
> the current instruction when a vm_event response requested it
> computed the instruction length in the hypervisor, adding non-trivial
> code dependencies. This patch allows a userspace vm_event client to
> simply request that the guest's EIP is set to an arbitary value,
> computed by the introspection application. In the future, other
> registers can also be set via a vm_event reply by using this flag.
> The VCPU needs to be paused for this flag to take effect.
>
> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
>
> ---
> Changes since V1:
>Â - Renamed the patch (VM_EVENT_FLAG_SET_EIP ->
>Â Â VM_EVENT_FLAG_SET_REGISTERS).
>Â - As suggested by Tamas Lengyel, EIP is now being set via a dedicated
>Â Â generic vm_event_set_registers() function that can be extended to
>Â Â set other registers in the future.

Isn't it a bad move to call the thing "set registers" but have it set
just EIP? If going forward you were to add more registers, you'd
need new flags anyway I suppose, and hence the public interface
part of this should be reverted (while the other internal
abstraction seems fine to me).

Jan

IMHO you should just add setting all registers included in the snapshot here rather then postpone it to a later patch.

Tamas

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