CPU15 errata workaround for 8xx by WD

CPU15 errata workaround for 8xx by WD

[Joakim Tjernlund]

Hi Wolfgang

Had a look at your CPU15 errata workaround for 8xx and I have a comment
or two:

1) I think you should make the sysctl support a compile time option
   as the overhead for sysctl support in the TLB handler is 6 instr. when
   the fix itself is only 4 instr.

2) You placed the workaround in the middle of the CPU6 workaround which will
   disable the CPU6 workaround(I think). Move it before the #ifdef CONFIG_8xx_CPU6
   and you should be fine.

3) Your test program uses the dcbst and dcbi instr. and these are buggy as they do not
   update the DAR register in the TLB exceptions. I guess you made sure that such errors
   will not happen?

4) The CPU15 bug has been around for years I think, what made it show up now? New toolchains?
   

 Jocke
   

Re: CPU15 errata workaround for 8xx by WD

[Wolfgang Denk]

Dear Joakim,

in message <BCEFJBPJCGFCNMMMIDBHOEPBCJAA.Joakim.Tjernlund@lumentis.se> you wrote:
> 
> Had a look at your CPU15 errata workaround for 8xx and I have a comment
> or two:
> 
> 1) I think you should make the sysctl support a compile time option
>    as the overhead for sysctl support in the TLB handler is 6 instr. when
>    the fix itself is only 4 instr.

You are right. For a permanent patch that would be better  -  in  our
case  it  was important to be able to turn on and off this workaround
dynamically with exactly the same kernel binary. We  wanted  to  have
real  proof that the effects we saw were caused by CPU15 erratum, and
that the workaround fixes these problems.

> 2) You placed the workaround in the middle of the CPU6 workaround which will
>    disable the CPU6 workaround(I think). Move it before the #ifdef CONFIG_8xx_CPU6
>    and you should be fine.

I think you are right.

> 3) Your test program uses the dcbst and dcbi instr. and these are buggy as they do not
>    update the DAR register in the TLB exceptions. I guess you made sure that such errors
>    will not happen?

Ummm... I think so.

> 4) The CPU15 bug has been around for years I think, what made it show up now? New toolchains?

No, this is not a toolchain issue. It's a processor problem.

I have to admit that I didn't believe our customer when  he  reported
that  he has problems that were caused by the CPU15 bug - I never saw
this on any other 8xx processor before, and as you say  it  has  been
mentioned  in  all errata sheets I can remember. As far as I can tell
it is only the MPC870/885 duet family of processors where this  CPU15
bug actually hits. I have no idea why.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Brain: an apparatus with which we think we think.    - Ambrose Bierce

RE: CPU15 errata workaround for 8xx by WD

[Joakim Tjernlund]

> Dear Joakim,
> 
> in message <BCEFJBPJCGFCNMMMIDBHOEPBCJAA.Joakim.Tjernlund@lumentis.se> you wrote:
> > 
> > Had a look at your CPU15 errata workaround for 8xx and I have a comment
> > or two:
> > 
> > 1) I think you should make the sysctl support a compile time option
> >    as the overhead for sysctl support in the TLB handler is 6 instr. when
> >    the fix itself is only 4 instr.
> 
> You are right. For a permanent patch that would be better  -  in  our
> case  it  was important to be able to turn on and off this workaround
> dynamically with exactly the same kernel binary. We  wanted  to  have
> real  proof that the effects we saw were caused by CPU15 erratum, and
> that the workaround fixes these problems.

Have you done any performance measurements with with/without the CPU15
patch? To always invalidate the previous and the next TLB(plus the extra code in the
TLB handler) seems expensive.

> 
> > 2) You placed the workaround in the middle of the CPU6 workaround which will
> >    disable the CPU6 workaround(I think). Move it before the #ifdef CONFIG_8xx_CPU6
> >    and you should be fine.
> 
> I think you are right.
> 
> > 3) Your test program uses the dcbst and dcbi instr. and these are buggy as they do not
> >    update the DAR register in the TLB exceptions. I guess you made sure that such errors
> >    will not happen?
> 
> Ummm... I think so.

Me too, but I only had a quick look.

> 
> > 4) The CPU15 bug has been around for years I think, what made it show up now? New toolchains?
> 
> No, this is not a toolchain issue. It's a processor problem.
> 
> I have to admit that I didn't believe our customer when  he  reported
> that  he has problems that were caused by the CPU15 bug - I never saw
> this on any other 8xx processor before, and as you say  it  has  been
> mentioned  in  all errata sheets I can remember. As far as I can tell
> it is only the MPC870/885 duet family of processors where this  CPU15
> bug actually hits. I have no idea why.

