Page MenuHomeFreeBSD
Differential D4037

Introduce support for HW watchpoints and single stepping for ARMv6/v7
ClosedPublic
Actions

Authored by zbb on Oct 30 2015, 4:17 PM.
Tags
None
Referenced Files
Unknown Object (File)
Nov 28 2024, 6:33 AM
Unknown Object (File)
Nov 28 2024, 6:33 AM
Unknown Object (File)
Nov 28 2024, 6:32 AM
Unknown Object (File)
Nov 28 2024, 6:32 AM
Unknown Object (File)
Nov 28 2024, 6:32 AM
Unknown Object (File)
Nov 28 2024, 6:32 AM
Unknown Object (File)
Nov 28 2024, 6:10 AM
Unknown Object (File)
Nov 27 2024, 11:42 AM
View All Files
Subscribers
kib
mmel

Details

Reviewers
andrew
imp
wma_semihalf.com
br
skra
• ian
Group Reviewers
ARM
Commits
rS294740: Introduce support for HW watchpoints and single stepping for ARMv6/v7
Summary

Allows for using hardware watchpoints for 1, 2, 4, 8 byte long addresses.
The default configuration of watchpoint is RW but code allows to select
RO or WO and X.
Since debugging registers are per-CPU (CP14) the watchpoint is set on
the CPU that was lucky (or not) to enter DDB.

HW breakpoints are used to perform single step in KDB.
When HW breakpoint is enabled all watchpoints are temporary disabled
to avoid recursive abort on both watchpoint and breakpoint.
In case of branch, the breakpoint is set to both - next instruction
and possible branch address. This requires at least 2 breakpoints
supported in the CPU however this is a must for ARMv6/v7 CPUs.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

zbb updated this revision to Diff 9825.Oct 30 2015, 4:17 PM
zbb retitled this revision from to Introduce support for HW watchpoints and single stepping for ARMv6/v7.
zbb updated this object.
zbb edited the test plan for this revision. (Show Details)
zbb added reviewers: • ian, imp, andrew, br.
zbb set the repository for this revision to rS FreeBSD src repository - subversion.
zbb added a parent revision: D4036: Add helper to catch single step debug event and distinguish it from bkpt.
andrew added a reviewer: ARM.Oct 30 2015, 4:19 PM
imp accepted this revision.Oct 30 2015, 4:28 PM
imp edited edge metadata.

This looks good to me, but I don't know the arm debug registers enough to know if they are right, so others should look at this too.

This revision is now accepted and ready to land.Oct 30 2015, 4:28 PM
andrew requested changes to this revision.Oct 30 2015, 4:30 PM
andrew edited edge metadata.

I can see some style issue, and would like to check a few things.

This revision now requires changes to proceed.Oct 30 2015, 4:30 PM
zbb added a comment.Oct 31 2015, 3:42 PM
In D4037#84452, @andrew wrote:

I can see some style issue, and would like to check a few things.

Thanks. Understood.
I'm waiting for details.

andrew added inline comments.Nov 2 2015, 3:04 PM
sys/arm/arm/debug_monitor.c
306

Why 0x3?

462

Why not just make this, and all calls to dprintf, a db_printf?

597

Does this not need an isb?

873

Can we rely on the isb below for this?

sys/arm/include/db_machdep.h
53–59
#include <machine/acle-compat.h>
...
#if __ARM_ARCH >= 6
sys/arm/include/kdb.h
39
#include <machine/acle-compat.h>
...
#if __ARM_ARCH >= 6
sys/conf/files.arm
30

Does this build on arm < v6? It seems to use registers not present there.

zbb added inline comments.Nov 2 2015, 7:24 PM
sys/arm/arm/debug_monitor.c
873

We need to be sure that OS lock is removed before accessing register below.

sys/conf/files.arm
30

Yes, it should be armv6 dependent

zbb updated this revision to Diff 9890.Nov 2 2015, 7:25 PM
zbb edited edge metadata.
zbb removed rS FreeBSD src repository - subversion as the repository for this revision.
zbb added a parent revision: D4070: Add DDB to opt_ddb.h.Nov 2 2015, 7:29 PM
zbb added a reviewer: wma_semihalf.com.
andrew added inline comments.Nov 3 2015, 9:40 AM
sys/arm/arm/db_trace.c
138

It doesn't seem this is defined on arm < v6.

zbb updated this revision to Diff 9904.Nov 3 2015, 12:28 PM
zbb marked 10 inline comments as done.Nov 3 2015, 5:13 PM

Thanks. Any more comments?

zbb added a comment.Nov 5 2015, 2:50 PM

Ping?

zbb added a reviewer: skra.Nov 10 2015, 2:04 PM
zbb added a comment.Nov 10 2015, 5:34 PM

So... do you accept this revision?

mmel added a subscriber: mmel.Nov 11 2015, 4:14 PM

Are you sure that CP14 is accessible on all v6+ cores (including qcom, marvell ones) ? Under all circumstances (e.g. in secure / not-secure world) ? If yes, i'm fine with this patch.

andrew added inline comments.Nov 12 2015, 1:09 PM
sys/arm/arm/debug_monitor.c
120–135

These and below should be #define<tab>DBG_FOO.

491

Why ~0U? This returns a signed integer.

496

i is a u_int, but this functions return an int.

537–540

return ((cp14_dbgdscrint_get() & DBGSCR_MDBG_EN) != 0);

zbb marked 4 inline comments as done.Nov 30 2015, 1:27 PM
zbb updated this revision to Diff 10597.Nov 30 2015, 1:27 PM
kib added a subscriber: kib.Nov 30 2015, 2:58 PM

You might find the r250851 interesting, it fixed hw watchpoints for the SMP amd64 ddb.

zbb added a child revision: D4338: SMP support for ARMv6/v7 HW watchpoints.Dec 1 2015, 7:26 PM
In D4037#90965, @kib wrote:

You might find the r250851 interesting, it fixed hw watchpoints for the SMP amd64 ddb.

Thanks. I will add it as a separate commit. Please check out this one: https://reviews.freebsd.org/D4338

Closed by commit rS294740: Introduce support for HW watchpoints and single stepping for ARMv6/v7 (authored by zbb). · Explain WhyJan 25 2016, 6:02 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
sys/
arm/
arm/
10 lines
946 lines
3 lines
2 lines
include/
12 lines
13 lines
80 lines
7 lines
18 lines
conf/
1 line

Diff 9904

View Options

sys/arm/arm/db_trace.c

Loading...
View Options

sys/arm/arm/debug_monitor.c

Loading...
View Options

sys/arm/arm/machdep.c

Loading...
View Options

sys/arm/arm/trap-v6.c

Loading...
View Options

sys/arm/include/cpu-v6.h

Loading...
View Options

sys/arm/include/db_machdep.h

Loading...
View Options

sys/arm/include/debug_monitor.h

Loading...
View Options

sys/arm/include/kdb.h

Loading...
View Options

sys/arm/include/sysreg.h

Loading...
View Options

sys/conf/files.arm

Loading...