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: <20190320221534.165ab87b@oasis.local.home> <20190321083317.GL6058@hirez.programming.kicks-ass.net>
 <20190321090241.GL6521@hirez.programming.kicks-ass.net> <20190321104517.GM6521@hirez.programming.kicks-ass.net>
 <20190321093242.4a948198@gandalf.local.home> <20190321172203.GS5996@hirez.programming.kicks-ass.net>
In-Reply-To: <20190321172203.GS5996@hirez.programming.kicks-ass.net>
From:   Andy Lutomirski <luto@amacapital.net>
Date:   Thu, 21 Mar 2019 11:05:06 -0700
Message-ID: <CALCETrXqmjJt7azNoEtxs2eBkDd-vjUAYvrjNh_RD30DAvksBw@mail.gmail.com>
Subject: Re: [RFC][PATCH] tracing/x86: Save CR2 before tracing irqsoff on error_entry
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     Steven Rostedt <rostedt@goodmis.org>,
        Juergen Gross <jgross@suse.com>,
        LKML <linux-kernel@vger.kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@kernel.org>, Borislav Petkov <bp@alien8.de>,
        Joel Fernandes <joel@joelfernandes.org>,
        He Zhe <zhe.he@windriver.com>,
        Linus Torvalds <torvalds@linux-foundation.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Mar 21, 2019 at 10:22 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Mar 21, 2019 at 09:32:42AM -0400, Steven Rostedt wrote:
> > On Thu, 21 Mar 2019 11:45:17 +0100
> > Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > > -   .if \paranoid
> > > +   .if \read_cr2
> > > +   mov     %cr2, %rdx                      /* XXX paravirt crap */
> > > +   .endif
> > > +
> >
> > I'm guessing this breaks paravirt, as that's one reason I didn't add
> > the read_rc in assembly.
>
> Still completely missing 32bit support.. but this has paravirt bits on.
>
> It changes the read_cr2 stuff to CALLEE_SAVED and implements it in asm
> to only require AX, this means that asm can use it without it clobbering
> all volatile regs.
>
> My compiler also tripped over the KVM asyn pagefault stuff, which I also
> converted to use the new form.
>
> Again, completely untested. And given that the last one just worked,
> this one will _have_ to explode and let the magic smoke out :-)
>

Ugh.

I certainly agree in principle that sticking the CR2 read into the asm
is the right solution.  But this patch makes the spaghetti even more
tangled.  Maybe we can rearrange the code a bit so that the entry
sequence saves at least one register before calling error_entry, so we
can do it the obvious way.

In the long run, I think the right solution is to rewrite even more of
this mess in C.  We really ought to be able to put the IRQ flag
tracing and the context tracking into C code.