All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2.1] arm64: kgdb: handle read-only text / modules
@ 2016-09-23  7:42 ` AKASHI Takahiro
  0 siblings, 0 replies; 8+ messages in thread
From: AKASHI Takahiro @ 2016-09-23  7:42 UTC (permalink / raw)
  To: catalin.marinas, will.deacon, jason.wessel
  Cc: mark.rutland, linux-arm-kernel, kgdb-bugreport, stable, AKASHI Takahiro

Handle read-only cases (CONFIG_DEBUG_RODATA/CONFIG_DEBUG_SET_MODULE_RONX)
by using aarch64_insn_write() instead of probe_kernel_write().
See how this works:
    commit 2f896d586610 ("arm64: use fixmap for text patching")

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Jason Wessel <jason.wessel@windriver.com>
Cc: <stable@vger.kernel.org> # 3.18-3.19: 2f896d5: arm64: use fixmap
Cc: <stable@vger.kernel.org> # 3.18-3.19: f6242ca: arm64: Fix text
Cc: <stable@vger.kernel.org> # 4.0-
---
 arch/arm64/include/asm/debug-monitors.h |  2 --
 arch/arm64/kernel/kgdb.c                | 36 ++++++++++++++++++++++-----------
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index 4b6b3f7..b71420a 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -61,8 +61,6 @@
 
 #define AARCH64_BREAK_KGDB_DYN_DBG	\
 	(AARCH64_BREAK_MON | (KGDB_DYN_DBG_BRK_IMM << 5))
-#define KGDB_DYN_BRK_INS_BYTE(x)	\
-	((AARCH64_BREAK_KGDB_DYN_DBG >> (8 * (x))) & 0xff)
 
 #define CACHE_FLUSH_IS_SAFE		1
 
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index 6732a27..b06a7a2 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -19,6 +19,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/bug.h>
 #include <linux/cpumask.h>
 #include <linux/irq.h>
 #include <linux/irq_work.h>
@@ -26,6 +27,8 @@
 #include <linux/kgdb.h>
 #include <linux/kprobes.h>
 #include <linux/percpu.h>
+#include <asm/debug-monitors.h>
+#include <asm/insn.h>
 #include <asm/ptrace.h>
 #include <asm/traps.h>
 
@@ -370,15 +373,24 @@ void kgdb_arch_exit(void)
 	unregister_die_notifier(&kgdb_notifier);
 }
 
-/*
- * ARM instructions are always in LE.
- * Break instruction is encoded in LE format
- */
-struct kgdb_arch arch_kgdb_ops = {
-	.gdb_bpt_instr = {
-		KGDB_DYN_BRK_INS_BYTE(0),
-		KGDB_DYN_BRK_INS_BYTE(1),
-		KGDB_DYN_BRK_INS_BYTE(2),
-		KGDB_DYN_BRK_INS_BYTE(3),
-	}
-};
+struct kgdb_arch arch_kgdb_ops;
+
+int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
+{
+	int err;
+
+	BUILD_BUG_ON(AARCH64_INSN_SIZE != BREAK_INSTR_SIZE);
+
+	err = aarch64_insn_read((void *)bpt->bpt_addr, (u32 *)bpt->saved_instr);
+	if (err)
+		return err;
+
+	return aarch64_insn_write((void *)bpt->bpt_addr,
+			(u32)AARCH64_BREAK_KGDB_DYN_DBG);
+}
+
+int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
+{
+	return aarch64_insn_write((void *)bpt->bpt_addr,
+			*(u32 *)bpt->saved_instr);
+}
-- 
2.10.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2.1] arm64: kgdb: handle read-only text / modules
@ 2016-09-23  7:42 ` AKASHI Takahiro
  0 siblings, 0 replies; 8+ messages in thread
From: AKASHI Takahiro @ 2016-09-23  7:42 UTC (permalink / raw)
  To: linux-arm-kernel

Handle read-only cases (CONFIG_DEBUG_RODATA/CONFIG_DEBUG_SET_MODULE_RONX)
by using aarch64_insn_write() instead of probe_kernel_write().
See how this works:
    commit 2f896d586610 ("arm64: use fixmap for text patching")

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Jason Wessel <jason.wessel@windriver.com>
Cc: <stable@vger.kernel.org> # 3.18-3.19: 2f896d5: arm64: use fixmap
Cc: <stable@vger.kernel.org> # 3.18-3.19: f6242ca: arm64: Fix text
Cc: <stable@vger.kernel.org> # 4.0-
---
 arch/arm64/include/asm/debug-monitors.h |  2 --
 arch/arm64/kernel/kgdb.c                | 36 ++++++++++++++++++++++-----------
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index 4b6b3f7..b71420a 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -61,8 +61,6 @@
 
 #define AARCH64_BREAK_KGDB_DYN_DBG	\
 	(AARCH64_BREAK_MON | (KGDB_DYN_DBG_BRK_IMM << 5))
