From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hollis Blanchard Subject: Re: [PATCH 2/5] kvmtrace: make cycle calculation architecture aware Date: Mon, 07 Jul 2008 10:45:01 -0500 Message-ID: <1215445501.11175.4.camel@localhost.localdomain> References: <1215439013-11480-1-git-send-email-ehrhardt@linux.vnet.ibm.com> <1215439013-11480-3-git-send-email-ehrhardt@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, avi@qumranet.com, kvm-ppc@vger.kernel.org To: ehrhardt@linux.vnet.ibm.com Return-path: Received: from e6.ny.us.ibm.com ([32.97.182.146]:35593 "EHLO e6.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753439AbYGGPpI (ORCPT ); Mon, 7 Jul 2008 11:45:08 -0400 In-Reply-To: <1215439013-11480-3-git-send-email-ehrhardt@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, 2008-07-07 at 15:56 +0200, ehrhardt@linux.vnet.ibm.com wrote: > From: Christian Ehrhardt > > The current implementation of kvmtrace uses always a 64 bit cycle variable, > but get_cycles() which is used to fill it is "unsigned long" which might be 32 > bit. > This reduces the accuracy e.g. on embedded powerpc since we would have a 64bit > value but get_cycle() only returns the low 32 bit. > To solve that this patch introduces kvm_arch_trace_cycles() which allows us > to make this calculation architecture aware. That way every architecture can > insert whatever fits best for their "kvmtrace cycle counter". > > Signed-off-by: Christian Ehrhardt Just one comment below... BTW, because this breaks the ia64 and s390 builds, it might be nice to CC the appropriate list/maintainer directly. > --- > > [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" > + : "=r" (ruval), "=r" (rlval), "=r" (ruval2) > + ); > + > + return (((__u64)ruval) << 32) | rlval; > +} You should use get_tb() here (see asm-powerpc/time.h). -- Hollis Blanchard IBM Linux Technology Center