Page MenuHomeFreeBSD

arm64: Check DMAP address is valid in PHYS_IN_DMAP
ClosedPublic

Authored by andrew on Apr 8 2024, 1:17 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, May 4, 6:11 PM
Unknown Object (File)
Fri, May 3, 7:15 AM
Unknown Object (File)
Fri, Apr 26, 4:53 AM
Unknown Object (File)
Wed, Apr 24, 8:16 PM
Unknown Object (File)
Apr 12 2024, 10:55 AM
Unknown Object (File)
Apr 11 2024, 5:31 PM
Unknown Object (File)
Apr 8 2024, 2:52 PM
Unknown Object (File)
Apr 8 2024, 1:26 PM

Details

Summary

When checking if a physical address is in the DMAP region we assume
all physical addresses between DMAP_MIN_PHYSADDR and DMAP_MAX_PHYSADDR
are able to be accesses through the DMAP. It may be the case that
there is device memory in this range that shouldn't be accessed through
the DMAP mappings.

Add a check to PHYS_IN_DMAP that the translated virtual address is a
valid kernel address. To support code that already checks the address
is valid add PHYS_IN_DMAP_FAST.

PR: 278233
Sponsored by: Arm Ltd

Diff Detail

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

Event Timeline

andrew requested review of this revision.Apr 8 2024, 1:17 PM

I believe pmap_map_io_transient() can use PHYS_IN_DMAP_FAST() as well.

IMHO, PHYS_IN_DMAP_RANGE() is a better name than PHYS_IN_DMAP_FAST().

@andrew Any thoughts on Mark's comments?

@alc Should we be waiting for you to look at this?

I'd like to make sure this gets into the tree and MFCed to stable/14 before the 14.1 release cycle gets going since it's necessary for supporting an upcoming EC2 instance type.

Rename to PHYS_IN_DMAP_RANGE

This revision is now accepted and ready to land.Tue, Apr 16, 1:11 PM

Why not deal with this issue in pmap_mapdev{,_attr}()? Specifically, if the given physical address falls within the DMAP region, don't call kva_alloc{,_aligned}(); instead, map the physical address at its corresponding DMAP virtual address. This is not all that different from amd64, where the DMAP (with appropriate attr settings) is used to access a good bit of device memory.

In D44677#1021479, @alc wrote:

Why not deal with this issue in pmap_mapdev{,_attr}()? Specifically, if the given physical address falls within the DMAP region, don't call kva_alloc{,_aligned}(); instead, map the physical address at its corresponding DMAP virtual address. This is not all that different from amd64, where the DMAP (with appropriate attr settings) is used to access a good bit of device memory.

Elaborating a bit, pmap_mapdev{,_attr}() would not be able to simply call pmap_kenter{,_device}() to create the mappings because L3, L2, etc. page table pages won't have been setup. It would need to call a function that is virtually identical to the for loop body in pmap_bootstrap_dmap().

We could look at that as a follow up, however I'm unlikely to have time to make such a change and have it ready and well tested for 14.1 given it's due to be branched in just over 2 weeks.

Any chance this can be landed in the next ~18 hours, aka before the weekly snapshot builds start at midnight UTC?

We could look at that as a follow up, however I'm unlikely to have time to make such a change and have it ready and well tested for 14.1 given it's due to be branched in just over 2 weeks.

I have no objections to this plan. I can probably write the new patch about 10-14 days from now.