* [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode() @ 2019-09-26 7:38 Candle Sun 2019-09-30 15:34 ` Will Deacon ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Candle Sun @ 2019-09-26 7:38 UTC (permalink / raw) To: will, mark.rutland, linux Cc: linux-arm-kernel, linux-kernel, Candle Sun, Nianfu Bai From: Candle Sun <candle.sun@unisoc.com> When ARMv8.1/ARMv8.2 cores are used in AArch32 mode, arch_hw_breakpoint_init() in arch/arm/kernel/hw_breakpoint.c will be used. From ARMv8 specification, different debug architecture versions defined: * 0110 ARMv8, v8 Debug architecture. * 0111 ARMv8.1, v8 Debug architecture, with Virtualization Host Extensions. * 1000 ARMv8.2, v8.2 Debug architecture. So missing ARMv8.1/ARMv8.2 cases will cause enable_monitor_mode() function returns -ENODEV, and arch_hw_breakpoint_init() will fail. Signed-off-by: Candle Sun <candle.sun@unisoc.com> Signed-off-by: Nianfu Bai <nianfu.bai@unisoc.com> --- arch/arm/include/asm/hw_breakpoint.h | 2 ++ arch/arm/kernel/hw_breakpoint.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/arch/arm/include/asm/hw_breakpoint.h b/arch/arm/include/asm/hw_breakpoint.h index ac54c06..9137ef6 100644 --- a/arch/arm/include/asm/hw_breakpoint.h +++ b/arch/arm/include/asm/hw_breakpoint.h @@ -53,6 +53,8 @@ static inline void decode_ctrl_reg(u32 reg, #define ARM_DEBUG_ARCH_V7_MM 4 #define ARM_DEBUG_ARCH_V7_1 5 #define ARM_DEBUG_ARCH_V8 6 +#define ARM_DEBUG_ARCH_V8_1 7 +#define ARM_DEBUG_ARCH_V8_2 8 /* Breakpoint */ #define ARM_BREAKPOINT_EXECUTE 0 diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c index b0c195e..cb99612 100644 --- a/arch/arm/kernel/hw_breakpoint.c +++ b/arch/arm/kernel/hw_breakpoint.c @@ -246,6 +246,8 @@ static int enable_monitor_mode(void) case ARM_DEBUG_ARCH_V7_ECP14: case ARM_DEBUG_ARCH_V7_1: case ARM_DEBUG_ARCH_V8: + case ARM_DEBUG_ARCH_V8_1: + case ARM_DEBUG_ARCH_V8_2: ARM_DBG_WRITE(c0, c2, 2, (dscr | ARM_DSCR_MDBGEN)); isb(); break; -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode() 2019-09-26 7:38 [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode() Candle Sun @ 2019-09-30 15:34 ` Will Deacon 2019-10-08 7:20 ` Candle Sun 2019-10-11 5:59 ` Uwe Kleine-König 2019-10-22 12:25 ` Will Deacon 2 siblings, 1 reply; 8+ messages in thread From: Will Deacon @ 2019-09-30 15:34 UTC (permalink / raw) To: Candle Sun Cc: mark.rutland, linux, linux-arm-kernel, linux-kernel, Candle Sun, Nianfu Bai On Thu, Sep 26, 2019 at 03:38:28PM +0800, Candle Sun wrote: > From: Candle Sun <candle.sun@unisoc.com> > > When ARMv8.1/ARMv8.2 cores are used in AArch32 mode, > arch_hw_breakpoint_init() in arch/arm/kernel/hw_breakpoint.c will be used. > > From ARMv8 specification, different debug architecture versions defined: > * 0110 ARMv8, v8 Debug architecture. > * 0111 ARMv8.1, v8 Debug architecture, with Virtualization Host Extensions. > * 1000 ARMv8.2, v8.2 Debug architecture. > > So missing ARMv8.1/ARMv8.2 cases will cause enable_monitor_mode() function > returns -ENODEV, and arch_hw_breakpoint_init() will fail. > > Signed-off-by: Candle Sun <candle.sun@unisoc.com> > Signed-off-by: Nianfu Bai <nianfu.bai@unisoc.com> > --- > arch/arm/include/asm/hw_breakpoint.h | 2 ++ > arch/arm/kernel/hw_breakpoint.c | 2 ++ > 2 files changed, 4 insertions(+) How did you test this? Will ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode() 2019-09-30 15:34 ` Will Deacon @ 2019-10-08 7:20 ` Candle Sun 2019-10-11 5:56 ` Candle Sun 0 siblings, 1 reply; 8+ messages in thread From: Candle Sun @ 2019-10-08 7:20 UTC (permalink / raw) To: Will Deacon Cc: mark.rutland, linux, linux-arm-kernel, lkml, Candle Sun, Nianfu Bai Hi Will, Sorry for not instant respond. On Mon, Sep 30, 2019 at 11:34 PM Will Deacon <will@kernel.org> wrote: > > On Thu, Sep 26, 2019 at 03:38:28PM +0800, Candle Sun wrote: > > From: Candle Sun <candle.sun@unisoc.com> > > > > When ARMv8.1/ARMv8.2 cores are used in AArch32 mode, > > arch_hw_breakpoint_init() in arch/arm/kernel/hw_breakpoint.c will be used. > > > > From ARMv8 specification, different debug architecture versions defined: > > * 0110 ARMv8, v8 Debug architecture. > > * 0111 ARMv8.1, v8 Debug architecture, with Virtualization Host Extensions. > > * 1000 ARMv8.2, v8.2 Debug architecture. > > > > So missing ARMv8.1/ARMv8.2 cases will cause enable_monitor_mode() function > > returns -ENODEV, and arch_hw_breakpoint_init() will fail. > > > > Signed-off-by: Candle Sun <candle.sun@unisoc.com> > > Signed-off-by: Nianfu Bai <nianfu.bai@unisoc.com> > > --- > > arch/arm/include/asm/hw_breakpoint.h | 2 ++ > > arch/arm/kernel/hw_breakpoint.c | 2 ++ > > 2 files changed, 4 insertions(+) > > How did you test this? > > Will We have the SoC with A55 cores. On one Android project, for saving memory usage, we let A55 run in aarch32 mode. While the following failures occue on Android CtsBionicTestCases: --sys_ptrace#watchpoint_imprecisede --sys_ptrace#hardware_breakpoint --sys_ptrace#watchpoint_stress The code snippet for testing is: static void check_hw_feature_supported(pid_t child, HwFeature feature) { #if defined(__arm__) long capabilities; long result = ptrace(PTRACE_GETHBPREGS, child, 0, &capabilities); if (result == -1) { EXPECT_EQ(EIO, errno); GTEST_SKIP() << "Hardware debug support disabled at kernel configuration time"; } uint8_t hb_count = capabilities & 0xff; capabilities >>= 8; uint8_t wp_count = capabilities & 0xff; capabilities >>= 8; uint8_t max_wp_size = capabilities & 0xff; if (max_wp_size == 0) { GTEST_SKIP() << "Kernel reports zero maximum watchpoint size"; } else if (feature == HwFeature::Watchpoint && wp_count == 0) { GTEST_SKIP() << "Kernel reports zero hardware watchpoints"; } else if (feature == HwFeature::Breakpoint && hb_count == 0) { GTEST_SKIP() << "Kernel reports zero hardware breakpoints"; } #elif defined(__aarch64__) user_hwdebug_state dreg_state; iovec iov; iov.iov_base = &dreg_state; iov.iov_len = sizeof(dreg_state); long result = ptrace(PTRACE_GETREGSET, child, feature == HwFeature::Watchpoint ? NT_ARM_HW_WATCH : NT_ARM_HW_BREAK, &iov); if (result == -1) { ASSERT_EQ(EINVAL, errno); } if ((dreg_state.dbg_info & 0xff) == 0) GTEST_SKIP() << "hardware support missing"; #else // We assume watchpoints and breakpoints are always supported on x86. UNUSED(child); UNUSED(feature); #endif } The max_wp_size field returned by __ptrace() from kernel is zero, which causes the test failures. After futher analysis, we found max_watchpoint_len variable is not right initialized in kernel arch_hw_breakpoint_init() function. Missing the case of ARM_DEBUG_ARCH_V8_2 in enable_monitor_mode() directly aborts the arch_hw_breakpoint_int(). Candle Best regards ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode() 2019-10-08 7:20 ` Candle Sun @ 2019-10-11 5:56 ` Candle Sun 0 siblings, 0 replies; 8+ messages in thread From: Candle Sun @ 2019-10-11 5:56 UTC (permalink / raw) To: Will Deacon Cc: mark.rutland, linux, linux-arm-kernel, lkml, Candle Sun, Nianfu Bai Will, Is the patch useful for you? Would you please give me some suggestions? Thank you. Regards, Candle On Tue, Oct 8, 2019 at 3:20 PM Candle Sun <candlesea@gmail.com> wrote: > > Hi Will, > Sorry for not instant respond. > > > On Mon, Sep 30, 2019 at 11:34 PM Will Deacon <will@kernel.org> wrote: > > > > On Thu, Sep 26, 2019 at 03:38:28PM +0800, Candle Sun wrote: > > > From: Candle Sun <candle.sun@unisoc.com> > > > > > > When ARMv8.1/ARMv8.2 cores are used in AArch32 mode, > > > arch_hw_breakpoint_init() in arch/arm/kernel/hw_breakpoint.c will be used. > > > > > > From ARMv8 specification, different debug architecture versions defined: > > > * 0110 ARMv8, v8 Debug architecture. > > > * 0111 ARMv8.1, v8 Debug architecture, with Virtualization Host Extensions. > > > * 1000 ARMv8.2, v8.2 Debug architecture. > > > > > > So missing ARMv8.1/ARMv8.2 cases will cause enable_monitor_mode() function > > > returns -ENODEV, and arch_hw_breakpoint_init() will fail. > > > > > > Signed-off-by: Candle Sun <candle.sun@unisoc.com> > > > Signed-off-by: Nianfu Bai <nianfu.bai@unisoc.com> > > > --- > > > arch/arm/include/asm/hw_breakpoint.h | 2 ++ > > > arch/arm/kernel/hw_breakpoint.c | 2 ++ > > > 2 files changed, 4 insertions(+) > > > > How did you test this? > > > > Will > > We have the SoC with A55 cores. On one Android project, for saving memory usage, > we let A55 run in aarch32 mode. > While the following failures occue on Android CtsBionicTestCases: > --sys_ptrace#watchpoint_imprecisede > --sys_ptrace#hardware_breakpoint > --sys_ptrace#watchpoint_stress > > The code snippet for testing is: > > static void check_hw_feature_supported(pid_t child, HwFeature feature) { > #if defined(__arm__) > long capabilities; > long result = ptrace(PTRACE_GETHBPREGS, child, 0, &capabilities); > if (result == -1) { > EXPECT_EQ(EIO, errno); > GTEST_SKIP() << "Hardware debug support disabled at kernel > configuration time"; > } > uint8_t hb_count = capabilities & 0xff; > capabilities >>= 8; > uint8_t wp_count = capabilities & 0xff; > capabilities >>= 8; > uint8_t max_wp_size = capabilities & 0xff; > if (max_wp_size == 0) { > GTEST_SKIP() << "Kernel reports zero maximum watchpoint size"; > } else if (feature == HwFeature::Watchpoint && wp_count == 0) { > GTEST_SKIP() << "Kernel reports zero hardware watchpoints"; > } else if (feature == HwFeature::Breakpoint && hb_count == 0) { > GTEST_SKIP() << "Kernel reports zero hardware breakpoints"; > } > #elif defined(__aarch64__) > user_hwdebug_state dreg_state; > iovec iov; > iov.iov_base = &dreg_state; > iov.iov_len = sizeof(dreg_state); > > long result = ptrace(PTRACE_GETREGSET, child, > feature == HwFeature::Watchpoint ? > NT_ARM_HW_WATCH : NT_ARM_HW_BREAK, &iov); > if (result == -1) { > ASSERT_EQ(EINVAL, errno); > } > if ((dreg_state.dbg_info & 0xff) == 0) GTEST_SKIP() << "hardware > support missing"; > #else > // We assume watchpoints and breakpoints are always supported on x86. > UNUSED(child); > UNUSED(feature); > #endif > } > > The max_wp_size field returned by __ptrace() from kernel is zero, > which causes the test failures. > > After futher analysis, we found max_watchpoint_len variable is not > right initialized in kernel > arch_hw_breakpoint_init() function. Missing the case of ARM_DEBUG_ARCH_V8_2 in > enable_monitor_mode() directly aborts the arch_hw_breakpoint_int(). > > Candle > Best regards ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode() 2019-09-26 7:38 [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode() Candle Sun 2019-09-30 15:34 ` Will Deacon @ 2019-10-11 5:59 ` Uwe Kleine-König 2019-10-11 6:53 ` Candle Sun 2019-10-22 12:25 ` Will Deacon 2 siblings, 1 reply; 8+ messages in thread From: Uwe Kleine-König @ 2019-10-11 5:59 UTC (permalink / raw) To: Candle Sun Cc: will, mark.rutland, linux, Nianfu Bai, Candle Sun, linux-kernel, linux-arm-kernel Hello, just noticed a typo in the subject line while going through my lakml mailbox: s/architecutre/architecture/ Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode() 2019-10-11 5:59 ` Uwe Kleine-König @ 2019-10-11 6:53 ` Candle Sun 0 siblings, 0 replies; 8+ messages in thread From: Candle Sun @ 2019-10-11 6:53 UTC (permalink / raw) To: Uwe Kleine-König Cc: Will Deacon, mark.rutland, linux, Nianfu Bai, Candle Sun, lkml, linux-arm-kernel Thanks Uwe for pointing out my typing error. Will, Is the patch ok? Do I need to send another version? Candle Candle On Fri, Oct 11, 2019 at 2:00 PM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > Hello, > > just noticed a typo in the subject line while going through my lakml > mailbox: > > s/architecutre/architecture/ > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode() 2019-09-26 7:38 [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode() Candle Sun 2019-09-30 15:34 ` Will Deacon 2019-10-11 5:59 ` Uwe Kleine-König @ 2019-10-22 12:25 ` Will Deacon 2019-10-22 12:37 ` Candle Sun 2 siblings, 1 reply; 8+ messages in thread From: Will Deacon @ 2019-10-22 12:25 UTC (permalink / raw) To: Candle Sun Cc: mark.rutland, linux, linux-arm-kernel, linux-kernel, Candle Sun, Nianfu Bai On Thu, Sep 26, 2019 at 03:38:28PM +0800, Candle Sun wrote: > From: Candle Sun <candle.sun@unisoc.com> > > When ARMv8.1/ARMv8.2 cores are used in AArch32 mode, > arch_hw_breakpoint_init() in arch/arm/kernel/hw_breakpoint.c will be used. > > From ARMv8 specification, different debug architecture versions defined: > * 0110 ARMv8, v8 Debug architecture. > * 0111 ARMv8.1, v8 Debug architecture, with Virtualization Host Extensions. > * 1000 ARMv8.2, v8.2 Debug architecture. > > So missing ARMv8.1/ARMv8.2 cases will cause enable_monitor_mode() function > returns -ENODEV, and arch_hw_breakpoint_init() will fail. > > Signed-off-by: Candle Sun <candle.sun@unisoc.com> > Signed-off-by: Nianfu Bai <nianfu.bai@unisoc.com> > --- > arch/arm/include/asm/hw_breakpoint.h | 2 ++ > arch/arm/kernel/hw_breakpoint.c | 2 ++ > 2 files changed, 4 insertions(+) [...] > diff --git a/arch/arm/include/asm/hw_breakpoint.h b/arch/arm/include/asm/hw_breakpoint.h > index ac54c06..9137ef6 100644 > --- a/arch/arm/include/asm/hw_breakpoint.h > +++ b/arch/arm/include/asm/hw_breakpoint.h > @@ -53,6 +53,8 @@ static inline void decode_ctrl_reg(u32 reg, > #define ARM_DEBUG_ARCH_V7_MM 4 > #define ARM_DEBUG_ARCH_V7_1 5 > #define ARM_DEBUG_ARCH_V8 6 > +#define ARM_DEBUG_ARCH_V8_1 7 > +#define ARM_DEBUG_ARCH_V8_2 8 Looks like you can also add: #define ARM_DEBUG_ARCH_V8_4 9 and treat that the same way. With that, and a fix to $SUBJECT: Acked-by: Will Deacon <will@kernel.org> Please put this into the patch system [1]. Will [1] https://www.arm.linux.org.uk/developer/patches/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode() 2019-10-22 12:25 ` Will Deacon @ 2019-10-22 12:37 ` Candle Sun 0 siblings, 0 replies; 8+ messages in thread From: Candle Sun @ 2019-10-22 12:37 UTC (permalink / raw) To: Will Deacon Cc: mark.rutland, linux, linux-arm-kernel, lkml, Candle Sun, Nianfu Bai On Tue, Oct 22, 2019 at 8:25 PM Will Deacon <will@kernel.org> wrote: > > On Thu, Sep 26, 2019 at 03:38:28PM +0800, Candle Sun wrote: > > From: Candle Sun <candle.sun@unisoc.com> > > > > When ARMv8.1/ARMv8.2 cores are used in AArch32 mode, > > arch_hw_breakpoint_init() in arch/arm/kernel/hw_breakpoint.c will be used. > > > > From ARMv8 specification, different debug architecture versions defined: > > * 0110 ARMv8, v8 Debug architecture. > > * 0111 ARMv8.1, v8 Debug architecture, with Virtualization Host Extensions. > > * 1000 ARMv8.2, v8.2 Debug architecture. > > > > So missing ARMv8.1/ARMv8.2 cases will cause enable_monitor_mode() function > > returns -ENODEV, and arch_hw_breakpoint_init() will fail. > > > > Signed-off-by: Candle Sun <candle.sun@unisoc.com> > > Signed-off-by: Nianfu Bai <nianfu.bai@unisoc.com> > > --- > > arch/arm/include/asm/hw_breakpoint.h | 2 ++ > > arch/arm/kernel/hw_breakpoint.c | 2 ++ > > 2 files changed, 4 insertions(+) > > [...] > > > diff --git a/arch/arm/include/asm/hw_breakpoint.h b/arch/arm/include/asm/hw_breakpoint.h > > index ac54c06..9137ef6 100644 > > --- a/arch/arm/include/asm/hw_breakpoint.h > > +++ b/arch/arm/include/asm/hw_breakpoint.h > > @@ -53,6 +53,8 @@ static inline void decode_ctrl_reg(u32 reg, > > #define ARM_DEBUG_ARCH_V7_MM 4 > > #define ARM_DEBUG_ARCH_V7_1 5 > > #define ARM_DEBUG_ARCH_V8 6 > > +#define ARM_DEBUG_ARCH_V8_1 7 > > +#define ARM_DEBUG_ARCH_V8_2 8 > > Looks like you can also add: > > #define ARM_DEBUG_ARCH_V8_4 9 > > and treat that the same way. With that, and a fix to $SUBJECT: > > Acked-by: Will Deacon <will@kernel.org> > > Please put this into the patch system [1]. > > Will > > [1] https://www.arm.linux.org.uk/developer/patches/ Thanks, Will. I will do it ASAP. Best regards, Candle ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-10-22 12:38 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-26 7:38 [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode() Candle Sun 2019-09-30 15:34 ` Will Deacon 2019-10-08 7:20 ` Candle Sun 2019-10-11 5:56 ` Candle Sun 2019-10-11 5:59 ` Uwe Kleine-König 2019-10-11 6:53 ` Candle Sun 2019-10-22 12:25 ` Will Deacon 2019-10-22 12:37 ` Candle Sun
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).