LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Frederic Weisbecker <frederic@kernel.org>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Frederic Weisbecker <frederic@kernel.org>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Chris Zankel <chris@zankel.net>,
	Paul Mackerras <paulus@samba.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Will Deacon <will.deacon@arm.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Rich Felker <dalias@libc.org>, Ingo Molnar <mingo@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Andy Lutomirski <luto@kernel.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Max Filippov <jcmvbkbc@gmail.com>
Subject: [PATCH 5/9] xtensa: Split breakpoint validation into "check" and "commit"
Date: Sun,  6 May 2018 21:19:51 +0200	[thread overview]
Message-ID: <1525634395-23380-6-git-send-email-frederic@kernel.org> (raw)
In-Reply-To: <1525634395-23380-1-git-send-email-frederic@kernel.org>

The breakpoint code mixes up attribute check and commit into a single
code entity. Therefore the validation may return an error due to
incorrect atributes while still leaving halfway modified architecture
breakpoint struct.

Prepare fox fixing this misdesign and separate both logics.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: Chris Zankel <chris@zankel.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 arch/xtensa/kernel/hw_breakpoint.c | 60 ++++++++++++++++++++++++++------------
 1 file changed, 42 insertions(+), 18 deletions(-)

diff --git a/arch/xtensa/kernel/hw_breakpoint.c b/arch/xtensa/kernel/hw_breakpoint.c
index b35656a..aae055d 100644
--- a/arch/xtensa/kernel/hw_breakpoint.c
+++ b/arch/xtensa/kernel/hw_breakpoint.c
@@ -45,15 +45,38 @@ int arch_check_bp_in_kernelspace(struct perf_event *bp)
 	return (va >= TASK_SIZE) && ((va + len - 1) >= TASK_SIZE);
 }
 
-/*
- * Construct an arch_hw_breakpoint from a perf_event.
- */
-static int arch_build_bp_info(struct perf_event *bp)
+static int hw_breakpoint_arch_check(struct perf_event *bp,
+				    const struct perf_event_attr *attr)
+{
+	/* Type */
+	switch (attr->bp_type) {
+	case HW_BREAKPOINT_X:
+	case HW_BREAKPOINT_R:
+	case HW_BREAKPOINT_W:
+	case HW_BREAKPOINT_RW:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Len */
+	if (attr->bp_len < 1 || attr->bp_len > 64 || !is_power_of_2(attr->bp_len))
+		return -EINVAL;
+
+	/* Address */
+	if (attr->bp_addr & (attr->bp_len - 1))
+		return -EINVAL;
+
+	return 0;
+}
+
+static void hw_breakpoint_arch_commit(struct perf_event *bp)
 {
 	struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+	struct perf_event_attr *attr = &bp->attr;
 
 	/* Type */
-	switch (bp->attr.bp_type) {
+	switch (attr->bp_type) {
 	case HW_BREAKPOINT_X:
 		info->type = XTENSA_BREAKPOINT_EXECUTE;
 		break;
@@ -67,29 +90,30 @@ static int arch_build_bp_info(struct perf_event *bp)
 		info->type = XTENSA_BREAKPOINT_LOAD | XTENSA_BREAKPOINT_STORE;
 		break;
 	default:
-		return -EINVAL;
+		WARN_ON_ONCE(1);
 	}
 
 	/* Len */
-	info->len = bp->attr.bp_len;
-	if (info->len < 1 || info->len > 64 || !is_power_of_2(info->len))
-		return -EINVAL;
+	info->len = attr->bp_len;
 
 	/* Address */
-	info->address = bp->attr.bp_addr;
-	if (info->address & (info->len - 1))
-		return -EINVAL;
-
-	return 0;
+	info->address = attr->bp_addr;
 }
 
+/*
+ * Validate the arch-specific HW Breakpoint register settings
+ */
 int arch_validate_hwbkpt_settings(struct perf_event *bp)
 {
-	int ret;
+	int err;
 
-	/* Build the arch_hw_breakpoint. */
-	ret = arch_build_bp_info(bp);
-	return ret;
+	err = hw_breakpoint_arch_check(bp, &bp->attr);
+	if (err)
+		return err;
+
+	hw_breakpoint_arch_commit(bp);
+
+	return 0;
 }
 
 int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
-- 
2.7.4

  parent reply	other threads:[~2018-05-06 19:21 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-06 19:19 [PATCH 0/9] breakpoint: Rework arch validation Frederic Weisbecker
2018-05-06 19:19 ` [PATCH 1/9] x86/breakpoint: Split validation into "check" and "commit" Frederic Weisbecker
2018-05-06 19:19 ` [PATCH 2/9] sh: Remove "struct arch_hw_breakpoint::name" unused field Frederic Weisbecker
2018-05-06 19:19 ` [PATCH 3/9] sh: Split breakpoint validation into "check" and "commit" Frederic Weisbecker
2018-05-06 19:19 ` [PATCH 4/9] arm: " Frederic Weisbecker
2018-05-08 11:13   ` Mark Rutland
2018-05-08 11:14     ` Mark Rutland
2018-05-09 11:32     ` Mark Rutland
2018-05-09 19:51       ` Andy Lutomirski
2018-05-11  2:37         ` Frederic Weisbecker
2018-05-15 13:35       ` Frederic Weisbecker
2018-05-06 19:19 ` Frederic Weisbecker [this message]
2018-05-06 19:19 ` [PATCH 6/9] arm64: " Frederic Weisbecker
2018-05-06 19:19 ` [PATCH 7/9] powerpc: " Frederic Weisbecker
2018-05-06 19:19 ` [PATCH 8/9] perf/breakpoint: Split breakpoint " Frederic Weisbecker
2018-05-07  0:46   ` Joel Fernandes
2018-05-15 13:53     ` Frederic Weisbecker
2018-05-15 15:18       ` Joel Fernandes
2018-05-09  9:17   ` Peter Zijlstra
2018-05-15  6:57     ` Ingo Molnar
2018-05-15 13:58       ` Frederic Weisbecker
2018-05-16  3:11     ` Frederic Weisbecker
2018-05-16  4:58       ` Andy Lutomirski
2018-05-19  2:42         ` Frederic Weisbecker
2018-05-06 19:19 ` [PATCH 9/9] perf/breakpoint: Only commit breakpoint to arch upon slot reservation success Frederic Weisbecker

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=1525634395-23380-6-git-send-email-frederic@kernel.org \
    --to=frederic@kernel.org \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=benh@kernel.crashing.org \
    --cc=catalin.marinas@arm.com \
    --cc=chris@zankel.net \
    --cc=dalias@libc.org \
    --cc=jcmvbkbc@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=namhyung@kernel.org \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=will.deacon@arm.com \
    --cc=ysato@users.sourceforge.jp \
    --subject='Re: [PATCH 5/9] xtensa: Split breakpoint validation into "check" and "commit"' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).