From mboxrd@z Thu Jan 1 00:00:00 1970 From: Razvan Cojocaru Subject: Re: [PATCH V2 2/2] xen: Introduce VM_EVENT_FLAG_SET_REGISTERS Date: Tue, 22 Sep 2015 18:35:11 +0300 Message-ID: <5601752F.3030807@bitdefender.com> References: <1442842294-6123-1-git-send-email-rcojocaru@bitdefender.com> <1442842294-6123-3-git-send-email-rcojocaru@bitdefender.com> <56018DA402000078000A4872@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <56018DA402000078000A4872@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: tamas@tklengyel.com, wei.liu2@citrix.com, ian.campbell@citrix.com, andrew.cooper3@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, stefano.stabellini@eu.citrix.co, stefano.stabellini@citrix.com, keir@xen.org List-Id: xen-devel@lists.xenproject.org On 09/22/2015 06:19 PM, Jan Beulich wrote: >>>> On 21.09.15 at 15:31, 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 >> >> --- >> 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. Thanks, Razvan