On 01/12/2015 03:30 AM, Christoffer Dall wrote:
> On Mon, Jan 12, 2015 at 09:18:52AM +0000, Andre Przywara wrote:
>> Hi,
>>
>> On 11/01/15 18:42, Christoffer Dall wrote:
>>> On Thu, Jan 08, 2015 at 01:33:35PM -0600, Wei Huang wrote:
>>>> On 01/07/2015 02:27 PM, Wei Huang wrote:
>>>>> 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
>>>>> for arm64 and linking new trace points with related functions.
>>>>>
>>>>> Signed-off-by: Wei Huang <wei@xxxxxxxxxx>
>>>>> ---
>>>>> arch/arm64/include/asm/kvm_arm.h | 2 ++
>>>>> arch/arm64/include/asm/kvm_emulate.h | 5 ++++
>>>>> arch/arm64/kvm/handle_exit.c | 8 ++++++
>>>>> arch/arm64/kvm/trace.h | 52 ++++++++++++++++++++++++++++++++++++
>>>>> 4 files changed, 67 insertions(+)
>>>>> 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 7fd3e27..e3e2350 100644
>>>>> --- a/arch/arm64/include/asm/kvm_arm.h
>>>>> +++ b/arch/arm64/include/asm/kvm_arm.h
>>>>> @@ -256,4 +256,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..4fdb907 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,6 +67,8 @@ 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)
>>>>> {
>>>>> + trace_kvm_wfi_arm64(*vcpu_pc(vcpu));
>>>>> +
>>>>> if (kvm_vcpu_get_hsr(vcpu) & ESR_EL2_EC_WFI_ISS_WFE)
>>>>> kvm_vcpu_on_spin(vcpu);
>>>>> else
>>>>> diff --git a/arch/arm64/kvm/trace.h b/arch/arm64/kvm/trace.h
>>>>> new file mode 100644
>>>>> index 0000000..96c7de0
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/kvm/trace.h
>>>>> @@ -0,0 +1,52 @@
>>>>> +#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_wfi_arm64,
>>>>> + TP_PROTO(unsigned long vcpu_pc),
>>>>> + TP_ARGS(vcpu_pc),
>>>>> +
>>>>> + TP_STRUCT__entry(
>>>>> + __field(unsigned long, vcpu_pc)
>>>>> + ),
>>>>> +
>>>>> + TP_fast_assign(
>>>>> + __entry->vcpu_pc = vcpu_pc;
>>>>> + ),
>>>>> +
>>>>> + TP_printk("guest executed wfxx at: 0x%08lx", __entry->vcpu_pc)
>>>>
>>>> I realized that there is a typo in TP_printk. "guest executed wfxx" is
>>>> supposed to be "guest executed wfi". Sorry. That is a debug string
>>>> accidentally left behind. Please correct it (or I can send another patch
>>>> if requested).
>>>>
>>>
>>> it really should all be _wfx, not wfi, and we should identify if it's a
>>> wfi or wfe we're seeing, now when we're at it. (It could be good for
>>> counting stats too).
>>
>> I have a patch for this in my queue. I will rebase it on top of Wei's
>> patch and send it out ASAP.
>>
> huh? I was thinking Wei just fixes up his patch and sends it with the
> required changes, not sure if we need to introduce more patches. But
> whatever...
>
Just sent a new version, combining both Andre's patch and mine. It only
covers arm64 part. Andre will send out the rest that touches 32-bit part.
Thanks,
-Wei
> -Christoffer
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
