[PATCH v2 5/6] arm: Use armv8_switch_to_el1 env to switch to EL1

[Andre Przywara]

On 8/19/21 4:53 PM, Peter Hoyes wrote:

Hi,

> From: Peter Hoyes <Peter.Hoyes at arm.com>
> 
> Use the environment variable armv8_switch_to_el1 to determine whether
> to switch to EL1 at runtime. This is an alternative to the
> CONFIG_ARMV8_SWITCH_TO_EL1 compile-time option.

This might be useful outside of the v8-R64 FVP. I cannot find 
CONFIG_ARMV8_SWITCH_TO_EL1 being set anywhere, which makes me wonder how 
this is used? Are there certain custom builds which define this somehow?

In any case forcing "kernel" entry in either EL1 or EL2, and deciding 
this at runtime sounds useful for certain scenarios in general, and be 
it for debugging and testing. So shall we get rid of this compile time 
option at all, or shall this be retained to avoid extra code?

> The environment variable will be ineffective if the ARMV8_MULTIENTRY
> config is used.
> 
> This is required by the Armv8r64 architecture, which must be able to
> boot at S-EL1 for Linux but may need to boot at other ELs for other
> systems.
> 
> Signed-off-by: Peter Hoyes <Peter.Hoyes at arm.com>
> ---
>   arch/arm/lib/bootm.c | 40 +++++++++++++++++++++++++---------------
>   1 file changed, 25 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index f60ee3a7e6..ea9bfe7570 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -317,7 +317,6 @@ __weak void update_os_arch_secondary_cores(uint8_t os_arch)
>   {
>   }
>   
> -#ifdef CONFIG_ARMV8_SWITCH_TO_EL1
>   static void switch_to_el1(void)
>   {
>   	if ((IH_ARCH_DEFAULT == IH_ARCH_ARM64) &&
> @@ -332,7 +331,6 @@ static void switch_to_el1(void)
>   				    ES_TO_AARCH64);
>   }
>   #endif
> -#endif
>   
>   /* Subcommand: GO */
>   static void boot_jump_linux(bootm_headers_t *images, int flag)
> @@ -359,21 +357,33 @@ static void boot_jump_linux(bootm_headers_t *images, int flag)
>   
>   		update_os_arch_secondary_cores(images->os.arch);
>   
> -#ifdef CONFIG_ARMV8_SWITCH_TO_EL1
> -		armv8_switch_to_el2((u64)images->ft_addr, 0, 0, 0,
> -				    (u64)switch_to_el1, ES_TO_AARCH64);
> +#ifdef CONFIG_ARMV8_MULTIENTRY
> +		int armv8_switch_to_el1 = -1;
>   #else
> -		if ((IH_ARCH_DEFAULT == IH_ARCH_ARM64) &&
> -		    (images->os.arch == IH_ARCH_ARM))
> -			armv8_switch_to_el2(0, (u64)gd->bd->bi_arch_number,
> -					    (u64)images->ft_addr, 0,
> -					    (u64)images->ep,
> -					    ES_TO_AARCH32);
> -		else
> -			armv8_switch_to_el2((u64)images->ft_addr, 0, 0, 0,
> -					    images->ep,
> -					    ES_TO_AARCH64);
> +		int armv8_switch_to_el1 = env_get_yesno("armv8_switch_to_el1");
>   #endif
> +#ifdef CONFIG_ARMV8_SWITCH_TO_EL1
> +		if (armv8_switch_to_el1 == -1) {
> +			armv8_switch_to_el1 = 1;
> +		}
> +#endif
> +		if (armv8_switch_to_el1 == 1) {

This looks confusing. Can't we use CONFIG_IS_ENABLED() and override 
armv8_switch_to_el1, then use this one variable to trigger the action?

Cheers,
Andre

> +			armv8_switch_to_el2((u64)images->ft_addr, 0, 0, 0,
> +					    (u64)switch_to_el1, ES_TO_AARCH64);
> +		} else {
> +			if ((IH_ARCH_DEFAULT == IH_ARCH_ARM64) &&
> +					(images->os.arch == IH_ARCH_ARM))
> +				armv8_switch_to_el2(0,
> +						    (u64)gd->bd->bi_arch_number,
> +						    (u64)images->ft_addr, 0,
> +						    (u64)images->ep,
> +						    ES_TO_AARCH32);
> +			else
> +				armv8_switch_to_el2((u64)images->ft_addr,
> +						    0, 0, 0,
> +						    images->ep,
> +						    ES_TO_AARCH64);
> +		}
>   	}
>   #else
>   	unsigned long machid = gd->bd->bi_arch_number;
>