Page MenuHomeFreeBSD

riscv: eliminate physmap global
ClosedPublic

Authored by mhorne on Apr 4 2022, 5:51 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 17 2024, 2:51 PM
Unknown Object (File)
Dec 22 2023, 11:23 PM
Unknown Object (File)
Dec 1 2023, 9:45 AM
Unknown Object (File)
Nov 6 2023, 6:20 AM
Unknown Object (File)
Oct 5 2023, 5:06 AM
Unknown Object (File)
Sep 29 2023, 4:15 AM
Unknown Object (File)
Aug 14 2023, 1:30 PM
Unknown Object (File)
Aug 3 2023, 10:49 PM
Subscribers

Details

Summary

Since physical memory management is now handled by subr_physmem.c, the
need to keep this global array has diminished. It is not referenced
outside of early boot-time, and is populated by physmem_avail() in
pmap_bootstrap(). Just allocate the array on the stack for the duration
of its lifetime.

The check against physmap[0] in initriscv() can be dropped altogether,
as there is no consequence for excluding a memory range twice.

Diff Detail

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

Event Timeline

mhorne requested review of this revision.Apr 4 2022, 5:51 PM
sys/riscv/riscv/pmap.c
654–665

It seems like we could eliminate this block, but I need to check the guarantees of physmem_avail(). But I believe it should become:

min_pa = physmap[0];
max_pa = physmap[physmap_idx];

where we don't divide physmap_idx by two.

sys/riscv/riscv/machdep.c
561

Last time I looked at this code, I wondered why we do this after pmap_bootstrap(). Isn't it possible that pmap_bootstrap_dmap() will use pages below 2MB for the direct map?

sys/riscv/riscv/machdep.c
561

This is a good question, and I'm not sure now if there was a reason.

At the moment pmap_bootstrap_dmap() is quite simple and will construct the dmap using 1G superpages only, not really respecting any excluded ranges. So it will always map pages below 2MB, but never use them. So for now, I think it is incorrect but inconsequential that this is called after pmap_bootstrap().

I have some half-baked patches to fix this.

561

And by 'fix this', I mean make the dmap construction smarter.

markj added inline comments.
sys/riscv/riscv/machdep.c
561

Oh, my mistake is that I read pmap_bootstrap() as allocating page table pages themselves starting from min_pa. I think it's probably okay to map the bottom 2MB. (Though, I believe we effectively ignore the "no-map" attribute in device trees, which as I understand it means "don't map this physical memory region at all.")

sys/riscv/riscv/pmap.c
654–665

I think you're right, but we should probably map each region separately rather than assuming that the physical address space doesn't contain large holes. That is, the existing code is a bit simplistic. Maybe that's what you mean by "make dmap construction smarter"?

(And if we don't expect the PA space to ever contain large holes, riscv's vmparam.h should define VM_PHYSSEG_DENSE rather than VM_PHYSSEG_SPARSE. The former enables a cheaper implementation of PHYS_TO_VM_PAGE(). But I doubt we can make this assertion about all riscv platforms.)

This revision is now accepted and ready to land.Apr 6 2022, 5:42 PM
sys/riscv/riscv/pmap.c
654–665

We definitely expect a segmented physical address space, I have seen it already with the beagle-v.

And yes, most likely we will end up iterating over/passing the physmap array directly to pmap_bootstrap_dmap().

This revision was automatically updated to reflect the committed changes.