Page MenuHomeFreeBSD

x86: consolidate hw watchpoint logic into new file
ClosedPublic

Authored by mhorne on Mar 9 2021, 6:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 21, 1:09 PM
Unknown Object (File)
Sat, Mar 16, 10:57 PM
Unknown Object (File)
Sat, Mar 16, 10:57 PM
Unknown Object (File)
Sat, Mar 16, 10:57 PM
Unknown Object (File)
Sat, Mar 16, 10:57 PM
Unknown Object (File)
Sat, Mar 16, 10:57 PM
Unknown Object (File)
Sat, Mar 16, 10:57 PM
Unknown Object (File)
Thu, Mar 14, 6:38 AM

Details

Summary

This is a prerequisite to using these functions outside of ddb, but also
provides some cleanup and minor refactoring. This code is almost
entirely duplicated between the two architectures, the only significant
difference being the lack of dbreg synchronization on i386.

Cleanups are:

  • demote some internal functions to static
  • use the constant NDBREGS instead of a '4' literal
  • remove K&R definitions
  • some added comments
Test Plan

Compiled on amd64 and i386. Tested setting/triggering hardware watchpoints in ddb on amd64.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 37868
Build 34757: arc lint + arc unit

Event Timeline

mhorne requested review of this revision.Mar 9 2021, 6:15 PM
sys/i386/i386/db_trace.c
792–795

Note that this block is dropped, but can be added back if desired.

sys/conf/files.x86
323

Does the content of the file needed for !DDB?

sys/x86/x86/dbreg.c
30

Was this code copy/pasted with trivial modifications?
I really do not think that the new license is applicable to it.

sys/conf/files.x86
323

No, they aren't used elsewhere yet. By the end of my patch series it can become ddb | gdb.

sys/x86/x86/dbreg.c
30

I made some new additions to this file in D29155, but still something that can be considered trivial. I do not feel strongly about keeping the notice.

Remove copyright notice, make the file optional on DDB.

sys/x86/include/reg.h
270 ↗(On Diff #85423)

My last suggestion, is to move these declarations to e.g. x86/include/x86_var.h
reg.h is more a kind of ABI interface between kernel and userspace, and existing fill/set declarations are arguably a mistake (see also the comment above them).

Move function declarations to x86_var.h.

Also move amd64_db_resume_dbreg(), as it is in the scope of the new file.

kib added inline comments.
sys/x86/x86/dbreg.c
38

Isn't this #ifdef redundant? Even if the file is used for gdb && !ddb configs eventually.

This revision is now accepted and ready to land.Mar 12 2021, 11:51 AM

Remove redundant #ifdef DDB around include. Add #ifdef DDB around functions using db_printf.

Fix calculation of wsize in dbreg_set_watchpoint on i386.

This revision now requires review to proceed.Mar 16 2021, 5:12 PM
This revision is now accepted and ready to land.Mar 19 2021, 6:24 PM