From mboxrd@z Thu Jan 1 00:00:00 1970 From: Razvan Cojocaru Subject: Re: [PATCH v3 1/2] hvm/vmx: save dr7 during vmx_vmcs_save Date: Mon, 22 Feb 2016 15:53:18 +0200 Message-ID: <56CB12CE.3030909@bitdefender.com> References: <1455563063-17012-1-git-send-email-tlengyel@novetta.com> <56C30C5802000078000D28E0@prv-mh.provo.novell.com> <56C74E76.5090008@citrix.com> <56CAD3EF.20801@bitdefender.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aXquL-0004Z2-GP for xen-devel@lists.xenproject.org; Mon, 22 Feb 2016 13:52:13 +0000 Received: from smtp03.buh.bitdefender.org (unknown [10.17.80.77]) by mx-sr.buh.bitdefender.com (Postfix) with ESMTP id 462C17FBEF for ; Mon, 22 Feb 2016 15:52:10 +0200 (EET) In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Lengyel, Tamas" Cc: Kevin Tian , Keir Fraser , Jan Beulich , Andrew Cooper , Jun Nakajima , "xen-devel@lists.xenproject.org" List-Id: xen-devel@lists.xenproject.org On 02/22/2016 03:51 PM, Lengyel, Tamas wrote: > > On Feb 22, 2016 04:23, "Razvan Cojocaru" > wrote: >> >> On 02/19/2016 07:26 PM, Lengyel, Tamas wrote: >> > >> > >> > On Fri, Feb 19, 2016 at 10:18 AM, Andrew Cooper >> > > >> > wrote: >> > >> > On 19/02/16 17:06, Lengyel, Tamas wrote: >> >> >> >> >> >> On Tue, Feb 16, 2016 at 3:47 AM, Jan Beulich >> >> >> wrote: >> >> >> >> >>> On 16.02.16 at 07:58, < >kevin.tian@intel.com > >> >> >> wrote: >> >> >> --- a/xen/arch/x86/hvm/vmx/vmx.c >> >> >> +++ b/xen/arch/x86/hvm/vmx/vmx.c >> >> >> @@ -490,6 +490,7 @@ static void vmx_vmcs_save(struct vcpu >> >> *v, struct hvm_hw_cpu >> >> >> *c) >> >> >> __vmread(GUEST_SYSENTER_CS, &c->sysenter_cs); >> >> >> __vmread(GUEST_SYSENTER_ESP, &c->sysenter_esp); >> >> >> __vmread(GUEST_SYSENTER_EIP, &c->sysenter_eip); >> >> >> + __vmread(GUEST_DR7, &c->dr7); >> >> >> >> >> > >> >> > Hi, Tamas, I didn't see the open closed around "v != >> >> current", if >> >> > I'm not missing some mails... Have you confirmed with Jan > that >> >> > he is OK with it? >> >> >> >> We didn't really settle on this yet. I'm not heavily > opposed to it >> >> remaining unconditional for now, but as said in the other > replay >> >> my fear is that this might later lead to further additions > which >> >> may then also be of no interest to the main (save/migration) >> >> user of this code. >> >> >> >> >> >> Andrew, any comment if this is OK from your perspective? >> > >> > I specifically suggested the use of vmx_save_dr() to make all debug >> > state consistent. >> > >> > >> > I might have missed that comment. >> > >> > >> > >> > I don't see much purpose in being able to introspect just %dr7. If >> > any debug related activities are going on, all debug registers are >> > relevant. >> > >> > Is this not the case? >> > >> > >> > Right now only dr7 is included in the automatic register snapshot sent >> > with each vm_event. I personally don't use any of them so I can't >> > comment on how it would be useful by itself (Razvan?). From my >> > perspective the only issue at hand has been that the current way dr7 was >> > gathered was incorrect. IMHO if someone needs the other debug registers >> > for each vm_event, that change can be introduced in a separate patch. >> >> Andrew is right, all debug registers are relevant for debug activities. >> In fact, I've checked with the introspection engine team, and they no >> longer use DR7 at the moment (I don't recall exactly why it has been >> requested when I first wrote the patch a few years ago). >> >> So if nobody minds - and I find it unlikely that anyone would - we can, >> for the moment, simply remove DR7 altogether from the registers sent >> with the vm_event. Should they become necessary, we should indeed >> include all of them in a future patch. >> > > I would rather not remove it if it's not necessary as it's a change in > the vm_event interface. If we do, we would have to bump the version, the > user needs to be aware of the change, etc. So while the whole vm_event > rework was done partly to allow us to do such changes, I would rather > just keep it for now. Fine with me. Thanks, Razvan