Paths

Table of Contentst

amd64 pmap: More unification for psind = 1 vs 2 in pmap_enter_largepage().
ClosedPublic
Actions

Authored by kib on Sep 21 2020, 4:14 PM.

Details

Reviewers

alc
markj

Commits

rS366030: amd64 pmap: More unification for psind = 1 vs 2 in pmap_enter_largepage().

Summary

Move

pkru check
wait for page alloc
wire accounting update
asserting allowed updates for valid mappings

out of psind conditions.

Also add assert that psind references supported page size.

[The diff was generated before r365951]

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kib created this revision.Sep 21 2020, 4:14 PM

Herald added a subscriber: imp. · View Herald TranscriptSep 21 2020, 4:14 PM

kib requested review of this revision.Sep 21 2020, 4:14 PM

Harbormaster completed remote builds in B33695: Diff 77306.Sep 21 2020, 4:14 PM

alc added inline comments.Sep 21 2020, 6:38 PM

sys/amd64/amd64/pmap.c
6547 ↗	(On Diff #77306)	`PG_FRAME` -> `PG_PS_FRAME` (This is sufficient to exclude the PAT bit from the comparison. There is no reason to handle 1 GB pages specifically.)
6568–6569 ↗	(On Diff #77306)	I don't understand this sentence. We often modify the paging structure, which I interpret to mean page table, with only a read lock on the vm map.

kib marked 2 inline comments as done.Sep 21 2020, 8:04 PM

kib added inline comments.

sys/amd64/amd64/pmap.c
6568–6569 ↗	(On Diff #77306)	Yes, it is (no longer) valid. Initial version of the largepage change instantiated mappings on mmap(2). My reasoning was that it does not make much sense to wait for access to the mmaped region where the pages are essentially wired. After discussion with Mark I moved the instantiation to fault handler but the comment was not properly adjusted. I deleted it now.

Remove no longer valid comment.
Compare only phys addresses when asserting allowed change in large mapping.

Harbormaster completed remote builds in B33701: Diff 77319.Sep 21 2020, 8:14 PM

markj accepted this revision.Sep 22 2020, 12:16 PM

This revision is now accepted and ready to land.Sep 22 2020, 12:16 PM

alc added inline comments.Sep 22 2020, 6:23 PM

sys/amd64/amd64/pmap.c
6510 ↗	(On Diff #77319)	Why rewalk the page table from the root? Use mp in the same way as below (lines 6531-6532) to more cheaply calculate the pdpe.
6515 ↗	(On Diff #77319)	In this case, there is no need to rewalk the page table from the root, i.e., repeat the pmap_pml4e(). pmap_pml4e_to_pdpe() can be used.

Avoid page table rewalking, as suggested by alc.

This revision now requires review to proceed.Sep 22 2020, 6:58 PM

Harbormaster completed remote builds in B33735: Diff 77368.Sep 22 2020, 6:58 PM

Is it documented anywhere what can versus can't be done to a non-transparent mapping?

sys/amd64/amd64/pmap.c
6548 ↗	(On Diff #77368)	`pde` -> `origpte`

This revision is now accepted and ready to land.Sep 22 2020, 10:50 PM

In D26513#590433, @alc wrote:

Is it documented anywhere what can versus can't be done to a non-transparent mapping?

No, there is no such documentation. Effectively we only allow permissions changes and emulate fake wiring status.

Even userspace API docs are not yet written. Arguably it is still premature.

Closed by commit rS366030: amd64 pmap: More unification for psind = 1 vs 2 in pmap_enter_largepage(). (authored by kib). · Explain WhySep 22 2020, 11:28 PM

This revision was automatically updated to reflect the committed changes.

kib added a commit: rS366030: amd64 pmap: More unification for psind = 1 vs 2 in pmap_enter_largepage()..

Revision Contents
Changeset List

Path

Size

head/

sys/

amd64/

pmap.c

77 lines

Diff 77392

View Options

head/sys/amd64/amd64/pmap.c

amd64 pmap: More unification for psind = 1 vs 2 in pmap_enter_largepage().ClosedPublicActions