That explains why I never seen this before, thanks.

A better fix would be to make the toolchain insert an extra NOP if
the last instr. in a page is a branch, but that will take some time.
Any GCC people on this list?

    Regards
            Joakim
> 
> Best regards,
> 
> Wolfgang Denk

RE: CPU15 errata workaround for 8xx by WD

[Wrobel Heinz-r39252]

Wolfgang, Joakim,
 
> I have to admit that I didn't believe our customer when  he  
> reported that  he has problems that were caused by the CPU15 
> bug - I never saw this on any other 8xx processor before, and 
> as you say  it  has  been mentioned  in  all errata sheets I 
> can remember. As far as I can tell it is only the MPC870/885 
> duet family of processors where this  CPU15 bug actually 
> hits. I have no idea why.

CPU15 is highly timing dependent. Even a single clock difference in the bus interface can make it (dis)appear.
It can happen on Duet's as on any other 8xx ... or not.

The simple workaround of invalidating the prev/next page is exactly that. Simple. It works, but for code with page locality it can hurt performance because you would get excessive reloads. A smarter workaround affecting only pages that need the workaround is preferrable, if it fits into the MMU table framework.

Heinz

RE: CPU15 errata workaround for 8xx by WD

[Joakim Tjernlund]

Hi Heinz 

> Wolfgang, Joakim,
>  
> > I have to admit that I didn't believe our customer when  he  
> > reported that  he has problems that were caused by the CPU15 
> > bug - I never saw this on any other 8xx processor before, and 
> > as you say  it  has  been mentioned  in  all errata sheets I 
> > can remember. As far as I can tell it is only the MPC870/885 
> > duet family of processors where this  CPU15 bug actually 
> > hits. I have no idea why.
> 
> CPU15 is highly timing dependent. Even a single clock difference in the bus interface can make it (dis)appear.
> It can happen on Duet's as on any other 8xx ... or not.

Ahh, better not change any timing then on our boards :)
I have been running the cpu15 program from Wolfgang for 2 days now and no problem so far.

> 
> The simple workaround of invalidating the prev/next page is exactly that. Simple. It works, but for code with page 
> locality it can hurt performance because you would get excessive reloads. A smarter workaround affecting only pages that 
> need the workaround is preferrable, if it fits into the MMU table framework.

Are it there any know timing patterns that won't show this bug?

> 
> Heinz

Since you work at Freescale and appear to know 8xx I want to ask this:

Are there any plans on documenting/fixing the buggy cache insns. such dcbz, icbi etc. that
don't update the DAR register when causing a TLB Miss/Error?

I got this report from Motorola in May 2003:

We have generated the case of dcbx instruction and tlb miss
in verilog. We see that the DAR is not updated and the root couse is a
follows:

The load store treats dcbx instructions as a mtspr so it gets the tlb miss
error but does not update the DAR.

DAR is updated by two kind of errors
1) During the address phase.
2) During the data phase.


When there is a load store instruction the tlb miss error comes together
with the address phase. On dcbx instructions it comes one clock after the
address phase. So the DAR update mechanism does not update the DAR.


 Jocke

Re: CPU15 errata workaround for 8xx by WD

[Wolfgang Denk]

Dear Jocke,

sorry for the late reply.

In message <BCEFJBPJCGFCNMMMIDBHOEPBCJAA.Joakim.Tjernlund@lumentis.se> you wrote:
> 
> Had a look at your CPU15 errata workaround for 8xx and I have a comment
> or two:
> 
> 1) I think you should make the sysctl support a compile time option
>    as the overhead for sysctl support in the TLB handler is 6 instr. when
>    the fix itself is only 4 instr.

You are right.

> 2) You placed the workaround in the middle of the CPU6 workaround which will
>    disable the CPU6 workaround(I think). Move it before the #ifdef CONFIG_8xx_CPU6
>    and you should be fine.

You are right again - we fixed this in our CVS tree now.


Best regards,

Wolfgang Denk

-- 
See us @ Embedded World, Nuremberg, Feb 22 - 24,  Hall 10.0 Booth 310
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
The management question ... is not _whether_ to build a pilot  system
and  throw  it away. You _will_ do that. The only question is whether
to plan in advance to build a throwaway, or to promise to deliver the
throwaway to customers.       - Fred Brooks, "The Mythical Man Month"