Page MenuHomeFreeBSD

Add pmap_change_prot on arm64
ClosedPublic

Authored by andrew on Sep 20 2021, 5:37 PM.

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
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sys/arm64/arm64/pmap.c
6256

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.Thu, Sep 23, 5:22 PM
sys/arm64/arm64/pmap.c
6014

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?

6089

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.

6260

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
6088

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
6014

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.

6089

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.Fri, Oct 1, 5:17 PM
This revision was not accepted when it landed; it landed in state Needs Review.Mon, Oct 11, 10:44 AM
This revision was automatically updated to reflect the committed changes.