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;
MIME-Version: 1.0
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> <alpine.DEB.2.21.1903211844010.1784@nanos.tec.linutronix.de>
In-Reply-To: <alpine.DEB.2.21.1903211844010.1784@nanos.tec.linutronix.de>
From:   Stephane Eranian <eranian@google.com>
Date:   Fri, 22 Mar 2019 12:04:55 -0700
Message-ID: <CABPqkBQ=hGNY8nd5WTONaMepH1ma=dBN=AdWOyOF_xe1K5JGHA@mail.gmail.com>
Subject: Re: [PATCH 1/8] perf/x86/intel: Fix memory corruption
To:     Thomas Gleixner <tglx@linutronix.de>
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
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Mar 21, 2019 at 10:51 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> 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.
>
I would expect this description to be included in the source code where the
allow_tsx_force_abort variable is defined and somewhere in the kernel
Documentation
because it is not trivial to understand what the control actually does
and the guarantees
you have when you toggle it.

> 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
>