On 09.09.2011, at 16:22, Fabien Chouteau wrote:
> On 09/09/2011 15:46, Alexander Graf wrote:
>>
>> On 09.09.2011, at 15:27, Fabien Chouteau wrote:
>>
>>> On 09/09/2011 12:55, Alexander Graf wrote:
>>>>
>>>> On 09.09.2011, at 12:36, Fabien Chouteau wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>> + }
>>>>>>>>> +
>>>>>>>>> + qemu_mod_timer(timer, *next);
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +static void booke_decr_cb(void *opaque)
>>>>>>>>> +{
>>>>>>>>> + CPUState *env;
>>>>>>>>> + ppc_tb_t *tb_env;
>>>>>>>>> + booke_timer_t *booke_timer;
>>>>>>>>> +
>>>>>>>>> + env = opaque;
>>>>>>>>> + tb_env = env->tb_env;
>>>>>>>>> + booke_timer = tb_env->opaque;
>>>>>>>>> + env->spr[SPR_BOOKE_TSR] |= TSR_DIS;
>>>>>>>>> + if (env->spr[SPR_BOOKE_TCR] & TCR_DIE) {
>>>>>>>>> + ppc_set_irq(env, PPC_INTERRUPT_DECR, 1);
>>>>>>>>> + }
>>>>>>>>
>>>>>>>> You will need this more often - also for the TCR write case - so
>>>>>>>> please put
>>>>>>>> the 3 lines above into a separate function and just call that here :)
>>>>>>>
>>>>>>> Actually the checks are never exactly the same, here we test DIE in
>>>>>>> TCR...
>>>>>>
>>>>>> Sure, we have 3 different tests for the 3 different bits we can
>>>>>> potentially set in TCR. The check always ends up being the same though:
>>>>>>
>>>>>> if (TSR & bit && TCR & bit)
>>>>>> set_irq(irq_for_bit);
>>>>>>
>>>>>> Most device emulators have a simple function for this called
>>>>>> "update_irq" that checks for all the bits and sets the respective irq
>>>>>> lines.
>>>>>>
>>>>>
>>>>> I know but we have two cases:
>>>>>
>>>>> - Timer hit: we check DIE in TCR
>>>>> - Rising edge of DIE in TCR (from user): check if DIS is set
>>>>>
>>>>> I don't think we can have a good generic function for this, and I don't
>>>>> forecast any improvement in code readability.
>>>>
>>>> update_decr_irq() {
>>>> if (TSR.DIS && TCR.DIE) {
>>>> set_irq(DECR);
>>>> } else {
>>>> unset_irq(DECR);
>>>> }
>>>> }
>>>>
>>>> Timer hit:
>>>>
>>>> TSR |= DIS;
>>>> update_decr_irq();
>>>>
>>>> Setting TCR:
>>>>
>>>> TCR |= DIE;
>>>> update_decr_irq();
>>>>
>>>> Or am I misunderstanding the conditions under which the irq actually
>>>> triggers? TCR.DIE is only the interrupt enabled flag - the timer can still
>>>> hit nevertheless. The level of the interrupt is determined by TSR.DIR which
>>>> is what the timer sets when it hits. Unless I completely misread the spec,
>>>> an
>>>> interrupt occurs when both of them are true. So all we need to do is have
>>>> that check and run it every time we change a value in TSR or TCR.
>>>
>>> Well OK, this can work to trigger the interrupts, not to clear them though.
>>> And it will call ppc_set_irq when it's not required.
>>
>> Calling ppc_set_irq shouldn't be an issue - at the end of the day it only
>> does a simple OR operation. Unsetting should be pretty obvious too though,
>> no? When either TSR.DIS or TCR.DIE are not set, the interrupt line is low.
>>
>
> if the interrupt is already set and you clear TCR.DIE, the interrupt has to
> remain set. The only way to unset an interrupt is to clear the corresponding
> bit in TSR (currently in store_booke_tsr).
Are you sure? I see several things in the 2.06 spec:
TSR.DIS:
0 A Decrementer event has not occurred.
1 A Decrementer event has occurred.
when MSR.EE=1 and TCR.DIE=1, a Decrementer interrupt is taken.
TCR.DIE:
0 Disable Decrementer interrupt
1 Enable Decrementer interrupt
Decrement to one and stop on zero
If TCR.ARE=0, TSR.DIS is set to 1, the value 0x0000_0000 is then placed into
the DEC, and the Decrementer stops decrementing.
A Decrementer interrupt occurs when no higher priority interrupt exists, a
Decrementer exception exists, and the exception is enabled.
The interrupt is enabled by TCR.DIE=1 and MSR.EE=1.
See Section 7.6.12, “Decrementer Interrupt” on page 1021 for details of
register behavior caused by the Decrementer inter- rupt.
7.6.12 Decrementer Interrupt
A Decrementer interrupt occurs when no higher priority interrupt exists (see
Section 7.9 on page 1037), a Decrementer exception exists (TSR.DIS=1), and the
exception is enabled. The interrupt is enabled by TCR.DIE=1 and MSR.EE=1. See
Section 9.3 on page 1047.
Software is responsible for clearing the Decre- menter exception status prior
to re-enabling the MSR.EE bit in order to avoid another redundant Decrementer
interrupt. To clear the Decrementer exception, the interrupt handling routine
must clear TSR.DIS. Clearing is done by writing a word to TSR using mtspr with
a 1 in any bit position that is to be cleared and 0 in all other bit positions.
The write-data to the TSR is not direct data, but a mask. A 1 causes the bit to
be cleared, and a 0 has no effect.
To me that sounds as if the decrementer interrupt gets injected only when
TSR.DIS=1, TCR.DIE=1 and MSR.EE=1. Unsetting any of these bits stops the
interrupt from being delivered.
Scott, can you please check up with the hardware guys if this is correct?
Alex
