On 4/24/2020 3:29 AM, Sean Christopherson wrote:
On Thu, Apr 16, 2020 at 06:15:08PM +0800, Xiaoyao Li wrote:
Once any #BP enabled in DR7, it will set KVM_DEBUGREG_BP_ENABLED, which
leads to reload DRn before every VM entry even if none of DRn changed.
Drop KVM_DEBUGREG_BP_ENABLED flag and set KVM_DEBUGREG_NEED_RELOAD flag
for the cases that DRn need to be reloaded instead, to avoid unnecessary
DRn reload.
Loading DRs on every VM-Enter _is_ necessary if there are breakpoints
enabled for the guest. The hardware DR values are not "stable", e.g. they
are loaded with the host's values immediately after saving the guest's
value (if DR_EXITING is disabled) in vcpu_enter_guest(), notably iff the
host has an active/enabled breakpoint.
May bad, bbviously I didn't think about it.
My bet is that DRs can be changed
from interrupt context as well.
So set KVM_DEBUGREG_NEED_RELOAD in vcpu_load won't help.
Loading DRs for the guest (not necessarily the same as the guest's DRs) is
necessary if a breakpoint is enabled so that the #DB is actually hit in
guest. It's a similar concept to instructions that consume MSR values,
e.g. SYSCALL, RDTSCP, etc..., even if KVM intercepts the MSR/DR, hardware
still needs the correct value so that the guest behavior is correct.
Signed-off-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxx>
---
arch/x86/include/asm/kvm_host.h | 3 +--
arch/x86/kvm/x86.c | 4 ++--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index f465c76e6e5a..87e2d020351e 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -509,9 +509,8 @@ struct kvm_pmu {
struct kvm_pmu_ops;
enum {
- KVM_DEBUGREG_BP_ENABLED = 1,
+ KVM_DEBUGREG_NEED_RELOAD = 1,
KVM_DEBUGREG_WONT_EXIT = 2,
- KVM_DEBUGREG_NEED_RELOAD = 4,
};
struct kvm_mtrr_range {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cce926658d10..71264df64001 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1086,9 +1086,8 @@ static void kvm_update_dr7(struct kvm_vcpu *vcpu)
else
dr7 = vcpu->arch.dr7;
kvm_x86_ops.set_dr7(vcpu, dr7);
- vcpu->arch.switch_db_regs &= ~KVM_DEBUGREG_BP_ENABLED;
if (dr7 & DR7_BP_EN_MASK)
- vcpu->arch.switch_db_regs |= KVM_DEBUGREG_BP_ENABLED;
+ vcpu->arch.switch_db_regs |= KVM_DEBUGREG_NEED_RELOAD;
}
static u64 kvm_dr6_fixed(struct kvm_vcpu *vcpu)
@@ -1128,6 +1127,7 @@ static int __kvm_set_dr(struct kvm_vcpu *vcpu, int dr, unsigned long val)
break;
}
+ vcpu->arch.switch_db_regs |= KVM_DEBUGREG_NEED_RELOAD;
return 0;
}
--
2.20.1
