Page MenuHomeFreeBSD

kvm: fix types for `kvm_walk_pages`
AcceptedPublic

Authored by emaste on Tue, Oct 8, 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

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

emaste created this revision.Tue, Oct 8, 6:29 PM
emaste added a reviewer: brooks.
emaste added inline comments.
sys/sys/types.h
260–261

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

brooks added inline comments.Tue, Oct 8, 9:26 PM
sys/sys/types.h
260–261

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.Wed, Oct 9, 12:59 PM
emaste added a comment.Wed, Oct 9, 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)Wed, Oct 9, 1:13 PM
jhb added inline comments.Wed, Oct 9, 9:32 PM
sys/sys/types.h
254–255

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.Wed, Oct 9, 11:40 PM

chase member rename

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

update comment per jhb

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

update comment again

will accepted this revision.Fri, Oct 11, 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.Fri, Oct 11, 5:21 PM