Page MenuHomeFreeBSD

Add pmap_change_prot on arm64
ClosedPublic

Authored by andrew on Sep 20 2021, 5:37 PM.
Tags
None
Referenced Files
F106099610: D32026.diff
Wed, Dec 25, 9:42 AM
Unknown Object (File)
Fri, Dec 13, 1:16 PM
Unknown Object (File)
Wed, Dec 4, 7:20 AM
Unknown Object (File)
Tue, Dec 3, 2:03 AM
Unknown Object (File)
Sun, Dec 1, 7:00 AM
Unknown Object (File)
Nov 23 2024, 10:21 PM
Unknown Object (File)
Nov 23 2024, 6:42 AM
Unknown Object (File)
Nov 21 2024, 8:55 AM

Details

Summary

Support changing the protection of preloaded kernel modules by
implementing pmap_change_prot on arm64 and calling it from
preload_protect.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 41890
Build 38778: arc lint + arc unit

Event Timeline

sys/arm64/arm64/pmap.c
6257

Enabling this will limit the ability to set breakpoints or fbt probes on preloaded modules. On amd64 there is a global write-protect bit that is disabled before these operations, but on arm64 we will need something more fine-grained. Do you have plans to handle this?

Enabling this will limit the ability to set breakpoints or fbt probes on preloaded modules. On amd64 there is a global write-protect bit that is disabled before these operations, but on arm64 we will need something more fine-grained. Do you have plans to handle this?

I've created D32053 to fix ddb and will look at dtrace later in the week.

Also update the DMAP when changing non-DMAP memory

alc added inline comments.
sys/arm64/arm64/pmap.c
6067

A related observation but not strictly speaking an issue with this change: Consider a case like a large loadable kernel module, e.g., ZFS, where the code could be mapped as 2MB pages. Here, we are unconditionally demoting the mapping even if the protection change could be applied to the 2MB mapping.

This revision is now accepted and ready to land.Sep 23 2021, 5:22 PM
sys/arm64/arm64/pmap.c
6015

I would make this an XXX comment. Presumably we should have ddb and dtrace temporarily modify the mapping in order for them to do their work?

How do we know that ddb and dtrace always write through the direct map?

6090

Shouldn't we try to update the direct map all at once, like amd64's version does? Otherwise, even if this function avoids demoting large mappings as Alan suggests above, we will end up demoting anyway since the direct map alias is updated a page at a time.

6261

Isn't kernel_vm_end the appropriate upper bound there?

BTW, on amd64 we avoid this by populating the radix tree with bootstrap PTPs in pmap_init().

mmel added inline comments.
sys/arm64/arm64/pmap.c
6089

I think it violates ARM by (even if only short-term) mapping the same physical page multiple times with different cache attributes. This is explicitly marked as undefined behavior in ARM. I'm afraid that the only conforming approach is the break-before-make approach - so first break all mappings and then recreate them with new attributes.

sys/arm64/arm64/pmap.c
6015

I've committed D32053 that uses the DMAP region to get a RW address. I'm planning on having ddb and dtrace temporally upgrade the DMAP memory to RW if needed.

6067

I already had a work in progress patch for this, although it still needs some polish.

6090

It shouldn't be too bad for the DMAP as we always map it RW so wouldn't be split when changing permissions. I can have a look at adding a variant of pmap_update_entry that takes multiple entries. We could then use it to change the permissions/attributes in a single operation & only demote when needed.

Update a comment and KASSERT

This revision now requires review to proceed.Oct 1 2021, 5:17 PM
This revision was not accepted when it landed; it landed in state Needs Review.Oct 11 2021, 10:44 AM
This revision was automatically updated to reflect the committed changes.