[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 3/8] x86/emul: Add pending_dbg field to x86_event
On 24.08.2023 17:26, Jinoh Kang wrote: > From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > > All #DB exceptions result in an update of %dr6, but this isn't captured in > Xen's handling. > > PV guests generally work by modifying %dr6 before raising #DB, whereas HVM > guests do nothing and have a single-step special case in the lowest levels of > {vmx,svm}_inject_event(). All of this is buggy, but in particular, task > switches with the trace flag never end up signalling BT in %dr6. > > To begin resolving this issue, add a new pending_dbg field to x86_event > (unioned with cr2 to avoid taking any extra space), and introduce > {pv,hvm}_inject_debug_exn() helpers to replace the current callers using > {pv,hvm}_inject_hw_exception(). > > A key property is that pending_dbg is taken with positive polarity to deal > with RTM sensibly. Most callers pass in a constant, but callers passing in a > hardware %dr6 value need to xor the value with X86_DR6_DEFAULT to flip the > polarity of RTM and reserved fields. > > For PV guests, move the ad-hoc updating of %dr6 into pv_inject_event(). This > in principle breaks the handing of RTM in do_debug(), but PV guests can't > actually enable MSR_DEBUGCTL.RTM yet, so this doesn't matter in practice. > > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> > [ jinoh: Rebase onto staging, forward declare struct vcpu ] > Signed-off-by: Jinoh Kang <jinoh.kang.kr@xxxxxxxxx> > --- > CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > CC: Jan Beulich <JBeulich@xxxxxxxx> > CC: Wei Liu <wl@xxxxxxx> > CC: Roger Pau Monné <roger.pau@xxxxxxxxxx> > CC: Jun Nakajima <jun.nakajima@xxxxxxxxx> > CC: Kevin Tian <kevin.tian@xxxxxxxxx> > CC: Tim Deegan <tim@xxxxxxx> > CC: Tamas K Lengyel <tamas@xxxxxxxxxxxxx> > CC: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx> > CC: Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx> > > v1 -> v2: [S-o-b fixes. More details below.] > > - Update DR6 for gdbsx when trapped in PV guest kernel mode I take it that this refers to ... > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -1989,17 +1989,17 @@ void do_debug(struct cpu_user_regs *regs) > return; > } > > - /* Save debug status register where guest OS can peek at it */ > - v->arch.dr6 |= (dr6 & ~X86_DR6_DEFAULT); > - v->arch.dr6 &= (dr6 | ~X86_DR6_DEFAULT); > - > if ( guest_kernel_mode(v, regs) && v->domain->debugger_attached ) > { > + /* Save debug status register where gdbsx can peek at it */ > + v->arch.dr6 |= (dr6 & ~X86_DR6_DEFAULT); > + v->arch.dr6 &= (dr6 | ~X86_DR6_DEFAULT); > + > domain_pause_for_debugger(); > return; > } ... this code movement. I'm afraid this should have resulted in you dropping the earlier R-b, and I'm further afraid I'm not convinced this is correct, despite seeing why you would want to do this. The issue is that this way you also alter guest-visible state, when the intention is that such now happen via ... > - pv_inject_hw_exception(X86_EXC_DB, X86_EVENT_NO_EC); > + pv_inject_debug_exn(dr6 ^ X86_DR6_DEFAULT); > } ... this call alone. I fear I can't currently see how to get both aspects right, other than by breaking up Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |