Re: [PATCH 0/2] [GIT PULL][v3.3] x86: Fix up CFI for the nested NMI

[Steven Rostedt <rostedt@goodmis.org>]

Hi Ingo,

On Mon, 2012-02-27 at 08:39 +0100, Ingo Molnar wrote:
> * Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > 
> > Ingo,
> > 
> > Jan posted a patch that fixes the CFI annotations. I recommend getting
> > this into 3.3 as this is new code and it would be nice to have CFI
> > correct. It also does a little simplification of it as well.
> > 
> > The second patch is comment changes only (very low impact on messing
> > anything up). I realized that the comments had some references to
> > previous approaches that I tried, and I fixed them to reflect what
> > the final result was. I also added some more comments to describe
> > the code a bit better.
> > 
> > Thanks,
> > 
> > -- Steve
> > 
> > Please pull the latest tip/x86/urgent tree, which can be found at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
> > tip/x86/urgent
> > 
> > Head SHA1: 79fb4ad63e8266ffac1f69bbb45a6f86570493e7
> > 
> > 
> > Jan Beulich (1):
> >       x86-64: Fix CFI annotations for NMI nesting code
> > 
> > Steven Rostedt (1):
> >       x86: Fix the NMI nesting comments
> > 
> > ----
> >  arch/x86/kernel/entry_64.S |   64 ++++++++++++++++++++++++--------------------
> >  1 files changed, 35 insertions(+), 29 deletions(-)
> 
> I don't think we want a 30+ lines diffstat to this rather 

Some of that was just movement of code.

> non-trivial NMI codepath - and it changes real instructions, not 
> just the CFI annotations.

The changes to the real code made the CFI code easier to fix. But if you
are nervous about the code change (which actually simplifies the code),
I can ask Jan (Cc'd) to break out the patch into two changes if
possible.

I'm not sure how the CFI can handle the current trampoline, but perhaps
we can just fix the main part of the code and leave the trampoline part
broken? Then we can add the rest of the CFI changes and the movement of
the trampoline back into the function for the next release.

> 
> Also, the 'update comments' commit does not belong into 
> x86/urgent either.

Hmm, I didn't know that fixing comments was for a merge window only.
Some of the comments are currently wrong and I didn't think we would
want those in a main release. While reading the code again I realized
that I could also add more comments to make it easier to understand. I
would think that comments would be fine for the -rc releases because
they have almost no chance of introducing bugs.


> 
> So either you do an obviously trivial patch that only adds CFI 
> annotations and nothing else, or I can pull these bits into 
> tip:x86/debug, for a v3.4 merge.

I'm fine with waiting for v3.4 before these changes get in.

-- Steve