On Fri, May 14, 2010 at 12:25:31PM +0530, K.Prasad wrote:

Okay. I will re-use single_step_exception() after modifications; it
appearsto have no in-kernel users for it.

It's called from exceptions-64s.S, head_32.S and head_8xx.S in
arch/powerpc/kernel.

quoted

Suppose the address at which the data breakpoint has been unmapped,
and the process has a handler for the SIGSEGV signal.  When we try to
single-step the load or store, we will get a DSI (0x300) interrupt,
call into do_page_fault, and end up sending the process a SIGSEGV.
That will invoke the signal handler, which can then do anything it
likes.  It can do a blocking system call, it can longjmp() back into
its main event, or it can return from the signal handler.  Only in the
last case will it retry the load or store, and then only if the signal
handler hasn't modified the NIP value in the signal frame.  That's
what I mean by "doesn't return to the instruction".

At the outset, this seemed to be a scary thing to happen; but turns out
to be harmful only to the extent of generating a false hw-breakpoint
exception in certain cases. A case-by-case basis analysis reveals thus:

Consider an instruction stream i1, i2, i3, ... iN, where i1 has
finished execution and i2 is about to be executed but has generated a
DSI interrupt with the above-mentioned conditions i.e. DSISR indicates a
DABR match + Page-Table entry not found. Now according to do_hash_page
in exception-64s.S (as pasted below), do_page_fault() and do_dabr() are
invoked one after the other.

_STATIC(do_hash_page)
	std	r3,_DAR(r1)
	std	r4,_DSISR(r1)

	andis.	r0,r4,0xa410		/* weird error? */
	bne-	handle_page_fault	/* if not, try to insert a HPTE */
	andis.  r0,r4,DSISR_DABRMATCH@h
	bne-    handle_dabr_fault

Note that bne is not a procedure call; we'll actually get two DSIs in
this scenario.  But I don't think that matters.  Also note that the
branch to handle_page_fault here is not for the HPTE-not-found case;
it's for the unusual errors.  So we'll handle the HPTE insertion after
handling the DABR match.

Thus, when control returns to user-space to instruction 'i2', the
hw_breakpoint_handler() has completed execution, and a SIGSEGV is pending
to be delivered and single-stepping enabled MSR_SE is set. Upon return to
user-space, the handler for SIGSEGV is executed and it may perform one of
the following (as you stated previously):
(a) Make a blocking syscall, eventually yielding the CPU to a new thread
(b) Jump to a different instruction in user-space, say iN, and not complete
the execution of instruction i2 at all.
(c) Return to instruction i2 and complete the execution.

In case of (a), the context-switches should not affect the ability to
single-step the instruction when the thread is eventually scheduled to
run. The thread, when scheduled onto the CPU will complete signal
handling, return to execute instruction i2, cause single-step exception,
restore breakpoints and run smoothly thereafter.

Right.  However, the thread is running the signal handler without the
DABR being set, which is unfortunate.

In case of (b), the new instruction iN is single-stepped, the breakpoint
values are restored and the hw-breakpoint exception callback is invoked
after iN is executed. The user of this breakpoint i.e. the caller of
register_user_hw_breakpoint() who had placed a breakpoint on addressed
accessed by instruction i2 will be confused to find that an unrelated
instruction (which may not be a load/store) has caused the breakpoint.

That's the case if the signal handler modifies the NIP value in the
register set saved on the stack and returns.  If the signal handler
instead simply jumps to instruction iN (e.g. with longjmp or
siglongjmp), we'll never get the single-step callback.

If so desired, we may adopt the 'trigger-before-execute' semantics for
user-space breakpoints wherein the hw-breakpoint callback (through
perf_bp_event()) is invoked in hw_breakpoint_handler() itself. This
would indicate to the user that the impending instruction causes a DABR
'hit' but it may or may not be executed due to the role of
signal-handler or due to self-modifying code (as mentioned below).

Kindly let me know what you think about it.

(c) is the normal execution path we desire. The instruction i2 will be
safely single-stepped and breakpoints are restored.

quoted

The instruction could be changed underneath us if the program is
multi-threaded and another thread writes another instruction to the
instruction word where the load or store is.  Or it could use mmap()
to map some other page at the address of the load or store.  Either
way we could end up with a different instruction there.

If the instruction that originally caused the DABR exception is changed,
the new instruction in its place would still single-step to restore
breakpoint values. However the user of breakpoint interface will be
confused to find that the callback is invoked for an irrelevant
instruction.

It could be circumvented, to an extent, through the use of
trigger-before-execute semantics (as described before).

I don't think we want to do trigger-before-execute.  Ideally what we
want to ensure at all times is that either DABR is set (enabled) or
MSR.SE is set, but not both.  To ensure that we'd have to modify the
signal delivery code and possibly other places.

quoted

If we do get a context switch, e.g. as a result of a page fault, and
then switch back to the task, it looks to me like we will end up with
MSR_SE and DABR both set.  I don't suppose that will actually cause
any real problem besides double-counting the hit.

Page fault exception will be handled before hw_breakpoint_handler(),
hence MSR_SE would not have been set if a context-switch happened in
pange-fault handling itself. I don't see a case where both MSR_SE and
DABR will be set together.

Imagine this scenario: we get the DABR match, set MSR_SE and return to
the task.  In the meantime another higher-priority task has become
runnable and our need_resched flag is set, so we call schedule() on
the way back out to usermode.  The other task runs and then blocks and
our task gets scheduled again.  As part of the context switch,
arch_install_hw_breakpoint() will get called and will set DABR.  So
we'll return to usermode with both DABR and MSE_SE set.

Thanks for the comments. Let me know if the analysis above is incorrect
or if I've failed to recognise any important issue that you pointed out.
I will send out a patch with changes to emulate_single_step() in the
next version of the patchset, if I don't hear any further comments.

We haven't discussed at all the case where the breakpoint is a per-cpu
breakpoint or where it's a per-task breakpoint but the DABR match
occurs within the kernel -- which can happen, even for a user address,
in __get_user, __put_user, __copy_tofrom_user, etc.  If the access
there is to a bad address, we'll invoke the exception case in
bad_page_fault(), which looks to be another place where we need to
recognize that single-stepping won't succeed and reinstall the DABR
setting.  Do we count that as an event or not? - I'm not sure.

Paul.