* Re: ppc64le and 32-bit LE userland compatibility
From: Daniel Kolesa @ 2020-06-02 2:03 UTC (permalink / raw)
To: Segher Boessenkool, Joseph Myers
Cc: libc-alpha, eery, musl, Will Springer,
Palmer Dabbelt via binutils, via libc-dev, linuxppc-dev
In-Reply-To: <20200602014227.GM31009@gate.crashing.org>
On Tue, Jun 2, 2020, at 03:42, Segher Boessenkool wrote:
> Hi Joseph,
>
> On Mon, Jun 01, 2020 at 09:28:25PM +0000, Joseph Myers wrote:
> > On Fri, 29 May 2020, Will Springer via Binutils wrote:
> >
> > > Hey all, a couple of us over in #talos-workstation on freenode have been
> > > working on an effort to bring up a Linux PowerPC userland that runs in 32-bit
> > > little-endian mode, aka ppcle. As far as we can tell, no ABI has ever been
> > > designated for this (unless you count the patchset from a decade ago [1]), so
> > > it's pretty much uncharted territory as far as Linux is concerned. We want to
> > > sync up with libc and the relevant kernel folks to establish the best path
> > > forward.
> >
> > As a general comment on the glibc side of things, if this is considered
> > like a new port, and it probably is, the same principles that apply to new
> > ports apply here.
> >
> > There's a general discussion at
> > <https://sourceware.org/glibc/wiki/NewPorts>, although much of that is
> > only applicable when adding new CPU architecture support. More specific
> > points include that new 32-bit ports should default to 64-bit time and
> > file offsets from the start, with no support for 32-bit time or offsets
> > (meaning that if you want to use this with some kind of library call
> > translation, the library call translation will need to deal with
> > corresponding type size conversions).
>
> Either that, or use the same as BE 32-bit PowerPC Linux, I'd say (it
> won't make things worse, and if it is easier?) But preferably the
> newer, better, thing of course :-)
>
> > And a new port should not be added
> > that uses the IBM long double format. You can use IEEE binary128 long
> > double, possibly with an ABI similar to that used on powerpc64le, or can
> > use long double = double, but should not support IBM long double, and
> > preferably should only have one long double format rather than using the
> > glibc support for building with different options resulting in functions
> > for different long double formats being called.
>
> You cannot use IEEE QP float ("binary128") here, but more on that in a
> later post.
>
> (I so very much agree about the problems having more than one long
> double format -- on the other hand, you'll just share it with BE, and
> with the existing powerpcle-linux (sup)port).
Well, it'd be nice to use the opportunity to ditch the IBM long doubles altogether, since these get in the way for me in various places (for instance, GCC still can't constant-fold them, which breaks on constexpr in C++, as well as makes it impossible to e.g. enable the runtime/standard library for GDC in GCC as that heavily relies on the `real` type in D, which maps directly to `long double` and druntime/phobos heavily relies on constant folding of the `real` type being functional; there are also assorted libraries and applications in the common userland that don't like the IBM format for one reason or another)
That said, that's also problematic:
1) ppc64le is going to newly use IEEE754 binary128, so we're all good there - baseline mandates VSX, so at least passing them is fine, which is the important thing (actual binary128 instructions came with ISA 3.0 but that can be detected at runtime, at least in glibc) - ibm128 is then implemented via symvers/compat, which is fine.
2) ppc64 for now uses IBM 128-bit long double, so it's problematic. I don't care about ELFv1, as it's legacy and has tons of drawbacks on its own, and there's a whole legacy ecosystem relying on it; that said, a new ELFv2 port (let's say, with ld64.so.3 dynamic linker) could easily default to another long double format, without worrying about compat - I propose this format to be binary64, as it's already used by musl on BE/64 (allows projects such as `gcompat` to work) and thus is already implemented in all toolchains we care about and can easily be flipped on, and is fully compatible with all CPUs that can run ppc64 code, even without VMX/VSX
3) ppcle would be a new port (let's say ld.so.2), so it could default to a new format; binary64 would be good
4) that leaves ppc32/BE, which would be the only outlier - while I could probably implement compat in much the same way as ppc64le does with binary128 (bump symvers for math symbols, leave older symvers for existing binaries, change defaults), this would be divergent from the existing port; glibc/IBM probably won't want to switch this, and while I would definitely like to, maintaining a divergent downstream patchset seems non-ideal. There is some basis for this - glibc did use to use binary64 on ppc32 and ppc64 BE in the past, and the compatibility symbols are still there under the old symvers, but not quite sure what to do there.
>
>
> Segher
>
Daniel
^ permalink raw reply
* Re: [musl] Re: ppc64le and 32-bit LE userland compatibility
From: Jeffrey Walton @ 2020-06-02 2:09 UTC (permalink / raw)
To: musl
Cc: libc-alpha, eery, Daniel Kolesa, Will Springer,
Palmer Dabbelt via binutils, via libc-dev, linuxppc-dev,
Joseph Myers
In-Reply-To: <20200602015802.GN31009@gate.crashing.org>
On Mon, Jun 1, 2020 at 9:58 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Tue, Jun 02, 2020 at 01:26:37AM +0200, Daniel Kolesa wrote:
> > On Mon, Jun 1, 2020, at 23:28, Joseph Myers wrote:
> > Are you sure this would be a new port? Glibc already works in this combination, as it seems to me it'd be best if it was just a variant of the existing 32-bit PowerPC port, sharing most conventions besides endianness with the BE port.
>
> That's right. Except it isn't an "official" existing port, never has
> been "officially" supported.
>
> > 128-bit IEEE long double would not work, since that relies on VSX being present (gcc will explicitly complain if it's not). I'd be all for using 64-bit long double, though (musl already does, on all ppc ports).
>
> The current IEEE QP float support requires VSX for its emulation, yes
> (possibly even Power8?) ...
I believe Steven Munroe has an implementation that includes emulation
for lesser POWER architectures at
https://github.com/munroesj52/pveclib. If I am not mistaken, Munroe 's
library supports back to POWER7.
Modern GCC is fine. Clang has problems because it lacks support for
ISO/IEC TS 18661 or N2341
(http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2341.pdf).
Jeff
^ permalink raw reply
* Re: [musl] Re: ppc64le and 32-bit LE userland compatibility
From: Daniel Kolesa @ 2020-06-02 2:12 UTC (permalink / raw)
To: Segher Boessenkool, musl
Cc: libc-alpha, eery, Will Springer, Palmer Dabbelt via binutils,
via libc-dev, linuxppc-dev, Joseph Myers
In-Reply-To: <20200602015802.GN31009@gate.crashing.org>
On Tue, Jun 2, 2020, at 03:58, Segher Boessenkool wrote:
> On Tue, Jun 02, 2020 at 01:26:37AM +0200, Daniel Kolesa wrote:
> > On Mon, Jun 1, 2020, at 23:28, Joseph Myers wrote:
> > Are you sure this would be a new port? Glibc already works in this combination, as it seems to me it'd be best if it was just a variant of the existing 32-bit PowerPC port, sharing most conventions besides endianness with the BE port.
>
> That's right. Except it isn't an "official" existing port, never has
> been "officially" supported.
>
> > 128-bit IEEE long double would not work, since that relies on VSX being present (gcc will explicitly complain if it's not). I'd be all for using 64-bit long double, though (musl already does, on all ppc ports).
>
> The current IEEE QP float support requires VSX for its emulation, yes
> (possibly even Power8?) As Mike reminded me today, it also requires
> __int128 support, which rules out anything 32-bit currently. Without
> that restriction, we could just make QP float passed in GPRs (use the
> ABIs for any struct passed that way), and that'll just work out with
> all ABIs, older or not.
>
> > While we're at long double, I'd actually be interested in transitioning the existing big endian ports in Void (64-bit and 32-bit, neither has VSX baseline requirement in my case) to using 64-bit long double, abandoning the IBM format altogether (little endian will transition to 128-bit IEEE long double once it's ready on your side, as that assumes POWER8 baseline which includes VSX).
>
> I recommend new ports that cannot jump to IEEE QP float directly to use
> long double == double for the time being, avoiding the extra
> complications that IBM double double would bring. But you'll still have
> a transition to IEEE 128 if you ever want to go there.
>
> But if you already use double-double, I don't know if the cost changing
> away from that is worth it now.
The transition cost is relatively low, which is why I'm thinking about this in the first place. For one, relatively few things use long double in the first place. For two, on ppc*, at least the bfd linker (which we use always in Void) always tags ELFs with an FP ABI tag, and things not using long double (or using 64-bit long double) don't receive this tag. It even tags *which* ABI is used. See:
```
$ readelf -a /usr/bin/echo|grep Power_ABI_FP
$
$ readelf -a /usr/lib/libc-2.30.so|grep Power_ABI_FP
Tag_GNU_Power_ABI_FP: hard float, 128-bit IBM long double
```
I went through this once already (I had the 64-bit ldbl transition nearly done) and the number of packages to rebuild in the whole repo was about 200-300 out of ~12000.
>
> > What would be the best way for me to proceed with that? I actually experimented with this, using the old glibc compat symbols from pre-ibm128 times, and I mostly had it working, except I haven't managed to find a way to switch the default symbols to 64-bit ones, which is problematic as linking everything against nldbl_nonshared is fragile and potentially quirky (breaks dlsym, function pointer equality across libraries, etc).
>
> Yup. "Rebuild the world" works :-/ I don't have any better advice,
> nothing you cannot figure out yourself.
See above.
>
> > There is also one more thing while we're at this. The 64-bit big endian Void port uses the ELFv2 ABI, even on glibc. This is not officially supported on glibc as far as I can tell, but it does work out of box, without any patching (things in general match little endian then, i.e. ld64.so.2 etc, but they're big endian). Is there any chance of making that support official?
>
> (I don't talk for glibc).
>
> The first thing needed is for "us" to have faith in it. That starts
> with seeing test results for the testsuites!
>
> (Something similar goes for the GCC port -- there is no official support
> for BE ELFv2, but of course it does work, and if we get test results we
> may keep it that way, hint hint :-) )
Well, FreeBSD defaults to it since 13; OpenBSD's new powerpc64 port (which is supposedly dual-endian) defaults to it; musl defaults to it on LE and BE. FreeBSD and OpenBSD have to, since they primarily target LLVM system toolchain (with GCC in ports) and ld.lld doesn't support ELFv1 (at all). Void's port was new (and any precompiled binaries would generally be enterprisey stuff which doesn't concern us enough - people can just make a chroot/container with say, Debian, if they really need to), so I felt like it didn't make sense to go with the legacy ABI (besides, function descriptors are gross ;)). The situation in the overall userland has been improving too, so the patch burden is actually very low nowadays.
>
>
> Segher
>
Daniel
^ permalink raw reply
* Re: ppc64le and 32-bit LE userland compatibility
From: Segher Boessenkool @ 2020-06-02 2:12 UTC (permalink / raw)
To: Joseph Myers
Cc: libc-alpha, eery, Daniel Kolesa, musl, Will Springer,
Palmer Dabbelt via binutils, via libc-dev, linuxppc-dev
In-Reply-To: <alpine.DEB.2.21.2006012329420.11121@digraph.polyomino.org.uk>
On Mon, Jun 01, 2020 at 11:45:51PM +0000, Joseph Myers wrote:
> On Tue, 2 Jun 2020, Daniel Kolesa wrote:
> > Are you sure this would be a new port? Glibc already works in this
> > combination, as it seems to me it'd be best if it was just a variant of
> > the existing 32-bit PowerPC port, sharing most conventions besides
> > endianness with the BE port.
>
> The supported glibc ABIs are listed at
> <https://sourceware.org/glibc/wiki/ABIList>.
powerpcle-linux already does work somewhat, and that should also he
worth something, official or not ;-)
(It has worked for very many years already... That is, I have built it
a lot, I have no idea about running a full distro that way).
> > 128-bit IEEE long double would not work, since that relies on VSX being
> > present (gcc will explicitly complain if it's not). I'd be all for using
>
> The minimum supported architecture for powerpc64le (POWER8) has VSX. My
> understanding was that the suggestion was for 32-bit userspace to run
> under powerpc64le kernels running on POWER8 or later, meaning that such a
> 32-bit LE port, and any ABI designed for such a port, can assume VSX is
> available. Or does VSX not work, at the hardware level, for 32-bit
> POWER8? (In which case you could pick another ABI for binary128 argument
> passing and return.)
The current powerpcle-linux runs on anything 6xx, or maybe older. It
isn't actually supported of course.
If the CPU is Power8, that does not mean VSX is available to you.
VSX works fine in 32-bit mode (with the standard gotcha that the GPRs
do not preserve the high part in all cases, so e.g. the m[ft]vsr insns
might not work as you want.
Passing IEEE QP float in GPRs would be natural for most ABIs, and it
should work fine indeed. That isn't currently supported in GCC (needs
some libgcc work), and it might need __int128 to work on 32-bit ports
first.
> > There is also one more thing while we're at this. The 64-bit big endian
> > Void port uses the ELFv2 ABI, even on glibc. This is not officially
> > supported on glibc as far as I can tell, but it does work out of box,
> > without any patching (things in general match little endian then, i.e.
> > ld64.so.2 etc, but they're big endian). Is there any chance of making
> > that support official?
>
> If you want to support ELFv2 for 64-bit big endian in glibc, again that
> should have a unique dynamic linker name, new symbol versions, only
> binary128 long double, etc. - avoid all the legacy aspects of the existing
> ELFv1 port rather than selectively saying that "ELFv1" itself is the only
> legacy aspect and keeping the others (when it's the others that are
> actually more problematic in glibc).
You should view it as a variant of the LE ELFv2 port, it has nothing
much to do with the other BE 64-bit PowerPC ports, other than being BE.
Segher
^ permalink raw reply
* Re: ppc64le and 32-bit LE userland compatibility
From: Daniel Kolesa @ 2020-06-02 2:17 UTC (permalink / raw)
To: Segher Boessenkool, Joseph Myers
Cc: libc-alpha, eery, musl, Will Springer,
Palmer Dabbelt via binutils, via libc-dev, linuxppc-dev
In-Reply-To: <20200602021245.GO31009@gate.crashing.org>
On Tue, Jun 2, 2020, at 04:12, Segher Boessenkool wrote:
> On Mon, Jun 01, 2020 at 11:45:51PM +0000, Joseph Myers wrote:
> > On Tue, 2 Jun 2020, Daniel Kolesa wrote:
> > > Are you sure this would be a new port? Glibc already works in this
> > > combination, as it seems to me it'd be best if it was just a variant of
> > > the existing 32-bit PowerPC port, sharing most conventions besides
> > > endianness with the BE port.
> >
> > The supported glibc ABIs are listed at
> > <https://sourceware.org/glibc/wiki/ABIList>.
>
> powerpcle-linux already does work somewhat, and that should also he
> worth something, official or not ;-)
>
> (It has worked for very many years already... That is, I have built it
> a lot, I have no idea about running a full distro that way).
>
> > > 128-bit IEEE long double would not work, since that relies on VSX being
> > > present (gcc will explicitly complain if it's not). I'd be all for using
> >
> > The minimum supported architecture for powerpc64le (POWER8) has VSX. My
> > understanding was that the suggestion was for 32-bit userspace to run
> > under powerpc64le kernels running on POWER8 or later, meaning that such a
> > 32-bit LE port, and any ABI designed for such a port, can assume VSX is
> > available. Or does VSX not work, at the hardware level, for 32-bit
> > POWER8? (In which case you could pick another ABI for binary128 argument
> > passing and return.)
>
> The current powerpcle-linux runs on anything 6xx, or maybe older. It
> isn't actually supported of course.
>
> If the CPU is Power8, that does not mean VSX is available to you.
>
> VSX works fine in 32-bit mode (with the standard gotcha that the GPRs
> do not preserve the high part in all cases, so e.g. the m[ft]vsr insns
> might not work as you want.
>
> Passing IEEE QP float in GPRs would be natural for most ABIs, and it
> should work fine indeed. That isn't currently supported in GCC (needs
> some libgcc work), and it might need __int128 to work on 32-bit ports
> first.
>
> > > There is also one more thing while we're at this. The 64-bit big endian
> > > Void port uses the ELFv2 ABI, even on glibc. This is not officially
> > > supported on glibc as far as I can tell, but it does work out of box,
> > > without any patching (things in general match little endian then, i.e.
> > > ld64.so.2 etc, but they're big endian). Is there any chance of making
> > > that support official?
> >
> > If you want to support ELFv2 for 64-bit big endian in glibc, again that
> > should have a unique dynamic linker name, new symbol versions, only
> > binary128 long double, etc. - avoid all the legacy aspects of the existing
> > ELFv1 port rather than selectively saying that "ELFv1" itself is the only
> > legacy aspect and keeping the others (when it's the others that are
> > actually more problematic in glibc).
>
> You should view it as a variant of the LE ELFv2 port, it has nothing
> much to do with the other BE 64-bit PowerPC ports, other than being BE.
He does have a point with multiarch though - e.g. Debian lets you have all architectures in the same system (with the foreign ones running through binfmt of course) and has individual sysroots for each arch, but dynamic linkers still need to be present in a single location. So, different or not, BE/LE should probably each get its own dynamic linker name (musl solves this nicely by using consistent ld-musl-<archname>so, glibc's naming is fairly arbitrary)
>
>
> Segher
>
Daniel
^ permalink raw reply
* Re: [musl] Re: ppc64le and 32-bit LE userland compatibility
From: Segher Boessenkool @ 2020-06-02 2:36 UTC (permalink / raw)
To: Daniel Kolesa
Cc: libc-alpha, eery, musl, Will Springer,
Palmer Dabbelt via binutils, via libc-dev, linuxppc-dev,
Joseph Myers
In-Reply-To: <51122625-15b3-408b-822c-69cdb7b8d5d9@www.fastmail.com>
On Tue, Jun 02, 2020 at 04:12:26AM +0200, Daniel Kolesa wrote:
> On Tue, Jun 2, 2020, at 03:58, Segher Boessenkool wrote:
> > I recommend new ports that cannot jump to IEEE QP float directly to use
> > long double == double for the time being, avoiding the extra
> > complications that IBM double double would bring. But you'll still have
> > a transition to IEEE 128 if you ever want to go there.
> >
> > But if you already use double-double, I don't know if the cost changing
> > away from that is worth it now.
>
> The transition cost is relatively low, which is why I'm thinking about this in the first place. For one, relatively few things use long double in the first place.
Then your cost switching to QP float later will be low as well. I envy
you :-)
> For two, on ppc*, at least the bfd linker (which we use always in Void) always tags ELFs with an FP ABI tag, and things not using long double (or using 64-bit long double) don't receive this tag. It even tags *which* ABI is used. See:
That works for statically linked stuff, sure. That is the easy case :-/
> I went through this once already (I had the 64-bit ldbl transition nearly done) and the number of packages to rebuild in the whole repo was about 200-300 out of ~12000.
Cool! Do you perchance have info you can share about which packages?
Offline, if you want.
> > > There is also one more thing while we're at this. The 64-bit big endian Void port uses the ELFv2 ABI, even on glibc. This is not officially supported on glibc as far as I can tell, but it does work out of box, without any patching (things in general match little endian then, i.e. ld64.so.2 etc, but they're big endian). Is there any chance of making that support official?
> >
> > (I don't talk for glibc).
> >
> > The first thing needed is for "us" to have faith in it. That starts
> > with seeing test results for the testsuites!
> >
> > (Something similar goes for the GCC port -- there is no official support
> > for BE ELFv2, but of course it does work, and if we get test results we
> > may keep it that way, hint hint :-) )
>
> Well, FreeBSD defaults to it since 13; OpenBSD's new powerpc64 port (which is supposedly dual-endian) defaults to it; musl defaults to it on LE and BE.
... and no one ever has sent us (GCC) test results (nothing I have seen
anyway). All we "officially" know is that Power7 BE ELFv2 was a
bring-up vehicle for the current powerpc64le-linux. Everyone tries not
to break things without reason to, of course, and things are a little
bit tested anyway because it is convenient to build ELFv2 stuff on BE
systems as well (if only to figure out effortlessly if some bug is due
to the ABI or due to the endianness), but if we do not know something is
used and we never officially supported it, we might just want to take it
away if it is inconvenient.
> FreeBSD and OpenBSD have to, since they primarily target LLVM system toolchain (with GCC in ports) and ld.lld doesn't support ELFv1 (at all). Void's port was new (and any precompiled binaries would generally be enterprisey stuff which doesn't concern us enough - people can just make a chroot/container with say, Debian, if they really need to),
Yeah, enterprisey enough, then just rebuild :-)
> so I felt like it didn't make sense to go with the legacy ABI (besides, function descriptors are gross ;)).
Descriptors are Great, you just do not understand the True Way!
> The situation in the overall userland has been improving too, so the patch burden is actually very low nowadays.
Is that because long double just isn't used a lot? Or are there more
reasons?
Segher
^ permalink raw reply
* Re: [RFC PATCH 1/4] powerpc/64s: Don't init FSCR_DSCR in __init_FSCR()
From: Michael Neuling @ 2020-06-02 2:48 UTC (permalink / raw)
To: Alistair Popple, Michael Ellerman; +Cc: linuxppc-dev, npiggin, jniethe5
In-Reply-To: <1626791.NDfB26j6xz@townsend>
On Fri, 2020-05-29 at 11:24 +1000, Alistair Popple wrote:
> For what it's worth I tested this series on Mambo PowerNV and it seems to
> correctly enable/disable the prefix FSCR bit based on the cpu feature so feel
> free to add:
>
> Tested-by: Alistair Popple <alistair@popple.id.au>
>
> Mikey is going to test out pseries.
FWIW this worked for me in the P10 + powervm sim testing.
Tested-by: Michael Neuling <mikey@neuling.org>
>
> - Alistair
>
> On Thursday, 28 May 2020 12:58:40 AM AEST Michael Ellerman wrote:
> > __init_FSCR() was added originally in commit 2468dcf641e4 ("powerpc:
> > Add support for context switching the TAR register") (Feb 2013), and
> > only set FSCR_TAR.
> >
> > At that point FSCR (Facility Status and Control Register) was not
> > context switched, so the setting was permanent after boot.
> >
> > Later we added initialisation of FSCR_DSCR to __init_FSCR(), in commit
> > 54c9b2253d34 ("powerpc: Set DSCR bit in FSCR setup") (Mar 2013), again
> > that was permanent after boot.
> >
> > Then commit 2517617e0de6 ("powerpc: Fix context switch DSCR on
> > POWER8") (Aug 2013) added a limited context switch of FSCR, just the
> > FSCR_DSCR bit was context switched based on thread.dscr_inherit. That
> > commit said "This clears the H/FSCR DSCR bit initially", but it
> > didn't, it left the initialisation of FSCR_DSCR in __init_FSCR().
> > However the initial context switch from init_task to pid 1 would clear
> > FSCR_DSCR because thread.dscr_inherit was 0.
> >
> > That commit also introduced the requirement that FSCR_DSCR be clear
> > for user processes, so that we can take the facility unavailable
> > interrupt in order to manage dscr_inherit.
> >
> > Then in commit 152d523e6307 ("powerpc: Create context switch helpers
> > save_sprs() and restore_sprs()") (Dec 2015) FSCR was added to
> > thread_struct. However it still wasn't fully context switched, we just
> > took the existing value and set FSCR_DSCR if the new thread had
> > dscr_inherit set. FSCR was still initialised at boot to FSCR_DSCR |
> > FSCR_TAR, but that value was not propagated into the thread_struct, so
> > the initial context switch set FSCR_DSCR back to 0.
> >
> > Finally commit b57bd2de8c6c ("powerpc: Improve FSCR init and context
> > switching") (Jun 2016) added a full context switch of the FSCR, and
> > added an initialisation of init_task.thread.fscr to FSCR_TAR |
> > FSCR_EBB, but omitted FSCR_DSCR.
> >
> > The end result is that swapper runs with FSCR_DSCR set because of the
> > initialisation in __init_FSCR(), but no other processes do, they use
> > the value from init_task.thread.fscr.
> >
> > Having FSCR_DSCR set for swapper allows it to access SPR 3 from
> > userspace, but swapper never runs userspace, so it has no useful
> > effect. It's also confusing to have the value initialised in two
> > places to two different values.
> >
> > So remove FSCR_DSCR from __init_FSCR(), this at least gets us to the
> > point where there's a single value of FSCR, even if it's still set in
> > two places.
> >
> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> > ---
> > arch/powerpc/kernel/cpu_setup_power.S | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/kernel/cpu_setup_power.S
> > b/arch/powerpc/kernel/cpu_setup_power.S index a460298c7ddb..f91ecb10d0ae
> > 100644
> > --- a/arch/powerpc/kernel/cpu_setup_power.S
> > +++ b/arch/powerpc/kernel/cpu_setup_power.S
> > @@ -184,7 +184,7 @@ _GLOBAL(__restore_cpu_power9)
> >
> > __init_FSCR:
> > mfspr r3,SPRN_FSCR
> > - ori r3,r3,FSCR_TAR|FSCR_DSCR|FSCR_EBB
> > + ori r3,r3,FSCR_TAR|FSCR_EBB
> > mtspr SPRN_FSCR,r3
> > blr
>
>
^ permalink raw reply
* Re: [musl] Re: ppc64le and 32-bit LE userland compatibility
From: Daniel Kolesa @ 2020-06-02 2:55 UTC (permalink / raw)
To: Segher Boessenkool, musl
Cc: libc-alpha, eery, Will Springer, Palmer Dabbelt via binutils,
via libc-dev, linuxppc-dev, Joseph Myers
In-Reply-To: <20200602023630.GP31009@gate.crashing.org>
On Tue, Jun 2, 2020, at 04:36, Segher Boessenkool wrote:
> On Tue, Jun 02, 2020 at 04:12:26AM +0200, Daniel Kolesa wrote:
> > On Tue, Jun 2, 2020, at 03:58, Segher Boessenkool wrote:
> > > I recommend new ports that cannot jump to IEEE QP float directly to use
> > > long double == double for the time being, avoiding the extra
> > > complications that IBM double double would bring. But you'll still have
> > > a transition to IEEE 128 if you ever want to go there.
> > >
> > > But if you already use double-double, I don't know if the cost changing
> > > away from that is worth it now.
> >
> > The transition cost is relatively low, which is why I'm thinking about this in the first place. For one, relatively few things use long double in the first place.
>
> Then your cost switching to QP float later will be low as well. I envy
> you :-)
>
> > For two, on ppc*, at least the bfd linker (which we use always in Void) always tags ELFs with an FP ABI tag, and things not using long double (or using 64-bit long double) don't receive this tag. It even tags *which* ABI is used. See:
>
> That works for statically linked stuff, sure. That is the easy case :-/
It works for dynamically linked stuff too, since anything either using or exposing any API from anywhere that contains a long double signature will result in the tag being emitted. The difference becomes fuzzier once you switch to -mlong-double-64, as then they effectively become one type, and you can no longer check for it. But when it's still on 128-bit (any format) it's reasonably trivial (mostly just check the whole repo for binaries/libraries, dump the tags everywhere, collect them...)
>
> > I went through this once already (I had the 64-bit ldbl transition nearly done) and the number of packages to rebuild in the whole repo was about 200-300 out of ~12000.
>
> Cool! Do you perchance have info you can share about which packages?
> Offline, if you want.
Sure. Here's the list of stuff I had to bump in our repo when I attempted the transition ~half a year ago: https://gist.github.com/q66/08720863a3aec12a6612356cd4c0110f
Of course, we've since gained packages, so it might be slightly different now, and our repos are smaller than something like Debian's, but otherwise this list is not super difficult to compile. I'll be compiling a new one for the ieee754 binary128 transition on glibc when the time comes :)
>
> > > > There is also one more thing while we're at this. The 64-bit big endian Void port uses the ELFv2 ABI, even on glibc. This is not officially supported on glibc as far as I can tell, but it does work out of box, without any patching (things in general match little endian then, i.e. ld64.so.2 etc, but they're big endian). Is there any chance of making that support official?
> > >
> > > (I don't talk for glibc).
> > >
> > > The first thing needed is for "us" to have faith in it. That starts
> > > with seeing test results for the testsuites!
> > >
> > > (Something similar goes for the GCC port -- there is no official support
> > > for BE ELFv2, but of course it does work, and if we get test results we
> > > may keep it that way, hint hint :-) )
> >
> > Well, FreeBSD defaults to it since 13; OpenBSD's new powerpc64 port (which is supposedly dual-endian) defaults to it; musl defaults to it on LE and BE.
>
> ... and no one ever has sent us (GCC) test results (nothing I have seen
> anyway). All we "officially" know is that Power7 BE ELFv2 was a
> bring-up vehicle for the current powerpc64le-linux. Everyone tries not
> to break things without reason to, of course, and things are a little
> bit tested anyway because it is convenient to build ELFv2 stuff on BE
> systems as well (if only to figure out effortlessly if some bug is due
> to the ABI or due to the endianness), but if we do not know something is
> used and we never officially supported it, we might just want to take it
> away if it is inconvenient.
Void keeps track of recent versions fairly closely, so if something does break, you can be sure you'll hear about it :) If there is a breakage in something common on ppc BE configurations, we're usually the first to come across it these days (the Adélie folks are doing a good job but they're usually on LTS versions, and Gentoo musl folks tend to be helpful, but still)
Unfortunately, the maintainer team is stretched relatively thin, and on ppc-related maintenance in the distro, it's mostly just me when it comes to major work, so deaing with all upstreams can be tricky. Still, I try to make sure we don't lag behind too much.
>
> > FreeBSD and OpenBSD have to, since they primarily target LLVM system toolchain (with GCC in ports) and ld.lld doesn't support ELFv1 (at all). Void's port was new (and any precompiled binaries would generally be enterprisey stuff which doesn't concern us enough - people can just make a chroot/container with say, Debian, if they really need to),
>
> Yeah, enterprisey enough, then just rebuild :-)
>
> > so I felt like it didn't make sense to go with the legacy ABI (besides, function descriptors are gross ;)).
>
> Descriptors are Great, you just do not understand the True Way!
>
> > The situation in the overall userland has been improving too, so the patch burden is actually very low nowadays.
>
> Is that because long double just isn't used a lot? Or are there more
> reasons?
I meant ELFv2+BE. Recently support was gained in OpenSSL, for example (though it was a fairly trivial patch... but then, it's pretty much always a fairly trivial patch; the most common pitfalls tend to be mixing of __LITTLE_ENDIAN__ and _CALL_ELF == 2, like, in 90% of cases when there's assembly code hidden behind __LITTLE_ENDIAN__ you can be almost sure it's actually just ELFv2, but those cases have been slowly disappearing)
>
>
> Segher
>
^ permalink raw reply
* [PATCH kernel] powerpc/perf: Stop crashing with generic_compat_pmu
From: Alexey Kardashevskiy @ 2020-06-02 2:56 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Madhavan Srinivasan
The bhrb_filter_map ("The Branch History Rolling Buffer") callback is
only defined in raw CPUs' power_pmu structs. The "architected" CPUs use
generic_compat_pmu which does not have this callback and crashed occur.
This add a NULL pointer check for bhrb_filter_map() which behaves as if
the callback returned an error.
This does not add the same check for config_bhrb() as the only caller
checks for cpuhw->bhrb_users which remains zero if bhrb_filter_map==0.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/perf/core-book3s.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 3dcfecf858f3..36870569bf9c 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1515,9 +1515,16 @@ static int power_pmu_add(struct perf_event *event, int ef_flags)
ret = 0;
out:
if (has_branch_stack(event)) {
- power_pmu_bhrb_enable(event);
- cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
- event->attr.branch_sample_type);
+ u64 bhrb_filter = -1;
+
+ if (ppmu->bhrb_filter_map)
+ bhrb_filter = ppmu->bhrb_filter_map(
+ event->attr.branch_sample_type);
+
+ if (bhrb_filter != -1) {
+ cpuhw->bhrb_filter = bhrb_filter;
+ power_pmu_bhrb_enable(event); /* Does bhrb_users++ */
+ }
}
perf_pmu_enable(event->pmu);
@@ -1839,7 +1846,6 @@ static int power_pmu_event_init(struct perf_event *event)
int n;
int err;
struct cpu_hw_events *cpuhw;
- u64 bhrb_filter;
if (!ppmu)
return -ENOENT;
@@ -1945,7 +1951,10 @@ static int power_pmu_event_init(struct perf_event *event)
err = power_check_constraints(cpuhw, events, cflags, n + 1);
if (has_branch_stack(event)) {
- bhrb_filter = ppmu->bhrb_filter_map(
+ u64 bhrb_filter = -1;
+
+ if (ppmu->bhrb_filter_map)
+ bhrb_filter = ppmu->bhrb_filter_map(
event->attr.branch_sample_type);
if (bhrb_filter == -1) {
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] powerpc/nvram: Replace kmalloc with kzalloc in the error message
From: Michael Ellerman @ 2020-06-02 2:57 UTC (permalink / raw)
To: Markus Elfring, Liao Pingfang, linuxppc-dev
Cc: Yi Wang, Tony Luck, Kees Cook, Wang Liang, Anton Vorontsov,
kernel-janitors, LKML, Colin Cross, Paul Mackerras, Xue Zhihong,
Greg Kroah-Hartman, Thomas Gleixner, Allison Randal
In-Reply-To: <c3d22d89-9133-30aa-8270-c515df214963@web.de>
Markus Elfring <Markus.Elfring@web.de> writes:
>> Please just remove the message instead, it's a tiny allocation that's
>> unlikely to ever fail, and the caller will print an error anyway.
>
> How do you think about to take another look at a previous update suggestion
> like the following?
>
> powerpc/nvram: Delete three error messages for a failed memory allocation
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/00845261-8528-d011-d3b8-e9355a231d3a@users.sourceforge.net/
> https://lore.kernel.org/linuxppc-dev/00845261-8528-d011-d3b8-e9355a231d3a@users.sourceforge.net/
> https://lore.kernel.org/patchwork/patch/752720/
> https://lkml.org/lkml/2017/1/19/537
That deleted the messages from nvram_scan_partitions(), but neither of
the callers of nvram_scan_paritions() check its return value or print
anything if it fails. So removing those messages would make those
failures silent which is not what we want.
cheers
^ permalink raw reply
* [PATCH 0/7] powerpc/watchpoint: Enable 2nd DAWR on baremetal and powervm
From: Ravi Bangoria @ 2020-06-02 4:00 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
Last series[1] was to add basic infrastructure support for more than
one watchpoint on Book3S powerpc. This series actually enables the 2nd
DAWR for baremetal and powervm. Kvm guest is still not supported. This
series depends on Alistair's "Base support for POWER10"[2] series.
[1]: https://lore.kernel.org/linuxppc-dev/20200514111741.97993-1-ravi.bangoria@linux.ibm.com/
[2]: https://lore.kernel.org/linuxppc-dev/20200521014341.29095-1-alistair@popple.id.au
Ravi Bangoria (7):
powerpc/watchpoint: Enable watchpoint functionality on power10 guest
powerpc/dt_cpu_ftrs: Add feature for 2nd DAWR
powerpc/watchpoint: Set CPU_FTR_DAWR1 based on pa-features bit
powerpc/watchpoint: Rename current H_SET_MODE DAWR macro
powerpc/watchpoint: Guest support for 2nd DAWR hcall
powerpc/watchpoint: Return available watchpoints dynamically
powerpc/watchpoint: Remove 512 byte boundary
arch/powerpc/include/asm/cputable.h | 13 +++++++++----
arch/powerpc/include/asm/hvcall.h | 3 ++-
arch/powerpc/include/asm/hw_breakpoint.h | 5 +++--
arch/powerpc/include/asm/machdep.h | 2 +-
arch/powerpc/include/asm/plpar_wrappers.h | 7 ++++++-
arch/powerpc/kernel/dawr.c | 2 +-
arch/powerpc/kernel/dt_cpu_ftrs.c | 7 +++++++
arch/powerpc/kernel/hw_breakpoint.c | 5 +++--
arch/powerpc/kernel/prom.c | 2 ++
arch/powerpc/kvm/book3s_hv.c | 2 +-
arch/powerpc/platforms/pseries/setup.c | 7 +++++--
11 files changed, 40 insertions(+), 15 deletions(-)
--
2.26.2
^ permalink raw reply
* [PATCH 2/7] powerpc/dt_cpu_ftrs: Add feature for 2nd DAWR
From: Ravi Bangoria @ 2020-06-02 4:01 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <20200602040106.127693-1-ravi.bangoria@linux.ibm.com>
Add new device-tree feature for 2nd DAWR. If this feature is present,
2nd DAWR is supported, otherwise not.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/cputable.h | 7 +++++--
arch/powerpc/kernel/dt_cpu_ftrs.c | 7 +++++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index e506d429b1af..3445c86e1f6f 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -214,6 +214,7 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTR_P9_TLBIE_ERAT_BUG LONG_ASM_CONST(0x0001000000000000)
#define CPU_FTR_P9_RADIX_PREFETCH_BUG LONG_ASM_CONST(0x0002000000000000)
#define CPU_FTR_ARCH_31 LONG_ASM_CONST(0x0004000000000000)
+#define CPU_FTR_DAWR1 LONG_ASM_CONST(0x0008000000000000)
#ifndef __ASSEMBLY__
@@ -497,14 +498,16 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTRS_POSSIBLE \
(CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | CPU_FTRS_POWER8 | \
CPU_FTR_ALTIVEC_COMP | CPU_FTR_VSX_COMP | CPU_FTRS_POWER9 | \
- CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
+ CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 | \
+ CPU_FTR_DAWR1)
#else
#define CPU_FTRS_POSSIBLE \
(CPU_FTRS_PPC970 | CPU_FTRS_POWER5 | \
CPU_FTRS_POWER6 | CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | \
CPU_FTRS_POWER8 | CPU_FTRS_CELL | CPU_FTRS_PA6T | \
CPU_FTR_VSX_COMP | CPU_FTR_ALTIVEC_COMP | CPU_FTRS_POWER9 | \
- CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
+ CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 | \
+ CPU_FTR_DAWR1)
#endif /* CONFIG_CPU_LITTLE_ENDIAN */
#endif
#else
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 0a41fce34165..c1c63c6724ea 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -568,6 +568,12 @@ static int __init feat_enable_mma(struct dt_cpu_feature *f)
return 1;
}
+static int __init feat_enable_dawr1(struct dt_cpu_feature *f)
+{
+ cur_cpu_spec->cpu_features |= CPU_FTR_DAWR1;
+ return 1;
+}
+
struct dt_cpu_feature_match {
const char *name;
int (*enable)(struct dt_cpu_feature *f);
@@ -643,6 +649,7 @@ static struct dt_cpu_feature_match __initdata
{"wait-v3", feat_enable, 0},
{"prefix-instructions", feat_enable, 0},
{"matrix-multiply-assist", feat_enable_mma, 0},
+ {"dawr1", feat_enable_dawr1, 0},
};
static bool __initdata using_dt_cpu_ftrs;
--
2.26.2
^ permalink raw reply related
* [PATCH 1/7] powerpc/watchpoint: Enable watchpoint functionality on power10 guest
From: Ravi Bangoria @ 2020-06-02 4:01 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <20200602040106.127693-1-ravi.bangoria@linux.ibm.com>
CPU_FTR_DAWR is by default enabled for host via CPU_FTRS_DT_CPU_BASE
(controlled by CONFIG_PPC_DT_CPU_FTRS). But cpu-features device-tree
node is not PAPR compatible and thus not yet used by kvm or pHyp
guests. Enable watchpoint functionality on power10 guest (both kvm
and powervm) by adding CPU_FTR_DAWR to CPU_FTRS_POWER10. Note that
this change does not enable 2nd DAWR support yet.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/cputable.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index bac2252c839e..e506d429b1af 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -478,7 +478,7 @@ static inline void cpu_feature_keys_init(void) { }
CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | CPU_FTR_PKEY | \
- CPU_FTR_ARCH_31)
+ CPU_FTR_ARCH_31 | CPU_FTR_DAWR)
#define CPU_FTRS_CELL (CPU_FTR_LWSYNC | \
CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
--
2.26.2
^ permalink raw reply related
* [PATCH 3/7] powerpc/watchpoint: Set CPU_FTR_DAWR1 based on pa-features bit
From: Ravi Bangoria @ 2020-06-02 4:01 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <20200602040106.127693-1-ravi.bangoria@linux.ibm.com>
bit 0 of byte 64 in pa-features property indicates availability of 2nd
DAWR registers. i.e. If this bit is set, 2nd DAWR is present, otherwise
not. Host generally uses "cpu-features", which masks "pa-features". But
"cpu-features" are still not used for guests and thus this change is
mostly applicable for guests only.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/kernel/prom.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 1dcf0e214a22..6de59de579f7 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -175,6 +175,8 @@ static struct ibm_pa_feature {
*/
{ .pabyte = 22, .pabit = 0, .cpu_features = CPU_FTR_TM_COMP,
.cpu_user_ftrs2 = PPC_FEATURE2_HTM_COMP | PPC_FEATURE2_HTM_NOSC_COMP },
+
+ { .pabyte = 64, .pabit = 0, .cpu_features = CPU_FTR_DAWR1 },
};
static void __init scan_features(unsigned long node, const unsigned char *ftrs,
--
2.26.2
^ permalink raw reply related
* [PATCH 4/7] powerpc/watchpoint: Rename current H_SET_MODE DAWR macro
From: Ravi Bangoria @ 2020-06-02 4:01 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <20200602040106.127693-1-ravi.bangoria@linux.ibm.com>
Current H_SET_MODE hcall macro name for setting/resetting DAWR0 is
H_SET_MODE_RESOURCE_SET_DAWR. Add suffix 0 to macro name as well.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/hvcall.h | 2 +-
arch/powerpc/include/asm/plpar_wrappers.h | 2 +-
arch/powerpc/kvm/book3s_hv.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index e90c073e437e..a7f6f1aeda6b 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -354,7 +354,7 @@
/* Values for 2nd argument to H_SET_MODE */
#define H_SET_MODE_RESOURCE_SET_CIABR 1
-#define H_SET_MODE_RESOURCE_SET_DAWR 2
+#define H_SET_MODE_RESOURCE_SET_DAWR0 2
#define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE 3
#define H_SET_MODE_RESOURCE_LE 4
diff --git a/arch/powerpc/include/asm/plpar_wrappers.h b/arch/powerpc/include/asm/plpar_wrappers.h
index 4497c8afb573..93eb133d572c 100644
--- a/arch/powerpc/include/asm/plpar_wrappers.h
+++ b/arch/powerpc/include/asm/plpar_wrappers.h
@@ -312,7 +312,7 @@ static inline long plpar_set_ciabr(unsigned long ciabr)
static inline long plpar_set_watchpoint0(unsigned long dawr0, unsigned long dawrx0)
{
- return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR, dawr0, dawrx0);
+ return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR0, dawr0, dawrx0);
}
static inline long plpar_signal_sys_reset(long cpu)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index a0cf17597838..26820b7bd75c 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -766,7 +766,7 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu, unsigned long mflags,
return H_P3;
vcpu->arch.ciabr = value1;
return H_SUCCESS;
- case H_SET_MODE_RESOURCE_SET_DAWR:
+ case H_SET_MODE_RESOURCE_SET_DAWR0:
if (!kvmppc_power8_compatible(vcpu))
return H_P2;
if (!ppc_breakpoint_available())
--
2.26.2
^ permalink raw reply related
* [PATCH 5/7] powerpc/watchpoint: Guest support for 2nd DAWR hcall
From: Ravi Bangoria @ 2020-06-02 4:01 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <20200602040106.127693-1-ravi.bangoria@linux.ibm.com>
2nd DAWR can be set/unset using H_SET_MODE hcall with resource value 5.
Enable powervm guest support with that. This has no effect on kvm guest
because kvm will return error if guest does hcall with resource value 5.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/hvcall.h | 1 +
arch/powerpc/include/asm/machdep.h | 2 +-
arch/powerpc/include/asm/plpar_wrappers.h | 5 +++++
arch/powerpc/kernel/dawr.c | 2 +-
arch/powerpc/platforms/pseries/setup.c | 7 +++++--
5 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index a7f6f1aeda6b..3f170b9496a1 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -357,6 +357,7 @@
#define H_SET_MODE_RESOURCE_SET_DAWR0 2
#define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE 3
#define H_SET_MODE_RESOURCE_LE 4
+#define H_SET_MODE_RESOURCE_SET_DAWR1 5
/* Values for argument to H_SIGNAL_SYS_RESET */
#define H_SIGNAL_SYS_RESET_ALL -1
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index 7bcb64444a39..a90b892f0bfe 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -131,7 +131,7 @@ struct machdep_calls {
unsigned long dabrx);
/* Set DAWR for this platform, leave empty for default implementation */
- int (*set_dawr)(unsigned long dawr,
+ int (*set_dawr)(int nr, unsigned long dawr,
unsigned long dawrx);
#ifdef CONFIG_PPC32 /* XXX for now */
diff --git a/arch/powerpc/include/asm/plpar_wrappers.h b/arch/powerpc/include/asm/plpar_wrappers.h
index 93eb133d572c..d7a1acc83593 100644
--- a/arch/powerpc/include/asm/plpar_wrappers.h
+++ b/arch/powerpc/include/asm/plpar_wrappers.h
@@ -315,6 +315,11 @@ static inline long plpar_set_watchpoint0(unsigned long dawr0, unsigned long dawr
return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR0, dawr0, dawrx0);
}
+static inline long plpar_set_watchpoint1(unsigned long dawr1, unsigned long dawrx1)
+{
+ return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR1, dawr1, dawrx1);
+}
+
static inline long plpar_signal_sys_reset(long cpu)
{
return plpar_hcall_norets(H_SIGNAL_SYS_RESET, cpu);
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index 500f52fa4711..cdc2dccb987d 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -37,7 +37,7 @@ int set_dawr(int nr, struct arch_hw_breakpoint *brk)
dawrx |= (mrd & 0x3f) << (63 - 53);
if (ppc_md.set_dawr)
- return ppc_md.set_dawr(dawr, dawrx);
+ return ppc_md.set_dawr(nr, dawr, dawrx);
if (nr == 0) {
mtspr(SPRN_DAWR0, dawr);
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 64d18f4bf093..b001cde1a2d7 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -832,12 +832,15 @@ static int pseries_set_xdabr(unsigned long dabr, unsigned long dabrx)
return plpar_hcall_norets(H_SET_XDABR, dabr, dabrx);
}
-static int pseries_set_dawr(unsigned long dawr, unsigned long dawrx)
+static int pseries_set_dawr(int nr, unsigned long dawr, unsigned long dawrx)
{
/* PAPR says we can't set HYP */
dawrx &= ~DAWRX_HYP;
- return plpar_set_watchpoint0(dawr, dawrx);
+ if (nr == 0)
+ return plpar_set_watchpoint0(dawr, dawrx);
+ else
+ return plpar_set_watchpoint1(dawr, dawrx);
}
#define CMO_CHARACTERISTICS_TOKEN 44
--
2.26.2
^ permalink raw reply related
* [PATCH 6/7] powerpc/watchpoint: Return available watchpoints dynamically
From: Ravi Bangoria @ 2020-06-02 4:01 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <20200602040106.127693-1-ravi.bangoria@linux.ibm.com>
So far Book3S Powerpc supported only one watchpoint. But Power10 is
introducing 2nd DAWR. Enable 2nd DAWR support for Power10. Availability
of 2nd DAWR will depend on CPU_FTR_DAWR1.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/cputable.h | 4 +++-
arch/powerpc/include/asm/hw_breakpoint.h | 5 +++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 3445c86e1f6f..f110f70a11bf 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -633,7 +633,9 @@ enum {
* Maximum number of hw breakpoint supported on powerpc. Number of
* breakpoints supported by actual hw might be less than this.
*/
-#define HBP_NUM_MAX 1
+#define HBP_NUM_MAX 2
+#define HBP_NUM_P7_TO_P9 1
+#define HBP_NUM_P10 2
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index f42a55eb77d2..0365a137d6e8 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -5,10 +5,11 @@
* Copyright 2010, IBM Corporation.
* Author: K.Prasad <prasad@linux.vnet.ibm.com>
*/
-
#ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
#define _PPC_BOOK3S_64_HW_BREAKPOINT_H
+#include <asm/cpu_has_feature.h>
+
#ifdef __KERNEL__
struct arch_hw_breakpoint {
unsigned long address;
@@ -46,7 +47,7 @@ struct arch_hw_breakpoint {
static inline int nr_wp_slots(void)
{
- return HBP_NUM_MAX;
+ return cpu_has_feature(CPU_FTR_DAWR1) ? HBP_NUM_P10 : HBP_NUM_P7_TO_P9;
}
#ifdef CONFIG_HAVE_HW_BREAKPOINT
--
2.26.2
^ permalink raw reply related
* [PATCH 7/7] powerpc/watchpoint: Remove 512 byte boundary
From: Ravi Bangoria @ 2020-06-02 4:01 UTC (permalink / raw)
To: mpe, mikey
Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec, oleg,
npiggin, linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <20200602040106.127693-1-ravi.bangoria@linux.ibm.com>
Power10 has removed 512 bytes boundary from match criteria. i.e. The match
range can be 512 bytes unaligned as well.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/kernel/hw_breakpoint.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 0000daf0e1da..270e655108f4 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -418,8 +418,9 @@ static int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw)
if (dawr_enabled()) {
max_len = DAWR_MAX_LEN;
- /* DAWR region can't cross 512 bytes boundary */
- if (ALIGN(start_addr, SZ_512M) != ALIGN(end_addr - 1, SZ_512M))
+ /* DAWR region can't cross 512 bytes boundary on p8 and p9 */
+ if (!cpu_has_feature(CPU_FTR_ARCH_31) &&
+ (ALIGN(start_addr, SZ_512M) != ALIGN(end_addr - 1, SZ_512M)))
return -EINVAL;
} else if (IS_ENABLED(CONFIG_PPC_8xx)) {
/* 8xx can setup a range without limitation */
--
2.26.2
^ permalink raw reply related
* [PATCH] cxl: Remove dead Kconfig options
From: Andrew Donnellan @ 2020-06-02 4:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: fbarrat
The CXL_AFU_DRIVER_OPS and CXL_LIB Kconfig options were added to coordinate
merging of new features. They no longer serve any purpose, so remove them.
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
---
drivers/misc/cxl/Kconfig | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/misc/cxl/Kconfig b/drivers/misc/cxl/Kconfig
index 39eec9031487..51aecafdcbdf 100644
--- a/drivers/misc/cxl/Kconfig
+++ b/drivers/misc/cxl/Kconfig
@@ -7,18 +7,10 @@ config CXL_BASE
bool
select PPC_COPRO_BASE
-config CXL_AFU_DRIVER_OPS
- bool
-
-config CXL_LIB
- bool
-
config CXL
tristate "Support for IBM Coherent Accelerators (CXL)"
depends on PPC_POWERNV && PCI_MSI && EEH
select CXL_BASE
- select CXL_AFU_DRIVER_OPS
- select CXL_LIB
default m
help
Select this option to enable driver support for IBM Coherent
--
2.20.1
^ permalink raw reply related
* [PATCH] hw_breakpoint: Fix build warnings with clang
From: Ravi Bangoria @ 2020-06-02 4:12 UTC (permalink / raw)
To: mpe
Cc: christophe.leroy, ravi.bangoria, mikey, apopple, linux-kernel,
npiggin, paulus, naveen.n.rao, linuxppc-dev
kbuild test robot reported few build warnings with hw_breakpoint code
when compiled with clang[1]. Fix those.
[1]: https://lore.kernel.org/linuxppc-dev/202005192233.oi9CjRtA%25lkp@intel.com/
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
Note: Prepared on top of powerpc/next.
arch/powerpc/include/asm/hw_breakpoint.h | 3 ---
include/linux/hw_breakpoint.h | 4 ++++
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index f42a55eb77d2..cb424799da0d 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -70,9 +70,6 @@ extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
unsigned long val, void *data);
int arch_install_hw_breakpoint(struct perf_event *bp);
void arch_uninstall_hw_breakpoint(struct perf_event *bp);
-int arch_reserve_bp_slot(struct perf_event *bp);
-void arch_release_bp_slot(struct perf_event *bp);
-void arch_unregister_hw_breakpoint(struct perf_event *bp);
void hw_breakpoint_pmu_read(struct perf_event *bp);
extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index 6058c3844a76..521481f0d320 100644
--- a/include/linux/hw_breakpoint.h
+++ b/include/linux/hw_breakpoint.h
@@ -80,6 +80,10 @@ extern int dbg_reserve_bp_slot(struct perf_event *bp);
extern int dbg_release_bp_slot(struct perf_event *bp);
extern int reserve_bp_slot(struct perf_event *bp);
extern void release_bp_slot(struct perf_event *bp);
+extern int hw_breakpoint_weight(struct perf_event *bp);
+extern int arch_reserve_bp_slot(struct perf_event *bp);
+extern void arch_release_bp_slot(struct perf_event *bp);
+extern void arch_unregister_hw_breakpoint(struct perf_event *bp);
extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
--
2.26.2
^ permalink raw reply related
* [PATCH 1/2] powerpc/pseries: remove cede offline state for CPUs
From: Nathan Lynch @ 2020-06-02 4:31 UTC (permalink / raw)
To: linuxppc-dev; +Cc: svaidy, npiggin, ego
In-Reply-To: <20200602043140.397746-1-nathanl@linux.ibm.com>
This effectively reverts commit 3aa565f53c39 ("powerpc/pseries: Add
hooks to put the CPU into an appropriate offline state"), which added
an offline mode for CPUs which uses the H_CEDE hcall instead of the
architected stop-self RTAS function in order to facilitate "folding"
of dedicated mode processors on PowerVM platforms to achieve energy
savings. This has been the default offline mode since its
introduction.
There's nothing about stop-self that would prevent the hypervisor from
achieving the energy savings available via H_CEDE, so the original
premise of this change appears to be flawed.
I also have encountered the claim that the transition to and from
ceded state is much faster than stop-self/start-cpu. Certainly we
would not want to use stop-self as an *idle* mode. That is what H_CEDE
is for. However, this difference is insignificant in the context of
Linux CPU hotplug, where the latency of an offline or online operation
on current systems is on the order of 100ms, mainly attributable to
all the various subsystems' cpuhp callbacks.
The cede offline mode also prevents accurate accounting, as discussed
before:
https://lore.kernel.org/linuxppc-dev/1571740391-3251-1-git-send-email-ego@linux.vnet.ibm.com/
Unconditionally use stop-self to offline processor threads. This is
the architected method for offlining CPUs on PAPR systems.
The "cede_offline" boot parameter is rendered obsolete.
Removing this code enables the removal of the partition suspend code
which temporarily onlines all present CPUs.
Fixes: 3aa565f53c39 ("powerpc/pseries: Add hooks to put the CPU into an appropriate offline state")
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
Documentation/core-api/cpu_hotplug.rst | 7 -
arch/powerpc/platforms/pseries/hotplug-cpu.c | 170 ++----------------
.../platforms/pseries/offline_states.h | 38 ----
arch/powerpc/platforms/pseries/pmem.c | 1 -
arch/powerpc/platforms/pseries/smp.c | 28 +--
5 files changed, 15 insertions(+), 229 deletions(-)
delete mode 100644 arch/powerpc/platforms/pseries/offline_states.h
diff --git a/Documentation/core-api/cpu_hotplug.rst b/Documentation/core-api/cpu_hotplug.rst
index 4a50ab7817f7..b1ae1ac159cf 100644
--- a/Documentation/core-api/cpu_hotplug.rst
+++ b/Documentation/core-api/cpu_hotplug.rst
@@ -50,13 +50,6 @@ Command Line Switches
This option is limited to the X86 and S390 architecture.
-``cede_offline={"off","on"}``
- Use this option to disable/enable putting offlined processors to an extended
- ``H_CEDE`` state on supported pseries platforms. If nothing is specified,
- ``cede_offline`` is set to "on".
-
- This option is limited to the PowerPC architecture.
-
``cpu0_hotplug``
Allow to shutdown CPU0.
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 3e8cbfe7a80f..d4b346355bb9 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -35,54 +35,10 @@
#include <asm/topology.h>
#include "pseries.h"
-#include "offline_states.h"
/* This version can't take the spinlock, because it never returns */
static int rtas_stop_self_token = RTAS_UNKNOWN_SERVICE;
-static DEFINE_PER_CPU(enum cpu_state_vals, preferred_offline_state) =
- CPU_STATE_OFFLINE;
-static DEFINE_PER_CPU(enum cpu_state_vals, current_state) = CPU_STATE_OFFLINE;
-
-static enum cpu_state_vals default_offline_state = CPU_STATE_OFFLINE;
-
-static bool cede_offline_enabled __read_mostly = true;
-
-/*
- * Enable/disable cede_offline when available.
- */
-static int __init setup_cede_offline(char *str)
-{
- return (kstrtobool(str, &cede_offline_enabled) == 0);
-}
-
-__setup("cede_offline=", setup_cede_offline);
-
-enum cpu_state_vals get_cpu_current_state(int cpu)
-{
- return per_cpu(current_state, cpu);
-}
-
-void set_cpu_current_state(int cpu, enum cpu_state_vals state)
-{
- per_cpu(current_state, cpu) = state;
-}
-
-enum cpu_state_vals get_preferred_offline_state(int cpu)
-{
- return per_cpu(preferred_offline_state, cpu);
-}
-
-void set_preferred_offline_state(int cpu, enum cpu_state_vals state)
-{
- per_cpu(preferred_offline_state, cpu) = state;
-}
-
-void set_default_offline_state(int cpu)
-{
- per_cpu(preferred_offline_state, cpu) = default_offline_state;
-}
-
static void rtas_stop_self(void)
{
static struct rtas_args args;
@@ -101,9 +57,7 @@ static void rtas_stop_self(void)
static void pseries_mach_cpu_die(void)
{
- unsigned int cpu = smp_processor_id();
unsigned int hwcpu = hard_smp_processor_id();
- u8 cede_latency_hint = 0;
local_irq_disable();
idle_task_exit();
@@ -112,49 +66,6 @@ static void pseries_mach_cpu_die(void)
else
xics_teardown_cpu();
- if (get_preferred_offline_state(cpu) == CPU_STATE_INACTIVE) {
- set_cpu_current_state(cpu, CPU_STATE_INACTIVE);
- if (ppc_md.suspend_disable_cpu)
- ppc_md.suspend_disable_cpu();
-
- cede_latency_hint = 2;
-
- get_lppaca()->idle = 1;
- if (!lppaca_shared_proc(get_lppaca()))
- get_lppaca()->donate_dedicated_cpu = 1;
-
- while (get_preferred_offline_state(cpu) == CPU_STATE_INACTIVE) {
- while (!prep_irq_for_idle()) {
- local_irq_enable();
- local_irq_disable();
- }
-
- extended_cede_processor(cede_latency_hint);
- }
-
- local_irq_disable();
-
- if (!lppaca_shared_proc(get_lppaca()))
- get_lppaca()->donate_dedicated_cpu = 0;
- get_lppaca()->idle = 0;
-
- if (get_preferred_offline_state(cpu) == CPU_STATE_ONLINE) {
- unregister_slb_shadow(hwcpu);
-
- hard_irq_disable();
- /*
- * Call to start_secondary_resume() will not return.
- * Kernel stack will be reset and start_secondary()
- * will be called to continue the online operation.
- */
- start_secondary_resume();
- }
- }
-
- /* Requested state is CPU_STATE_OFFLINE at this point */
- WARN_ON(get_preferred_offline_state(cpu) != CPU_STATE_OFFLINE);
-
- set_cpu_current_state(cpu, CPU_STATE_OFFLINE);
unregister_slb_shadow(hwcpu);
rtas_stop_self();
@@ -200,24 +111,13 @@ static void pseries_cpu_die(unsigned int cpu)
int cpu_status = 1;
unsigned int pcpu = get_hard_smp_processor_id(cpu);
- if (get_preferred_offline_state(cpu) == CPU_STATE_INACTIVE) {
- cpu_status = 1;
- for (tries = 0; tries < 5000; tries++) {
- if (get_cpu_current_state(cpu) == CPU_STATE_INACTIVE) {
- cpu_status = 0;
- break;
- }
- msleep(1);
- }
- } else if (get_preferred_offline_state(cpu) == CPU_STATE_OFFLINE) {
+ for (tries = 0; tries < 25; tries++) {
+ cpu_status = smp_query_cpu_stopped(pcpu);
+ if (cpu_status == QCSS_STOPPED ||
+ cpu_status == QCSS_HARDWARE_ERROR)
+ break;
+ cpu_relax();
- for (tries = 0; tries < 25; tries++) {
- cpu_status = smp_query_cpu_stopped(pcpu);
- if (cpu_status == QCSS_STOPPED ||
- cpu_status == QCSS_HARDWARE_ERROR)
- break;
- cpu_relax();
- }
}
if (cpu_status != 0) {
@@ -359,28 +259,15 @@ static int dlpar_offline_cpu(struct device_node *dn)
if (get_hard_smp_processor_id(cpu) != thread)
continue;
- if (get_cpu_current_state(cpu) == CPU_STATE_OFFLINE)
+ if (!cpu_online(cpu))
break;
- if (get_cpu_current_state(cpu) == CPU_STATE_ONLINE) {
- set_preferred_offline_state(cpu,
- CPU_STATE_OFFLINE);
- cpu_maps_update_done();
- timed_topology_update(1);
- rc = device_offline(get_cpu_device(cpu));
- if (rc)
- goto out;
- cpu_maps_update_begin();
- break;
- }
-
- /*
- * The cpu is in CPU_STATE_INACTIVE.
- * Upgrade it's state to CPU_STATE_OFFLINE.
- */
- set_preferred_offline_state(cpu, CPU_STATE_OFFLINE);
- WARN_ON(plpar_hcall_norets(H_PROD, thread) != H_SUCCESS);
- __cpu_die(cpu);
+ cpu_maps_update_done();
+ timed_topology_update(1);
+ rc = device_offline(get_cpu_device(cpu));
+ if (rc)
+ goto out;
+ cpu_maps_update_begin();
break;
}
if (cpu == num_possible_cpus()) {
@@ -414,8 +301,6 @@ static int dlpar_online_cpu(struct device_node *dn)
for_each_present_cpu(cpu) {
if (get_hard_smp_processor_id(cpu) != thread)
continue;
- BUG_ON(get_cpu_current_state(cpu)
- != CPU_STATE_OFFLINE);
cpu_maps_update_done();
timed_topology_update(1);
find_and_online_cpu_nid(cpu);
@@ -1013,27 +898,8 @@ static struct notifier_block pseries_smp_nb = {
.notifier_call = pseries_smp_notifier,
};
-#define MAX_CEDE_LATENCY_LEVELS 4
-#define CEDE_LATENCY_PARAM_LENGTH 10
-#define CEDE_LATENCY_PARAM_MAX_LENGTH \
- (MAX_CEDE_LATENCY_LEVELS * CEDE_LATENCY_PARAM_LENGTH * sizeof(char))
-#define CEDE_LATENCY_TOKEN 45
-
-static char cede_parameters[CEDE_LATENCY_PARAM_MAX_LENGTH];
-
-static int parse_cede_parameters(void)
-{
- memset(cede_parameters, 0, CEDE_LATENCY_PARAM_MAX_LENGTH);
- return rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1,
- NULL,
- CEDE_LATENCY_TOKEN,
- __pa(cede_parameters),
- CEDE_LATENCY_PARAM_MAX_LENGTH);
-}
-
static int __init pseries_cpu_hotplug_init(void)
{
- int cpu;
int qcss_tok;
#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
@@ -1056,16 +922,8 @@ static int __init pseries_cpu_hotplug_init(void)
smp_ops->cpu_die = pseries_cpu_die;
/* Processors can be added/removed only on LPAR */
- if (firmware_has_feature(FW_FEATURE_LPAR)) {
+ if (firmware_has_feature(FW_FEATURE_LPAR))
of_reconfig_notifier_register(&pseries_smp_nb);
- cpu_maps_update_begin();
- if (cede_offline_enabled && parse_cede_parameters() == 0) {
- default_offline_state = CPU_STATE_INACTIVE;
- for_each_online_cpu(cpu)
- set_default_offline_state(cpu);
- }
- cpu_maps_update_done();
- }
return 0;
}
diff --git a/arch/powerpc/platforms/pseries/offline_states.h b/arch/powerpc/platforms/pseries/offline_states.h
deleted file mode 100644
index 51414aee2862..000000000000
--- a/arch/powerpc/platforms/pseries/offline_states.h
+++ /dev/null
@@ -1,38 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _OFFLINE_STATES_H_
-#define _OFFLINE_STATES_H_
-
-/* Cpu offline states go here */
-enum cpu_state_vals {
- CPU_STATE_OFFLINE,
- CPU_STATE_INACTIVE,
- CPU_STATE_ONLINE,
- CPU_MAX_OFFLINE_STATES
-};
-
-#ifdef CONFIG_HOTPLUG_CPU
-extern enum cpu_state_vals get_cpu_current_state(int cpu);
-extern void set_cpu_current_state(int cpu, enum cpu_state_vals state);
-extern void set_preferred_offline_state(int cpu, enum cpu_state_vals state);
-extern void set_default_offline_state(int cpu);
-#else
-static inline enum cpu_state_vals get_cpu_current_state(int cpu)
-{
- return CPU_STATE_ONLINE;
-}
-
-static inline void set_cpu_current_state(int cpu, enum cpu_state_vals state)
-{
-}
-
-static inline void set_preferred_offline_state(int cpu, enum cpu_state_vals state)
-{
-}
-
-static inline void set_default_offline_state(int cpu)
-{
-}
-#endif
-
-extern enum cpu_state_vals get_preferred_offline_state(int cpu);
-#endif
diff --git a/arch/powerpc/platforms/pseries/pmem.c b/arch/powerpc/platforms/pseries/pmem.c
index f860a897a9e0..f827de7087e9 100644
--- a/arch/powerpc/platforms/pseries/pmem.c
+++ b/arch/powerpc/platforms/pseries/pmem.c
@@ -24,7 +24,6 @@
#include <asm/topology.h>
#include "pseries.h"
-#include "offline_states.h"
static struct device_node *pmem_node;
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index ad61e90032da..a8a070269151 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -44,8 +44,6 @@
#include <asm/svm.h>
#include "pseries.h"
-#include "offline_states.h"
-
/*
* The Primary thread of each non-boot processor was started from the OF client
@@ -108,10 +106,7 @@ static inline int smp_startup_cpu(unsigned int lcpu)
/* Fixup atomic count: it exited inside IRQ handler. */
task_thread_info(paca_ptrs[lcpu]->__current)->preempt_count = 0;
-#ifdef CONFIG_HOTPLUG_CPU
- if (get_cpu_current_state(lcpu) == CPU_STATE_INACTIVE)
- goto out;
-#endif
+
/*
* If the RTAS start-cpu token does not exist then presume the
* cpu is already spinning.
@@ -126,9 +121,6 @@ static inline int smp_startup_cpu(unsigned int lcpu)
return 0;
}
-#ifdef CONFIG_HOTPLUG_CPU
-out:
-#endif
return 1;
}
@@ -143,10 +135,6 @@ static void smp_setup_cpu(int cpu)
vpa_init(cpu);
cpumask_clear_cpu(cpu, of_spin_mask);
-#ifdef CONFIG_HOTPLUG_CPU
- set_cpu_current_state(cpu, CPU_STATE_ONLINE);
- set_default_offline_state(cpu);
-#endif
}
static int smp_pSeries_kick_cpu(int nr)
@@ -163,20 +151,6 @@ static int smp_pSeries_kick_cpu(int nr)
* the processor will continue on to secondary_start
*/
paca_ptrs[nr]->cpu_start = 1;
-#ifdef CONFIG_HOTPLUG_CPU
- set_preferred_offline_state(nr, CPU_STATE_ONLINE);
-
- if (get_cpu_current_state(nr) == CPU_STATE_INACTIVE) {
- long rc;
- unsigned long hcpuid;
-
- hcpuid = get_hard_smp_processor_id(nr);
- rc = plpar_hcall_norets(H_PROD, hcpuid);
- if (rc != H_SUCCESS)
- printk(KERN_ERR "Error: Prod to wake up processor %d "
- "Ret= %ld\n", nr, rc);
- }
-#endif
return 0;
}
--
2.25.4
^ permalink raw reply related
* [PATCH 0/2] pseries extended cede cpu offline mode removal
From: Nathan Lynch @ 2020-06-02 4:31 UTC (permalink / raw)
To: linuxppc-dev; +Cc: svaidy, npiggin, ego
I propose removing the "extended cede" offline mode for CPUs as well
as the partition suspend code which accommodates it by temporarily
onlining all CPUs prior to suspending the LPAR.
Detailed justifications are within the individual commit messages.
I'm hoping to move the pseries partition suspend implementation to
the Linux suspend framework, and I expect that having only one offline
mode for CPUs will make that task quite a bit less complex.
Nathan Lynch (2):
powerpc/pseries: remove cede offline state for CPUs
powerpc/rtas: don't online CPUs for partition suspend
Documentation/core-api/cpu_hotplug.rst | 7 -
arch/powerpc/include/asm/rtas.h | 2 -
arch/powerpc/kernel/rtas.c | 122 +------------
arch/powerpc/platforms/pseries/hotplug-cpu.c | 170 ++----------------
.../platforms/pseries/offline_states.h | 38 ----
arch/powerpc/platforms/pseries/pmem.c | 1 -
arch/powerpc/platforms/pseries/smp.c | 28 +--
arch/powerpc/platforms/pseries/suspend.c | 22 +--
8 files changed, 18 insertions(+), 372 deletions(-)
delete mode 100644 arch/powerpc/platforms/pseries/offline_states.h
--
2.25.4
^ permalink raw reply
* [PATCH 2/2] powerpc/rtas: don't online CPUs for partition suspend
From: Nathan Lynch @ 2020-06-02 4:31 UTC (permalink / raw)
To: linuxppc-dev; +Cc: svaidy, npiggin, ego
In-Reply-To: <20200602043140.397746-1-nathanl@linux.ibm.com>
Partition suspension, used for hibernation and migration, requires
that the OS place all but one of the LPAR's processor threads into one
of two states prior to calling the ibm,suspend-me RTAS function:
* the architected offline state (via RTAS stop-self); or
* the H_JOIN hcall, which does not return until the partition
resumes execution
Using H_CEDE as the offline mode, introduced by
commit 3aa565f53c39 ("powerpc/pseries: Add hooks to put the CPU into
an appropriate offline state"), means that any threads which are
offline from Linux's point of view must be moved to one of those two
states before a partition suspension can proceed.
This was eventually addressed in commit 120496ac2d2d ("powerpc: Bring
all threads online prior to migration/hibernation"), which added code
to temporarily bring up any offline processor threads so they can call
H_JOIN. Conceptually this is fine, but the implementation has had
multiple races with cpu hotplug operations initiated from user
space[1][2][3], the error handling is fragile, and it generates
user-visible cpu hotplug events which is a lot of noise for a platform
feature that's supposed to minimize disruption to workloads.
With commit 3aa565f53c39 ("powerpc/pseries: Add hooks to put the CPU
into an appropriate offline state") reverted, this code becomes
unnecessary, so remove it. Since any offline CPUs now are truly
offline from the platform's point of view, it is no longer necessary
to bring up CPUs only to have them call H_JOIN and then go offline
again upon resuming. Only active threads are required to call H_JOIN;
stopped threads can be left alone.
[1] commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
serialization during LPM")
[2] commit 9fb603050ffd ("powerpc/rtas: retry when cpu offline races
with suspend/migration")
[3] commit dfd718a2ed1f ("powerpc/rtas: Fix a potential race between
CPU-Offline & Migration")
Fixes: 120496ac2d2d ("powerpc: Bring all threads online prior to migration/hibernation")
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/include/asm/rtas.h | 2 -
arch/powerpc/kernel/rtas.c | 122 +----------------------
arch/powerpc/platforms/pseries/suspend.c | 22 +---
3 files changed, 3 insertions(+), 143 deletions(-)
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 3c1887351c71..bd227e0eab07 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -368,8 +368,6 @@ extern int rtas_set_indicator_fast(int indicator, int index, int new_value);
extern void rtas_progress(char *s, unsigned short hex);
extern int rtas_suspend_cpu(struct rtas_suspend_me_data *data);
extern int rtas_suspend_last_cpu(struct rtas_suspend_me_data *data);
-extern int rtas_online_cpus_mask(cpumask_var_t cpus);
-extern int rtas_offline_cpus_mask(cpumask_var_t cpus);
extern int rtas_ibm_suspend_me(u64 handle);
struct rtc_time;
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index c5fa251b8950..01210593d60c 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -842,96 +842,6 @@ static void rtas_percpu_suspend_me(void *info)
__rtas_suspend_cpu((struct rtas_suspend_me_data *)info, 1);
}
-enum rtas_cpu_state {
- DOWN,
- UP,
-};
-
-#ifndef CONFIG_SMP
-static int rtas_cpu_state_change_mask(enum rtas_cpu_state state,
- cpumask_var_t cpus)
-{
- if (!cpumask_empty(cpus)) {
- cpumask_clear(cpus);
- return -EINVAL;
- } else
- return 0;
-}
-#else
-/* On return cpumask will be altered to indicate CPUs changed.
- * CPUs with states changed will be set in the mask,
- * CPUs with status unchanged will be unset in the mask. */
-static int rtas_cpu_state_change_mask(enum rtas_cpu_state state,
- cpumask_var_t cpus)
-{
- int cpu;
- int cpuret = 0;
- int ret = 0;
-
- if (cpumask_empty(cpus))
- return 0;
-
- for_each_cpu(cpu, cpus) {
- struct device *dev = get_cpu_device(cpu);
-
- switch (state) {
- case DOWN:
- cpuret = device_offline(dev);
- break;
- case UP:
- cpuret = device_online(dev);
- break;
- }
- if (cpuret < 0) {
- pr_debug("%s: cpu_%s for cpu#%d returned %d.\n",
- __func__,
- ((state == UP) ? "up" : "down"),
- cpu, cpuret);
- if (!ret)
- ret = cpuret;
- if (state == UP) {
- /* clear bits for unchanged cpus, return */
- cpumask_shift_right(cpus, cpus, cpu);
- cpumask_shift_left(cpus, cpus, cpu);
- break;
- } else {
- /* clear bit for unchanged cpu, continue */
- cpumask_clear_cpu(cpu, cpus);
- }
- }
- cond_resched();
- }
-
- return ret;
-}
-#endif
-
-int rtas_online_cpus_mask(cpumask_var_t cpus)
-{
- int ret;
-
- ret = rtas_cpu_state_change_mask(UP, cpus);
-
- if (ret) {
- cpumask_var_t tmp_mask;
-
- if (!alloc_cpumask_var(&tmp_mask, GFP_KERNEL))
- return ret;
-
- /* Use tmp_mask to preserve cpus mask from first failure */
- cpumask_copy(tmp_mask, cpus);
- rtas_offline_cpus_mask(tmp_mask);
- free_cpumask_var(tmp_mask);
- }
-
- return ret;
-}
-
-int rtas_offline_cpus_mask(cpumask_var_t cpus)
-{
- return rtas_cpu_state_change_mask(DOWN, cpus);
-}
-
int rtas_ibm_suspend_me(u64 handle)
{
long state;
@@ -939,8 +849,6 @@ int rtas_ibm_suspend_me(u64 handle)
unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
struct rtas_suspend_me_data data;
DECLARE_COMPLETION_ONSTACK(done);
- cpumask_var_t offline_mask;
- int cpuret;
if (!rtas_service_present("ibm,suspend-me"))
return -ENOSYS;
@@ -961,9 +869,6 @@ int rtas_ibm_suspend_me(u64 handle)
return -EIO;
}
- if (!alloc_cpumask_var(&offline_mask, GFP_KERNEL))
- return -ENOMEM;
-
atomic_set(&data.working, 0);
atomic_set(&data.done, 0);
atomic_set(&data.error, 0);
@@ -972,24 +877,8 @@ int rtas_ibm_suspend_me(u64 handle)
lock_device_hotplug();
- /* All present CPUs must be online */
- cpumask_andnot(offline_mask, cpu_present_mask, cpu_online_mask);
- cpuret = rtas_online_cpus_mask(offline_mask);
- if (cpuret) {
- pr_err("%s: Could not bring present CPUs online.\n", __func__);
- atomic_set(&data.error, cpuret);
- goto out;
- }
-
cpu_hotplug_disable();
- /* Check if we raced with a CPU-Offline Operation */
- if (!cpumask_equal(cpu_present_mask, cpu_online_mask)) {
- pr_info("%s: Raced against a concurrent CPU-Offline\n", __func__);
- atomic_set(&data.error, -EAGAIN);
- goto out_hotplug_enable;
- }
-
/* Call function on all CPUs. One of us will make the
* rtas call
*/
@@ -1000,18 +889,11 @@ int rtas_ibm_suspend_me(u64 handle)
if (atomic_read(&data.error) != 0)
printk(KERN_ERR "Error doing global join\n");
-out_hotplug_enable:
- cpu_hotplug_enable();
- /* Take down CPUs not online prior to suspend */
- cpuret = rtas_offline_cpus_mask(offline_mask);
- if (cpuret)
- pr_warn("%s: Could not restore CPUs to offline state.\n",
- __func__);
+ cpu_hotplug_enable();
-out:
unlock_device_hotplug();
- free_cpumask_var(offline_mask);
+
return atomic_read(&data.error);
}
#else /* CONFIG_PPC_PSERIES */
diff --git a/arch/powerpc/platforms/pseries/suspend.c b/arch/powerpc/platforms/pseries/suspend.c
index 0a24a5a185f0..f789693f61f4 100644
--- a/arch/powerpc/platforms/pseries/suspend.c
+++ b/arch/powerpc/platforms/pseries/suspend.c
@@ -132,15 +132,11 @@ static ssize_t store_hibernate(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
- cpumask_var_t offline_mask;
int rc;
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
- if (!alloc_cpumask_var(&offline_mask, GFP_KERNEL))
- return -ENOMEM;
-
stream_id = simple_strtoul(buf, NULL, 16);
do {
@@ -150,32 +146,16 @@ static ssize_t store_hibernate(struct device *dev,
} while (rc == -EAGAIN);
if (!rc) {
- /* All present CPUs must be online */
- cpumask_andnot(offline_mask, cpu_present_mask,
- cpu_online_mask);
- rc = rtas_online_cpus_mask(offline_mask);
- if (rc) {
- pr_err("%s: Could not bring present CPUs online.\n",
- __func__);
- goto out;
- }
-
stop_topology_update();
rc = pm_suspend(PM_SUSPEND_MEM);
start_topology_update();
-
- /* Take down CPUs not online prior to suspend */
- if (!rtas_offline_cpus_mask(offline_mask))
- pr_warn("%s: Could not restore CPUs to offline "
- "state.\n", __func__);
}
stream_id = 0;
if (!rc)
rc = count;
-out:
- free_cpumask_var(offline_mask);
+
return rc;
}
--
2.25.4
^ permalink raw reply related
* Re: [PATCH] powerpc/nvram: Replace kmalloc with kzalloc in the error message
From: Markus Elfring @ 2020-06-02 5:01 UTC (permalink / raw)
To: Michael Ellerman, Liao Pingfang, linuxppc-dev
Cc: Yi Wang, Tony Luck, Kees Cook, Wang Liang, Anton Vorontsov,
kernel-janitors, LKML, Colin Cross, Paul Mackerras, Xue Zhihong,
Greg Kroah-Hartman, Thomas Gleixner, Allison Randal
In-Reply-To: <87imgai394.fsf@mpe.ellerman.id.au>
>>> Please just remove the message instead, it's a tiny allocation that's
>>> unlikely to ever fail, and the caller will print an error anyway.
>>
>> How do you think about to take another look at a previous update suggestion
>> like the following?
>>
>> powerpc/nvram: Delete three error messages for a failed memory allocation
>> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/00845261-8528-d011-d3b8-e9355a231d3a@users.sourceforge.net/
>> https://lore.kernel.org/linuxppc-dev/00845261-8528-d011-d3b8-e9355a231d3a@users.sourceforge.net/
>> https://lore.kernel.org/patchwork/patch/752720/
>> https://lkml.org/lkml/2017/1/19/537
>
> That deleted the messages from nvram_scan_partitions(), but neither of
> the callers of nvram_scan_paritions() check its return value or print
> anything if it fails. So removing those messages would make those
> failures silent which is not what we want.
* How do you think about information like the following?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=f359287765c04711ff54fbd11645271d8e5ff763#n883
“…
These generic allocation functions all emit a stack dump on failure when used
without __GFP_NOWARN so there is no use in emitting an additional failure
message when NULL is returned.
…”
* Would you like to clarify software development concerns around
the Linux allocation failure report any more?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH] hw_breakpoint: Fix build warnings with clang
From: Christophe Leroy @ 2020-06-02 5:10 UTC (permalink / raw)
To: Ravi Bangoria, mpe
Cc: christophe.leroy, apopple, mikey, linux-kernel, npiggin, paulus,
naveen.n.rao, linuxppc-dev
In-Reply-To: <20200602041208.128913-1-ravi.bangoria@linux.ibm.com>
Le 02/06/2020 à 06:12, Ravi Bangoria a écrit :
> kbuild test robot reported few build warnings with hw_breakpoint code
> when compiled with clang[1]. Fix those.
>
> [1]: https://lore.kernel.org/linuxppc-dev/202005192233.oi9CjRtA%25lkp@intel.com/
>
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> Note: Prepared on top of powerpc/next.
>
> arch/powerpc/include/asm/hw_breakpoint.h | 3 ---
> include/linux/hw_breakpoint.h | 4 ++++
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
> index f42a55eb77d2..cb424799da0d 100644
> --- a/arch/powerpc/include/asm/hw_breakpoint.h
> +++ b/arch/powerpc/include/asm/hw_breakpoint.h
> @@ -70,9 +70,6 @@ extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
> unsigned long val, void *data);
> int arch_install_hw_breakpoint(struct perf_event *bp);
> void arch_uninstall_hw_breakpoint(struct perf_event *bp);
> -int arch_reserve_bp_slot(struct perf_event *bp);
> -void arch_release_bp_slot(struct perf_event *bp);
> -void arch_unregister_hw_breakpoint(struct perf_event *bp);
> void hw_breakpoint_pmu_read(struct perf_event *bp);
> extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
>
> diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
> index 6058c3844a76..521481f0d320 100644
> --- a/include/linux/hw_breakpoint.h
> +++ b/include/linux/hw_breakpoint.h
> @@ -80,6 +80,10 @@ extern int dbg_reserve_bp_slot(struct perf_event *bp);
> extern int dbg_release_bp_slot(struct perf_event *bp);
> extern int reserve_bp_slot(struct perf_event *bp);
> extern void release_bp_slot(struct perf_event *bp);
> +extern int hw_breakpoint_weight(struct perf_event *bp);
> +extern int arch_reserve_bp_slot(struct perf_event *bp);
> +extern void arch_release_bp_slot(struct perf_event *bp);
> +extern void arch_unregister_hw_breakpoint(struct perf_event *bp);
Please no new 'extern'. In the old days 'extern' keyword was used, but
new code shall not introduce new unnecessary usage of 'extern' keyword.
See report from Checkpatch below:
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description
(prefer a maximum 75 chars per line)
#9:
[1]:
https://lore.kernel.org/linuxppc-dev/202005192233.oi9CjRtA%25lkp@intel.com/
CHECK:AVOID_EXTERNS: extern prototypes should be avoided in .h files
#40: FILE: include/linux/hw_breakpoint.h:83:
+extern int hw_breakpoint_weight(struct perf_event *bp);
CHECK:AVOID_EXTERNS: extern prototypes should be avoided in .h files
#41: FILE: include/linux/hw_breakpoint.h:84:
+extern int arch_reserve_bp_slot(struct perf_event *bp);
CHECK:AVOID_EXTERNS: extern prototypes should be avoided in .h files
#42: FILE: include/linux/hw_breakpoint.h:85:
+extern void arch_release_bp_slot(struct perf_event *bp);
CHECK:AVOID_EXTERNS: extern prototypes should be avoided in .h files
#43: FILE: include/linux/hw_breakpoint.h:86:
+extern void arch_unregister_hw_breakpoint(struct perf_event *bp);
total: 0 errors, 1 warnings, 4 checks, 19 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or
--fix-inplace.
the.patch has style problems, please review.
NOTE: Ignored message types: ARCH_INCLUDE_LINUX BIT_MACRO
COMPARISON_TO_NULL DT_SPLIT_BINDING_PATCH EMAIL_SUBJECT
FILE_PATH_CHANGES GLOBAL_INITIALISERS LINE_SPACING MULTIPLE_ASSIGNMENTS
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
Christophe
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox