Re: [kvm-unit-tests PATCH v3 13/13] x86/pmu: Update testcases to cover AMD PMU

[Sean Christopherson <seanjc@google.com>]

On Fri, Aug 19, 2022, Like Xu wrote:
> diff --git a/lib/x86/processor.h b/lib/x86/processor.h
> index 0324220..10bca27 100644
> --- a/lib/x86/processor.h
> +++ b/lib/x86/processor.h
> @@ -793,6 +793,9 @@ static inline void flush_tlb(void)
>  
>  static inline u8 pmu_version(void)
>  {
> +	if (!is_intel())
> +		return 0;
> +
>  	return cpuid(10).a & 0xff;
>  }
>  
> @@ -806,19 +809,39 @@ static inline bool this_cpu_has_perf_global_ctrl(void)
>  	return pmu_version() > 1;
>  }
>  
> +#define AMD64_NUM_COUNTERS                             4
> +#define AMD64_NUM_COUNTERS_CORE                                6
> +
> +static inline bool has_amd_perfctr_core(void)
> +{
> +	return cpuid(0x80000001).c & BIT_ULL(23);

Add an X86_FEATURE_*, maybe X86_FEATURE_AMD_PERF_EXTENSIONS?

> +}
> +
>  static inline u8 pmu_nr_gp_counters(void)
>  {
> -	return (cpuid(10).a >> 8) & 0xff;
> +	if (is_intel()) {

No curly braces.

> +		return (cpuid(10).a >> 8) & 0xff;
> +	} else if (!has_amd_perfctr_core()) {

Drop the "else", the above "if" is terminal.

> +		return AMD64_NUM_COUNTERS;
> +	}
> +
> +	return AMD64_NUM_COUNTERS_CORE;
>  }
>  
>  static inline u8 pmu_gp_counter_width(void)
>  {
> -	return (cpuid(10).a >> 16) & 0xff;
> +	if (is_intel())
> +		return (cpuid(10).a >> 16) & 0xff;
> +	else
> +		return 48;

Please add a #define for this magic number.

>  }
>  
>  static inline u8 pmu_gp_counter_mask_length(void)
>  {
> -	return (cpuid(10).a >> 24) & 0xff;
> +	if (is_intel())
> +		return (cpuid(10).a >> 24) & 0xff;
> +	else
> +		return pmu_nr_gp_counters();
>  }
>  
>  static inline u8 pmu_nr_fixed_counters(void)
> @@ -843,6 +866,9 @@ static inline u8 pmu_fixed_counter_width(void)
>  
>  static inline bool pmu_gp_counter_is_available(int i)
>  {
> +	if (!is_intel())
> +		return i < pmu_nr_gp_counters();
> +
>  	/* CPUID.0xA.EBX bit is '1 if they counter is NOT available. */
>  	return !(cpuid(10).b & BIT(i));
>  }
> diff --git a/x86/pmu.c b/x86/pmu.c
> index 0706cb1..b6ab10c 100644
> --- a/x86/pmu.c
> +++ b/x86/pmu.c
> @@ -62,6 +62,11 @@ struct pmu_event {
>  	{"fixed 1", MSR_CORE_PERF_FIXED_CTR0, 10*N, 10.2*N},
>  	{"fixed 2", MSR_CORE_PERF_FIXED_CTR0 + 1, 1*N, 30*N},
>  	{"fixed 3", MSR_CORE_PERF_FIXED_CTR0 + 2, 0.1*N, 30*N}
> +}, amd_gp_events[] = {
> +	{"core cycles", 0x0076, 1*N, 50*N},
> +	{"instructions", 0x00c0, 10*N, 10.2*N},
> +	{"branches", 0x00c2, 1*N, 1.1*N},
> +	{"branch misses", 0x00c3, 0, 0.1*N},
>  };
>  
>  #define PMU_CAP_FW_WRITES	(1ULL << 13)
> @@ -105,14 +110,24 @@ static bool check_irq(void)
>  
>  static bool is_gp(pmu_counter_t *evt)
>  {
> +	if (!is_intel())
> +		return true;
> +
>  	return evt->ctr < MSR_CORE_PERF_FIXED_CTR0 ||
>  		evt->ctr >= MSR_IA32_PMC0;
>  }
>  
>  static int event_to_global_idx(pmu_counter_t *cnt)
>  {
> -	return cnt->ctr - (is_gp(cnt) ? gp_counter_base :
> -		(MSR_CORE_PERF_FIXED_CTR0 - FIXED_CNT_INDEX));
> +	if (is_intel())
> +		return cnt->ctr - (is_gp(cnt) ? gp_counter_base :
> +			(MSR_CORE_PERF_FIXED_CTR0 - FIXED_CNT_INDEX));
> +
> +	if (gp_counter_base == MSR_F15H_PERF_CTR0) {

Unnecessary curly braces.

> +		return (cnt->ctr - gp_counter_base) / 2;
> +	} else {
> +		return cnt->ctr - gp_counter_base;
> +	}
>  }
>  
>  static struct pmu_event* get_counter_event(pmu_counter_t *cnt)
> @@ -736,5 +783,11 @@ int main(int ac, char **av)
>  		report_prefix_pop();
>  	}
>  
> +	if (!is_intel()) {
> +		report_prefix_push("K7");
> +		amd_switch_to_non_perfctr_core();
> +		check_counters();

"K7" prefix needs to be popped.

> +	}
> +
>  	return report_summary();
>  }
> -- 
> 2.37.2
>