Page MenuHomeFreeBSD

kvm: fix types for `kvm_walk_pages`
ClosedPublic

Authored by emaste on Oct 8 2019, 6:29 PM.

Details

Summary

As with other libkvm interfaces use maximum-sized types to support cross-debugging (e.g. a 64-bit vmcore on a 32-bit host). See https://lists.freebsd.org/pipermail/svn-src-all/2019-February/176051.html for further discussion.

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

emaste created this revision.Oct 8 2019, 6:29 PM
emaste added a reviewer: brooks.
emaste added inline comments.
sys/sys/types.h
260–261 ↗(On Diff #63048)

there was some suggestion of introducing kvm_ types -- kvm_paddr_t, kvm_vaddr_t?

brooks added inline comments.Oct 8 2019, 9:26 PM
sys/sys/types.h
260–261 ↗(On Diff #63048)

It probably makes sense to have a kpaddr_t in general. I'm not sure how much value lvm_* variants add in practice.

emaste added a reviewer: will.Oct 9 2019, 12:59 PM
emaste added a comment.Oct 9 2019, 1:05 PM

Note that the types are the same size on LP64 archs. This breaks the ABI on ILP32.

emaste edited the summary of this revision. (Show Details)Oct 9 2019, 1:13 PM
jhb added inline comments.Oct 9 2019, 9:32 PM
sys/sys/types.h
255 ↗(On Diff #63048)

This comment needs updating, perhaps:

Types suitable for exporting physical addresses, virtual addresses (pointers), and
memory object sizes from the kernel independent of native word size.  These
should be used in place of vm_paddr_t, (u)intptr_t, and size_t in structs which
contain such types that are shared with userspace.
emaste updated this revision to Diff 63105.Oct 9 2019, 11:40 PM

chase member rename

emaste updated this revision to Diff 63106.Oct 9 2019, 11:42 PM

update comment per jhb

emaste updated this revision to Diff 63107.Oct 9 2019, 11:44 PM

update comment again

will accepted this revision.Oct 11 2019, 5:21 PM

I'm fine with these changes, even though they break the API. For my purposes, it isn't a big deal, especially with the version bump.

I don't think I can say much about the appropriateness of the types, but it makes sense to me to have them.

This revision is now accepted and ready to land.Oct 11 2019, 5:21 PM
This revision was automatically updated to reflect the committed changes.