Re: [Qemu-devel] [PATCH] [PowerPC][RFC] booke timers

[Scott Wood]

On Thu, 30 Jun 2011 17:51:10 +0200
Fabien Chouteau <address@hidden> wrote:

> On 28/06/2011 19:49, Scott Wood wrote:
> > On Tue, 28 Jun 2011 15:35:00 +0200
> > Fabien Chouteau <address@hidden> wrote:
> > 
> >> On 27/06/2011 22:28, Scott Wood wrote:
> >>> On Mon, 27 Jun 2011 18:14:06 +0200
> >>> Fabien Chouteau <address@hidden> wrote:
> >>>
> >>>> While working on the emulation of the freescale p2010 (e500v2) I 
> >>>> realized that
> >>>> there's no implementation of booke's timers features. Currently mpc8544 
> >>>> uses
> >>>> ppc_emb (ppc_emb_timers_init) which is close but not exactly like booke 
> >>>> (for
> >>>> example booke uses different SPR).
> >>>
> >>> ppc_emb_timers_init should be renamed something less generic, then.
> >>
> >> Agreed, can you help me to find a better name?
> > 
> > What chips are covered by it?  40x?
> 
> The function is used by ppc4xx_init (ppc4xx_devs.c) and ppc440_init_xilinx
> (virtex_ml507.c), so I guess ppc_4xx_timers_int will be fine...

Doesn't 440 have normal booke timers?

> >>> I think some changes in the decrementer code are needed to provide booke
> >>> semantics -- no raising the interrupt based on the high bit of decr, and
> >>> stop counting when reach zero.
> >>
> >> Can you please explain, I don't see where I'm not compliant with booke's
> >> decrementer.
> > 
> > It's not an issue with this code specifically, but existing behavior in the
> > decrementer code that isn't appropriate for booke.
> > 
> > On classic/server powerpc, when decrementer hits zero, it wraps around, and
> > the upper bit of DECR is used to signal the interrupt.  On booke, when
> > decrementer hits zero, it stops, and the upper bit of DECR is not special.
> > 
> 
> And that's why I implemented the booke_decr_cb function that will emulate this
> behavior.

booke_decr_cb() doesn't control what happens when DECR is read after an
expiration, nor does it control whether an interrupt is raised if software
writes DECR with the high bit set.

> >> It's just a mask to keep only the defined bits.
> > 
> > Just seems unnecessary, and potentially harmful if CPU-specific code wants
> > to interpret implementation-defined bits, or if new bits are architected
> > in the future.
> >
> 
> On the other hand, undefined bit should always be read as zeros.

I don't think there's any such requirement.

-Scott