Skip to content

Commit b7737c3

Browse files
kevin-brodsky-armctmarinas
authored andcommitted
arm64: mm: Simplify check in arch_kfence_init_pool()
TL;DR: checking force_pte_mapping() in arch_kfence_init_pool() is sufficient Commit ce2b3a5 ("arm64: mm: Don't sleep in split_kernel_leaf_mapping() when in atomic context") recently added an arm64 implementation of arch_kfence_init_pool() to ensure that the KFENCE pool is PTE-mapped. Assuming that the pool was not initialised early, block splitting is necessary if the linear mapping is not fully PTE-mapped, in other words if force_pte_mapping() is false. arch_kfence_init_pool() currently makes another check: whether BBML2-noabort is supported, i.e. whether we are *able* to split block mappings. This check is however unnecessary, because force_pte_mapping() is always true if KFENCE is enabled and BBML2-noabort is not supported. This must be the case by design, since KFENCE requires PTE-mapped pages in all cases. We can therefore remove that check. The situation is different in split_kernel_leaf_mapping(), as that function is called unconditionally regardless of the configuration. If BBML2-noabort is not supported, it cannot do anything and bails out. If force_pte_mapping() is true, there is nothing to do and it also bails out, but these are independent checks. Commit 53357f1 ("arm64: mm: Tidy up force_pte_mapping()") grouped these checks into a helper, split_leaf_mapping_possible(). This isn't so helpful as only split_kernel_leaf_mapping() should check both. Revert the parts of that commit that introduced the helper, reintroducing the more accurate comments in split_kernel_leaf_mapping(). Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com> Reviewed-by: Ryan Roberts <ryan.roberts@arm.com> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
1 parent 8f0b4cc commit b7737c3

1 file changed

Lines changed: 16 additions & 17 deletions

File tree

arch/arm64/mm/mmu.c

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -767,30 +767,29 @@ static inline bool force_pte_mapping(void)
767767
return rodata_full || arm64_kfence_can_set_direct_map() || is_realm_world();
768768
}
769769

770-
static inline bool split_leaf_mapping_possible(void)
771-
{
772-
/*
773-
* !BBML2_NOABORT systems should never run into scenarios where we would
774-
* have to split. So exit early and let calling code detect it and raise
775-
* a warning.
776-
*/
777-
if (!system_supports_bbml2_noabort())
778-
return false;
779-
return !force_pte_mapping();
780-
}
781-
782770
static DEFINE_MUTEX(pgtable_split_lock);
783771

784772
int split_kernel_leaf_mapping(unsigned long start, unsigned long end)
785773
{
786774
int ret;
787775

788776
/*
789-
* Exit early if the region is within a pte-mapped area or if we can't
790-
* split. For the latter case, the permission change code will raise a
791-
* warning if not already pte-mapped.
777+
* !BBML2_NOABORT systems should not be trying to change permissions on
778+
* anything that is not pte-mapped in the first place. Just return early
779+
* and let the permission change code raise a warning if not already
780+
* pte-mapped.
792781
*/
793-
if (!split_leaf_mapping_possible() || is_kfence_address((void *)start))
782+
if (!system_supports_bbml2_noabort())
783+
return 0;
784+
785+
/*
786+
* If the region is within a pte-mapped area, there is no need to try to
787+
* split. Additionally, CONFIG_DEBUG_PAGEALLOC and CONFIG_KFENCE may
788+
* change permissions from atomic context so for those cases (which are
789+
* always pte-mapped), we must not go any further because taking the
790+
* mutex below may sleep.
791+
*/
792+
if (force_pte_mapping() || is_kfence_address((void *)start))
794793
return 0;
795794

796795
/*
@@ -1089,7 +1088,7 @@ bool arch_kfence_init_pool(void)
10891088
int ret;
10901089

10911090
/* Exit early if we know the linear map is already pte-mapped. */
1092-
if (!split_leaf_mapping_possible())
1091+
if (force_pte_mapping())
10931092
return true;
10941093

10951094
/* Kfence pool is already pte-mapped for the early init case. */

0 commit comments

Comments
 (0)