* [PATCH] arm64: initialize all of CNTHCTL_EL2
@ 2021-08-18 16:15 Mark Rutland
2021-08-18 19:37 ` Oliver Upton
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Mark Rutland @ 2021-08-18 16:15 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: catalin.marinas, mark.rutland, maz, oupton, will
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
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] arm64: initialize all of CNTHCTL_EL2
2021-08-18 16:15 [PATCH] arm64: initialize all of CNTHCTL_EL2 Mark Rutland
@ 2021-08-18 19:37 ` Oliver Upton
2021-08-19 8:15 ` Marc Zyngier
2021-08-19 9:04 ` Will Deacon
2 siblings, 0 replies; 4+ messages in thread
From: Oliver Upton @ 2021-08-18 19:37 UTC (permalink / raw)
To: Mark Rutland; +Cc: linux-arm-kernel, catalin.marinas, maz, will
On Wed, Aug 18, 2021 at 9:15 AM 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.
s/CNPTOFF_EL2/CNTHCTL_EL2
>
> 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
> --
> 2.11.0
>
Besides noted above:
Reviewed-by: Oliver Upton <oupton@google.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] arm64: initialize all of CNTHCTL_EL2
2021-08-18 16:15 [PATCH] arm64: initialize all of CNTHCTL_EL2 Mark Rutland
2021-08-18 19:37 ` Oliver Upton
@ 2021-08-19 8:15 ` Marc Zyngier
2021-08-19 9:04 ` Will Deacon
2 siblings, 0 replies; 4+ messages in thread
From: Marc Zyngier @ 2021-08-19 8:15 UTC (permalink / raw)
To: Mark Rutland; +Cc: linux-arm-kernel, catalin.marinas, oupton, will
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] arm64: initialize all of CNTHCTL_EL2
2021-08-18 16:15 [PATCH] arm64: initialize all of CNTHCTL_EL2 Mark Rutland
2021-08-18 19:37 ` Oliver Upton
2021-08-19 8:15 ` Marc Zyngier
@ 2021-08-19 9:04 ` Will Deacon
2 siblings, 0 replies; 4+ messages in thread
From: Will Deacon @ 2021-08-19 9:04 UTC (permalink / raw)
To: Mark Rutland, linux-arm-kernel
Cc: catalin.marinas, kernel-team, Will Deacon, oupton, maz
On Wed, 18 Aug 2021 17:15:35 +0100, Mark Rutland 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.
>
> [...]
Applied to arm64 (for-next/fixes), thanks!
[1/1] arm64: initialize all of CNTHCTL_EL2
https://git.kernel.org/arm64/c/bde8fff82e4a
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-08-19 9:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-18 16:15 [PATCH] arm64: initialize all of CNTHCTL_EL2 Mark Rutland
2021-08-18 19:37 ` Oliver Upton
2021-08-19 8:15 ` Marc Zyngier
2021-08-19 9:04 ` Will Deacon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.