Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 21 Mar 2019 18:51:54 +0100 (CET)
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Stephane Eranian <eranian@google.com>
cc:     Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>, Jiri Olsa <jolsa@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>, tonyj@suse.com,
        nelson.dsouza@intel.com
Subject: Re: [PATCH 1/8] perf/x86/intel: Fix memory corruption
In-Reply-To: <CABPqkBSZbAbU5h=t2f=oMre080ezSq78LaNQO5xEdRV1nczuew@mail.gmail.com>
Message-ID: <alpine.DEB.2.21.1903211844010.1784@nanos.tec.linutronix.de>
References: <20190314130113.919278615@infradead.org> <20190314130705.441549378@infradead.org> <CABPqkBT9=XRLinbhA0T5iztM6NGd5fO39-kQoB5j3Evfvfh2yA@mail.gmail.com> <20190319110549.GC5996@hirez.programming.kicks-ass.net> <CABPqkBQapZFCyJNRjP-GUjfOy62=rJU=Fv=cYPxPTnVFBkqtVA@mail.gmail.com>
 <20190319182041.GO5996@hirez.programming.kicks-ass.net> <CABPqkBQgmT+V29+eD6_rABzNBCo-R22N3PvPX4rWxSWEC0wPPg@mail.gmail.com> <20190320222220.GA2490@worktop.programming.kicks-ass.net> <20190321123849.GN6521@hirez.programming.kicks-ass.net>
 <alpine.DEB.2.21.1903211741120.1784@nanos.tec.linutronix.de> <CABPqkBSZbAbU5h=t2f=oMre080ezSq78LaNQO5xEdRV1nczuew@mail.gmail.com>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, 21 Mar 2019, Stephane Eranian wrote:
> On Thu, Mar 21, 2019 at 9:45 AM Thomas Gleixner <tglx@linutronix.de> 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.
> >
> Correct. This is what I meant.
> The issue is what does the kernel guarantee when it boots?
> 
> I see:
> static bool allow_tsx_force_abort = true;
> 
> Therefore you must ensure the MSR is set to reflect that state on boot.
> So you have to force it to that value to be in sync which is what your
> new patch is doing.

The initial state should be that the MSR TFA bit is 0. The software state
is a different beast.

allow_tsx_force_abort

  false			Do not set MSR TFA bit (Make TSX work with PMC3) and
  			exclude PMC3 from being used.

  true			Set the MSR TFA bit when PMC3 is used by perf, clear it
  			when PMC3 is not longer in use.

Now, if the firmware or the kexec has the TFA bit set in the MSR and PMC3
is not in use then TSX always aborts pointlessly. It's not a fatal isseu,
but it's inconsistent.

So independent of the state of allow_tsx_force_abort the kernel has to
clear the MSR TSA bit when the CPUs are brought up.

The state of allow_tsx_force_abort is solely relevant after CPUs coming up
to decide whether PMC3 can be used by perf or not.

Thanks,

	tglx