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 7/9] powerpc: Split breakpoint validation into "check" and "commit"
Date: Sun,  6 May 2018 21:19:53 +0200	[thread overview]
Message-ID: <1525634395-23380-8-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/powerpc/kernel/hw_breakpoint.c | 82 ++++++++++++++++++++++++-------------
 1 file changed, 53 insertions(+), 29 deletions(-)

diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 4c1012b..c39fc86 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -138,33 +138,20 @@ int arch_bp_generic_fields(int type, int *gen_bp_type)
 	return 0;
 }
 
-/*
- * Validate the arch-specific HW Breakpoint register settings
- */
-int arch_validate_hwbkpt_settings(struct perf_event *bp)
+static int hw_breakpoint_arch_check(struct perf_event *bp,
+				    const struct perf_event_attr *attr)
 {
-	int ret = -EINVAL, length_max;
-	struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+	int length_max;
 
 	if (!bp)
-		return ret;
+		return -EINVAL;
 
-	info->type = HW_BRK_TYPE_TRANSLATE;
-	if (bp->attr.bp_type & HW_BREAKPOINT_R)
-		info->type |= HW_BRK_TYPE_READ;
-	if (bp->attr.bp_type & HW_BREAKPOINT_W)
-		info->type |= HW_BRK_TYPE_WRITE;
-	if (info->type == HW_BRK_TYPE_TRANSLATE)
-		/* must set alteast read or write */
-		return ret;
-	if (!(bp->attr.exclude_user))
-		info->type |= HW_BRK_TYPE_USER;
-	if (!(bp->attr.exclude_kernel))
-		info->type |= HW_BRK_TYPE_KERNEL;
-	if (!(bp->attr.exclude_hv))
-		info->type |= HW_BRK_TYPE_HYP;
-	info->address = bp->attr.bp_addr;
-	info->len = bp->attr.bp_len;
+	/* Must set at least read or write */
+	if (!(attr->bp_type & (HW_BREAKPOINT_R | HW_BREAKPOINT_W)))
+		return -EINVAL;
+
+	if (!ppc_breakpoint_available())
+		return -ENODEV;
 
 	/*
 	 * Since breakpoint length can be a maximum of HW_BREAKPOINT_LEN(8)
@@ -172,19 +159,56 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp)
 	 * HW_BREAKPOINT_ALIGN by rounding off to the lower address, the
 	 * 'symbolsize' should satisfy the check below.
 	 */
-	if (!ppc_breakpoint_available())
-		return -ENODEV;
 	length_max = 8; /* DABR */
 	if (cpu_has_feature(CPU_FTR_DAWR)) {
 		length_max = 512 ; /* 64 doublewords */
 		/* DAWR region can't cross 512 boundary */
-		if ((bp->attr.bp_addr >> 10) != 
-		    ((bp->attr.bp_addr + bp->attr.bp_len - 1) >> 10))
+		if ((attr->bp_addr >> 10) !=
+		    ((attr->bp_addr + attr->bp_len - 1) >> 10))
 			return -EINVAL;
 	}
-	if (info->len >
-	    (length_max - (info->address & HW_BREAKPOINT_ALIGN)))
+	if (attr->bp_len >
+	    (length_max - (attr->bp_addr & HW_BREAKPOINT_ALIGN)))
 		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;
+
+	info->type = HW_BRK_TYPE_TRANSLATE;
+	if (attr->bp_type & HW_BREAKPOINT_R)
+		info->type |= HW_BRK_TYPE_READ;
+	if (attr->bp_type & HW_BREAKPOINT_W)
+		info->type |= HW_BRK_TYPE_WRITE;
+
+	if (!attr->exclude_user)
+		info->type |= HW_BRK_TYPE_USER;
+	if (!attr->exclude_kernel)
+		info->type |= HW_BRK_TYPE_KERNEL;
+	if (!attr->exclude_hv)
+		info->type |= HW_BRK_TYPE_HYP;
+
+	info->address = attr->bp_addr;
+	info->len = attr->bp_len;
+}
+
+/*
+ * Validate the arch-specific HW Breakpoint register settings
+ */
+int arch_validate_hwbkpt_settings(struct perf_event *bp)
+{
+	int err;
+
+	err = hw_breakpoint_arch_check(bp, &bp->attr);
+	if (err)
+		return err;
+
+	hw_breakpoint_arch_commit(bp);
+
 	return 0;
 }
 
-- 
2.7.4

  parent reply	other threads:[~2018-05-06 19:20 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 ` [PATCH 5/9] xtensa: " Frederic Weisbecker
2018-05-06 19:19 ` [PATCH 6/9] arm64: " Frederic Weisbecker
2018-05-06 19:19 ` Frederic Weisbecker [this message]
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-8-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 7/9] powerpc: 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).