Page MenuHomeFreeBSD

Let arm64 pmap_qenter() and pmap_kenter() unconditionally set NX.
ClosedPublic

Authored by markj on Oct 24 2019, 3:25 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 23 2024, 12:31 PM
Unknown Object (File)
Dec 23 2023, 12:36 AM
Unknown Object (File)
Nov 15 2023, 9:15 AM
Unknown Object (File)
Nov 11 2023, 2:49 AM
Unknown Object (File)
Nov 9 2023, 5:12 PM
Unknown Object (File)
Nov 9 2023, 2:57 AM
Unknown Object (File)
Oct 14 2023, 8:17 AM
Unknown Object (File)
Oct 8 2023, 4:05 PM

Details

Test Plan

I booted a ThunderX with this diff applied.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.Oct 24 2019, 4:50 PM

Diff broken by rS354193 'Set the userspace execute never bit on kernel mappings' (looks like that sets UXN in the else case instead).

This revision now requires review to proceed.Nov 4 2019, 4:59 PM
This revision is now accepted and ready to land.Nov 4 2019, 5:18 PM
alc added inline comments.
sys/arm64/arm64/pmap.c
1260 ↗(On Diff #63930)

While you're here, I'd like to suggest that it would good from a stylistic standpoint to add ATTR_AP(ATTR_AP_RW).

sys/arm64/arm64/pmap.c
1378 ↗(On Diff #63930)

Just an FYI, and not a comment on this patch. I think that we should be implementing break-before-make here. Occasionally, we have used pmap_qenter() to replace a valid mapping. That is why I did not change this to the simpler pmap_store() when I introduced pmap_store().

markj marked an inline comment as done.

Explicitly specify ATTR_AP_RW for mappings created by pmap_kenter().

This revision now requires review to proceed.Nov 4 2019, 6:40 PM
This revision is now accepted and ready to land.Nov 4 2019, 6:46 PM
sys/arm64/arm64/pmap.c
1378 ↗(On Diff #63930)

I think this can happen with some implementations of sf_buf_map() (though not on arm64). Are there other examples?

sys/arm64/arm64/pmap.c
1378 ↗(On Diff #63930)

I won't swear to it, but I thought that I recently saw pmap_qenter() being used to remap a virtual page to a different physical page on a different NUMA node.

Can you commit this or shall I commit D22241 first? They conflict as they both change pmap_qenter() and pmap_kenter().

Can you commit this or shall I commit D22241 first? They conflict as they both change pmap_qenter() and pmap_kenter().

Please go ahead and commit your patch, I will come back to this next week.