On 14/09/2023 4:04 pm, Jan Beulich wrote: > On 13.09.2023 01:21, Andrew Cooper wrote: >> --- a/xen/arch/x86/x86_emulate/x86_emulate.c >> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c >> @@ -8379,7 +8379,10 @@ x86_emulate( >> if ( !mode_64bit() ) >> _regs.r(ip) = (uint32_t)_regs.r(ip); >> >> - /* Should a singlestep #DB be raised? */ >> + if ( singlestep ) >> + ctxt->retire.pending_dbg |= X86_DR6_BS; > We set "singlestep" about first thing in the function. Is it really correct > to latch that into pending_dbg without regard to rc? (Perhaps yes, seeing > the comment next to the field declaration.)
Honestly, without seeing some real RTL, I'm not sure. The thing that is definitely buggy in this retire logic is saying "don't set singlestep if MovSS is pending". The correct behaviour is to set both PENDING_DBG.BS and INTERRUPTIBILITY.MOVSS, the latter of which causes #DB to be skipped on this instruction boundary. That's why we get the XSA-260 behaviour - when the SYSCALL/INT/etc completes and we hit the subsequent instruction boundary, PENDING_DBG still has values latched in it. With the DONE path being removed, and with the plan to wire in the VMCS register for emulations that occur when something is already pending, then it probably does want to be restricted back to the OKAY case. e.g. We shouldn't latch BS in EXCEPTION Case, or we'd create more XSA-260-like behaviour. I'll adjust in light of this. ~Andrew
