[PATCH] armv8: mmu: support unmapping regions with set_one_region()

[PATCH] armv8: mmu: support unmapping regions with set_one_region()

[Casey Connolly]

Currently set_one_region() implicitly assumes that we want to map a
region and aggressively splits blocks into tables to do this, but when
called with PTE_TYPE_FAULT to unmap a currently mapped region it may
try to unnecessarily split blocks which doesn't make sense if the entire
block should actually be unmapped. In the end it then has to walk every
single page and create a bunch of empty tables.

Introduce a check for this kind of behaviour and optimise with a fast
path, if we're unmapping a region >= the size of this entry then we can
just unmap the entire PTE and whatever it contains.

This fixes some bogus empty tables being left behind when carving out
reserved memory regions on Qualcomm, and should improve the performance
of the break-before-make in mmu_change_region_attr().

Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
---
 arch/arm/cpu/armv8/cache_v8.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
index 39479df7b21f..b1a1fd571341 100644
--- a/arch/arm/cpu/armv8/cache_v8.c
+++ b/arch/arm/cpu/armv8/cache_v8.c
@@ -939,8 +939,20 @@ static u64 set_one_region(u64 start, u64 size, u64 attrs, bool flag, int level)
 	int levelshift = level2shift(level);
 	u64 levelsize = 1ULL << levelshift;
 	u64 *pte = find_pte(start, level);
 
+	/*
+	 * If we're trying to unmap a region then check if it's already unmapped or if it's bigger
+	 * then the PTE we're looking at right now, in the first case we can do nothing and in the
+	 * second case we just need to unmap this page/block.
+	 * Otherwise we will needlessly create new tables until we have traversed every single page
+	 * in the region.
+	 */
+	if (attrs == PTE_TYPE_FAULT && (pte_type(pte) == PTE_TYPE_FAULT || size >= levelsize)) {
+		*pte &= ~PMD_ATTRMASK;
+		return levelsize;
+	}
+
 	/* Can we can just modify the current level block PTE? */
 	if (is_aligned(start, size, levelsize)) {
 		if (flag) {
 			*pte &= ~PMD_ATTRMASK;
@@ -1081,8 +1093,12 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs)
 	flush_dcache_range(gd->arch.tlb_addr,
 			   gd->arch.tlb_addr + gd->arch.tlb_size);
 	__asm_invalidate_tlb_all();
 
+	/* If we were unmapping a region then we're done! */
+	if (attrs == PTE_TYPE_FAULT)
+		return;
+
 	mmu_change_region_attr_nobreak(addr, siz, attrs);
 }
 
 int pgprot_set_attrs(phys_addr_t addr, size_t size, enum pgprot_attrs perm)
-- 
2.53.0

Re: [PATCH] armv8: mmu: support unmapping regions with set_one_region()

[Sumit Garg]

On Mon, May 04, 2026 at 07:53:54PM +0200, Casey Connolly wrote:
> Currently set_one_region() implicitly assumes that we want to map a
> region and aggressively splits blocks into tables to do this, but when
> called with PTE_TYPE_FAULT to unmap a currently mapped region it may
> try to unnecessarily split blocks which doesn't make sense if the entire
> block should actually be unmapped. In the end it then has to walk every
> single page and create a bunch of empty tables.
> 
> Introduce a check for this kind of behaviour and optimise with a fast
> path, if we're unmapping a region >= the size of this entry then we can
> just unmap the entire PTE and whatever it contains.
> 
> This fixes some bogus empty tables being left behind when carving out
> reserved memory regions on Qualcomm, and should improve the performance
> of the break-before-make in mmu_change_region_attr().
> 
> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
> ---
>  arch/arm/cpu/armv8/cache_v8.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

Nice optimization, FWIW:

Acked-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Tested-by: Sumit Garg <sumit.garg@oss.qualcomm.com>

-Sumit

