From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59227) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zi3Rt-0001Il-6q for qemu-devel@nongnu.org; Fri, 02 Oct 2015 12:44:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zi3Rn-0001uB-8S for qemu-devel@nongnu.org; Fri, 02 Oct 2015 12:44:45 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:40109) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zi3Rm-0001tm-Vt for qemu-devel@nongnu.org; Fri, 02 Oct 2015 12:44:39 -0400 Message-ID: <560EB472.1000901@codeaurora.org> Date: Fri, 02 Oct 2015 12:44:34 -0400 From: Christopher Covington MIME-Version: 1.0 References: <1430417667-4245-5-git-send-email-christopher.covington@linaro.org> <1443123824-26866-1-git-send-email-cov@codeaurora.org> <560A9B3B.3090407@codeaurora.org> In-Reply-To: <560A9B3B.3090407@codeaurora.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] target-arm: Use common CPU cycle infrastructure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , Paolo Bonzini Cc: Laurent Vivier , Peter Crosthwaite , "qemu-devel@nongnu.org Developers" , Alistair Francis On 09/29/2015 10:07 AM, Christopher Covington wrote: > On 09/28/2015 06:05 PM, Alistair Francis wrote: >> On Thu, Sep 24, 2015 at 12:43 PM, Christopher Covington >> wrote: >>> cpu_get_ticks() provides a common interface across targets for >>> calculating CPU cycles. Using this fixes PMCCNTR reads when -icount >>> is specified (previously a non-increasing value was returned). >>> >>> Signed-off-by: Christopher Covington >>> --- >>> target-arm/helper.c | 9 +++------ >>> 1 file changed, 3 insertions(+), 6 deletions(-) >>> >>> diff --git a/target-arm/helper.c b/target-arm/helper.c >>> index 7dc49cb..32923fb 100644 >>> --- a/target-arm/helper.c >>> +++ b/target-arm/helper.c >>> @@ -729,8 +729,7 @@ void pmccntr_sync(CPUARMState *env) >>> { >>> uint64_t temp_ticks; >>> >>> - temp_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL), >>> - get_ticks_per_sec(), 1000000); >>> + temp_ticks = cpu_get_ticks(); > >> Also I don't think this is correct. cpu_get_ticks() returns the host >> CPU cycle counter, when in this case we want the guest cycles. > > I too find the use of host CPU cycles quite perplexing. Paolo suggested it > [1]. Maybe there are timeouts in some software that tend to work better in > such a mode. Perhaps it is faster, although my intuition is that it's just > providing a facade of frequency scaling to the guest. > > 1. https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg00162.html > > I like to declare guest instructions per guest CPU cycles = 1. As I understand > it, an "-icount 0" pair of parameters is how to do this in QEMU for x86. I'd > like it to work for ARM. > > I wrote a simple assembly test case which I'm working on porting it to the > kvm-unit-tests framework. In the non-icount case, I saw roughly the same order > of magnitude for guest IPC before and after the patch. I'd like to also write > CPU frequency (guest CPU cycles per generic timer guest seconds) and (M)IPS > (guest instructions per generic timer guest seconds) tests. I've sent out the CPI test case and while exercising it I noticed that Laurent's patch fixed -icount. So my original goal has been accomplished. I'm happy to rebase this patch if the authorities (Peter Maydell?) think using cpu_get_ticks() is the right thing to do. Otherwise I'll probably try to move on to support for the instructions event in the ARM PMU. Thanks, Christopher Covington -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project