-#define KGDB_DYN_BRK_INS_BYTE(x)	\
-	((AARCH64_BREAK_KGDB_DYN_DBG >> (8 * (x))) & 0xff)
 
 #define CACHE_FLUSH_IS_SAFE		1
 
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index 6732a27..b06a7a2 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -19,6 +19,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/bug.h>
 #include <linux/cpumask.h>
 #include <linux/irq.h>
 #include <linux/irq_work.h>
@@ -26,6 +27,8 @@
 #include <linux/kgdb.h>
 #include <linux/kprobes.h>
 #include <linux/percpu.h>
+#include <asm/debug-monitors.h>
+#include <asm/insn.h>
 #include <asm/ptrace.h>
 #include <asm/traps.h>
 
@@ -370,15 +373,24 @@ void kgdb_arch_exit(void)
 	unregister_die_notifier(&kgdb_notifier);
 }
 
-/*
- * ARM instructions are always in LE.
- * Break instruction is encoded in LE format
- */
-struct kgdb_arch arch_kgdb_ops = {
-	.gdb_bpt_instr = {
-		KGDB_DYN_BRK_INS_BYTE(0),
-		KGDB_DYN_BRK_INS_BYTE(1),
-		KGDB_DYN_BRK_INS_BYTE(2),
-		KGDB_DYN_BRK_INS_BYTE(3),
-	}
-};
+struct kgdb_arch arch_kgdb_ops;
+
+int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
+{
+	int err;
+
+	BUILD_BUG_ON(AARCH64_INSN_SIZE != BREAK_INSTR_SIZE);
+
+	err = aarch64_insn_read((void *)bpt->bpt_addr, (u32 *)bpt->saved_instr);
+	if (err)
+		return err;
+
+	return aarch64_insn_write((void *)bpt->bpt_addr,
+			(u32)AARCH64_BREAK_KGDB_DYN_DBG);
+}
+
+int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
+{
+	return aarch64_insn_write((void *)bpt->bpt_addr,
+			*(u32 *)bpt->saved_instr);
+}
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2.1] arm64: kgdb: handle read-only text / modules
  2016-09-23  7:42 ` AKASHI Takahiro
@ 2016-09-23  9:49   ` Catalin Marinas
  -1 siblings, 0 replies; 8+ messages in thread
From: Catalin Marinas @ 2016-09-23  9:49 UTC (permalink / raw)
  To: AKASHI Takahiro
  Cc: will.deacon, jason.wessel, mark.rutland, kgdb-bugreport, stable,
	linux-arm-kernel

On Fri, Sep 23, 2016 at 04:42:08PM +0900, AKASHI Takahiro wrote:
> Handle read-only cases (CONFIG_DEBUG_RODATA/CONFIG_DEBUG_SET_MODULE_RONX)
> by using aarch64_insn_write() instead of probe_kernel_write().
> See how this works:
>     commit 2f896d586610 ("arm64: use fixmap for text patching")
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Jason Wessel <jason.wessel@windriver.com>
> Cc: <stable@vger.kernel.org> # 3.18-3.19: 2f896d5: arm64: use fixmap
> Cc: <stable@vger.kernel.org> # 3.18-3.19: f6242ca: arm64: Fix text
> Cc: <stable@vger.kernel.org> # 4.0-

Queued for 4.8 with a slight change in the last Cc: tag above:

Cc: <stable@vger.kernel.org> # 3.18.x-

Thanks.

-- 
Catalin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2.1] arm64: kgdb: handle read-only text / modules
@ 2016-09-23  9:49   ` Catalin Marinas
  0 siblings, 0 replies; 8+ messages in thread
From: Catalin Marinas @ 2016-09-23  9:49 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Sep 23, 2016 at 04:42:08PM +0900, AKASHI Takahiro wrote:
> Handle read-only cases (CONFIG_DEBUG_RODATA/CONFIG_DEBUG_SET_MODULE_RONX)
> by using aarch64_insn_write() instead of probe_kernel_write().
> See how this works:
>     commit 2f896d586610 ("arm64: use fixmap for text patching")
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Jason Wessel <jason.wessel@windriver.com>
> Cc: <stable@vger.kernel.org> # 3.18-3.19: 2f896d5: arm64: use fixmap
> Cc: <stable@vger.kernel.org> # 3.18-3.19: f6242ca: arm64: Fix text
> Cc: <stable@vger.kernel.org> # 4.0-

Queued for 4.8 with a slight change in the last Cc: tag above:

Cc: <stable@vger.kernel.org> # 3.18.x-

Thanks.

-- 
Catalin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2.1] arm64: kgdb: handle read-only text / modules
  2016-09-23  9:49   ` Catalin Marinas
