Page MenuHomeFreeBSD

Introduce kdb-level watchpoint functions
ClosedPublic

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

Details

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
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sys/sys/kdb.h
130

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)
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?

sys/x86/x86/dbreg.c
292 ↗(On Diff #85410)

Why do we have both this db_md_set_watchpoint?

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

Rebase on top of changes to D29153. Remove some unneeded #ifdef lines.

jhb added inline comments.
sys/sys/kdb.h
130

I would probably just spell it '_X' rather than '_EXEC' FWIW.

This revision is now accepted and ready to land.Fri, Mar 19, 6:28 PM