Re: [PATCH] perf kvm: Fix arm64 VM-EXIT event detection

Re: [PATCH] perf kvm: Fix arm64 VM-EXIT event detection

[Zenghui Yu]

[ +Cc the original author of perf kvm stat ]

On 2023/8/8 11:57, Haixin Yu wrote:
> A VM-EXIT event shall start at kvm:kvm_exit, and end with the next
> kvm:kvm_entry. But it's reversed on arm64, which means the time running
> guest code in fact.

Good catch! It'd be great if Sergey could take a look.

> 
> Example:
> 
>  # perf kvm stat record -p 2772 -- sleep 1
> 
> Before this change:
> 
>  # perf kvm stat report
> 
> Analyze events for all VMs, all VCPUs:
> 
>     VM-EXIT Samples Samples%   Time%   Min Time   Max Time    Avg time
> 
>     IRQ      756     100.00%  100.00%   2.28us    3674.38us   1375.66us ( +-   4.34% )
> 
> Total Samples:756, Total events handled time:1039996.94us.
> 
> After:
> 
>  # perf kvm stat report
> 
> Analyze events for all VMs, all VCPUs:
> 
>     VM-EXIT Samples Samples%   Time%   Min Time   Max Time    Avg time
> 
>     IRQ     772      100.00%  100.00%   0.24us     62.86us    6.34us ( +-   3.55% )
> 
> Total Samples:772, Total events handled time:4890.80us.
> 
> IRQ average handled time decreases to 6us, which is a more rational result
> as KVM handles IRQ by returning to guest directly.
> 
> Signed-off-by: Haixin Yu <yuhaixin.yhx@linux.alibaba.com>
> ---
>  tools/perf/arch/arm64/util/kvm-stat.c | 24 +++---------------------
>  tools/perf/builtin-kvm.c              |  2 +-
>  2 files changed, 4 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/perf/arch/arm64/util/kvm-stat.c b/tools/perf/arch/arm64/util/kvm-stat.c
> index 6611aa21cba9..eeb659ceedc2 100644
> --- a/tools/perf/arch/arm64/util/kvm-stat.c
> +++ b/tools/perf/arch/arm64/util/kvm-stat.c
> @@ -21,7 +21,7 @@ const char *kvm_events_tp[] = {
>  	NULL,
>  };
>  
> -static void event_get_key(struct evsel *evsel,
> +void exit_event_get_key(struct evsel *evsel,
>  			  struct perf_sample *sample,
>  			  struct event_key *key)
>  {
> @@ -40,27 +40,9 @@ static void event_get_key(struct evsel *evsel,
>  	}
>  }
>  
> -static bool event_begin(struct evsel *evsel,
> -			struct perf_sample *sample __maybe_unused,
> -			struct event_key *key __maybe_unused)
> -{
> -	return evsel__name_is(evsel, kvm_entry_trace);
> -}
> -
> -static bool event_end(struct evsel *evsel,
> -		      struct perf_sample *sample,
> -		      struct event_key *key)
> -{
> -	if (evsel__name_is(evsel, kvm_exit_trace)) {
> -		event_get_key(evsel, sample, key);
> -		return true;
> -	}
> -	return false;
> -}
> -
>  static struct kvm_events_ops exit_events = {
> -	.is_begin_event = event_begin,
> -	.is_end_event	= event_end,
> +	.is_begin_event = exit_event_begin,
> +	.is_end_event	= exit_event_end,
>  	.decode_key	= exit_event_decode_key,
>  	.name		= "VM-EXIT"
>  };
> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index 71165036e4ca..498cd8e21134 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -615,7 +615,7 @@ static const char *get_filename_for_perf_kvm(void)
>  
>  #if defined(HAVE_KVM_STAT_SUPPORT) && defined(HAVE_LIBTRACEEVENT)
>  
> -void exit_event_get_key(struct evsel *evsel,
> +void __weak exit_event_get_key(struct evsel *evsel,
>  			struct perf_sample *sample,
>  			struct event_key *key)
>  {