Re: [Qemu-devel] [Qemu-ppc] [RESEND][PATCH] booke timers

[Alexander Graf <agraf@suse.de>]

Alexander Graf wrote:
> Fabien Chouteau wrote:
>   
>> On 12/09/2011 19:23, Scott Wood wrote:
>>   
>>     
>>> On 09/09/2011 09:58 AM, Alexander Graf wrote:
>>>     
>>>       
>>>> On 09.09.2011, at 16:22, Fabien Chouteau wrote:
>>>>       
>>>>         
>>>>> 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:
>>>>       
>>>>         
>>> [snip]
>>>     
>>>       
>>>> 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?
>>>>       
>>>>         
>>> This is how I've always understood it to work (assuming the interrupt
>>> hasn't already been delivered, of course).  Fabien, do you have real
>>> hardware that you see behave the way you describe?
>>>
>>>     
>>>       
>> No I don't, it was just my understanding of Book-E documentation. I've tried
>> your solution (below) with VxWorks, and it works like a charm.
>>
>> static void booke_update_irq(CPUState *env)
>> {
>>     ppc_set_irq(env, PPC_INTERRUPT_DECR,
>>                 (env->spr[SPR_BOOKE_TSR] & TSR_DIS
>>                  && env->spr[SPR_BOOKE_TCR] & TCR_DIE));
>>
>>     ppc_set_irq(env, PPC_INTERRUPT_WDT,
>>                 (env->spr[SPR_BOOKE_TSR] & TSR_WIS
>>                  && env->spr[SPR_BOOKE_TCR] & TCR_WIE));
>>
>>     ppc_set_irq(env, PPC_INTERRUPT_FIT,
>>                 (env->spr[SPR_BOOKE_TSR] & TSR_FIS
>>                  && env->spr[SPR_BOOKE_TCR] & TCR_FIE));
>> }
>>   
>>     
>
> Awesome! Please also check on MSR.EE and send a new patch then :)
>   

Ah, the EE check is in target-ppc/helper.c:ppc_hw_interrupt. Very
confusing (and probably wrong because it could generate spurious
interrupts), but it should be enough for now.


Alex