From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christian Ehrhardt Subject: Re: [PATCH 2/5] kvmtrace: make cycle calculation architecture aware Date: Wed, 09 Jul 2008 10:25:43 +0200 Message-ID: <48747607.2010509@linux.vnet.ibm.com> References: <1215439013-11480-1-git-send-email-ehrhardt@linux.vnet.ibm.com> <1215439013-11480-3-git-send-email-ehrhardt@linux.vnet.ibm.com> <1215445501.11175.4.camel@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, avi@qumranet.com, kvm-ppc@vger.kernel.org To: Hollis Blanchard Return-path: Received: from mtagate4.uk.ibm.com ([195.212.29.137]:59406 "EHLO mtagate4.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750804AbYGII0F (ORCPT ); Wed, 9 Jul 2008 04:26:05 -0400 In-Reply-To: <1215445501.11175.4.camel@localhost.localdomain> Sender: kvm-owner@vger.kernel.org List-ID: Hollis Blanchard wrote: > On Mon, 2008-07-07 at 15:56 +0200, ehrhardt@linux.vnet.ibm.com wrote: > =20 >> From: Christian Ehrhardt >> >> The current implementation of kvmtrace uses always a 64 bit cycle va= riable, >> but get_cycles() which is used to fill it is "unsigned long" which m= ight be 32 >> bit. >> This reduces the accuracy e.g. on embedded powerpc since we would ha= ve a 64bit >> value but get_cycle() only returns the low 32 bit. >> To solve that this patch introduces kvm_arch_trace_cycles() which al= lows us >> to make this calculation architecture aware. That way every architec= ture can >> insert whatever fits best for their "kvmtrace cycle counter". >> >> Signed-off-by: Christian Ehrhardt >> =20 > > Just one comment below... > > BTW, because this breaks the ia64 and s390 builds, it might be nice t= o > CC the appropriate list/maintainer directly. > > =20 you're right - I'll add dummy stubs for ia64&s390 using the classic=20 get_cycle() call and resubmit the series. >> --- >> >> [diffstat] >> arch/powerpc/kvm/powerpc.c | 25 +++++++++++++++++++++++++ >> arch/x86/kvm/x86.c | 5 +++++ >> include/linux/kvm_host.h | 2 ++ >> virt/kvm/kvm_trace.c | 2 +- >> 4 files changed, 33 insertions(+), 1 deletion(-) >> >> [diff] >> >> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c >> --- a/arch/powerpc/kvm/powerpc.c >> +++ b/arch/powerpc/kvm/powerpc.c >> @@ -521,6 +521,31 @@ >> return r; >> } >> >> +/* >> + * We need a 64 bit value here and the default get_cycles has only = 32bit (tbl) >> + * on 32bit ppc. Since we have a 64bit counter for that we provide = it here in >> + * full resolution for the trace records. >> +*/ >> +__u64 kvm_arch_trace_cycles() >> +{ >> + unsigned long ruval; >> + unsigned long ruval2; >> + unsigned long rlval; >> + >> + /* get a consistant pair of upper/lower timebase (no wrap occured)= */ >> + asm volatile( >> + "loop:\n" >> + "mftbu %0\n" >> + "mftbl %1\n" >> + "mftbu %2\n" >> + "cmpw %0, %2\n" >> + "bne loop" >> + : "=3Dr" (ruval), "=3Dr" (rlval), "=3Dr" (ruval2) >> + ); >> + >> + return (((__u64)ruval) << 32) | rlval; >> +} >> =20 > > You should use get_tb() here (see asm-powerpc/time.h). > =20 yep does the same, I didn't see it and coded the asm from the processor= =20 manual hint at the time base register. Anyway - I'll change the code to get_tb() - thanks for the hint. --=20 Gr=FCsse / regards,=20 Christian Ehrhardt IBM Linux Technology Center, Open Virtualization