Messages in this thread | | | Date | Tue, 12 Jun 2012 19:07:25 +0200 | From | Robert Richter <> | Subject | Re: [PATCH] perf/x86: check ucode before disabling PEBS on SandyBridge |
| |
On 08.06.12 15:26:12, Peter Zijlstra wrote: > +static const u32 snb_ucode_rev = 0x28; > + > +static void intel_snb_verify_ucode(void) > +{ > + u32 rev = UINT_MAX; > + int pebs_broken = 0; > + int cpu; > + > + get_online_cpus(); > + /* > + * Because the microcode loader is bloody stupid and allows different > + * revisions per cpu and does strictly per-cpu loading, we now have to > + * check all cpus to determine the minimally installed revision. > + * > + * This makes updating the microcode O(n^2) in the number of CPUs :/ > + */ > + for_each_online_cpu(cpu) > + rev = min(cpu_data(cpu).microcode, rev); > + put_online_cpus(); > + > + pebs_broken = (rev < snb_ucode_rev); > + > + if (pebs_broken == x86_pmu.pebs_broken) > + return; > + > + /* > + * Serialized by the microcode lock.. > + */ > + if (x86_pmu.pebs_broken) { > + pr_info("PEBS enabled due to micro-code update\n"); > + x86_pmu.pebs_broken = 0; > + } else { > + pr_info("PEBS disabled due to CPU errata, " > + "please upgrade micro-code to at least %x (current: %x)\n", > + snb_ucode_rev, rev); > + x86_pmu.pebs_broken = 1; > + } > +} > + > +static int intel_snb_ucode_notifier(struct notifier_block *self, > + unsigned long action, void *_uci) > +{ > + /* > + * Since ucode cannot be down-graded, and no future ucode revision > + * is known to break PEBS again, we're ok with MICROCODE_CAN_UPDATE. > + */ > + > + if (action == MICROCODE_UPDATED) > + intel_snb_verify_ucode(); > + > + return NOTIFY_DONE; > +} > + > static __init void intel_sandybridge_quirk(void) > { > - pr_warn("PEBS disabled due to CPU errata\n"); > - x86_pmu.pebs = 0; > - x86_pmu.pebs_constraints = NULL; > + intel_snb_verify_ucode(); > + /* > + * we're still single threaded, so while there's a hole here, > + * you can't trigger it. > + */ > + microcode_notifier(intel_snb_ucode_notifier); > }
Instead of registering a microcode notifier, why not checking the availability of pebs dynamically with each syscall in intel_pmu_hw_config()? It looks like intel_snb_verify_ucode() is not that much expensive. We can perform the check only if the event could be for pebs and if pebs is broken. The check could be repeated when setting up a new event after ucode could potentially has been updated (e.g. after bringing a cpu online or so).
-Robert
-- Advanced Micro Devices, Inc. Operating System Research Center
| |