All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix failed msr tracing
@ 2010-01-25 17:36 Avi Kivity
  2010-01-25 17:36 ` [PATCH 1/2] KVM: Fix msr trace Avi Kivity
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Avi Kivity @ 2010-01-25 17:36 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

We don't trace failed msr access (wrmsr or rdmsr which end up generating a
#GP), which loses important data.

Avi Kivity (2):
  KVM: Fix msr trace
  KVM: Trace failed msr reads and writes

 arch/x86/kvm/svm.c   |   13 ++++++++-----
 arch/x86/kvm/trace.h |   27 ++++++++++++++++-----------
 arch/x86/kvm/vmx.c   |    5 +++--
 3 files changed, 27 insertions(+), 18 deletions(-)


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] KVM: Fix msr trace
  2010-01-25 17:36 [PATCH 0/2] Fix failed msr tracing Avi Kivity
@ 2010-01-25 17:36 ` Avi Kivity
  2010-01-25 17:36 ` [PATCH 2/2] KVM: Trace failed msr reads and writes Avi Kivity
  2010-01-26 12:40 ` [PATCH 0/2] Fix failed msr tracing Marcelo Tosatti
  2 siblings, 0 replies; 5+ messages in thread
From: Avi Kivity @ 2010-01-25 17:36 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

