[PATCH] powerpc/fadump: update kernel logs before fadump crash begins
When we hit the fadump crash via the panic path the pstore update is
missing. This is observed when commit 8341f2f222d7 ("sysrq: Use panic()
to force a crash") changed the sysrq-trigger to take panic path instead
of die path.
The PPC panic event handler addresses the system panic in two different
ways based on the system configuration. It first allows the FADump (if
configured) to handle the kernel panic else forwards the call to platform
specific panic function. Now pstore update is missing only if FADump
handles the kernel panic, the platform-specific panic function do update
the pstore by calling panic_flush_kmsg_end function.
The simplest approach to handle this issue is to add pstore update in PPC
panic handler before FADump handles the panic. But this leads to multiple
pstore updates in case FADump is not configured and platform-specific
panic function serves the kernel panic.
Hence the function panic_flush_kmsg_end (used by the platform-specific
panic function to update the kernel logs) is split into two functions, one
will update the pstore (called in ppc panic event handler) and others will
flush the kmsg on the console (called in platform specific panic function).
Signed-off-by: Sourabh Jain
---
arch/powerpc/include/asm/bug.h | 2 ++
arch/powerpc/kernel/setup-common.c | 1 +
arch/powerpc/kernel/traps.c| 12 +++-
arch/powerpc/platforms/ps3/setup.c | 2 +-
arch/powerpc/platforms/pseries/setup.c | 2 +-
5 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 338f36cd9934..9268551a69bc 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -118,6 +118,8 @@ extern void _exception_pkey(struct pt_regs *, unsigned
long, int);
extern void die(const char *, struct pt_regs *, long);
extern bool die_will_crash(void);
extern void panic_flush_kmsg_start(void);
+extern void panic_flush_kmsg_dump(void);
+extern void panic_flush_kmsg_console(void);
extern void panic_flush_kmsg_end(void);
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/kernel/setup-common.c
b/arch/powerpc/kernel/setup-common.c
index 7f8c890360fe..2d546a9e8bb1 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -699,6 +699,7 @@ static int ppc_panic_event(struct notifier_block *this,
* want interrupts to be hard disabled.
*/
hard_irq_disable();
+ panic_flush_kmsg_dump();
/*
* If firmware-assisted dump has been registered then trigger
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 82a3438300fd..bb6bc19992b3 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -169,15 +169,25 @@ extern void panic_flush_kmsg_start(void)
bust_spinlocks(1);
}
-extern void panic_flush_kmsg_end(void)
+extern void panic_flush_kmsg_dump(void)
{
printk_safe_flush_on_panic();
kmsg_dump(KMSG_DUMP_PANIC);
+}
+
+extern void panic_flush_kmsg_console(void)
+{
bust_spinlocks(0);
debug_locks_off();
console_flush_on_panic(CONSOLE_FLUSH_PENDING);
}
+extern void panic_flush_kmsg_end(void)
+{
+ panic_flush_kmsg_dump();
+ panic_flush_kmsg_console();
+}
+
static unsigned long oops_begin(struct pt_regs *regs)
{
int cpu;
diff --git a/arch/powerpc/platforms/ps3/setup.c
b/arch/powerpc/platforms/ps3/setup.c
index b29368931c56..f96ba34284a1 100644
--- a/arch/powerpc/platforms/ps3/setup.c
+++ b/arch/powerpc/platforms/ps3/setup.c
@@ -101,7 +101,7 @@ static void ps3_panic(char *str)
printk(" System does not reboot automatically.\n");
printk(" Please press POWER button.\n");
printk("\n");
- panic_flush_kmsg_end();
+ panic_flush_kmsg_console();
while(1)
lv1_pause(1);
diff --git a/arch/powerpc/platforms/pseries/setup.c
b/arch/powerpc/platforms/pseries/setup.c
index 0c8421dd01ab..66ecb88c4b8e 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -788,7 +788,7 @@ static void __init pSeries_setup_arch(void)
static void pseries_panic(char *str)
{
- panic_flush_kmsg_end();
+ panic_flush_kmsg_console();
rtas_os_term(str);
}
--
2.25.4
[powerpc:merge] BUILD SUCCESS ec7b8eb9bc7a519047485c95f7292b48f5b73fe6
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge branch HEAD: ec7b8eb9bc7a519047485c95f7292b48f5b73fe6 module: break nested ARCH_HAS_STRICT_MODULE_RWX and STRICT_MODULE_RWX #ifdefs elapsed time: 747m configs tested: 134 configs skipped: 4 The following configs have been built successfully. More configs may be tested in the coming days. arm defconfig arm allyesconfig arm allmodconfig arm allnoconfig arm64allyesconfig arm64 defconfig arm64allmodconfig arm64 allnoconfig powerpc allyesconfig m68k m5475evb_defconfig nios2 10m50_defconfig mips malta_kvm_defconfig m68kmvme147_defconfig arm imote2_defconfig arm lpc18xx_defconfig sh kfr2r09-romimage_defconfig arm spitz_defconfig c6x alldefconfig arm orion5x_defconfig powerpc mpc5200_defconfig arcvdk_hs38_defconfig sh se7724_defconfig powerpc mpc512x_defconfig sh sh7785lcr_32bit_defconfig mips ip28_defconfig arm s5pv210_defconfig mips ip32_defconfig mips tb0287_defconfig ia64generic_defconfig arc nps_defconfig s390 alldefconfig h8300 defconfig i386 allnoconfig powerpc mgcoge_defconfig mips bmips_stb_defconfig arm alldefconfig powerpc pasemi_defconfig arcvdk_hs38_smp_defconfig s390 allnoconfig arm lpc32xx_defconfig riscv allnoconfig mips ath79_defconfig mips loongson1c_defconfig armmagician_defconfig arm pxa910_defconfig arc nsimosci_hs_defconfig alphaalldefconfig armmvebu_v5_defconfig mipsqi_lb60_defconfig i386 allyesconfig i386defconfig i386 debian-10.3 ia64 allmodconfig ia64defconfig ia64 allnoconfig ia64 allyesconfig m68k allmodconfig m68k allnoconfig m68k sun3_defconfig m68kdefconfig m68k allyesconfig nios2 defconfig nios2allyesconfig openriscdefconfig c6x allyesconfig c6x allnoconfig openrisc allyesconfig nds32 defconfig nds32 allnoconfig csky allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig xtensa defconfig h8300allmodconfig h8300allyesconfig arc defconfig arc allyesconfig sh allmodconfig shallnoconfig microblazeallnoconfig mips allyesconfig mips allnoconfig mips allmodconfig pariscallnoconfig parisc defconfig parisc allyesconfig parisc allmodconfig powerpc rhel-kconfig powerpc allmodconfig powerpc allnoconfig powerpc defconfig i386 randconfig-a001-20200605 i386 randconfig-a006-20200605 i386 randconfig-a002-20200605 i386 randconfig-a005-20200605 i386 randconfig-a004-20200605 i386 randconfig-a003-20200605 x86_64 randconfig-a002-20200605 x86_64
Re: [musl] Re: ppc64le and 32-bit LE userland compatibility
On Sat, Jun 6, 2020, at 02:12, Segher Boessenkool wrote: > On Fri, Jun 05, 2020 at 11:59:32PM +0200, Daniel Kolesa wrote: > > On Fri, Jun 5, 2020, at 19:27, Segher Boessenkool wrote: > > > > Third party precompiled stuff doesn't really need to concern us, since > > > > none really exists. > > > > > > ... Yet. And if you claim you support ELFv2, not mentioning the ways > > > your implementation deviates from it, users will be unhappy. > > > > Will they? > > Yes; not only *your* users, but also users of the "actual" ELFv2 (for > BE), if anything starts using that: the ABI is defined, and at least at > some point it worked, it is not unreasonable to think someone might > want to start using it). I guess you mean like, if third party software starts shipping builds using ELFv2 and people suddenly find these things don't work because $reasons. The ABI itself wouldn't cause issues here as at binary interface level we're still compatible, but problems may actually arise in case glibc has a modern HW targeting ELFv2 port with IEEE quad float at some point. (not to mention that would be 100% incompatible at binary level because symvers, so i want to resolve this until then either way) > > Just don't pretend X and Y are the same if they are not, people do not > like to be misled. Just be clear upfront what the differences are, and > everyone will be happy. Confusing people and wasting their time won't > make you very popular though ;-) > > > The system explicitly mentions that the minimum target in the binary > > packages is 970. Users can't be expecting features that the hardware we > > support doesn't support :) > > But you are not the only user of ELFv2. Sure, the point was more like "ELFv2 + 970 minimum", both of which are explicitly mentioned, cannot really mean anything else than what it already does. > > > Also, we're not the only ones that do this - there's musl of course, but > > there's also the BSDs. FreeBSD 13 uses ELFv2, and supports all the old > > hardware including processors without VMX, let alone VSX. OpenBSD is likely > > to take the same path. But I'm not opposed to making this explicit, if > > that's what it takes to make other people happy with it. > > Yeah, please do :-) > > > > > It's also still an upgrade over ELFv1 regardless (I mean, the same > > > > things apply there). > > > > > > Yeah, in mostly minor ways, but it all adds up for sure. > > > > It's made my life simpler on numerous occasions, > > Great to hear that! > > > and allowed us to bring in software that'd otherwise take significant > > patching (no software is complete until it has its own assembly > > implementation of coroutines or something like that :P), > > .. but does it have a mail client? Probably some ad-hoc, informally specified, bug-ridden implementations of half of Common Lisp as well... > > > > That depends on what you call the average case. Code that is control > > > and memory-bound will not benefit much from *anything* :-) > > > > Average case is, quite literally, an average case - i.e. the average of all > > the software packages shipped in a distro :) > > Ah, mostly boring stuff :-) > > > > Yeah, but it helps quite a bit if your system (shared) libraries get all > > > improvements they can as well. > > > > Well, glibc will still benefit automatically to a degree even if not built > > for a modern baseline, since it has runtime checks in place already; as for > > other things... well, at least for Void, I already mentioned before we're > > as much of a source distro as a binary one - people can easily rebuild > > things that bottleneck them, with modern CFLAGS, and still have things be > > interoperable. > > Yeah, good point there. > > > > I'm not trying to dissuade you from not requiring VSX and 2.07 -- this > > > sounds like your best option, given the constraints. I'm just saying > > > the cost is not trivial (even ignoring the ABI divergence). > > > > Of course the cost is there - it's just not something I can do anything > > about. I generally recommend that people who can run LE should run LE. > > We're a bi-endian distribution, so there is a complete, fully functional, > > ISA-2.07-baseline ppc64le variant (in fact, it's our best-supported port, > > with greatest repo coverage and testing), as well as the 970-targeting > > ppc64 variant. > > Ah, so your BE target is mostly for legacy hardware? That took a while > to sink in, sorry! :-) That's what the average user on BE is expected to be. I'm not opinionated enough to force BE on an average modern hardware user, there's just too many drawbacks nowadays, some of them inherent, like limited graphics hardware support (the choice of HW is limited, modern GPUs don't work at all, older GPUs are limited to OpenGL 3.2 even if they support newer API, and even if newer GPUs *were* working, they'd be crippled performance-wise as modern GPUs are always LE these days - as a gamedev person I need
Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.8-1 tag
Linus Torvalds writes: > On Fri, Jun 5, 2020 at 12:01 PM Linus Torvalds > wrote: >> >> ..and then when I actually compared whether I otherwise got the same >> result as you, I realized that this all depends on the module tree. >> >> I'll go merge that first, and then re-do this all. Oh well. > > Ok, redone and pushed out. Can you check that the end result makes sense? Yep, looks good to me. Has passed all my basic build & boot tests. cheers
Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.8-1 tag
Linus Torvalds writes:
> On Fri, Jun 5, 2020 at 9:38 AM Michael Ellerman wrote:
>>
>> I've pushed the result of my resolution of the conflicts to the powerpc/merge
>> branch, if you want to look at that, though I've also tried to describe it in
>> full below.
>
> I ended up doing the machine_check_exception() differently, because I
> felt the code itself was done wrong and I wanted to add a note about
> that.
>
> Having the same function have completely different semantics depending
> on a platform issue is just fundamentally wrong, and makes not just
> for fragile code, but also means that you can't do single image
> kernels.
Yeah I agree it's not that nicely structured.
In this case CONFIG_PPC_BOOK3S_64 is one of our top-level compile time
switches anyway, so the single image thing at least isn't a concern.
> It should be two different functions, possibly just
>
>non_nmi_fn() { ... }
>
>nmi_fn() { nmi_enter(); non_nmi_fn(); nmi_exit(); }
>
> and now you don't have odd rules for the same function that depends on
> how the platform happens to call it.
Agreed.
> I didn't do the above. I did something that looked like the old code,
> but had a comment. Oh well.
Thanks, we can restructure it later.
> But thanks for describing the merge, I'd have missed the place where
> there was a new use of pgd_oiffset().
Yeah I missed it to begin with :}
> ..and then when I actually compared whether I otherwise got the same
> result as you, I realized that this all depends on the module tree.
>
> I'll go merge that first, and then re-do this all. Oh well.
Darn it. I figured you'd have merged that by the time you saw my pull,
will mention anything similar in future.
cheers
Re: [musl] Re: ppc64le and 32-bit LE userland compatibility
On Fri, Jun 05, 2020 at 11:59:32PM +0200, Daniel Kolesa wrote: > On Fri, Jun 5, 2020, at 19:27, Segher Boessenkool wrote: > > > Third party precompiled stuff doesn't really need to concern us, since > > > none really exists. > > > > ... Yet. And if you claim you support ELFv2, not mentioning the ways > > your implementation deviates from it, users will be unhappy. > > Will they? Yes; not only *your* users, but also users of the "actual" ELFv2 (for BE), if anything starts using that: the ABI is defined, and at least at some point it worked, it is not unreasonable to think someone might want to start using it). Just don't pretend X and Y are the same if they are not, people do not like to be misled. Just be clear upfront what the differences are, and everyone will be happy. Confusing people and wasting their time won't make you very popular though ;-) > The system explicitly mentions that the minimum target in the binary packages > is 970. Users can't be expecting features that the hardware we support > doesn't support :) But you are not the only user of ELFv2. > Also, we're not the only ones that do this - there's musl of course, but > there's also the BSDs. FreeBSD 13 uses ELFv2, and supports all the old > hardware including processors without VMX, let alone VSX. OpenBSD is likely > to take the same path. But I'm not opposed to making this explicit, if that's > what it takes to make other people happy with it. Yeah, please do :-) > > > It's also still an upgrade over ELFv1 regardless (I mean, the same things > > > apply there). > > > > Yeah, in mostly minor ways, but it all adds up for sure. > > It's made my life simpler on numerous occasions, Great to hear that! > and allowed us to bring in software that'd otherwise take significant > patching (no software is complete until it has its own assembly > implementation of coroutines or something like that :P), .. but does it have a mail client? > > That depends on what you call the average case. Code that is control > > and memory-bound will not benefit much from *anything* :-) > > Average case is, quite literally, an average case - i.e. the average of all > the software packages shipped in a distro :) Ah, mostly boring stuff :-) > > Yeah, but it helps quite a bit if your system (shared) libraries get all > > improvements they can as well. > > Well, glibc will still benefit automatically to a degree even if not built > for a modern baseline, since it has runtime checks in place already; as for > other things... well, at least for Void, I already mentioned before we're as > much of a source distro as a binary one - people can easily rebuild things > that bottleneck them, with modern CFLAGS, and still have things be > interoperable. Yeah, good point there. > > I'm not trying to dissuade you from not requiring VSX and 2.07 -- this > > sounds like your best option, given the constraints. I'm just saying > > the cost is not trivial (even ignoring the ABI divergence). > > Of course the cost is there - it's just not something I can do anything > about. I generally recommend that people who can run LE should run LE. We're > a bi-endian distribution, so there is a complete, fully functional, > ISA-2.07-baseline ppc64le variant (in fact, it's our best-supported port, > with greatest repo coverage and testing), as well as the 970-targeting ppc64 > variant. Ah, so your BE target is mostly for legacy hardware? That took a while to sink in, sorry! :-) > I know about the biarch case as well, and there is also multilib, as an even > more elaborate form of that. That's not directly related to what I originally > said, though Biarch is why -m32 and -m64 can work at all (for building user binaries). My point is that this does *not* work for most finer ABI (or OS) -related points -- you really do need a toolchain built specifically for that config. Segher
Re: ppc64le and 32-bit LE userland compatibility
On Saturday, May 30, 2020 3:17:24 PM PDT Will Springer wrote: > On Saturday, May 30, 2020 8:37:43 AM PDT Christophe Leroy wrote: > > There is a series at > > https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=173231 > > to switch powerpc to the Generic C VDSO. > > > > Can you try and see whether it fixes your issue ? > > > > Christophe > > Sure thing, I spotted that after making the initial post. Will report > back with results. > > Will [she/her] Sorry for the wait, I just sat down to work on this again yesterday. Tested this series on top of stable/linux-5.7.y (5.7.0 at the time of writing), plus the one-line signal handler patch. Had to rewind to the state of powerpc/merge at the time of the mail before the patch would apply, then cherry-picked to 5.6 until I realized the patchset used some functionality that didn't land until 5.7, so I moved it there. Good news is that `date` now works correctly with the vdso call in 32-bit LE. Bad news is it seems to have broken things on the 64-bit side—in my testing, Void kicks off runit but hangs after starting eudev, and in a Debian Stretch system, systemd doesn't get to the point of printing anything whatsoever. (I had to `init=/bin/sh` to confirm the date worked in ppcle, although in ppc64le running `date` also hung the system when it made the vdso call...) Not sure how to approach debugging that, so I'd appreciate any pointers. Will [she/her]
Re: [musl] Re: ppc64le and 32-bit LE userland compatibility
Hi! On Fri, Jun 05, 2020 at 01:50:46PM -0400, Rich Felker wrote: > On Fri, Jun 05, 2020 at 12:27:02PM -0500, Segher Boessenkool wrote: > > > I'm also not really all that convinced that vectors make a huge > > > difference in non-specialized code (autovectorization still has a way to > > > go) > > > > They do make a huge difference, depending on the application of course. > > But VSX is not just vectors even: it also gives you twice as many > > floating point scalars (64 now), and in newer versions of the ISA it can > > be beneficially used for integer scalars even. > > Vectorization is useful for a lot of things, and I'm sure there are > specialized workloads that benefit from 64 scalars, but I've never > encountered a place where having more than 16 registers made a > practical difference. 20 years ago 32 FP registers was already often a limitation, making FFT and similar kernels almost twice slower than they could otherwise be. Things are only *worse* with short vectors, not better. In general with floating point data you need more registers (because you have more state to look at concurrently) than with integer data. *Of course* having 64 floating point registers does not matter if your whole program only ever uses three floating point values, total, let alone concurrently. > The fact that there are specialized areas where this stuff matters > does not imply there aren't huge domains where it's completely > irrelevant. There are very few domains where ISA 2.07 does not have significant advantages over ISA 2.01. That is Power8 vs. Power4. > > No, that is exactly the point of requiring ISA 2.07. Anything can use > > ISA 2.07 (incl. VSX) without checking first, and without having a > > fallback to some other implementation. Going from ISA 2.01 to 2.07 is > > more than a decade of improvements, it is not trivial at all. > > This only affects code that's non-portable and PPC-specific, which a No, it does not. It is not only about vector registers, either. > I think a lot of the unnecessary fighting on this topic is arising > from differences of opinion over what an ABI entails. I would call > what you're talking about a "platform" and more of a platform-specific > *API* than an ABI -- it's about guarantees of interfaces available to > the programmer, not implementation details of linkage. No, this is very much about the ABI. The B stands for Binary. Which is what this is about. Segher
Re: [musl] Re: ppc64le and 32-bit LE userland compatibility
On Fri, Jun 5, 2020, at 19:27, Segher Boessenkool wrote: > On Fri, Jun 05, 2020 at 04:18:18AM +0200, Daniel Kolesa wrote: > > On Fri, Jun 5, 2020, at 01:35, Segher Boessenkool wrote: > > > > The thing is, I've yet to see in which way the ELFv2 ABI *actually* > > > > requires VSX - I don't think compiling for 970 introduces any actual > > > > differences. There will be omissions, yes - but then the more accurate > > > > thing would be to say that a subset of ELFv2 is used, rather than it > > > > being a different ABI per se. > > > > > > Two big things are that binaries that someone else made are supposed to > > > work for you as well -- including binaries using VSX registers, or any > > > instructions that require ISA 2.07 (or some older ISA after 970). This > > > includes DSOs (shared libraries). So for a distribution this means that > > > they will not use VSX *anywhere*, or only in very specialised things. > > > That is a many-years setback, for people/situations where it could be > > > used. > > > > Third party precompiled stuff doesn't really need to concern us, since none > > really exists. > > ... Yet. And if you claim you support ELFv2, not mentioning the ways > your implementation deviates from it, users will be unhappy. Will they? The system explicitly mentions that the minimum target in the binary packages is 970. Users can't be expecting features that the hardware we support doesn't support :) Also, we're not the only ones that do this - there's musl of course, but there's also the BSDs. FreeBSD 13 uses ELFv2, and supports all the old hardware including processors without VMX, let alone VSX. OpenBSD is likely to take the same path. But I'm not opposed to making this explicit, if that's what it takes to make other people happy with it. > > > It's also still an upgrade over ELFv1 regardless (I mean, the same things > > apply there). > > Yeah, in mostly minor ways, but it all adds up for sure. It's made my life simpler on numerous occasions, and allowed us to bring in software that'd otherwise take significant patching (no software is complete until it has its own assembly implementation of coroutines or something like that :P), or would be outright impossible without a lot of work (e.g. the lld linker). > > > I'm also not really all that convinced that vectors make a huge difference > > in non-specialized code (autovectorization still has a way to go) > > They do make a huge difference, depending on the application of course. I've yet to see non-specialized things without explicit VSX impls where the difference is more than 5%, usually it's less. > But VSX is not just vectors even: it also gives you twice as many > floating point scalars (64 now), and in newer versions of the ISA it can > be beneficially used for integer scalars even. > > > and code written to use vector instructions should probably check auxval > > and take those paths at runtime. > > No, that is exactly the point of requiring ISA 2.07. Anything can use > ISA 2.07 (incl. VSX) without checking first, and without having a > fallback to some other implementation. Going from ISA 2.01 to 2.07 is > more than a decade of improvements, it is not trivial at all. Portable code will need runtime checks anyway if they target big endian systems, especially existing ones, since modern vector instructions can be used even in legacy ABI, where you get no such guarantee. Sure, it's nice to not have to worry about it, but once everything is considered, going with a more modern ABI is still a big net gain for us, even if the guarantees are not all there :) > > > > As for other instructions, fair enough, but from my rough testing, it > > doesn't make such a massive difference for average case > > That depends on what you call the average case. Code that is control > and memory-bound will not benefit much from *anything* :-) Average case is, quite literally, an average case - i.e. the average of all the software packages shipped in a distro :) > > > (and where it does, one can always rebuild their thing with > > CFLAGS=-mcpu=power9) > > Yeah, but it helps quite a bit if your system (shared) libraries get all > improvements they can as well. Well, glibc will still benefit automatically to a degree even if not built for a modern baseline, since it has runtime checks in place already; as for other things... well, at least for Void, I already mentioned before we're as much of a source distro as a binary one - people can easily rebuild things that bottleneck them, with modern CFLAGS, and still have things be interoperable. > > > I'm not trying to dissuade you from not requiring VSX and 2.07 -- this > sounds like your best option, given the constraints. I'm just saying > the cost is not trivial (even ignoring the ABI divergence). Of course the cost is there - it's just not something I can do anything about. I generally recommend that people who can run LE should run LE. We're a bi-endian
Re: [PATCH v10 5/6] ndctl/papr_scm, uapi: Add support for PAPR nvdimm specific methods
On Fri, Jun 5, 2020 at 12:50 PM Ira Weiny wrote: > > On Fri, Jun 05, 2020 at 05:11:35AM +0530, Vaibhav Jain wrote: > > Introduce support for PAPR NVDIMM Specific Methods (PDSM) in papr_scm > > module and add the command family NVDIMM_FAMILY_PAPR to the white list > > of NVDIMM command sets. Also advertise support for ND_CMD_CALL for the > > nvdimm command mask and implement necessary scaffolding in the module > > to handle ND_CMD_CALL ioctl and PDSM requests that we receive. > > > > The layout of the PDSM request as we expect from libnvdimm/libndctl is > > described in newly introduced uapi header 'papr_pdsm.h' which > > defines a new 'struct nd_pdsm_cmd_pkg' header. This header is used > > to communicate the PDSM request via member > > 'nd_cmd_pkg.nd_command' and size of payload that need to be > > sent/received for servicing the PDSM. > > > > A new function is_cmd_valid() is implemented that reads the args to > > papr_scm_ndctl() and performs sanity tests on them. A new function > > papr_scm_service_pdsm() is introduced and is called from > > papr_scm_ndctl() in case of a PDSM request is received via ND_CMD_CALL > > command from libnvdimm. > > > > Cc: "Aneesh Kumar K . V" > > Cc: Dan Williams > > Cc: Michael Ellerman > > Cc: Ira Weiny > > Signed-off-by: Vaibhav Jain > > --- > > Changelog: > > > > v9..v10: > > * Simplified 'struct nd_pdsm_cmd_pkg' by removing the > > 'payload_version' field. > > * Removed the corrosponding documentation on versioning and backward > > compatibility from 'papr_pdsm.h' > > * Reduced the size of reserved fields to 4-bytes making 'struct > > nd_pdsm_cmd_pkg' 64 + 8 bytes long. > > * Updated is_cmd_valid() to enforce validation checks on pdsm > > commands. [ Dan Williams ] > > * Added check for reserved fields being set to '0' in is_cmd_valid() > > [ Ira ] > > * Moved changes for checking cmd_rc == NULL and logging improvements > > to a separate prelim patch [ Ira ]. > > * Moved pdsm package validation checks from papr_scm_service_pdsm() > > to is_cmd_valid(). > > * Marked papr_scm_service_pdsm() return type as 'void' since errors > > are reported in nd_pdsm_cmd_pkg.cmd_status field. > > > > Resend: > > * Added ack from Aneesh. > > > > v8..v9: > > * Reduced the usage of term SCM replacing it with appropriate > > replacement [ Dan Williams, Aneesh ] > > * Renamed 'papr_scm_pdsm.h' to 'papr_pdsm.h' > > * s/PAPR_SCM_PDSM_*/PAPR_PDSM_*/g > > * s/NVDIMM_FAMILY_PAPR_SCM/NVDIMM_FAMILY_PAPR/g > > * Minor updates to 'papr_psdm.h' to replace usage of term 'SCM'. > > * Minor update to patch description. > > > > v7..v8: > > * Removed the 'payload_offset' field from 'struct > > nd_pdsm_cmd_pkg'. Instead command payload is always assumed to start > > at 'nd_pdsm_cmd_pkg.payload'. [ Aneesh ] > > * To enable introducing new fields to 'struct nd_pdsm_cmd_pkg', > > 'reserved' field of 10-bytes is introduced. [ Aneesh ] > > * Fixed a typo in "Backward Compatibility" section of papr_scm_pdsm.h > > [ Ira ] > > > > Resend: > > * None > > > > v6..v7 : > > * Removed the re-definitions of __packed macro from papr_scm_pdsm.h > > [Mpe]. > > * Removed the usage of __KERNEL__ macros in papr_scm_pdsm.h [Mpe]. > > * Removed macros that were unused in papr_scm.c from papr_scm_pdsm.h > > [Mpe]. > > * Made functions defined in papr_scm_pdsm.h as static inline. [Mpe] > > > > v5..v6 : > > * Changed the usage of the term DSM to PDSM to distinguish it from the > > ACPI term [ Dan Williams ] > > * Renamed papr_scm_dsm.h to papr_scm_pdsm.h and updated various struct > > to reflect the new terminology. > > * Updated the patch description and title to reflect the new terminology. > > * Squashed patch to introduce new command family in 'ndctl.h' with > > this patch [ Dan Williams ] > > * Updated the papr_scm_pdsm method starting index from 0x1 to 0x0 > > [ Dan Williams ] > > * Removed redundant license text from the papr_scm_psdm.h file. > > [ Dan Williams ] > > * s/envelop/envelope/ at various places [ Dan Williams ] > > * Added '__packed' attribute to command package header to gaurd > > against different compiler adding paddings between the fields. > > [ Dan Williams] > > * Converted various pr_debug to dev_debug [ Dan Williams ] > > > > v4..v5 : > > * None > > > > v3..v4 : > > * None > > > > v2..v3 : > > * Updated the patch prefix to 'ndctl/uapi' [Aneesh] > > > > v1..v2 : > > * None > > --- > > arch/powerpc/include/uapi/asm/papr_pdsm.h | 98 +++ > > arch/powerpc/platforms/pseries/papr_scm.c | 113 +- > > include/uapi/linux/ndctl.h| 1 + > > 3 files changed, 207 insertions(+), 5 deletions(-) > > create mode 100644 arch/powerpc/include/uapi/asm/papr_pdsm.h > > > > diff --git a/arch/powerpc/include/uapi/asm/papr_pdsm.h > > b/arch/powerpc/include/uapi/asm/papr_pdsm.h > > new file mode 100644 > > index ..8b1a4f8fa316 > > --- /dev/null > > +++ b/arch/powerpc/include/uapi/asm/papr_pdsm.h > > @@ -0,0
Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.8-1 tag
On Fri, Jun 5, 2020 at 12:01 PM Linus Torvalds wrote: > > ..and then when I actually compared whether I otherwise got the same > result as you, I realized that this all depends on the module tree. > > I'll go merge that first, and then re-do this all. Oh well. Ok, redone and pushed out. Can you check that the end result makes sense? Linus
Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.8-1 tag
The pull request you sent on Sat, 06 Jun 2020 02:38:49 +1000: > https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git > tags/powerpc-5.8-1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/7ae77150d94d3b535c7b85e6b3647113095e79bf Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [PATCH v10 5/6] ndctl/papr_scm, uapi: Add support for PAPR nvdimm specific methods
On Fri, Jun 05, 2020 at 05:11:35AM +0530, Vaibhav Jain wrote: > Introduce support for PAPR NVDIMM Specific Methods (PDSM) in papr_scm > module and add the command family NVDIMM_FAMILY_PAPR to the white list > of NVDIMM command sets. Also advertise support for ND_CMD_CALL for the > nvdimm command mask and implement necessary scaffolding in the module > to handle ND_CMD_CALL ioctl and PDSM requests that we receive. > > The layout of the PDSM request as we expect from libnvdimm/libndctl is > described in newly introduced uapi header 'papr_pdsm.h' which > defines a new 'struct nd_pdsm_cmd_pkg' header. This header is used > to communicate the PDSM request via member > 'nd_cmd_pkg.nd_command' and size of payload that need to be > sent/received for servicing the PDSM. > > A new function is_cmd_valid() is implemented that reads the args to > papr_scm_ndctl() and performs sanity tests on them. A new function > papr_scm_service_pdsm() is introduced and is called from > papr_scm_ndctl() in case of a PDSM request is received via ND_CMD_CALL > command from libnvdimm. > > Cc: "Aneesh Kumar K . V" > Cc: Dan Williams > Cc: Michael Ellerman > Cc: Ira Weiny > Signed-off-by: Vaibhav Jain > --- > Changelog: > > v9..v10: > * Simplified 'struct nd_pdsm_cmd_pkg' by removing the > 'payload_version' field. > * Removed the corrosponding documentation on versioning and backward > compatibility from 'papr_pdsm.h' > * Reduced the size of reserved fields to 4-bytes making 'struct > nd_pdsm_cmd_pkg' 64 + 8 bytes long. > * Updated is_cmd_valid() to enforce validation checks on pdsm > commands. [ Dan Williams ] > * Added check for reserved fields being set to '0' in is_cmd_valid() > [ Ira ] > * Moved changes for checking cmd_rc == NULL and logging improvements > to a separate prelim patch [ Ira ]. > * Moved pdsm package validation checks from papr_scm_service_pdsm() > to is_cmd_valid(). > * Marked papr_scm_service_pdsm() return type as 'void' since errors > are reported in nd_pdsm_cmd_pkg.cmd_status field. > > Resend: > * Added ack from Aneesh. > > v8..v9: > * Reduced the usage of term SCM replacing it with appropriate > replacement [ Dan Williams, Aneesh ] > * Renamed 'papr_scm_pdsm.h' to 'papr_pdsm.h' > * s/PAPR_SCM_PDSM_*/PAPR_PDSM_*/g > * s/NVDIMM_FAMILY_PAPR_SCM/NVDIMM_FAMILY_PAPR/g > * Minor updates to 'papr_psdm.h' to replace usage of term 'SCM'. > * Minor update to patch description. > > v7..v8: > * Removed the 'payload_offset' field from 'struct > nd_pdsm_cmd_pkg'. Instead command payload is always assumed to start > at 'nd_pdsm_cmd_pkg.payload'. [ Aneesh ] > * To enable introducing new fields to 'struct nd_pdsm_cmd_pkg', > 'reserved' field of 10-bytes is introduced. [ Aneesh ] > * Fixed a typo in "Backward Compatibility" section of papr_scm_pdsm.h > [ Ira ] > > Resend: > * None > > v6..v7 : > * Removed the re-definitions of __packed macro from papr_scm_pdsm.h > [Mpe]. > * Removed the usage of __KERNEL__ macros in papr_scm_pdsm.h [Mpe]. > * Removed macros that were unused in papr_scm.c from papr_scm_pdsm.h > [Mpe]. > * Made functions defined in papr_scm_pdsm.h as static inline. [Mpe] > > v5..v6 : > * Changed the usage of the term DSM to PDSM to distinguish it from the > ACPI term [ Dan Williams ] > * Renamed papr_scm_dsm.h to papr_scm_pdsm.h and updated various struct > to reflect the new terminology. > * Updated the patch description and title to reflect the new terminology. > * Squashed patch to introduce new command family in 'ndctl.h' with > this patch [ Dan Williams ] > * Updated the papr_scm_pdsm method starting index from 0x1 to 0x0 > [ Dan Williams ] > * Removed redundant license text from the papr_scm_psdm.h file. > [ Dan Williams ] > * s/envelop/envelope/ at various places [ Dan Williams ] > * Added '__packed' attribute to command package header to gaurd > against different compiler adding paddings between the fields. > [ Dan Williams] > * Converted various pr_debug to dev_debug [ Dan Williams ] > > v4..v5 : > * None > > v3..v4 : > * None > > v2..v3 : > * Updated the patch prefix to 'ndctl/uapi' [Aneesh] > > v1..v2 : > * None > --- > arch/powerpc/include/uapi/asm/papr_pdsm.h | 98 +++ > arch/powerpc/platforms/pseries/papr_scm.c | 113 +- > include/uapi/linux/ndctl.h| 1 + > 3 files changed, 207 insertions(+), 5 deletions(-) > create mode 100644 arch/powerpc/include/uapi/asm/papr_pdsm.h > > diff --git a/arch/powerpc/include/uapi/asm/papr_pdsm.h > b/arch/powerpc/include/uapi/asm/papr_pdsm.h > new file mode 100644 > index ..8b1a4f8fa316 > --- /dev/null > +++ b/arch/powerpc/include/uapi/asm/papr_pdsm.h > @@ -0,0 +1,98 @@ > +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */ > +/* > + * PAPR nvDimm Specific Methods (PDSM) and structs for libndctl > + * > + * (C) Copyright IBM 2020 > + * > + * Author: Vaibhav Jain > + */ > + > +#ifndef _UAPI_ASM_POWERPC_PAPR_PDSM_H_ >
Re: [PATCH v10 4/6] powerpc/papr_scm: Improve error logging and handling papr_scm_ndctl()
On Fri, Jun 5, 2020 at 10:13 AM Ira Weiny wrote:
>
> On Fri, Jun 05, 2020 at 05:11:34AM +0530, Vaibhav Jain wrote:
> > Since papr_scm_ndctl() can be called from outside papr_scm, its
> > exposed to the possibility of receiving NULL as value of 'cmd_rc'
> > argument. This patch updates papr_scm_ndctl() to protect against such
> > possibility by assigning it pointer to a local variable in case cmd_rc
> > == NULL.
> >
> > Finally the patch also updates the 'default' clause of the switch-case
> > block removing a 'return' statement thereby ensuring that value of
> > 'cmd_rc' is always logged when papr_scm_ndctl() returns.
> >
> > Cc: "Aneesh Kumar K . V"
> > Cc: Dan Williams
> > Cc: Michael Ellerman
> > Cc: Ira Weiny
> > Signed-off-by: Vaibhav Jain
> > ---
> > Changelog:
> >
> > v9..v10
> > * New patch in the series
>
> Thanks for making this a separate patch it is easier to see what is going on
> here.
>
> > ---
> > arch/powerpc/platforms/pseries/papr_scm.c | 10 --
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/papr_scm.c
> > b/arch/powerpc/platforms/pseries/papr_scm.c
> > index 0c091622b15e..6512fe6a2874 100644
> > --- a/arch/powerpc/platforms/pseries/papr_scm.c
> > +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> > @@ -355,11 +355,16 @@ static int papr_scm_ndctl(struct
> > nvdimm_bus_descriptor *nd_desc,
> > {
> > struct nd_cmd_get_config_size *get_size_hdr;
> > struct papr_scm_priv *p;
> > + int rc;
> >
> > /* Only dimm-specific calls are supported atm */
> > if (!nvdimm)
> > return -EINVAL;
> >
> > + /* Use a local variable in case cmd_rc pointer is NULL */
> > + if (!cmd_rc)
> > + cmd_rc =
> > +
>
> This protects you from the NULL. However...
>
> > p = nvdimm_provider_data(nvdimm);
> >
> > switch (cmd) {
> > @@ -381,12 +386,13 @@ static int papr_scm_ndctl(struct
> > nvdimm_bus_descriptor *nd_desc,
> > break;
> >
> > default:
> > - return -EINVAL;
> > + dev_dbg(>pdev->dev, "Unknown command = %d\n", cmd);
> > + *cmd_rc = -EINVAL;
>
> ... I think you are conflating rc and cmd_rc...
>
> > }
> >
> > dev_dbg(>pdev->dev, "returned with cmd_rc = %d\n", *cmd_rc);
> >
> > - return 0;
> > + return *cmd_rc;
>
> ... this changes the behavior of the current commands. Now if the underlying
> papr_scm_meta_[get|set]() fails you return that failure as rc rather than 0.
>
> Is that ok?
The expectation is that rc is "did the command get sent to the device,
or did it fail for 'transport' reasons". The role of cmd_rc is to
translate the specific status response of the command into a common
error code. The expectations are:
rc < 0: Error code, Linux terminated the ioctl before talking to hardware
rc == 0: Linux successfully submitted the command to hardware, cmd_rc
is valid for command specific response
rc > 0: Linux successfully submitted the command, but detected that
only a subset of the data was accepted for "write"-style commands, or
that only subset of data was returned for "read"-style commands. I.e.
short-write / short-read semantics. cmd_rc is valid in this case and
its up to userspace to determine if a short transfer is an error or
not.
> Also 'logging cmd_rc' in the invalid cmd case does not seem quite right unless
> you really want rc to be cmd_rc.
>
> The architecture is designed to separate errors which occur in the kernel vs
> errors in the firmware/dimm. Are they always the same? The current code
> differentiates them.
Yeah, they're distinct, transport vs end-point / command-specific
status returns.
Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.8-1 tag
On Fri, Jun 5, 2020 at 9:38 AM Michael Ellerman wrote:
>
> I've pushed the result of my resolution of the conflicts to the powerpc/merge
> branch, if you want to look at that, though I've also tried to describe it in
> full below.
I ended up doing the machine_check_exception() differently, because I
felt the code itself was done wrong and I wanted to add a note about
that.
Having the same function have completely different semantics depending
on a platform issue is just fundamentally wrong, and makes not just
for fragile code, but also means that you can't do single image
kernels.
It should be two different functions, possibly just
non_nmi_fn() { ... }
nmi_fn() { nmi_enter(); non_nmi_fn(); nmi_exit(); }
and now you don't have odd rules for the same function that depends on
how the platform happens to call it.
I didn't do the above. I did something that looked like the old code,
but had a comment. Oh well.
But thanks for describing the merge, I'd have missed the place where
there was a new use of pgd_oiffset().
..and then when I actually compared whether I otherwise got the same
result as you, I realized that this all depends on the module tree.
I'll go merge that first, and then re-do this all. Oh well.
Linus
Re: [PATCH v10 6/6] powerpc/papr_scm: Implement support for PAPR_PDSM_HEALTH
On Fri, Jun 05, 2020 at 05:11:36AM +0530, Vaibhav Jain wrote:
> This patch implements support for PDSM request 'PAPR_PDSM_HEALTH'
> that returns a newly introduced 'struct nd_papr_pdsm_health' instance
> containing dimm health information back to user space in response to
> ND_CMD_CALL. This functionality is implemented in newly introduced
> papr_pdsm_health() that queries the nvdimm health information and
> then copies this information to the package payload whose layout is
> defined by 'struct nd_papr_pdsm_health'.
>
> Cc: "Aneesh Kumar K . V"
> Cc: Dan Williams
> Cc: Michael Ellerman
> Cc: Ira Weiny
> Signed-off-by: Vaibhav Jain
> ---
> Changelog:
>
> v9..v10:
> * Removed code in papr_pdsm_health that performed validation on pdsm
> payload version and corrosponding struct and defines used for
> validation of payload version.
> * Dropped usage of struct papr_pdsm_health in 'struct
> papr_scm_priv'. Instead papr_psdm_health() now uses
> 'papr_scm_priv.health_bitmap' to populate the pdsm payload.
> * Above change also fixes the problem where this patch was removing
> the code that was previously introduced in this patch-series.
> [ Ira ]
> * Introduced a new def ND_PDSM_ENVELOPE_HDR_SIZE that indicates the
> space allocated to 'struct nd_pdsm_cmd_pkg' fields except 'struct
> nd_cmd_pkg'. This def is useful in validating payload sizes.
> * Reworked papr_pdsm_health() to enforce a specific payload size for
> 'PAPR_PDSM_HEALTH' pdsm request.
>
> Resend:
> * Added ack from Aneesh.
>
> v8..v9:
> * s/PAPR_SCM_PDSM_HEALTH/PAPR_PDSM_HEALTH/g [ Dan , Aneesh ]
> * s/PAPR_SCM_PSDM_DIMM_*/PAPR_PDSM_DIMM_*/g
> * Renamed papr_scm_get_health() to papr_psdm_health()
> * Updated patch description to replace papr-scm dimm with nvdimm.
>
> v7..v8:
> * None
>
> Resend:
> * None
>
> v6..v7:
> * Updated flags_show() to use seq_buf_printf(). [Mpe]
> * Updated papr_scm_get_health() to use newly introduced
> __drc_pmem_query_health() bypassing the cache [Mpe].
>
> v5..v6:
> * Added attribute '__packed' to 'struct nd_papr_pdsm_health_v1' to
> gaurd against possibility of different compilers adding different
> paddings to the struct [ Dan Williams ]
>
> * Updated 'struct nd_papr_pdsm_health_v1' to use __u8 instead of
> 'bool' and also updated drc_pmem_query_health() to take this into
> account. [ Dan Williams ]
>
> v4..v5:
> * None
>
> v3..v4:
> * Call the DSM_PAPR_SCM_HEALTH service function from
> papr_scm_service_dsm() instead of papr_scm_ndctl(). [Aneesh]
>
> v2..v3:
> * Updated struct nd_papr_scm_dimm_health_stat_v1 to use '__xx' types
> as its exported to the userspace [Aneesh]
> * Changed the constants DSM_PAPR_SCM_DIMM_XX indicating dimm health
> from enum to #defines [Aneesh]
>
> v1..v2:
> * New patch in the series
> ---
> arch/powerpc/include/uapi/asm/papr_pdsm.h | 33 +++
> arch/powerpc/platforms/pseries/papr_scm.c | 70 +++
> 2 files changed, 103 insertions(+)
>
> diff --git a/arch/powerpc/include/uapi/asm/papr_pdsm.h
> b/arch/powerpc/include/uapi/asm/papr_pdsm.h
> index 8b1a4f8fa316..c4c990ede5d4 100644
> --- a/arch/powerpc/include/uapi/asm/papr_pdsm.h
> +++ b/arch/powerpc/include/uapi/asm/papr_pdsm.h
> @@ -71,12 +71,17 @@ struct nd_pdsm_cmd_pkg {
> __u8 payload[]; /* In/Out: Sub-cmd data buffer */
> } __packed;
>
> +/* Calculate size used by the pdsm header fields minus 'struct nd_cmd_pkg' */
> +#define ND_PDSM_ENVELOPE_HDR_SIZE \
> + (sizeof(struct nd_pdsm_cmd_pkg) - sizeof(struct nd_cmd_pkg))
> +
This is kind of a weird name for this.
Isn't this just the ND PDSM header size? What is 'envelope' mean here?
> /*
> * Methods to be embedded in ND_CMD_CALL request. These are sent to the
> kernel
> * via 'nd_pdsm_cmd_pkg.hdr.nd_command' member of the ioctl struct
> */
> enum papr_pdsm {
> PAPR_PDSM_MIN = 0x0,
> + PAPR_PDSM_HEALTH,
> PAPR_PDSM_MAX,
> };
>
> @@ -95,4 +100,32 @@ static inline void *pdsm_cmd_to_payload(struct
> nd_pdsm_cmd_pkg *pcmd)
> return (void *)(pcmd->payload);
> }
>
> +/* Various nvdimm health indicators */
> +#define PAPR_PDSM_DIMM_HEALTHY 0
> +#define PAPR_PDSM_DIMM_UNHEALTHY 1
> +#define PAPR_PDSM_DIMM_CRITICAL 2
> +#define PAPR_PDSM_DIMM_FATAL 3
> +
> +/*
> + * Struct exchanged between kernel & ndctl in for PAPR_PDSM_HEALTH
> + * Various flags indicate the health status of the dimm.
> + *
> + * dimm_unarmed : Dimm not armed. So contents wont persist.
> + * dimm_bad_shutdown : Previous shutdown did not persist contents.
> + * dimm_bad_restore : Contents from previous shutdown werent restored.
> + * dimm_scrubbed : Contents of the dimm have been scrubbed.
> + * dimm_locked : Contents of the dimm cant be modified until
> CEC reboot
> + * dimm_encrypted: Contents of dimm are encrypted.
> + * dimm_health : Dimm health indicator. One of
> PAPR_PDSM_DIMM_
> + */
> +struct
Re: [RESEND PATCH v9 4/5] ndctl/papr_scm,uapi: Add support for PAPR nvdimm specific methods
On Fri, Jun 5, 2020 at 8:22 AM Vaibhav Jain wrote: [..] > > Oh, why not define a maximal health payload with all the attributes > > you know about today, leave some room for future expansion, and then > > report a validity flag for each attribute? This is how the "intel" > > smart-health payload works. If they ever needed to extend the payload > > they would increase the size and add more validity flags. Old > > userspace never groks the new fields, new userspace knows to ask for > > and parse the larger payload. > > > > See the flags field in 'struct nd_intel_smart' (in ndctl) and the > > translation of those flags to ndctl generic attribute flags > > intel_cmd_smart_get_flags(). > > > > In general I'd like ndctl to understand the superset of all health > > attributes across all vendors. For the truly vendor specific ones it > > would mean that the health flags with a specific "papr_scm" back-end > > just would never be set on an "intel" device. I.e. look at the "hpe" > > and "msft" health backends. They only set a subset of the valid flags > > that could be reported. > > Thanks, this sounds good. Infact papr_scm implementation in ndctl does > advertises support for only a subset of ND_SMART_* flags right now. > > Using 'flags' instead of 'version' was indeed discussed during > v7..v9. However re-looking at the 'msft' and 'hpe' implementations the > approach of maximal health payload tagged with a flags field looks more > intuitive and I would prefer implementing this scheme in this patch-set. > > The current set health data exchanged with between libndctl and > papr_scm via 'struct nd_papr_pdsm_health' (e.g various health status > bits , nvdimm arming status etc) are guaranteed to be always available > hence associating their availability with a flag wont be much useful as > the flag will be always set. > > However as you suggested, extending the 'struct nd_papr_pdsm_health' in > future to accommodate new attributes like 'life-remaining' can be done > via adding them to the end of the struct and setting a flag field to > indicate its presence. > > So I have the following proposal: > * Add a new '__u32 extension_flags' field at beginning of 'struct > nd_papr_pdsm_health' > * Set the size of the struct to 184-bytes which is the maximum possible > size for a pdsm payload. > * 'papr_scm' kernel driver will currently set 'extension_flag' to 0 > indicating no extension fields. > > * Future patch that adds support for 'life-remaining' add the new-field > at the end of known fields in 'struct nd_papr_pdsm_health'. > * When provided to papr_scm kernel module, if 'life-remaining' data is > available its populated and corresponding flag set in > 'extension_flags' field indicating its presence. > * When received by libndctl papr_scm implementation its tests if the > extension_flags have associated 'life-remaining' flag set and if yes > then return ND_SMART_USED_VALID flag back from > ndctl_cmd_smart_get_flags(). > > Implementing first 3 items above in the current patchset should be > fairly trivial. > > Does that sounds reasonable ? This sounds good to me.
Re: [musl] Re: ppc64le and 32-bit LE userland compatibility
On Fri, Jun 05, 2020 at 12:27:02PM -0500, Segher Boessenkool wrote: > On Fri, Jun 05, 2020 at 04:18:18AM +0200, Daniel Kolesa wrote: > > On Fri, Jun 5, 2020, at 01:35, Segher Boessenkool wrote: > > > > The thing is, I've yet to see in which way the ELFv2 ABI *actually* > > > > requires VSX - I don't think compiling for 970 introduces any actual > > > > differences. There will be omissions, yes - but then the more accurate > > > > thing would be to say that a subset of ELFv2 is used, rather than it > > > > being a different ABI per se. > > > > > > Two big things are that binaries that someone else made are supposed to > > > work for you as well -- including binaries using VSX registers, or any > > > instructions that require ISA 2.07 (or some older ISA after 970). This > > > includes DSOs (shared libraries). So for a distribution this means that > > > they will not use VSX *anywhere*, or only in very specialised things. > > > That is a many-years setback, for people/situations where it could be > > > used. > > > > Third party precompiled stuff doesn't really need to concern us, since none > > really exists. > > Yet. And if you claim you support ELFv2, not mentioning the ways > your implementation deviates from it, users will be unhappy. > > > It's also still an upgrade over ELFv1 regardless (I mean, the same things > > apply there). > > Yeah, in mostly minor ways, but it all adds up for sure. > > > I'm also not really all that convinced that vectors make a huge difference > > in non-specialized code (autovectorization still has a way to go) > > They do make a huge difference, depending on the application of course. > But VSX is not just vectors even: it also gives you twice as many > floating point scalars (64 now), and in newer versions of the ISA it can > be beneficially used for integer scalars even. Vectorization is useful for a lot of things, and I'm sure there are specialized workloads that benefit from 64 scalars, but I've never encountered a place where having more than 16 registers made a practical difference. The fact that there are specialized areas where this stuff matters does not imply there aren't huge domains where it's completely irrelevant. > > and code written to use vector instructions should probably check > > auxval and take those paths at runtime. > > No, that is exactly the point of requiring ISA 2.07. Anything can use > ISA 2.07 (incl. VSX) without checking first, and without having a > fallback to some other implementation. Going from ISA 2.01 to 2.07 is > more than a decade of improvements, it is not trivial at all. This only affects code that's non-portable and PPC-specific, which a lot of people have no interest in and don't care about. Any portable code is going to either only use vectors via the compiler's choice to vectorize or conditionally on being one of a set of supported targets with a vector ISA it supports available. Anyone building for a target that doesn't have them just gets the portable version of the code. I think a lot of the unnecessary fighting on this topic is arising from differences of opinion over what an ABI entails. I would call what you're talking about a "platform" and more of a platform-specific *API* than an ABI -- it's about guarantees of interfaces available to the programmer, not implementation details of linkage. Rich
Re: [musl] Re: ppc64le and 32-bit LE userland compatibility
On Fri, Jun 05, 2020 at 04:18:18AM +0200, Daniel Kolesa wrote: > On Fri, Jun 5, 2020, at 01:35, Segher Boessenkool wrote: > > > The thing is, I've yet to see in which way the ELFv2 ABI *actually* > > > requires VSX - I don't think compiling for 970 introduces any actual > > > differences. There will be omissions, yes - but then the more accurate > > > thing would be to say that a subset of ELFv2 is used, rather than it > > > being a different ABI per se. > > > > Two big things are that binaries that someone else made are supposed to > > work for you as well -- including binaries using VSX registers, or any > > instructions that require ISA 2.07 (or some older ISA after 970). This > > includes DSOs (shared libraries). So for a distribution this means that > > they will not use VSX *anywhere*, or only in very specialised things. > > That is a many-years setback, for people/situations where it could be > > used. > > Third party precompiled stuff doesn't really need to concern us, since none > really exists. ... Yet. And if you claim you support ELFv2, not mentioning the ways your implementation deviates from it, users will be unhappy. > It's also still an upgrade over ELFv1 regardless (I mean, the same things > apply there). Yeah, in mostly minor ways, but it all adds up for sure. > I'm also not really all that convinced that vectors make a huge difference in > non-specialized code (autovectorization still has a way to go) They do make a huge difference, depending on the application of course. But VSX is not just vectors even: it also gives you twice as many floating point scalars (64 now), and in newer versions of the ISA it can be beneficially used for integer scalars even. > and code written to use vector instructions should probably check auxval and > take those paths at runtime. No, that is exactly the point of requiring ISA 2.07. Anything can use ISA 2.07 (incl. VSX) without checking first, and without having a fallback to some other implementation. Going from ISA 2.01 to 2.07 is more than a decade of improvements, it is not trivial at all. > As for other instructions, fair enough, but from my rough testing, it doesn't > make such a massive difference for average case That depends on what you call the average case. Code that is control and memory-bound will not benefit much from *anything* :-) > (and where it does, one can always rebuild their thing with > CFLAGS=-mcpu=power9) Yeah, but it helps quite a bit if your system (shared) libraries get all improvements they can as well. I'm not trying to dissuade you from not requiring VSX and 2.07 -- this sounds like your best option, given the constraints. I'm just saying the cost is not trivial (even ignoring the ABI divergence). > > The target name allows to make such distinctions: this could for example > > be powerpc64-*-linux-void (maybe I put the distinction in the wrong > > part of the name here? The glibc people will know better, and "void" is > > probably not a great name anyway). > > Hm, I'm not a huge fan of putting ABI specifics in the triplet, it feels > wrong - there is no precedent for it with POWER (ARM did it with EABI though), Maybe look at what the various BSDs use? We do have things like this. > the last part should remain 'gnu' as it's still glibc; besides, gcc is > compiled for exactly one target triplet, and traditionally with ppc compilers > it's always been possible to target everything with just one compiler > (endian, 32bit, 64bit, abi...). This isn't completely true. Yes, the compiler allows you to change word size, endianness, ABI, some more things. That does not mean you can actually build working binaries for all resulting combinations. As a trivial example, it will still pick up the same libraries from the same library paths usually, and those will spectacularly fail to work. We are biarch for some targets, which means that both powerpc-linux targets and powerpc64-linux targets can actually handle both of those, with just -m32 or -m64 needed to switch which configuration is used. But you cannot magically transparently switch to many other configurations: for those, you just build a separate toolchain for that specfic (variant) configuration, in the general case. > The best way would probably be adding a new -mabi, e.g. -mabi=elfv2-novsx > (just an example), which would behave exactly like -mabi=elfv2, except it'd > emit some extra detection macro Yeah, that sounds like a good idea. Patches welcome :-) (A separate target name is still needed, but this will make development simpler for sure). Segher
Re: [PATCH v10 4/6] powerpc/papr_scm: Improve error logging and handling papr_scm_ndctl()
On Fri, Jun 05, 2020 at 05:11:34AM +0530, Vaibhav Jain wrote:
> Since papr_scm_ndctl() can be called from outside papr_scm, its
> exposed to the possibility of receiving NULL as value of 'cmd_rc'
> argument. This patch updates papr_scm_ndctl() to protect against such
> possibility by assigning it pointer to a local variable in case cmd_rc
> == NULL.
>
> Finally the patch also updates the 'default' clause of the switch-case
> block removing a 'return' statement thereby ensuring that value of
> 'cmd_rc' is always logged when papr_scm_ndctl() returns.
>
> Cc: "Aneesh Kumar K . V"
> Cc: Dan Williams
> Cc: Michael Ellerman
> Cc: Ira Weiny
> Signed-off-by: Vaibhav Jain
> ---
> Changelog:
>
> v9..v10
> * New patch in the series
Thanks for making this a separate patch it is easier to see what is going on
here.
> ---
> arch/powerpc/platforms/pseries/papr_scm.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c
> b/arch/powerpc/platforms/pseries/papr_scm.c
> index 0c091622b15e..6512fe6a2874 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -355,11 +355,16 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor
> *nd_desc,
> {
> struct nd_cmd_get_config_size *get_size_hdr;
> struct papr_scm_priv *p;
> + int rc;
>
> /* Only dimm-specific calls are supported atm */
> if (!nvdimm)
> return -EINVAL;
>
> + /* Use a local variable in case cmd_rc pointer is NULL */
> + if (!cmd_rc)
> + cmd_rc =
> +
This protects you from the NULL. However...
> p = nvdimm_provider_data(nvdimm);
>
> switch (cmd) {
> @@ -381,12 +386,13 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor
> *nd_desc,
> break;
>
> default:
> - return -EINVAL;
> + dev_dbg(>pdev->dev, "Unknown command = %d\n", cmd);
> + *cmd_rc = -EINVAL;
... I think you are conflating rc and cmd_rc...
> }
>
> dev_dbg(>pdev->dev, "returned with cmd_rc = %d\n", *cmd_rc);
>
> - return 0;
> + return *cmd_rc;
... this changes the behavior of the current commands. Now if the underlying
papr_scm_meta_[get|set]() fails you return that failure as rc rather than 0.
Is that ok?
Also 'logging cmd_rc' in the invalid cmd case does not seem quite right unless
you really want rc to be cmd_rc.
The architecture is designed to separate errors which occur in the kernel vs
errors in the firmware/dimm. Are they always the same? The current code
differentiates them.
Ira
> }
>
> static ssize_t flags_show(struct device *dev,
> --
> 2.26.2
>
[GIT PULL] Please pull powerpc/linux.git powerpc-5.8-1 tag
Hi Linus,
Please pull powerpc updates for 5.8.
Unfortunately we've ended up with quite a few conflicts, which is primarily my
fault for pushing things to next too late. Lesson learnt.
I've pushed the result of my resolution of the conflicts to the powerpc/merge
branch, if you want to look at that, though I've also tried to describe it in
full below.
Firstly there's a conflict in arch/powerpc/kernel/traps.c in
machine_check_exception() vs 69ea03b56ed2 ("hardirq/nmi: Allow nested
nmi_enter()"). That change made nmi_enter() handle nesting natively, but in
parallel we changed our code to only call nmi_enter() on some configurations
which interacts badly.
The condition on the call to nmi_enter() needs updating, as well as the two
calls to nmi_exit(), and then the comment as well. So I've just included the end
result for the bulk of the function:
void machine_check_exception(struct pt_regs *regs)
{
int recover = 0;
/*
* BOOK3S_64 does not call this handler as a non-maskable interrupt
* (it uses its own early real-mode handler to handle the MCE proper
* and then raises irq_work to call this handler when interrupts are
* enabled).
*/
if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64))
nmi_enter();
__this_cpu_inc(irq_stat.mce_exceptions);
...
if (check_io_access(regs))
goto bail;
if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64))
nmi_exit();
die("Machine check", regs, SIGBUS);
/* Must die if the interrupt is not recoverable */
if (!(regs->msr & MSR_RI))
die("Unrecoverable Machine check", regs, SIGBUS);
return;
bail:
if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64))
nmi_exit();
}
Then there's two conflicts with 2fb4706057bc ("powerpc: add support for folded
p4d page tables").
The first one in arch/powerpc/mm/kasan/kasan_init_32.c in
kasan_remap_early_shadow_ro() is simple, just remove the for loop entirely.
Then in arch/powerpc/mm/ptdump/ptdump.c, in walk_pagetables(), the for loop
should end up being:
for (i = pgd_index(addr); i < PTRS_PER_PGD; i++, pgd++, addr +=
PGDIR_SIZE) {
p4d_t *p4d = p4d_offset(pgd, 0);
if (p4d_none(*p4d) || p4d_is_leaf(*p4d))
note_page(st, addr, 1, p4d_val(*p4d), PGDIR_SIZE);
else if (is_hugepd(__hugepd(p4d_val(*p4d
walk_hugepd(st, (hugepd_t *)p4d, addr, PGDIR_SHIFT, 1);
else
/* p4d exists */
walk_pud(st, p4d, addr);
}
Finally, we need this hunk applied to avoid a build break:
diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h
b/arch/powerpc/include/asm/nohash/32/pgtable.h
index 61fc9e8f12d3..af7f13cf90cf 100644
--- a/arch/powerpc/include/asm/nohash/32/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
@@ -239,7 +239,7 @@ static inline pte_basic_t pte_update(struct mm_struct *mm,
unsigned long addr, p
pte_basic_t old = pte_val(*p);
pte_basic_t new = (old & ~(pte_basic_t)clr) | set;
int num, i;
- pmd_t *pmd = pmd_offset(pud_offset(pgd_offset(mm, addr), addr), addr);
+ pmd_t *pmd = pmd_offset(pud_offset(p4d_offset(pgd_offset(mm, addr),
addr), addr), addr);
if (!huge)
num = PAGE_SIZE / SZ_4K;
Let me know if any of that doesn't make sense or otherwise causes problems.
There's also a few out-of-area changes which I'll highlight FYI:
arch/s390/include/asm/pgtable.h # 93a98695f2f9 mm: change
pmdp_huge_get_and_clear_full take vm_area_struct as arg
include/asm-generic/pgtable.h
mm/huge_memory.c
drivers/input/serio/# e4f4ffa8a98c input: i8042 - Remove special
PowerPC handling
include/linux/hw_breakpoint.h # ef3534a94fdb hw-breakpoints: Fix build
warnings with clang
kernel/events/hw_breakpoint.c # 29da4f91c0c1 powerpc/watchpoint: Don't allow
concurrent perf and ptrace events
sound/... # f16dca3e30c1 sound: ac97: Remove sound driver
for ancient platform
cheers
The following changes since commit ae83d0b416db002fe95601e7f97f64b59514d936:
Linux 5.7-rc2 (2020-04-19 14:35:30 -0700)
are available in the git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git
tags/powerpc-5.8-1
for you to fetch changes up to 1395375c592770fe5158a592944aaeed67fa94ff:
Merge branch 'topic/ppc-kvm' into next (2020-06-03 13:44:51 +1000)
--
powerpc updates for 5.8
- Support for userspace to send requests directly to the on-chip GZIP
accelerator on Power9.
- Rework of our lockless page table walking (__find_linux_pte()) to make it
safe against parallel page table manipulations without relying on an IPI for
serialisation.
- A series of fixes & enhancements to make our machine check handling more
robust.
-
Re: Boot issue with the latest Git kernel
On 04 June 2020 at 7:15 pm, Christophe Leroy wrote: Yes today's linux-next boots on my powerpc 8xx board. Christophe Hello Christophe, Thanks for testing. I was able to perform a 'git bisect' [1] and identified the bad commit. [2] I reverted this commit and after that the kernel boots and works without any problems. Could you please check this commit? Thanks, Christian [1] https://forum.hyperion-entertainment.com/viewtopic.php?p=50772#p50772 [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2ba3e6947aed9bb9575eb1603c0ac6e39185d32a
Re: [PATCH v2] cxl: Remove dead Kconfig option
Andrew Donnellan writes: > The CXL_AFU_DRIVER_OPS Kconfig option was added to coordinate merging of > new features. It no longer serves any purpose, so remove it. > > Signed-off-by: Andrew Donnellan > > --- > v1->v2: > - keep CXL_LIB for now to avoid breaking a driver that is currently out of > tree Sorry I already merged v1. cheers
Re: [PATCH] tpm: ibmvtpm: Wait for ready buffer before probing for TPM2 attributes
On 6/5/20 2:37 AM, David Gibson wrote:
The tpm2_get_cc_attrs_tbl() call will result in TPM commands being issued,
which will need the use of the internal command/response buffer. But,
we're issuing this *before* we've waited to make sure that buffer is
allocated.
This can result in intermittent failures to probe if the hypervisor / TPM
implementation doesn't respond quickly enough. I find it fails almost
every time with an 8 vcpu guest under KVM with software emulated TPM.
Uuuh. Thanks!
Fixes: 18b3670d79ae9 "tpm: ibmvtpm: Add support for TPM2"
Signed-off-by: David Gibson
Reviewed-by: Stefan Berger
---
drivers/char/tpm/tpm_ibmvtpm.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 09fe45246b8c..994385bf37c0 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -683,13 +683,6 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
if (rc)
goto init_irq_cleanup;
- if (!strcmp(id->compat, "IBM,vtpm20")) {
- chip->flags |= TPM_CHIP_FLAG_TPM2;
- rc = tpm2_get_cc_attrs_tbl(chip);
- if (rc)
- goto init_irq_cleanup;
- }
-
if (!wait_event_timeout(ibmvtpm->crq_queue.wq,
ibmvtpm->rtce_buf != NULL,
HZ)) {
@@ -697,6 +690,13 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
goto init_irq_cleanup;
}
+ if (!strcmp(id->compat, "IBM,vtpm20")) {
+ chip->flags |= TPM_CHIP_FLAG_TPM2;
+ rc = tpm2_get_cc_attrs_tbl(chip);
+ if (rc)
+ goto init_irq_cleanup;
+ }
+
return tpm_chip_register(chip);
init_irq_cleanup:
do {
RE: [RESEND PATCH v9 4/5] ndctl/papr_scm, uapi: Add support for PAPR nvdimm specific methods
"Williams, Dan J" writes: >> -Original Message- >> From: Vaibhav Jain >> Sent: Thursday, June 4, 2020 2:06 AM >> To: Williams, Dan J ; linuxppc- >> d...@lists.ozlabs.org; linux-nvd...@lists.01.org; linux- >> ker...@vger.kernel.org >> Cc: Santosh Sivaraj ; Aneesh Kumar K . V >> ; Steven Rostedt ; >> Oliver O'Halloran ; Weiny, Ira >> Subject: RE: [RESEND PATCH v9 4/5] ndctl/papr_scm,uapi: Add support for >> PAPR nvdimm specific methods >> >> Hi Dan, >> >> Thanks for review and insights on this. My responses below: >> >> "Williams, Dan J" writes: >> >> > [ forgive formatting I'm temporarily stuck using Outlook this week... >> > ] >> > >> >> From: Vaibhav Jain >> > [..] >> >> >> >> Introduce support for PAPR NVDIMM Specific Methods (PDSM) in >> papr_scm >> >> module and add the command family NVDIMM_FAMILY_PAPR to the >> white >> >> list of NVDIMM command sets. Also advertise support for ND_CMD_CALL >> >> for the nvdimm command mask and implement necessary scaffolding in >> >> the module to handle ND_CMD_CALL ioctl and PDSM requests that we >> receive. >> >> >> >> The layout of the PDSM request as we expect from libnvdimm/libndctl >> >> is described in newly introduced uapi header 'papr_pdsm.h' which >> >> defines a new 'struct nd_pdsm_cmd_pkg' header. This header is used to >> >> communicate the PDSM request via member >> 'nd_cmd_pkg.nd_command' and >> >> size of payload that need to be sent/received for servicing the PDSM. >> >> >> >> A new function is_cmd_valid() is implemented that reads the args to >> >> papr_scm_ndctl() and performs sanity tests on them. A new function >> >> papr_scm_service_pdsm() is introduced and is called from >> >> papr_scm_ndctl() in case of a PDSM request is received via >> >> ND_CMD_CALL command from libnvdimm. >> >> >> >> Cc: "Aneesh Kumar K . V" >> >> Cc: Dan Williams >> >> Cc: Michael Ellerman >> >> Cc: Ira Weiny >> >> Reviewed-by: Aneesh Kumar K.V >> >> Signed-off-by: Vaibhav Jain >> >> --- >> >> Changelog: >> >> >> >> Resend: >> >> * Added ack from Aneesh. >> >> >> >> v8..v9: >> >> * Reduced the usage of term SCM replacing it with appropriate >> >> replacement [ Dan Williams, Aneesh ] >> >> * Renamed 'papr_scm_pdsm.h' to 'papr_pdsm.h' >> >> * s/PAPR_SCM_PDSM_*/PAPR_PDSM_*/g >> >> * s/NVDIMM_FAMILY_PAPR_SCM/NVDIMM_FAMILY_PAPR/g >> >> * Minor updates to 'papr_psdm.h' to replace usage of term 'SCM'. >> >> * Minor update to patch description. >> >> >> >> v7..v8: >> >> * Removed the 'payload_offset' field from 'struct >> >> nd_pdsm_cmd_pkg'. Instead command payload is always assumed to >> start >> >> at 'nd_pdsm_cmd_pkg.payload'. [ Aneesh ] >> >> * To enable introducing new fields to 'struct nd_pdsm_cmd_pkg', >> >> 'reserved' field of 10-bytes is introduced. [ Aneesh ] >> >> * Fixed a typo in "Backward Compatibility" section of papr_scm_pdsm.h >> >> [ Ira ] >> >> >> >> Resend: >> >> * None >> >> >> >> v6..v7 : >> >> * Removed the re-definitions of __packed macro from papr_scm_pdsm.h >> >> [Mpe]. >> >> * Removed the usage of __KERNEL__ macros in papr_scm_pdsm.h >> [Mpe]. >> >> * Removed macros that were unused in papr_scm.c from >> papr_scm_pdsm.h >> >> [Mpe]. >> >> * Made functions defined in papr_scm_pdsm.h as static inline. [Mpe] >> >> >> >> v5..v6 : >> >> * Changed the usage of the term DSM to PDSM to distinguish it from the >> >> ACPI term [ Dan Williams ] >> >> * Renamed papr_scm_dsm.h to papr_scm_pdsm.h and updated various >> >> struct >> >> to reflect the new terminology. >> >> * Updated the patch description and title to reflect the new terminology. >> >> * Squashed patch to introduce new command family in 'ndctl.h' with >> >> this patch [ Dan Williams ] >> >> * Updated the papr_scm_pdsm method starting index from 0x1 to >> 0x0 >> >> [ Dan Williams ] >> >> * Removed redundant license text from the papr_scm_psdm.h file. >> >> [ Dan Williams ] >> >> * s/envelop/envelope/ at various places [ Dan Williams ] >> >> * Added '__packed' attribute to command package header to gaurd >> >> against different compiler adding paddings between the fields. >> >> [ Dan Williams] >> >> * Converted various pr_debug to dev_debug [ Dan Williams ] >> >> >> >> v4..v5 : >> >> * None >> >> >> >> v3..v4 : >> >> * None >> >> >> >> v2..v3 : >> >> * Updated the patch prefix to 'ndctl/uapi' [Aneesh] >> >> >> >> v1..v2 : >> >> * None >> >> --- >> >> arch/powerpc/include/uapi/asm/papr_pdsm.h | 136 >> >> ++ >> arch/powerpc/platforms/pseries/papr_scm.c | >> >> 101 +++- >> >> include/uapi/linux/ndctl.h| 1 + >> >> 3 files changed, 232 insertions(+), 6 deletions(-) create mode >> >> 100644 arch/powerpc/include/uapi/asm/papr_pdsm.h >> >> >> >> diff --git a/arch/powerpc/include/uapi/asm/papr_pdsm.h >> >> b/arch/powerpc/include/uapi/asm/papr_pdsm.h >> >> new file mode 100644 >> >> index ..6407fefcc007 >> >> --- /dev/null >> >> +++ b/arch/powerpc/include/uapi/asm/papr_pdsm.h >> >> @@ -0,0
Re: [PATCH v1 2/4] KVM: PPC: Book3S HV: track shared GFNs of secure VMs
> >diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c > >b/arch/powerpc/kvm/book3s_64_mmu_radix.c > >index 803940d..3448459 100644 > >--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c > >+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c > >@@ -1100,7 +1100,7 @@ void kvmppc_radix_flush_memslot(struct kvm *kvm, > > unsigned int shift; > > if (kvm->arch.secure_guest & KVMPPC_SECURE_INIT_START) > >-kvmppc_uvmem_drop_pages(memslot, kvm, true); > >+kvmppc_uvmem_drop_pages(memslot, kvm, true, false); > > Why purge_gfn is false here? > That call function is called when dropping an hot plugged memslot. This function does not know, under what context it is called. Since its job is to just flush the memslot, it cannot assume anything about purging the pages in the memslot. .snip.. RP
[PATCH] powerpc/mm: Fix typo in IS_ENABLED()
From: Joe Perches
IS_ENABLED() matches names exactly, so the missing "CONFIG_" prefix
means this code would never be built.
Also fixes a missing newline in pr_warn().
Signed-off-by: Joe Perches
Link:
https://lore.kernel.org/lkml/b08611018fdb6d88757c6008a5c02fa0e07b32fb.ca...@perches.com
Signed-off-by: Kees Cook
---
arch/powerpc/mm/book3s64/hash_utils.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c
b/arch/powerpc/mm/book3s64/hash_utils.c
index 8ed2411c3f39..cf2e1b06e5d4 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -660,11 +660,10 @@ static void __init htab_init_page_sizes(void)
* Pick a size for the linear mapping. Currently, we only
* support 16M, 1M and 4K which is the default
*/
- if (IS_ENABLED(STRICT_KERNEL_RWX) &&
+ if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) &&
(unsigned long)_stext % 0x100) {
if (mmu_psize_defs[MMU_PAGE_16M].shift)
- pr_warn("Kernel not 16M aligned, "
- "disabling 16M linear map alignment");
+ pr_warn("Kernel not 16M aligned, disabling 16M
linear map alignment\n");
aligned = false;
}
--
2.25.1
--
Kees Cook
Re: [PATCH v4 1/4] riscv: Move kernel mapping to vmalloc zone
Hi Zong,
Le 6/3/20 à 10:52 PM, Zong Li a écrit :
On Wed, Jun 3, 2020 at 4:01 PM Alexandre Ghiti wrote:
This is a preparatory patch for relocatable kernel.
The kernel used to be linked at PAGE_OFFSET address and used to be loaded
physically at the beginning of the main memory. Therefore, we could use
the linear mapping for the kernel mapping.
But the relocated kernel base address will be different from PAGE_OFFSET
and since in the linear mapping, two different virtual addresses cannot
point to the same physical address, the kernel mapping needs to lie outside
the linear mapping.
In addition, because modules and BPF must be close to the kernel (inside
+-2GB window), the kernel is placed at the end of the vmalloc zone minus
2GB, which leaves room for modules and BPF. The kernel could not be
placed at the beginning of the vmalloc zone since other vmalloc
allocations from the kernel could get all the +-2GB window around the
kernel which would prevent new modules and BPF programs to be loaded.
Signed-off-by: Alexandre Ghiti
---
arch/riscv/boot/loader.lds.S | 3 +-
arch/riscv/include/asm/page.h| 10 +-
arch/riscv/include/asm/pgtable.h | 38 ++---
arch/riscv/kernel/head.S | 3 +-
arch/riscv/kernel/module.c | 4 +--
arch/riscv/kernel/vmlinux.lds.S | 3 +-
arch/riscv/mm/init.c | 58 +---
arch/riscv/mm/physaddr.c | 2 +-
8 files changed, 88 insertions(+), 33 deletions(-)
diff --git a/arch/riscv/boot/loader.lds.S b/arch/riscv/boot/loader.lds.S
index 47a5003c2e28..62d94696a19c 100644
--- a/arch/riscv/boot/loader.lds.S
+++ b/arch/riscv/boot/loader.lds.S
@@ -1,13 +1,14 @@
/* SPDX-License-Identifier: GPL-2.0 */
#include
+#include
OUTPUT_ARCH(riscv)
ENTRY(_start)
SECTIONS
{
- . = PAGE_OFFSET;
+ . = KERNEL_LINK_ADDR;
.payload : {
*(.payload)
diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index 2d50f76efe48..48bb09b6a9b7 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -90,18 +90,26 @@ typedef struct page *pgtable_t;
#ifdef CONFIG_MMU
extern unsigned long va_pa_offset;
+extern unsigned long va_kernel_pa_offset;
extern unsigned long pfn_base;
#define ARCH_PFN_OFFSET(pfn_base)
#else
#define va_pa_offset 0
+#define va_kernel_pa_offset0
#define ARCH_PFN_OFFSET(PAGE_OFFSET >> PAGE_SHIFT)
#endif /* CONFIG_MMU */
extern unsigned long max_low_pfn;
extern unsigned long min_low_pfn;
+extern unsigned long kernel_virt_addr;
#define __pa_to_va_nodebug(x) ((void *)((unsigned long) (x) + va_pa_offset))
-#define __va_to_pa_nodebug(x) ((unsigned long)(x) - va_pa_offset)
+#define linear_mapping_va_to_pa(x) ((unsigned long)(x) - va_pa_offset)
+#define kernel_mapping_va_to_pa(x) \
+ ((unsigned long)(x) - va_kernel_pa_offset)
+#define __va_to_pa_nodebug(x) \
+ (((x) >= PAGE_OFFSET) ? \
+ linear_mapping_va_to_pa(x) : kernel_mapping_va_to_pa(x))
#ifdef CONFIG_DEBUG_VIRTUAL
extern phys_addr_t __virt_to_phys(unsigned long x);
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 35b60035b6b0..94ef3b49dfb6 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -11,23 +11,29 @@
#include
-#ifndef __ASSEMBLY__
-
-/* Page Upper Directory not used in RISC-V */
-#include
-#include
-#include
-#include
-
-#ifdef CONFIG_MMU
+#ifndef CONFIG_MMU
+#define KERNEL_VIRT_ADDR PAGE_OFFSET
+#define KERNEL_LINK_ADDR PAGE_OFFSET
+#else
+/*
+ * Leave 2GB for modules and BPF that must lie within a 2GB range around
+ * the kernel.
+ */
+#define KERNEL_VIRT_ADDR (VMALLOC_END - SZ_2G + 1)
+#define KERNEL_LINK_ADDR KERNEL_VIRT_ADDR
#define VMALLOC_SIZE (KERN_VIRT_SIZE >> 1)
#define VMALLOC_END (PAGE_OFFSET - 1)
#define VMALLOC_START(PAGE_OFFSET - VMALLOC_SIZE)
#define BPF_JIT_REGION_SIZE(SZ_128M)
-#define BPF_JIT_REGION_START (PAGE_OFFSET - BPF_JIT_REGION_SIZE)
-#define BPF_JIT_REGION_END (VMALLOC_END)
+#define BPF_JIT_REGION_START PFN_ALIGN((unsigned long)&_end)
+#define BPF_JIT_REGION_END (BPF_JIT_REGION_START + BPF_JIT_REGION_SIZE)
+
+#ifdef CONFIG_64BIT
+#define VMALLOC_MODULE_START BPF_JIT_REGION_END
+#define VMALLOC_MODULE_END (((unsigned long)&_start & PAGE_MASK) + SZ_2G)
+#endif
/*
* Roughly size the vmemmap space to be large enough to fit enough
@@ -57,9 +63,16 @@
#define FIXADDR_SIZE PGDIR_SIZE
#endif
#define FIXADDR_START(FIXADDR_TOP - FIXADDR_SIZE)
-
#endif
+#ifndef __ASSEMBLY__
+
+/* Page Upper Directory not used in RISC-V */
+#include
+#include
+#include
+#include
+
#ifdef CONFIG_64BIT
#include
#else
@@ -483,6 +496,7 @@ static inline void __kernel_map_pages(struct page *page,
int numpages, int enabl
#define
[PATCH AUTOSEL 4.9 5/6] drivers/net/ibmvnic: Update VNIC protocol version reporting
From: Thomas Falcon [ Upstream commit 784688993ebac34dffe44a9f2fabbe126ebfd4db ] VNIC protocol version is reported in big-endian format, but it is not byteswapped before logging. Fix that, and remove version comparison as only one protocol version exists at this time. Signed-off-by: Thomas Falcon Signed-off-by: David S. Miller Signed-off-by: Sasha Levin --- drivers/net/ethernet/ibm/ibmvnic.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 897a87ae8655..20f7ab4aa2f1 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -3362,12 +3362,10 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq, dev_err(dev, "Error %ld in VERSION_EXCHG_RSP\n", rc); break; } - dev_info(dev, "Partner protocol version is %d\n", -crq->version_exchange_rsp.version); - if (be16_to_cpu(crq->version_exchange_rsp.version) < - ibmvnic_version) - ibmvnic_version = + ibmvnic_version = be16_to_cpu(crq->version_exchange_rsp.version); + dev_info(dev, "Partner protocol version is %d\n", +ibmvnic_version); send_cap_queries(adapter); break; case QUERY_CAPABILITY_RSP: -- 2.25.1
[PATCH AUTOSEL 4.14 7/8] drivers/net/ibmvnic: Update VNIC protocol version reporting
From: Thomas Falcon [ Upstream commit 784688993ebac34dffe44a9f2fabbe126ebfd4db ] VNIC protocol version is reported in big-endian format, but it is not byteswapped before logging. Fix that, and remove version comparison as only one protocol version exists at this time. Signed-off-by: Thomas Falcon Signed-off-by: David S. Miller Signed-off-by: Sasha Levin --- drivers/net/ethernet/ibm/ibmvnic.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 956fbb164e6f..85c11dafb4cd 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -3560,12 +3560,10 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq, dev_err(dev, "Error %ld in VERSION_EXCHG_RSP\n", rc); break; } - dev_info(dev, "Partner protocol version is %d\n", -crq->version_exchange_rsp.version); - if (be16_to_cpu(crq->version_exchange_rsp.version) < - ibmvnic_version) - ibmvnic_version = + ibmvnic_version = be16_to_cpu(crq->version_exchange_rsp.version); + dev_info(dev, "Partner protocol version is %d\n", +ibmvnic_version); send_cap_queries(adapter); break; case QUERY_CAPABILITY_RSP: -- 2.25.1
[PATCH AUTOSEL 4.19 8/9] drivers/net/ibmvnic: Update VNIC protocol version reporting
From: Thomas Falcon [ Upstream commit 784688993ebac34dffe44a9f2fabbe126ebfd4db ] VNIC protocol version is reported in big-endian format, but it is not byteswapped before logging. Fix that, and remove version comparison as only one protocol version exists at this time. Signed-off-by: Thomas Falcon Signed-off-by: David S. Miller Signed-off-by: Sasha Levin --- drivers/net/ethernet/ibm/ibmvnic.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index abfd990ba4d8..645298628b6f 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -4295,12 +4295,10 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq, dev_err(dev, "Error %ld in VERSION_EXCHG_RSP\n", rc); break; } - dev_info(dev, "Partner protocol version is %d\n", -crq->version_exchange_rsp.version); - if (be16_to_cpu(crq->version_exchange_rsp.version) < - ibmvnic_version) - ibmvnic_version = + ibmvnic_version = be16_to_cpu(crq->version_exchange_rsp.version); + dev_info(dev, "Partner protocol version is %d\n", +ibmvnic_version); send_cap_queries(adapter); break; case QUERY_CAPABILITY_RSP: -- 2.25.1
[PATCH AUTOSEL 5.4 13/14] drivers/net/ibmvnic: Update VNIC protocol version reporting
From: Thomas Falcon [ Upstream commit 784688993ebac34dffe44a9f2fabbe126ebfd4db ] VNIC protocol version is reported in big-endian format, but it is not byteswapped before logging. Fix that, and remove version comparison as only one protocol version exists at this time. Signed-off-by: Thomas Falcon Signed-off-by: David S. Miller Signed-off-by: Sasha Levin --- drivers/net/ethernet/ibm/ibmvnic.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index aaa03ce5796f..5a42ddeecfe5 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -4536,12 +4536,10 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq, dev_err(dev, "Error %ld in VERSION_EXCHG_RSP\n", rc); break; } - dev_info(dev, "Partner protocol version is %d\n", -crq->version_exchange_rsp.version); - if (be16_to_cpu(crq->version_exchange_rsp.version) < - ibmvnic_version) - ibmvnic_version = + ibmvnic_version = be16_to_cpu(crq->version_exchange_rsp.version); + dev_info(dev, "Partner protocol version is %d\n", +ibmvnic_version); send_cap_queries(adapter); break; case QUERY_CAPABILITY_RSP: -- 2.25.1
[PATCH AUTOSEL 5.6 16/17] drivers/net/ibmvnic: Update VNIC protocol version reporting
From: Thomas Falcon [ Upstream commit 784688993ebac34dffe44a9f2fabbe126ebfd4db ] VNIC protocol version is reported in big-endian format, but it is not byteswapped before logging. Fix that, and remove version comparison as only one protocol version exists at this time. Signed-off-by: Thomas Falcon Signed-off-by: David S. Miller Signed-off-by: Sasha Levin --- drivers/net/ethernet/ibm/ibmvnic.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 3de549c6c693..197dc5b2c090 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -4678,12 +4678,10 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq, dev_err(dev, "Error %ld in VERSION_EXCHG_RSP\n", rc); break; } - dev_info(dev, "Partner protocol version is %d\n", -crq->version_exchange_rsp.version); - if (be16_to_cpu(crq->version_exchange_rsp.version) < - ibmvnic_version) - ibmvnic_version = + ibmvnic_version = be16_to_cpu(crq->version_exchange_rsp.version); + dev_info(dev, "Partner protocol version is %d\n", +ibmvnic_version); send_cap_queries(adapter); break; case QUERY_CAPABILITY_RSP: -- 2.25.1
Re: [PATCH 6/7] powerpc/perf: power10 Performance Monitoring support
Hi Athira, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on powerpc/next] [also build test WARNING on next-20200605] [cannot apply to kvm-ppc/kvm-ppc-next mpe/next v5.7] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Athira-Rajeev/powerpc-perf-Add-support-for-power10-PMU-Hardware/20200605-161850 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: powerpc-allyesconfig (attached as .config) compiler: powerpc64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>, old ones prefixed by <<): >> arch/powerpc/perf/power10-pmu.c:405:5: warning: no previous prototype for >> 'init_power10_pmu' [-Wmissing-prototypes] 405 | int init_power10_pmu(void) | ^~~~ vim +/init_power10_pmu +405 arch/powerpc/perf/power10-pmu.c 404 > 405 int init_power10_pmu(void) --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH v1 2/4] KVM: PPC: Book3S HV: track shared GFNs of secure VMs
Le 31/05/2020 à 04:27, Ram Pai a écrit :
During the life of SVM, its GFNs can transition from secure to shared
state and vice-versa. Since the kernel does not track GFNs that are
shared, it is not possible to disambiguate a shared GFN from a GFN whose
PFN has not yet been migrated to a device-PFN.
The ability to identify a shared GFN is needed to skip migrating its PFN
to device PFN. This functionality is leveraged in a subsequent patch.
Add the ability to identify the state of a GFN.
Cc: Paul Mackerras
Cc: Benjamin Herrenschmidt
Cc: Michael Ellerman
Cc: Bharata B Rao
Cc: Aneesh Kumar K.V
Cc: Sukadev Bhattiprolu
Cc: Laurent Dufour
Cc: Thiago Jung Bauermann
Cc: David Gibson
Cc: Claudio Carvalho
Cc: kvm-...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Reviewed-by: Thiago Jung Bauermann
Signed-off-by: Ram Pai
---
arch/powerpc/include/asm/kvm_book3s_uvmem.h | 6 +-
arch/powerpc/kvm/book3s_64_mmu_radix.c | 2 +-
arch/powerpc/kvm/book3s_hv.c| 2 +-
arch/powerpc/kvm/book3s_hv_uvmem.c | 115 ++--
4 files changed, 113 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s_uvmem.h
b/arch/powerpc/include/asm/kvm_book3s_uvmem.h
index 5a9834e..f0c5708 100644
--- a/arch/powerpc/include/asm/kvm_book3s_uvmem.h
+++ b/arch/powerpc/include/asm/kvm_book3s_uvmem.h
@@ -21,7 +21,8 @@ unsigned long kvmppc_h_svm_page_out(struct kvm *kvm,
int kvmppc_send_page_to_uv(struct kvm *kvm, unsigned long gfn);
unsigned long kvmppc_h_svm_init_abort(struct kvm *kvm);
void kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *free,
-struct kvm *kvm, bool skip_page_out);
+struct kvm *kvm, bool skip_page_out,
+bool purge_gfn);
#else
static inline int kvmppc_uvmem_init(void)
{
@@ -75,6 +76,7 @@ static inline int kvmppc_send_page_to_uv(struct kvm *kvm,
unsigned long gfn)
static inline void
kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *free,
- struct kvm *kvm, bool skip_page_out) { }
+ struct kvm *kvm, bool skip_page_out,
+ bool purge_gfn) { }
#endif /* CONFIG_PPC_UV */
#endif /* __ASM_KVM_BOOK3S_UVMEM_H__ */
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c
b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index 803940d..3448459 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -1100,7 +1100,7 @@ void kvmppc_radix_flush_memslot(struct kvm *kvm,
unsigned int shift;
if (kvm->arch.secure_guest & KVMPPC_SECURE_INIT_START)
- kvmppc_uvmem_drop_pages(memslot, kvm, true);
+ kvmppc_uvmem_drop_pages(memslot, kvm, true, false);
Why purge_gfn is false here?
That call function is called when dropping an hot plugged memslot.
That's being said, when called by kvmppc_core_commit_memory_region_hv(), the mem
slot is then free by kvmppc_uvmem_slot_free() so that shared state will not
remain long but there is a window...
if (kvm->arch.secure_guest & KVMPPC_SECURE_INIT_DONE)
return;
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 103d13e..4c62bfe 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -5467,7 +5467,7 @@ static int kvmhv_svm_off(struct kvm *kvm)
continue;
kvm_for_each_memslot(memslot, slots) {
- kvmppc_uvmem_drop_pages(memslot, kvm, true);
+ kvmppc_uvmem_drop_pages(memslot, kvm, true, true);
uv_unregister_mem_slot(kvm->arch.lpid, memslot->id);
}
}
diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c
b/arch/powerpc/kvm/book3s_hv_uvmem.c
index ea4a1f1..2ef1e03 100644
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -99,14 +99,56 @@
static DEFINE_SPINLOCK(kvmppc_uvmem_bitmap_lock);
#define KVMPPC_UVMEM_PFN (1UL << 63)
+#define KVMPPC_UVMEM_SHARED(1UL << 62)
+#define KVMPPC_UVMEM_FLAG_MASK (KVMPPC_UVMEM_PFN | KVMPPC_UVMEM_SHARED)
+#define KVMPPC_UVMEM_PFN_MASK (~KVMPPC_UVMEM_FLAG_MASK)
struct kvmppc_uvmem_slot {
struct list_head list;
unsigned long nr_pfns;
unsigned long base_pfn;
+ /*
+* pfns array has an entry for each GFN of the memory slot.
+*
+* The GFN can be in one of the following states.
+*
+* (a) Secure - The GFN is secure. Only Ultravisor can access it.
+* (b) Shared - The GFN is shared. Both Hypervisor and Ultravisor
+* can access it.
+* (c) Normal - The GFN is a normal. Only Hypervisor can access it.
+*
+* Secure GFN is associated with a devicePFN. Its pfn[] has
+* KVMPPC_UVMEM_PFN flag set, and has the value of the device PFN
+* KVMPPC_UVMEM_SHARED flag unset, and has the
[PATCH 7/7] powerpc/perf: support BHRB disable bit and new filtering modes
PowerISA v3.1 has few updates for the Branch History Rolling Buffer(BHRB).
First is the addition of BHRB disable bit and second new filtering
modes for BHRB.
BHRB disable is controlled via Monitor Mode Control Register A (MMCRA)
bit 26, namely "BHRB Recording Disable (BHRBRD)". This field controls
whether BHRB entries are written when BHRB recording is enabled by other
bits. Patch implements support for this BHRB disable bit.
Secondly PowerISA v3.1 introduce filtering support for
PERF_SAMPLE_BRANCH_IND_CALL/COND. The patch adds BHRB filter support
for "ind_call" and "cond" in power10_bhrb_filter_map().
'commit bb19af816025 ("powerpc/perf: Prevent kernel address leak to userspace
via BHRB buffer")'
added a check in bhrb_read() to filter the kernel address from BHRB buffer.
Patch here modified
it to avoid that check for PowerISA v3.1 based processors, since PowerISA v3.1
allows
only MSR[PR]=1 address to be written to BHRB buffer.
Signed-off-by: Athira Rajeev
---
arch/powerpc/perf/core-book3s.c | 27 +--
arch/powerpc/perf/isa207-common.c | 13 +
arch/powerpc/perf/power10-pmu.c | 13 +++--
arch/powerpc/platforms/powernv/idle.c | 14 ++
4 files changed, 59 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 44c86a9..d3856ff 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -464,9 +464,13 @@ static void power_pmu_bhrb_read(struct perf_event *event,
struct cpu_hw_events *
* addresses at this point. Check the privileges before
* exporting it to userspace (avoid exposure of regions
* where we could have speculative execution)
+* Incase of ISA 310, BHRB will capture only user-space
+* address,hence include a check before filtering code
*/
- if (is_kernel_addr(addr) &&
perf_allow_kernel(>attr) != 0)
- continue;
+ if (!(ppmu->flags & PPMU_ARCH_310S))
+ if (is_kernel_addr(addr) &&
+ perf_allow_kernel(>attr) != 0)
+ continue;
/* Branches are read most recent first (ie. mfbhrb 0 is
* the most recent branch).
@@ -1210,7 +1214,7 @@ static void write_mmcr0(struct cpu_hw_events *cpuhw,
unsigned long mmcr0)
static void power_pmu_disable(struct pmu *pmu)
{
struct cpu_hw_events *cpuhw;
- unsigned long flags, mmcr0, val;
+ unsigned long flags, mmcr0, val, mmcra = 0;
if (!ppmu)
return;
@@ -1243,12 +1247,23 @@ static void power_pmu_disable(struct pmu *pmu)
mb();
isync();
+ val = mmcra = cpuhw->mmcr[2];
+
/*
* Disable instruction sampling if it was enabled
*/
- if (cpuhw->mmcr[2] & MMCRA_SAMPLE_ENABLE) {
- mtspr(SPRN_MMCRA,
- cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE);
+ if (cpuhw->mmcr[2] & MMCRA_SAMPLE_ENABLE)
+ mmcra = cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE;
+
+ /* Disable BHRB via mmcra [:26] for p10 if needed */
+ if (!(cpuhw->mmcr[2] & MMCRA_BHRB_DISABLE))
+ mmcra |= MMCRA_BHRB_DISABLE;
+
+ /* Write SPRN_MMCRA if mmcra has either disabled
+* instruction sampling or BHRB
+*/
+ if (val != mmcra) {
+ mtspr(SPRN_MMCRA, mmcra);
mb();
isync();
}
diff --git a/arch/powerpc/perf/isa207-common.c
b/arch/powerpc/perf/isa207-common.c
index 7d4839e..463d925 100644
--- a/arch/powerpc/perf/isa207-common.c
+++ b/arch/powerpc/perf/isa207-common.c
@@ -404,6 +404,12 @@ int isa207_compute_mmcr(u64 event[], int n_ev,
mmcra = mmcr1 = mmcr2 = mmcr3 = 0;
+ /* Disable bhrb unless explicitly requested
+* by setting MMCRA [:26] bit.
+*/
+ if (cpu_has_feature(CPU_FTR_ARCH_31))
+ mmcra |= MMCRA_BHRB_DISABLE;
+
/* Second pass: assign PMCs, set all MMCR1 fields */
for (i = 0; i < n_ev; ++i) {
pmc = (event[i] >> EVENT_PMC_SHIFT) & EVENT_PMC_MASK;
@@ -475,10 +481,17 @@ int isa207_compute_mmcr(u64 event[], int n_ev,
}
if (event[i] & EVENT_WANTS_BHRB) {
+ /* set MMCRA[:26] to 0 for Power10 to enable BHRB */
+ if (cpu_has_feature(CPU_FTR_ARCH_31))
+ mmcra &= ~MMCRA_BHRB_DISABLE;
val = (event[i] >> EVENT_IFM_SHIFT) & EVENT_IFM_MASK;
mmcra |= val
[PATCH 6/7] powerpc/perf: power10 Performance Monitoring support
Base enablement patch to register performance monitoring
hardware support for power10. Patch introduce the raw event
encoding format, defines the supported list of events, config
fields for the event attributes and their corresponding bit values
which are exported via sysfs.
Patch also enhances the support function in isa207_common.c to
include power10 pmu hardware.
[Enablement of base PMU driver code]
Signed-off-by: Madhavan Srinivasan
[Addition of ISA macros for counter support functions]
Signed-off-by: Athira Rajeev
---
arch/powerpc/perf/Makefile | 2 +-
arch/powerpc/perf/core-book3s.c | 2 +
arch/powerpc/perf/internal.h| 1 +
arch/powerpc/perf/isa207-common.c | 59 -
arch/powerpc/perf/isa207-common.h | 33 ++-
arch/powerpc/perf/power10-events-list.h | 81 ++
arch/powerpc/perf/power10-pmu.c | 422
7 files changed, 589 insertions(+), 11 deletions(-)
create mode 100644 arch/powerpc/perf/power10-events-list.h
create mode 100644 arch/powerpc/perf/power10-pmu.c
diff --git a/arch/powerpc/perf/Makefile b/arch/powerpc/perf/Makefile
index 53d614e..c02854d 100644
--- a/arch/powerpc/perf/Makefile
+++ b/arch/powerpc/perf/Makefile
@@ -9,7 +9,7 @@ obj-$(CONFIG_PPC_PERF_CTRS) += core-book3s.o bhrb.o
obj64-$(CONFIG_PPC_PERF_CTRS) += ppc970-pmu.o power5-pmu.o \
power5+-pmu.o power6-pmu.o power7-pmu.o \
isa207-common.o power8-pmu.o power9-pmu.o \
- generic-compat-pmu.o
+ generic-compat-pmu.o power10-pmu.o
obj32-$(CONFIG_PPC_PERF_CTRS) += mpc7450-pmu.o
obj-$(CONFIG_PPC_POWERNV) += imc-pmu.o
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 6de81d1..44c86a9 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2331,6 +2331,8 @@ static int __init init_ppc64_pmu(void)
return 0;
else if (!init_power9_pmu())
return 0;
+ else if (!init_power10_pmu())
+ return 0;
else if (!init_ppc970_pmu())
return 0;
else
diff --git a/arch/powerpc/perf/internal.h b/arch/powerpc/perf/internal.h
index f755c64..80bbf72 100644
--- a/arch/powerpc/perf/internal.h
+++ b/arch/powerpc/perf/internal.h
@@ -9,4 +9,5 @@
extern int init_power7_pmu(void);
extern int init_power8_pmu(void);
extern int init_power9_pmu(void);
+extern int init_power10_pmu(void);
extern int init_generic_compat_pmu(void);
diff --git a/arch/powerpc/perf/isa207-common.c
b/arch/powerpc/perf/isa207-common.c
index 4c86da5..7d4839e 100644
--- a/arch/powerpc/perf/isa207-common.c
+++ b/arch/powerpc/perf/isa207-common.c
@@ -55,7 +55,9 @@ static bool is_event_valid(u64 event)
{
u64 valid_mask = EVENT_VALID_MASK;
- if (cpu_has_feature(CPU_FTR_ARCH_300))
+ if (cpu_has_feature(CPU_FTR_ARCH_31))
+ valid_mask = p10_EVENT_VALID_MASK;
+ else if (cpu_has_feature(CPU_FTR_ARCH_300))
valid_mask = p9_EVENT_VALID_MASK;
return !(event & ~valid_mask);
@@ -69,6 +71,14 @@ static inline bool is_event_marked(u64 event)
return false;
}
+static unsigned long sdar_mod_val(u64 event)
+{
+ if (cpu_has_feature(CPU_FTR_ARCH_31))
+ return p10_SDAR_MODE(event);
+
+ return p9_SDAR_MODE(event);
+}
+
static void mmcra_sdar_mode(u64 event, unsigned long *mmcra)
{
/*
@@ -79,7 +89,7 @@ static void mmcra_sdar_mode(u64 event, unsigned long *mmcra)
* MMCRA[SDAR_MODE] will be programmed as "0b01" for continous sampling
* mode and will be un-changed when setting MMCRA[63] (Marked events).
*
-* Incase of Power9:
+* Incase of Power9/power10:
* Marked event: MMCRA[SDAR_MODE] will be set to 0b00 ('No Updates'),
* or if group already have any marked events.
* For rest
@@ -90,8 +100,8 @@ static void mmcra_sdar_mode(u64 event, unsigned long *mmcra)
if (cpu_has_feature(CPU_FTR_ARCH_300)) {
if (is_event_marked(event) || (*mmcra & MMCRA_SAMPLE_ENABLE))
*mmcra &= MMCRA_SDAR_MODE_NO_UPDATES;
- else if (p9_SDAR_MODE(event))
- *mmcra |= p9_SDAR_MODE(event) << MMCRA_SDAR_MODE_SHIFT;
+ else if (sdar_mod_val(event))
+ *mmcra |= sdar_mod_val(event) << MMCRA_SDAR_MODE_SHIFT;
else
*mmcra |= MMCRA_SDAR_MODE_DCACHE;
} else
@@ -134,7 +144,11 @@ static bool is_thresh_cmp_valid(u64 event)
/*
* Check the mantissa upper two bits are not zero, unless the
* exponent is also zero. See the THRESH_CMP_MANTISSA doc.
+* Power10: thresh_cmp is replaced by l2_l3 event select.
*/
+ if (cpu_has_feature(CPU_FTR_ARCH_31))
+
[PATCH 5/7] powerpc/perf: Update Power PMU cache_events to u64 type
Events of type PERF_TYPE_HW_CACHE was described for Power PMU
as: int (*cache_events)[type][op][result];
where type, op, result values unpacked from the event attribute config
value is used to generate the raw event code at runtime.
So far the event code values which used to create these cache-related
events were within 32 bit and `int` type worked. In power10,
some of the event codes are of 64-bit value and hence update the
Power PMU cache_events to `u64` type in `power_pmu` struct.
Also propagate this change to existing all PMU driver code paths
which are using ppmu->cache_events.
Signed-off-by: Athira Rajeev
---
arch/powerpc/include/asm/perf_event_server.h | 2 +-
arch/powerpc/perf/core-book3s.c | 2 +-
arch/powerpc/perf/generic-compat-pmu.c | 2 +-
arch/powerpc/perf/mpc7450-pmu.c | 2 +-
arch/powerpc/perf/power5+-pmu.c | 2 +-
arch/powerpc/perf/power5-pmu.c | 2 +-
arch/powerpc/perf/power6-pmu.c | 2 +-
arch/powerpc/perf/power7-pmu.c | 2 +-
arch/powerpc/perf/power8-pmu.c | 2 +-
arch/powerpc/perf/power9-pmu.c | 2 +-
arch/powerpc/perf/ppc970-pmu.c | 2 +-
11 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/perf_event_server.h
b/arch/powerpc/include/asm/perf_event_server.h
index 895aeaa..cb207f8 100644
--- a/arch/powerpc/include/asm/perf_event_server.h
+++ b/arch/powerpc/include/asm/perf_event_server.h
@@ -47,7 +47,7 @@ struct power_pmu {
const struct attribute_group**attr_groups;
int n_generic;
int *generic_events;
- int (*cache_events)[PERF_COUNT_HW_CACHE_MAX]
+ u64 (*cache_events)[PERF_COUNT_HW_CACHE_MAX]
[PERF_COUNT_HW_CACHE_OP_MAX]
[PERF_COUNT_HW_CACHE_RESULT_MAX];
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 9db72cd..6de81d1 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1818,7 +1818,7 @@ static void hw_perf_event_destroy(struct perf_event
*event)
static int hw_perf_cache_event(u64 config, u64 *eventp)
{
unsigned long type, op, result;
- int ev;
+ u64 ev;
if (!ppmu->cache_events)
return -EINVAL;
diff --git a/arch/powerpc/perf/generic-compat-pmu.c
b/arch/powerpc/perf/generic-compat-pmu.c
index 5e5a54d..eb8a6aaf 100644
--- a/arch/powerpc/perf/generic-compat-pmu.c
+++ b/arch/powerpc/perf/generic-compat-pmu.c
@@ -101,7 +101,7 @@ enum {
* 0 means not supported, -1 means nonsensical, other values
* are event codes.
*/
-static int generic_compat_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
+static u64 generic_compat_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
[ C(L1D) ] = {
[ C(OP_READ) ] = {
[ C(RESULT_ACCESS) ] = 0,
diff --git a/arch/powerpc/perf/mpc7450-pmu.c b/arch/powerpc/perf/mpc7450-pmu.c
index 4d5ef92..cf1eb89 100644
--- a/arch/powerpc/perf/mpc7450-pmu.c
+++ b/arch/powerpc/perf/mpc7450-pmu.c
@@ -354,7 +354,7 @@ static void mpc7450_disable_pmc(unsigned int pmc, unsigned
long mmcr[])
* 0 means not supported, -1 means nonsensical, other values
* are event codes.
*/
-static int mpc7450_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
+static u64 mpc7450_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
[C(L1D)] = {/* RESULT_ACCESS RESULT_MISS */
[C(OP_READ)] = {0, 0x225 },
[C(OP_WRITE)] = { 0, 0x227 },
diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c
index f857454..9252281 100644
--- a/arch/powerpc/perf/power5+-pmu.c
+++ b/arch/powerpc/perf/power5+-pmu.c
@@ -618,7 +618,7 @@ static void power5p_disable_pmc(unsigned int pmc, unsigned
long mmcr[])
* 0 means not supported, -1 means nonsensical, other values
* are event codes.
*/
-static int power5p_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
+static u64 power5p_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
[C(L1D)] = {/* RESULT_ACCESS RESULT_MISS */
[C(OP_READ)] = {0x1c10a8, 0x3c1088},
[C(OP_WRITE)] = { 0x2c10a8, 0xc10c3 },
diff --git a/arch/powerpc/perf/power5-pmu.c b/arch/powerpc/perf/power5-pmu.c
index da52eca..3b36630 100644
--- a/arch/powerpc/perf/power5-pmu.c
+++ b/arch/powerpc/perf/power5-pmu.c
@@ -560,7 +560,7 @@ static void power5_disable_pmc(unsigned int pmc, unsigned
long mmcr[])
* 0 means not supported, -1 means nonsensical, other values
* are event codes.
*/
-static int power5_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
+static u64 power5_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
[C(L1D)] = {/* RESULT_ACCESS RESULT_MISS
[PATCH 4/7] powerpc/perf: Add power10_feat to dt_cpu_ftrs
From: Madhavan Srinivasan
Add power10 feature function to dt_cpu_ftrs.c along
with a power10 specific init() to initialize pmu sprs.
Signed-off-by: Madhavan Srinivasan
---
arch/powerpc/include/asm/reg.h| 3 +++
arch/powerpc/kernel/cpu_setup_power.S | 7 +++
arch/powerpc/kernel/dt_cpu_ftrs.c | 26 ++
3 files changed, 36 insertions(+)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 21a1b2d..900ada1 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -1068,6 +1068,9 @@
#define MMCR0_PMC2_LOADMISSTIME0x5
#endif
+/* BHRB disable bit for PowerISA v3.10 */
+#define MMCRA_BHRB_DISABLE 0x0020
+
/*
* SPRG usage:
*
diff --git a/arch/powerpc/kernel/cpu_setup_power.S
b/arch/powerpc/kernel/cpu_setup_power.S
index efdcfa7..e8b3370c 100644
--- a/arch/powerpc/kernel/cpu_setup_power.S
+++ b/arch/powerpc/kernel/cpu_setup_power.S
@@ -233,3 +233,10 @@ __init_PMU_ISA207:
li r5,0
mtspr SPRN_MMCRS,r5
blr
+
+__init_PMU_ISA31:
+ li r5,0
+ mtspr SPRN_MMCR3,r5
+ LOAD_REG_IMMEDIATE(r5, MMCRA_BHRB_DISABLE)
+ mtspr SPRN_MMCRA,r5
+ blr
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c
b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 3a40951..f482286 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -450,6 +450,31 @@ static int __init feat_enable_pmu_power9(struct
dt_cpu_feature *f)
return 1;
}
+static void init_pmu_power10(void)
+{
+ init_pmu_power9();
+
+ mtspr(SPRN_MMCR3, 0);
+ mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);
+}
+
+static int __init feat_enable_pmu_power10(struct dt_cpu_feature *f)
+{
+ hfscr_pmu_enable();
+
+ init_pmu_power10();
+ init_pmu_registers = init_pmu_power10;
+
+ cur_cpu_spec->cpu_features |= CPU_FTR_MMCRA;
+ cur_cpu_spec->cpu_user_features |= PPC_FEATURE_PSERIES_PERFMON_COMPAT;
+
+ cur_cpu_spec->num_pmcs = 6;
+ cur_cpu_spec->pmc_type = PPC_PMC_IBM;
+ cur_cpu_spec->oprofile_cpu_type = "ppc64/power10";
+
+ return 1;
+}
+
static int __init feat_enable_tm(struct dt_cpu_feature *f)
{
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
@@ -639,6 +664,7 @@ struct dt_cpu_feature_match {
{"pc-relative-addressing", feat_enable, 0},
{"machine-check-power9", feat_enable_mce_power9, 0},
{"performance-monitor-power9", feat_enable_pmu_power9, 0},
+ {"performance-monitor-power10", feat_enable_pmu_power10, 0},
{"event-based-branch-v3", feat_enable, 0},
{"random-number-generator", feat_enable, 0},
{"system-call-vectored", feat_disable, 0},
--
1.8.3.1
[PATCH 2/7] KVM: PPC: Book3S HV: Save/restore new PMU registers
PowerISA v3.1 has added new performance monitoring unit (PMU)
special purpose registers (SPRs). They are
Monitor Mode Control Register 3 (MMCR3)
Sampled Instruction Event Register A (SIER2)
Sampled Instruction Event Register B (SIER3)
Patch addes support to save/restore these new
SPRs while entering/exiting guest.
Signed-off-by: Athira Rajeev
---
arch/powerpc/include/asm/kvm_book3s_asm.h | 2 +-
arch/powerpc/include/asm/kvm_host.h | 4 ++--
arch/powerpc/kernel/asm-offsets.c | 3 +++
arch/powerpc/kvm/book3s_hv.c | 6 --
arch/powerpc/kvm/book3s_hv_interrupts.S | 8
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 24
6 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s_asm.h
b/arch/powerpc/include/asm/kvm_book3s_asm.h
index 45704f2..078f464 100644
--- a/arch/powerpc/include/asm/kvm_book3s_asm.h
+++ b/arch/powerpc/include/asm/kvm_book3s_asm.h
@@ -119,7 +119,7 @@ struct kvmppc_host_state {
void __iomem *xive_tima_virt;
u32 saved_xirr;
u64 dabr;
- u64 host_mmcr[7]; /* MMCR 0,1,A, SIAR, SDAR, MMCR2, SIER */
+ u64 host_mmcr[10]; /* MMCR 0,1,A, SIAR, SDAR, MMCR2, SIER, MMCR3,
SIER2/3 */
u32 host_pmc[8];
u64 host_purr;
u64 host_spurr;
diff --git a/arch/powerpc/include/asm/kvm_host.h
b/arch/powerpc/include/asm/kvm_host.h
index 1dc6310..8bc2122 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -637,12 +637,12 @@ struct kvm_vcpu_arch {
u32 ccr1;
u32 dbsr;
- u64 mmcr[5];
+ u64 mmcr[6];
u32 pmc[8];
u32 spmc[2];
u64 siar;
u64 sdar;
- u64 sier;
+ u64 sier[3];
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
u64 tfhar;
u64 texasr;
diff --git a/arch/powerpc/kernel/asm-offsets.c
b/arch/powerpc/kernel/asm-offsets.c
index 9b9cde0..3ec3f37 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -697,6 +697,9 @@ int main(void)
HSTATE_FIELD(HSTATE_SDAR, host_mmcr[4]);
HSTATE_FIELD(HSTATE_MMCR2, host_mmcr[5]);
HSTATE_FIELD(HSTATE_SIER, host_mmcr[6]);
+ HSTATE_FIELD(HSTATE_MMCR3, host_mmcr[7]);
+ HSTATE_FIELD(HSTATE_SIER2, host_mmcr[8]);
+ HSTATE_FIELD(HSTATE_SIER3, host_mmcr[9]);
HSTATE_FIELD(HSTATE_PMC1, host_pmc[0]);
HSTATE_FIELD(HSTATE_PMC2, host_pmc[1]);
HSTATE_FIELD(HSTATE_PMC3, host_pmc[2]);
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index a0cf175..9e3840b 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1695,7 +1695,8 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu,
u64 id,
*val = get_reg_val(id, vcpu->arch.sdar);
break;
case KVM_REG_PPC_SIER:
- *val = get_reg_val(id, vcpu->arch.sier);
+ i = id - KVM_REG_PPC_SIER;
+ *val = get_reg_val(id, vcpu->arch.sier[i]);
break;
case KVM_REG_PPC_IAMR:
*val = get_reg_val(id, vcpu->arch.iamr);
@@ -1916,7 +1917,8 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu,
u64 id,
vcpu->arch.sdar = set_reg_val(id, *val);
break;
case KVM_REG_PPC_SIER:
- vcpu->arch.sier = set_reg_val(id, *val);
+ i = id - KVM_REG_PPC_SIER;
+ vcpu->arch.sier[i] = set_reg_val(id, *val);
break;
case KVM_REG_PPC_IAMR:
vcpu->arch.iamr = set_reg_val(id, *val);
diff --git a/arch/powerpc/kvm/book3s_hv_interrupts.S
b/arch/powerpc/kvm/book3s_hv_interrupts.S
index 63fd81f..59822cb 100644
--- a/arch/powerpc/kvm/book3s_hv_interrupts.S
+++ b/arch/powerpc/kvm/book3s_hv_interrupts.S
@@ -140,6 +140,14 @@ BEGIN_FTR_SECTION
std r8, HSTATE_MMCR2(r13)
std r9, HSTATE_SIER(r13)
END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
+BEGIN_FTR_SECTION
+ mfspr r5, SPRN_MMCR3
+ mfspr r6, SPRN_SIER2
+ mfspr r7, SPRN_SIER3
+ std r5, HSTATE_MMCR3(r13)
+ std r6, HSTATE_SIER2(r13)
+ std r7, HSTATE_SIER3(r13)
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_31)
mfspr r3, SPRN_PMC1
mfspr r5, SPRN_PMC2
mfspr r6, SPRN_PMC3
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 7194389..57b6c14 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -3436,6 +3436,14 @@ END_FTR_SECTION_IFSET(CPU_FTR_PMAO_BUG)
mtspr SPRN_SIAR, r7
mtspr SPRN_SDAR, r8
BEGIN_FTR_SECTION
+ ld r5, VCPU_MMCR + 40(r4)
+ ld r6, VCPU_SIER + 8(r4)
+ ld r7, VCPU_SIER + 16(r4)
+ mtspr SPRN_MMCR3, r5
+ mtspr SPRN_SIER2, r6
+ mtspr SPRN_SIER3, r7
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_31)
+BEGIN_FTR_SECTION
[PATCH 3/7] powerpc/xmon: Add PowerISA v3.1 PMU SPRs
From: Madhavan Srinivasan
PowerISA v3.1 added three new perfromance
monitoring unit (PMU) speical purpose register (SPR).
They are Monitor Mode Control Register 3 (MMCR3),
Sampled Instruction Event Register 2 (SIER2),
Sampled Instruction Event Register 3 (SIER3).
Patch here adds a new dump function dump_310_sprs
to print these SPR values.
Signed-off-by: Madhavan Srinivasan
---
arch/powerpc/xmon/xmon.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index b34d703..6fd5f4e 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -2023,6 +2023,20 @@ static void dump_300_sprs(void)
#endif
}
+static void dump_310_sprs(void)
+{
+#ifdef CONFIG_PPC64
+ if (!cpu_has_feature(CPU_FTR_ARCH_31))
+ return;
+
+ printf("mmcr3 = %.16lx\n",
+ mfspr(SPRN_MMCR3));
+
+ printf("sier2 = %.16lx sier3 = %.16lx\n",
+ mfspr(SPRN_SIER2), mfspr(SPRN_SIER3));
+#endif
+}
+
static void dump_one_spr(int spr, bool show_unimplemented)
{
unsigned long val;
@@ -2077,6 +2091,7 @@ static void super_regs(void)
dump_206_sprs();
dump_207_sprs();
dump_300_sprs();
+ dump_310_sprs();
return;
}
--
1.8.3.1
[PATCH 1/7] powerpc/perf: Add support for ISA3.1 PMU SPRs
From: Madhavan Srinivasan
PowerISA v3.1 includes new performance monitoring unit(PMU)
special purpose registers (SPRs). They are
Monitor Mode Control Register 3 (MMCR3)
Sampled Instruction Event Register 2 (SIER2)
Sampled Instruction Event Register 3 (SIER3)
MMCR3 is added for further sampling related configuration
control. SIER2/SIER3 are added to provide additional
information about the sampled instruction.
Patch adds new PPMU flag called "PPMU_ARCH_310S" to support
handling of these new SPRs, updates the struct thread_struct
to include these new SPRs, increase the size of mmcr[] array
by one to include MMCR3 in struct cpu_hw_event. This is needed
to support programming of MMCR3 SPR during event_[enable/disable].
Patch also adds the sysfs support for the MMCR3 SPR along with
SPRN_ macros for these new pmu sprs.
Signed-off-by: Madhavan Srinivasan
---
arch/powerpc/include/asm/perf_event_server.h | 1 +
arch/powerpc/include/asm/processor.h | 4
arch/powerpc/include/asm/reg.h | 6 ++
arch/powerpc/kernel/sysfs.c | 8
arch/powerpc/perf/core-book3s.c | 29 ++--
5 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/perf_event_server.h
b/arch/powerpc/include/asm/perf_event_server.h
index 3e9703f..895aeaa 100644
--- a/arch/powerpc/include/asm/perf_event_server.h
+++ b/arch/powerpc/include/asm/perf_event_server.h
@@ -69,6 +69,7 @@ struct power_pmu {
#define PPMU_HAS_SIER 0x0040 /* Has SIER */
#define PPMU_ARCH_207S 0x0080 /* PMC is architecture v2.07S */
#define PPMU_NO_SIAR 0x0100 /* Do not use SIAR */
+#define PPMU_ARCH_310S 0x0200 /* Has MMCR3, SIER2 and SIER3 */
/*
* Values for flags to get_alternatives()
diff --git a/arch/powerpc/include/asm/processor.h
b/arch/powerpc/include/asm/processor.h
index 52a6783..a466e94 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -272,6 +272,10 @@ struct thread_struct {
unsignedmmcr0;
unsignedused_ebb;
+ unsigned long mmcr3;
+ unsigned long sier2;
+ unsigned long sier3;
+
#endif
};
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 88e6c78..21a1b2d 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -876,7 +876,9 @@
#define MMCR0_FCHV 0x0001UL /* freeze conditions in hypervisor mode */
#define SPRN_MMCR1 798
#define SPRN_MMCR2 785
+#define SPRN_MMCR3 754
#define SPRN_UMMCR2769
+#define SPRN_UMMCR3738
#define SPRN_MMCRA 0x312
#define MMCRA_SDSYNC 0x8000UL /* SDAR synced with SIAR */
#define MMCRA_SDAR_DCACHE_MISS 0x4000UL
@@ -918,6 +920,10 @@
#define SIER_SIHV0x100 /* Sampled MSR_HV */
#define SIER_SIAR_VALID 0x040 /* SIAR contents valid */
#define SIER_SDAR_VALID 0x020 /* SDAR contents valid */
+#define SPRN_SIER2 752
+#define SPRN_SIER3 753
+#define SPRN_USIER2736
+#define SPRN_USIER3737
#define SPRN_SIAR 796
#define SPRN_SDAR 797
#define SPRN_TACR 888
diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index 571b325..46b4ebc 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -622,8 +622,10 @@ void ppc_enable_pmcs(void)
SYSFS_PMCSETUP(pmc8, SPRN_PMC8);
SYSFS_PMCSETUP(mmcra, SPRN_MMCRA);
+SYSFS_PMCSETUP(mmcr3, SPRN_MMCR3);
static DEVICE_ATTR(mmcra, 0600, show_mmcra, store_mmcra);
+static DEVICE_ATTR(mmcr3, 0600, show_mmcr3, store_mmcr3);
#endif /* HAS_PPC_PMC56 */
@@ -886,6 +888,9 @@ static int register_cpu_online(unsigned int cpu)
#ifdef CONFIG_PMU_SYSFS
if (cpu_has_feature(CPU_FTR_MMCRA))
device_create_file(s, _attr_mmcra);
+
+ if (cpu_has_feature(CPU_FTR_ARCH_31))
+ device_create_file(s, _attr_mmcr3);
#endif /* CONFIG_PMU_SYSFS */
if (cpu_has_feature(CPU_FTR_PURR)) {
@@ -980,6 +985,9 @@ static int unregister_cpu_online(unsigned int cpu)
#ifdef CONFIG_PMU_SYSFS
if (cpu_has_feature(CPU_FTR_MMCRA))
device_remove_file(s, _attr_mmcra);
+
+ if (cpu_has_feature(CPU_FTR_ARCH_31))
+ device_remove_file(s, _attr_mmcr3);
#endif /* CONFIG_PMU_SYSFS */
if (cpu_has_feature(CPU_FTR_PURR)) {
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 13b9dd5..9db72cd 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -39,10 +39,10 @@ struct cpu_hw_events {
unsigned int flags[MAX_HWEVENTS];
/*
* The order of the MMCR array is:
-* - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2
+* - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2, MMCR3
* - 32-bit, MMCR0, MMCR1, MMCR2
*/
- unsigned long mmcr[4];
+ unsigned
[PATCH 0/7] powerpc/perf: Add support for power10 PMU Hardware
The patch series adds support for power10 PMU hardware. And code changes are based on powerpc/next. Athira Rajeev (4): KVM: PPC: Book3S HV: Save/restore new PMU registers powerpc/perf: Update Power PMU cache_events to u64 type powerpc/perf: power10 Performance Monitoring support powerpc/perf: support BHRB disable bit and new filtering modes Madhavan Srinivasan (3): powerpc/perf: Add support for ISA3.1 PMU SPRs powerpc/xmon: Add PowerISA v3.1 PMU SPRs powerpc/perf: Add power10_feat to dt_cpu_ftrs arch/powerpc/include/asm/kvm_book3s_asm.h| 2 +- arch/powerpc/include/asm/kvm_host.h | 4 +- arch/powerpc/include/asm/perf_event_server.h | 3 +- arch/powerpc/include/asm/processor.h | 4 + arch/powerpc/include/asm/reg.h | 9 + arch/powerpc/kernel/asm-offsets.c| 3 + arch/powerpc/kernel/cpu_setup_power.S| 7 + arch/powerpc/kernel/dt_cpu_ftrs.c| 26 ++ arch/powerpc/kernel/sysfs.c | 8 + arch/powerpc/kvm/book3s_hv.c | 6 +- arch/powerpc/kvm/book3s_hv_interrupts.S | 8 + arch/powerpc/kvm/book3s_hv_rmhandlers.S | 24 ++ arch/powerpc/perf/Makefile | 2 +- arch/powerpc/perf/core-book3s.c | 60 +++- arch/powerpc/perf/generic-compat-pmu.c | 2 +- arch/powerpc/perf/internal.h | 1 + arch/powerpc/perf/isa207-common.c| 72 - arch/powerpc/perf/isa207-common.h| 33 +- arch/powerpc/perf/mpc7450-pmu.c | 2 +- arch/powerpc/perf/power10-events-list.h | 81 + arch/powerpc/perf/power10-pmu.c | 431 +++ arch/powerpc/perf/power5+-pmu.c | 2 +- arch/powerpc/perf/power5-pmu.c | 2 +- arch/powerpc/perf/power6-pmu.c | 2 +- arch/powerpc/perf/power7-pmu.c | 2 +- arch/powerpc/perf/power8-pmu.c | 2 +- arch/powerpc/perf/power9-pmu.c | 2 +- arch/powerpc/perf/ppc970-pmu.c | 2 +- arch/powerpc/platforms/powernv/idle.c| 14 + arch/powerpc/xmon/xmon.c | 15 + 30 files changed, 796 insertions(+), 35 deletions(-) create mode 100644 arch/powerpc/perf/power10-events-list.h create mode 100644 arch/powerpc/perf/power10-pmu.c -- 1.8.3.1
[PATCH] tpm: ibmvtpm: Wait for ready buffer before probing for TPM2 attributes
The tpm2_get_cc_attrs_tbl() call will result in TPM commands being issued,
which will need the use of the internal command/response buffer. But,
we're issuing this *before* we've waited to make sure that buffer is
allocated.
This can result in intermittent failures to probe if the hypervisor / TPM
implementation doesn't respond quickly enough. I find it fails almost
every time with an 8 vcpu guest under KVM with software emulated TPM.
Fixes: 18b3670d79ae9 "tpm: ibmvtpm: Add support for TPM2"
Signed-off-by: David Gibson
---
drivers/char/tpm/tpm_ibmvtpm.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 09fe45246b8c..994385bf37c0 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -683,13 +683,6 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
if (rc)
goto init_irq_cleanup;
- if (!strcmp(id->compat, "IBM,vtpm20")) {
- chip->flags |= TPM_CHIP_FLAG_TPM2;
- rc = tpm2_get_cc_attrs_tbl(chip);
- if (rc)
- goto init_irq_cleanup;
- }
-
if (!wait_event_timeout(ibmvtpm->crq_queue.wq,
ibmvtpm->rtce_buf != NULL,
HZ)) {
@@ -697,6 +690,13 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
goto init_irq_cleanup;
}
+ if (!strcmp(id->compat, "IBM,vtpm20")) {
+ chip->flags |= TPM_CHIP_FLAG_TPM2;
+ rc = tpm2_get_cc_attrs_tbl(chip);
+ if (rc)
+ goto init_irq_cleanup;
+ }
+
return tpm_chip_register(chip);
init_irq_cleanup:
do {
--
2.26.2
