LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] powerpc: Use common error handling code in setup_new_fdt()
From: Dan Carpenter @ 2018-03-15 11:57 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: SF Markus Elfring, linuxppc-dev, Benjamin Herrenschmidt,
	Borislav Petkov, Brijesh Singh, Josh Sklar, Kees Cook,
	Michael Ellerman, Paul Mackerras, Thomas Gleixner, Tom Lendacky,
	LKML, kernel-janitors
In-Reply-To: <87fu52jqtc.fsf@morokweng.localdomain>

On Wed, Mar 14, 2018 at 06:22:07PM -0300, Thiago Jung Bauermann wrote:
> 
> SF Markus Elfring <elfring@users.sourceforge.net> writes:
> 
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Sun, 11 Mar 2018 09:03:42 +0100
> >
> > Add a jump target so that a bit of exception handling can be better reused
> > at the end of this function.
> >
> > This issue was detected by using the Coccinelle software.
> >
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> >  arch/powerpc/kernel/machine_kexec_file_64.c | 28 ++++++++++++----------------
> >  1 file changed, 12 insertions(+), 16 deletions(-)
> 
> I liked it. Thanks!
> 
> Reviewed-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> 

You know that compilers already re-use string constants so this doesn't
actually save memory?  Also we should be preserving the error codes
instead of always returning -EINVAL.

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH v3] x86: treat pkey-0 special
From: Dave Hansen @ 2018-03-15 15:55 UTC (permalink / raw)
  To: Thomas Gleixner, Ram Pai
  Cc: mingo, mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-kernel,
	akpm, benh, paulus, khandual, aneesh.kumar, bsingharora, hbabu,
	mhocko, bauerman, ebiederm, corbet, arnd, fweimer, msuchanek,
	Ulrich.Weigand
In-Reply-To: <alpine.DEB.2.21.1803151039430.1525@nanos.tec.linutronix.de>

On 03/15/2018 02:46 AM, Thomas Gleixner wrote:
>> +	if (!pkey || !mm_pkey_is_allocated(mm, pkey))
> Why this extra check? mm_pkey_is_allocated(mm, 0) should not return true
> ever. If it does, then this wants to be fixed.

I was thinking that we _do_ actually want it to seem allocated.  It just
get "allocated" implicitly when an mm is created.  I think that will
simplify the code if we avoid treating it specially in as many places as
possible.

^ permalink raw reply

* Re: [PATCH v3] x86: treat pkey-0 special
From: Thomas Gleixner @ 2018-03-15 16:13 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Ram Pai, mingo, mpe, linuxppc-dev, linux-mm, x86, linux-arch,
	linux-kernel, akpm, benh, paulus, khandual, aneesh.kumar,
	bsingharora, hbabu, mhocko, bauerman, ebiederm, corbet, arnd,
	fweimer, msuchanek, Ulrich.Weigand
In-Reply-To: <f5ef79ef-122a-e0a3-9b8e-d49c33f4a417@intel.com>

On Thu, 15 Mar 2018, Dave Hansen wrote:

> On 03/15/2018 02:46 AM, Thomas Gleixner wrote:
> >> +	if (!pkey || !mm_pkey_is_allocated(mm, pkey))
> > Why this extra check? mm_pkey_is_allocated(mm, 0) should not return true
> > ever. If it does, then this wants to be fixed.
> 
> I was thinking that we _do_ actually want it to seem allocated.  It just
> get "allocated" implicitly when an mm is created.  I think that will
> simplify the code if we avoid treating it specially in as many places as
> possible.

That works as well.

^ permalink raw reply

* Re: [PATCH v8 0/6] add support for relative references in special sections
From: Ard Biesheuvel @ 2018-03-15 16:29 UTC (permalink / raw)
  To: linux-arm-kernel, Andrew Morton
  Cc: Ard Biesheuvel, Arnd Bergmann, Kees Cook, Will Deacon,
	Michael Ellerman, Thomas Garnier, Thomas Gleixner,
	Serge E. Hallyn, Bjorn Helgaas, Benjamin Herrenschmidt,
	Russell King, Paul Mackerras, Catalin Marinas, Petr Mladek,
	Ingo Molnar, Nicolas Pitre, Josh Poimboeuf, Steven Rostedt,
	Sergey Senozhatsky, Linus Torvalds, Jessica Yu,
	Linux Kernel Mailing List, linuxppc-dev, the arch/x86 maintainers
In-Reply-To: <20180311123815.17916-1-ard.biesheuvel@linaro.org>

On 11 March 2018 at 12:38, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> This adds support for emitting special sections such as initcall arrays,
> PCI fixups and tracepoints as relative references rather than absolute
> references. This reduces the size by 50% on 64-bit architectures, but
> more importantly, it removes the need for carrying relocation metadata
> for these sections in relocatable kernels (e.g., for KASLR) that needs
> to be fixed up at boot time. On arm64, this reduces the vmlinux footprint
> of such a reference by 8x (8 byte absolute reference + 24 byte RELA entry
> vs 4 byte relative reference)
>
> Patch #3 was sent out before as a single patch. This series supersedes
> the previous submission. This version makes relative ksymtab entries
> dependent on the new Kconfig symbol HAVE_ARCH_PREL32_RELOCATIONS rather
> than trying to infer from kbuild test robot replies for which architectures
> it should be blacklisted.
>
> Patch #1 introduces the new Kconfig symbol HAVE_ARCH_PREL32_RELOCATIONS,
> and sets it for the main architectures that are expected to benefit the
> most from this feature, i.e., 64-bit architectures or ones that use
> runtime relocations.
>
> Patch #2 add support for #define'ing __DISABLE_EXPORTS to get rid of
> ksymtab/kcrctab sections in decompressor and EFI stub objects when
> rebuilding existing C files to run in a different context.
>
> Patches #4 - #6 implement relative references for initcalls, PCI fixups
> and tracepoints, respectively, all of which produce sections with order
> ~1000 entries on an arm64 defconfig kernel with tracing enabled. This
> means we save about 28 KB of vmlinux space for each of these patches.
>
> [From the v7 series blurb, which included the jump_label patches as well]:
>   For the arm64 kernel, all patches combined reduce the memory footprint of
>   vmlinux by about 1.3 MB (using a config copied from Ubuntu that has KASLR
>   enabled), of which ~1 MB is the size reduction of the RELA section in .init,
>   and the remaining 300 KB is reduction of .text/.data.
>
> Branch:
> git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git relative-special-sections-v8
>

All,

