Page MenuHomeFreeBSD

kern: physmem: operate on whole page boundaries for *_avail
Needs ReviewPublic

Authored by kevans on Nov 6 2021, 7:27 PM.
Tags
None
Referenced Files
F102983491: D32873.diff
Tue, Nov 19, 11:38 AM
Unknown Object (File)
Mon, Nov 11, 8:22 AM
Unknown Object (File)
Fri, Nov 8, 7:15 PM
Unknown Object (File)
Thu, Oct 31, 3:46 AM
Unknown Object (File)
Thu, Oct 31, 3:34 AM
Unknown Object (File)
Sat, Oct 26, 6:45 AM
Unknown Object (File)
Sat, Oct 26, 6:44 AM
Unknown Object (File)
Sat, Oct 26, 6:43 AM

Details

Reviewers
kib
markj
manu
Summary

hwregion start/end should be rounded up and down as appropriate to
only account for pages that are wholly usable. Conversely, exregions
that cover partial pages should in-fact cover the whole page.

While here, choose amore widely available definition of MAX_PHYS_ADDR.
ARM defines it as just below kernbase, which isn't quite what we want,
but the value that we do want isn't available early enough to be usable.
Other architectures define it either as the previous definition did, or
they define it as the highest address that can be indexed.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 42805
Build 39693: arc lint + arc unit

Event Timeline

kevans requested review of this revision.Nov 6 2021, 7:27 PM
sys/kern/subr_physmem.c
65–66

I think this define should go somewhere in machine/param.h or machine/vmparam.h.

E.g. on i386 PAE or PAE_TABLES, it is 64bit max, same on powerpc with PAE-like configuration.

In fact, on both i386 and amd64 we have cpu_getmaxphyaddr() but I am not sure if this function is usable there.

68

vm_size_t is not enough for PAE-like configs

75

(Just to put the note somewhere, it is not specific to the review)
I do not think that you need size_t for *cnt, int is enough.

sys/kern/subr_physmem.c
177

Too much spaces before '='

sys/kern/subr_physmem.c
65–66

It looks like cpu_getmaxphyaddr() is usable after finishidentcpu(), which is before we need it at getmemsize() time in both

kevans marked 2 inline comments as done.

Review commentary - move max phys addr out of subr_physmem, enlarge sizes, space

sys/kern/subr_physmem.c
68

I fixed this elsewhere, as well, to vm_ooffset_t. physmem_exclude_region has been changed to a pair of uint64_t, though, to match physmem_hardware_region.

75

I'll follow-up on this separately, thanks for the reminder

sys/arm64/include/vmparam.h
155

I do not believe this is right. VM_MAX_ADDRESS seems to be defined as max virtual address. IMO it is simpler to explcitly write out .ffff..s.

sys/kern/subr_physmem.c
417

Is this explanation generally applicable?

I do not find it useful to trim end of memory on x86. Except you might want to truncate it to small superpage size (weird term, yes), although I believe hw is even more picky there.

sys/kern/subr_physmem.c
164–166

This doesn't look correct when both the start and size are both rounded.

e.g. With a page size of 4096, start of 1, and size of 8190 the rounded start will be 4096 and size trunc_page(4096 + 8190) = 8192 when it should be trunc_page(1 + 8190) = 4096

177

This also looks wrong in some sub page cases

sys/kern/subr_physmem.c
43

We already include this below.