[kvm-unit-tests PATCH v5 1/3] x86: Use report_skip to log messages when tests are skipped

[kvm-unit-tests PATCH v5 1/3] x86: Use report_skip to log messages when tests are skipped

[Yang Weijiang]

report_skip() prints message with "SKIP:" prefix to explictly
tell which test is skipped, making the report screening easier.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 x86/vmx_tests.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 4d581e7..27ab5ed 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -4107,7 +4107,7 @@ static void test_vpid(void)
 	int i;
 
 	if (!is_vpid_supported()) {
-		printf("Secondary controls and/or VPID not supported\n");
+		report_skip("Secondary controls and/or VPID not supported\n");
 		return;
 	}
 
@@ -4614,7 +4614,7 @@ static void test_nmi_ctrls(void)
 
 	if ((ctrl_pin_rev.clr & (PIN_NMI | PIN_VIRT_NMI)) !=
 	    (PIN_NMI | PIN_VIRT_NMI)) {
-		printf("NMI exiting and Virtual NMIs are not supported !\n");
+		report_skip("NMI exiting and Virtual NMIs are not supported !\n");
 		return;
 	}
 
@@ -4724,7 +4724,7 @@ static void test_ept_eptp(void)
 
 	if (!((ctrl_cpu_rev[0].clr & CPU_SECONDARY) &&
 	    (ctrl_cpu_rev[1].clr & CPU_EPT))) {
-		printf("\"CPU secondary\" and/or \"enable EPT\" execution controls are not supported !\n");
+		report_skip("\"CPU secondary\" and/or \"enable EPT\" execution controls are not supported !\n");
 		return;
 	}
 
@@ -4884,7 +4884,7 @@ static void test_pml(void)
 
 	if (!((ctrl_cpu_rev[0].clr & CPU_SECONDARY) &&
 	    (ctrl_cpu_rev[1].clr & CPU_EPT) && (ctrl_cpu_rev[1].clr & CPU_PML))) {
-		printf("\"Secondary execution\" control or \"enable EPT\" control or \"enable PML\" control is not supported !\n");
+		report_skip("\"Secondary execution\" control or \"enable EPT\" control or \"enable PML\" control is not supported !\n");
 		return;
 	}
 
@@ -4936,7 +4936,7 @@ static void test_vmx_preemption_timer(void)
 
 	if (!((ctrl_exit_rev.clr & EXI_SAVE_PREEMPT) ||
 	    (ctrl_pin_rev.clr & PIN_PREEMPT))) {
-		printf("\"Save-VMX-preemption-timer\" control and/or \"Enable-VMX-preemption-timer\" control is not supported\n");
+		report_skip("\"Save-VMX-preemption-timer\" control and/or \"Enable-VMX-preemption-timer\" control is not supported\n");
 		return;
 	}
 
@@ -5060,7 +5060,7 @@ static void vmx_mtf_test(void)
 	handler old_gp, old_db;
 
 	if (!(ctrl_cpu_rev[0].clr & CPU_MTF)) {
-		printf("CPU does not support the 'monitor trap flag' processor-based VM-execution control.\n");
+		report_skip("CPU does not support the 'monitor trap flag' processor-based VM-execution control.\n");
 		return;
 	}
 
@@ -5163,12 +5163,12 @@ static void vmx_mtf_pdpte_test(void)
 		return;
 
 	if (!(ctrl_cpu_rev[0].clr & CPU_MTF)) {
-		printf("CPU does not support 'monitor trap flag.'\n");
+		report_skip("CPU does not support 'monitor trap flag.'\n");
 		return;
 	}
 
 	if (!(ctrl_cpu_rev[1].clr & CPU_URG)) {
-		printf("CPU does not support 'unrestricted guest.'\n");
+		report_skip("CPU does not support 'unrestricted guest.'\n");
 		return;
 	}
 
