Hi, On Tue, Oct 16, 2018 at 02:18:25PM -0700, Jim Mattson wrote: > On Tue, Oct 16, 2018 at 1:37 PM, Christian Ehrhardt <lk@xxxxxxx> wrote: > > > - set_debugreg(0, 6); > > + if (vcpu->arch.dr6 & ~DR6_RESERVED) > > + set_debugreg(0, 6); > > This will skip setting %dr6 when DR6.RTM is clear. That seems > incorrect, since DR6.RTM is active-low. Should this be: > > if (vcpu->arch.dr6 != DR6_RESERVED) > set_debugreg(0, 6); Oh, bummer. I didn't realize that there is a non-reserved bit that defaults to 1 and not to zero, now. Thanks for pointing this out. Currently, DR6_RESERVED includes %dr6.RTM, i.e. the check above ignores the current value of %dr6.RTM. Thus technically the code is ok both on machines that support RTM and on machines that don't. However, DR6_RESERVED should not contain bit 16 anymore. >From this point there are two options: - Ignore the value of the RTM bit along with the reserved bits and continue to write zero to %dr6. This is equivalent to the current code but I'd make the fact that RTM is included in the mask explicit. - Do not ignore the value of dr6.RTM but expect it to be set by default. In this case the reset value changes from zero to DB_RTM (0x10000). What do you think? regards Christian
