Re: [Minios-devel] [UNIKRAFT PATCHv5 26/46] plat/common: Add counter workaround for Cortex-A73 erratum 858921

["Wei Chen (Arm Technology China)" <Wei.Chen@xxxxxxx>]

> -----Original Message-----
> From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> Sent: 2018年9月7日 17:58
> To: Julien Grall <Julien.Grall@xxxxxxx>; Wei Chen (Arm Technology China)
> <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
> Subject: Re: [Minios-devel] [UNIKRAFT PATCHv5 26/46] plat/common: Add counter
> workaround for Cortex-A73 erratum 858921
> 
> 
> 
> On 13.08.2018 11:03, Julien Grall wrote:
> > Hi Wei,
> >
> > On 10/08/18 08:08, Wei Chen wrote:
> >> The errata #858921 describes that Cortex-A73 (r0p0 - r0p2)
> >> counter read can return a wrong value when the counter crosses
> >> a 32bit boundary, but newer Cortex-A73 are not affected.
> >>
> >> The workaround involves performing the read twice, compare
> >> bit[32] of the two read values. If bit[32] is different,
> >> keep the first value, otherwise keep the second value.
> >>
> >> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> >> ---
> >>   arch/arm/arm64/Config.uk |  9 +++++++++
> >>   plat/common/arm/time.c   | 20 ++++++++++++++++++++
> >>   2 files changed, 29 insertions(+)
> >>
> >> diff --git a/arch/arm/arm64/Config.uk b/arch/arm/arm64/Config.uk
> >> index 7797516..07bc8ec 100644
> >> --- a/arch/arm/arm64/Config.uk
> >> +++ b/arch/arm/arm64/Config.uk
> >> @@ -46,3 +46,12 @@ config MARCH_ARM64_CORTEXA75
> >>           Compile for Armv8.2 Cortex-A75 (and compatible) CPUs
> >>   endchoice
> >> +
> >> +config ARM64_ERRATUM_858921
> >> +    bool "Workaround for Cortex-A73 erratum 858921"
> >> +    default n
> >> +    depends on MARCH_ARM64_CORTEXA73
> >
> > I don't think this is correct here. MARCH_ARM64_CORTEXA73 is about how
> > the code was optimized for a given processor. It would still be possible
> > to run a code compiled with generic option on Cortex-A73.
> >
> > So you want to at least drop the depends on here and possibly default y
> > for CORTEX_A73 and GENERIC.
> >
> > Cheers,
> 
> I am okay with Juliens suggestion. I agree that it is safer to have the
> erratum enabled on default for people that do not know if their SOC is
> affected or not.
> We may need to think about the MARCH_ARM64_NATIVE and the other cases
> where the could would run on A73. Probably these should also go to the
> `depends on` list?
> 

Ok, that sounds sensible. I will fix it in next version, and add
MARCH_ARM64_NATIVE to depends on list.

> >
> >
> >> +    help
> >> +      This option enables a workaround for Cortex-A73 (r0p0 - r0p2),
> >> +      whose counter may return a wrong value when the counter crosses
> >> +      a 32-bit boundary. The newer Cortex-A73 are not affected.
> >> diff --git a/plat/common/arm/time.c b/plat/common/arm/time.c
> >> index 4d955f3..1829938 100644
> >> --- a/plat/common/arm/time.c
> >> +++ b/plat/common/arm/time.c
> >> @@ -60,10 +60,30 @@ static inline uint64_t get_counter_frequency(void)
> >>       return SYSREG_READ(cntfrq_el0);
> >>   }
> >> +#ifdef CONFIG_ARM64_ERRATUM_858921
> >> +/*
> >> + * The errata #858921 describes that Cortex-A73 (r0p0 - r0p2) counter
> >> + * read can return a wrong value when the counter crosses a 32bit
> >> boundary.
> >> + * But newer Cortex-A73 are not affected.
> >> + *
> >> + * The workaround involves performing the read twice, compare bit[32] of
> >> + * the two read values. If bit[32] is different, keep the first value,
> >> + * otherwise keep the second value.
> >> + */
> >> +static uint64_t read_virtual_count(void)
> >> +{
> >> +    uint64_t val_1st, val_2nd;
> >> +
> >> +    val_1st = SYSREG_READ(cntvct_el0);
> >> +    val_2nd = SYSREG_READ(cntvct_el0);
> >> +    return (((val_1st ^ val_2nd) >> 32) & 1) ? val_1st : val_2nd;
> >> +}
> >> +#else
> >>   static inline uint64_t read_virtual_count(void)
> >>   {
> >>       return SYSREG_READ(cntvct_el0);
> >>   }
> >> +#endif
> >>   /*
> >>    * monotonic_clock(): returns # of nanoseconds passed since
> >>
> >
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel