From: "Jan Beulich" Subject: Re: [RFC] x86: more debugging adjustments Date: Thu, 22 Nov 2007 15:47:05 +0000 Message-ID: <4745B289.76E4.0078.0@novell.com> References: <4745893A.76E4.0078.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org >>> Keir Fraser 22.11.07 15:50 >>> >Inject-sstep: Looks fine in principle. A few specific questions: > * Why is all the RFLAGS.RF logic removed from platform.c? As it seems to me it was misplaced there (yes, I added it, but I probably didn't understand the execution flow properly back then). The instruction completes in hvm_io_assist(), there's no register state writeback = elsewhere that I could see, and hence the updating of EFLAGS.RF also belongs just there. > * Why is DR6 adjustment done in svm_hvm_inject_exception() rather than >svm_inject_exception() (to match vmx_inject_exception())? Because a) svm_hvm_inject_exception() is the counterpart of vmx_inject_exception(),= svm_inject_exception() is an internal helper. > * Why do some instruction emulations pass NULL to update_guest_eip() and >hence bypass #DB injection? That seems bogus. That's intentional (to a certain degree): Especially for the HLT case I = wasn't really certain injecting an exception here would have the intended effect. I'm pretty sure you'd have to suppress the normal HLT handling in that = case, and it seemed safer for a first cut to not inject an exception at all here = (thus simply retaining current behavior for this special case). For SVM's VMEXIT_EXCEPTION_BP case it seems certainly wrong to inject one. >Io-brkp: This one needs more explanation about exactly what things are = being >improved. There's more going on here than your brief explanation below. = My Oh, I see that I didn't describe the original purpose of emulating the #DB on I/O port accesses - I must have thought that this is implied by the = name. This is being done by not committing the respective enable bits to %dr7, but rather tracking them in the DR5 shadow. Beyond that, the only things I didn't describe are mere cleanup items, = like replacing numeric literals with manifest constants. >only comment at this point is that I don't see that supporting = CR4.DE=3D=3D0 is You probably mean CR4.DE=3D=3D1? If you doubt that, why did you apply the recent change to allow the guest to drive the bit? >very useful. I don't subscribe to the view that we should support every >little detail of x86 architecture just because it's there, especially for = PV >guests. I think that a kernel debugger in a guest should certainly have the = capability of doing this, but I understand your reservation to a certain degree. >All other patches are applied and I queued up a couple for 3.1.3 also. Thanks. Btw., the 'official' tree appears to be stuck again... Jan