Re: [PATCH v2 2/6] perf kvm stat: Remove use of the arch directory

[Leo Yan <leo.yan@linux.dev>]

On Sat, Jan 31, 2026 at 12:02:20PM -0800, Ian Rogers wrote:

[...]

> @@ -1666,7 +1661,7 @@ kvm_events_record(struct perf_kvm_stat *kvm, int argc, const char **argv)
>  		return ret;
>  	}
>  
> -	for (events_tp = kvm_events_tp; *events_tp; events_tp++)
> +	for (events_tp = kvm_events_tp(); *events_tp; events_tp++)
>  		events_tp_size++;
>  
>  	rec_argc = ARRAY_SIZE(record_args) + argc + 2 +
> @@ -1681,7 +1676,7 @@ kvm_events_record(struct perf_kvm_stat *kvm, int argc, const char **argv)
>  
>  	for (j = 0; j < events_tp_size; j++) {
>  		rec_argv[i++] = STRDUP_FAIL_EXIT("-e");
> -		rec_argv[i++] = STRDUP_FAIL_EXIT(kvm_events_tp[j]);
> +		rec_argv[i++] = STRDUP_FAIL_EXIT(kvm_events_tp()[j]);
>  	}

Nitpick: we can assign reuse events_tp throughout the
kvm_events_record().  Something like:

  events_tp = kvm_events_tp();
  for (j = 0; events_tp[j]; j++)
       events_tp_size++;

  ...

  for (j = 0; j < events_tp_size; j++) {
       rec_argv[i++] = STRDUP_FAIL_EXIT("-e");
       rec_argv[i++] = STRDUP_FAIL_EXIT(events_tp[j]);
  }

[...]

> +int setup_kvm_events_tp(struct perf_kvm_stat *kvm)
> +{
> +	switch (EM_HOST) {
> +	case EM_PPC:
> +	case EM_PPC64:
> +		return __setup_kvm_events_tp_powerpc(kvm);
> +	default:
> +		return 0;
> +	}
> +}
> +
> +int cpu_isa_init(struct perf_kvm_stat *kvm, const char *cpuid)
> +{
> +	switch (EM_HOST) {
> +	case EM_AARCH64:
> +		return __cpu_isa_init_arm64(kvm);
> +	case EM_LOONGARCH:
> +		return __cpu_isa_init_loongarch(kvm);
> +	case EM_PPC:
> +	case EM_PPC64:
> +		return __cpu_isa_init_powerpc(kvm);
> +	case EM_RISCV:
> +		return __cpu_isa_init_riscv(kvm);
> +	case EM_S390:
> +		return __cpu_isa_init_s390(kvm, cpuid);
> +	case EM_X86_64:
> +	case EM_386:
> +		return __cpu_isa_init_x86(kvm, cpuid);
> +	default:
> +		pr_err("Unsupported kvm-stat host %d\n", EM_HOST);
> +		return -1;
> +	}
> +}

For a general solution, I'd prefer to use "install" + callback methods
rather than the opened code.  E.g., each arch installs a structure
with callbacks:

  struct kvm_stat_arch kvm_stat_aarch64 {
      .setup_events_tp = NULL;
      .cpu_isa_init    = __cpu_isa_init_arm64;
      ...
  };

Then at the init phase, we can install arch's structure:

  switch (EM_HOST) {
  case EM_AARCH64:
      kvm_stat_arch_init(&kvm_stat_aarch64);
      break;
  ...
  }

Afterwards, it is no need to check EM_HOST anymore, it can simply
invoke the arch's callback:

  int cpu_isa_init(struct perf_kvm_stat *kvm, const char *cpuid)
  {
      if (!arch_kvm_stat || !arch_kvm_stat->cpu_isa_init)
          return -1;

      return arch_kvm_stat->cpu_isa_init(kvm, cpuid);
  }

As a result, we can avoid spreading "switch (EM_HOST)" everywhere.

Thanks,
Leo