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

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

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?
> > +    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