Page MenuHomeFreeBSD

sys: Consolidate common implementation details of PV entries.
ClosedPublic

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

Details

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 requested review of this revision.Sep 23 2022, 10:13 PM

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

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.

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.
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.

Indeed. Are you going to ask him?