All of lore.kernel.org
 help / color / mirror / Atom feed
From: oleg@redhat.com (Oleg Nesterov)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 8/8] ARM64: Add uprobe support
Date: Sun, 4 Jan 2015 19:40:12 +0100	[thread overview]
Message-ID: <20150104184012.GA12614@redhat.com> (raw)
In-Reply-To: <54A944ED.5010408@redhat.com>

On 01/04, Pratyush Anand wrote:
>
> On Friday 02 January 2015 10:53 PM, Oleg Nesterov wrote:
>> But the main question is: why do we need add/find_ss_context ?? Please
>> explain.
>>
>
> See arch/arm64/kernel/debug-monitors.c: call_step_hook
>
> Unlike breakpoint exception, there is no ESR info check for step
> exception. So, it is the responsibility of step handler
> (uprobe_single_step_handler) to make sure that exception was generated
> for it.

Yes, yes, this is clear. My point was, we can (I think) rely on
uprobe_post_sstep_notifier() which checks ->active_uprobe != NULL.

And I guess you understood what I meant, but since I wasn't clear let
me repeat to ensure we really understand each other.

Can't

	uprobe_single_step_handler(regs, esr)
	{
		if (user_mode(regs) && uprobe_post_sstep_notifier(regs))
			return HANDLED;
		return ERROR;
	}

work without this step_ctx logic?

If everything is correct, the probed task can execute a single (xol) insn
in user-mode before the trap. If ->active_uprobe is set we know that we
expect the ss trap in user-mode, and nothing else except this xol insn can
generate it?

Perhaps arm64 needs additional checks, I dunno... If you think that the
->active_uprobe check is not enough you can probably also verify that
"utask->state = UTASK_SSTEP" and/or "regs->pc - 4 == utask->xol_vaddr",
but so far it seems to me that these additional checks can only make sense
under WARN_ON().

Oleg.

WARNING: multiple messages have this Message-ID (diff)
From: Oleg Nesterov <oleg@redhat.com>
To: Pratyush Anand <panand@redhat.com>
Cc: linux-arm-kernel@lists.infradead.org, linux@arm.linux.org.uk,
	tixy@linaro.org, ananth@in.ibm.com, sandeepa.prabhu@linaro.org,
	catalin.marinas@arm.com, will.deacon@arm.com,
	linux-kernel@vger.kernel.org, anil.s.keshavamurthy@intel.com,
	masami.hiramatsu.pt@hitachi.com, wcohen@redhat.com
Subject: Re: [RFC 8/8] ARM64: Add uprobe support
Date: Sun, 4 Jan 2015 19:40:12 +0100	[thread overview]
Message-ID: <20150104184012.GA12614@redhat.com> (raw)
In-Reply-To: <54A944ED.5010408@redhat.com>

On 01/04, Pratyush Anand wrote:
>
> On Friday 02 January 2015 10:53 PM, Oleg Nesterov wrote:
>> But the main question is: why do we need add/find_ss_context ?? Please
>> explain.
>>
>
> See arch/arm64/kernel/debug-monitors.c: call_step_hook
>
> Unlike breakpoint exception, there is no ESR info check for step
> exception. So, it is the responsibility of step handler
> (uprobe_single_step_handler) to make sure that exception was generated
> for it.

Yes, yes, this is clear. My point was, we can (I think) rely on
uprobe_post_sstep_notifier() which checks ->active_uprobe != NULL.

And I guess you understood what I meant, but since I wasn't clear let
me repeat to ensure we really understand each other.

Can't

	uprobe_single_step_handler(regs, esr)
	{
		if (user_mode(regs) && uprobe_post_sstep_notifier(regs))
			return HANDLED;
		return ERROR;
	}

work without this step_ctx logic?

If everything is correct, the probed task can execute a single (xol) insn
in user-mode before the trap. If ->active_uprobe is set we know that we
expect the ss trap in user-mode, and nothing else except this xol insn can
generate it?

Perhaps arm64 needs additional checks, I dunno... If you think that the
->active_uprobe check is not enough you can probably also verify that
"utask->state = UTASK_SSTEP" and/or "regs->pc - 4 == utask->xol_vaddr",
but so far it seems to me that these additional checks can only make sense
under WARN_ON().

