Message-ID: <20171217083243.di3gthb5rykta6jp@gmail.com>
Date:   Sun, 17 Dec 2017 09:32:43 +0100
From:   Ingo Molnar <mingo@...nel.org>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Borislav Petkov <bp@...en8.de>
Subject: Re: [GIT PULL] x86 fixes


* Andy Lutomirski <luto@...nel.org> wrote:

> > Is this tree looking good to you standalone?
> 
> I think this stuff is looking okay, although I'm a bit mystified by
> the whole merge base thing.  But if the backporters and Linus like it,
> then whatever.

To explain the merge base thing: considering that v4.14 is an LTS kernel the PTI 
merge base thing is really mostly about keeping this commit count low:

  triton:~/tip> git log --no-merges --oneline v4.14..tip/WIP.x86/pti.base | wc -l
  63

Versus the closest upstream work-alike equivalent base tree, which is, roughly:

  triton:~/tip> git log --no-merges --oneline v4.14..99306dfc067e | wc -l
  1088

1088 is a lot of commits to backport, all sourced from early in the merge window - 
which would also require the identification of ~dozens of random followup fixes 
later in the merge window and the -rc process - which fixes might not even 
cherry-pick cleanly due to other interactions...

So the upstream price of the 'PTI merge base' is 9 cherry-picks, to avoid 
backporting either 1000+ commits to -stable (not workable to -stable folks, 
especially as such wide backports also tend to explode exponentially by pulling in 
random dependencies as you try to backport them further back) or the non-Git 
backporting of 100+ iffy low level x86 entry code commits to the LTS kernel (still 
a nightmare to both the -stable and us x86 maintainers).

Note that doing this also allows tip:x86/pti to remain on this almost-v4.14 base 
for a few more weeks after an upstream merge, so it can collect any eventual fixes 
and minor enhancements into a linear series of commits. This, considering the 
complexity of PTI, is good both for bisectability and for backporting.

So as long as the ~9 cherry-picks are cleanly structured and are explicitly 
marked, this is the best all around solution we could think of.

( The somewhat weird git-merge gynastics in the tree I sent to Linus are really 
  about merging specific versions of upstream that are content-equivalent with
  the cherry-picks, to avoid massive conflicts. v4.15 had various other changes to 
  the files affected by the cherry-picking. If done naively the cherry-picked tree 
  merge can generate over a dozen nasty conflicts. Doing it this way also creates 
  more confidence in the cherry-picked base tree itself: -stable folks can trust 
  it more because it will be 'obviously' equivalent to upstream as expressed by
  the conflict-free merge. )

I believe the fact that the PTI patches have already been successfully backported 
to v4.9, with only minor additional cherry-picking, demonstrates that this is the 
right approach.

> I few things I noticed in the PTI tree:
> 
> "x86/mm/pti: Map ESPFIX into user space" has a leftover pr_err().
> Sorry, my bad, I've spent *way* too long looking at this crap to
> retain my sanity.  Also, if you're feeling like being super tidy, the
> init/main.c change in their could be folded in to whatever patch adds
> pti_init() in the first place, but it doesn't really matter.
> 
> "x86/pti: Map the vsyscall page if needed" has a change to
> pgtable_64.h that could be folded into an earlier patch.  This is
> probably my fault for applying Dave Hansen's cleanup request to the
> wrong patch.
> 
> "x86/mm/64: Make a full PGD-entry size hole in the memory map" would
> benefit from a mention of "5-level" somewhere in the subject or
> changelog.
> 
> In "x86/fixmap: Add debugstore entries to cpu_entry_area", I think the
> function "set_percpu_fixmap_ptes" is misnamed.  It should be something
> like "allocate_percpu_fixmap_ptes", perhaps, and it should either warn
> or do nothing if the PTE is already present, I think.  As it stands,
> it's a wee bit dangerous.
> 
> X86_BUG_CPU_SECURE_MODE_PTI should be added to DISABLED_FEATURES or
> DISABLED_BUGS or whatever if it's not configured in, which will reduce
> bloat.  Borislav, that's kind of up your alley, since I don't think
> the appropriate mask even exists right now.
> 
> 
> 
> Anyway, I stuck a few minor fixups here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/log/?h=x86/pti

Thanks, we'll integrate all of this!

Also note that regarding PTI LDT handling, our plan is to stick to the review 
feedback consensus: i.e. we'll apply your PTI LDT fixes as the initial approach, 
plus an optional series later on once PTI is upstream, with the VMA based bits by 
Thomas and PeterZ subject to a fresh round of thinking & evaluation. Even if we 
decide to do the VMA approach, I don't think that aspect of PTI will be 
backported.

This should further simplify logistics and offloads risks as well.

	Ingo

Powered by blists - more mailing lists