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

["Yang, Weijiang" <weijiang.yang@xxxxxxxxx>]

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@xxxxxxxxxx>
> > Signed-off-by: Yang Weijiang <weijiang.yang@xxxxxxxxx>
> >
> > 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