Apart from the cleanup patches just sent I also have two bigger ones in the queue. I''m not certain they qualify for 3.2 at this point, though: While most of them can be considered enhancements, the io-brkp one (which depends on the inject-sstep one) also fixes the issue of guests on x86-64 not being able to utilize 8-byte breakpoints, and it reduces the likelihood of needing to restore the debug registers by only looking at the enable bits in the respective conditions (as HVM was already doing). Therefore I''d like to understand whether these patches can go in in their current shape, or whether I should split out at least the 8-byte bug fix and submit this separately. Thanks, Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Inject-sstep: Looks fine in principle. A few specific questions: * Why is all the RFLAGS.RF logic removed from platform.c? * Why is DR6 adjustment done in svm_hvm_inject_exception() rather than svm_inject_exception() (to match vmx_inject_exception())? * Why do some instruction emulations pass NULL to update_guest_eip() and hence bypass #DB injection? That seems bogus. 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 only comment at this point is that I don''t see that supporting CR4.DE==0 is 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. All other patches are applied and I queued up a couple for 3.1.3 also. -- Keir On 22/11/07 12:50, "Jan Beulich" <jbeulich@novell.com> wrote:> Apart from the cleanup patches just sent I also have two bigger ones in > the queue. I''m not certain they qualify for 3.2 at this point, though: While > most of them can be considered enhancements, the io-brkp one (which > depends on the inject-sstep one) also fixes the issue of guests on x86-64 > not being able to utilize 8-byte breakpoints, and it reduces the likelihood > of needing to restore the debug registers by only looking at the enable > bits in the respective conditions (as HVM was already doing). > > Therefore I''d like to understand whether these patches can go in in their > current shape, or whether I should split out at least the 8-byte bug fix > and submit this separately. > > Thanks, Jan > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 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. MyOh, 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==0 isYou probably mean CR4.DE==1? 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 22/11/07 15:47, "Jan Beulich" <jbeulich@novell.com> wrote:>> * 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.Ah, you could be right. I don''t have that much experience with EFLAGS.TF, but from the reference manuals it looks rather as if, on return from an exception or interrupt, when EFLAGS.TF is re-set by IRET, you actually do not #DB with EIP pointing at the instruction you IRET to? And this is because single-step #DB is a trap? Also I suppose that a software exception or interrupt causes single-step #DB to be skipped when you actually execute INT n, INT3, INTO or whatever. So you effectively never see single-step #DB with EIP pointing at the instruction following one of those INT instructions? I suppose I could validate this for myself. :-) Anyway, apart from that the patches are fine. Re-submit with Signed-off-by lines and I''ll drop them in. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel