Re: [RFC Patch 2/2][Bugfix][x86][hw-breakpoint] Fix return-code tonotifier chain in hw_breakpoint_handler

[K.Prasad]

On Thu, Dec 31, 2009 at 01:38:09AM +0100, Frederic Weisbecker wrote:
> On Sat, Dec 26, 2009 at 11:58:33PM +0530, K.Prasad wrote:
> > The hw-breakpoint handler will return NOTIFY_DONE for user-space breakpoints
> > to generate SIGTRAP signal (and not for kernel-space addresses). 
> > 
> > Signed-off-by: K.Prasad <prasad@xxxxxxxxxxxxxxxxxx>
> > ---
> >  arch/x86/kernel/hw_breakpoint.c |    9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > Index: linux-2.6-tip/arch/x86/kernel/hw_breakpoint.c
> > ===================================================================
> > --- linux-2.6-tip.orig/arch/x86/kernel/hw_breakpoint.c
> > +++ linux-2.6-tip/arch/x86/kernel/hw_breakpoint.c
> > @@ -502,8 +502,6 @@ static int __kprobes hw_breakpoint_handl
> >  		rcu_read_lock();
> >  
> >  		bp = per_cpu(bp_per_reg[i], cpu);
> > -		if (bp)
> > -			rc = NOTIFY_DONE;
> >  		/*
> >  		 * Reset the 'i'th TRAP bit in dr6 to denote completion of
> >  		 * exception handling
> > @@ -517,6 +515,13 @@ static int __kprobes hw_breakpoint_handl
> >  			rcu_read_unlock();
> >  			break;
> >  		}
> > +		/*
> > +		 * Further processing in do_debug() is needed for a) user-space
> > +		 * breakpoints (to generate signals) and b) when the system has
> > +		 * taken exception due to multiple causes
> > +		 */
> > +		if (bp->attr.bp_addr < TASK_SIZE)
> > +			rc = NOTIFY_DONE;
> >  
> >  		perf_bp_event(bp, args->regs);
> >  
> > 
> 
> 
> Oh and now that I see this patch, the previous one indeed makes sense
> with this check:
> 
> if (dr6 & (~DR_TRAP_BITS))
> 	rc = NOTIFY_DONE;
> 
> That said, it means thread.debugreg6 won't get the reserved bits anymore.
> I see some use of them from kvm (it restores the reserved bits on guest<->host
> switch). Not sure if this inconsistency could affect kvm...
> 

Can you point me to the relevant code?
Anyway will copy this to Jan Kiszka <jan.kiszka@xxxxxx> to hear what
this change means to KVM...on a similar note, will be happy to be
re-assured by Roland/Oleg about the patch's harmlessness to the
user-space (ptrace/utrace).

Hi Jan,
	Patch 20091226182725.GB9494@xxxxxxxxxx introduces a change that

Patch 1/2: Clears the arch-reserved bits from debug status register.
This helps easy bitwise operations - such as the check for non-trap bits in
hw_breakpoint_handler. A check for the same using
"if (dr6 & (~DR_TRAP_BITS))" throws incorrect results due to the
presence of preset reserved bits.

Let us know if you foresee any harm from the said change to the
behaviour seen under KVM.

Thanks,
K.Prasad

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/