[PATCH] hardware debug register handling

[PATCH] hardware debug register handling

[Nguyen Anh Quynh]

[-- Attachment #1: Type: text/plain, Size: 248 bytes --]

Currently, when handling hardware breakpoints, we always set values
for all the 4 hardware debug registers, regardless it is needed or
not. This patch fixes the bug.

Signed-off-by: Nguyen Anh Quynh <aquynh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

[-- Attachment #2: patch15.patch --]
[-- Type: text/x-patch, Size: 2000 bytes --]

diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
index 7bdffe6..c6187c8 100644
--- a/drivers/kvm/kvm.h
+++ b/drivers/kvm/kvm.h
@@ -180,7 +180,7 @@ struct kvm_mmu_memory_cache {
  */
 struct kvm_guest_debug {
 	int enabled;
-	unsigned long bp[4];
+	struct kvm_breakpoint bp[4];
 	int singlestep;
 };
 
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 2c4f01b..88ca030 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -706,20 +706,23 @@ static int set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_debug_guest *dbg)
 {
 	unsigned long dr7 = 0x400;
 	int old_singlestep;
+	int i;
 
 	old_singlestep = vcpu->guest_debug.singlestep;
-
 	vcpu->guest_debug.enabled = dbg->enabled;
-	if (vcpu->guest_debug.enabled) {
-		int i;
 
+	for (i = 0; i < 4; ++i)
+		vcpu->guest_debug.bp[i].enabled = 0;
+
+	if (vcpu->guest_debug.enabled) {
 		dr7 |= 0x200;  /* exact */
 		for (i = 0; i < 4; ++i) {
-			if (!dbg->breakpoints[i].enabled)
-				continue;
-			vcpu->guest_debug.bp[i] = dbg->breakpoints[i].address;
-			dr7 |= 2 << (i*2);    /* global enable */
-			dr7 |= 0 << (i*4+16); /* execution breakpoint */
+			if (dbg->breakpoints[i].enabled) {
+				vcpu->guest_debug.bp[i].enabled = 1;
+				vcpu->guest_debug.bp[i].address = dbg->breakpoints[i].address;
+				dr7 |= 2 << (i*2);    /* global enable */
+				dr7 |= 0 << (i*4+16); /* execution breakpoint */
+			}
 		}
 
 		vcpu->guest_debug.singlestep = dbg->singlestep;
@@ -1519,12 +1522,18 @@ static void do_interrupt_requests(struct kvm_vcpu *vcpu,
 
 static void kvm_guest_debug_pre(struct kvm_vcpu *vcpu)
 {
+#define SET_DEBUGREG(i) \
+	if (dbg->bp[i].enabled) { \
+		set_debugreg(dbg->bp[i].address, i); \
+	}
+
 	struct kvm_guest_debug *dbg = &vcpu->guest_debug;
 
-	set_debugreg(dbg->bp[0], 0);
-	set_debugreg(dbg->bp[1], 1);
-	set_debugreg(dbg->bp[2], 2);
-	set_debugreg(dbg->bp[3], 3);
+	SET_DEBUGREG(0);
+	SET_DEBUGREG(1);
+	SET_DEBUGREG(2);
+	SET_DEBUGREG(3);
+#undef SET_DEBUGREG
 
 	if (dbg->singlestep) {
 		unsigned long flags;

[-- Attachment #3: Type: text/plain, Size: 286 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[-- Attachment #4: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH] hardware debug register handling

[Avi Kivity]

Nguyen Anh Quynh wrote:
> Currently, when handling hardware breakpoints, we always set values
> for all the 4 hardware debug registers, regardless it is needed or
> not. This patch fixes the bug.

Why is this a problem?  Only enabled breakpoints are programmed into dr7.

> @@ -1519,12 +1522,18 @@ static void do_interrupt_requests(struct kvm_vcpu *vcpu,
>  
>  static void kvm_guest_debug_pre(struct kvm_vcpu *vcpu)
>  {
> +#define SET_DEBUGREG(i) \
> +	if (dbg->bp[i].enabled) { \
> +		set_debugreg(dbg->bp[i].address, i); \
> +	}
> +
>  	struct kvm_guest_debug *dbg = &vcpu->guest_debug;
>  
> -	set_debugreg(dbg->bp[0], 0);
> -	set_debugreg(dbg->bp[1], 1);
> -	set_debugreg(dbg->bp[2], 2);
> -	set_debugreg(dbg->bp[3], 3);
> +	SET_DEBUGREG(0);
> +	SET_DEBUGREG(1);
> +	SET_DEBUGREG(2);
> +	SET_DEBUGREG(3);
> +#undef SET_DEBUGREG
>   

Why those ugly macros?

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH] hardware debug register handling

[Nguyen Anh Quynh]

On 7/19/07, Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote:
> Nguyen Anh Quynh wrote:
> > Currently, when handling hardware breakpoints, we always set values
> > for all the 4 hardware debug registers, regardless it is needed or
> > not. This patch fixes the bug.
>
> Why is this a problem?  Only enabled breakpoints are programmed into dr7.
>

Ah, I see your point.

> > @@ -1519,12 +1522,18 @@ static void do_interrupt_requests(struct kvm_vcpu *vcpu,
> >
> >  static void kvm_guest_debug_pre(struct kvm_vcpu *vcpu)
> >  {
> > +#define SET_DEBUGREG(i) \
> > +     if (dbg->bp[i].enabled) { \
> > +             set_debugreg(dbg->bp[i].address, i); \
> > +     }
> > +
> >       struct kvm_guest_debug *dbg = &vcpu->guest_debug;
> >
> > -     set_debugreg(dbg->bp[0], 0);
> > -     set_debugreg(dbg->bp[1], 1);
> > -     set_debugreg(dbg->bp[2], 2);
> > -     set_debugreg(dbg->bp[3], 3);
> > +     SET_DEBUGREG(0);
> > +     SET_DEBUGREG(1);
> > +     SET_DEBUGREG(2);
> > +     SET_DEBUGREG(3);
> > +#undef SET_DEBUGREG
> >
>
> Why those ugly macros?
>

It checks to see if it is necessary to set debug register.

But as it actually depends on dr7, so just ignore this patch.

Thanks,
Q

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/