From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:41440)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <tommusta@gmail.com>) id 1WXyFk-0000Ia-HK
	for qemu-devel@nongnu.org; Wed, 09 Apr 2014 15:33:53 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <tommusta@gmail.com>) id 1WXyFb-0000aE-DT
	for qemu-devel@nongnu.org; Wed, 09 Apr 2014 15:33:44 -0400
Message-ID: <5345A080.8040101@gmail.com>
Date: Wed, 09 Apr 2014 14:33:20 -0500
From: Tom Musta <tommusta@gmail.com>
MIME-Version: 1.0
References: <1396817714-26768-1-git-send-email-agraf@suse.de>
	<5344545C.7060608@gmail.com> <534454CD.2060500@suse.de>
In-Reply-To: <534454CD.2060500@suse.de>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v2] PPC: Clean up DECR implementation
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alexander Graf <agraf@suse.de>
Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org

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.