Re: [Qemu-ppc] [Qemu-devel] [PATCH v2] PPC: Clean up DECR implementation

[Alexander Graf]

> Am 09.04.2014 um 21:33 schrieb Tom Musta <address@hidden>:
> 
>> On 4/8/2014 2:58 PM, Alexander Graf wrote:
>>> On 04/08/2014 09:56 PM, Tom Musta wrote:
>>> On 4/6/2014 3:55 PM, Alexander Graf wrote:
>>> <snip>
>>> 
>>>> @@ -806,6 +838,10 @@ clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, 
>>>> uint32_t freq)
>>>>      tb_env = g_malloc0(sizeof(ppc_tb_t));
>>>>      env->tb_env = tb_env;
>>>>      tb_env->flags = PPC_DECR_UNDERFLOW_TRIGGERED;
>>>> +    if (env->insns_flags & PPC_SEGMENT_64B) {
>>>> +        /* All Book3S 64bit CPUs implement level based DEC logic */
>>>> +        tb_env->flags |= PPC_DECR_UNDERFLOW_LEVEL;
>>>> +    }
>>>>      /* Create new timer */
>>>>      tb_env->decr_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, 
>>>> &cpu_ppc_decr_cb, cpu);
>>>>      if (0) {
>>> Equating Book3S with PPC_SEGMENT_64B is clever ... is it too clever?  
>>> Especially since
>>> the SLB Bridge is in the phased-out category and consequently we should 
>>> expect future
>>> Book3S implementations to not support this instruction category.
>> 
>> Maybe it's too clever :). I'm very open to suggestions on how to figure this 
>> out otherwise. Or maybe we should just rework the way timers get created and 
>> make them be part of the core itself?
>> 
>> 
>> Alex
> 
> A somewhat more practical approach than redesigning timer init:  The phrasing 
> introduced into Book3S
> that corresponds to your UNDERFLOW_LEVEL flag has existed at least since ISA 
> 2.03.  And 2.03 introduced
> some new features, like SPE and Altivec.  So ...

e6500 supports Altivec :)

> 
>    if (env->insns_flags & (PPC_SEGMENT_64B | PPC_SPE | PPC_ALTIVEC)) {
>        /* All Book3S 64bit CPUs implement level based DEC logic */
>        tb_env->flags |= PPC_DECR_UNDERFLOW_LEVEL;
>    }
> 
> would catch a few more.  I'm not sure we get into this code for Book3E 
> machines, but if you are worried
> about that you could also ensure that insns_flags doesn't have PPC_BOOKE on.

Phew, I'm not quite sure. We can do it as an interim solution, but really the 
timer is a core property rather than a machine device :). So eventually this 
should get reworked.


Alex