> 
> diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
> index 39479df7b21f..b1a1fd571341 100644
> --- a/arch/arm/cpu/armv8/cache_v8.c
> +++ b/arch/arm/cpu/armv8/cache_v8.c
> @@ -939,8 +939,20 @@ static u64 set_one_region(u64 start, u64 size, u64 attrs, bool flag, int level)
>  	int levelshift = level2shift(level);
>  	u64 levelsize = 1ULL << levelshift;
>  	u64 *pte = find_pte(start, level);
>  
> +	/*
> +	 * If we're trying to unmap a region then check if it's already unmapped or if it's bigger
> +	 * then the PTE we're looking at right now, in the first case we can do nothing and in the
> +	 * second case we just need to unmap this page/block.
> +	 * Otherwise we will needlessly create new tables until we have traversed every single page
> +	 * in the region.
> +	 */
> +	if (attrs == PTE_TYPE_FAULT && (pte_type(pte) == PTE_TYPE_FAULT || size >= levelsize)) {
> +		*pte &= ~PMD_ATTRMASK;
> +		return levelsize;
> +	}
> +
>  	/* Can we can just modify the current level block PTE? */
>  	if (is_aligned(start, size, levelsize)) {
>  		if (flag) {
>  			*pte &= ~PMD_ATTRMASK;
> @@ -1081,8 +1093,12 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs)
>  	flush_dcache_range(gd->arch.tlb_addr,
>  			   gd->arch.tlb_addr + gd->arch.tlb_size);
>  	__asm_invalidate_tlb_all();
>  
> +	/* If we were unmapping a region then we're done! */
> +	if (attrs == PTE_TYPE_FAULT)
> +		return;
> +
>  	mmu_change_region_attr_nobreak(addr, siz, attrs);
>  }
>  
>  int pgprot_set_attrs(phys_addr_t addr, size_t size, enum pgprot_attrs perm)
> -- 
> 2.53.0
> 

Re: [PATCH] armv8: mmu: support unmapping regions with set_one_region()

[Casey Connolly]

On 05/05/2026 11:59, Sumit Garg wrote:
> On Mon, May 04, 2026 at 07:53:54PM +0200, Casey Connolly wrote:
>> Currently set_one_region() implicitly assumes that we want to map a
>> region and aggressively splits blocks into tables to do this, but when
>> called with PTE_TYPE_FAULT to unmap a currently mapped region it may
>> try to unnecessarily split blocks which doesn't make sense if the entire
>> block should actually be unmapped. In the end it then has to walk every
>> single page and create a bunch of empty tables.
>>
>> Introduce a check for this kind of behaviour and optimise with a fast
>> path, if we're unmapping a region >= the size of this entry then we can
>> just unmap the entire PTE and whatever it contains.
>>
>> This fixes some bogus empty tables being left behind when carving out
>> reserved memory regions on Qualcomm, and should improve the performance
>> of the break-before-make in mmu_change_region_attr().
>>
>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
>> ---
>>  arch/arm/cpu/armv8/cache_v8.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
> 
> Nice optimization, FWIW:
> 
> Acked-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> Tested-by: Sumit Garg <sumit.garg@oss.qualcomm.com>

Could you drop this on the v2 instead? Sorry for the confusion this
version had a silly bug.
> 
> -Sumit
> 
>>
>> diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
>> index 39479df7b21f..b1a1fd571341 100644
>> --- a/arch/arm/cpu/armv8/cache_v8.c
>> +++ b/arch/arm/cpu/armv8/cache_v8.c
>> @@ -939,8 +939,20 @@ static u64 set_one_region(u64 start, u64 size, u64 attrs, bool flag, int level)
>>  	int levelshift = level2shift(level);
>>  	u64 levelsize = 1ULL << levelshift;
>>  	u64 *pte = find_pte(start, level);
>>  
>> +	/*
>> +	 * If we're trying to unmap a region then check if it's already unmapped or if it's bigger
>> +	 * then the PTE we're looking at right now, in the first case we can do nothing and in the
>> +	 * second case we just need to unmap this page/block.
>> +	 * Otherwise we will needlessly create new tables until we have traversed every single page
>> +	 * in the region.
>> +	 */
>> +	if (attrs == PTE_TYPE_FAULT && (pte_type(pte) == PTE_TYPE_FAULT || size >= levelsize)) {
>> +		*pte &= ~PMD_ATTRMASK;
>> +		return levelsize;
>> +	}
>> +
>>  	/* Can we can just modify the current level block PTE? */
>>  	if (is_aligned(start, size, levelsize)) {
>>  		if (flag) {
>>  			*pte &= ~PMD_ATTRMASK;
>> @@ -1081,8 +1093,12 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs)
>>  	flush_dcache_range(gd->arch.tlb_addr,
>>  			   gd->arch.tlb_addr + gd->arch.tlb_size);
>>  	__asm_invalidate_tlb_all();
>>  
>> +	/* If we were unmapping a region then we're done! */
>> +	if (attrs == PTE_TYPE_FAULT)
>> +		return;
>> +
>>  	mmu_change_region_attr_nobreak(addr, siz, attrs);
>>  }
>>  
>>  int pgprot_set_attrs(phys_addr_t addr, size_t size, enum pgprot_attrs perm)
>> -- 
>> 2.53.0
>>

-- 
// Casey (she/her)