Re: [PATCH 03/13] arm64: head.S: always initialize PSTATE

From: James Morse <james.morse@arm.com>
To: Mark Rutland <mark.rutland@arm.com>,
	linux-arm-kernel@lists.infradead.org
Cc: catalin.marinas@arm.com, robin.murphy@arm.com, hch@lst.de,
	will@kernel.org
Subject: Re: [PATCH 03/13] arm64: head.S: always initialize PSTATE
Date: Wed, 30 Sep 2020 17:09:47 +0100	[thread overview]
Message-ID: <ec708d49-0ddf-6cfd-f1f5-100210549318@arm.com> (raw)
In-Reply-To: <20200925160722.27155-4-mark.rutland@arm.com>

Hi Mark,

On 25/09/2020 17:07, Mark Rutland wrote:
> As with SCTLR_ELx and other control registers, some PSTATE bits are
> UNKNOWN out-of-reset, and we may not be able to rely on hardware of
> firmware

(hardware or firmware)

> to initialize them to our liking prior to entry to the kernel,
> e.g. in the primary/secondary boot paths and return from idle/suspend.
> 
> It would be more robust (and easier to reason about) if we consistently
> initialized PSTATE to a default value, as we do with control registers.
> This will ensure that the kernel is not adversely affected by bits it is
> not aware of, e.g. when support for a feature such as PAN/UAO is
> disabled.
> 
> This patch ensures that PSTATE is consistently initialized at boot time
> via an ERET. This is not intended to relax the existing requirements
> (e.g. DAIF bits must still be set prior to entering the kernel). For
> features detected dynamically (which may require system-wide support),
> it is still necessary to subsequently modify PSTATE.

> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 9bb07be45249a..b8ee9a62c9b61 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -494,17 +494,22 @@ EXPORT_SYMBOL(kimage_vaddr)
>   * booted in EL1 or EL2 respectively.
>   */
>  SYM_FUNC_START(init_kernel_el)
> -	msr	SPsel, #1			// We want to use SP_EL{1,2}
>  	mrs	x0, CurrentEL
>  	cmp	x0, #CurrentEL_EL2
> -	b.eq	1f
> +	b.eq	init_el2
> +
> +SYM_INNER_LABEL(init_el1, SYM_L_LOCAL)
>  	mov_q	x0, INIT_SCTLR_EL1_MMU_OFF
>  	msr	sctlr_el1, x0
> -	mov	w0, #BOOT_CPU_MODE_EL1		// This cpu booted in EL1

>  	isb

Could you add a comment covering why this ISB isn't rolled into the ERET?
I'm guessing its in case SCTLR_EL1 previously had v8.5s EOS 'Exception Exit is Context
Synchronizing' clear.

> -	ret
> +	mov_q	x0, INIT_PSTATE_EL1
> +	msr	spsr_el1, x0
> +	msr	elr_el1, lr
> +	mov	w0, #BOOT_CPU_MODE_EL1
> +	eret

Thanks,

James

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel