On Thu, Mar 21, 2019 at 06:17:01PM +0100, Thomas Gleixner wrote: > On Thu, 21 Mar 2019, Peter Zijlstra wrote: > > On Thu, Mar 21, 2019 at 05:45:41PM +0100, Thomas Gleixner wrote: > > > On Thu, 21 Mar 2019, Peter Zijlstra wrote: > > > > Subject: perf/x86/intel: Initialize TFA MSR > > > > > > > > Stephane reported that we don't initialize the TFA MSR, which could lead > > > > to trouble if the RESET value is not 0 or on kexec. > > > > > > That sentence doesn't parse. > > > > > > Stephane reported that the TFA MSR is not initialized by the kernel, but > > > the TFA bit could set by firmware or as a leftover from a kexec, which > > > makes the state inconsistent. > > > > > > > Reported-by: Stephane Eranian <eran...@google.com> > > > > Signed-off-by: Peter Zijlstra (Intel) <pet...@infradead.org> > > > > --- > > > > arch/x86/events/intel/core.c | 6 ++++++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c > > > > index 8baa441d8000..2d3caf2d1384 100644 > > > > --- a/arch/x86/events/intel/core.c > > > > +++ b/arch/x86/events/intel/core.c > > > > @@ -3575,6 +3575,12 @@ static void intel_pmu_cpu_starting(int cpu) > > > > > > > > cpuc->lbr_sel = NULL; > > > > > > > > + if (x86_pmu.flags & PMU_FL_TFA) { > > > > + WARN_ON_ONCE(cpuc->tfa_shadow); > > > > > > Hmm. I wouldn't warn here as this is a legit state for kexec/kdump and I > > > don't think we can figure out whether this comes directly from the > > > firmware. > > > > Even on kexec, the cpuc will be freshly allocated and zerod I think. We > > only inherit hardware state, not software state. > > Ouch, misread the patch of course ... What about cpu hotplug? Does that > free/alloc or reuse?
re-use I think, but since we kill all events before taking the CPU down, it _should_ have cleared the MSR while doing that. Might be best to have someone test this.. someone with ucode and a machine etc.. :-)