- data is 64 bits wide, not unsigned long
- rw is confusingly named

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/trace.h |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 1cb3d0e..45903a9 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -246,23 +246,23 @@ TRACE_EVENT(kvm_page_fault,
  * Tracepoint for guest MSR access.
  */
 TRACE_EVENT(kvm_msr,
-	TP_PROTO(unsigned int rw, unsigned int ecx, unsigned long data),
-	TP_ARGS(rw, ecx, data),
+	TP_PROTO(unsigned write, u32 ecx, u64 data),
+	TP_ARGS(write, ecx, data),
 
 	TP_STRUCT__entry(
-		__field(	unsigned int,	rw		)
-		__field(	unsigned int,	ecx		)
-		__field(	unsigned long,	data		)
+		__field(	unsigned,	write		)
+		__field(	u32,		ecx		)
+		__field(	u64,		data		)
 	),
 
 	TP_fast_assign(
-		__entry->rw		= rw;
+		__entry->write		= write;
 		__entry->ecx		= ecx;
 		__entry->data		= data;
 	),
 
-	TP_printk("msr_%s %x = 0x%lx",
-		  __entry->rw ? "write" : "read",
+	TP_printk("msr_%s %x = 0x%llx",
+		  __entry->write ? "write" : "read",
 		  __entry->ecx, __entry->data)
 );
 
-- 
1.6.5.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] KVM: Trace failed msr reads and writes
  2010-01-25 17:36 [PATCH 0/2] Fix failed msr tracing Avi Kivity
  2010-01-25 17:36 ` [PATCH 1/2] KVM: Fix msr trace Avi Kivity
@ 2010-01-25 17:36 ` Avi Kivity
  2010-01-25 17:47   ` [PATCH 2/2 FIXED] " Avi Kivity
  2010-01-26 12:40 ` [PATCH 0/2] Fix failed msr tracing Marcelo Tosatti
  2 siblings, 1 reply; 5+ messages in thread
From: Avi Kivity @ 2010-01-25 17:36 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

Record failed msrs reads and writes, and the fact that they failed as well.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/svm.c   |   13 ++++++++-----
 arch/x86/kvm/trace.h |   17 +++++++++++------
 arch/x86/kvm/vmx.c   |    5 +++--
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 8d7cb62..a0da182 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2172,9 +2172,10 @@ static int rdmsr_interception(struct vcpu_svm *svm)
 	u32 ecx = svm->vcpu.arch.regs[VCPU_REGS_RCX];
 	u64 data;
 
-	if (svm_get_msr(&svm->vcpu, ecx, &data))
+	if (svm_get_msr(&svm->vcpu, ecx, &data)) {
+		trace_kvm_msr_read_ex(ecx);
 		kvm_inject_gp(&svm->vcpu, 0);
-	else {
+	} else {
 		trace_kvm_msr_read(ecx, data);
 
 		svm->vcpu.arch.regs[VCPU_REGS_RAX] = data & 0xffffffff;
@@ -2266,13 +2267,15 @@ static int wrmsr_interception(struct vcpu_svm *svm)
 	u64 data = (svm->vcpu.arch.regs[VCPU_REGS_RAX] & -1u)
 		| ((u64)(svm->vcpu.arch.regs[VCPU_REGS_RDX] & -1u) << 32);
 
-	trace_kvm_msr_write(ecx, data);
 
 	svm->next_rip = kvm_rip_read(&svm->vcpu) + 2;
-	if (svm_set_msr(&svm->vcpu, ecx, data))
+	if (svm_set_msr(&svm->vcpu, ecx, data)) {
+		trace_kvm_msr_write_ex(ecx, data);
 		kvm_inject_gp(&svm->vcpu, 0);
-	else
+	} else {
+		trace_kvm_msr_write(ecx, data);
 		skip_emulated_instruction(&svm->vcpu);
+	}
 	return 1;
 }
 
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 45903a9..df55a66 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -246,28 +246,33 @@ TRACE_EVENT(kvm_page_fault,
  * Tracepoint for guest MSR access.
  */
 TRACE_EVENT(kvm_msr,
-	TP_PROTO(unsigned write, u32 ecx, u64 data),
-	TP_ARGS(write, ecx, data),
+	    TP_PROTO(unsigned write, u32 ecx, u64 data, bool exception),
+	    TP_ARGS(write, ecx, data, exception),
 
 	TP_STRUCT__entry(
 		__field(	unsigned,	write		)
 		__field(	u32,		ecx		)
 		__field(	u64,		data		)
+		__field(	u8,		exception	)
 	),
 
 	TP_fast_assign(
 		__entry->write		= write;
 		__entry->ecx		= ecx;
 		__entry->data		= data;
+		__entry->exception	= exception;
 	),
 
-	TP_printk("msr_%s %x = 0x%llx",
+	TP_printk("msr_%s %x = 0x%llx%s",
 		  __entry->write ? "write" : "read",
-		  __entry->ecx, __entry->data)
+		  __entry->ecx, __entry->data,
+		  __entry->exception ? " (#GP)" : "")
 );
 
-#define trace_kvm_msr_read(ecx, data)		trace_kvm_msr(0, ecx, data)
-#define trace_kvm_msr_write(ecx, data)		trace_kvm_msr(1, ecx, data)
+#define trace_kvm_msr_read(ecx, data)      trace_kvm_msr(0, ecx, data, false)
+#define trace_kvm_msr_write(ecx, data)     trace_kvm_msr(1, ecx, data, false)
+#define trace_kvm_msr_read_ex(ecx)         trace_kvm_msr(0, ecx, 0, true)
+#define trace_kvm_msr_write_ex(ecx, data)  trace_kvm_msr(1, ecx, data, true)
 
 /*
  * Tracepoint for guest CR access.
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 9f56110..0846e55 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3143,6 +3143,7 @@ static int handle_rdmsr(struct kvm_vcpu *vcpu)
 	u64 data;
 
 	if (vmx_get_msr(vcpu, ecx, &data)) {
+		trace_kvm_msr_read_ex(ecx);
 		kvm_inject_gp(vcpu, 0);
 		return 1;
 	}
@@ -3162,13 +3163,13 @@ static int handle_wrmsr(struct kvm_vcpu *vcpu)
 	u64 data = (vcpu->arch.regs[VCPU_REGS_RAX] & -1u)
 		| ((u64)(vcpu->arch.regs[VCPU_REGS_RDX] & -1u) << 32);
 
-	trace_kvm_msr_write(ecx, data);
-
 	if (vmx_set_msr(vcpu, ecx, data) != 0) {
+		trace_kvm_msr_write_ex(ecx, data);
 		kvm_inject_gp(vcpu, 0);
 		return 1;
 	}
 
+	trace_kvm_msr_write(ecx, data);
 	skip_emulated_instruction(vcpu);
 	return 1;
 }
-- 
1.6.5.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2 FIXED] KVM: Trace failed msr reads and writes
  2010-01-25 17:36 ` [PATCH 2/2] KVM: Trace failed msr reads and writes Avi Kivity
@ 2010-01-25 17:47   ` Avi Kivity
  0 siblings, 0 replies; 5+ messages in thread
From: Avi Kivity @ 2010-01-25 17:47 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

Record failed msrs reads and writes, and the fact that they failed as well.

Signed-off-by: Avi Kivity <avi@redhat.com>
---

(cosmetic indentation fix)

 arch/x86/kvm/svm.c   |   13 ++++++++-----
 arch/x86/kvm/trace.h |   17 +++++++++++------
 arch/x86/kvm/vmx.c   |    5 +++--
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 8d7cb62..a0da182 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2172,9 +2172,10 @@ static int rdmsr_interception(struct vcpu_svm *svm)
 	u32 ecx = svm->vcpu.arch.regs[VCPU_REGS_RCX];
 	u64 data;
 
-	if (svm_get_msr(&svm->vcpu, ecx, &data))
+	if (svm_get_msr(&svm->vcpu, ecx, &data)) {
+		trace_kvm_msr_read_ex(ecx);
 		kvm_inject_gp(&svm->vcpu, 0);
-	else {
+	} else {
 		trace_kvm_msr_read(ecx, data);
 
 		svm->vcpu.arch.regs[VCPU_REGS_RAX] = data & 0xffffffff;
@@ -2266,13 +2267,15 @@ static int wrmsr_interception(struct vcpu_svm *svm)
 	u64 data = (svm->vcpu.arch.regs[VCPU_REGS_RAX] & -1u)
 		| ((u64)(svm->vcpu.arch.regs[VCPU_REGS_RDX] & -1u) << 32);
 
-	trace_kvm_msr_write(ecx, data);
 
 	svm->next_rip = kvm_rip_read(&svm->vcpu) + 2;
-	if (svm_set_msr(&svm->vcpu, ecx, data))
+	if (svm_set_msr(&svm->vcpu, ecx, data)) {
+		trace_kvm_msr_write_ex(ecx, data);
 		kvm_inject_gp(&svm->vcpu, 0);
-	else
+	} else {
+		trace_kvm_msr_write(ecx, data);
 		skip_emulated_instruction(&svm->vcpu);
+	}
 	return 1;
 }
 
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 45903a9..df55a66 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -246,28 +246,33 @@ TRACE_EVENT(kvm_page_fault,
  * Tracepoint for guest MSR access.
  */
 TRACE_EVENT(kvm_msr,
-	TP_PROTO(unsigned write, u32 ecx, u64 data),
-	TP_ARGS(write, ecx, data),
+	TP_PROTO(unsigned write, u32 ecx, u64 data, bool exception),
+	TP_ARGS(write, ecx, data, exception),
 
 	TP_STRUCT__entry(
 		__field(	unsigned,	write		)
 		__field(	u32,		ecx		)
 		__field(	u64,		data		)
+		__field(	u8,		exception	)
 	),
 
 	TP_fast_assign(
 		__entry->write		= write;
 		__entry->ecx		= ecx;
 		__entry->data		= data;
+		__entry->exception	= exception;
 	),
 
-	TP_printk("msr_%s %x = 0x%llx",
+	TP_printk("msr_%s %x = 0x%llx%s",
 		  __entry->write ? "write" : "read",
-		  __entry->ecx, __entry->data)
+		  __entry->ecx, __entry->data,
+		  __entry->exception ? " (#GP)" : "")
 );
 
-#define trace_kvm_msr_read(ecx, data)		trace_kvm_msr(0, ecx, data)
-#define trace_kvm_msr_write(ecx, data)		trace_kvm_msr(1, ecx, data)
+#define trace_kvm_msr_read(ecx, data)      trace_kvm_msr(0, ecx, data, false)
+#define trace_kvm_msr_write(ecx, data)     trace_kvm_msr(1, ecx, data, false)
+#define trace_kvm_msr_read_ex(ecx)         trace_kvm_msr(0, ecx, 0, true)
+#define trace_kvm_msr_write_ex(ecx, data)  trace_kvm_msr(1, ecx, data, true)
 
 /*
  * Tracepoint for guest CR access.
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 9f56110..0846e55 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3143,6 +3143,7 @@ static int handle_rdmsr(struct kvm_vcpu *vcpu)
 	u64 data;
 
 	if (vmx_get_msr(vcpu, ecx, &data)) {
+		trace_kvm_msr_read_ex(ecx);
 		kvm_inject_gp(vcpu, 0);
 		return 1;
 	}
@@ -3162,13 +3163,13 @@ static int handle_wrmsr(struct kvm_vcpu *vcpu)
 	u64 data = (vcpu->arch.regs[VCPU_REGS_RAX] & -1u)
 		| ((u64)(vcpu->arch.regs[VCPU_REGS_RDX] & -1u) << 32);
 
-	trace_kvm_msr_write(ecx, data);
-
 	if (vmx_set_msr(vcpu, ecx, data) != 0) {
+		trace_kvm_msr_write_ex(ecx, data);
 		kvm_inject_gp(vcpu, 0);
 		return 1;
 	}
 
+	trace_kvm_msr_write(ecx, data);
 	skip_emulated_instruction(vcpu);
 	return 1;
 }
-- 
1.6.5.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] Fix failed msr tracing
  2010-01-25 17:36 [PATCH 0/2] Fix failed msr tracing Avi Kivity
  2010-01-25 17:36 ` [PATCH 1/2] KVM: Fix msr trace Avi Kivity
  2010-01-25 17:36 ` [PATCH 2/2] KVM: Trace failed msr reads and writes Avi Kivity
@ 2010-01-26 12:40 ` Marcelo Tosatti
  2 siblings, 0 replies; 5+ messages in thread
From: Marcelo Tosatti @ 2010-01-26 12:40 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm

On Mon, Jan 25, 2010 at 07:36:02PM +0200, Avi Kivity wrote:
> We don't trace failed msr access (wrmsr or rdmsr which end up generating a
> #GP), which loses important data.
> 
> Avi Kivity (2):
>   KVM: Fix msr trace
>   KVM: Trace failed msr reads and writes
> 
>  arch/x86/kvm/svm.c   |   13 ++++++++-----
>  arch/x86/kvm/trace.h |   27 ++++++++++++++++-----------
>  arch/x86/kvm/vmx.c   |    5 +++--
>  3 files changed, 27 insertions(+), 18 deletions(-)

Applied, thanks.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-01-26 12:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-01-25 17:36 [PATCH 0/2] Fix failed msr tracing Avi Kivity
2010-01-25 17:36 ` [PATCH 1/2] KVM: Fix msr trace Avi Kivity
2010-01-25 17:36 ` [PATCH 2/2] KVM: Trace failed msr reads and writes Avi Kivity
2010-01-25 17:47   ` [PATCH 2/2 FIXED] " Avi Kivity
2010-01-26 12:40 ` [PATCH 0/2] Fix failed msr tracing Marcelo Tosatti

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.