Introduce kdb-level watchpoint functions
ClosedPublic
Actions

Authored by mhorne on Mar 9 2021, 6:36 PM.

Details

Reviewers

jhb
markj
kib
manu

Group Reviewers

ARM
arm64

Commits

rG5a2933d0bf9f: arm: implement kdb watchpoint functions
rG3ef68bc62c1e: arm64: implement kdb watchpoint functions
rG15dc1d44528a: x86: implement kdb watchpoint functions
rG763107f26c3c: Introduce kdb-level watchpoint functions

Summary

This basically mirrors what already exists in ddb, but provides a
slightly improved interface. It allows the caller to specify the
watchpoint access type, and returns more specific error codes to
differentiate failure cases.

These functions will be used to support hardware watchpoint use in
gdb(4). It is about the same amount of work to add per-arch calls there
than it is to lift this interface to KDB, so I opted for the latter.

Stubs are provided for architectures lacking hardware watchpoint logic
(mips, powerpc, riscv). Other architectures are handled in individual
commits, but presented here for easier review.

Removal of the ddb routines will be in a follow-up commit.

Summary of error codes:

ENXIO, not supported by platform
EBUSY, no free hardware watchpoints
EINVAL, invalid address, size, or access type

Test Plan

Tinderbox'd. Runtime testing done with later commits in the series.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

mhorne created this revision.Mar 9 2021, 6:36 PM

mhorne held this revision as a draft.

Herald added a reviewer: manu. · View Herald TranscriptMar 9 2021, 6:36 PM

Herald added subscribers: jhibbits, rpokala, emaste and 2 others. · View Herald Transcript

Harbormaster completed remote builds in B37702: Diff 85410.Mar 9 2021, 6:36 PM

mhorne added inline comments.Mar 9 2021, 6:51 PM

sys/sys/kdb.h
131	This particular value is unused, but maps to analogous execute bits in the supported debug register implementations, so I don't think it hurts to have.

mhorne published this revision for review.Mar 9 2021, 6:53 PM

mhorne edited the summary of this revision. (Show Details)

mhorne added a child revision: D29156: ddb: replace watchpoint set/clear functions.Mar 9 2021, 7:12 PM

mhorne added a parent revision: D29153: x86: consolidate hw watchpoint logic into new file.

mhorne mentioned this in D29153: x86: consolidate hw watchpoint logic into new file.Mar 9 2021, 7:57 PM

mhorne added a child revision: D29173: gdb: allow setting/removing hardware watchpoints.Mar 9 2021, 9:14 PM

kib added inline comments.Mar 13 2021, 3:35 AM

sys/amd64/amd64/db_trace.c
392 ↗	(On Diff #85410)	Is this line too long?
sys/x86/x86/dbreg.c
248 ↗	(On Diff #85410)	This is strange. Should it be #if defined(DDB) \|\| defined(KDB) or do I miss something?

andrew added inline comments.Mar 16 2021, 1:47 PM

sys/x86/x86/dbreg.c
292 ↗	(On Diff #85410)	Why do we have both this `db_md_set_watchpoint`?

mhorne added inline comments.Mar 16 2021, 5:31 PM

sys/x86/x86/dbreg.c
248 ↗	(On Diff #85410)	The `#endifs` are in different places. Still, this construct only makes sense under the original assumption that `dbreg.c` would always be compiled in. I will update this review shortly.
292 ↗	(On Diff #85410)	The new function will replace `db_md_set_watchpoint` in the next commit, see D29156. Then, both ddb and gdb will use the new functions (D29173).