pmap_get_tables() can be used to obtain the l1 through l3 page table entries for a given virtual address and mapping.
It is required for minidump support.
The function is already declared in riscv/include/pmap.h.
Details
Details
- Reviewers
markj jhb br - Commits
- rS344827: Add pmap_get_tables() for riscv.
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| sys/riscv/riscv/pmap.c | ||
|---|---|---|
| 3431 ↗ | (On Diff #51032) | Certainly we can for kernel_pmap. I can't speak confidently about other mappings but I doubt this function ever be used on those. I could turn it into an assert or just remove it. Any preference? |
| 3448 ↗ | (On Diff #51032) | I didn't find any references to L1 superpages, but seeing as they aren't disallowed I agree it's a good check to have. Side question: any idea why we use PTE_RX? The spec says PTE_RWX should all be zero for page tables but most of the checks omit the W bit. Is this intentional? |
| sys/riscv/riscv/pmap.c | ||
|---|---|---|
| 3431 ↗ | (On Diff #51032) | pmap_pinit() allocates the L1 page as part of pmap initialization. I think it might as well be removed. In general I find it a bit weird to assert that a pointer is non-NULL immediately before dereferencing it. |
| 3448 ↗ | (On Diff #51032) | They are permitted in SV39; see section 4.4.1 in the privileged architecture reference. I believe it's done that way because the spec currently disallows setting W without R (while it does allow X without R), so checking just R and X is sufficient. @br could confirm. |
Comment Actions
This looks ok to me. I'll go through the minidump review before committing it, though.
| sys/riscv/riscv/pmap.c | ||
|---|---|---|
| 3431 ↗ | (On Diff #51093) | Style: comments should end with a period. |
| sys/riscv/riscv/pmap.c | ||
|---|---|---|
| 3457 ↗ | (On Diff #51123) | Hmm, re-reading this, why do we not check for PTE_V in the L3 entry? |
| sys/riscv/riscv/pmap.c | ||
|---|---|---|
| 3457 ↗ | (On Diff #51123) | Never mind, I should have read the minidump code before asking. |