diff mbox

[GIT,PULL,03/36] arm/arm64: KVM: add tracing support for arm64 exit handler

Message ID 1422007385-14730-4-git-send-email-christoffer.dall@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Christoffer Dall Jan. 23, 2015, 10:02 a.m. UTC 
From: Wei Huang <wei@redhat.com>

arm64 uses its own copy of exit handler (arm64/kvm/handle_exit.c).
Currently this file doesn't hook up with any trace points. As a result
users might not see certain events (e.g. HVC & WFI) while using ftrace
with arm64 KVM. This patch fixes this issue by adding a new trace file
and defining two trace events (one of which is shared by wfi and wfe)
for arm64. The new trace points are then linked with related functions
in handle_exit.c.

Signed-off-by: Wei Huang <wei@redhat.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 arch/arm64/include/asm/kvm_arm.h     |  2 ++
 arch/arm64/include/asm/kvm_emulate.h |  5 ++++
 arch/arm64/kvm/handle_exit.c         | 13 +++++++--
 arch/arm64/kvm/trace.h               | 55 ++++++++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm64/kvm/trace.h

Comments

Mark Rutland Jan. 23, 2015, 11:35 a.m. UTC | #1 
Hi Christoffer,

On Fri, Jan 23, 2015 at 10:02:32AM +0000, Christoffer Dall wrote:
> From: Wei Huang <wei@redhat.com>
> 
> arm64 uses its own copy of exit handler (arm64/kvm/handle_exit.c).
> Currently this file doesn't hook up with any trace points. As a result
> users might not see certain events (e.g. HVC & WFI) while using ftrace
> with arm64 KVM. This patch fixes this issue by adding a new trace file
> and defining two trace events (one of which is shared by wfi and wfe)
> for arm64. The new trace points are then linked with related functions
> in handle_exit.c.

Stephen Rothwell reported a couple of conflicts between this patch and
my ESR rework in -next:

https://lkml.org/lkml/2015/1/22/7
https://lkml.org/lkml/2015/1/22/8

I'd hoped we'd be able to fix that before this went further upstream.

The easiest way I could see to do that was to merge my esr branch (which
is stable and Catalin has pulled) branch into the kvm-arm tree, and fix
the conflicts there.

Is there any chance we can fix that now?

Thanks,
Mark.

> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
>  arch/arm64/include/asm/kvm_arm.h     |  2 ++
>  arch/arm64/include/asm/kvm_emulate.h |  5 ++++
>  arch/arm64/kvm/handle_exit.c         | 13 +++++++--
>  arch/arm64/kvm/trace.h               | 55 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 73 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm64/kvm/trace.h
> 
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index 8afb863..3da2d3a 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -257,4 +257,6 @@
>  
>  #define ESR_EL2_EC_WFI_ISS_WFE	(1 << 0)
>  
> +#define ESR_EL2_HVC_IMM_MASK	((1UL << 16) - 1)
> +
>  #endif /* __ARM64_KVM_ARM_H__ */
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 8127e45..a6fa2d2 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -126,6 +126,11 @@ static inline phys_addr_t kvm_vcpu_get_fault_ipa(const struct kvm_vcpu *vcpu)
>  	return ((phys_addr_t)vcpu->arch.fault.hpfar_el2 & HPFAR_MASK) << 8;
>  }
>  
> +static inline u32 kvm_vcpu_hvc_get_imm(const struct kvm_vcpu *vcpu)
> +{
> +	return kvm_vcpu_get_hsr(vcpu) & ESR_EL2_HVC_IMM_MASK;
> +}
> +
>  static inline bool kvm_vcpu_dabt_isvalid(const struct kvm_vcpu *vcpu)
>  {
>  	return !!(kvm_vcpu_get_hsr(vcpu) & ESR_EL2_ISV);
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 34b8bd0..6a7eb3c 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -26,12 +26,18 @@
>  #include <asm/kvm_mmu.h>
>  #include <asm/kvm_psci.h>
>  
> +#define CREATE_TRACE_POINTS
> +#include "trace.h"
> +
>  typedef int (*exit_handle_fn)(struct kvm_vcpu *, struct kvm_run *);
>  
>  static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  {
>  	int ret;
>  
> +	trace_kvm_hvc_arm64(*vcpu_pc(vcpu), *vcpu_reg(vcpu, 0),
> +			    kvm_vcpu_hvc_get_imm(vcpu));
> +
>  	ret = kvm_psci_call(vcpu);
>  	if (ret < 0) {
>  		kvm_inject_undefined(vcpu);
> @@ -61,10 +67,13 @@ static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
>   */
>  static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  {
> -	if (kvm_vcpu_get_hsr(vcpu) & ESR_EL2_EC_WFI_ISS_WFE)
> +	if (kvm_vcpu_get_hsr(vcpu) & ESR_EL2_EC_WFI_ISS_WFE) {
> +		trace_kvm_wfx_arm64(*vcpu_pc(vcpu), true);
>  		kvm_vcpu_on_spin(vcpu);
> -	else
> +	} else {
> +		trace_kvm_wfx_arm64(*vcpu_pc(vcpu), false);
>  		kvm_vcpu_block(vcpu);
> +	}
>  
>  	kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
>  
> diff --git a/arch/arm64/kvm/trace.h b/arch/arm64/kvm/trace.h
> new file mode 100644
> index 0000000..157416e9
> --- /dev/null
> +++ b/arch/arm64/kvm/trace.h
> @@ -0,0 +1,55 @@
> +#if !defined(_TRACE_ARM64_KVM_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_ARM64_KVM_H
> +
> +#include <linux/tracepoint.h>
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM kvm
> +
> +TRACE_EVENT(kvm_wfx_arm64,
> +	TP_PROTO(unsigned long vcpu_pc, bool is_wfe),
> +	TP_ARGS(vcpu_pc, is_wfe),
> +
> +	TP_STRUCT__entry(
> +		__field(unsigned long,	vcpu_pc)
> +		__field(bool,		is_wfe)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->vcpu_pc = vcpu_pc;
> +		__entry->is_wfe  = is_wfe;
> +	),
> +
> +	TP_printk("guest executed wf%c at: 0x%08lx",
> +		  __entry->is_wfe ? 'e' : 'i', __entry->vcpu_pc)
> +);
> +
> +TRACE_EVENT(kvm_hvc_arm64,
> +	TP_PROTO(unsigned long vcpu_pc, unsigned long r0, unsigned long imm),
> +	TP_ARGS(vcpu_pc, r0, imm),
> +
> +	TP_STRUCT__entry(
> +		__field(unsigned long, vcpu_pc)
> +		__field(unsigned long, r0)
> +		__field(unsigned long, imm)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->vcpu_pc = vcpu_pc;
> +		__entry->r0 = r0;
> +		__entry->imm = imm;
> +	),
> +
> +	TP_printk("HVC at 0x%08lx (r0: 0x%08lx, imm: 0x%lx)",
> +		  __entry->vcpu_pc, __entry->r0, __entry->imm)
> +);
> +
> +#endif /* _TRACE_ARM64_KVM_H */
> +
> +#undef TRACE_INCLUDE_PATH
> +#define TRACE_INCLUDE_PATH .
> +#undef TRACE_INCLUDE_FILE
> +#define TRACE_INCLUDE_FILE trace
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> -- 
> 2.1.2.330.g565301e.dirty
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
> 
--
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
Paolo Bonzini Jan. 23, 2015, 12:01 p.m. UTC | #2 
On 23/01/2015 12:35, Mark Rutland wrote:
> 
> https://lkml.org/lkml/2015/1/22/7
> https://lkml.org/lkml/2015/1/22/8
> 
> I'd hoped we'd be able to fix that before this went further upstream.
> 
> The easiest way I could see to do that was to merge my esr branch (which
> is stable and Catalin has pulled) branch into the kvm-arm tree, and fix
> the conflicts there.
> 
> Is there any chance we can fix that now?

I'll fix the conflicts in the kvm tree.  Please wait for further
instructions...

Paolo
--
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
Paolo Bonzini Jan. 23, 2015, 12:49 p.m. UTC | #3 
On 23/01/2015 13:01, Paolo Bonzini wrote:
> 
> 
> On 23/01/2015 12:35, Mark Rutland wrote:
>>
>> https://lkml.org/lkml/2015/1/22/7
>> https://lkml.org/lkml/2015/1/22/8
>>
>> I'd hoped we'd be able to fix that before this went further upstream.
>>
>> The easiest way I could see to do that was to merge my esr branch (which
>> is stable and Catalin has pulled) branch into the kvm-arm tree, and fix
>> the conflicts there.
>>
>> Is there any chance we can fix that now?
> 
> I'll fix the conflicts in the kvm tree.  Please wait for further
> instructions...

Ok folks, please check that the resolution in the queue branch of
git://git.kernel.org/pub/scm/virt/kvm/kvm.git is okay.

Paolo
--
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
Christoffer Dall Jan. 23, 2015, 12:55 p.m. UTC | #4 
On Fri, Jan 23, 2015 at 01:49:41PM +0100, Paolo Bonzini wrote:
> 
> 
> On 23/01/2015 13:01, Paolo Bonzini wrote:
> > 
> > 
> > On 23/01/2015 12:35, Mark Rutland wrote:
> >>
> >> https://lkml.org/lkml/2015/1/22/7
> >> https://lkml.org/lkml/2015/1/22/8
> >>
> >> I'd hoped we'd be able to fix that before this went further upstream.
> >>
> >> The easiest way I could see to do that was to merge my esr branch (which
> >> is stable and Catalin has pulled) branch into the kvm-arm tree, and fix
> >> the conflicts there.
> >>
> >> Is there any chance we can fix that now?
> > 
> > I'll fix the conflicts in the kvm tree.  Please wait for further
> > instructions...
> 
> Ok folks, please check that the resolution in the queue branch of
> git://git.kernel.org/pub/scm/virt/kvm/kvm.git is okay.
> 
Looks great, I gave a quick spin on Juno and no problems, thanks Paolo!

-Christoffer
--
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
Mark Rutland Jan. 23, 2015, 1:41 p.m. UTC | #5 
On Fri, Jan 23, 2015 at 12:49:41PM +0000, Paolo Bonzini wrote:
> 
> 
> On 23/01/2015 13:01, Paolo Bonzini wrote:
> > 
> > 
> > On 23/01/2015 12:35, Mark Rutland wrote:
> >>
> >> https://lkml.org/lkml/2015/1/22/7
> >> https://lkml.org/lkml/2015/1/22/8
> >>
> >> I'd hoped we'd be able to fix that before this went further upstream.
> >>
> >> The easiest way I could see to do that was to merge my esr branch (which
> >> is stable and Catalin has pulled) branch into the kvm-arm tree, and fix
> >> the conflicts there.
> >>
> >> Is there any chance we can fix that now?
> > 
> > I'll fix the conflicts in the kvm tree.  Please wait for further
> > instructions...
> 
> Ok folks, please check that the resolution in the queue branch of
> git://git.kernel.org/pub/scm/virt/kvm/kvm.git is okay.

That looks fine to me. Git was happy to merge the arm64 for-next/core
branch, and the result boots fine on my Juno.

Thanks for handling the fixup!

Mark.
--
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
diff mbox

Patch

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 8afb863..3da2d3a 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -257,4 +257,6 @@ 
 
 #define ESR_EL2_EC_WFI_ISS_WFE	(1 << 0)
 
+#define ESR_EL2_HVC_IMM_MASK	((1UL << 16) - 1)
+
 #endif /* __ARM64_KVM_ARM_H__ */
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 8127e45..a6fa2d2 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -126,6 +126,11 @@  static inline phys_addr_t kvm_vcpu_get_fault_ipa(const struct kvm_vcpu *vcpu)
 	return ((phys_addr_t)vcpu->arch.fault.hpfar_el2 & HPFAR_MASK) << 8;
 }
 
+static inline u32 kvm_vcpu_hvc_get_imm(const struct kvm_vcpu *vcpu)
+{
+	return kvm_vcpu_get_hsr(vcpu) & ESR_EL2_HVC_IMM_MASK;
+}
+
 static inline bool kvm_vcpu_dabt_isvalid(const struct kvm_vcpu *vcpu)
 {
 	return !!(kvm_vcpu_get_hsr(vcpu) & ESR_EL2_ISV);
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 34b8bd0..6a7eb3c 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -26,12 +26,18 @@ 
 #include <asm/kvm_mmu.h>
 #include <asm/kvm_psci.h>
 
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
 typedef int (*exit_handle_fn)(struct kvm_vcpu *, struct kvm_run *);
 
 static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
 	int ret;
 
+	trace_kvm_hvc_arm64(*vcpu_pc(vcpu), *vcpu_reg(vcpu, 0),
+			    kvm_vcpu_hvc_get_imm(vcpu));
+
 	ret = kvm_psci_call(vcpu);
 	if (ret < 0) {
 		kvm_inject_undefined(vcpu);
@@ -61,10 +67,13 @@  static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
  */
 static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
-	if (kvm_vcpu_get_hsr(vcpu) & ESR_EL2_EC_WFI_ISS_WFE)
+	if (kvm_vcpu_get_hsr(vcpu) & ESR_EL2_EC_WFI_ISS_WFE) {
+		trace_kvm_wfx_arm64(*vcpu_pc(vcpu), true);
 		kvm_vcpu_on_spin(vcpu);
-	else
+	} else {
+		trace_kvm_wfx_arm64(*vcpu_pc(vcpu), false);
 		kvm_vcpu_block(vcpu);
+	}
 
 	kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
 
diff --git a/arch/arm64/kvm/trace.h b/arch/arm64/kvm/trace.h
new file mode 100644
index 0000000..157416e9
--- /dev/null
+++ b/arch/arm64/kvm/trace.h
@@ -0,0 +1,55 @@ 
+#if !defined(_TRACE_ARM64_KVM_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_ARM64_KVM_H
+
+#include <linux/tracepoint.h>
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM kvm
+
+TRACE_EVENT(kvm_wfx_arm64,
+	TP_PROTO(unsigned long vcpu_pc, bool is_wfe),
+	TP_ARGS(vcpu_pc, is_wfe),
+
+	TP_STRUCT__entry(
+		__field(unsigned long,	vcpu_pc)
+		__field(bool,		is_wfe)
+	),
+
+	TP_fast_assign(
+		__entry->vcpu_pc = vcpu_pc;
+		__entry->is_wfe  = is_wfe;
+	),
+
+	TP_printk("guest executed wf%c at: 0x%08lx",
+		  __entry->is_wfe ? 'e' : 'i', __entry->vcpu_pc)
+);
+
+TRACE_EVENT(kvm_hvc_arm64,
+	TP_PROTO(unsigned long vcpu_pc, unsigned long r0, unsigned long imm),
+	TP_ARGS(vcpu_pc, r0, imm),
+
+	TP_STRUCT__entry(
+		__field(unsigned long, vcpu_pc)
+		__field(unsigned long, r0)
+		__field(unsigned long, imm)
+	),
+
+	TP_fast_assign(
+		__entry->vcpu_pc = vcpu_pc;
+		__entry->r0 = r0;
+		__entry->imm = imm;
+	),
+
+	TP_printk("HVC at 0x%08lx (r0: 0x%08lx, imm: 0x%lx)",
+		  __entry->vcpu_pc, __entry->r0, __entry->imm)
+);
+
+#endif /* _TRACE_ARM64_KVM_H */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE trace
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>