sys: Consolidate common implementation details of PV entries.
ClosedPublic
Actions

Authored by jhb on Sep 23 2022, 10:13 PM.

Details

Reviewers

kib
markj
manu
andrew

Commits

rG4d90a5afc51b: sys: Consolidate common implementation details of PV entries.

Summary

Add a <sys/_pv_entry.h> intended for use in <machine/pmap.h> to
define struct pv_entry, pv_chunk, and related macros and inline
functions.

Note that powerpc does not yet use this as while the mmu_radix pmap
in powerpc uses the new scheme (albeit with fewer PV entries in a
chunk than normal due to an used pv_pmap field in struct pv_entry),
the Book-E pmaps for powerpc use the older style PV entries without
chunks (and thus require the pv_pmap field).

Suggested by: kib
Sponsored by: DARPA

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 47721
Build 44608: arc lint + arc unit

Event Timeline

jhb created this revision.Sep 23 2022, 10:13 PM

Herald added a reviewer: manu. · View Herald TranscriptSep 23 2022, 10:13 PM

Herald added subscribers: emaste, andrew, imp. · View Herald Transcript

jhb requested review of this revision.Sep 23 2022, 10:13 PM

Harbormaster completed remote builds in B47510: Diff 110897.Sep 23 2022, 10:13 PM

I've booted this on amd64 and i386 (bhyve VMs), armv7 and arm64 (RPi), and RISC-V (QEMU).

jhb added inline comments.Sep 23 2022, 10:22 PM

sys/amd64/amd64/pmap.c
5095–5098	Possibly these should all follow the pattern I use for arm64 in D36502
sys/riscv/riscv/pmap.c
1870–1872	This pattern is also duplicated in the various pmap's and could perhaps be subsumed into a new inline function in the new header (e.g. pc_init() or the like). Possibly though there is some larger pieces of duplicated code among these pmap's that could still be duplicated.
sys/sys/_pv_entry.h
129	I should probably axe the blank line here since this is new code.

kib added inline comments.Sep 24 2022, 11:51 PM

sys/sys/_pv_entry.h
76	Perhaps add #elif #error unsupported page size #endif ?
79	64/32 can be replaced by `__CHAR_BIT__ & __SIZEOF_LONG`. May be, even 0xffff..ffful can be replaced by (-1ul)
82	Probably it is better to change #else to `#elif __SIZEOF_LONG == 4`, and add an error, because __SIZEOF_LONG is gcc/clang extension. Unless the CHAR_BIT works good enough.
111	Could you, please, add {} around loop body?

jhb marked 2 inline comments as done.Oct 5 2022, 11:05 PM

jhb added inline comments.

sys/sys/_pv_entry.h
76	Well, there are other unsupported combinations (e.g. 16k pages on ILP32). I could maybe add a single '#ifndef _NPCPV' that complains about an unsupported page size or ABI or something like that.
79	Both clang and GCC define a `__LONG_WIDTH__` that will work.

Various review feedback.
Copy the pattern for initializing pc_freemask from arm64.

Harbormaster completed remote builds in B47721: Diff 111528.Oct 6 2022, 11:14 PM

kib accepted this revision.Oct 7 2022, 2:14 AM

This revision is now accepted and ready to land.Oct 7 2022, 2:14 AM

Everything in _pv_entry.h except for the notion of there being a struct pv_entry with at least a virtual address and TAILQ is original to peter@. I don't see a reason for a Regents copyright.

In D36685#838118, @alc wrote:

Everything in _pv_entry.h except for the notion of there being a struct pv_entry with at least a virtual address and TAILQ is original to peter@. I don't see a reason for a Regents copyright.

Mmmm, ok. I can drop the UCB copyright, but @peter would have to be the one to change it to a 2 clause license.

In D36685#838132, @jhb wrote:

In D36685#838118, @alc wrote:

Everything in _pv_entry.h except for the notion of there being a struct pv_entry with at least a virtual address and TAILQ is original to peter@. I don't see a reason for a Regents copyright.

Mmmm, ok. I can drop the UCB copyright, but @peter would have to be the one to change it to a 2 clause license.