Hi Jing,
On Thu, 17 Mar 2022 00:56:30 +0000,
Jing Zhang <jingzhangos@xxxxxxxxxx> wrote:
>
> This tracepoint only provides a hook for poking vcpu exits information,
> not exported to tracefs.
> A timestamp is added for the last vcpu exit, which would be useful for
> analysis for vcpu exits.
The trace itself gives you a timestamp. Why do you need an extra one?
>
> Signed-off-by: Jing Zhang <jingzhangos@xxxxxxxxxx>
> ---
> arch/arm64/include/asm/kvm_host.h | 3 +++
> arch/arm64/kvm/arm.c | 2 ++
> arch/arm64/kvm/trace_arm.h | 8 ++++++++
> 3 files changed, 13 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index daa68b053bdc..576f2c18d008 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -415,6 +415,9 @@ struct kvm_vcpu_arch {
>
> /* Arch specific exit reason */
> enum arm_exit_reason exit_reason;
> +
> + /* Timestamp for the last vcpu exit */
> + u64 last_exit_time;
> };
>
> /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index f49ebdd9c990..98631f79c182 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -783,6 +783,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> ret = 1;
> run->exit_reason = KVM_EXIT_UNKNOWN;
> while (ret > 0) {
> + trace_kvm_vcpu_exits(vcpu);
Exit? We haven't entered the guest yet!
> /*
> * Check conditions before entering the guest
> */
> @@ -898,6 +899,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> local_irq_enable();
>
> trace_kvm_exit(ret, kvm_vcpu_trap_get_class(vcpu), *vcpu_pc(vcpu));
> + vcpu->arch.last_exit_time = ktime_to_ns(ktime_get());
Why isn't the above tracepoint sufficient? It gives you the EC, and
comes with a timestamp for free. And why should *everyone* pay the
price of this timestamp update if not tracing?
>
> /* Exit types that need handling before we can be preempted */
> handle_exit_early(vcpu, ret);
> diff --git a/arch/arm64/kvm/trace_arm.h b/arch/arm64/kvm/trace_arm.h
> index 33e4e7dd2719..3e7dfd640e23 100644
> --- a/arch/arm64/kvm/trace_arm.h
> +++ b/arch/arm64/kvm/trace_arm.h
> @@ -301,6 +301,14 @@ TRACE_EVENT(kvm_timer_emulate,
> __entry->timer_idx, __entry->should_fire)
> );
>
> +/*
> + * Following tracepoints are not exported in tracefs and provide hooking
> + * mechanisms only for testing and debugging purposes.
> + */
> +DECLARE_TRACE(kvm_vcpu_exits,
> + TP_PROTO(struct kvm_vcpu *vcpu),
> + TP_ARGS(vcpu));
> +
> #endif /* _TRACE_ARM_ARM64_KVM_H */
>
> #undef TRACE_INCLUDE_PATH
I guess this is the only bit I actually like about this series: a
generic, out of the way mechanism to let people hook whatever they
want and dump the state they need.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
