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

[zbb]

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.

[imp]

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.

[andrew]

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

[zbb]

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]

sys/arm/arm/debug_monitor.c

305 ↗

(On Diff #9825)

Why 0x3?

461 ↗

(On Diff #9825)

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

596 ↗

(On Diff #9825)

Does this not need an isb?

872 ↗

(On Diff #9825)

Can we rely on the isb below for this?

sys/arm/include/db_machdep.h

53 ↗

(On Diff #9825)

#include <machine/acle-compat.h>
...
#if __ARM_ARCH >= 6

sys/arm/include/kdb.h

39 ↗

(On Diff #9825)

#include <machine/acle-compat.h>
...
#if __ARM_ARCH >= 6

sys/conf/files.arm

30 ↗

(On Diff #9825)

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

[zbb]

sys/arm/arm/debug_monitor.c

872 ↗

(On Diff #9825)

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

sys/conf/files.arm

30 ↗

(On Diff #9825)

Yes, it should be armv6 dependent

[andrew]

sys/arm/arm/db_trace.c

135 ↗

(On Diff #9890)

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

[zbb]

Thanks. Any more comments?

[zbb]

Ping?

[zbb]

So... do you accept this revision?

[mmel]

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]

sys/arm/arm/debug_monitor.c

120–135 ↗

(On Diff #9904)

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

491 ↗

(On Diff #9904)

Why ~0U? This returns a signed integer.

496 ↗

(On Diff #9904)

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

537–540 ↗

(On Diff #9904)

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

[kib]

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

[zbb]

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