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:35 AM, Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx> wrote:

On 09/22/2015 06:19 PM, Jan Beulich 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).

Well, the way I've read Tamas' request is that he'd like other registers
to be set as well in vm_event_set_registers() (such as EAX, and so on) -
but since I'm not sure what he'd like added and how to test his
scenarios, I thought I could just set EIP for now, and either add what
he's requesting in future versions of the series, or allow him to extend
the code as needed with future patches.

I think that the end goal for Tamas would be to just, if this flag is
set, to set at least some of the registers that come back from the
vm_event reply to the VCPU that caused the vm_event request.

Yeap, my idea would be to just set all registers to the values included in the snapshot sent back by the user. If the user doesn't change a register value in the snapshot he received, that register would effectively be reset to the same value it already had. Not perfect but IMHO harmless and reduces the code required to keep track of things.

Tamas

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