Hi Linus,
Please pull these arm64 fixes for -rc4. The main thing here is a
long-awaited workaround for a CPU erratum on ThunderX2 which we have
developed in conjunction with engineers from Cavium/Marvell. At the moment,
the workaround is unconditionally enabled for affected CPUs at runtime
but we may add a command-line option to disable it in future if performance
numbers show up indicating a significant cost for real workloads.
The other fixes are summarised in the tag.
Note that the workaround code ended up being based on -rc2, so I had a
bit of a faff trying to generate the right diffstat for this pull request
after merging that branch into our fixes branch based on -rc1. In the end
I had to emulate the pull locally because I couldn't figure out how to
drive request-pull correctly despite the shortlog being correct. I'd love
to know what I should've done instead.
Thanks,
Will
--->8
The following changes since commit 3e7c93bd04edfb0cae7dad1215544c9350254b8f:
arm64: armv8_deprecated: Checking return value for memory allocation (2019-10-08 13:34:04 +0100)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git tags/arm64-fixes
for you to fetch changes up to 777d062e5bee0e3c0751cdcbce116a76ee2310ec:
Merge branch 'errata/tx2-219' into for-next/fixes (2019-10-17 13:42:42 -0700)
----------------------------------------------------------------
arm64 fixes for -rc4
- Work around Cavium/Marvell ThunderX2 erratum #219
- Fix regression in mlock() ABI caused by sign-extension of TTBR1 addresses
- More fixes to the spurious kernel fault detection logic
- Fix pathological preemption race when enabling some CPU features at boot
- Drop broken kcore macros in favour of generic implementations
- Fix userspace view of ID_AA64ZFR0_EL1 when SVE is disabled
- Avoid NULL dereference on allocation failure during hibernation
----------------------------------------------------------------
Chris von Recklinghausen (1):
arm64: Fix kcore macros after 52-bit virtual addressing fallout
Julien Grall (1):
arm64: cpufeature: Treat ID_AA64ZFR0_EL1 as RAZ when SVE is not enabled
Julien Thierry (1):
arm64: entry.S: Do not preempt from IRQ before all cpufeatures are enabled
Marc Zyngier (4):
arm64: KVM: Trap VM ops when ARM64_WORKAROUND_CAVIUM_TX2_219_TVM is set
arm64: Enable workaround for Cavium TX2 erratum 219 when running SMT
arm64: Avoid Cavium TX2 erratum 219 when switching TTBR
arm64: Allow CAVIUM_TX2_ERRATUM_219 to be selected
Mark Rutland (1):
arm64: mm: fix inverted PAR_EL1.F check
Pavel Tatashin (1):
arm64: hibernate: check pgd table allocation
Will Deacon (2):
arm64: tags: Preserve tags for addresses translated via TTBR1
Merge branch 'errata/tx2-219' into for-next/fixes
Yang Yingliang (1):
arm64: sysreg: fix incorrect definition of SYS_PAR_EL1_F
Documentation/arm64/silicon-errata.rst | 2 +
arch/arm64/Kconfig | 17 +++++++++
arch/arm64/include/asm/asm-uaccess.h | 7 ++--
arch/arm64/include/asm/cpucaps.h | 4 +-
arch/arm64/include/asm/memory.h | 10 ++++-
arch/arm64/include/asm/pgtable.h | 3 --
arch/arm64/include/asm/sysreg.h | 2 +-
arch/arm64/kernel/cpu_errata.c | 38 +++++++++++++++++++
arch/arm64/kernel/cpufeature.c | 15 +++++---
arch/arm64/kernel/entry.S | 8 ++--
arch/arm64/kernel/hibernate.c | 9 ++++-
arch/arm64/kernel/process.c | 18 +++++++++
arch/arm64/kvm/hyp/switch.c | 69 +++++++++++++++++++++++++++++++++-
arch/arm64/mm/fault.c | 6 ++-
include/linux/sched.h | 1 +
15 files changed, 186 insertions(+), 23 deletions(-)
On Thu, Oct 17, 2019 at 4:43 PM Will Deacon <[email protected]> wrote:
>
> Note that the workaround code ended up being based on -rc2, so I had a
> bit of a faff trying to generate the right diffstat for this pull request
> after merging that branch into our fixes branch based on -rc1. In the end
> I had to emulate the pull locally because I couldn't figure out how to
> drive request-pull correctly despite the shortlog being correct. I'd love
> to know what I should've done instead.
You did the right thing.
When there are multiple merge bases, a regular "git diff" doesn't work
since it's fundamentally about two end-points (well, it _can_ work
almost by mistake, but doesn't work in the general case). So the only
way to get a "proper" diff is to do a merge and then diff the result.
That said, I also accept the output of "git diff" which will then have
a lot of noise from all the _other_ work done between the two merge
bases. I can figure out what happened, and do my own two-endpoint diff
and see what happened, and still se that "yes, that's what the pull
request meant, and that's why the diffstat is garbage".
What you did is the "good quality" pull request, though.
In general, people who aren't doing fancy things with git should never
get to the "multiple merge bases" situation, and then the regular pull
request logic works fine.
And people likme you who are doing multiple branches and know what
they are doing are also able to them handle the "uhhuh, I need to do a
merge to get a good diffstat" situation.
Linus
On Thu, Oct 17, 2019 at 05:06:54PM -0700, Linus Torvalds wrote:
> On Thu, Oct 17, 2019 at 4:43 PM Will Deacon <[email protected]> wrote:
> >
> > Note that the workaround code ended up being based on -rc2, so I had a
> > bit of a faff trying to generate the right diffstat for this pull request
> > after merging that branch into our fixes branch based on -rc1. In the end
> > I had to emulate the pull locally because I couldn't figure out how to
> > drive request-pull correctly despite the shortlog being correct. I'd love
> > to know what I should've done instead.
>
> You did the right thing.
>
> When there are multiple merge bases, a regular "git diff" doesn't work
> since it's fundamentally about two end-points (well, it _can_ work
> almost by mistake, but doesn't work in the general case). So the only
> way to get a "proper" diff is to do a merge and then diff the result.
>
> That said, I also accept the output of "git diff" which will then have
> a lot of noise from all the _other_ work done between the two merge
> bases. I can figure out what happened, and do my own two-endpoint diff
> and see what happened, and still se that "yes, that's what the pull
> request meant, and that's why the diffstat is garbage".
>
> What you did is the "good quality" pull request, though.
Thanks, that's helpful to know for next time. I guess I'm most surprised by
the discrepancy between the shortlog and the diffstat, whereas I intuitively
expected them to be generated in the same way.
Will
On Fri, Oct 18, 2019 at 10:42 AM Will Deacon <[email protected]> wrote:
>
> Thanks, that's helpful to know for next time. I guess I'm most surprised by
> the discrepancy between the shortlog and the diffstat, whereas I intuitively
> expected them to be generated in the same way.
So logs and diffs are fundamentally different.
A log is an operation on a _set_ of commits (that's the whole point -
you don't list the beginning and the end, you list all the commits in
between), while a diff is fundamentally an operation on two end-points
and shows the code difference between those two points.
And the summary versions of those operations (shortlog and diffstat)
are no different.
So as a set operation, "shortlog" has no issues with multiple merge
bases. Doing a shortlog is still just a set difference between your
commits and the upstream commits, and the number of merge bases is
irrelevant. "List all commits that I have, but upstream doesn't have"
is a very straightforward and natural set operation.
But as a "two endpoints" operation, diffstat has real problems any
time you have more than two endpoints - when you have multiple merge
bases, you fundamentally have more than two endpoints: you have all of
the merge bases, and then you have your end result.
What you doing the merge does is to turn the multiple merge bases into
just one point: the thing you merged against now becomes the common
merge point, and now you have a "two endpoints" for the diffstat: the
thing you merged against, and your end result are now the two points
that you can diff against.
But the shortlog is always correct, because it just doesn't even care
about that whole issue.
Linus
On Fri, Oct 18, 2019 at 12:43:49AM +0100, Will Deacon wrote:
> Hi Linus,
>
> Please pull these arm64 fixes for -rc4. The main thing here is a
> long-awaited workaround for a CPU erratum on ThunderX2 which we have
> developed in conjunction with engineers from Cavium/Marvell. At the moment,
> the workaround is unconditionally enabled for affected CPUs at runtime
> but we may add a command-line option to disable it in future if performance
> numbers show up indicating a significant cost for real workloads.
As the Cavium/Marvell engineer who was involved in this, I will note
that I had suggested a patch providing a runtime override[1] while
providing safe defaults.
Marc's patchset adds a trap to hypervisor in the system call path when
KPTI is enabled, and KPTI is generally enabled on stock VM images. So
normal users will see some performance regression (e.g I see something
in the range of 3-4% on guest kernel compile).
As a policy, I don't agree with having errata workarounds that can be
left to the discretion of the admin to be forced at compile time.
Since most of these workarounds use run-time code patching with
alternatives, there is no need to do this at all.
But given that this is already merged and cc:ed to stable, I will see
if I can come up with enough data to convince Will.
JC
[1]
https://lore.kernel.org/linux-arm-kernel/[email protected]/T/
* Linus Torvalds <[email protected]> wrote:
> What you doing the merge does is to turn the multiple merge bases into
> just one point: the thing you merged against now becomes the common
> merge point, and now you have a "two endpoints" for the diffstat: the
> thing you merged against, and your end result are now the two points
> that you can diff against.
>
> But the shortlog is always correct, because it just doesn't even care
> about that whole issue.
FWIW I regularly ran into this problem too and resolved it manually by
'emulating' your merge. (Once every 20-30 pull requests or so. Finally
ended up scripting around request-pull altogether.)
I think at least once I ran into that and sent you a 'slightly wrong'
diffstat - and maybe there's also been a few cases where you noticed
diffstats that didn't match your merge result, double checked it yourself
and didn't complain about it because you knew that this is a "git
request-pull" artifact?
Most of the time I notice it like Will did because the diffstat is
obviously weird and it's good to check pull requests a second (and a
third :-) time as well, but it's possible to have relatively small
distances between the merge bases where the diffstat doesn't look
'obviously' bogus and mistakes can slip through.
Anyway, a small Git feature request: it would be super useful if "git
request-pull" output was a bit more dependable and at least warned about
this and didn't include what is, from the viewpoint of the person doing
the merge, a bogus diffstat. (Generating the correct diffstat is probably
beyond request-pull's abilities: it would require changing the working
tree to actually perform the merge - while request-pull is a read-only
operation right now. But detecting the condition and warning about it
should be possible?)
Thanks,
Ingo
On Mon, Oct 21, 2019 at 2:47 AM Ingo Molnar <[email protected]> wrote:
>
> I think at least once I ran into that and sent you a 'slightly wrong'
> diffstat - and maybe there's also been a few cases where you noticed
> diffstats that didn't match your merge result, double checked it yourself
> and didn't complain about it because you knew that this is a "git
> request-pull" artifact?
Right. If I see a diffstat that doesn't match, I just look at what a
non-merged diffstat would have looked like, and if that matches I know
what happened.
There are other reasons why diffstats won't match, of course. Like me
just having merged part of the same commits from another source (or
multiple trees applying the same patch). So it's not _just_ due to
multiple merge bases that the mis-match can happen.
> Most of the time I notice it like Will did because the diffstat is
> obviously weird and it's good to check pull requests a second (and a
> third :-) time as well, but it's possible to have relatively small
> distances between the merge bases where the diffstat doesn't look
> 'obviously' bogus and mistakes can slip through.
Yup.
> Anyway, a small Git feature request: it would be super useful if "git
> request-pull" output was a bit more dependable and at least warned about
> this and didn't include what is, from the viewpoint of the person doing
> the merge, a bogus diffstat.
Well, warning for it would be fairly simple. Giving the "right" result
isn't simple, though, since the merge might need manual fixup to
succeed.
The warning you can check yourself: just do
git merge-base --all upstream mybranch
and if you get more than one result, you know you are in the situation
where a diff from the merge base might not work (it *might* work, but
probably won't).
You can play around with it yourself, of course. Look at the
git-request-puill.sh script, it says something like this:
merge_base=$(git merge-base $baserev $headrev) ||
die "fatal: No commits in common between $base and $head"
and you could add something like
all_merge_bases="$(git merge-base --all $baserev $headrev)"
and then add a warning if "all_merge_bases" doesn't match "merge_base".
Linus
Hello,
I added the git list to Cc:. For the new readers: The context of this
thread can be found at
https://lwn.net/ml/linux-kernel/20191017234348.wcbbo2njexn7ixpk@willie-the-truck/
On Mon, Oct 21, 2019 at 08:46:58AM +0200, Ingo Molnar wrote:
> Anyway, a small Git feature request: it would be super useful if "git
> request-pull" output was a bit more dependable and at least warned about
> this and didn't include what is, from the viewpoint of the person doing
> the merge, a bogus diffstat. (Generating the correct diffstat is probably
> beyond request-pull's abilities: it would require changing the working
> tree to actually perform the merge - while request-pull is a read-only
> operation right now. But detecting the condition and warning about it
> should be possible?)
I think Will's case is still an easy one compared with what could
actually happen.
The related history looks as follows:
,-. ,-. ,-. ,-. ,-.
v5.4-rc1 --| |-...-| |-- v5.4-rc2 --| |-..-| |-..-| |-- v5.4-rc3
\ `-' `-' \ `-' /-' `-'
\ ,-. ,-. \ ,-/ ,-. ,-.
`--| |-...-| |--------------------|*|----| |-...-|H|
`-' `-' \ `-' `-' /-'
\ ,-. ,-. /
`--| |-...-| |-----'
`-' `-'
Will asked Linus to merge the Commit marked 'H', the two merge bases are
v5.4-rc2 and '*'.
(FTR:
* = 3e7c93bd04edfb0cae7dad1215544c9350254b8f
H = 777d062e5bee0e3c0751cdcbce116a76ee2310ec
, they can be found in
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git)
The formally correct way to create the diffstat is to merge v5.4-rc2 and
'*' (in general: all merge bases) and calculate the diff between this
merge and the to-be-merged-commit. Compared to what Will did (i.e. merge
Linus' HEAD and this branch and then diff @~ with @) doing it the way I
described has the advantage(?) that commits that conflict with this
merge request in Linus' tree since the merge bases are not in the way.
In this case this can be done automatically:
$ git read-tree --index-output=tralala v5.4-rc2 3e7c93bd04edfb0cae7dad1215544c9350254b8f
$ GIT_INDEX=tralala git write-tree
6a2acfd1870d9da3c330ea9b648a7e858b5ee39f
$ git diff --stat 6a2acfd1870d9da3c330ea9b648a7e858b5ee39f 777d062e5bee0e3c0751cdcbce116a76ee2310ec
Documentation/arm64/silicon-errata.rst | 2 ++
arch/arm64/Kconfig | 17 ++++++++++++++
arch/arm64/include/asm/asm-uaccess.h | 7 +++---
arch/arm64/include/asm/cpucaps.h | 4 +++-
arch/arm64/include/asm/memory.h | 10 ++++++--
arch/arm64/include/asm/pgtable.h | 3 ---
arch/arm64/include/asm/sysreg.h | 2 +-
arch/arm64/kernel/cpu_errata.c | 38 +++++++++++++++++++++++++++++++
arch/arm64/kernel/cpufeature.c | 15 ++++++++----
arch/arm64/kernel/entry.S | 8 ++++---
arch/arm64/kernel/hibernate.c | 9 +++++++-
arch/arm64/kernel/process.c | 18 +++++++++++++++
arch/arm64/kvm/hyp/switch.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
arch/arm64/mm/fault.c | 6 ++++-
include/linux/sched.h | 1 +
15 files changed, 186 insertions(+), 23 deletions(-)
Would be great if git-request-pull learned to do that.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |