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