Page MenuHomeFreeBSD

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

Authored by zbb on Oct 30 2015, 4:17 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 20, 4:44 AM
Unknown Object (File)
Tue, Feb 27, 3:51 PM
Unknown Object (File)
Feb 23 2024, 2:23 AM
Unknown Object (File)
Feb 20 2024, 4:45 PM
Unknown Object (File)
Jan 29 2024, 9:38 PM
Unknown Object (File)
Nov 10 2023, 8:10 PM
Unknown Object (File)
Nov 9 2023, 9:11 PM
Unknown Object (File)
Nov 8 2023, 8:01 PM
Subscribers

Details

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 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.
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
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.

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.

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 edited edge metadata.
zbb removed rS FreeBSD src repository - subversion as the repository for this revision.
sys/arm/arm/db_trace.c
138

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

zbb marked 10 inline comments as done.Nov 3 2015, 5:13 PM

Thanks. Any more comments?

So... do you accept this revision?

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.

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

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

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

This revision was automatically updated to reflect the committed changes.