On 09/04/2009 03:51 PM, Jan Kiszka wrote:
Based on Avi's suggestion: Do not save the host debug registers on guest
entry as they are already present in the thread state. Moreover, only
restore them if the current host thread is being debugged. But as KGDB
accesses the debug register directly, we have to fall back to existing
pattern in that case.
Signed-off-by: Jan Kiszka<jan.kiszka@xxxxxxxxxxx>
---
arch/x86/kvm/x86.c | 48 +++++++++++++++++++++++++++++++++---------------
1 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 891234b..036a2c5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -37,6 +37,7 @@
#include<linux/iommu.h>
#include<linux/intel-iommu.h>
#include<linux/cpufreq.h>
+#include<linux/kgdb.h>
#include<trace/events/kvm.h>
#undef TRACE_INCLUDE_FILE
#define CREATE_TRACE_POINTS
@@ -3627,14 +3628,21 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
kvm_guest_enter();
- get_debugreg(vcpu->arch.host_dr6, 6);
- get_debugreg(vcpu->arch.host_dr7, 7);
+ /*
+ * kgdb accesses the debug registers directly, so we have to save them
+ * and restore those values on return from the guest.
+ */
+ if (unlikely(kgdb_in_use())) {
+ if (unlikely(vcpu->arch.switch_db_regs)) {
+ get_debugreg(vcpu->arch.host_db[0], 0);
+ get_debugreg(vcpu->arch.host_db[1], 1);
+ get_debugreg(vcpu->arch.host_db[2], 2);
+ get_debugreg(vcpu->arch.host_db[3], 3);
+ }
+ get_debugreg(vcpu->arch.host_dr6, 6);
+ get_debugreg(vcpu->arch.host_dr7, 7);
+ }
if (unlikely(vcpu->arch.switch_db_regs)) {
- get_debugreg(vcpu->arch.host_db[0], 0);
- get_debugreg(vcpu->arch.host_db[1], 1);
- get_debugreg(vcpu->arch.host_db[2], 2);
- get_debugreg(vcpu->arch.host_db[3], 3);
-
set_debugreg(0, 7);
set_debugreg(vcpu->arch.eff_db[0], 0);
set_debugreg(vcpu->arch.eff_db[1], 1);
@@ -3645,15 +3653,25 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
trace_kvm_entry(vcpu->vcpu_id);
kvm_x86_ops->run(vcpu);
- if (unlikely(vcpu->arch.switch_db_regs)) {
- set_debugreg(0, 7);
- set_debugreg(vcpu->arch.host_db[0], 0);
- set_debugreg(vcpu->arch.host_db[1], 1);
- set_debugreg(vcpu->arch.host_db[2], 2);
- set_debugreg(vcpu->arch.host_db[3], 3);
+ if (unlikely(kgdb_in_use())) {
+ if (unlikely(vcpu->arch.switch_db_regs)) {
+ set_debugreg(vcpu->arch.host_db[0], 0);
+ set_debugreg(vcpu->arch.host_db[1], 1);
+ set_debugreg(vcpu->arch.host_db[2], 2);
+ set_debugreg(vcpu->arch.host_db[3], 3);
+ }
+ set_debugreg(vcpu->arch.host_dr6, 6);
+ set_debugreg(vcpu->arch.host_dr7, 7);
+ } else if (unlikely(test_thread_flag(TIF_DEBUG))) {
+ if (unlikely(vcpu->arch.switch_db_regs)) {
+ set_debugreg(current->thread.debugreg0, 0);
+ set_debugreg(current->thread.debugreg1, 1);
+ set_debugreg(current->thread.debugreg2, 2);
+ set_debugreg(current->thread.debugreg3, 3);
+ }
+ set_debugreg(current->thread.debugreg6, 6);
+ set_debugreg(current->thread.debugreg7, 7);
}
- set_debugreg(vcpu->arch.host_dr6, 6);
- set_debugreg(vcpu->arch.host_dr7, 7);
Please, let's have just save/nosave, not different kinds of save/restore.
But really, this looks very hacky. It's better to have kgdb integrate
more closely with the kernel debug register support instead of kvm
juggling between the two.
Something like
struct debug_registers thread_info::debugregs
extern struct debug_registers global_debug_registers;
and a function that loads a mix of the debug registers from the
thread-local and global settings. The context switch path can call that
function as well as kvm.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