I think this has converged to a mergeable state now (taking the
mini-tweak for patch #3 into account I provided in a reply)

Are there any objections to this being merged?

Andrew,

If nobody objects, is this something you would care to pick up for the
next cycle? Do I need to respin to incorporate the mini-tweak and
apply Nico's ack to patch #2?

Thanks,
Ard.


> Changes since v7:
> - dropped the jump_label patches, these will be revisited in a separate series
> - reorder __DISABLE_EXPORTS with __KSYM_DEPS__ check in #2
> - use offset_to_ptr() helper function to abstract the relative pointer
>   conversion [int *off -> (ulong)off + *off] (#3 - #6)
> - rebase onto v4.16-rc3
>
> Changes since v6:
> - drop S390 from patch #1 introducing HAVE_ARCH_PREL32_RELOCATIONS: kbuild
>   robot threw me some s390 curveballs, and given that s390 does not define
>   CONFIG_RELOCATABLE in the first place, it does not benefit as much from
>   relative references as arm64, x86 and power do
> - add patch to allow symbol exports to be disabled at compilation unit
>   granularity (#2)
> - get rid of arm64 vmlinux.lds.S hunk to ensure code generated by __ADDRESSABLE
>   gets discarded from the EFI stub - it is no longer needed after adding #2 (#1)
> - change _ADDRESSABLE() to emit a data reference, not a code reference - this
>   is another simplification made possible by patch #2 (#3)
> - add Steven's ack to #6
> - split x86 jump_label patch into two (#9, #10)
>
> Changes since v5:
> - add missing jump_label prototypes to s390 jump_label.h (#6)
> - fix inverted condition in call to jump_entry_is_module_init() (#6)
>
> Changes since v4:
> - add patches to convert x86 and arm64 to use relative references for jump
>   tables (#6 - #8)
> - rename PCI patch and add Bjorn's ack (#4)
> - rebase onto v4.15-rc5
>
> Changes since v3:
> - fix module unload issue in patch #5 reported by Jessica, by reusing the
>   updated routine for_each_tracepoint_range() for the quiescent check at
>   module unload time; this requires this routine to be moved before
>   tracepoint_module_going() in kernel/tracepoint.c
> - add Jessica's ack to #2
> - rebase onto v4.14-rc1
>
> Changes since v2:
> - Revert my slightly misguided attempt to appease checkpatch, which resulted
>   in needless churn and worse code. This v3 is based on v1 with a few tweaks
>   that were actually reasonable checkpatch warnings: unnecessary braces (as
>   pointed out by Ingo) and other minor whitespace misdemeanors.
>
> Changes since v1:
> - Remove checkpatch errors to the extent feasible: in some cases, this
>   involves moving extern declarations into C files, and switching to
>   struct definitions rather than typedefs. Some errors are impossible
>   to fix: please find the remaining ones after the diffstat.
> - Used 'int' instead if 'signed int' for the various offset fields: there
>   is no ambiguity between architectures regarding its signedness (unlike
>   'char')
> - Refactor the different patches to be more uniform in the way they define
>   the section entry type and accessors in the .h file, and avoid the need to
>   add #ifdefs to the C code.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Thomas Garnier <thgarnie@google.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: James Morris <james.l.morris@oracle.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Nicolas Pitre <nico@linaro.org>
> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Jessica Yu <jeyu@kernel.org>
>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: x86@kernel.org
>
> Ard Biesheuvel (6):
>   arch: enable relative relocations for arm64, power and x86
>   module: allow symbol exports to be disabled
>   module: use relative references for __ksymtab entries
>   init: allow initcall tables to be emitted using relative references
>   PCI: Add support for relative addressing in quirk tables
>   kernel: tracepoints: add support for relative references
>
>  arch/Kconfig                          | 10 ++++
>  arch/arm64/Kconfig                    |  1 +
>  arch/powerpc/Kconfig                  |  1 +
>  arch/x86/Kconfig                      |  1 +
>  arch/x86/boot/compressed/kaslr.c      |  5 +-
>  arch/x86/include/asm/Kbuild           |  1 +
>  arch/x86/include/asm/export.h         |  5 --
>  drivers/firmware/efi/libstub/Makefile |  3 +-
>  drivers/pci/quirks.c                  | 12 +++--
>  include/asm-generic/export.h          | 12 ++++-
>  include/linux/compiler.h              | 19 +++++++
>  include/linux/export.h                | 57 +++++++++++++++-----
>  include/linux/init.h                  | 44 +++++++++++----
>  include/linux/pci.h                   | 20 +++++++
>  include/linux/tracepoint.h            | 19 +++++--
>  init/main.c                           | 32 +++++------
>  kernel/module.c                       | 32 ++++++++---
>  kernel/printk/printk.c                |  4 +-
>  kernel/tracepoint.c                   | 49 +++++++++--------
>  security/security.c                   |  4 +-
>  20 files changed, 240 insertions(+), 91 deletions(-)
>  delete mode 100644 arch/x86/include/asm/export.h
>
> --
> 2.15.1
>

^ permalink raw reply

* Re: [PATCH v3] x86: treat pkey-0 special
From: Ram Pai @ 2018-03-15 17:21 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Thomas Gleixner, mingo, mpe, linuxppc-dev, linux-mm, x86,
	linux-arch, linux-kernel, akpm, benh, paulus, khandual,
	aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
	corbet, arnd, fweimer, msuchanek, Ulrich.Weigand
In-Reply-To: <f5ef79ef-122a-e0a3-9b8e-d49c33f4a417@intel.com>

On Thu, Mar 15, 2018 at 08:55:31AM -0700, Dave Hansen wrote:
> On 03/15/2018 02:46 AM, Thomas Gleixner wrote:
> >> +	if (!pkey || !mm_pkey_is_allocated(mm, pkey))
> > Why this extra check? mm_pkey_is_allocated(mm, 0) should not return true
> > ever. If it does, then this wants to be fixed.
> 
> I was thinking that we _do_ actually want it to seem allocated.  It just
> get "allocated" implicitly when an mm is created.  I think that will
> simplify the code if we avoid treating it specially in as many places as
> possible.

I think, the logic that makes pkey-0 special must to go
in arch-neutral code.   How about checking for pkey-0 in sys_pkey_free()
itself?

RP

^ permalink raw reply

* Re: [PATCH v3] x86: treat pkey-0 special
From: Dave Hansen @ 2018-03-15 17:31 UTC (permalink / raw)
  To: Ram Pai
  Cc: Thomas Gleixner, mingo, mpe, linuxppc-dev, linux-mm, x86,
	linux-arch, linux-kernel, akpm, benh, paulus, khandual,
	aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
	corbet, arnd, fweimer, msuchanek, Ulrich.Weigand
In-Reply-To: <20180315172129.GD1060@ram.oc3035372033.ibm.com>

On 03/15/2018 10:21 AM, Ram Pai wrote:
> On Thu, Mar 15, 2018 at 08:55:31AM -0700, Dave Hansen wrote:
>> On 03/15/2018 02:46 AM, Thomas Gleixner wrote:
>>>> +	if (!pkey || !mm_pkey_is_allocated(mm, pkey))
>>> Why this extra check? mm_pkey_is_allocated(mm, 0) should not return true
>>> ever. If it does, then this wants to be fixed.
>> I was thinking that we _do_ actually want it to seem allocated.  It just
>> get "allocated" implicitly when an mm is created.  I think that will
>> simplify the code if we avoid treating it specially in as many places as
>> possible.
> I think, the logic that makes pkey-0 special must to go
> in arch-neutral code.   How about checking for pkey-0 in sys_pkey_free()
> itself?

This is for protection against shooting yourself in the foot?  Yes, that
can go in sys_pkey_free().

Does this need manpage and/or selftests updates?

^ permalink raw reply

* Re: [PATCH v3] x86: treat pkey-0 special
From: Ram Pai @ 2018-03-15 17:39 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Thomas Gleixner, mingo, mpe, linuxppc-dev, linux-mm, x86,
	linux-arch, linux-kernel, akpm, benh, paulus, khandual,
	aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
	corbet, arnd, fweimer, msuchanek, Ulrich.Weigand
In-Reply-To: <2bf8e659-5a8d-a2d5-ea52-e4d395ea2201@intel.com>

On Thu, Mar 15, 2018 at 10:31:51AM -0700, Dave Hansen wrote:
> On 03/15/2018 10:21 AM, Ram Pai wrote:
> > On Thu, Mar 15, 2018 at 08:55:31AM -0700, Dave Hansen wrote:
> >> On 03/15/2018 02:46 AM, Thomas Gleixner wrote:
> >>>> +	if (!pkey || !mm_pkey_is_allocated(mm, pkey))
> >>> Why this extra check? mm_pkey_is_allocated(mm, 0) should not return true
> >>> ever. If it does, then this wants to be fixed.
> >> I was thinking that we _do_ actually want it to seem allocated.  It just
> >> get "allocated" implicitly when an mm is created.  I think that will
> >> simplify the code if we avoid treating it specially in as many places as
> >> possible.
> > I think, the logic that makes pkey-0 special must to go
> > in arch-neutral code.   How about checking for pkey-0 in sys_pkey_free()
> > itself?
> 
> This is for protection against shooting yourself in the foot?  Yes, that
> can go in sys_pkey_free().
> 
> Does this need manpage and/or selftests updates?

Yes. it needs selftest, manpage and documentation updates too.

Unfortunately I am not getting enough reviewed-by for my selftests
and documentation changes. :-(  Need help!


-- 
Ram Pai

^ permalink raw reply

* Re: [PATCH] powerpc: Use common error handling code in setup_new_fdt()
From: Thiago Jung Bauermann @ 2018-03-15 18:34 UTC (permalink / raw)
  To: Joe Perches
  Cc: Dan Carpenter, SF Markus Elfring, linuxppc-dev,
	Benjamin Herrenschmidt, Borislav Petkov, Brijesh Singh,
	Josh Sklar, Kees Cook, Michael Ellerman, Paul Mackerras,
	Thomas Gleixner, Tom Lendacky, LKML, kernel-janitors
In-Reply-To: <1521126196.22221.10.camel@perches.com>


Joe Perches <joe@perches.com> writes:

> On Thu, 2018-03-15 at 14:57 +0300, Dan Carpenter wrote:
>> On Wed, Mar 14, 2018 at 06:22:07PM -0300, Thiago Jung Bauermann wrote:
>> > 
>> > SF Markus Elfring <elfring@users.sourceforge.net> writes:
>> > 
>> > > From: Markus Elfring <elfring@users.sourceforge.net>
>> > > Date: Sun, 11 Mar 2018 09:03:42 +0100
>> > > 
>> > > Add a jump target so that a bit of exception handling can be better reused
>> > > at the end of this function.
>> > > 
>> > > This issue was detected by using the Coccinelle software.
>> > > 
>> > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>> > > ---
>> > >  arch/powerpc/kernel/machine_kexec_file_64.c | 28 ++++++++++++----------------
>> > >  1 file changed, 12 insertions(+), 16 deletions(-)
>> > 
>> > I liked it. Thanks!
>> > 
>> > Reviewed-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
>> > 
>> 
>> You know that compilers already re-use string constants so this doesn't
>> actually save memory?
>
> And modern compilers create their own jump labels
> so this doesn't change object code either?

IMHO it's an improvement to the source code itself. I wasn't thinking
about the object file.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

^ permalink raw reply

* [PATCH RFC rebase 0/9] powerpc barrier_nospec
From: Michal Suchanek @ 2018-03-15 19:15 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Kate Stewart, Madhavan Srinivasan, Paul Mackerras,
	Michael Neuling, Bryant G. Ly, Mahesh Salgaonkar, Naveen N. Rao,
	Daniel Axtens, Nicholas Piggin, Al Viro, David Gibson,
	Greg Kroah-Hartman, linux-kernel, Sergey Senozhatsky,
	Cédric Le Goater, Masami Hiramatsu, Andrew Donnellan,
	Philippe Ombredanne, Joe Perches, Oliver O'Halloran,
	Andrew Morton, Tobin C. Harding, Michal Suchanek
In-Reply-To: <20180313200108.GA4082@hirez.programming.kicks-ass.net>

Yes, it is good idea to add some commit messages.

Also I rebased the patches on top v3 of series

Setup RFI flush after PowerVM LPM migration

Thanks

Michal

Michal Suchanek (9):
  powerpc: Add barrier_nospec
  powerpc: Use barrier_nospec in copy_from_user
  powerpc/64: Use barrier_nospec in syscall entry
  powerpc/64s: Use barrier_nospec in RFI_FLUSH_SLOT
  powerpc/64s: Add support for ori barrier_nospec patching
  powerpc/64: Patch barrier_nospec in modules
  powerpc/64: barrier_nospec: Add debugfs trigger
  powerpc/64s: barrier_nospec: Add hcall triggerr
  powerpc/64: barrier_nospec: Add commandline trigger

 arch/powerpc/include/asm/barrier.h        |  9 ++++
 arch/powerpc/include/asm/exception-64s.h  |  2 +-
 arch/powerpc/include/asm/feature-fixups.h |  9 ++++
 arch/powerpc/include/asm/setup.h          | 11 ++++
 arch/powerpc/include/asm/uaccess.h        | 11 +++-
 arch/powerpc/kernel/entry_64.S            |  3 ++
 arch/powerpc/kernel/module.c              |  6 +++
 arch/powerpc/kernel/setup_64.c            | 87 +++++++++++++++++++++++++++++++
 arch/powerpc/kernel/vmlinux.lds.S         |  7 +++
 arch/powerpc/lib/feature-fixups.c         | 47 ++++++++++++++---
 arch/powerpc/platforms/pseries/mobility.c |  2 +-
 arch/powerpc/platforms/pseries/pseries.h  |  2 +-
 arch/powerpc/platforms/pseries/setup.c    | 37 +++++++++----
 13 files changed, 213 insertions(+), 20 deletions(-)

-- 
2.13.6

^ permalink raw reply

* [PATCH RFC rebase 1/9] powerpc: Add barrier_nospec
From: Michal Suchanek @ 2018-03-15 19:15 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Kate Stewart, Madhavan Srinivasan, Paul Mackerras,
	Michael Neuling, Bryant G. Ly, Mahesh Salgaonkar, Naveen N. Rao,
	Daniel Axtens, Nicholas Piggin, Al Viro, David Gibson,
	Greg Kroah-Hartman, linux-kernel, Sergey Senozhatsky,
	Cédric Le Goater, Masami Hiramatsu, Andrew Donnellan,
	Philippe Ombredanne, Joe Perches, Oliver O'Halloran,
	Andrew Morton, Tobin C. Harding, Michal Suchanek
In-Reply-To: <cover.1521141122.git.msuchanek@suse.de>

When the firmware supports it an otherwise useless combination of ORI
instruction arguments is interpreted as speculation barrier. Implement
barrier_nospec using this instruction.

Based on the out-of-tree gmb() implementation.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/include/asm/barrier.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
index 10daa1d56e0a..8e47b3abe405 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -75,6 +75,15 @@ do {									\
 	___p1;								\
 })
 
+/* TODO: add patching so this can be disabled */
+/* Prevent speculative execution past this barrier. */
+#define barrier_nospec_asm ori 31,31,0
+#ifdef __ASSEMBLY__
+#define barrier_nospec barrier_nospec_asm
+#else
+#define barrier_nospec() __asm__ __volatile__ (stringify_in_c(barrier_nospec_asm) : : :)
+#endif
+
 #include <asm-generic/barrier.h>
 
 #endif /* _ASM_POWERPC_BARRIER_H */
-- 
2.13.6

^ permalink raw reply related

* [PATCH RFC rebase 2/9] powerpc: Use barrier_nospec in copy_from_user
From: Michal Suchanek @ 2018-03-15 19:15 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Kate Stewart, Madhavan Srinivasan, Paul Mackerras,
	Michael Neuling, Bryant G. Ly, Mahesh Salgaonkar, Naveen N. Rao,
	Daniel Axtens, Nicholas Piggin, Al Viro, David Gibson,
	Greg Kroah-Hartman, linux-kernel, Sergey Senozhatsky,
	Cédric Le Goater, Masami Hiramatsu, Andrew Donnellan,
	Philippe Ombredanne, Joe Perches, Oliver O'Halloran,
	Andrew Morton, Tobin C. Harding, Michal Suchanek
In-Reply-To: <cover.1521141122.git.msuchanek@suse.de>

This is based on x86 patch doing the same.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/include/asm/uaccess.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 51bfeb8777f0..af9b0e731f46 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -248,6 +248,7 @@ do {								\
 	__chk_user_ptr(ptr);					\
 	if (!is_kernel_addr((unsigned long)__gu_addr))		\
 		might_fault();					\
+	barrier_nospec();					\
 	__get_user_size(__gu_val, __gu_addr, (size), __gu_err);	\
 	(x) = (__typeof__(*(ptr)))__gu_val;			\
 	__gu_err;						\
@@ -258,8 +259,10 @@ do {								\
 	long __gu_err = -EFAULT;					\
 	unsigned long  __gu_val = 0;					\
 	const __typeof__(*(ptr)) __user *__gu_addr = (ptr);		\
+	int can_access = access_ok(VERIFY_READ, __gu_addr, (size));	\
 	might_fault();							\
-	if (access_ok(VERIFY_READ, __gu_addr, (size)))			\
+	barrier_nospec();						\
+	if (can_access)							\
 		__get_user_size(__gu_val, __gu_addr, (size), __gu_err);	\
 	(x) = (__force __typeof__(*(ptr)))__gu_val;				\
 	__gu_err;							\
@@ -271,6 +274,7 @@ do {								\
 	unsigned long __gu_val;					\
 	const __typeof__(*(ptr)) __user *__gu_addr = (ptr);	\
 	__chk_user_ptr(ptr);					\
+	barrier_nospec();					\
 	__get_user_size(__gu_val, __gu_addr, (size), __gu_err);	\
 	(x) = (__force __typeof__(*(ptr)))__gu_val;			\
 	__gu_err;						\
@@ -298,15 +302,19 @@ static inline unsigned long raw_copy_from_user(void *to,
 
 		switch (n) {
 		case 1:
+			barrier_nospec();
 			__get_user_size(*(u8 *)to, from, 1, ret);
 			break;
 		case 2:
+			barrier_nospec();
 			__get_user_size(*(u16 *)to, from, 2, ret);
 			break;
 		case 4:
+			barrier_nospec();
 			__get_user_size(*(u32 *)to, from, 4, ret);
 			break;
 		case 8:
+			barrier_nospec();
 			__get_user_size(*(u64 *)to, from, 8, ret);
 			break;
 		}
@@ -314,6 +322,7 @@ static inline unsigned long raw_copy_from_user(void *to,
 			return 0;
 	}
 
+	barrier_nospec();
 	return __copy_tofrom_user((__force void __user *)to, from, n);
 }
 
-- 
2.13.6

^ permalink raw reply related

* [PATCH RFC rebase 3/9] powerpc/64: Use barrier_nospec in syscall entry
From: Michal Suchanek @ 2018-03-15 19:15 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Kate Stewart, Madhavan Srinivasan, Paul Mackerras,
	Michael Neuling, Bryant G. Ly, Mahesh Salgaonkar, Naveen N. Rao,
	Daniel Axtens, Nicholas Piggin, Al Viro, David Gibson,
	Greg Kroah-Hartman, linux-kernel, Sergey Senozhatsky,
	Cédric Le Goater, Masami Hiramatsu, Andrew Donnellan,
	Philippe Ombredanne, Joe Perches, Oliver O'Halloran,
	Andrew Morton, Tobin C. Harding, Michal Suchanek
In-Reply-To: <cover.1521141122.git.msuchanek@suse.de>

On powerpc syscall entry is done in assembly so patch in an explicit
barrier_nospec.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/kernel/entry_64.S | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 2cb5109a7ea3..7bfc4cf48af2 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -36,6 +36,7 @@
 #include <asm/context_tracking.h>
 #include <asm/tm.h>
 #include <asm/ppc-opcode.h>
+#include <asm/barrier.h>
 #include <asm/export.h>
 #ifdef CONFIG_PPC_BOOK3S
 #include <asm/exception-64s.h>
@@ -159,6 +160,7 @@ system_call:			/* label this so stack traces look sane */
 	andi.	r11,r10,_TIF_SYSCALL_DOTRACE
 	bne	.Lsyscall_dotrace		/* does not return */
 	cmpldi	0,r0,NR_syscalls
+	barrier_nospec
 	bge-	.Lsyscall_enosys
 
 .Lsyscall:
@@ -319,6 +321,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	ld	r10,TI_FLAGS(r10)
 
 	cmpldi	r0,NR_syscalls
+	barrier_nospec
 	blt+	.Lsyscall
 
 	/* Return code is already in r3 thanks to do_syscall_trace_enter() */
-- 
2.13.6

^ permalink raw reply related

* [PATCH RFC rebase 4/9] powerpc/64s: Use barrier_nospec in RFI_FLUSH_SLOT
From: Michal Suchanek @ 2018-03-15 19:15 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Kate Stewart, Madhavan Srinivasan, Paul Mackerras,
	Michael Neuling, Bryant G. Ly, Mahesh Salgaonkar, Naveen N. Rao,
	Daniel Axtens, Nicholas Piggin, Al Viro, David Gibson,
	Greg Kroah-Hartman, linux-kernel, Sergey Senozhatsky,
	Cédric Le Goater, Masami Hiramatsu, Andrew Donnellan,
	Philippe Ombredanne, Joe Perches, Oliver O'Halloran,
	Andrew Morton, Tobin C. Harding, Michal Suchanek
In-Reply-To: <cover.1521141122.git.msuchanek@suse.de>

The RFI flush support patches the speculation barrier into
RFI_FLUSH_SLOT as part of the RFI flush. Use separate barrier_nospec
instead.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/include/asm/exception-64s.h | 2 +-
 arch/powerpc/lib/feature-fixups.c        | 9 +++------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
index 471b2274fbeb..bb5a3052b29b 100644
--- a/arch/powerpc/include/asm/exception-64s.h
+++ b/arch/powerpc/include/asm/exception-64s.h
@@ -81,9 +81,9 @@
  * L1-D cache when returning to userspace or a guest.
  */
 #define RFI_FLUSH_SLOT							\
+	barrier_nospec_asm;						\
 	RFI_FLUSH_FIXUP_SECTION;					\
 	nop;								\
-	nop;								\
 	nop
 
 #define RFI_TO_KERNEL							\
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 35f80ab7cbd8..4cc2f0c5c863 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -119,7 +119,7 @@ void do_feature_fixups(unsigned long value, void *fixup_start, void *fixup_end)
 #ifdef CONFIG_PPC_BOOK3S_64
 void do_rfi_flush_fixups(enum l1d_flush_type types)
 {
-	unsigned int instrs[3], *dest;
+	unsigned int instrs[2], *dest;
 	long *start, *end;
 	int i;
 
@@ -128,15 +128,13 @@ void do_rfi_flush_fixups(enum l1d_flush_type types)
 
 	instrs[0] = 0x60000000; /* nop */
 	instrs[1] = 0x60000000; /* nop */
-	instrs[2] = 0x60000000; /* nop */
 
 	if (types & L1D_FLUSH_FALLBACK)
-		/* b .+16 to fallback flush */
-		instrs[0] = 0x48000010;
+		/* b .+12 to fallback flush */
+		instrs[0] = 0x4800000c;
 
 	i = 0;
 	if (types & L1D_FLUSH_ORI) {
-		instrs[i++] = 0x63ff0000; /* ori 31,31,0 speculation barrier */
 		instrs[i++] = 0x63de0000; /* ori 30,30,0 L1d flush*/
 	}
 
@@ -150,7 +148,6 @@ void do_rfi_flush_fixups(enum l1d_flush_type types)
 
 		patch_instruction(dest, instrs[0]);
 		patch_instruction(dest + 1, instrs[1]);
-		patch_instruction(dest + 2, instrs[2]);
 	}
 
 	printk(KERN_DEBUG "rfi-flush: patched %d locations (%s flush)\n", i,
-- 
2.13.6

^ permalink raw reply related

* [PATCH RFC rebase 6/9] powerpc/64: Patch barrier_nospec in modules
From: Michal Suchanek @ 2018-03-15 19:15 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Kate Stewart, Madhavan Srinivasan, Paul Mackerras,
	Michael Neuling, Bryant G. Ly, Mahesh Salgaonkar, Naveen N. Rao,
	Daniel Axtens, Nicholas Piggin, Al Viro, David Gibson,
	Greg Kroah-Hartman, linux-kernel, Sergey Senozhatsky,
	Cédric Le Goater, Masami Hiramatsu, Andrew Donnellan,
	Philippe Ombredanne, Joe Perches, Oliver O'Halloran,
	Andrew Morton, Tobin C. Harding, Michal Suchanek
In-Reply-To: <cover.1521141122.git.msuchanek@suse.de>

Note that unlike RFI which is patched only in kernel the nospec state
reflects settings at the time the module was loaded.

Iterating all modules and re-patching every time the settings change is
not implemented.

Based on lwsync patching.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/include/asm/setup.h  |  5 ++++-
 arch/powerpc/kernel/module.c      |  6 ++++++
 arch/powerpc/kernel/setup_64.c    |  4 ++--
 arch/powerpc/lib/feature-fixups.c | 17 ++++++++++++++---
 4 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/setup.h b/arch/powerpc/include/asm/setup.h
index c7e9e66c2a38..92520d2483b8 100644
--- a/arch/powerpc/include/asm/setup.h
+++ b/arch/powerpc/include/asm/setup.h
@@ -58,7 +58,10 @@ enum spec_barrier_type {
 void setup_rfi_flush(enum l1d_flush_type, bool enable);
 void do_rfi_flush_fixups(enum l1d_flush_type types);
 void setup_barrier_nospec(enum spec_barrier_type, bool enable);
-void do_barrier_nospec_fixups(enum spec_barrier_type type);
+void do_barrier_nospec_fixups_kernel(enum spec_barrier_type type);
+void do_barrier_nospec_fixups(enum spec_barrier_type type,
+			      void *start, void *end);
+extern enum spec_barrier_type powerpc_barrier_nospec;
 
 #endif /* !__ASSEMBLY__ */
 
diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
index 3f7ba0f5bf29..7b6d0ec06a21 100644
--- a/arch/powerpc/kernel/module.c
+++ b/arch/powerpc/kernel/module.c
@@ -72,6 +72,12 @@ int module_finalize(const Elf_Ehdr *hdr,
 		do_feature_fixups(powerpc_firmware_features,
 				  (void *)sect->sh_addr,
 				  (void *)sect->sh_addr + sect->sh_size);
+
+	sect = find_section(hdr, sechdrs, "__spec_barrier_fixup");
+	if (sect != NULL)
+		do_barrier_nospec_fixups(powerpc_barrier_nospec,
+				  (void *)sect->sh_addr,
+				  (void *)sect->sh_addr + sect->sh_size);
 #endif
 
 	sect = find_section(hdr, sechdrs, "__lwsync_fixup");
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 767240074cad..f60e0e3b5ad2 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -910,11 +910,11 @@ void barrier_nospec_enable(bool enable)
 
 	if (enable) {
 		powerpc_barrier_nospec = barrier_nospec_type;
-		do_barrier_nospec_fixups(powerpc_barrier_nospec);
+		do_barrier_nospec_fixups_kernel(powerpc_barrier_nospec);
 		on_each_cpu(do_nothing, NULL, 1);
 	} else {
 		powerpc_barrier_nospec = SPEC_BARRIER_NONE;
-		do_barrier_nospec_fixups(powerpc_barrier_nospec);
+		do_barrier_nospec_fixups_kernel(powerpc_barrier_nospec);
 	}
 }
 
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index dfeb7feeccef..a529ac6b2a5d 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -160,14 +160,15 @@ void do_rfi_flush_fixups(enum l1d_flush_type types)
 						: "unknown");
 }
 
-void do_barrier_nospec_fixups(enum spec_barrier_type type)
+void do_barrier_nospec_fixups(enum spec_barrier_type type,
+			      void *fixup_start, void *fixup_end)
 {
 	unsigned int instr, *dest;
 	long *start, *end;
 	int i;
 
-	start = PTRRELOC(&__start___spec_barrier_fixup),
-	end = PTRRELOC(&__stop___spec_barrier_fixup);
+	start = fixup_start;
+	end = fixup_end;
 
 	instr = 0x60000000; /* nop */
 
@@ -186,6 +187,16 @@ void do_barrier_nospec_fixups(enum spec_barrier_type type)
 	printk(KERN_DEBUG "barrier-nospec: patched %d locations\n", i);
 }
 
+void do_barrier_nospec_fixups_kernel(enum spec_barrier_type type)
+{
+	void *start, *end;
+
+	start = PTRRELOC(&__start___spec_barrier_fixup),
+	end = PTRRELOC(&__stop___spec_barrier_fixup);
+
+	do_barrier_nospec_fixups(type, start, end);
+}
+
 #endif /* CONFIG_PPC_BOOK3S_64 */
 
 void do_lwsync_fixups(unsigned long value, void *fixup_start, void *fixup_end)
-- 
2.13.6

^ permalink raw reply related

* [PATCH RFC rebase 5/9] powerpc/64s: Add support for ori barrier_nospec patching
From: Michal Suchanek @ 2018-03-15 19:15 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Kate Stewart, Madhavan Srinivasan, Paul Mackerras,
	Michael Neuling, Bryant G. Ly, Mahesh Salgaonkar, Naveen N. Rao,
	Daniel Axtens, Nicholas Piggin, Al Viro, David Gibson,
	Greg Kroah-Hartman, linux-kernel, Sergey Senozhatsky,
	Cédric Le Goater, Masami Hiramatsu, Andrew Donnellan,
	Philippe Ombredanne, Joe Perches, Oliver O'Halloran,
	Andrew Morton, Tobin C. Harding, Michal Suchanek
In-Reply-To: <cover.1521141122.git.msuchanek@suse.de>

Based on the RFI patching. This is required to be able to disable the
speculation barrier.

Only one barrier type is supported and it does nothing when the firmware
does not enable it. Also re-patching modules is not supported So the
only meaningful thing that can be done is patching out the speculation
barrier at boot when the user says it is not wanted.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/include/asm/barrier.h        |  4 ++--
 arch/powerpc/include/asm/feature-fixups.h |  9 +++++++++
 arch/powerpc/include/asm/setup.h          |  8 ++++++++
 arch/powerpc/kernel/setup_64.c            | 30 ++++++++++++++++++++++++++++++
 arch/powerpc/kernel/vmlinux.lds.S         |  7 +++++++
 arch/powerpc/lib/feature-fixups.c         | 27 +++++++++++++++++++++++++++
 6 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
index 8e47b3abe405..4079a95e84c2 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -75,9 +75,9 @@ do {									\
 	___p1;								\
 })
 
-/* TODO: add patching so this can be disabled */
 /* Prevent speculative execution past this barrier. */
-#define barrier_nospec_asm ori 31,31,0
+#define barrier_nospec_asm SPEC_BARRIER_FIXUP_SECTION;			\
+				nop
 #ifdef __ASSEMBLY__
 #define barrier_nospec barrier_nospec_asm
 #else
diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
index 1e82eb3caabd..9d3382618ffd 100644
--- a/arch/powerpc/include/asm/feature-fixups.h
+++ b/arch/powerpc/include/asm/feature-fixups.h
@@ -195,11 +195,20 @@ label##3:					       	\
 	FTR_ENTRY_OFFSET 951b-952b;			\
 	.popsection;
 
+#define SPEC_BARRIER_FIXUP_SECTION			\
+953:							\
+	.pushsection __spec_barrier_fixup,"a";		\
+	.align 2;					\
+954:							\
+	FTR_ENTRY_OFFSET 953b-954b;			\
+	.popsection;
+
 
 #ifndef __ASSEMBLY__
 #include <linux/types.h>
 
 extern long __start___rfi_flush_fixup, __stop___rfi_flush_fixup;
+extern long __start___spec_barrier_fixup, __stop___spec_barrier_fixup;
 
 void apply_feature_fixups(void);
 void setup_feature_keys(void);
diff --git a/arch/powerpc/include/asm/setup.h b/arch/powerpc/include/asm/setup.h
index bbcdf929be54..c7e9e66c2a38 100644
--- a/arch/powerpc/include/asm/setup.h
+++ b/arch/powerpc/include/asm/setup.h
@@ -49,8 +49,16 @@ enum l1d_flush_type {
 	L1D_FLUSH_MTTRIG	= 0x8,
 };
 
+/* These are bit flags */
+enum spec_barrier_type {
+	SPEC_BARRIER_NONE	= 0x1,
+	SPEC_BARRIER_ORI	= 0x2,
+};
+
 void setup_rfi_flush(enum l1d_flush_type, bool enable);
 void do_rfi_flush_fixups(enum l1d_flush_type types);
+void setup_barrier_nospec(enum spec_barrier_type, bool enable);
+void do_barrier_nospec_fixups(enum spec_barrier_type type);
 
 #endif /* !__ASSEMBLY__ */
 
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 4ec4a27b36a9..767240074cad 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -815,6 +815,10 @@ static enum l1d_flush_type enabled_flush_types;
 static void *l1d_flush_fallback_area;
 static bool no_rfi_flush;
 bool rfi_flush;
+enum spec_barrier_type powerpc_barrier_nospec;
+static enum spec_barrier_type barrier_nospec_type;
+static bool no_nospec;
+bool barrier_nospec_enabled;
 
 static int __init handle_no_rfi_flush(char *p)
 {
@@ -900,6 +904,32 @@ void setup_rfi_flush(enum l1d_flush_type types, bool enable)
 		rfi_flush_enable(enable);
 }
 
+void barrier_nospec_enable(bool enable)
+{
+	barrier_nospec_enabled = enable;
+
+	if (enable) {
+		powerpc_barrier_nospec = barrier_nospec_type;
+		do_barrier_nospec_fixups(powerpc_barrier_nospec);
+		on_each_cpu(do_nothing, NULL, 1);
+	} else {
+		powerpc_barrier_nospec = SPEC_BARRIER_NONE;
+		do_barrier_nospec_fixups(powerpc_barrier_nospec);
+	}
+}
+
+void setup_barrier_nospec(enum spec_barrier_type type, bool enable)
+{
+	/*
+	 * Only one barrier type is supported and it does nothing when the
+	 * firmware does not enable it. So the only meaningful thing to do
+	 * here is check the user preference.
+	 */
+	barrier_nospec_type = SPEC_BARRIER_ORI;
+
+	barrier_nospec_enable(!no_nospec && enable);
+}
+
 #ifdef CONFIG_DEBUG_FS
 static int rfi_flush_set(void *data, u64 val)
 {
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index c8af90ff49f0..744b58ff77f1 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -139,6 +139,13 @@ SECTIONS
 		*(__rfi_flush_fixup)
 		__stop___rfi_flush_fixup = .;
 	}
+
+	. = ALIGN(8);
+	__spec_barrier_fixup : AT(ADDR(__spec_barrier_fixup) - LOAD_OFFSET) {
+		__start___spec_barrier_fixup = .;
+		*(__spec_barrier_fixup)
+		__stop___spec_barrier_fixup = .;
+	}
 #endif
 
 	EXCEPTION_TABLE(0)
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 4cc2f0c5c863..dfeb7feeccef 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -159,6 +159,33 @@ void do_rfi_flush_fixups(enum l1d_flush_type types)
 		(types &  L1D_FLUSH_MTTRIG)     ? "mttrig type"
 						: "unknown");
 }
+
+void do_barrier_nospec_fixups(enum spec_barrier_type type)
+{
+	unsigned int instr, *dest;
+	long *start, *end;
+	int i;
+
+	start = PTRRELOC(&__start___spec_barrier_fixup),
+	end = PTRRELOC(&__stop___spec_barrier_fixup);
+
+	instr = 0x60000000; /* nop */
+
+	if (type == SPEC_BARRIER_ORI) {
+		pr_info("barrier_nospec: using ORI speculation barrier\n");
+		instr = 0x63ff0000; /* ori 31,31,0 speculation barrier */
+	}
+
+	for (i = 0; start < end; start++, i++) {
+		dest = (void *)start + *start;
+
+		pr_devel("patching dest %lx\n", (unsigned long)dest);
+		patch_instruction(dest, instr);
+	}
+
+	printk(KERN_DEBUG "barrier-nospec: patched %d locations\n", i);
+}
+
 #endif /* CONFIG_PPC_BOOK3S_64 */
 
 void do_lwsync_fixups(unsigned long value, void *fixup_start, void *fixup_end)
-- 
2.13.6

^ permalink raw reply related

* [PATCH RFC rebase 7/9] powerpc/64: barrier_nospec: Add debugfs trigger
From: Michal Suchanek @ 2018-03-15 19:15 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Kate Stewart, Madhavan Srinivasan, Paul Mackerras,
	Michael Neuling, Bryant G. Ly, Mahesh Salgaonkar, Naveen N. Rao,
	Daniel Axtens, Nicholas Piggin, Al Viro, David Gibson,
	Greg Kroah-Hartman, linux-kernel, Sergey Senozhatsky,
	Cédric Le Goater, Masami Hiramatsu, Andrew Donnellan,
	Philippe Ombredanne, Joe Perches, Oliver O'Halloran,
	Andrew Morton, Tobin C. Harding, Michal Suchanek
In-Reply-To: <cover.1521141122.git.msuchanek@suse.de>

Copypasta from rfi implementation

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/kernel/setup_64.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index f60e0e3b5ad2..f6678a7b6114 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -963,6 +963,41 @@ static __init int rfi_flush_debugfs_init(void)
 	return 0;
 }
 device_initcall(rfi_flush_debugfs_init);
+
+static int barrier_nospec_set(void *data, u64 val)
+{
+	switch (val) {
+	case 0:
+	case 1:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (!!val == !!barrier_nospec_enabled)
+		return 0;
+
+	barrier_nospec_enable(!!val);
+
+	return 0;
+}
+
+static int barrier_nospec_get(void *data, u64 *val)
+{
+	*val = barrier_nospec_enabled ? 1 : 0;
+	return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(fops_barrier_nospec,
+			barrier_nospec_get, barrier_nospec_set, "%llu\n");
+
+static __init int barrier_nospec_debugfs_init(void)
+{
+	debugfs_create_file("barrier_nospec", 0600, powerpc_debugfs_root, NULL,
+			    &fops_barrier_nospec);
+	return 0;
+}
+device_initcall(barrier_nospec_debugfs_init);
 #endif
 
 ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute *attr, char *buf)
-- 
2.13.6

^ permalink raw reply related

* [PATCH RFC rebase 8/9] powerpc/64s: barrier_nospec: Add hcall triggerr
From: Michal Suchanek @ 2018-03-15 19:15 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Kate Stewart, Madhavan Srinivasan, Paul Mackerras,
	Michael Neuling, Bryant G. Ly, Mahesh Salgaonkar, Naveen N. Rao,
	Daniel Axtens, Nicholas Piggin, Al Viro, David Gibson,
	Greg Kroah-Hartman, linux-kernel, Sergey Senozhatsky,
	Cédric Le Goater, Masami Hiramatsu, Andrew Donnellan,
	Philippe Ombredanne, Joe Perches, Oliver O'Halloran,
	Andrew Morton, Tobin C. Harding, Michal Suchanek
In-Reply-To: <cover.1521141122.git.msuchanek@suse.de>

Adapted from the RFI implementation

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/platforms/pseries/mobility.c |  2 +-
 arch/powerpc/platforms/pseries/pseries.h  |  2 +-
 arch/powerpc/platforms/pseries/setup.c    | 37 ++++++++++++++++++++++---------
 3 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index 8a8033a249c7..9d506be1580e 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -349,7 +349,7 @@ void post_mobility_fixup(void)
 			"failed: %d\n", rc);
 
 	/* Possibly switch to a new RFI flush type */
-	pseries_setup_rfi_flush();
+	pseries_setup_rfi_nospec();
 
 	return;
 }
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 27cdcb69fd18..d49670c67686 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -100,6 +100,6 @@ static inline unsigned long cmo_get_page_size(void)
 
 int dlpar_workqueue_init(void);
 
-void pseries_setup_rfi_flush(void);
+void pseries_setup_rfi_nospec(void);
 
 #endif /* _PSERIES_PSERIES_H */
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 9877c3dfcdc8..4b899a4db6dd 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -459,30 +459,47 @@ static void __init find_and_init_phbs(void)
 	of_pci_check_probe_only();
 }
 
-void pseries_setup_rfi_flush(void)
+void pseries_setup_rfi_nospec(void)
 {
 	struct h_cpu_char_result result;
-	enum l1d_flush_type types;
-	bool enable;
+	enum l1d_flush_type flush_types;
+	enum spec_barrier_type barrier_type;
+	bool flush_enable;
+	bool barrier_enable;
 	long rc;
 
 	/* Enable by default */
-	enable = true;
-	types = L1D_FLUSH_FALLBACK;
+	flush_enable = true;
+	flush_types = L1D_FLUSH_FALLBACK;
+	barrier_enable = true;
+	/* no fallback available if the firmware does not tell us */
+	barrier_type = SPEC_BARRIER_NONE;
 
 	rc = plpar_get_cpu_characteristics(&result);
 	if (rc == H_SUCCESS) {
 		if (result.character & H_CPU_CHAR_L1D_FLUSH_TRIG2)
-			types |= L1D_FLUSH_MTTRIG;
+			flush_types |= L1D_FLUSH_MTTRIG;
 		if (result.character & H_CPU_CHAR_L1D_FLUSH_ORI30)
-			types |= L1D_FLUSH_ORI;
+			flush_types |= L1D_FLUSH_ORI;
+		if (result.character & H_CPU_CHAR_SPEC_BAR_ORI31)
+			barrier_type |= SPEC_BARRIER_ORI;
 
 		if ((!(result.behaviour & H_CPU_BEHAV_L1D_FLUSH_PR)) ||
 		    (!(result.behaviour & H_CPU_BEHAV_FAVOUR_SECURITY)))
-			enable = false;
+			flush_enable = false;
+		/*
+		 * Do not check H_CPU_BEHAV_BNDS_CHK_SPEC_BAR - the ORI does
+		 * nothing anyway when not supported.
+		 */
+		if ((!(result.behaviour & H_CPU_BEHAV_FAVOUR_SECURITY)))
+			barrier_enable = false;
+	} else {
+		/* Default to fallback if case hcall is not available */
+		flush_types = L1D_FLUSH_FALLBACK;
 	}
 
-	setup_rfi_flush(types, enable);
+	setup_barrier_nospec(barrier_type, barrier_enable);
+	setup_rfi_flush(flush_types, flush_enable);
 }
 
 #ifdef CONFIG_PCI_IOV
@@ -658,7 +675,7 @@ static void __init pSeries_setup_arch(void)
 
 	fwnmi_init();
 
-	pseries_setup_rfi_flush();
+	pseries_setup_rfi_nospec();
 
 	/* By default, only probe PCI (can be overridden by rtas_pci) */
 	pci_add_flags(PCI_PROBE_ONLY);
-- 
2.13.6

^ permalink raw reply related

* [PATCH RFC rebase 9/9] powerpc/64: barrier_nospec: Add commandline trigger
From: Michal Suchanek @ 2018-03-15 19:15 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Kate Stewart, Madhavan Srinivasan, Paul Mackerras,
	Michael Neuling, Bryant G. Ly, Mahesh Salgaonkar, Naveen N. Rao,
	Daniel Axtens, Nicholas Piggin, Al Viro, David Gibson,
	Greg Kroah-Hartman, linux-kernel, Sergey Senozhatsky,
	Cédric Le Goater, Masami Hiramatsu, Andrew Donnellan,
	Philippe Ombredanne, Joe Perches, Oliver O'Halloran,
	Andrew Morton, Tobin C. Harding, Michal Suchanek
In-Reply-To: <cover.1521141122.git.msuchanek@suse.de>

Add commandline options spectre_v2 and nospectre_v2

These are named same as similar x86 options regardless of actual effect
to not require platform-specific configuration.

Supported options:
nospectre_v2 or spectre_v2=off - speculation barrier not used
spectre_v2=on or spectre_v2=auto - speculation barrier used

Changing the settings after boot is not supported and VM migration may
change requirements so auto is same as on.

Based on s390 implementation

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/kernel/setup_64.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index f6678a7b6114..c74e656265df 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -840,6 +840,28 @@ static int __init handle_no_pti(char *p)
 }
 early_param("nopti", handle_no_pti);
 
+static int __init nospectre_v2_setup_early(char *str)
+{
+	no_nospec = true;
+	return 0;
+}
+early_param("nospectre_v2", nospectre_v2_setup_early);
+
+static int __init spectre_v2_setup_early(char *str)
+{
+	if (str && !strncmp(str, "on", 2))
+		no_nospec = false;
+
+	if (str && !strncmp(str, "off", 3))
+		no_nospec = true;
+
+	if (str && !strncmp(str, "auto", 4))
+		no_nospec = false;
+
+	return 0;
+}
+early_param("spectre_v2", spectre_v2_setup_early);
+
 static void do_nothing(void *unused)
 {
 	/*
-- 
2.13.6

^ permalink raw reply related

* Re: [PATCH v3 1/5] rfi-flush: Move the logic to avoid a redo into the debugfs code
From: Murilo Opsfelder Araujo @ 2018-03-15 20:36 UTC (permalink / raw)
  To: Mauricio Faria de Oliveira, linuxppc-dev, mpe, msuchanek
In-Reply-To: <1521067243-19520-2-git-send-email-mauricfo@linux.vnet.ibm.com>

On 03/14/2018 07:40 PM, Mauricio Faria de Oliveira wrote:
> From: Michael Ellerman <mpe@ellerman.id.au>
> 
> rfi_flush_enable() includes a check to see if we're already
> enabled (or disabled), and in that case does nothing.
> 
> But that means calling setup_rfi_flush() a 2nd time doesn't actually
> work, which is a bit confusing.
> 
> Move that check into the debugfs code, where it really belongs.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/setup_64.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index c388cc3..3efc01a 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -846,9 +846,6 @@ static void do_nothing(void *unused)
> 
>  void rfi_flush_enable(bool enable)
>  {
> -	if (rfi_flush == enable)
> -		return;
> -
>  	if (enable) {
>  		do_rfi_flush_fixups(enabled_flush_types);
>  		on_each_cpu(do_nothing, NULL, 1);
> @@ -902,13 +899,19 @@ void __init setup_rfi_flush(enum l1d_flush_type types, bool enable)
>  #ifdef CONFIG_DEBUG_FS
>  static int rfi_flush_set(void *data, u64 val)
>  {
> +	bool enable;
> +
>  	if (val == 1)
> -		rfi_flush_enable(true);
> +		enable = true;
>  	else if (val == 0)
> -		rfi_flush_enable(false);
> +		enable = false;
>  	else
>  		return -EINVAL;
> 
> +	/* Only do anything if we're changing state */
> +	if (enable != rfi_flush)

Hi, Mauricio.

Do we need to take into account if no_rfi_flush == true?

    if ((enable != rfi_flush) && !no_rfi_flush)

> +		rfi_flush_enable(enable);
> +
>  	return 0;
>  }
> 

Cheers
Murilo

^ permalink raw reply

* Re: [PATCH RFC rebase 2/9] powerpc: Use barrier_nospec in copy_from_user
From: Linus Torvalds @ 2018-03-15 21:37 UTC (permalink / raw)
  To: Michal Suchanek
  Cc: ppc-dev, Kate Stewart, Madhavan Srinivasan, Paul Mackerras,
	Michael Neuling, Bryant G. Ly, Mahesh Salgaonkar, Naveen N. Rao,
	Daniel Axtens, Nicholas Piggin, Al Viro, David Gibson,
	Greg Kroah-Hartman, Linux Kernel Mailing List, Sergey Senozhatsky,
	Cédric Le Goater, Masami Hiramatsu, Andrew Donnellan,
	Philippe Ombredanne, Joe Perches, Oliver O'Halloran,
	Andrew Morton, Tobin C. Harding
In-Reply-To: <32268431948dc1a32264a98a76d41d71ae7536b3.1521141122.git.msuchanek@suse.de>

On Thu, Mar 15, 2018 at 12:15 PM, Michal Suchanek <msuchanek@suse.de> wrote:
> This is based on x86 patch doing the same.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -258,8 +259,10 @@ do {                                                               \
>         long __gu_err = -EFAULT;                                        \
>         unsigned long  __gu_val = 0;                                    \
>         const __typeof__(*(ptr)) __user *__gu_addr = (ptr);             \
> +       int can_access = access_ok(VERIFY_READ, __gu_addr, (size));     \
>         might_fault();                                                  \
> -       if (access_ok(VERIFY_READ, __gu_addr, (size)))                  \
> +       barrier_nospec();                                               \
> +       if (can_access)                                                 \
>                 __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
>         (x) = (__force __typeof__(*(ptr)))__gu_val;                             \
>         __gu_err;                                                       \

Is the above really correct? The barrier is *before* the conditional
branch that might be mis-predicted.

I don't know how the ppc barrier works, but that sounds completely bogus.

               Linus

^ permalink raw reply

* Re: [RFC] powerpc/xive: Remove irq from queue when it is shutdown
From: Benjamin Herrenschmidt @ 2018-03-15 22:56 UTC (permalink / raw)
  To: Cédric Le Goater, Frederic Barrat, linuxppc-dev
  Cc: andrew.donnellan, vaibhav
In-Reply-To: <ba73a10a-320c-914e-5d7e-1ff212fdd6c5@kaod.org>

On Wed, 2018-03-14 at 18:47 +0100, Cédric Le Goater wrote:
> We could also loop on the ESB 'P' bit to wait for the irqs to be handled,
> using :
> 
>         xive_esb_read(irq, XIVE_ESB_GET)
> 
> which has no side effect. It looks simpler to me. Is that possible ? 

But you can race with something sending another one... I prefer the
guarantee that after final masking, we check the queue contents on
shutdown.

That will be solid vs. other type of interrupts as well.

Cheers,
Ben.

^ permalink raw reply

* Re: [RFC] powerpc/xive: Remove irq from queue when it is shutdown
From: Benjamin Herrenschmidt @ 2018-03-15 22:59 UTC (permalink / raw)
  To: Frederic Barrat, linuxppc-dev, clg; +Cc: andrew.donnellan, vaibhav
In-Reply-To: <20180314165808.26628-1-fbarrat@linux.vnet.ibm.com>

On Wed, 2018-03-14 at 17:58 +0100, Frederic Barrat wrote:
> +                       if (irq == hw_irq) {
> +                               cur &= 1 << 31;
> +                               cur |= XIVE_BAD_IRQ;
> +                               *(q->qpage + idx) = cpu_to_be32(cur);
> +                       }
> +
> +                       idx = (idx + 1) & q->msk;
> +                       if (idx == 0)
> +                               toggle ^= 1;
> +               } while (irq && (count < q->msk));

Safer to make the replacement an atomic with cmpxhg just in case you
found an old one trailing the queue that's just getting updated by HW.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH RFC rebase 3/9] powerpc/64: Use barrier_nospec in syscall entry
From: Nicholas Piggin @ 2018-03-16  5:18 UTC (permalink / raw)
  To: Michal Suchanek
  Cc: linuxppc-dev, Kate Stewart, Madhavan Srinivasan, Paul Mackerras,
	Michael Neuling, Bryant G. Ly, Mahesh Salgaonkar, Naveen N. Rao,
	Daniel Axtens, Al Viro, David Gibson, Greg Kroah-Hartman,
	linux-kernel, Sergey Senozhatsky, Cédric Le Goater,
	Masami Hiramatsu, Andrew Donnellan, Philippe Ombredanne,
	Joe Perches, Oliver O'Halloran, Andrew Morton,
	Tobin C. Harding
In-Reply-To: <b8253774e223530c808eed65d116deecc46ad36b.1521141122.git.msuchanek@suse.de>

On Thu, 15 Mar 2018 20:15:52 +0100
Michal Suchanek <msuchanek@suse.de> wrote:

> On powerpc syscall entry is done in assembly so patch in an explicit
> barrier_nospec.

Same comment as Linus for this -- the barriers are before the branch here,
so is it possible the branch instruction can be speculative while the index
is used to load the syscall table?

Thanks,
Nick

> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  arch/powerpc/kernel/entry_64.S | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 2cb5109a7ea3..7bfc4cf48af2 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -36,6 +36,7 @@
>  #include <asm/context_tracking.h>
>  #include <asm/tm.h>
>  #include <asm/ppc-opcode.h>
> +#include <asm/barrier.h>
>  #include <asm/export.h>
>  #ifdef CONFIG_PPC_BOOK3S
>  #include <asm/exception-64s.h>
> @@ -159,6 +160,7 @@ system_call:			/* label this so stack traces look sane */
>  	andi.	r11,r10,_TIF_SYSCALL_DOTRACE
>  	bne	.Lsyscall_dotrace		/* does not return */
>  	cmpldi	0,r0,NR_syscalls
> +	barrier_nospec
>  	bge-	.Lsyscall_enosys
>  
>  .Lsyscall:
> @@ -319,6 +321,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  	ld	r10,TI_FLAGS(r10)
>  
>  	cmpldi	r0,NR_syscalls
> +	barrier_nospec
>  	blt+	.Lsyscall
>  
>  	/* Return code is already in r3 thanks to do_syscall_trace_enter() */

^ permalink raw reply

* build warning in drivers/macintosh/rack-meter.c
From: Stephen Rothwell @ 2018-03-16  5:46 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, PowerPC; +Cc: Aaro Koskinen

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

Hi all,

These warnings have beeen around for a while now;

drivers/macintosh/rack-meter.c:157:2: warning: 'memset' used with length equal to number of elements without multiplication by element size [-Wmemset-elt-size]
  memset(rdma->buf1, 0, ARRAY_SIZE(rdma->buf1));
  ^~~~~~
drivers/macintosh/rack-meter.c:158:2: warning: 'memset' used with length equal to number of elements without multiplication by element size [-Wmemset-elt-size]
  memset(rdma->buf2, 0, ARRAY_SIZE(rdma->buf2));
  ^~~~~~

Introduced by commit

  4f7bef7a9f69 ("drivers: macintosh: rack-meter: fix bogus memsets")

I guess updating to a newer compiler has started generating these.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH V4 1/3] powerpc/mm: Add support for handling > 512TB address in SLB miss
From: Michael Ellerman @ 2018-03-16  6:35 UTC (permalink / raw)
  To: Aneesh Kumar K.V, benh, paulus; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180307121440.20188-2-aneesh.kumar@linux.vnet.ibm.com>

Hi Aneesh,

Some comments ...

"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:
> For address above 512TB we allocate additional mmu context. To make it all
> easy address above 512TB is handled with IR/DR=3D1 and with stack frame s=
etup.
>
> We do the additional context allocation in SLB miss handler. If the conte=
xt is
> not allocated, we enable interrupts and allocate the context and retry the
> access which will again result in a SLB miss.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

This doesn't build for g5_defconfig:

  In file included from ../arch/powerpc/include/asm/mmu.h:314:0,
                   from ../arch/powerpc/include/asm/lppaca.h:36,
                   from ../arch/powerpc/include/asm/paca.h:21,
                   from ../arch/powerpc/include/asm/hw_irq.h:63,
                   from ../arch/powerpc/include/asm/irqflags.h:12,
                   from ../include/linux/irqflags.h:16,
                   from ../include/linux/spinlock.h:54,
                   from ../include/linux/mmzone.h:8,
                   from ../arch/powerpc/include/asm/pgtable.h:7,
                   from ../arch/powerpc/mm/slb.c:17:
  ../arch/powerpc/mm/slb.c: In function =E2=80=98handle_multi_context_slb_m=
iss=E2=80=99:
  ../arch/powerpc/include/asm/book3s/64/mmu.h:208:25: error: array subscrip=
t is above array bounds [-Werror=3Darray-bounds]
    return ctx->extended_id[index];
           ~~~~~~~~~~~~~~~~^~~~~~~
  ../arch/powerpc/mm/slb.c:417:30: error: array subscript is above array bo=
unds [-Werror=3Darray-bounds]
    if (!mm->context.extended_id[index])
         ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
  ../arch/powerpc/mm/slb.c:418:26: error: array subscript is above array bo=
unds [-Werror=3Darray-bounds]
     mm->context.extended_id[index] =3D context_id;
     ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~


> diff --git a/arch/powerpc/include/asm/book3s/64/hash-4k.h b/arch/powerpc/=
include/asm/book3s/64/hash-4k.h
> index 67c5475311ee..af2ba9875f18 100644
> --- a/arch/powerpc/include/asm/book3s/64/hash-4k.h
> +++ b/arch/powerpc/include/asm/book3s/64/hash-4k.h
> @@ -11,6 +11,12 @@
>  #define H_PUD_INDEX_SIZE  9
>  #define H_PGD_INDEX_SIZE  9
>=20=20
> +/*
> + * No of address bits below which we use the default context

Can we use "Number", "No" means no, to mean number you need "No." but
that's ugly.

> + * for slb allocation. For 4k this is 64TB.
> + */
> +#define H_BITS_FIRST_CONTEXT	46

It's actually the number of address bits *per* context right?

"Context" is also a bit of a overloaded term, eg. context !=3D
mm_context_t, maybe "context id" would be clearer?

So maybe H_BITS_PER_CONTEXT_ID?


This values is essentially VA_BITS - CONTEXT_BITS right?

Except VA_BITS is actually 65 in this case, but that's not clear at all
from the code :/

It'd be really nice if this was calculated, or if the other values
(VA_BITS/CONTEXT_BITS) were calculated based on it.

> diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h b/arch/powerpc=
/include/asm/book3s/64/hash-64k.h
> index 3bcf269f8f55..0ee0fc1ad675 100644
> --- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
> +++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
> @@ -6,6 +6,11 @@
>  #define H_PMD_INDEX_SIZE  10
>  #define H_PUD_INDEX_SIZE  7
>  #define H_PGD_INDEX_SIZE  8

Can we get some newlines between code?

> +/*
> + * No of address bits below which we use the default context
> + * for slb allocation. For 64k this is 512TB.
> + */
> +#define H_BITS_FIRST_CONTEXT	49

This really is =3D=3D VA_BITS - CONTEXT_BITS.

> diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc=
/include/asm/book3s/64/mmu-hash.h
> index 50ed64fba4ae..8ee83f6e9c84 100644
> --- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> +++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> @@ -691,8 +691,8 @@ static inline int user_segment_size(unsigned long add=
r)
>  	return MMU_SEGSIZE_256M;
>  }
>=20=20
> -static inline unsigned long get_vsid(unsigned long context, unsigned lon=
g ea,
> -				     int ssize)
> +static inline unsigned long __get_vsid(unsigned long context, unsigned l=
ong ea,
> +				       int ssize)

If you're going to realign it the 'i' in int should line up with the 'u'
in unsigned.

Why did this become __get_vsid()? We don't have a get_vsid() that I can see?

> diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/incl=
ude/asm/book3s/64/mmu.h
> index 777778579305..a70adbb7ec56 100644
> --- a/arch/powerpc/include/asm/book3s/64/mmu.h
> +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
> @@ -91,7 +91,15 @@ struct slice_mask {
>  };
>=20=20
>  typedef struct {
> -	mm_context_id_t id;
> +	union {
> +		/*
> +		 * One context for each 512TB.
> +		 * First 512TB context is saved in id and is also used
> +		 * as PIDR.
> +		 */

Can you make it clearer how this is used on hash vs radix.

> +		mm_context_id_t id;
> +		mm_context_id_t extended_id[TASK_SIZE_USER64/TASK_CONTEXT_SIZE];

If my math is right this is typically 4PB / 512T =3D=3D 8 =3D=3D 64 bytes.

Which is small enough to not worry about, but would be good to mention
in the change log at least.

> +	};
>  	u16 user_psize;		/* page size index */
>=20=20
>  	/* Number of bits in the mm_cpumask */
> @@ -193,5 +201,21 @@ extern void radix_init_pseries(void);
>  static inline void radix_init_pseries(void) { };
>  #endif
>=20=20
> +static inline int get_esid_context(mm_context_t *ctx, unsigned long ea)
> +{
> +	int index =3D ea >> H_BITS_FIRST_CONTEXT;

Using "esid" in the name is a bit confusing, because the value is an EA,
not an ESID. So change the name or what's passed?

I think change the name, the callers both have an EA so it makes sense
to pass that.

> +

I know we check ea elsewhere, but a:

  VM_BUG_ON(index >=3D ARRAY_SIZE(extened_id))

Would make me feel better I think, and index being unsigned.

> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/=
asm/processor.h
> index 01299cdc9806..70d65b482504 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -119,9 +119,16 @@ void release_thread(struct task_struct *);
>   */
>  #define TASK_SIZE_USER64		TASK_SIZE_512TB
>  #define DEFAULT_MAP_WINDOW_USER64	TASK_SIZE_128TB
> +#define TASK_CONTEXT_SIZE		TASK_SIZE_512TB
>  #else
>  #define TASK_SIZE_USER64		TASK_SIZE_64TB
>  #define DEFAULT_MAP_WINDOW_USER64	TASK_SIZE_64TB
> +/*
> + * We don't need allocate extended context id for 4K
> + * page size. We limit max address on this config to
> + * 64TB.
> + */

/*
 * We don't need to allocate extended context ids for 4K page size, because
 * we limit the max effective address on this config to 64TB.
 */

???

> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/e=
xceptions-64s.S
> index 3ac87e53b3da..166b8c0f1830 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -620,8 +620,12 @@ END_MMU_FTR_SECTION_IFCLR(MMU_FTR_TYPE_RADIX)
>  	ld	r10,PACA_EXSLB+EX_LR(r13)
>  	lwz	r9,PACA_EXSLB+EX_CCR(r13)	/* get saved CR */
>  	mtlr	r10

Blank line would be nice.

> +	/*
> +	 * Large address, check whether we have to allocate new
> +	 * contexts.
> +	 */

That doesn't need to wrap.

	/* Large address, check whether we need to allocate new contexts. */

> +	beq-	8f
>=20=20
> -	beq-	8f		/* if bad address, make full stack frame */

And we ended up with two blank lines here which is probably overkill.
=20
> @@ -710,7 +714,7 @@ EXC_COMMON_BEGIN(bad_addr_slb)
>  	std	r10, _TRAP(r1)
>  2:	bl	save_nvgprs
>  	addi	r3, r1, STACK_FRAME_OVERHEAD
> -	bl	slb_miss_bad_addr

You removed the only call to this but didn't remove the function or its
prototype AFAICS.

> diff --git a/arch/powerpc/mm/mmu_context_book3s64.c b/arch/powerpc/mm/mmu=
_context_book3s64.c
> index 80acad52b006..ccc88fa7c35c 100644
> --- a/arch/powerpc/mm/mmu_context_book3s64.c
> +++ b/arch/powerpc/mm/mmu_context_book3s64.c
> @@ -178,6 +178,19 @@ void __destroy_context(int context_id)
>  }
>  EXPORT_SYMBOL_GPL(__destroy_context);
>=20=20
> +static void destroy_contexts(mm_context_t *ctx)
> +{
> +	int index, context_id;
> +
> +	spin_lock(&mmu_context_lock);
> +	for (index =3D 0; index < (TASK_SIZE_USER64/TASK_CONTEXT_SIZE); index++=
) {

ARRAY_SIZE(ctx->extended_id) ?

> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
> index 13cfe413b40d..a93887ee9b22 100644
> --- a/arch/powerpc/mm/slb.c
> +++ b/arch/powerpc/mm/slb.c
> @@ -23,6 +23,7 @@
>  #include <asm/smp.h>
>  #include <linux/compiler.h>
>  #include <linux/mm_types.h>
> +#include <linux/context_tracking.h>

Our includes are generally not well sorted, but can you try to keep them
alphabetical. So after compiler.h.

> @@ -340,3 +341,156 @@ void slb_initialize(void)
>=20=20
>  	asm volatile("isync":::"memory");
>  }
> +
> +/*
> + * Only handle insert of 1TB slb entries.

What if we don't have them (enabled) ?

> + */
> +static void insert_slb_entry(unsigned long vsid, unsigned long ea,
> +			     int bpsize, int ssize)

Parameter alignment again.

> +{
> +	int slb_cache_index;
> +	unsigned long flags;
> +	enum slb_index index;
> +	unsigned long vsid_data, esid_data;

Can you clean those up:

	unsigned long vsid_data, esid_data, flags;
	enum slb_index index;
	int slb_cache_index;

> +
> +	/*
> +	 * We are irq disabled, hence should be safe
> +	 * to access PACA.

Doesn't need to wrap.

> +	 */
> +	index =3D  get_paca()->stab_rr;
                ^
Unneeded extra space there

Blank line please.

> +	/*
> +	 * simple round roubin replacement of slb.

"Simple round-robin"

Although it's not really, it's round-robin but not starting at zero,
starting at SLB_NUM_BOLTED.

> +	 */
> +	if (index < mmu_slb_size)
> +		index++;
> +	else
> +		index =3D SLB_NUM_BOLTED;

Blank line.

> +	get_paca()->stab_rr =3D index;
> +
> +	flags =3D SLB_VSID_USER | mmu_psize_defs[bpsize].sllp;
> +	vsid_data =3D  (vsid << SLB_VSID_SHIFT_1T) | flags |
> +		((unsigned long) ssize << SLB_VSID_SSIZE_SHIFT);

	vsid_data =3D (vsid << SLB_VSID_SHIFT_1T) | flags |
        	    ((unsigned long) ssize << SLB_VSID_SSIZE_SHIFT);

> +	esid_data =3D mk_esid_data(ea, mmu_highuser_ssize, index);
> +
> +	asm volatile("slbmte %0, %1" : : "r" (vsid_data), "r" (esid_data)
> +		     : "memory");
Blank line.
> +	/*
> +	 * Now update slb cache entries
> +	 */
> +	slb_cache_index =3D get_paca()->slb_cache_ptr;
> +	if (slb_cache_index < SLB_CACHE_ENTRIES) {
> +		/*
> +		 * We have space in slb cache for optimized switch_slb().
> +		 * Top 36 bits from esid_data as per ISA
> +		 */
> +		get_paca()->slb_cache[slb_cache_index++] =3D esid_data >> 28;
> +	}
Blank line.
> +	/*
> +	 * if we are full, just increment and return.
> +	 */
> +	get_paca()->slb_cache_ptr++;
> +}
> +
> +static void alloc_extended_context(struct mm_struct *mm, unsigned long e=
a)
> +{
> +	int context_id;
> +
> +	int index =3D ea >> H_BITS_FIRST_CONTEXT;
> +
> +	/*
> +	 * we need to do locking only here. If this value was not set before
> +	 * we will have taken an SLB miss and will reach here. The value will
> +	 * be either 0 or a valid extended context. We need to make sure two
> +	 * parallel SLB miss don't end up allocating extended_context for the
> +	 * same range. The locking below ensures that. For now we take the
> +	 * heavy mmap_sem. But can be changed to per mm_context_t custom lock
> +	 * if needed.
> +	 */
> +	down_read(&mm->mmap_sem);

> +	context_id =3D hash__alloc_context_id();
> +	if (context_id < 0) {
> +		up_read(&mm->mmap_sem);
> +		pagefault_out_of_memory();
> +		return;
> +	}

> +	/* Check for parallel allocation after holding lock */
> +	if (!mm->context.extended_id[index])
> +		mm->context.extended_id[index] =3D context_id;
> +	else
> +		__destroy_context(context_id);

> +	up_read(&mm->mmap_sem);
> +}
> +
> +static void __handle_multi_context_slb_miss(struct pt_regs *regs,
> +					    unsigned long ea)
> +{
> +	int context, bpsize;
> +	unsigned long vsid;
> +	struct mm_struct *mm =3D current->mm;
> +
> +	context =3D get_esid_context(&mm->context, ea);
> +	if (!context) {
> +		/*
> +		 * haven't allocated context yet for this range.
> +		 * Enable irq and allo context and return. We will
> +		 * take an slb miss on this again and come here with
> +		 * allocated context.
> +		 */
> +		/* We restore the interrupt state now */
> +		if (!arch_irq_disabled_regs(regs))
> +			local_irq_enable();
> +		return alloc_extended_context(mm, ea);

I guess I thought we'd do this somewhere in the slice code, when an
address above the limit is first used?

Doing it this way means a stray access to >=3D 512TB will get a lot
further, ie. we'll come in here allocate a context etc, install an SLB
entry and only then realise there's nothing mapped when we look in the
hash. I'd prefer we caught it earlier if possible.

> +	}
> +	/*
> +	 * We are always above 1TB, hence use high user segment size.
> +	 */
> +	vsid =3D __get_vsid(context, ea, mmu_highuser_ssize);
> +	bpsize =3D get_slice_psize(mm, ea);
> +
> +	insert_slb_entry(vsid, ea, bpsize, mmu_highuser_ssize);
> +}
> +
> +/*
> + * exception_enter() handling? FIXME!!

???

> + */
> +void handle_multi_context_slb_miss(struct pt_regs *regs)

I don't love this name, maybe slb_miss_large_addr()?

That would also free-up the non-underscore name for the actual core of
the handler above.

> +{
> +	enum ctx_state prev_state =3D exception_enter();
> +	unsigned long ea =3D regs->dar;
> +
> +	/*
> +	 * Kernel always runs with single context. Hence
> +	 * anything that request for multi context is
> +	 * considered bad slb request.
> +	 */
> +	if (!user_mode(regs))
> +		return bad_page_fault(regs, ea, SIGSEGV);

Can't copy/to_from_user() hit this case?

> +	if (REGION_ID(ea) !=3D USER_REGION_ID)
> +		goto slb_bad_addr;

Blank line please :)

> +	/*
> +	 * Are we beyound what the page table layout support ?

"supports" ?

> +	 */
> +	if ((ea & ~REGION_MASK) >=3D H_PGTABLE_RANGE)
> +		goto slb_bad_addr;
> +
> +#ifdef CONFIG_PPC_MM_SLICES

That can only ever be true in this file, right?

> +	/*
> +	 * consider this as bad slb if we take an slb miss
> +	 * on an address above addr limit.

It's a bad "access" not a "bad slb".

We should probably spell slb SLB in the comments too.

> +	 */
> +	if (ea >=3D current->mm->context.slb_addr_limit)
> +		goto slb_bad_addr;
> +#endif

Blank ..

> +	/* Lower address should be handled by asm code */

"should have been handled"

> +	if (ea < (1UL << H_BITS_FIRST_CONTEXT))
> +		goto slb_bad_addr;
> +
> +	__handle_multi_context_slb_miss(regs, ea);
> +	exception_exit(prev_state);
> +	return;
> +
> +slb_bad_addr:
> +	_exception(SIGSEGV, regs, SEGV_BNDERR, ea);
> +	exception_exit(prev_state);
> +}

I think this would work better if we had:

void slb_bad_addr(regs, ea)
{
	_exception(SIGSEGV, regs, SEGV_BNDERR, ea);
}

...
void handle_multi_context_slb_miss(struct pt_regs *regs)
{
...
	if (ea < (1UL << H_BITS_FIRST_CONTEXT)) {
		slb_bad_addr(regs, ea);
                goto out;
        }
...
	__handle_multi_context_slb_miss(regs, ea);
out:
	exception_exit(prev_state);
}

> diff --git a/arch/powerpc/mm/slb_low.S b/arch/powerpc/mm/slb_low.S
> index 2c7c717fd2ea..c66cb06e73a1 100644
> --- a/arch/powerpc/mm/slb_low.S
> +++ b/arch/powerpc/mm/slb_low.S
> @@ -75,10 +75,12 @@ ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_68_BIT_VA)
>   */
>  _GLOBAL(slb_allocate)
>  	/*
> -	 * check for bad kernel/user address
> +	 * Check for address range for which we need to handle multi context. F=
or
> +         * the default context we allocate the slb via the fast path. Fo=
r large
> +         * address we branch out to C-code and look at additional contex=
t allocated.

Leading white space is fubar.


cheers

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox