Re: [PATCH V5 2/4] perf/x86/kvm: Avoid unnecessary work in guest filtering

[Liang, Kan]

On 1/16/2019 8:14 AM, Borislav Petkov wrote:
> > +static __init void intel_isolation_quirk(void)
> > +{
> > +	x86_pmu.check_microcode = intel_check_isolation;
> > +	intel_check_isolation();
> > +}
> > +
> >   static const struct { int id; char *name; } intel_arch_events_map[] __initconst = {
> >   	{ PERF_COUNT_HW_CPU_CYCLES, "cpu cycles" },
> >   	{ PERF_COUNT_HW_INSTRUCTIONS, "instructions" },
> > @@ -4424,6 +4483,7 @@ __init int intel_pmu_init(void)
> >   	case INTEL_FAM6_HASWELL_X:
> >   	case INTEL_FAM6_HASWELL_ULT:
> >   	case INTEL_FAM6_HASWELL_GT3E:
> > +		x86_add_quirk(intel_isolation_quirk);
> And reportedly, the quirks are one-off things - not what this one
> needs to do. So you need to run this unconditionally at the end of
> intel_pmu_init() and get rid of all that quirks indirection.

Hi Borislav,

Thanks for the review. I will change the code according to all your 
comments.

Besides, I will do one more change which impacts your last comment.

Current intel_check_isolation() will set x86_pmu.pebs_isolated=0 for the 
platforms not in the table. It's OK for now. But it will be a problem 
for future platforms.
I've confirmed with Andi. The microcode patch has been merged into 
future platforms. So we have to always set x86_pmu.pebs_isolated=1 for 
future platforms. That means we have to add the CPU model to the table 
for each new platforms. It will be hard to maintain.

I plan to rename x86_pmu.pebs_isolated to x86_pmu.no_pebs_isolation. The 
default value for x86_pmu.no_pebs_isolation is 0. So we don't need to 
worry about the future platforms.
The unconditional check will be moved to the begin of intel_pmu_init(). 
The old platforms, which doesn't have microcode fix, will be specially 
handled by adding x86_pmu.no_pebs_isolation=1 after each "case :" of old 
platforms.

Thanks,
Kan