Re: [PATCH] arm64: initialize all of CNTHCTL_EL2

[Marc Zyngier <maz@kernel.org>]

On Wed, 18 Aug 2021 17:15:35 +0100,
Mark Rutland <mark.rutland@arm.com> wrote:
> 
> In __init_el2_timers we initialize CNTHCTL_EL2.{EL1PCEN,EL1PCTEN} with a
> RMW sequence, leaving all other bits UNKNOWN.
> 
> In general, we should initialize all bits in a register rather than
> using an RMW sequence, since most bits are UNKNOWN out of reset, and as
> new bits are added to the reigster their reset value might not result in
> expected behaviour.
> 
> In the case of CNTHCTL_EL2, FEAT_ECV added a number of new control bits
> in previously RES0 bits, which reset to UNKNOWN values, and may cause
> issues for EL1 and EL0:
> 
> * CNTHCTL_EL2.ECV enables the CNTPOFF_EL2 offset (which itself resets to
>   an UNKNOWN value) at EL0 and EL1. Since the offset could reset to
>   distinct values across CPUs, when the control bit resets to 1 this
>   could break timekeeping generally.
> 
> * CNTHCTL_EL2.{EL1TVT,EL1TVCT} trap EL0 and EL1 accesses to the EL1
>   virtual timer/counter registers to EL2. When reset to 1, this could
>   cause unexpected traps to EL2.
> 
> Initializing these bits to zero avoids these problems, and all other
> bits in CNPTOFF_EL2 other than EL1PCEN and EL1PCTEN can safely be reset
> to zero.
> 
> This patch ensures we initialize CNTHCTL_EL2 accordingly, only setting
> EL1PCEN and EL1PCTEN, and setting all other bits to zero.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oupton@google.com>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/include/asm/el2_setup.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
> index 21fa330f498dd..b83fb24954b77 100644
> --- a/arch/arm64/include/asm/el2_setup.h
> +++ b/arch/arm64/include/asm/el2_setup.h
> @@ -33,8 +33,7 @@
>   * EL2.
>   */
>  .macro __init_el2_timers
> -	mrs	x0, cnthctl_el2
> -	orr	x0, x0, #3			// Enable EL1 physical timers
> +	mov	x0, #3				// Enable EL1 physical timers
>  	msr	cnthctl_el2, x0
>  	msr	cntvoff_el2, xzr		// Clear virtual offset
>  .endm

With the typo spotted by Oliver fixed:

Acked-by: Marc Zyngier <maz@kernel.org>

	M.

-- 
Without deviation from the norm, progress is not possible.

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