@ 2016-09-23 10:23     ` Catalin Marinas
  -1 siblings, 0 replies; 8+ messages in thread
From: Catalin Marinas @ 2016-09-23 10:23 UTC (permalink / raw)
  To: AKASHI Takahiro
  Cc: mark.rutland, kgdb-bugreport, will.deacon, stable, jason.wessel,
	linux-arm-kernel

On Fri, Sep 23, 2016 at 10:49:53AM +0100, Catalin Marinas wrote:
> On Fri, Sep 23, 2016 at 04:42:08PM +0900, AKASHI Takahiro wrote:
> > Handle read-only cases (CONFIG_DEBUG_RODATA/CONFIG_DEBUG_SET_MODULE_RONX)
> > by using aarch64_insn_write() instead of probe_kernel_write().
> > See how this works:
> >     commit 2f896d586610 ("arm64: use fixmap for text patching")
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Jason Wessel <jason.wessel@windriver.com>
> > Cc: <stable@vger.kernel.org> # 3.18-3.19: 2f896d5: arm64: use fixmap
> > Cc: <stable@vger.kernel.org> # 3.18-3.19: f6242ca: arm64: Fix text
> > Cc: <stable@vger.kernel.org> # 4.0-
> 
> Queued for 4.8 with a slight change in the last Cc: tag above:
> 
> Cc: <stable@vger.kernel.org> # 3.18.x-

I tried to apply this patch to 3.18, 4.1, 4.4, 4.7. It fails on all of
them with smaller or slightly larger conflicts.

So, I'll merge it in 4.8 without any "Cc: stable" tags, just a "Fixes:"
one for the commit introducing CONFIG_DEBUG_SET_MODULE_RONX (3.18). Once
the patch reaches mainline, could you please back-port and test it on
the stable kernel versions and send separate patches to
stable@vger.kernel.org?

Thanks.

-- 
Catalin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2.1] arm64: kgdb: handle read-only text / modules
@ 2016-09-23 10:23     ` Catalin Marinas
  0 siblings, 0 replies; 8+ messages in thread
From: Catalin Marinas @ 2016-09-23 10:23 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Sep 23, 2016 at 10:49:53AM +0100, Catalin Marinas wrote:
> On Fri, Sep 23, 2016 at 04:42:08PM +0900, AKASHI Takahiro wrote:
> > Handle read-only cases (CONFIG_DEBUG_RODATA/CONFIG_DEBUG_SET_MODULE_RONX)
> > by using aarch64_insn_write() instead of probe_kernel_write().
> > See how this works:
> >     commit 2f896d586610 ("arm64: use fixmap for text patching")
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Jason Wessel <jason.wessel@windriver.com>
> > Cc: <stable@vger.kernel.org> # 3.18-3.19: 2f896d5: arm64: use fixmap
> > Cc: <stable@vger.kernel.org> # 3.18-3.19: f6242ca: arm64: Fix text
> > Cc: <stable@vger.kernel.org> # 4.0-
> 
> Queued for 4.8 with a slight change in the last Cc: tag above:
> 
> Cc: <stable@vger.kernel.org> # 3.18.x-

I tried to apply this patch to 3.18, 4.1, 4.4, 4.7. It fails on all of
them with smaller or slightly larger conflicts.

So, I'll merge it in 4.8 without any "Cc: stable" tags, just a "Fixes:"
one for the commit introducing CONFIG_DEBUG_SET_MODULE_RONX (3.18). Once
the patch reaches mainline, could you please back-port and test it on
the stable kernel versions and send separate patches to
stable at vger.kernel.org?

Thanks.

-- 
Catalin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2.1] arm64: kgdb: handle read-only text / modules
  2016-09-23 10:23     ` Catalin Marinas
@ 2016-09-25  0:29       ` AKASHI Takahiro
  -1 siblings, 0 replies; 8+ messages in thread
From: AKASHI Takahiro @ 2016-09-25  0:29 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: mark.rutland, kgdb-bugreport, will.deacon, stable, jason.wessel,
	linux-arm-kernel

Hi Catalin,

