Subject: Re: [PATCH] powerpc/fsl-booke: Work around erratum A-006958

From: Scott Wood <[email-masked]>
Date: Tue, 16 Jul 2013 13:16:40 -0500

On 07/16/2013 03:28:06 AM, David Laight wrote:
> > On 07/15/2013 11:53:54 AM, Scott Wood wrote:
> > > On 07/15/2013 03:45:36 AM, David Laight wrote:
> > >> Also, if the high word changes, there is no need to loop.
> > >> Just return the second value with a low word of zero
> > >> (the returned count happened while the function was active).
> > >
> > > That would be more complicated than looping.
>=20
> I'm not that familiar with the ppc instructions set, but on x86
> the equivalent instructions are synchronising ones - so can have
> performance penalties, so a few extra 'normal' instructions to
> avoid re-reading the timebase counter may be worth while.

The core manual doesn't list any special syncronization for it, though =20
it does take 4 cycles.

> The branch for the loop might also be statically predicted 'taken'
> leading to a branch misprediction penalty as well.

The branch predictor should work fine most of the time.

> > That said, it's since been confirmed internally that the low word
> > should always be zero when this happens, so we could share the Cell
> > workaround code -- as long as we do something special in the =20
> timebase
> > sync code so that we don't get stuck if the timebase happens to be
> > frozen with TBL=3D=3D0.  This would avoid the need for scratch register=
s
> > (other than CR0).
>=20
> Yes - if the actual errata is that the low word is read one clock
> later then the high word then the only illegal value is one where
> the low word is zero.

It's actually the timebase itself that is updated in the low word =20
first...

> In that case it is sufficient to reread the counter - it can't be
> wrong again!

...which means, while it *probably* won't be wrong again, I'd want a =20
statement from the hardware people that this is guaranteed if we were =20
to rely on it.

The workaround for a similar bug on Cell rereads until the lower half =20
is non-zero, which will be fine as long as we avoid the workaround in =20
timebase sync code (when the timebase is frozen).

-Scott=