Messages in this thread | | | From | "Liang, Kan" <> | Subject | Re: [PATCH V5 2/4] perf/x86/kvm: Avoid unnecessary work in guest filtering | Date | Wed, 16 Jan 2019 13:58:46 -0500 |
| |
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
| |