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

[Fabien Chouteau]

On 30/06/2011 21:26, Scott Wood wrote:
> 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?
>

Actually ppc_emb_timers_init should be renamed ppc40x_timers_init, and Xilinx's
ppc440 should use the new implementation of 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.
>

OK, I will add a condition to avoid this on booke processors.

>>>> 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.

My bad ;)

"The handling of reserved bits in System Registers (e.g. Integer Exception
Register, Floating-Point Status and Control Register) is
implementation-dependent. Software is permitted to write any value to such a
bit with no visible effect on processors that implement this version of Book E.
A subsequent reading of the bit returns a 0 if the last value written to the
bit was 0 and returns an undefined value (0 or 1) otherwise."

-- 
Fabien Chouteau