Hi,
> On 9 Nov 2020, at 08:21, Penny Zheng <penny.zh...@arm.com> wrote:
>
> CNTVCT_EL0 or CNTPCT_EL0 counter read in Cortex-A73 (all versions)
> might return a wrong value when the counter crosses a 32bit boundary.
>
> Until now, there is no case for Xen itself to access CNTVCT_EL0,
> and it also should be the Guest OS's responsibility to deal with
> this part.
>
> But for CNTPCT, there exists several cases in Xen involving reading
> CNTPCT, so a possible workaround is that performing the read twice,
> and to return one or the other depending on whether a transition has
> taken place.
>
> Signed-off-by: Penny Zheng <penny.zh...@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marq...@arm.com>
Regards
Bertrand
> ---
> docs/misc/arm/silicon-errata.txt | 1 +
> xen/arch/arm/Kconfig | 18 ++++++++++++++++++
> xen/arch/arm/cpuerrata.c | 8 ++++++++
> xen/arch/arm/vtimer.c | 2 +-
> xen/include/asm-arm/cpuerrata.h | 2 ++
> xen/include/asm-arm/cpufeature.h | 3 ++-
> xen/include/asm-arm/time.h | 22 +++++++++++++++++++++-
> 7 files changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/docs/misc/arm/silicon-errata.txt
> b/docs/misc/arm/silicon-errata.txt
> index 1f18a9df58..552c4151d3 100644
> --- a/docs/misc/arm/silicon-errata.txt
> +++ b/docs/misc/arm/silicon-errata.txt
> @@ -51,6 +51,7 @@ stable hypervisors.
> | ARM | Cortex-A57 | #1319537 | N/A
> |
> | ARM | Cortex-A72 | #1319367 | N/A
> |
> | ARM | Cortex-A72 | #853709 | N/A
> |
> +| ARM | Cortex-A73 | #858921 | ARM_ERRATUM_858921
> |
> | ARM | Cortex-A76 | #1165522 | N/A
> |
> | ARM | Neoverse-N1 | #1165522 | N/A
> | ARM | MMU-500 | #842869 | N/A
> |
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 2777388265..f938dd21bd 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -226,6 +226,24 @@ config ARM64_ERRATUM_834220
>
> If unsure, say Y.
>
> +config ARM_ERRATUM_858921
> + bool "Cortex-A73: 858921: Possible wrong read value for CNTVCT or
> CNTPCT"
> + default y
> + help
> + This option adds an alternative code sequence to work around ARM
> + erratum 858921 on Cortex-A73 (all versions).
> +
> + Affected Cortex-A73 might return wrong read value for CNTVCT or CNTPCT
> + when the counter crosses a 32bit boundary.
> +
> + The workaround involves performing the read twice, and to return
> + one or the other value depending on whether a transition has taken
> place.
> + Please note that this does not necessarily enable the workaround,
> + as it depends on the alternative framework, which will only patch
> + the kernel if an affected CPU is detected.
> +
> + If unsure, say Y.
> +
> endmenu
>
> config ARM64_HARDEN_BRANCH_PREDICTOR
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index 6731d873e8..567911d380 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -469,6 +469,14 @@ static const struct arm_cpu_capabilities arm_errata[] = {
> .capability = ARM_SSBD,
> .matches = has_ssbd_mitigation,
> },
> +#endif
> +#ifdef CONFIG_ARM_ERRATUM_858921
> + {
> + /* Cortex-A73 (all versions) */
> + .desc = "ARM erratum 858921",
> + .capability = ARM_WORKAROUND_858921,
> + MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
> + },
> #endif
> {
> /* Neoverse r0p0 - r2p0 */
> diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
> index 6d39fc944f..c2b27915c6 100644
> --- a/xen/arch/arm/vtimer.c
> +++ b/xen/arch/arm/vtimer.c
> @@ -62,7 +62,7 @@ static void virt_timer_expired(void *data)
>
> int domain_vtimer_init(struct domain *d, struct xen_arch_domainconfig *config)
> {
> - d->arch.virt_timer_base.offset = READ_SYSREG64(CNTPCT_EL0);
> + d->arch.virt_timer_base.offset = get_cycles();
> d->time_offset.seconds = ticks_to_ns(d->arch.virt_timer_base.offset -
> boot_count);
> do_div(d->time_offset.seconds, 1000000000);
>
> diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
> index 88ef3ca934..8d7e7b9375 100644
> --- a/xen/include/asm-arm/cpuerrata.h
> +++ b/xen/include/asm-arm/cpuerrata.h
> @@ -28,6 +28,8 @@ static inline bool check_workaround_##erratum(void)
> \
> CHECK_WORKAROUND_HELPER(766422, ARM32_WORKAROUND_766422, CONFIG_ARM_32)
> CHECK_WORKAROUND_HELPER(834220, ARM64_WORKAROUND_834220, CONFIG_ARM_64)
> CHECK_WORKAROUND_HELPER(ssbd, ARM_SSBD, CONFIG_ARM_SSBD)
> +CHECK_WORKAROUND_HELPER(858921, ARM_WORKAROUND_858921,
> + CONFIG_ARM_ERRATUM_858921)
>
> #undef CHECK_WORKAROUND_HELPER
>
> diff --git a/xen/include/asm-arm/cpufeature.h
> b/xen/include/asm-arm/cpufeature.h
> index 10878ead8a..016a9fe203 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -45,8 +45,9 @@
> #define ARM_SSBD 7
> #define ARM_SMCCC_1_1 8
> #define ARM64_WORKAROUND_AT_SPECULATE 9
> +#define ARM_WORKAROUND_858921 10
>
> -#define ARM_NCAPS 10
> +#define ARM_NCAPS 11
>
> #ifndef __ASSEMBLY__
>
> diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h
> index 9cb6f9b0b4..1b2c13614b 100644
> --- a/xen/include/asm-arm/time.h
> +++ b/xen/include/asm-arm/time.h
> @@ -3,6 +3,7 @@
>
> #include <asm/sysregs.h>
> #include <asm/system.h>
> +#include <asm/cpuerrata.h>
>
> #define DT_MATCH_TIMER \
> DT_MATCH_COMPATIBLE("arm,armv7-timer"), \
> @@ -13,7 +14,26 @@ typedef uint64_t cycles_t;
> static inline cycles_t get_cycles (void)
> {
> isb();
> - return READ_SYSREG64(CNTPCT_EL0);
> + /*
> + * ARM_WORKAROUND_858921: Cortex-A73 (all versions) counter read
> + * can return a wrong value when the counter crosses a 32bit
> boundary.
> + */
> + if ( !check_workaround_858921() )
> + return READ_SYSREG64(CNTPCT_EL0);
> + else
> + {
> + /*
> + * A recommended workaround for erratum 858921 is to:
> + * 1- Read twice CNTPCT.
> + * 2- Compare bit[32] of the two read values.
> + * - If bit[32] is different, keep the old value.
> + * - If bit[32] is the same, keep the new value.
> + */
> + cycles_t old, new;
> + old = READ_SYSREG64(CNTPCT_EL0);
> + new = READ_SYSREG64(CNTPCT_EL0);
> + return (((old ^ new) >> 32) & 1) ? old : new;
> + }
> }
>
> /* List of timer's IRQ */
> --
> 2.25.1
>
