Re: [PATCH 2/5] kvmtrace: make cycle calculation architecture aware

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hollis Blanchard wrote:
On Mon, 2008-07-07 at 15:56 +0200, ehrhardt@xxxxxxxxxxxxxxxxxx wrote:
From: Christian Ehrhardt <ehrhardt@xxxxxxxxxxxxxxxxxx>

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 <ehrhardt@xxxxxxxxxxxxxxxxxx>

Just one comment below...

BTW, because this breaks the ia64 and s390 builds, it might be nice to
CC the appropriate list/maintainer directly.

you're right - I'll add dummy stubs for ia64&s390 using the classic 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"
+		: "=r" (ruval), "=r" (rlval), "=r" (ruval2)
+	);
+
+	return (((__u64)ruval) << 32) | rlval;
+}

You should use get_tb() here (see asm-powerpc/time.h).
yep does the same, I didn't see it and coded the asm from the processor manual hint at the time base register.
Anyway - I'll change the code to get_tb() - thanks for the hint.

--

Grüsse / regards, Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [KVM Development]     [KVM ARM]     [KVM ia64]     [Linux Virtualization]     [Linux USB Devel]     [Linux Video]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux