To simplify the creation of the direct map (DMAP) region on arm64 move
it from the pre-C code into pmap. This simplifies the DMAP creation
as we can use the notmal index macros, and should reduce the number
of pages needed to hold the level 1 tables to just those needed.
Details
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 44825 Build 41713: arc lint + arc unit
Event Timeline
- Check if we need a new L0 -> L1 map when va & pa change
- Use Ln_ENTRIES as the invalid slot number
I can't see any bugs in the patch but now pmap_bootstrap_dmap() looks overly hairy to me. Can we at least make pmap_bootstrap_dmap_l2() responsible for creating L1 table entries, similar to how pmap_bootstrap_dmap_l1() is responsible for creating L0 table entries?
| sys/arm64/arm64/pmap.c | ||
|---|---|---|
| 810 | va doesn't need to be a pointer. | |
| 810 | pmap_bootstrap_dmap_l1() updates freemempos directly. It would be nice to be consistent. | |
| sys/arm64/arm64/pmap.c | ||
|---|---|---|
| 830 | Rather than this bit-masking operation, I would argue for asserting that l1 (or *freemempos) is page aligned. If l1 isn't page aligned, then the problems will go beyond a bogus l0 table entry. | |
| 837 | Can't this assignment be inside the above if statement? | |
| 948–970 | Can't this assignment be inside the above if statement? | |
- Use a state struct rather than pass individual pointers
- Have pmap_bootstrap_dmap_l2 call pmap_bootstrap_dmap_l1
Remove physical address masks from the page table creation and add asserts for alignment
| sys/arm64/arm64/pmap.c | ||
|---|---|---|
| 840 | I would suggest doing the zeroing before hooking the page into the page table. Otherwise, when I see things done in this order, I pause and ponder if there might be a possible problem/race. | |
| 877–878 | Please swap the order of these two lines for consistency with pmap_bootstrap_dmap_l1(). | |
| 880–884 | I would suggest doing the zeroing before hooking the page into the page table. | |
- Reorder to zero memory before attaching it to the table
- Rename functions to make them consistant with what they're creating
- Reorder code to be consistant between similar functions
- Create level 3 pages so we only create the DMAP mappings needed
This seems to break booting at least one of our Altra systems (never get back to a console after loader). I can throw a debugger on it if there's anything particularly of interest for me to probe at.
| sys/arm64/arm64/pmap.c | ||
|---|---|---|
| 928 | We seem to be hitting this assertion: Thread 1 hit Breakpoint 2, pmap_bootstrap_dmap_l2_block (state=state@entry=0xffff000000fa6740, i=i@entry=2) at /usr/home/kyle.evans/freebsd/sys/arm64/arm64/pmap.c:928
928 MPASS((state->va & L2_OFFSET) == 0);
(gdb) print *state
$1 = {va = 18446638546234179584, pa = 26714636288, l1 = 0xffff000005a24000, l2 = 0xffff000005a25000, l3 = 0xffff000005a26000,
l0_slot = 320, l1_slot = 23, l2_slot = 450, freemempos = 18446462598827372544}
(gdb) bt
#0 pmap_bootstrap_dmap_l2_block (state=state@entry=0xffff000000fa6740, i=i@entry=2)
at /usr/home/kyle.evans/freebsd/sys/arm64/arm64/pmap.c:928
#1 0xffff000000776210 in pmap_bootstrap_dmap (freemempos=<optimized out>, kern_l1=<optimized out>, min_pa=<optimized out>)
at /usr/home/kyle.evans/freebsd/sys/arm64/arm64/pmap.c:1010
#2 pmap_bootstrap (l0pt=<optimized out>, l1pt=<optimized out>, kernstart=<optimized out>, kernlen=<optimized out>)
at /usr/home/kyle.evans/freebsd/sys/arm64/arm64/pmap.c:1158
#3 0xffff000000771c08 in initarm (abp=0xffff000000fa6a60) at /usr/home/kyle.evans/freebsd/sys/arm64/arm64/machdep.c:817
#4 0xffff0000000008b4 in _start () at /usr/home/kyle.evans/freebsd/sys/arm64/arm64/locore.S:152
(gdb) print physmap
$2 = {1073741824, 26714046464, 26714505216, 26714636288, 26715291648, 26716274688, 26716995584, 26772373504, 26774011904,
26843545600, 0 <repeats 116 times>} | |
| sys/arm64/arm64/pmap.c | ||
|---|---|---|
| 928 | Ah, a bunch of intermediates were optimized out. Here's this for completeness: (gdb) print *abp
$5 = {modulep = 18446462598827315200, kern_l1pt = 18446462598749208576, kern_delta = 18446462572182896640,
kern_stack = 18446462598749237248, kern_l0pt = 18446462598749212672, kern_ttbr0 = 26566332416, boot_el = 1, pad = 0} | |
| sys/arm64/arm64/pmap.c | ||
|---|---|---|
| 1009 | Kyle, try changing this line as above. | |
| sys/arm64/arm64/pmap.c | ||
|---|---|---|
| 1009 | Perfect- we're back to multi-user with this change. Thanks! | |
Attempt to fix when regions are smaller than a level 2 bloc ksize
- Move the size checks to _l2_block and _l3_page functions
- Add asserts for pa <= physmap[i + 1]