Oleg.


  reply	other threads:[~2015-01-04 18:40 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-31 15:21 [RFC 0/8] ARM64: Uprobe support added Pratyush Anand
2014-12-31 15:21 ` Pratyush Anand
2014-12-31 15:21 ` [RFC 1/8] ARM64: Move BRK opcodes defines from kprobes.h to insn.h Pratyush Anand
2015-01-08 16:55   ` Will Deacon
2015-01-08 16:55     ` Will Deacon
2015-01-08 17:31     ` Pratyush Anand
2015-01-08 17:31       ` Pratyush Anand
2014-12-31 15:21 ` [RFC 2/8] ARM64: Refactor kprobes-arm64 Pratyush Anand
2015-01-08 16:55   ` Will Deacon
2015-01-08 16:55     ` Will Deacon
2015-01-08 17:33     ` Pratyush Anand
2015-01-08 17:33       ` Pratyush Anand
2015-01-08 17:36       ` Will Deacon
2015-01-08 17:36         ` Will Deacon
2015-01-08 17:39         ` Pratyush Anand
2015-01-08 17:39           ` Pratyush Anand
2014-12-31 15:21 ` [RFC 3/8] Kernel/uprobe: Define arch_uprobe_exception_notify as __weak Pratyush Anand
2015-01-02 17:43   ` Oleg Nesterov
2015-01-02 17:43     ` Oleg Nesterov
2015-01-04 13:50     ` Pratyush Anand
2015-01-04 13:50       ` Pratyush Anand
2014-12-31 15:21 ` [RFC 4/8] ARM64: Add instruction_pointer_set function Pratyush Anand
2015-01-08 16:59   ` Will Deacon
2015-01-08 16:59     ` Will Deacon
2015-01-09  5:18     ` Pratyush Anand
2015-01-09  5:18       ` Pratyush Anand
2014-12-31 15:21 ` [RFC 5/8] ARM64: Re-factor flush_ptrace_access Pratyush Anand
2015-01-02 17:51   ` Oleg Nesterov
2015-01-02 17:51     ` Oleg Nesterov
2015-01-02 18:19     ` Oleg Nesterov
2015-01-02 18:19       ` Oleg Nesterov
2015-01-04 13:50       ` Pratyush Anand
2015-01-04 13:50         ` Pratyush Anand
2014-12-31 15:21 ` [RFC 6/8] ARM64: Handle TRAP_HWBRKPT for user mode as well Pratyush Anand
2015-01-02 18:05   ` Oleg Nesterov
2015-01-02 18:05     ` Oleg Nesterov
2015-01-08 17:01     ` Will Deacon
2015-01-08 17:01       ` Will Deacon
2015-01-08 17:51       ` Pratyush Anand
2015-01-08 17:51         ` Pratyush Anand
2014-12-31 15:21 ` [RFC 7/8] ARM64: Handle TRAP_BRKPT " Pratyush Anand
2014-12-31 15:21 ` [RFC 8/8] ARM64: Add uprobe support Pratyush Anand
2015-01-02 17:23   ` Oleg Nesterov
2015-01-02 17:23     ` Oleg Nesterov
2015-01-04 13:49     ` Pratyush Anand
2015-01-04 13:49       ` Pratyush Anand
2015-01-04 18:40       ` Oleg Nesterov [this message]
2015-01-04 18:40         ` Oleg Nesterov
2015-01-05  4:17         ` Pratyush Anand
2015-01-05  4:17           ` Pratyush Anand
2015-01-08 17:03   ` Will Deacon
2015-01-08 17:03     ` Will Deacon
2015-01-08 17:54     ` Pratyush Anand
2015-01-08 17:54       ` Pratyush Anand
2015-01-09 17:45       ` Oleg Nesterov
2015-01-09 17:45         ` Oleg Nesterov
2015-01-12  4:50         ` Pratyush Anand
2015-01-12  4:50           ` Pratyush Anand
2015-01-09 17:59   ` Oleg Nesterov
2015-01-09 17:59     ` Oleg Nesterov
2015-01-12  5:04     ` Pratyush Anand
2015-01-12  5:04       ` Pratyush Anand
2015-01-12  6:45       ` Pratyush Anand
2015-01-12  6:45         ` Pratyush Anand
2015-01-12 14:38         ` Oleg Nesterov
2015-01-12 14:38           ` Oleg Nesterov
2015-01-12 14:28       ` Oleg Nesterov
2015-01-12 14:28         ` Oleg Nesterov
2015-01-01  1:59 ` [RFC 0/8] ARM64: Uprobe support added Pratyush Anand
2015-01-01  1:59   ` Pratyush Anand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150104184012.GA12614@redhat.com \
    --to=oleg@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.