Page MenuHomeFreeBSD

riscv: rework page table bootstrap
ClosedPublic

Authored by mhorne on May 23 2024, 7:53 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jul 6, 8:37 PM
Unknown Object (File)
Sat, Jul 6, 8:37 PM
Unknown Object (File)
Sat, Jul 6, 8:37 PM
Unknown Object (File)
Sat, Jul 6, 1:27 PM
Unknown Object (File)
Mon, Jul 1, 5:27 PM
Unknown Object (File)
Mon, Jul 1, 1:16 AM
Unknown Object (File)
Sat, Jun 29, 12:39 AM
Unknown Object (File)
Fri, Jun 28, 9:48 PM
Subscribers

Details

Summary

The overall goal of the change is to reduce the amount of work done in
locore assembly, and defer as much as possible until pmap_bootstrap().
Currently, half the setup is done in assembly, and then we pass the l1pt
address to pmap_bootstrap() where it is amended with other mappings.

Inspiration and understanding has been taken from amd64's
create_pagetables() routine, and I try to present the page table
construction in the same way: a linear procedure with commentary
explaining what we are doing and why. Thus the core of the new
implementation is contained in pmap_create_pagetables().

Once pmap_create_pagetables() has finished, we switch to the new
pagetable root and leave the bootstrap ones created by locore behind,
resulting in a minimal 8kB of wasted space.

Having the whole procedure in one place, in C code, allows it to be more
easily understood, while also making it more amenable to future changes
which may depend on CPU feature/errata detection.

Note that with this change the size of the early devmap is bumped up
from one to four L2 pages (8MB).

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/riscv/riscv/locore.S
122–125

Is it possible for the KERNBASE and direct mapping to overlap? Is that handled at all? I'd guess it's unlikely to arise in practice, but I'm not sure what the physical address space "typically" looks like on riscv hardware.

151

A bit tedious, but it'd be nicer to have these comments in D45324.

sys/riscv/riscv/pmap.c
721

Same question about the calculation here.

723

Dividing the size of the kernel by Ln_ENTRIES looks odd. Should this be howmany(howmany(kernlen, L2_SIZE), Ln_ENTRIES)?

I'd also suggest noting that this calculation assumes that kernstart is aligned.

726

Just to clarify my understanding, this is used for things like witness, dmesg buffer, ...? Or is it slop for the page table pages allocated here?

769

Maybe "invalid" rather than "uninitialized"?

822
827–829
sys/riscv/riscv/locore.S
122–125

I guess if the physical memory holding the kernel falls within the address range of [0xffffffc000000000,0xffffffc040000000] we might have a problem.

I am not aware of any limitation on what address ranges physical memory might occupy for RISC-V, but we tend to see it in the lower parts of the address space, i.e. beginning below the 4GB boundary and extending beyond it. It would be really strange to see something like the above so I am not worried about it at present...

sys/riscv/riscv/pmap.c
726

The most immediate need is for the dpcpu and msgbufp allocations performed after this function returns. It is not meant as slop for page table pages; we do not bother mapping page tables themselves (recursive map on amd64), as this memory is always accessed through the DMAP on this platform.

Per the comment in amd64's nkpt_init(), there may be other allocations before vm_mem_init(), which these bootstrap L3 pages will provide for.

I'm noticing now that some of this is wasted by calling pmap_bootstrap_dmap() after all this. I will fix this in the next update, as well as improving the commentary a bit.

mhorne marked 7 inline comments as done.

Handle feedback from markj.

sys/riscv/riscv/pmap.c
251

Can we just make this an out-parameter of pmap_create_pagetables()?

367

In actual usage, this argument is used to set other PTE bits, e.g., PTE_V.

745

Should this be asserted?

Handle comment from markj.

mhorne added inline comments.
sys/riscv/riscv/pmap.c
367

Thanks, clarified.

BTW I am not totally tied to adding these macros, or expanding their usage in the file, but they definitely made the code easier to read while I was writing it.

745

No, because we will not arrive to the assertion if it was not the case.

This revision is now accepted and ready to land.Thu, Jun 20, 3:52 PM
This revision was automatically updated to reflect the committed changes.
mhorne marked 2 inline comments as done.