On 18/08/2023 4:44 pm, Jinoh Kang wrote: > Xen has a bug where hardware breakpoint exceptions (DR_TRAP<n>) are > erroneously recognized as single-stepping exceptions (DR_STEP).
I expected this to come back and bite. https://lore.kernel.org/xen-devel/1528120755-17455-1-git-send-email-andrew.coop...@citrix.com/ Xen's %dr6 handling is very broken, and then Spectre/Meltdown happened and some how my bugfixes are now 5 years old and still incomplete. I've got a form of this series rebased onto staging, which I'll need to dust off. That said, I was not aware of this specific case going wrong. (But I can't say I'm surprised.) Thankyou for the test case. If I'm reading it right, the problem is that when %dr0 genuinely triggers, we VMExit (to break #DB infinite loops), and on re-injecting the #DB back to the guest, we blindly set singlestep? This is wrong for #DB faults, and you're using an instruction breakpoint so that matches. However, it is far more complicated for #DB traps, where hardware leaves it up to the VMM to merge status bits, and it's different between PV, VT-x and SVM. Worse than that, there is an bug on Intel hardware where if a singlestep is genuinely pending, then data breakpoint information isn't passed to the VMM on VMExit. I have yet to persuade Intel to fix this, despite quite a lot of trying. Looking at patch 3, I think I can see how it fixes your bug, but I don't think the logic is correct in all cases. In particular, look at my series for the cases where X86_DR6_DEFAULT is used to flip polarity. Notably, PV and SVM have different dr6 polarity to VT-x's pending_dbg field. Also, on Intel you're supposed to leave pending bits in pending_dbg and not inject #DB directly, in order for the pipeline to get the exception priority in the right order. This I didn't get around to fixing at the time. I suspect what we'll need to do is combine parts of your series and parts of mine. I never got around to fixing the introspection bugs in mine (that's a far larger task to do nicely), and yours is more targetted. ~Andrew
