* Re: kvm trace support for ppc
@ 2008-05-14 21:37 ` Hollis Blanchard
0 siblings, 0 replies; 17+ messages in thread
From: Hollis Blanchard @ 2008-05-14 21:37 UTC (permalink / raw)
To: Tan, Li; +Cc: kvm-devel, kvm-ppc-devel
On Tuesday 13 May 2008 03:06:19 Tan, Li wrote:
> Hollisb,
> I have 2 more questions:
> 1. seems record won't be overwritten because current code is as following:
> /*
> * The relay channel is used in "no-overwrite" mode, it keeps trace of how
> * many times we encountered a full subbuffer, to tell user space app the
> * lost records there were.
> */
> static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf,
> void *prev_subbuf, size_t prev_padding)
> {
> struct kvm_trace *kt;
>
> if (!relay_buf_full(buf))
> return 1;
>
> kt = buf->chan->private_data;
> atomic_inc(&kt->lost_records);
>
> return 0;
> }
>
> 2. Then needn't expose debugfs entry "kvmtrace-metadata", we can use
existing relayfs to output struct metadata with kmagic, if we update code as
following?
> static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf,
> void *prev_subbuf, size_t prev_padding)
> {
> struct kvm_trace *kt;
>
> if (!relay_buf_full(buf))
> {
> if (!prev_subbuf) {
> //here is executed only once (when the channel is opened)
> subbuf_start_reserve(buf, sizeof(struct metadata));
> ((struct metadata *)subbuf)->kmagic = 0x1234;
> }
>
> return 1;
> }
>
> kt = buf->chan->private_data;
> atomic_inc(&kt->lost_records);
>
> return 0;
> }
Ah, I didn't understand what the "lost records" handling was about. Given that
it won't be lost, it would be OK for the kernel to export the header, and in
that case I guess you would want it to be the same size as the other records.
I'm not sure how I feel about that from a layering point of view, but at
least it would be functional.
--
Hollis Blanchard
IBM Linux Technology Center
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [kvm-ppc-devel] kvm trace support for ppc
2008-05-14 21:37 ` Hollis Blanchard
@ 2008-05-15 1:20 ` Tan, Li
-1 siblings, 0 replies; 17+ messages in thread
From: Tan, Li @ 2008-05-15 1:20 UTC (permalink / raw)
To: Hollis Blanchard; +Cc: kvm-devel, kvm-ppc-devel
[-- Attachment #1: Type: text/plain, Size: 2981 bytes --]
-----Original Message-----
From: Hollis Blanchard [mailto:hollisb@us.ibm.com]
Sent: 2008年5月15日 5:37
To: Tan, Li
Cc: kvm-devel; kvm-ppc-devel@lists.sourceforge.net
Subject: Re: kvm trace support for ppc
On Tuesday 13 May 2008 03:06:19 Tan, Li wrote:
> Hollisb,
> I have 2 more questions:
> 1. seems record won't be overwritten because current code is as following:
> /*
> * The relay channel is used in "no-overwrite" mode, it keeps trace of how
> * many times we encountered a full subbuffer, to tell user space app the
> * lost records there were.
> */
> static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf,
> void *prev_subbuf, size_t prev_padding)
> {
> struct kvm_trace *kt;
>
> if (!relay_buf_full(buf))
> return 1;
>
> kt = buf->chan->private_data;
> atomic_inc(&kt->lost_records);
>
> return 0;
> }
>
> 2. Then needn't expose debugfs entry "kvmtrace-metadata", we can use
existing relayfs to output struct metadata with kmagic, if we update code as
following?
> static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf,
> void *prev_subbuf, size_t prev_padding)
> {
> struct kvm_trace *kt;
>
> if (!relay_buf_full(buf))
> {
> if (!prev_subbuf) {
> //here is executed only once (when the channel is opened)
> subbuf_start_reserve(buf, sizeof(struct metadata));
> ((struct metadata *)subbuf)->kmagic = 0x1234;
> }
>
> return 1;
> }
>
> kt = buf->chan->private_data;
> atomic_inc(&kt->lost_records);
>
> return 0;
> }
Ah, I didn't understand what the "lost records" handling was about. Given that
it won't be lost, it would be OK for the kernel to export the header, and in
that case I guess you would want it to be the same size as the other records.
I'm not sure how I feel about that from a layering point of view, but at
least it would be functional.
[tan] The relay channel is used in "no-overwrite" mode, so it will lost any new items if encounters a full subbuffer, "lost records" is to count lost records. Yes, metadata is the same size as the other records.
This solution needn't change kvmtrace user app, only need to change kvmtrace_format.pl like following:
if i == 1:
line = sys.stdin.read(struct.calcsize(HDRREC))
(kmgc, umgc, mgcpad) = struct.unpack(HDRREC, line)
if kmgc == 0x1234:
rev = False
else:
rev = True
continue
# If i > 1:
line = sys.stdin.read(struct.calcsize(HDRREC))
(event, pid, vcpu_id) = struct.unpack(HDRREC, line)
...
if rev:
event = reverse_int(event)
pid = reverse_int(pid)
vcpu_id = reverse_int(vcpu_id)
tsc = reverse_qword(tsc)
d1 = reverse_int(d1)
d2 = reverse_int(d2)
d3 = reverse_int(d3)
d4 = reverse_int(d4)
d5 = reverse_int(d5)
--
Hollis Blanchard
IBM Linux Technology Center
[-- Attachment #2: Type: text/plain, Size: 230 bytes --]
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
[-- Attachment #3: Type: text/plain, Size: 170 bytes --]
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: kvm trace support for ppc
@ 2008-05-15 1:20 ` Tan, Li
0 siblings, 0 replies; 17+ messages in thread
From: Tan, Li @ 2008-05-15 1:20 UTC (permalink / raw)
To: Hollis Blanchard; +Cc: kvm-devel, kvm-ppc-devel
[-- Attachment #1: Type: text/plain, Size: 2981 bytes --]
-----Original Message-----
From: Hollis Blanchard [mailto:hollisb@us.ibm.com]
Sent: 2008年5月15日 5:37
To: Tan, Li
Cc: kvm-devel; kvm-ppc-devel@lists.sourceforge.net
Subject: Re: kvm trace support for ppc
On Tuesday 13 May 2008 03:06:19 Tan, Li wrote:
> Hollisb,
> I have 2 more questions:
> 1. seems record won't be overwritten because current code is as following:
> /*
> * The relay channel is used in "no-overwrite" mode, it keeps trace of how
> * many times we encountered a full subbuffer, to tell user space app the
> * lost records there were.
> */
> static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf,
> void *prev_subbuf, size_t prev_padding)
> {
> struct kvm_trace *kt;
>
> if (!relay_buf_full(buf))
> return 1;
>
> kt = buf->chan->private_data;
> atomic_inc(&kt->lost_records);
>
> return 0;
> }
>
> 2. Then needn't expose debugfs entry "kvmtrace-metadata", we can use
existing relayfs to output struct metadata with kmagic, if we update code as
following?
> static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf,
> void *prev_subbuf, size_t prev_padding)
> {
> struct kvm_trace *kt;
>
> if (!relay_buf_full(buf))
> {
> if (!prev_subbuf) {
> //here is executed only once (when the channel is opened)
> subbuf_start_reserve(buf, sizeof(struct metadata));
> ((struct metadata *)subbuf)->kmagic = 0x1234;
> }
>
> return 1;
> }
>
> kt = buf->chan->private_data;
> atomic_inc(&kt->lost_records);
>
> return 0;
> }
Ah, I didn't understand what the "lost records" handling was about. Given that
it won't be lost, it would be OK for the kernel to export the header, and in
that case I guess you would want it to be the same size as the other records.
I'm not sure how I feel about that from a layering point of view, but at
least it would be functional.
[tan] The relay channel is used in "no-overwrite" mode, so it will lost any new items if encounters a full subbuffer, "lost records" is to count lost records. Yes, metadata is the same size as the other records.
This solution needn't change kvmtrace user app, only need to change kvmtrace_format.pl like following:
if i == 1:
line = sys.stdin.read(struct.calcsize(HDRREC))
(kmgc, umgc, mgcpad) = struct.unpack(HDRREC, line)
if kmgc == 0x1234:
rev = False
else:
rev = True
continue
# If i > 1:
line = sys.stdin.read(struct.calcsize(HDRREC))
(event, pid, vcpu_id) = struct.unpack(HDRREC, line)
...
if rev:
event = reverse_int(event)
pid = reverse_int(pid)
vcpu_id = reverse_int(vcpu_id)
tsc = reverse_qword(tsc)
d1 = reverse_int(d1)
d2 = reverse_int(d2)
d3 = reverse_int(d3)
d4 = reverse_int(d4)
d5 = reverse_int(d5)
--
Hollis Blanchard
IBM Linux Technology Center
[-- Attachment #2: Type: text/plain, Size: 230 bytes --]
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
[-- Attachment #3: Type: text/plain, Size: 158 bytes --]
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [kvm-ppc-devel] kvm trace support for ppc
2008-05-14 21:37 ` Hollis Blanchard
@ 2008-05-16 6:26 ` Tan, Li
-1 siblings, 0 replies; 17+ messages in thread
From: Tan, Li @ 2008-05-16 6:26 UTC (permalink / raw)
To: Hollis Blanchard; +Cc: kvm-devel, kvm-ppc-devel
[-- Attachment #1: Type: text/plain, Size: 3726 bytes --]
struct metadata {
u32 kmagic; /* stores kernel defined metadata read from debugfs entry*/
u32 umagic; /* stores userspace tool defined metadata */
u32 extra; /* it is redundant, only use to fit into record. */
}
seems umagic and extra are no need now.
I guess they were designed as record pad to keep the format of trace log transparent to kvmtrace_format, but now kvmtrace_format has to know the format: the first record is metadata
So I’ll change to
struct metadata {
u32 kmagic; /* stores kernel defined metadata */
}
Tan Li
-----Original Message-----
From: Tan, Li
Sent: 2008年5月15日 9:20
To: 'Hollis Blanchard'
Cc: kvm-devel; kvm-ppc-devel@lists.sourceforge.net
Subject: RE: kvm trace support for ppc
-----Original Message-----
From: Hollis Blanchard [mailto:hollisb@us.ibm.com]
Sent: 2008年5月15日 5:37
To: Tan, Li
Cc: kvm-devel; kvm-ppc-devel@lists.sourceforge.net
Subject: Re: kvm trace support for ppc
On Tuesday 13 May 2008 03:06:19 Tan, Li wrote:
> Hollisb,
> I have 2 more questions:
> 1. seems record won't be overwritten because current code is as following:
> /*
> * The relay channel is used in "no-overwrite" mode, it keeps trace of how
> * many times we encountered a full subbuffer, to tell user space app the
> * lost records there were.
> */
> static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf,
> void *prev_subbuf, size_t prev_padding)
> {
> struct kvm_trace *kt;
>
> if (!relay_buf_full(buf))
> return 1;
>
> kt = buf->chan->private_data;
> atomic_inc(&kt->lost_records);
>
> return 0;
> }
>
> 2. Then needn't expose debugfs entry "kvmtrace-metadata", we can use
existing relayfs to output struct metadata with kmagic, if we update code as
following?
> static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf,
> void *prev_subbuf, size_t prev_padding)
> {
> struct kvm_trace *kt;
>
> if (!relay_buf_full(buf))
> {
> if (!prev_subbuf) {
> //here is executed only once (when the channel is opened)
> subbuf_start_reserve(buf, sizeof(struct metadata));
> ((struct metadata *)subbuf)->kmagic = 0x1234;
> }
>
> return 1;
> }
>
> kt = buf->chan->private_data;
> atomic_inc(&kt->lost_records);
>
> return 0;
> }
Ah, I didn't understand what the "lost records" handling was about. Given that
it won't be lost, it would be OK for the kernel to export the header, and in
that case I guess you would want it to be the same size as the other records.
I'm not sure how I feel about that from a layering point of view, but at
least it would be functional.
[tan] The relay channel is used in "no-overwrite" mode, so it will lost any new items if encounters a full subbuffer, "lost records" is to count lost records. Yes, metadata is the same size as the other records.
This solution needn't change kvmtrace user app, only need to change kvmtrace_format.pl like following:
if i == 1:
line = sys.stdin.read(struct.calcsize(HDRREC))
(kmgc, umgc, mgcpad) = struct.unpack(HDRREC, line)
if kmgc == 0x1234:
rev = False
else:
rev = True
continue
# If i > 1:
line = sys.stdin.read(struct.calcsize(HDRREC))
(event, pid, vcpu_id) = struct.unpack(HDRREC, line)
...
if rev:
event = reverse_int(event)
pid = reverse_int(pid)
vcpu_id = reverse_int(vcpu_id)
tsc = reverse_qword(tsc)
d1 = reverse_int(d1)
d2 = reverse_int(d2)
d3 = reverse_int(d3)
d4 = reverse_int(d4)
d5 = reverse_int(d5)
--
Hollis Blanchard
IBM Linux Technology Center
[-- Attachment #2: Type: text/plain, Size: 230 bytes --]
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
[-- Attachment #3: Type: text/plain, Size: 170 bytes --]
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: kvm trace support for ppc
@ 2008-05-16 6:26 ` Tan, Li
0 siblings, 0 replies; 17+ messages in thread
From: Tan, Li @ 2008-05-16 6:26 UTC (permalink / raw)
To: Hollis Blanchard; +Cc: kvm-devel, kvm-ppc-devel
[-- Attachment #1: Type: text/plain, Size: 3726 bytes --]
struct metadata {
u32 kmagic; /* stores kernel defined metadata read from debugfs entry*/
u32 umagic; /* stores userspace tool defined metadata */
u32 extra; /* it is redundant, only use to fit into record. */
}
seems umagic and extra are no need now.
I guess they were designed as record pad to keep the format of trace log transparent to kvmtrace_format, but now kvmtrace_format has to know the format: the first record is metadata
So I’ll change to
struct metadata {
u32 kmagic; /* stores kernel defined metadata */
}
Tan Li
-----Original Message-----
From: Tan, Li
Sent: 2008年5月15日 9:20
To: 'Hollis Blanchard'
Cc: kvm-devel; kvm-ppc-devel@lists.sourceforge.net
Subject: RE: kvm trace support for ppc
-----Original Message-----
From: Hollis Blanchard [mailto:hollisb@us.ibm.com]
Sent: 2008年5月15日 5:37
To: Tan, Li
Cc: kvm-devel; kvm-ppc-devel@lists.sourceforge.net
Subject: Re: kvm trace support for ppc
On Tuesday 13 May 2008 03:06:19 Tan, Li wrote:
> Hollisb,
> I have 2 more questions:
> 1. seems record won't be overwritten because current code is as following:
> /*
> * The relay channel is used in "no-overwrite" mode, it keeps trace of how
> * many times we encountered a full subbuffer, to tell user space app the
> * lost records there were.
> */
> static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf,
> void *prev_subbuf, size_t prev_padding)
> {
> struct kvm_trace *kt;
>
> if (!relay_buf_full(buf))
> return 1;
>
> kt = buf->chan->private_data;
> atomic_inc(&kt->lost_records);
>
> return 0;
> }
>
> 2. Then needn't expose debugfs entry "kvmtrace-metadata", we can use
existing relayfs to output struct metadata with kmagic, if we update code as
following?
> static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf,
> void *prev_subbuf, size_t prev_padding)
> {
> struct kvm_trace *kt;
>
> if (!relay_buf_full(buf))
> {
> if (!prev_subbuf) {
> //here is executed only once (when the channel is opened)
> subbuf_start_reserve(buf, sizeof(struct metadata));
> ((struct metadata *)subbuf)->kmagic = 0x1234;
> }
>
> return 1;
> }
>
> kt = buf->chan->private_data;
> atomic_inc(&kt->lost_records);
>
> return 0;
> }
Ah, I didn't understand what the "lost records" handling was about. Given that
it won't be lost, it would be OK for the kernel to export the header, and in
that case I guess you would want it to be the same size as the other records.
I'm not sure how I feel about that from a layering point of view, but at
least it would be functional.
[tan] The relay channel is used in "no-overwrite" mode, so it will lost any new items if encounters a full subbuffer, "lost records" is to count lost records. Yes, metadata is the same size as the other records.
This solution needn't change kvmtrace user app, only need to change kvmtrace_format.pl like following:
if i == 1:
line = sys.stdin.read(struct.calcsize(HDRREC))
(kmgc, umgc, mgcpad) = struct.unpack(HDRREC, line)
if kmgc == 0x1234:
rev = False
else:
rev = True
continue
# If i > 1:
line = sys.stdin.read(struct.calcsize(HDRREC))
(event, pid, vcpu_id) = struct.unpack(HDRREC, line)
...
if rev:
event = reverse_int(event)
pid = reverse_int(pid)
vcpu_id = reverse_int(vcpu_id)
tsc = reverse_qword(tsc)
d1 = reverse_int(d1)
d2 = reverse_int(d2)
d3 = reverse_int(d3)
d4 = reverse_int(d4)
d5 = reverse_int(d5)
--
Hollis Blanchard
IBM Linux Technology Center
[-- Attachment #2: Type: text/plain, Size: 230 bytes --]
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
[-- Attachment #3: Type: text/plain, Size: 158 bytes --]
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] [RFC][PATCH] kvm: kvmtrace: kvm_trace in kernel for supporting big_endian
2008-05-14 21:37 ` Hollis Blanchard
` (2 preceding siblings ...)
(?)
@ 2008-05-20 6:53 ` Tan, Li
2008-05-20 7:03 ` [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format " Tan, Li
2008-05-20 16:25 ` [PATCH] [RFC][PATCH] kvm: kvmtrace: kvm_trace in kernel " Avi Kivity
-1 siblings, 2 replies; 17+ messages in thread
From: Tan, Li @ 2008-05-20 6:53 UTC (permalink / raw)
To: kvm
>From 63283d32ad5faf0845fa0358fd71d119f4dc0e3d Mon Sep 17 00:00:00 2001
From: Tan Li <li.tan@intel.com>
Date: Mon, 19 May 2008 17:18:54 +0800
Subject: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvm_trace in kernel for
supporting big_endian
Currently kvmtrace is not portable. This will prevent from copying a
trace file from big-endian target to little-endian workstation for
analysis.
In the patch, kernel outputs metadata containing a magic number to trace
log.
Signed-off-by: Tan Li <li.tan@intel.com>
---
include/linux/kvm.h | 4 ++--
virt/kvm/kvm_trace.c | 18 ++++++++++++------
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index a281afe..ca08cb1 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -294,14 +294,14 @@ struct kvm_trace_rec {
__u32 vcpu_id;
union {
struct {
- __u32 cycle_lo, cycle_hi;
+ __u64 cycle_u64;
__u32 extra_u32[KVM_TRC_EXTRA_MAX];
} cycle;
struct {
__u32 extra_u32[KVM_TRC_EXTRA_MAX];
} nocycle;
} u;
-};
+} __attribute__((packed));
#define KVMIO 0xAE
diff --git a/virt/kvm/kvm_trace.c b/virt/kvm/kvm_trace.c
index 0e49547..58141f3 100644
--- a/virt/kvm/kvm_trace.c
+++ b/virt/kvm/kvm_trace.c
@@ -72,11 +72,7 @@ static void kvm_add_trace(void *probe_private, void
*call_data,
rec.cycle_in = p->cycle_in;
if (rec.cycle_in) {
- u64 cycle = 0;
-
- cycle = get_cycles();
- rec.u.cycle.cycle_lo = (u32)cycle;
- rec.u.cycle.cycle_hi = (u32)(cycle >> 32);
+ rec.u.cycle.cycle_u64 = get_cycles();
for (i = 0; i < rec.extra_u32; i++)
rec.u.cycle.extra_u32[i] = va_arg(*args, u32);
@@ -114,8 +110,18 @@ static int kvm_subbuf_start_callback(struct
rchan_buf *buf, void *subbuf,
{
struct kvm_trace *kt;
- if (!relay_buf_full(buf))
+ if (!relay_buf_full(buf)) {
+ if (!prev_subbuf) {
+ /*
+ * executed only once when the channel is opened
+ * save metadata as first record
+ */
+ subbuf_start_reserve(buf, sizeof(u32));
+ *(u32 *)subbuf = 0x12345678;
+ }
+
return 1;
+ }
kt = buf->chan->private_data;
atomic_inc(&kt->lost_records);
--
1.5.3.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for supporting big_endian
2008-05-20 6:53 ` [PATCH] [RFC][PATCH] kvm: kvmtrace: kvm_trace in kernel for supporting big_endian Tan, Li
@ 2008-05-20 7:03 ` Tan, Li
2008-05-20 16:22 ` Avi Kivity
2008-05-20 16:25 ` [PATCH] [RFC][PATCH] kvm: kvmtrace: kvm_trace in kernel " Avi Kivity
1 sibling, 1 reply; 17+ messages in thread
From: Tan, Li @ 2008-05-20 7:03 UTC (permalink / raw)
To: kvm
>From 9d39264bdfb00a1a717074e486a948a578547c50 Mon Sep 17 00:00:00 2001
From: Tan Li <li.tan@intel.com>
Date: Tue, 20 May 2008 09:14:41 +0800
Subject: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for
supporting big_endian
Currently kvmtrace is not portable, and prevent from copying a trace
file from big-endian target to little-endian workstation for analysis.
In the patch, kvmtrace_format reads and checks the magic number from
trace log. if needed, then change bytes order of all fields in records
followed.
Signed-off-by: Tan Li <li.tan@intel.com>
---
user/kvmtrace_format | 46
++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 46 insertions(+), 0 deletions(-)
diff --git a/user/kvmtrace_format b/user/kvmtrace_format
index 9e7cfd4..386d855 100755
--- a/user/kvmtrace_format
+++ b/user/kvmtrace_format
@@ -55,6 +55,22 @@ def read_defs(defs_file):
return defs
+def reverse_bytes(origin, bytes_cnt):
+ i = 0
+ rtn = 0L
+ while i < bytes_cnt:
+ i = i + 1
+ b = origin & 0xff
+ origin = origin >> 8
+ rtn = (rtn << 8) + b
+ return rtn
+
+def reverse_int(origin):
+ return reverse_bytes(origin, 4)
+
+def reverse_qword(origin):
+ return reverse_bytes(origin, 8)
+
def sighand(x,y):
global interrupted
interrupted = 1
@@ -100,18 +116,40 @@ D2REC = "II"
D3REC = "III"
D4REC = "IIII"
D5REC = "IIIII"
+KMAGIC = "I"
last_tsc = 0
+reverse_flag = 0
i=0
while not interrupted:
try:
i=i+1
+
+ if i == 1:
+ line = sys.stdin.read(struct.calcsize(KMAGIC))
+ if not line:
+ break
+ kmgc = struct.unpack(KMAGIC, line)[0]
+
+ # if "kvmtrace-metadata".kmagic != kmagic,
+ # then need change byte order of all fields
+ if kmgc == 0x12345678:
+ reverse_flag = False
+ else:
+ reverse_flag = True
+ continue
+
line = sys.stdin.read(struct.calcsize(HDRREC))
if not line:
break
(event, pid, vcpu_id) = struct.unpack(HDRREC, line)
+
+ if reverse_flag:
+ event = reverse_int(event)
+ pid = reverse_int(pid)
+ vcpu_id = reverse_int(vcpu_id)
n_data = event >> 28 & 0x7
tsc_in = event >> 31
@@ -155,6 +193,14 @@ while not interrupted:
break
(d1, d2, d3, d4, d5) = struct.unpack(D5REC, line)
+ if reverse_flag:
+ tsc = reverse_qword(tsc)
+ d1 = reverse_int(d1)
+ d2 = reverse_int(d2)
+ d3 = reverse_int(d3)
+ d4 = reverse_int(d4)
+ d5 = reverse_int(d5)
+
event &= 0x0fffffff
# provide relative TSC
--
1.5.3.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for supporting big_endian
2008-05-20 7:03 ` [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format " Tan, Li
@ 2008-05-20 16:22 ` Avi Kivity
2008-05-21 7:36 ` Tan, Li
0 siblings, 1 reply; 17+ messages in thread
From: Avi Kivity @ 2008-05-20 16:22 UTC (permalink / raw)
To: Tan, Li; +Cc: kvm
Tan, Li wrote:
> From 9d39264bdfb00a1a717074e486a948a578547c50 Mon Sep 17 00:00:00 2001
> From: Tan Li <li.tan@intel.com>
> Date: Tue, 20 May 2008 09:14:41 +0800
> Subject: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for
> supporting big_endian
>
> Currently kvmtrace is not portable, and prevent from copying a trace
> file from big-endian target to little-endian workstation for analysis.
>
> In the patch, kvmtrace_format reads and checks the magic number from
> trace log. if needed, then change bytes order of all fields in records
> followed.
>
>
> +def reverse_bytes(origin, bytes_cnt):
> + i = 0
> + rtn = 0L
> + while i < bytes_cnt:
> + i = i + 1
> + b = origin & 0xff
> + origin = origin >> 8
> + rtn = (rtn << 8) + b
> + return rtn
> +
> +def reverse_int(origin):
> + return reverse_bytes(origin, 4)
> +
> +def reverse_qword(origin):
> + return reverse_bytes(origin, 8)
> +
> def sighand(x,y):
> global interrupted
> interrupted = 1
> @@ -100,18 +116,40 @@ D2REC = "II"
> D3REC = "III"
> D4REC = "IIII"
> D5REC = "IIIII"
> +KMAGIC = "I"
>
The python struct module supports reading big-endian and little-endian
data. So you can simply switch the format strings if you have the wrong
endianness.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for supporting big_endian
2008-05-20 16:22 ` Avi Kivity
@ 2008-05-21 7:36 ` Tan, Li
2008-05-21 9:21 ` Avi Kivity
0 siblings, 1 reply; 17+ messages in thread
From: Tan, Li @ 2008-05-21 7:36 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
-----Original Message-----
From: Avi Kivity [mailto:avi@qumranet.com]
Sent: 2008年5月21日 0:23
To: Tan, Li
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for supporting big_endian
Tan, Li wrote:
> From 9d39264bdfb00a1a717074e486a948a578547c50 Mon Sep 17 00:00:00 2001
> From: Tan Li <li.tan@intel.com>
> Date: Tue, 20 May 2008 09:14:41 +0800
> Subject: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for
> supporting big_endian
>
> Currently kvmtrace is not portable, and prevent from copying a trace
> file from big-endian target to little-endian workstation for analysis.
>
> In the patch, kvmtrace_format reads and checks the magic number from
> trace log. if needed, then change bytes order of all fields in records
> followed.
>
>
> +def reverse_bytes(origin, bytes_cnt):
> + i = 0
> + rtn = 0L
> + while i < bytes_cnt:
> + i = i + 1
> + b = origin & 0xff
> + origin = origin >> 8
> + rtn = (rtn << 8) + b
> + return rtn
> +
> +def reverse_int(origin):
> + return reverse_bytes(origin, 4)
> +
> +def reverse_qword(origin):
> + return reverse_bytes(origin, 8)
> +
> def sighand(x,y):
> global interrupted
> interrupted = 1
> @@ -100,18 +116,40 @@ D2REC = "II"
> D3REC = "III"
> D4REC = "IIII"
> D5REC = "IIIII"
> +KMAGIC = "I"
>
The python struct module supports reading big-endian and little-endian
data. So you can simply switch the format strings if you have the wrong
endianness.
--
error compiling committee.c: too many arguments to function
[tan] thanks for your comments. Then it looks like:
def reverse_int(origin):
s = struct.pack('<I', origin)
return struct.unpack('>I', s)
def reverse_qword(origin):
s = struct.pack('<Q', origin)
return struct.unpack('>Q', s)
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for supporting big_endian
2008-05-21 7:36 ` Tan, Li
@ 2008-05-21 9:21 ` Avi Kivity
2008-05-22 0:30 ` Tan, Li
0 siblings, 1 reply; 17+ messages in thread
From: Avi Kivity @ 2008-05-21 9:21 UTC (permalink / raw)
To: Tan, Li; +Cc: kvm
Tan, Li wrote:
> -----Original Message-----
> From: Avi Kivity [mailto:avi@qumranet.com]
> Sent: 2008年5月21日 0:23
> To: Tan, Li
> Cc: kvm@vger.kernel.org
> Subject: Re: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for supporting big_endian
>
> Tan, Li wrote:
>
>> From 9d39264bdfb00a1a717074e486a948a578547c50 Mon Sep 17 00:00:00 2001
>> From: Tan Li <li.tan@intel.com>
>> Date: Tue, 20 May 2008 09:14:41 +0800
>> Subject: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for
>> supporting big_endian
>>
>> Currently kvmtrace is not portable, and prevent from copying a trace
>> file from big-endian target to little-endian workstation for analysis.
>>
>> In the patch, kvmtrace_format reads and checks the magic number from
>> trace log. if needed, then change bytes order of all fields in records
>> followed.
>>
>>
>> +def reverse_bytes(origin, bytes_cnt):
>> + i = 0
>> + rtn = 0L
>> + while i < bytes_cnt:
>> + i = i + 1
>> + b = origin & 0xff
>> + origin = origin >> 8
>> + rtn = (rtn << 8) + b
>> + return rtn
>> +
>> +def reverse_int(origin):
>> + return reverse_bytes(origin, 4)
>> +
>> +def reverse_qword(origin):
>> + return reverse_bytes(origin, 8)
>> +
>> def sighand(x,y):
>> global interrupted
>> interrupted = 1
>> @@ -100,18 +116,40 @@ D2REC = "II"
>> D3REC = "III"
>> D4REC = "IIII"
>> D5REC = "IIIII"
>> +KMAGIC = "I"
>>
>>
>
> The python struct module supports reading big-endian and little-endian
> data. So you can simply switch the format strings if you have the wrong
> endianness.
>
>
> [tan] thanks for your comments. Then it looks like:
> def reverse_int(origin):
> s = struct.pack('<I', origin)
> return struct.unpack('>I', s)
>
> def reverse_qword(origin):
> s = struct.pack('<Q', origin)
> return struct.unpack('>Q', s)
>
I meant, you can change the definitions of D3REC, D4REC, etc. and not
worry about reversing once you've decoded the magic word.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 17+ messages in thread* RE: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for supporting big_endian
2008-05-21 9:21 ` Avi Kivity
@ 2008-05-22 0:30 ` Tan, Li
2008-05-25 9:34 ` Avi Kivity
0 siblings, 1 reply; 17+ messages in thread
From: Tan, Li @ 2008-05-22 0:30 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
-----Original Message-----
From: Avi Kivity [mailto:avi@qumranet.com]
Sent: 2008年5月21日 17:21
To: Tan, Li
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for supporting big_endian
Tan, Li wrote:
> -----Original Message-----
> From: Avi Kivity [mailto:avi@qumranet.com]
> Sent: 2008年5月21日 0:23
> To: Tan, Li
> Cc: kvm@vger.kernel.org
> Subject: Re: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for supporting big_endian
>
> Tan, Li wrote:
>
>> From 9d39264bdfb00a1a717074e486a948a578547c50 Mon Sep 17 00:00:00 2001
>> From: Tan Li <li.tan@intel.com>
>> Date: Tue, 20 May 2008 09:14:41 +0800
>> Subject: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for
>> supporting big_endian
>>
>> Currently kvmtrace is not portable, and prevent from copying a trace
>> file from big-endian target to little-endian workstation for analysis.
>>
>> In the patch, kvmtrace_format reads and checks the magic number from
>> trace log. if needed, then change bytes order of all fields in records
>> followed.
>>
>>
>> +def reverse_bytes(origin, bytes_cnt):
>> + i = 0
>> + rtn = 0L
>> + while i < bytes_cnt:
>> + i = i + 1
>> + b = origin & 0xff
>> + origin = origin >> 8
>> + rtn = (rtn << 8) + b
>> + return rtn
>> +
>> +def reverse_int(origin):
>> + return reverse_bytes(origin, 4)
>> +
>> +def reverse_qword(origin):
>> + return reverse_bytes(origin, 8)
>> +
>> def sighand(x,y):
>> global interrupted
>> interrupted = 1
>> @@ -100,18 +116,40 @@ D2REC = "II"
>> D3REC = "III"
>> D4REC = "IIII"
>> D5REC = "IIIII"
>> +KMAGIC = "I"
>>
>>
>
> The python struct module supports reading big-endian and little-endian
> data. So you can simply switch the format strings if you have the wrong
> endianness.
>
>
> [tan] thanks for your comments. Then it looks like:
> def reverse_int(origin):
> s = struct.pack('<I', origin)
> return struct.unpack('>I', s)
>
> def reverse_qword(origin):
> s = struct.pack('<Q', origin)
> return struct.unpack('>Q', s)
>
I meant, you can change the definitions of D3REC, D4REC, etc. and not
worry about reversing once you've decoded the magic word.
--
error compiling committee.c: too many arguments to function
[tan] so it looks like:
if reverse_flag == 0:
# print "host and data file are same endian order"
D3REC = "III"
D4REC = "IIII"
D5REC = "IIIII"
else:
If pack('<h', 1) == pack('=h',1):
# print "host is Little Endian, data file is big endian"
D3REC = ">III"
D4REC = ">IIII"
D5REC = ">IIIII"
elif pack('>h', 1) == pack('=h',1):
# print "host is big Endian, data file is little endian"
D3REC = "<III"
D4REC = "<IIII"
D5REC = "<IIIII"
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for supporting big_endian
2008-05-22 0:30 ` Tan, Li
@ 2008-05-25 9:34 ` Avi Kivity
2008-05-26 5:38 ` Tan, Li
0 siblings, 1 reply; 17+ messages in thread
From: Avi Kivity @ 2008-05-25 9:34 UTC (permalink / raw)
To: Tan, Li; +Cc: kvm
Tan, Li wrote:
> I meant, you can change the definitions of D3REC, D4REC, etc. and not
> worry about reversing once you've decoded the magic word.
>
>
> [tan] so it looks like:
> if reverse_flag == 0:
> # print "host and data file are same endian order"
> D3REC = "III"
> D4REC = "IIII"
> D5REC = "IIIII"
> else:
> If pack('<h', 1) == pack('=h',1):
> # print "host is Little Endian, data file is big endian"
> D3REC = ">III"
> D4REC = ">IIII"
> D5REC = ">IIIII"
> elif pack('>h', 1) == pack('=h',1):
> # print "host is big Endian, data file is little endian"
> D3REC = "<III"
> D4REC = "<IIII"
> D5REC = "<IIIII"
>
Not sure about the tests. You aren't looking at the data from the file,
how can you conclude it's little endian or big endian?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 17+ messages in thread* RE: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for supporting big_endian
2008-05-25 9:34 ` Avi Kivity
@ 2008-05-26 5:38 ` Tan, Li
2008-05-28 11:17 ` Avi Kivity
0 siblings, 1 reply; 17+ messages in thread
From: Tan, Li @ 2008-05-26 5:38 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
According to http://docs.python.org/lib/module-struct.html
Character Byte order Size and alignment
@ native native
= native standard
< little-endian standard
> big-endian standard
! network (= big-endian) standard
> [tan] so it looks like:
> if reverse_flag == 0:
> # print "host and data file are same endian
order"
> D3REC = "III"
> else:
#reverse_flag == 1: host and data file are DIFF endian order
> If pack('<h', 1) == pack('=h',1): # this implys host is
Little Endian,
so data file is big endian"
> D3REC = ">III"
> elif pack('>h', 1) == pack('=h',1): # this implys host
is big Endian,
so data file is little endian"
> D3REC = "<III"
> D4REC = "<IIII"
> D5REC = "<IIIII"
>
Not sure about the tests. You aren't looking at the data from the file,
how can you conclude it's little endian or big endian?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for supporting big_endian
2008-05-26 5:38 ` Tan, Li
@ 2008-05-28 11:17 ` Avi Kivity
2008-06-02 1:25 ` Tan, Li
0 siblings, 1 reply; 17+ messages in thread
From: Avi Kivity @ 2008-05-28 11:17 UTC (permalink / raw)
To: Tan, Li; +Cc: kvm
Tan, Li wrote:
> According to http://docs.python.org/lib/module-struct.html
> Character Byte order Size and alignment
> @ native native
> = native standard
> < little-endian standard
>
Oh okay. You are relying on the user to supply the reverse flag.
But you don't need to do that. You can start with the formats as "<I"
and similar. Read the magic word. If it mismatches, switch to ">I" and
start again.
This way you get autodetection of the format, and shorter code as well.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for supporting big_endian
2008-05-28 11:17 ` Avi Kivity
@ 2008-06-02 1:25 ` Tan, Li
0 siblings, 0 replies; 17+ messages in thread
From: Tan, Li @ 2008-06-02 1:25 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
Yes, in this way only need to handle 2 situations, no need to have 3 situations.
Tan Li
-----Original Message-----
From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On Behalf Of Avi Kivity
Sent: 2008年5月28日 19:17
To: Tan, Li
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format for supporting big_endian
Tan, Li wrote:
> According to http://docs.python.org/lib/module-struct.html
> Character Byte order Size and alignment
> @ native native
> = native standard
> < little-endian standard
>
Oh okay. You are relying on the user to supply the reverse flag.
But you don't need to do that. You can start with the formats as "<I"
and similar. Read the magic word. If it mismatches, switch to ">I" and
start again.
This way you get autodetection of the format, and shorter code as well.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvm_trace in kernel for supporting big_endian
2008-05-20 6:53 ` [PATCH] [RFC][PATCH] kvm: kvmtrace: kvm_trace in kernel for supporting big_endian Tan, Li
2008-05-20 7:03 ` [PATCH] [RFC][PATCH] kvm: kvmtrace: kvmtrace_format " Tan, Li
@ 2008-05-20 16:25 ` Avi Kivity
1 sibling, 0 replies; 17+ messages in thread
From: Avi Kivity @ 2008-05-20 16:25 UTC (permalink / raw)
To: Tan, Li; +Cc: kvm
Tan, Li wrote:
> From 63283d32ad5faf0845fa0358fd71d119f4dc0e3d Mon Sep 17 00:00:00 2001
> From: Tan Li <li.tan@intel.com>
> Date: Mon, 19 May 2008 17:18:54 +0800
> Subject: [PATCH] [RFC][PATCH] kvm: kvmtrace: kvm_trace in kernel for
> supporting big_endian
>
> Currently kvmtrace is not portable. This will prevent from copying a
> trace file from big-endian target to little-endian workstation for
> analysis.
>
> In the patch, kernel outputs metadata containing a magic number to trace
> log.
>
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> index a281afe..ca08cb1 100644
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -294,14 +294,14 @@ struct kvm_trace_rec {
> __u32 vcpu_id;
> union {
> struct {
> - __u32 cycle_lo, cycle_hi;
> + __u64 cycle_u64;
> __u32 extra_u32[KVM_TRC_EXTRA_MAX];
> } cycle;
>
For this bit, you'll need to change 'II' to 'L' in the format, no?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 17+ messages in thread