On 01/07/2015 08:13 AM, Christoffer Dall wrote:
> On Wed, Jan 07, 2015 at 12:01:14PM +0000, Marc Zyngier wrote:
>> Hi Wei,
>>
>> On 07/01/15 07:29, Wei Huang wrote:
>>> arm64 uses its own copy of exit handler (arm64/kvm/handle_exit.c).
>>> Currently this file doesn't hook up with tracepoints defined in
>>> arm/kvm/trace.h file. As a result users might not see certain tracing
>>> events (e.g. HVC & WFI) while using ftrace with arm64 KVM. This patch
>>> fixes this issue by adding related tracepoints in arm64/kvm/handle_exit.c
>>> file.
>>>
>>> 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 | 7 +++++++
>>> 3 files changed, 14 insertions(+)
>>>
>>> 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..ccdb781 100644
>>> --- a/arch/arm64/kvm/handle_exit.c
>>> +++ b/arch/arm64/kvm/handle_exit.c
>>> @@ -26,12 +26,17 @@
>>> #include <asm/kvm_mmu.h>
>>> #include <asm/kvm_psci.h>
>>>
>>> +#include "../../arm/kvm/trace.h"
>>
>> I'm not overly fond of this. The usual split between arm and arm64 is
>> that we share the C code, but have private includes. I know it is not
>> ideal, but it looks way better than this.
>>
>> How about creating a local trace.h that only contains the trace points
>> for arm64?
>>
> Agreed, it sounds better given the circumstances.
>
> Otherwise the content of the patch is fine with me.
Just sent; please review. Few points:
* New tracing file is created as arm64/kvm/trace.h;
* In order to force inclusion of new trace.h file, CREATE_TRACE_POINTS
has to be re-defined in arm64/kvm/handle_exit.c. Otherwise compilation
error;
* Names of new trace points had been changed to kvm_blahblah_arm64 in
order to avoid conflicts with existing functions in arm/kvm/trace.h. Not
perfect, I know;
* __field of TP_STRUCT__entry is cleaned up in arm64/kvm/trace.h,
compared with arm/kvm/trace.h. Otherwise checkpatch.pl would complain;
* Tested on ARM64 machine.
Thanks,
-Wei
>
> -Christoffer
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
