diff mbox series

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

Message ID 20220628093203.73160-2-weijiang.yang@intel.com (mailing list archive)
State New, archived
Headers show
Series [kvm-unit-tests,v4,1/2] x86: Skip perf related tests when platform cannot support | expand

Commit Message

Yang, Weijiang June 28, 2022, 9:32 a.m. UTC
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]
---
 lib/x86/processor.h |  1 +
 x86/pmu_lbr.c       | 32 +++++++++++++-------------------
 2 files changed, 14 insertions(+), 19 deletions(-)

Comments

Sean Christopherson July 7, 2022, 8:28 p.m. UTC | #1
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
>
Yang, Weijiang July 8, 2022, 12:57 a.m. UTC | #2
On 7/8/2022 4:28 AM, Sean Christopherson wrote:
> 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>
Will change it.
>
>>   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()) {
Yep.
>>   		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());
Will change it. Thank you!
>
>>   	printf("LBR version:		 %ld\n", perf_cap & PMU_CAP_LBR_FMT);
>>   
>>   	/* Look for LBR from and to MSRs */
>> -- 
>> 2.27.0
>>
diff mbox series

Patch

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) {
 		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);
 	printf("LBR version:		 %ld\n", perf_cap & PMU_CAP_LBR_FMT);
 
 	/* Look for LBR from and to MSRs */