[RFC PATCH 3/4] arm64: mmu_change_region_attr() add an option not to break PTEs

[Heinrich Schuchardt]

On 1/30/25 07:20, Ilias Apalodimas wrote:
> The ARM ARM on section 8.17.1 describes the cases where
> break-before-make is required when changing live page tables.
> Since we can use this function to tweak block and page permssions,
> where BBM is not required add an extra argument to the function.
>
> While at it add a function we can use to change the attributes of normal
> memory without breaking the pages tables first
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
>   arch/arm/cpu/armv8/cache_v8.c           | 27 ++++++++++++++++++++++++-
>   arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 10 ++++-----
>   arch/arm/include/asm/system.h           |  3 ++-
>   arch/arm/mach-snapdragon/board.c        |  2 +-
>   4 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
> index a4ca56c8ed42..60d0870013e8 100644
> --- a/arch/arm/cpu/armv8/cache_v8.c
> +++ b/arch/arm/cpu/armv8/cache_v8.c
> @@ -968,11 +968,14 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
>    * The procecess is break-before-make. The target region will be marked as
>    * invalid during the process of changing.
>    */
> -void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs)
> +void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs, bool bbm)

The function description should be moved to the include or be completed
here.

>   {
>   	int level;
>   	u64 r, size, start;
>
> +	if (!bbm)
> +		goto skip_bbm;
> +
>   	start = addr;
>   	size = siz;
>   	/*
> @@ -997,6 +1000,7 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs)
>   			   gd->arch.tlb_addr + gd->arch.tlb_size);
>   	__asm_invalidate_tlb_all();
>
> +skip_bbm:
>   	/*
>   	 * Loop through the address range until we find a page granule that fits
>   	 * our alignment constraints, then set it to the new cache attributes
> @@ -1020,6 +1024,27 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs)
>   	__asm_invalidate_tlb_all();
>   }
>
> +void mmu_set_attrs(phys_addr_t addr, size_t size, int type)

Please, add a function description.

The function should either be static or added to an include file.

> +{
> +	u64 attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_INNER_SHARE | PTE_TYPE_VALID;
> +
> +	switch (type) {
> +	case 1:
> +	/* RO */
> +		attrs |= PTE_BLOCK_PXN | PTE_BLOCK_UXN | PTE_BLOCK_RO;
> +		break;
> +	case 2:
> +		/* RX */
> +		attrs |= PTE_BLOCK_RO;
> +		break;
> +	case 3:
> +		/* RW */
> +		attrs |= PTE_BLOCK_PXN | PTE_BLOCK_UXN;
> +	}
> +
> +	mmu_change_region_attr(addr, size, attrs, false);
> +}
> +
>   #else	/* !CONFIG_IS_ENABLED(SYS_DCACHE_OFF) */
>
>   /*
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> index d2d3e346a36f..caf1dab05936 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> @@ -1573,7 +1573,7 @@ void update_early_mmu_table(void)
>   					PTE_BLOCK_MEMTYPE(MT_NORMAL)	|
>   					PTE_BLOCK_OUTER_SHARE		|
>   					PTE_BLOCK_NS			|
> -					PTE_TYPE_VALID);
> +					PTE_TYPE_VALID, true);
>   	} else {
>   		mmu_change_region_attr(
>   					CFG_SYS_SDRAM_BASE,
> @@ -1581,7 +1581,7 @@ void update_early_mmu_table(void)
>   					PTE_BLOCK_MEMTYPE(MT_NORMAL)	|
>   					PTE_BLOCK_OUTER_SHARE		|
>   					PTE_BLOCK_NS			|
> -					PTE_TYPE_VALID);
> +					PTE_TYPE_VALID, true);
>   #ifdef CONFIG_SYS_DDR_BLOCK3_BASE
>   #ifndef CONFIG_SYS_DDR_BLOCK2_SIZE
>   #error "Missing CONFIG_SYS_DDR_BLOCK2_SIZE"
> @@ -1594,7 +1594,7 @@ void update_early_mmu_table(void)
>   					PTE_BLOCK_MEMTYPE(MT_NORMAL)	|
>   					PTE_BLOCK_OUTER_SHARE		|
>   					PTE_BLOCK_NS			|
> -					PTE_TYPE_VALID);
> +					PTE_TYPE_VALID, true);
>   			mmu_change_region_attr(
>   					CONFIG_SYS_DDR_BLOCK3_BASE,
>   					gd->ram_size -
> @@ -1603,7 +1603,7 @@ void update_early_mmu_table(void)
>   					PTE_BLOCK_MEMTYPE(MT_NORMAL)	|
>   					PTE_BLOCK_OUTER_SHARE		|
>   					PTE_BLOCK_NS			|
> -					PTE_TYPE_VALID);
> +					PTE_TYPE_VALID, true);
>   		} else
>   #endif
>   		{
> @@ -1614,7 +1614,7 @@ void update_early_mmu_table(void)
>   					PTE_BLOCK_MEMTYPE(MT_NORMAL)	|
>   					PTE_BLOCK_OUTER_SHARE		|
>   					PTE_BLOCK_NS			|
> -					PTE_TYPE_VALID);
> +					PTE_TYPE_VALID, true);
>   		}
>   	}
>   }
> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> index dbf9ab43e280..0a8ca19e45e9 100644
> --- a/arch/arm/include/asm/system.h
> +++ b/arch/arm/include/asm/system.h
> @@ -287,8 +287,9 @@ void flush_l3_cache(void);
>    * @emerg: Also map the region in the emergency table
>    */
>   void mmu_map_region(phys_addr_t start, u64 size, bool emerg);
> -void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs);
> +void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs, bool bbm);

Please, add a function description.

Best regards

Heinrich

>
> +void mmu_set_attrs(phys_addr_t addr, size_t size, int type);
>   /*
>    * smc_call() - issue a secure monitor call
>    *
> diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
> index 2ef936aab757..13f4e8e640ef 100644
> --- a/arch/arm/mach-snapdragon/board.c
> +++ b/arch/arm/mach-snapdragon/board.c
> @@ -577,7 +577,7 @@ static void carve_out_reserved_memory(void)
>   		if (i == count || start + size < res[i].start - SZ_2M) {
>   			debug("  0x%016llx - 0x%016llx: reserved\n",
>   			      start, start + size);
> -			mmu_change_region_attr(start, size, PTE_TYPE_FAULT);
> +			mmu_change_region_attr(start, size, PTE_TYPE_FAULT, true);
>   			/* If this is the final region then quit here before we index
>   			 * out of bounds...
>   			 */