Re: [kvm-unit-tests PATCH v4 2/2] x86: Check platform vPMU capabilities before run lbr tests

[Sean Christopherson <seanjc@google.com>]

On Tue, Jun 28, 2022, Yang Weijiang wrote:
> Use new helper to check whether pmu is available and Perfmon/Debug
> capbilities are supported before read MSR_IA32_PERF_CAPABILITIES to
> avoid test failure. The issue can be captured when enable_pmu=0.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> 
> v4:
> - Put the X86_FEATURE_PDCM to the right place. [Sean]
> ---

<version info goes here>

>  lib/x86/processor.h |  1 +
>  x86/pmu_lbr.c       | 32 +++++++++++++-------------------
>  2 files changed, 14 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/x86/processor.h b/lib/x86/processor.h
> index 7b6ee92..7a35c7f 100644
> --- a/lib/x86/processor.h
> +++ b/lib/x86/processor.h
> @@ -146,6 +146,7 @@ static inline bool is_intel(void)
>   */
>  #define	X86_FEATURE_MWAIT		(CPUID(0x1, 0, ECX, 3))
>  #define	X86_FEATURE_VMX			(CPUID(0x1, 0, ECX, 5))
> +#define	X86_FEATURE_PDCM		(CPUID(0x1, 0, ECX, 15))
>  #define	X86_FEATURE_PCID		(CPUID(0x1, 0, ECX, 17))
>  #define	X86_FEATURE_MOVBE		(CPUID(0x1, 0, ECX, 22))
>  #define	X86_FEATURE_TSC_DEADLINE_TIMER	(CPUID(0x1, 0, ECX, 24))
> diff --git a/x86/pmu_lbr.c b/x86/pmu_lbr.c
> index 688634d..497df1e 100644
> --- a/x86/pmu_lbr.c
> +++ b/x86/pmu_lbr.c
> @@ -15,6 +15,7 @@
>  #define MSR_LBR_SELECT		0x000001c8
>  
>  volatile int count;
> +u32 lbr_from, lbr_to;
>  
>  static noinline int compute_flag(int i)
>  {
> @@ -38,18 +39,6 @@ static noinline int lbr_test(void)
>  	return 0;
>  }
>  
> -union cpuid10_eax {
> -	struct {
> -		unsigned int version_id:8;
> -		unsigned int num_counters:8;
> -		unsigned int bit_width:8;
> -		unsigned int mask_length:8;
> -	} split;
> -	unsigned int full;
> -} eax;
> -
> -u32 lbr_from, lbr_to;
> -
>  static void init_lbr(void *index)
>  {
>  	wrmsr(lbr_from + *(int *) index, 0);
> @@ -63,7 +52,7 @@ static bool test_init_lbr_from_exception(u64 index)
>  
>  int main(int ac, char **av)
>  {
> -	struct cpuid id = cpuid(10);
> +	u8 version = pmu_version();
>  	u64 perf_cap;
>  	int max, i;
>  
> @@ -74,19 +63,24 @@ int main(int ac, char **av)
>  		return 0;
>  	}
>  
> -	perf_cap = rdmsr(MSR_IA32_PERF_CAPABILITIES);
> -	eax.full = id.a;
> -
> -	if (!eax.split.version_id) {
> +	if (!version) {

If the previous patch exposes cpu_has_pmu(), then this open coded check goes away
in favor of the more obvious:

	if (!cpu_has_pmu()) {

>  		printf("No pmu is detected!\n");
>  		return report_summary();
>  	}
> +
> +	if (!this_cpu_has(X86_FEATURE_PDCM)) {
> +		printf("Perfmon/Debug Capabilities MSR isn't supported\n");
> +		return report_summary();
> +	}
> +
> +	perf_cap = rdmsr(MSR_IA32_PERF_CAPABILITIES);
> +
>  	if (!(perf_cap & PMU_CAP_LBR_FMT)) {
> -		printf("No LBR is detected!\n");
> +		printf("(Architectural) LBR is not supported.\n");
>  		return report_summary();
>  	}
>  
> -	printf("PMU version:		 %d\n", eax.split.version_id);
> +	printf("PMU version:		 %d\n", version);

And with the open coded check gone, this can be:
	
	printf("PMU version:		 %d\n", pmu_version());

>  	printf("LBR version:		 %ld\n", perf_cap & PMU_CAP_LBR_FMT);
>  
>  	/* Look for LBR from and to MSRs */
> -- 
> 2.27.0
>