Re: [PATCH v2 4/6] ARM: MMU: map text segment ro and data segments execute never

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 6/17/25 16:28, Sascha Hauer wrote:
> With this all segments in the DRAM except the text segment are mapped
> execute-never so that only the barebox code can actually be executed.
> Also map the readonly data segment readonly so that it can't be
> modified.
> 
> The mapping is only implemented in barebox proper. The PBL still maps
> the whole DRAM rwx.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> +config ARM_MMU_PERMISSIONS
> +	bool "Map with extended RO/X permissions"
> +	default y
> +	help
> +	  Enable this option to map readonly sections as readonly, executable
> +	  sections as readonly/executable and the remainder of the SDRAM as
> +	  read/write/non-executable.
> +	  Traditionally barebox maps the whole SDRAM as read/write/execute.
> +	  You get this behaviour by disabling this option which is meant as
> +	  a debugging facility. It can go away once the extended permission
> +	  settings are proved to work reliable.
> +
>  config ARM_SEMIHOSTING
>  	bool "enable ARM semihosting support"
>  	select SEMIHOSTING
> diff --git a/arch/arm/cpu/lowlevel_32.S b/arch/arm/cpu/lowlevel_32.S
> index 960a92b78c0adaf815948517ba917ae85ae65e27..5d524faf9cff9a8b545044169b8255279dd8ab0b 100644
> --- a/arch/arm/cpu/lowlevel_32.S
> +++ b/arch/arm/cpu/lowlevel_32.S
> @@ -70,6 +70,7 @@ THUMB(	orr	r12, r12, #PSR_T_BIT	)
>  	orr	r12, r12, #CR_U
>  	bic	r12, r12, #CR_A
>  #else
> +	orr	r12, r12, #CR_S

FTR, because I looked it up: CR_S was deprecated for ARMv6, but before
that, it was the system protection bit and is a prerequisite for access
permission. So this change looks fine.

>From ARMv5 ARM at:
https://developer.arm.com/documentation/ddi0100/i/?lang=en

> @@ -627,11 +666,7 @@ void mmu_early_enable(unsigned long membase, unsigned long memsize, unsigned lon
>  
>  	set_ttbr(ttb);
>  
> -	/* For the XN bit to take effect, we can't be using DOMAIN_MANAGER. */
> -	if (cpu_architecture() >= CPU_ARCH_ARMv7)
> -		set_domain(DOMAIN_CLIENT);
> -	else
> -		set_domain(DOMAIN_MANAGER);
> +	set_domain(DOMAIN_CLIENT);

Good. I had added this check ultimately out of laziness.

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |