Page MenuHomeFreeBSD

libkvm: add kvm_walk_pages API.
ClosedPublic

Authored by will on Sep 8 2017, 5:13 PM.

Details

Summary

This API allows callers to enumerate all known pages, including any
direct map & kernel map virtual addresses, physical addresses, size,
offset into the core, & protection configured.

For architectures that support direct map addresses, also generate pages
for any direct map only addresses that are not associated with kernel
map addresses.

Fix page size portability issue left behind from previous kvm page table
lookup interface.

Test Plan

Install libkvm, make sure kgdb et al still work (should be a no-op).

Diff Detail

Repository
rS 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

will created this revision.Sep 8 2017, 5:13 PM
will updated this revision to Diff 32820.Sep 8 2017, 7:49 PM
  • kvm: pass up better error messages.
will added a reviewer: jhb.Sep 8 2017, 7:50 PM
jhb added inline comments.Sep 29 2017, 9:07 PM
lib/libkvm/kvm.c
503 ↗(On Diff #32820)

Minor style nit: parens around return value.

lib/libkvm/kvm_aarch64.h
47 ↗(On Diff #32820)

Tab after #define instead of space

lib/libkvm/kvm_arm.h
56 ↗(On Diff #32820)

Another tab vs space.

lib/libkvm/kvm_minidump_amd64.c
69 ↗(On Diff #32820)

I found it somewhat confusing at first that there was both pde_get() and pte_get() for amd64, it might warrant a short block comment above these functions explaining that version 1 dumps use pte_get() and version 2 use pde_get()

342 ↗(On Diff #32820)

Since this always reads pdes, does it only work for version 2? If so, we should check the version?

will updated this revision to Diff 33638.Oct 2 2017, 5:26 PM
will marked 2 inline comments as done.
  • libkvm: fix minor nits.
will added inline comments.Oct 2 2017, 5:26 PM
lib/libkvm/kvm_minidump_amd64.c
342 ↗(On Diff #32820)

You're right.

Version 2 was added in 2010, so all versions after that point only generate v2 minidumps. Would it be acceptable to simply leave it out of this change?

jhb added inline comments.Oct 2 2017, 5:49 PM
lib/libkvm/kvm_minidump_amd64.c
342 ↗(On Diff #32820)

I think just adding a version check and failing for version 1 dumps is probably fine. I'd just rather have an explicit failure than undefined behavior.

will updated this revision to Diff 33640.Oct 2 2017, 5:51 PM
  • libkvm: require v2 minidumps for walk_pages on amd64.
will marked an inline comment as done.Oct 2 2017, 5:53 PM

On amd64, this now requires v2 minidumps (generated since 2010).

will added a comment.Oct 30 2017, 7:39 PM

@jhb Any further feedback? Does this look ready for HEAD?

jhb accepted this revision.Nov 6 2017, 8:48 PM

This should be fine. I think the diff in phab is just a diff relative to the previous rather than the full diff FWIW.

This revision is now accepted and ready to land.Nov 6 2017, 8:48 PM
This revision was automatically updated to reflect the committed changes.