@@ -7175,7 +7175,7 @@ static void test_efer(u32 fld, const char * fld_name, u32 ctrl_fld,
 		efer_reserved_bits &= ~EFER_NX;
 
 	if (!ctrl_bit1) {
-		printf("\"Load-IA32-EFER\" exit control not supported\n");
+		report_skip("\"Load-IA32-EFER\" exit control not supported\n");
 		goto test_entry_exit_mode;
 	}
 
@@ -7258,7 +7258,7 @@ static void test_host_efer(void)
 static void test_guest_efer(void)
 {
 	if (!(ctrl_enter_rev.clr & ENT_LOAD_EFER)) {
-		printf("\"Load-IA32-EFER\" entry control not supported\n");
+		report_skip("\"Load-IA32-EFER\" entry control not supported\n");
 		return;
 	}
 
@@ -7349,7 +7349,7 @@ static void test_load_host_pat(void)
 	 * "load IA32_PAT" VM-exit control
 	 */
 	if (!(ctrl_exit_rev.clr & EXI_LOAD_PAT)) {
-		printf("\"Load-IA32-PAT\" exit control not supported\n");
+		report_skip("\"Load-IA32-PAT\" exit control not supported\n");
 		return;
 	}
 
@@ -7491,7 +7491,7 @@ static void test_perf_global_ctrl(u32 nr, const char *name, u32 ctrl_nr,
 static void test_load_host_perf_global_ctrl(void)
 {
 	if (!(ctrl_exit_rev.clr & EXI_LOAD_PERF)) {
-		printf("\"load IA32_PERF_GLOBAL_CTRL\" exit control not supported\n");
+		report_skip("\"load IA32_PERF_GLOBAL_CTRL\" exit control not supported\n");
 		return;
 	}
 
@@ -7503,7 +7503,7 @@ static void test_load_host_perf_global_ctrl(void)
 static void test_load_guest_perf_global_ctrl(void)
 {
 	if (!(ctrl_enter_rev.clr & ENT_LOAD_PERF)) {
-		printf("\"load IA32_PERF_GLOBAL_CTRL\" entry control not supported\n");
+		report_skip("\"load IA32_PERF_GLOBAL_CTRL\" entry control not supported\n");
 		return;
 	}
 
@@ -7809,7 +7809,7 @@ static void test_load_guest_pat(void)
 	 * "load IA32_PAT" VM-entry control
 	 */
 	if (!(ctrl_enter_rev.clr & ENT_LOAD_PAT)) {
-		printf("\"Load-IA32-PAT\" entry control not supported\n");
+		report_skip("\"Load-IA32-PAT\" entry control not supported\n");
 		return;
 	}
 
@@ -7833,7 +7833,7 @@ static void test_load_guest_bndcfgs(void)
 	u64 bndcfgs;
 
 	if (!(ctrl_enter_rev.clr & ENT_LOAD_BNDCFGS)) {
-		printf("\"Load-IA32-BNDCFGS\" entry control not supported\n");
+		report_skip("\"Load-IA32-BNDCFGS\" entry control not supported\n");
 		return;
 	}
 
@@ -9970,7 +9970,7 @@ static void sipi_test_ap_thread(void *data)
 static void vmx_sipi_signal_test(void)
 {
 	if (!(rdmsr(MSR_IA32_VMX_MISC) & MSR_IA32_VMX_MISC_ACTIVITY_WAIT_SIPI)) {
-		printf("\tACTIVITY_WAIT_SIPI state is not supported.\n");
+		report_skip("\tACTIVITY_WAIT_SIPI state is not supported.\n");
 		return;
 	}
 
@@ -10271,18 +10271,18 @@ static void vmx_vmcs_shadow_test(void)
 	struct vmcs *shadow;
 
 	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY)) {
-		printf("\t'Activate secondary controls' not supported.\n");
+		report_skip("\t'Activate secondary controls' not supported.\n");
 		return;
 	}
 
 	if (!(ctrl_cpu_rev[1].clr & CPU_SHADOW_VMCS)) {
-		printf("\t'VMCS shadowing' not supported.\n");
+		report_skip("\t'VMCS shadowing' not supported.\n");
 		return;
 	}
 
 	if (!(rdmsr(MSR_IA32_VMX_MISC) &
 	      MSR_IA32_VMX_MISC_VMWRITE_SHADOW_RO_FIELDS)) {
-		printf("\tVMWRITE can't modify VM-exit information fields.\n");
+		report_skip("\tVMWRITE can't modify VM-exit information fields.\n");
 		return;
 	}
 

base-commit: ca85dda2671e88d34acfbca6de48a9ab32b1810d
-- 
2.31.1

[kvm-unit-tests PATCH v5 2/3] x86: Skip perf related tests when platform cannot support

[Yang Weijiang]

Add helpers to check whether MSR_CORE_PERF_GLOBAL_CTRL and rdpmc are
supported in KVM. When pmu is disabled with enable_pmu=0, reading
MSR_CORE_PERF_GLOBAL_CTRL or executing rdpmc leads to #GP, so skip
related tests in this case to avoid test failure.

Opportunistically hoist mwait check function as helper and change
related code.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>

---

v5:
 1. Move cleanup changes to another separated pre-patch.[Sean]
 2. Hoist pmu and mwait capability checks as helpers.

 lib/x86/processor.h | 20 ++++++++++++++++++++
 x86/vmx_tests.c     | 21 +++++++++++++--------
 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 9a0dad6..59cedc9 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -690,4 +690,24 @@ static inline bool cpuid_osxsave(void)
 	return cpuid(1).c & (1 << (X86_FEATURE_OSXSAVE % 32));
 }
 
+static inline u8 pmu_version(void)
+{
+	return cpuid(10).a & 0xff;
+}
+
+static inline bool cpu_has_perf_global_ctrl(void)
+{
+	return pmu_version() > 1;
+}
+
+static inline bool cpu_has_pmu(void)
+{
+	return !!pmu_version();
+}
+
+static inline bool cpu_has_mwait(void)
+{
+	return this_cpu_has(X86_FEATURE_MWAIT);
+}
+
 #endif
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 27ab5ed..cddee1e 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -847,11 +847,6 @@ u64 cr3;
 
 typedef bool (*supported_fn)(void);
 
-static bool monitor_supported(void)
-{
-	return this_cpu_has(X86_FEATURE_MWAIT);
-}
-
 struct insn_table {
 	const char *name;
 	u32 flag;
@@ -880,8 +875,8 @@ static struct insn_table insn_table[] = {
 	{"HLT",  CPU_HLT, insn_hlt, INSN_CPU0, 12, 0, 0, 0},
 	{"INVLPG", CPU_INVLPG, insn_invlpg, INSN_CPU0, 14,
 		0x12345678, 0, FIELD_EXIT_QUAL},
-	{"MWAIT", CPU_MWAIT, insn_mwait, INSN_CPU0, 36, 0, 0, 0, &monitor_supported},
-	{"RDPMC", CPU_RDPMC, insn_rdpmc, INSN_CPU0, 15, 0, 0, 0},
+	{"MWAIT", CPU_MWAIT, insn_mwait, INSN_CPU0, 36, 0, 0, 0, &cpu_has_mwait},
+	{"RDPMC", CPU_RDPMC, insn_rdpmc, INSN_CPU0, 15, 0, 0, 0, &cpu_has_pmu},
 	{"RDTSC", CPU_RDTSC, insn_rdtsc, INSN_CPU0, 16, 0, 0, 0},
 	{"CR3 load", CPU_CR3_LOAD, insn_cr3_load, INSN_CPU0, 28, 0x3, 0,
 		FIELD_EXIT_QUAL},
@@ -891,7 +886,7 @@ static struct insn_table insn_table[] = {
 		FIELD_EXIT_QUAL},
 	{"CR8 store", CPU_CR8_STORE, insn_cr8_store, INSN_CPU0, 28, 0x18, 0,
 		FIELD_EXIT_QUAL},
-	{"MONITOR", CPU_MONITOR, insn_monitor, INSN_CPU0, 39, 0, 0, 0, &monitor_supported},
+	{"MONITOR", CPU_MONITOR, insn_monitor, INSN_CPU0, 39, 0, 0, 0, &cpu_has_mwait},
 	{"PAUSE", CPU_PAUSE, insn_pause, INSN_CPU0, 40, 0, 0, 0},
 	// Flags for Secondary Processor-Based VM-Execution Controls
 	{"WBINVD", CPU_WBINVD, insn_wbinvd, INSN_CPU1, 54, 0, 0, 0},
@@ -7490,6 +7485,11 @@ static void test_perf_global_ctrl(u32 nr, const char *name, u32 ctrl_nr,
 
 static void test_load_host_perf_global_ctrl(void)
 {
+	if (!cpu_has_perf_global_ctrl()) {
+		report_skip("IA32_PERF_GLOBAL_CTRL not supported\n");
+		return;
+	}
+
 	if (!(ctrl_exit_rev.clr & EXI_LOAD_PERF)) {
 		report_skip("\"load IA32_PERF_GLOBAL_CTRL\" exit control not supported\n");
 		return;
@@ -7502,6 +7502,11 @@ static void test_load_host_perf_global_ctrl(void)
 
 static void test_load_guest_perf_global_ctrl(void)
 {
+	if (!cpu_has_perf_global_ctrl()) {
+		report_skip("IA32_PERF_GLOBAL_CTRL not supported\n");
+		return;
+	}
+
 	if (!(ctrl_enter_rev.clr & ENT_LOAD_PERF)) {
 		report_skip("\"load IA32_PERF_GLOBAL_CTRL\" entry control not supported\n");
 		return;
-- 
2.31.1

[kvm-unit-tests PATCH v5 3/3] x86: Check platform vPMU capabilities before run lbr tests

[Yang Weijiang]

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>

---

v5:
 Use new helpers to check pmu availability and get pmu version.[Sean]

 lib/x86/processor.h |  1 +
 x86/pmu_lbr.c       | 31 ++++++++++++-------------------
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 59cedc9..b3b22b8 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..c6ddabf 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,6 @@ static bool test_init_lbr_from_exception(u64 index)
 
 int main(int ac, char **av)
 {
-	struct cpuid id = cpuid(10);
 	u64 perf_cap;
 	int max, i;
 
@@ -74,19 +62,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 (!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", pmu_version());
 	printf("LBR version:		 %ld\n", perf_cap & PMU_CAP_LBR_FMT);
 
 	/* Look for LBR from and to MSRs */
-- 
2.31.1

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

[Sean Christopherson]

On Fri, Jul 08, 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>
> 
> ---
> 
> v5:
>  Use new helpers to check pmu availability and get pmu version.[Sean]
> 
>  lib/x86/processor.h |  1 +
>  x86/pmu_lbr.c       | 31 ++++++++++++-------------------

What about x86/pmu.c?  It has pretty much all the same issues.

> @@ -74,19 +62,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 (!cpu_has_pmu()) {
>  		printf("No pmu is detected!\n");

Please opportunistically switch these to report_skip() (and drop the \n), or fix
them in patch 1 too.  Either way is fine.  And obviously use report_skip() for
the new PDCM check.

Re: [kvm-unit-tests PATCH v5 1/3] x86: Use report_skip to log messages when tests are skipped

[Sean Christopherson]

On Fri, Jul 08, 2022, Yang Weijiang wrote:
> report_skip() prints message with "SKIP:" prefix to explictly
> tell which test is skipped, making the report screening easier.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>  x86/vmx_tests.c | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 4d581e7..27ab5ed 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -4107,7 +4107,7 @@ static void test_vpid(void)
>  	int i;
>  
>  	if (!is_vpid_supported()) {
> -		printf("Secondary controls and/or VPID not supported\n");
> +		report_skip("Secondary controls and/or VPID not supported\n");

All of the manual newlines need to be dropped, report_skip() automatically inserts
a newline.

Re: [kvm-unit-tests PATCH v5 1/3] x86: Use report_skip to log messages when tests are skipped

[Yang, Weijiang]

On 7/8/2022 11:51 PM, Sean Christopherson wrote:
> On Fri, Jul 08, 2022, Yang Weijiang wrote:
>> report_skip() prints message with "SKIP:" prefix to explictly
>> tell which test is skipped, making the report screening easier.
>>
>> Suggested-by: Sean Christopherson <seanjc@google.com>
>> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>> ---
>>   x86/vmx_tests.c | 38 +++++++++++++++++++-------------------
>>   1 file changed, 19 insertions(+), 19 deletions(-)
>>
>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
>> index 4d581e7..27ab5ed 100644
>> --- a/x86/vmx_tests.c
>> +++ b/x86/vmx_tests.c
>> @@ -4107,7 +4107,7 @@ static void test_vpid(void)
>>   	int i;
>>   
>>   	if (!is_vpid_supported()) {
>> -		printf("Secondary controls and/or VPID not supported\n");
>> +		report_skip("Secondary controls and/or VPID not supported\n");
> All of the manual newlines need to be dropped, report_skip() automatically inserts
> a newline.
Thanks Sean. I've rebuilt the whole series and posted the new one here:

-----https://lore.kernel.org/all/20220711041841.126648-1-weijiang.yang@intel.com