On Fri, Sep 23, 2016 at 11:23:40AM +0100, Catalin Marinas wrote:
> On Fri, Sep 23, 2016 at 10:49:53AM +0100, Catalin Marinas wrote:
> > On Fri, Sep 23, 2016 at 04:42:08PM +0900, AKASHI Takahiro wrote:
> > > Handle read-only cases (CONFIG_DEBUG_RODATA/CONFIG_DEBUG_SET_MODULE_RONX)
> > > by using aarch64_insn_write() instead of probe_kernel_write().
> > > See how this works:
> > >     commit 2f896d586610 ("arm64: use fixmap for text patching")
> > > 
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Will Deacon <will.deacon@arm.com>
> > > Cc: Jason Wessel <jason.wessel@windriver.com>
> > > Cc: <stable@vger.kernel.org> # 3.18-3.19: 2f896d5: arm64: use fixmap
> > > Cc: <stable@vger.kernel.org> # 3.18-3.19: f6242ca: arm64: Fix text
> > > Cc: <stable@vger.kernel.org> # 4.0-
> > 
> > Queued for 4.8 with a slight change in the last Cc: tag above:
> > 
> > Cc: <stable@vger.kernel.org> # 3.18.x-
> 
> I tried to apply this patch to 3.18, 4.1, 4.4, 4.7. It fails on all of
> them with smaller or slightly larger conflicts.

Oh, too bad.
I guest we'd better revert the following patches as well:
    c696b93 arm64/debug: Simplify BRK insn opcode declarations
    c172d99 arm64/debug: More consistent naming for the BRK ESR template macro
but I will check anyway.

> So, I'll merge it in 4.8 without any "Cc: stable" tags, just a "Fixes:"
> one for the commit introducing CONFIG_DEBUG_SET_MODULE_RONX (3.18). Once
> the patch reaches mainline, could you please back-port and test it on
> the stable kernel versions and send separate patches to
> stable@vger.kernel.org?

Thanks,
-Takahiro AKASHI

> Thanks.
> 
> -- 
> Catalin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2.1] arm64: kgdb: handle read-only text / modules
@ 2016-09-25  0:29       ` AKASHI Takahiro
  0 siblings, 0 replies; 8+ messages in thread
From: AKASHI Takahiro @ 2016-09-25  0:29 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Catalin,

On Fri, Sep 23, 2016 at 11:23:40AM +0100, Catalin Marinas wrote:
> On Fri, Sep 23, 2016 at 10:49:53AM +0100, Catalin Marinas wrote:
> > On Fri, Sep 23, 2016 at 04:42:08PM +0900, AKASHI Takahiro wrote:
> > > Handle read-only cases (CONFIG_DEBUG_RODATA/CONFIG_DEBUG_SET_MODULE_RONX)
> > > by using aarch64_insn_write() instead of probe_kernel_write().
> > > See how this works:
> > >     commit 2f896d586610 ("arm64: use fixmap for text patching")
> > > 
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Will Deacon <will.deacon@arm.com>
> > > Cc: Jason Wessel <jason.wessel@windriver.com>
> > > Cc: <stable@vger.kernel.org> # 3.18-3.19: 2f896d5: arm64: use fixmap
> > > Cc: <stable@vger.kernel.org> # 3.18-3.19: f6242ca: arm64: Fix text
> > > Cc: <stable@vger.kernel.org> # 4.0-
> > 
> > Queued for 4.8 with a slight change in the last Cc: tag above:
> > 
> > Cc: <stable@vger.kernel.org> # 3.18.x-
> 
> I tried to apply this patch to 3.18, 4.1, 4.4, 4.7. It fails on all of
> them with smaller or slightly larger conflicts.

Oh, too bad.
I guest we'd better revert the following patches as well:
    c696b93 arm64/debug: Simplify BRK insn opcode declarations
    c172d99 arm64/debug: More consistent naming for the BRK ESR template macro
but I will check anyway.

> So, I'll merge it in 4.8 without any "Cc: stable" tags, just a "Fixes:"
> one for the commit introducing CONFIG_DEBUG_SET_MODULE_RONX (3.18). Once
> the patch reaches mainline, could you please back-port and test it on
> the stable kernel versions and send separate patches to
> stable at vger.kernel.org?

Thanks,
-Takahiro AKASHI

> Thanks.
> 
> -- 
> Catalin

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-09-25  0:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-23  7:42 [PATCH v2.1] arm64: kgdb: handle read-only text / modules AKASHI Takahiro
2016-09-23  7:42 ` AKASHI Takahiro
2016-09-23  9:49 ` Catalin Marinas
2016-09-23  9:49   ` Catalin Marinas
2016-09-23 10:23   ` Catalin Marinas
2016-09-23 10:23     ` Catalin Marinas
2016-09-25  0:29     ` AKASHI Takahiro
2016-09-25  0:29       ` AKASHI Takahiro

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.