* [PATCH] powerpc/lib: fix book3s/32 boot failure due to code patching
From: Christophe Leroy @ 2019-05-15 6:39 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Greg KH,
erhard_f, Michael Neuling
Cc: linuxppc-dev, linux-kernel, stable
[Backport of upstream commit b45ba4a51cde29b2939365ef0c07ad34c8321789]
On powerpc32, patch_instruction() is called by apply_feature_fixups()
which is called from early_init()
There is the following note in front of early_init():
* Note that the kernel may be running at an address which is different
* from the address that it was linked at, so we must use RELOC/PTRRELOC
* to access static data (including strings). -- paulus
Therefore init_mem_is_free must be accessed with PTRRELOC()
Fixes: 1c38a84d4586 ("powerpc: Avoid code patching freed init sections")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=203597
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
Can't apply the upstream commit as such due to several other unrelated stuff
like for instance STRICT_KERNEL_RWX which are missing.
So instead, using same approach as for commit 252eb55816a6f69ef9464cad303cdb3326cdc61d
---
arch/powerpc/lib/code-patching.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 14535ad4cdd1..c312955977ce 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -23,7 +23,7 @@ int patch_instruction(unsigned int *addr, unsigned int instr)
int err;
/* Make sure we aren't patching a freed init section */
- if (init_mem_is_free && init_section_contains(addr, 4)) {
+ if (*PTRRELOC(&init_mem_is_free) && init_section_contains(addr, 4)) {
pr_debug("Skipping init section patching addr: 0x%px\n", addr);
return 0;
}
--
2.13.3
^ permalink raw reply related
* Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
From: Daniel Axtens @ 2019-05-15 6:36 UTC (permalink / raw)
To: Herbert Xu
Cc: leo.barbosa, nayna, Stephan Mueller, Nayna, omosnacek,
Eric Biggers, marcelo.cerri, pfsmorigo, linux-crypto, leitao,
George Wilson, linuxppc-dev
In-Reply-To: <20190515035336.y42wzhs3wzqdpwzn@gondor.apana.org.au>
Herbert Xu <herbert@gondor.apana.org.au> writes:
> On Wed, May 15, 2019 at 03:35:51AM +1000, Daniel Axtens wrote:
>>
>> By all means disable vmx ctr if I don't get an answer to you in a
>> timeframe you are comfortable with, but I am going to at least try to
>> have a look.
>
> I'm happy to give you guys more time. How much time do you think
> you will need?
>
Give me till the end of the week: if I haven't solved it by then I will
probably have to give up and go on to other things anyway.
(FWIW, it seems to happen when encoding greater than 4 but less than 8
AES blocks - in particular with both 7 and 5 blocks encoded I can see it
go wrong from block 4 onwards. No idea why yet, and the asm is pretty
dense, but that's where I'm at at the moment.)
Regards,
Daniel
> Thanks,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [v4 PATCH 1/2] [PowerPC] Add simd.h implementation
From: Christophe Leroy @ 2019-05-15 6:27 UTC (permalink / raw)
To: Shawn Landden; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20190515013725.2198-1-shawn@git.icu>
Le 15/05/2019 à 03:37, Shawn Landden a écrit :
> Based off the x86 one.
>
> WireGuard really wants to be able to do SIMD in interrupts,
> so it can accelerate its in-bound path.
>
> Signed-off-by: Shawn Landden <shawn@git.icu>
> ---
Could you please as usual list here the changes provided by each version
to ease the review ?
Thanks
Christophe
> arch/powerpc/include/asm/simd.h | 17 +++++++++++++++++
> arch/powerpc/kernel/process.c | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 47 insertions(+)
> create mode 100644 arch/powerpc/include/asm/simd.h
>
> diff --git a/arch/powerpc/include/asm/simd.h b/arch/powerpc/include/asm/simd.h
> new file mode 100644
> index 000000000..2fe26f258
> --- /dev/null
> +++ b/arch/powerpc/include/asm/simd.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +/*
> + * may_use_simd - whether it is allowable at this time to issue SIMD
> + * instructions or access the SIMD register file
> + *
> + * It's always ok in process context (ie "not interrupt")
> + * but it is sometimes ok even from an irq.
> + */
> +#ifdef CONFIG_PPC_FPU
> +extern bool may_use_simd(void);
> +#else
> +static inline bool may_use_simd(void)
> +{
> + return false;
> +}
> +#endif
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index dd9e0d538..ef534831f 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -345,6 +345,36 @@ static int restore_altivec(struct task_struct *tsk)
> }
> return 0;
> }
> +
> +/*
> + * Were we in user mode when we were
> + * interrupted?
> + *
> + * Doing kernel_altivec/vsx_begin/end() is ok if we are running
> + * in an interrupt context from user mode - we'll just
> + * save the FPU state as required.
> + */
> +static bool interrupted_user_mode(void)
> +{
> + struct pt_regs *regs = get_irq_regs();
> +
> + return regs && user_mode(regs);
> +}
> +
> +/*
> + * Can we use FPU in kernel mode with the
> + * whole "kernel_fpu/altivec/vsx_begin/end()" sequence?
> + *
> + * It's always ok in process context (ie "not interrupt")
> + * but it is sometimes ok even from an irq.
> + */
> +bool may_use_simd(void)
> +{
> + return !in_interrupt() ||
> + interrupted_user_mode();
> +}
> +EXPORT_SYMBOL(may_use_simd);
> +
> #else
> #define loadvec(thr) 0
> static inline int restore_altivec(struct task_struct *tsk) { return 0; }
>
^ permalink raw reply
* Re: [PATCH] powerpc/mm/book3s64: Implement STRICT_MODULE_RWX
From: Christophe Leroy @ 2019-05-15 6:24 UTC (permalink / raw)
To: Russell Currey, linuxppc-dev
In-Reply-To: <20190515013000.16085-1-ruscur@russell.cc>
Le 15/05/2019 à 03:30, Russell Currey a écrit :
> Strict module RWX is just like strict kernel RWX, but for modules - so
> loadable modules aren't marked both writable and executable at the same
> time. This is handled by the generic code in kernel/module.c, and
> simply requires the architecture to implement the set_memory() set of
> functions, declared with ARCH_HAS_SET_MEMORY.
>
> The set_memory() family of functions are implemented for book3s64
> MMUs (so Hash and Radix), however they could likely be adapted to work
> for other platforms as well and made more generic. I did it this way
> since they're the platforms I have the most understanding of and ability
> to test.
Based on this patch, I have drafted a generic implementation. Please
comment and test. I'll test on my side on PPC32.
Christophe
>
> There's nothing other than these functions required to turn
> ARCH_HAS_STRICT_MODULE_RWX on, so turn that on too.
>
> With STRICT_MODULE_RWX enabled, there are as many W+X pages at runtime
> as there are with CONFIG_MODULES=n (none), so in my testing it works
> well on both Hash and Radix.
>
> There's a TODO in the code for also applying the page permission changes
> to the backing pages in the linear mapping: this is pretty simple for
> Radix and (seemingly) a lot harder for Hash, so I've left it for now
> since there's still a notable security benefit for the patch as-is.
>
> Technically can be enabled without STRICT_KERNEL_RWX, but I don't think
> that gets you a whole lot, so I think we should leave it off by default
> until we can get STRICT_KERNEL_RWX to the point where it's enabled by
> default.
>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> ---
> arch/powerpc/Kconfig | 2 +
> arch/powerpc/include/asm/set_memory.h | 12 +++
> arch/powerpc/mm/book3s64/Makefile | 2 +-
> arch/powerpc/mm/book3s64/pageattr.c | 106 ++++++++++++++++++++++++++
> 4 files changed, 121 insertions(+), 1 deletion(-)
> create mode 100644 arch/powerpc/include/asm/set_memory.h
> create mode 100644 arch/powerpc/mm/book3s64/pageattr.c
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d7996cfaceca..9e1bfa81bc5a 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -131,7 +131,9 @@ config PPC
> select ARCH_HAS_PTE_SPECIAL
> select ARCH_HAS_MEMBARRIER_CALLBACKS
> select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC64
> + select ARCH_HAS_SET_MEMORY if PPC_BOOK3S_64
> select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !RELOCATABLE && !HIBERNATION)
> + select ARCH_HAS_STRICT_MODULE_RWX if PPC_BOOK3S_64
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_HAS_UACCESS_FLUSHCACHE if PPC64
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> diff --git a/arch/powerpc/include/asm/set_memory.h b/arch/powerpc/include/asm/set_memory.h
> new file mode 100644
> index 000000000000..5323a8b06f98
> --- /dev/null
> +++ b/arch/powerpc/include/asm/set_memory.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +#ifndef _ASM_POWERPC_SET_MEMORY_H
> +#define _ASM_POWERPC_SET_MEMORY_H
> +
> +#ifdef CONFIG_PPC_BOOK3S_64
> +int set_memory_ro(unsigned long addr, int numpages);
> +int set_memory_rw(unsigned long addr, int numpages);
> +int set_memory_nx(unsigned long addr, int numpages);
> +int set_memory_x(unsigned long addr, int numpages);
> +#endif
> +
> +#endif
> diff --git a/arch/powerpc/mm/book3s64/Makefile b/arch/powerpc/mm/book3s64/Makefile
> index 974b4fc19f4f..09c5afadf235 100644
> --- a/arch/powerpc/mm/book3s64/Makefile
> +++ b/arch/powerpc/mm/book3s64/Makefile
> @@ -5,7 +5,7 @@ ccflags-y := $(NO_MINIMAL_TOC)
> CFLAGS_REMOVE_slb.o = $(CC_FLAGS_FTRACE)
>
> obj-y += hash_pgtable.o hash_utils.o slb.o \
> - mmu_context.o pgtable.o hash_tlb.o
> + mmu_context.o pgtable.o hash_tlb.o pageattr.o
> obj-$(CONFIG_PPC_NATIVE) += hash_native.o
> obj-$(CONFIG_PPC_RADIX_MMU) += radix_pgtable.o radix_tlb.o
> obj-$(CONFIG_PPC_4K_PAGES) += hash_4k.o
> diff --git a/arch/powerpc/mm/book3s64/pageattr.c b/arch/powerpc/mm/book3s64/pageattr.c
> new file mode 100644
> index 000000000000..d6afa89fb407
> --- /dev/null
> +++ b/arch/powerpc/mm/book3s64/pageattr.c
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Page attribute and set_memory routines for Radix and Hash MMUs
> + *
> + * Derived from the arm64 implementation.
> + *
> + * Author: Russell Currey <ruscur@russell.cc>
> + *
> + * Copyright 2019, IBM Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
Above text should be removed as it is redundant with the
SPDX-Licence-Identifier
> + */
> +
> +#include <linux/mm.h>
> +#include <linux/vmalloc.h>
> +
> +#include <asm/mmu.h>
> +#include <asm/page.h>
> +#include <asm/pgtable.h>
> +
> +// we need this to have a single pointer to pass into apply_to_page_range()
> +struct page_change_data {
> + pgprot_t set_mask;
> + pgprot_t clear_mask;
> +};
> +
> +static pte_t clear_pte_bit(pte_t pte, pgprot_t prot)
> +{
> + return __pte(pte_val(pte) & ~pgprot_val(prot));
> +}
> +
> +static pte_t set_pte_bit(pte_t pte, pgprot_t prot)
> +{
> + return __pte(pte_val(pte) | pgprot_val(prot));
> +}
> +
> +static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long addr,
> + void *data)
> +{
> + struct page_change_data *cdata = data;
> + pte_t pte = READ_ONCE(*ptep);
> +
> + pte = clear_pte_bit(pte, cdata->clear_mask);
> + pte = set_pte_bit(pte, cdata->set_mask);
> +
> + set_pte_at(&init_mm, addr, ptep, pte);
> + return 0;
> +}
> +
> +static int change_memory(unsigned long addr, int numpages, pgprot_t set_mask,
> + pgprot_t clear_mask)
> +{
> + unsigned long size = numpages * PAGE_SIZE;
> + unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
> + unsigned long end = PAGE_ALIGN(start + size);
We have aligned the start and the size, so the end is automatically aligned.
Christophe
> + struct page_change_data data;
> + struct vm_struct *area;
> + int ret;
> +
> + if (!numpages)
> + return 0;
> +
> + // only operate on VM areas for now
> + area = find_vm_area((void *)addr);
> + if (!area || end > (unsigned long)area->addr + area->size ||
> + !(area->flags & VM_ALLOC))
> + return -EINVAL;
> +
> + // TODO: also apply change to the backing pages in the linear mapping
> + data.set_mask = set_mask;
> + data.clear_mask = clear_mask;
> +
> + ret = apply_to_page_range(&init_mm, start, size, change_page_range,
> + &data);
> +
> + flush_tlb_kernel_range(start, end);
> + return ret;
> +}
> +
> +int set_memory_ro(unsigned long addr, int numpages)
> +{
> + return change_memory(addr, numpages,
> + __pgprot(0), __pgprot(_PAGE_WRITE));
> +}
> +
> +int set_memory_rw(unsigned long addr, int numpages)
> +{
> + return change_memory(addr, numpages,
> + __pgprot(_PAGE_RW), __pgprot(0));
> +}
> +
> +int set_memory_nx(unsigned long addr, int numpages)
> +{
> + return change_memory(addr, numpages,
> + __pgprot(0), __pgprot(_PAGE_EXEC));
> +}
> +
> +int set_memory_x(unsigned long addr, int numpages)
> +{
> + return change_memory(addr, numpages,
> + __pgprot(_PAGE_EXEC), __pgprot(0));
> +}
>
^ permalink raw reply
* Re: [PATCH] vsprintf: Do not break early boot with probing addresses
From: Sergey Senozhatsky @ 2019-05-15 6:21 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Petr Mladek, linux-arch@vger.kernel.org, Sergey Senozhatsky,
Heiko Carstens, linux-s390@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, Rasmus Villemoes,
linux-kernel@vger.kernel.org, Steven Rostedt, Michal Hocko,
Sergey Senozhatsky, David Laight, Stephen Rothwell,
Andy Shevchenko, Linus Torvalds, Martin Schwidefsky,
Tobin C . Harding
In-Reply-To: <CAMuHMdUhy3uB+G23uXh__F2Y_Jsam5uS1Q5jJC95kWAOEM8WRA@mail.gmail.com>
On (05/14/19 21:13), Geert Uytterhoeven wrote:
> I would immediately understand there's a missing IS_ERR() check in a
> function that can return -EINVAL, without having to add a new printk()
> to find out what kind of bogus value has been received, and without
> having to reboot, and trying to reproduce...
But chances are that missing IS_ERR() will crash the kernel sooner
or later (in general case), if not in sprintf() then somewhere else.
-ss
^ permalink raw reply
* [RFC PATCH] powerpc/mm: Implement STRICT_MODULE_RWX
From: Christophe Leroy @ 2019-05-15 6:20 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Russell Currey
Cc: linuxppc-dev, linux-kernel
Strict module RWX is just like strict kernel RWX, but for modules - so
loadable modules aren't marked both writable and executable at the same
time. This is handled by the generic code in kernel/module.c, and
simply requires the architecture to implement the set_memory() set of
functions, declared with ARCH_HAS_SET_MEMORY.
There's nothing other than these functions required to turn
ARCH_HAS_STRICT_MODULE_RWX on, so turn that on too.
With STRICT_MODULE_RWX enabled, there are as many W+X pages at runtime
as there are with CONFIG_MODULES=n (none), so in Russel's testing it works
well on both Hash and Radix book3s64.
There's a TODO in the code for also applying the page permission changes
to the backing pages in the linear mapping: this is pretty simple for
Radix and (seemingly) a lot harder for Hash, so I've left it for now
since there's still a notable security benefit for the patch as-is.
Technically can be enabled without STRICT_KERNEL_RWX, but
that doesn't gets you a whole lot, so we should leave it off by default
until we can get STRICT_KERNEL_RWX to the point where it's enabled by
default.
Signed-off-by: Russell Currey <ruscur@russell.cc>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
Generic implementation based on Russel's patch ("powerpc/mm/book3s64: Implement STRICT_MODULE_RWX")
Untested
arch/powerpc/Kconfig | 2 +
arch/powerpc/include/asm/set_memory.h | 32 +++++++++++++
arch/powerpc/mm/Makefile | 2 +-
arch/powerpc/mm/pageattr.c | 85 +++++++++++++++++++++++++++++++++++
4 files changed, 120 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/include/asm/set_memory.h
create mode 100644 arch/powerpc/mm/pageattr.c
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d7996cfaceca..1f1423e3d818 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -131,7 +131,9 @@ config PPC
select ARCH_HAS_PTE_SPECIAL
select ARCH_HAS_MEMBARRIER_CALLBACKS
select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC64
+ select ARCH_HAS_SET_MEMORY
select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !RELOCATABLE && !HIBERNATION)
+ select ARCH_HAS_STRICT_MODULE_RWX if PPC_BOOK3S_64 || PPC32
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_HAS_UACCESS_FLUSHCACHE if PPC64
select ARCH_HAS_UBSAN_SANITIZE_ALL
diff --git a/arch/powerpc/include/asm/set_memory.h b/arch/powerpc/include/asm/set_memory.h
new file mode 100644
index 000000000000..4b9683f3b3dd
--- /dev/null
+++ b/arch/powerpc/include/asm/set_memory.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+#ifndef _ASM_POWERPC_SET_MEMORY_H
+#define _ASM_POWERPC_SET_MEMORY_H
+
+#define SET_MEMORY_RO 1
+#define SET_MEMORY_RW 2
+#define SET_MEMORY_NX 3
+#define SET_MEMORY_X 4
+
+int change_memory(unsigned long addr, int numpages, int action);
+
+static inline int set_memory_ro(unsigned long addr, int numpages)
+{
+ return change_memory(addr, numpages, SET_MEMORY_RO);
+}
+
+static inline int set_memory_rw(unsigned long addr, int numpages)
+{
+ return change_memory(addr, numpages, SET_MEMORY_RW);
+}
+
+static inline int set_memory_nx(unsigned long addr, int numpages)
+{
+ return change_memory(addr, numpages, SET_MEMORY_NX);
+}
+
+static inline int set_memory_x(unsigned long addr, int numpages)
+{
+ return change_memory(addr, numpages, SET_MEMORY_X);
+}
+
+#endif
diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
index 0f499db315d6..b683d1c311b3 100644
--- a/arch/powerpc/mm/Makefile
+++ b/arch/powerpc/mm/Makefile
@@ -7,7 +7,7 @@ ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
obj-y := fault.o mem.o pgtable.o mmap.o \
init_$(BITS).o pgtable_$(BITS).o \
- pgtable-frag.o \
+ pgtable-frag.o pageattr.o \
init-common.o mmu_context.o drmem.o
obj-$(CONFIG_PPC_MMU_NOHASH) += nohash/
obj-$(CONFIG_PPC_BOOK3S_32) += book3s32/
diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
new file mode 100644
index 000000000000..3e8f2c203a00
--- /dev/null
+++ b/arch/powerpc/mm/pageattr.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Page attribute and set_memory routines
+ *
+ * Derived from the arm64 implementation.
+ *
+ * Author: Russell Currey <ruscur@russell.cc>
+ *
+ * Copyright 2019, IBM Corporation.
+ *
+ */
+
+#include <linux/mm.h>
+#include <linux/set_memory.h>
+#include <linux/vmalloc.h>
+
+#include <asm/mmu.h>
+#include <asm/page.h>
+#include <asm/pgtable.h>
+
+static int change_page_ro(pte_t *ptep, pgtable_t token, unsigned long addr, void *data)
+{
+ set_pte_at(&init_mm, addr, ptep, pte_wrprotect(READ_ONCE(*ptep)));
+ return 0;
+}
+
+static int change_page_rw(pte_t *ptep, pgtable_t token, unsigned long addr, void *data)
+{
+ set_pte_at(&init_mm, addr, ptep, pte_mkwrite(READ_ONCE(*ptep)));
+ return 0;
+}
+
+static int change_page_nx(pte_t *ptep, pgtable_t token, unsigned long addr, void *data)
+{
+ set_pte_at(&init_mm, addr, ptep, pte_exprotect(READ_ONCE(*ptep)));
+ return 0;
+}
+
+static int change_page_x(pte_t *ptep, pgtable_t token, unsigned long addr, void *data)
+{
+ set_pte_at(&init_mm, addr, ptep, pte_mkexec(READ_ONCE(*ptep)));
+ return 0;
+}
+
+int change_memory(unsigned long addr, int numpages, int action)
+{
+ unsigned long size = numpages * PAGE_SIZE;
+ unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
+ unsigned long end = start + size;
+ struct vm_struct *area;
+ int ret;
+
+ if (!numpages)
+ return 0;
+
+ // only operate on VM areas for now
+ area = find_vm_area((void *)addr);
+ if (!area || end > (unsigned long)area->addr + area->size ||
+ !(area->flags & VM_ALLOC))
+ return -EINVAL;
+
+ // TODO: also apply change to the backing pages in the linear mapping
+
+ switch (action) {
+ case SET_MEMORY_RO:
+ ret = apply_to_page_range(&init_mm, start, size, change_page_ro, NULL);
+ break;
+ case SET_MEMORY_RW:
+ ret = apply_to_page_range(&init_mm, start, size, change_page_rw, NULL);
+ break;
+ case SET_MEMORY_NX:
+ ret = apply_to_page_range(&init_mm, start, size, change_page_nx, NULL);
+ break;
+ case SET_MEMORY_X:
+ ret = apply_to_page_range(&init_mm, start, size, change_page_x, NULL);
+ break;
+ default:
+ WARN_ON(true);
+ return -EINVAL;
+ }
+
+ flush_tlb_kernel_range(start, end);
+ return ret;
+}
--
2.13.3
^ permalink raw reply related
* Re: [PATCH] powerpc/security: Fix build break
From: Greg Kroah-Hartman @ 2019-05-15 6:08 UTC (permalink / raw)
To: Joel Stanley; +Cc: linuxppc-dev, linux-kernel, stable, Josh Poimboeuf
In-Reply-To: <20190515051830.GA18166@kroah.com>
On Wed, May 15, 2019 at 07:18:30AM +0200, Greg Kroah-Hartman wrote:
> On Wed, May 15, 2019 at 02:22:06PM +0930, Joel Stanley wrote:
> > This fixes a build break introduced in with the recent round of CPU
> > bug patches.
> >
> > arch/powerpc/kernel/security.c: In function ‘setup_barrier_nospec’:
> > arch/powerpc/kernel/security.c:59:21: error: implicit declaration of
> > function ‘cpu_mitigations_off’ [-Werror=implicit-function-declaration]
> > if (!no_nospec && !cpu_mitigations_off())
> > ^~~~~~~~~~~~~~~~~~~
> >
> > Fixes: 782e69efb3df ("powerpc/speculation: Support 'mitigations=' cmdline option")
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> > This should be applied to the 4.14 and 4.19 trees. There is no issue
> > with 5.1. The commit message contains a fixes line for the commit in
> > Linus tree.
> > ---
> > arch/powerpc/kernel/security.c | 1 +
> > 1 file changed, 1 insertion(+)
>
> Isn't this just commit 42e2acde1237 ("powerpc/64s: Include cpu header")?
Which I have now queued up.
^ permalink raw reply
* Re: [PATCH] powerpc/security: Fix build break
From: Greg Kroah-Hartman @ 2019-05-15 5:18 UTC (permalink / raw)
To: Joel Stanley; +Cc: linuxppc-dev, linux-kernel, stable, Josh Poimboeuf
In-Reply-To: <20190515045206.10610-1-joel@jms.id.au>
On Wed, May 15, 2019 at 02:22:06PM +0930, Joel Stanley wrote:
> This fixes a build break introduced in with the recent round of CPU
> bug patches.
>
> arch/powerpc/kernel/security.c: In function ‘setup_barrier_nospec’:
> arch/powerpc/kernel/security.c:59:21: error: implicit declaration of
> function ‘cpu_mitigations_off’ [-Werror=implicit-function-declaration]
> if (!no_nospec && !cpu_mitigations_off())
> ^~~~~~~~~~~~~~~~~~~
>
> Fixes: 782e69efb3df ("powerpc/speculation: Support 'mitigations=' cmdline option")
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> This should be applied to the 4.14 and 4.19 trees. There is no issue
> with 5.1. The commit message contains a fixes line for the commit in
> Linus tree.
> ---
> arch/powerpc/kernel/security.c | 1 +
> 1 file changed, 1 insertion(+)
Isn't this just commit 42e2acde1237 ("powerpc/64s: Include cpu header")?
thanks,
greg k-h
^ permalink raw reply
* [PATCH] powerpc/security: Fix build break
From: Joel Stanley @ 2019-05-15 4:52 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linuxppc-dev, linux-kernel, stable, Josh Poimboeuf
This fixes a build break introduced in with the recent round of CPU
bug patches.
arch/powerpc/kernel/security.c: In function ‘setup_barrier_nospec’:
arch/powerpc/kernel/security.c:59:21: error: implicit declaration of
function ‘cpu_mitigations_off’ [-Werror=implicit-function-declaration]
if (!no_nospec && !cpu_mitigations_off())
^~~~~~~~~~~~~~~~~~~
Fixes: 782e69efb3df ("powerpc/speculation: Support 'mitigations=' cmdline option")
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
This should be applied to the 4.14 and 4.19 trees. There is no issue
with 5.1. The commit message contains a fixes line for the commit in
Linus tree.
---
arch/powerpc/kernel/security.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index e9af5d9badf2..68d4ec373cfc 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -4,6 +4,7 @@
//
// Copyright 2018, Michael Ellerman, IBM Corporation.
+#include <linux/cpu.h>
#include <linux/kernel.h>
#include <linux/device.h>
#include <linux/seq_buf.h>
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] powerpc/boot: fix broken way to pass CONFIG options
From: Masahiro Yamada @ 2019-05-15 4:04 UTC (permalink / raw)
To: Oliver
Cc: Rob Herring, Rodrigo R. Galvao, Linux Kernel Mailing List,
Nicholas Piggin, Mark Greer, Paul Mackerras, Joel Stanley,
linuxppc-dev
In-Reply-To: <CAOSf1CFqiKK-=aRU0kYPajY8rmjrFVdMi+AA692rXwLrC7S2Lg@mail.gmail.com>
On Mon, May 13, 2019 at 11:24 PM Oliver <oohall@gmail.com> wrote:
>
> On Mon, May 13, 2019 at 9:23 PM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > Commit 5e9dcb6188a4 ("powerpc/boot: Expose Kconfig symbols to wrapper")
> > was wrong, but commit e41b93a6be57 ("powerpc/boot: Fix build failures
> > with -j 1") was also wrong.
> >
> > Check-in source files never ever depend on build artifacts.
> >
> > The correct dependency is:
> >
> > $(obj)/serial.o: $(obj)/autoconf.h
> >
> > However, copying autoconf.h to arch/power/boot/ is questionable
> > in the first place.
> >
> > arch/powerpc/Makefile adopted multiple ways to pass CONFIG options.
> >
> > arch/powerpc/boot/decompress.c references CONFIG_KERNEL_GZIP and
> > CONFIG_KERNEL_XZ, which are passed via the command line.
> >
> > arch/powerpc/boot/serial.c includes the copied autoconf.h to
> > reference a couple of CONFIG options.
> >
> > Do not do this.
> >
> > We should have already learned that including autoconf.h from each
> > source file is really fragile.
> >
> > In fact, it is already broken.
> >
> > arch/powerpc/boot/ppc_asm.h references CONFIG_PPC_8xx, but
> > arch/powerpc/boot/utils.S is not given any way to access CONFIG
> > options. So, CONFIG_PPC_8xx is never defined here.
> >
> > Just pass $(LINUXINCLUDE) and remove all broken code.
>
> I'm not sure how safe this is.
I chose to use $(LINUXINCLUDE) since this is
what other boot decompressors do.
(e.g. arch/arm/boot/compressed)
With this, two headers are included:
include/linux/kconfig.h
include/generated/autoconf.h
The first one enables to use IS_ENABLE(), etc.
The second one is the list of CONFIG options.
If you want to minimize the number of included headers,
you can add
-include include/generated/autoconf.h instead.
> The original reason for the
> CONFIG_KERNEL_XZ hack in the makefile was because the kernel headers
> couldn't be included directly. The bootwrapper is compiled with a
> 32bit toolchain when the kernel is compiled for 64bit big endian
> because of older systems with broken firmware that can't load 64bit
> ELFs directly.
I do not see CONFIG_32BIT or CONFIG_64BIT in arch/power/boot.
If you are saying "including autoconf.h itself is bad",
we should revert 5e9dcb6188a40e604e66dc30fab30c2be89aa1cc
But, I doubt it.
> When I added XZ support to the wrapper I did experiment
> with including the kernel headers directly and couldn't make it work
> reliably.
Right. I am pretty sure it won't work.
But, it is unrelated to including CONFIG options.
I am not a PPC developer, so I am not excited about
looking into boot wrapper.
When I was seeing Makefiles for my Kbuild refactoring,
I found
$(srctree)/$(src)/serial.c: $(obj)/autoconf.h
then I shuddered.
If PPC folks want to keep the breakage as is,
I can send the following.
It it Michael's call.
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -202,7 +202,7 @@ $(obj)/empty.c:
$(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds : $(obj)/%: $(srctree)/$(src)/%.S
$(Q)cp $< $@
-$(srctree)/$(src)/serial.c: $(obj)/autoconf.h
+$(obj)/serial.o: $(obj)/autoconf.h
$(obj)/autoconf.h: $(obj)/%: $(objtree)/include/generated/%
$(Q)cp $< $@
> I don't remember what the exact reason was, but I think it
> was something to do with the generated headers not always matching
> what you would expect when compiling for 32bit. It's also possible I
> was just being paranoid. Either way it's about time we found a real
> fix...
>
> The stuff in serial.c and ppc_asm.h was added later to work around
> other issues without anyone thinking too hard about it. Oh well.
>
> > I also removed the -traditional flag to make include/linux/kconfig.h
> > work. I do not understand why it needs to imitate the behavior of
> > pre-standard C preprocessors.
>
> I'm not sure why it's there either. The boot wrapper was re-written at
> some point so it might just be a hold over from the dawn of time.
>
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >
> > arch/powerpc/boot/.gitignore | 2 --
> > arch/powerpc/boot/Makefile | 14 +++-----------
> > arch/powerpc/boot/serial.c | 1 -
> > 3 files changed, 3 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/.gitignore b/arch/powerpc/boot/.gitignore
> > index 32034a0cc554..6610665fcf5e 100644
> > --- a/arch/powerpc/boot/.gitignore
> > +++ b/arch/powerpc/boot/.gitignore
> > @@ -44,5 +44,3 @@ fdt_sw.c
> > fdt_wip.c
> > libfdt.h
> > libfdt_internal.h
> > -autoconf.h
> > -
> > diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> > index 73d1f3562978..b8a82be2af2a 100644
> > --- a/arch/powerpc/boot/Makefile
> > +++ b/arch/powerpc/boot/Makefile
> > @@ -20,9 +20,6 @@
> >
> > all: $(obj)/zImage
> >
> > -compress-$(CONFIG_KERNEL_GZIP) := CONFIG_KERNEL_GZIP
> > -compress-$(CONFIG_KERNEL_XZ) := CONFIG_KERNEL_XZ
> > -
> > ifdef CROSS32_COMPILE
> > BOOTCC := $(CROSS32_COMPILE)gcc
> > BOOTAR := $(CROSS32_COMPILE)ar
> > @@ -34,7 +31,7 @@ endif
> > BOOTCFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> > -fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx \
> > -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \
> > - -D$(compress-y)
> > + $(LINUXINCLUDE)
> >
> > ifdef CONFIG_PPC64_BOOT_WRAPPER
> > BOOTCFLAGS += -m64
> > @@ -51,7 +48,7 @@ BOOTCFLAGS += -mlittle-endian
> > BOOTCFLAGS += $(call cc-option,-mabi=elfv2)
> > endif
> >
> > -BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -traditional -nostdinc
> > +BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -nostdinc
> >
> > BOOTARFLAGS := -cr$(KBUILD_ARFLAGS)
> >
> > @@ -202,14 +199,9 @@ $(obj)/empty.c:
> > $(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds : $(obj)/%: $(srctree)/$(src)/%.S
> > $(Q)cp $< $@
> >
> > -$(srctree)/$(src)/serial.c: $(obj)/autoconf.h
> > -
> > -$(obj)/autoconf.h: $(obj)/%: $(objtree)/include/generated/%
> > - $(Q)cp $< $@
> > -
> > clean-files := $(zlib-) $(zlibheader-) $(zliblinuxheader-) \
> > $(zlib-decomp-) $(libfdt) $(libfdtheader) \
> > - autoconf.h empty.c zImage.coff.lds zImage.ps3.lds zImage.lds
> > + empty.c zImage.coff.lds zImage.ps3.lds zImage.lds
> >
> > quiet_cmd_bootcc = BOOTCC $@
> > cmd_bootcc = $(BOOTCC) -Wp,-MD,$(depfile) $(BOOTCFLAGS) -c -o $@ $<
> > diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c
> > index b0491b8c0199..9457863147f9 100644
> > --- a/arch/powerpc/boot/serial.c
> > +++ b/arch/powerpc/boot/serial.c
> > @@ -18,7 +18,6 @@
> > #include "stdio.h"
> > #include "io.h"
> > #include "ops.h"
> > -#include "autoconf.h"
> >
> > static int serial_open(void)
> > {
> > --
> > 2.17.1
> >
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
From: Herbert Xu @ 2019-05-15 3:53 UTC (permalink / raw)
To: Daniel Axtens
Cc: leo.barbosa, nayna, Stephan Mueller, Nayna, omosnacek,
Eric Biggers, marcelo.cerri, pfsmorigo, linux-crypto, leitao,
George Wilson, linuxppc-dev
In-Reply-To: <877eat0wi0.fsf@dja-thinkpad.axtens.net>
On Wed, May 15, 2019 at 03:35:51AM +1000, Daniel Axtens wrote:
>
> By all means disable vmx ctr if I don't get an answer to you in a
> timeframe you are comfortable with, but I am going to at least try to
> have a look.
I'm happy to give you guys more time. How much time do you think
you will need?
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [v4 PATCH 2/2] [PowerPC] Allow use of SIMD in interrupts from kernel code
From: Shawn Landden @ 2019-05-15 1:37 UTC (permalink / raw)
Cc: Paul Mackerras, Shawn Landden, linuxppc-dev
In-Reply-To: <20190515013725.2198-1-shawn@git.icu>
This second patch is separate because it could be wrong,
like I am not sure about how kernel thread migration works,
and it is even allowing simd in preemptible kernel code.
v4: fix build without CONFIG_AVX
Signed-off-by: Shawn Landden <shawn@git.icu>
---
arch/powerpc/include/asm/switch_to.h | 15 +---
arch/powerpc/kernel/process.c | 117 +++++++++++++++++++--------
2 files changed, 88 insertions(+), 44 deletions(-)
diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
index 5b03d8a82..c79f7d24a 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -30,10 +30,7 @@ extern void enable_kernel_fp(void);
extern void flush_fp_to_thread(struct task_struct *);
extern void giveup_fpu(struct task_struct *);
extern void save_fpu(struct task_struct *);
-static inline void disable_kernel_fp(void)
-{
- msr_check_and_clear(MSR_FP);
-}
+extern void disable_kernel_fp(void);
#else
static inline void save_fpu(struct task_struct *t) { }
static inline void flush_fp_to_thread(struct task_struct *t) { }
@@ -44,10 +41,7 @@ extern void enable_kernel_altivec(void);
extern void flush_altivec_to_thread(struct task_struct *);
extern void giveup_altivec(struct task_struct *);
extern void save_altivec(struct task_struct *);
-static inline void disable_kernel_altivec(void)
-{
- msr_check_and_clear(MSR_VEC);
-}
+extern void disable_kernel_altivec(void);
#else
static inline void save_altivec(struct task_struct *t) { }
static inline void __giveup_altivec(struct task_struct *t) { }
@@ -56,10 +50,7 @@ static inline void __giveup_altivec(struct task_struct *t) { }
#ifdef CONFIG_VSX
extern void enable_kernel_vsx(void);
extern void flush_vsx_to_thread(struct task_struct *);
-static inline void disable_kernel_vsx(void)
-{
- msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
-}
+extern void disable_kernel_vsx(void);
#endif
#ifdef CONFIG_SPE
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index ef534831f..0136fd132 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -170,6 +170,29 @@ void __msr_check_and_clear(unsigned long bits)
EXPORT_SYMBOL(__msr_check_and_clear);
#ifdef CONFIG_PPC_FPU
+/*
+ * Track whether the kernel is using the FPU state
+ * currently.
+ *
+ * This flag is used:
+ *
+ * - by IRQ context code to potentially use the FPU
+ * if it's unused.
+ *
+ * - to debug kernel_fpu/altivec/vsx_begin()/end() correctness
+ */
+static DEFINE_PER_CPU(bool, in_kernel_fpu);
+
+static bool kernel_fpu_disabled(void)
+{
+ return this_cpu_read(in_kernel_fpu);
+}
+
+static bool interrupted_kernel_fpu_idle(void)
+{
+ return !kernel_fpu_disabled();
+}
+
static void __giveup_fpu(struct task_struct *tsk)
{
unsigned long msr;
@@ -230,7 +253,8 @@ void enable_kernel_fp(void)
{
unsigned long cpumsr;
- WARN_ON(preemptible());
+ WARN_ON_ONCE(this_cpu_read(in_kernel_fpu));
+ this_cpu_write(in_kernel_fpu, true);
cpumsr = msr_check_and_set(MSR_FP);
@@ -251,6 +275,15 @@ void enable_kernel_fp(void)
}
EXPORT_SYMBOL(enable_kernel_fp);
+void disable_kernel_fp(void)
+{
+ WARN_ON_ONCE(!this_cpu_read(in_kernel_fpu));
+ this_cpu_write(in_kernel_fpu, false);
+
+ msr_check_and_clear(MSR_FP);
+}
+EXPORT_SYMBOL(disable_kernel_fp);
+
static int restore_fp(struct task_struct *tsk)
{
if (tsk->thread.load_fp || tm_active_with_fp(tsk)) {
@@ -260,6 +293,37 @@ static int restore_fp(struct task_struct *tsk)
}
return 0;
}
+
+/*
+ * Were we in user mode when we were
+ * interrupted?
+ *
+ * Doing kernel_altivec/vsx_begin/end() is ok if we are running
+ * in an interrupt context from user mode - we'll just
+ * save the FPU state as required.
+ */
+static bool interrupted_user_mode(void)
+{
+ struct pt_regs *regs = get_irq_regs();
+
+ return regs && user_mode(regs);
+}
+
+/*
+ * Can we use FPU in kernel mode with the
+ * whole "kernel_fpu/altivec/vsx_begin/end()" sequence?
+ *
+ * It's always ok in process context (ie "not interrupt")
+ * but it is sometimes ok even from an irq.
+ */
+bool may_use_simd(void)
+{
+ return !in_interrupt() ||
+ interrupted_user_mode() ||
+ interrupted_kernel_fpu_idle();
+}
+EXPORT_SYMBOL(may_use_simd);
+
#else
static int restore_fp(struct task_struct *tsk) { return 0; }
#endif /* CONFIG_PPC_FPU */
@@ -295,7 +359,8 @@ void enable_kernel_altivec(void)
{
unsigned long cpumsr;
- WARN_ON(preemptible());
+ WARN_ON_ONCE(this_cpu_read(in_kernel_fpu));
+ this_cpu_write(in_kernel_fpu, true);
cpumsr = msr_check_and_set(MSR_VEC);
@@ -316,6 +381,14 @@ void enable_kernel_altivec(void)
}
EXPORT_SYMBOL(enable_kernel_altivec);
+extern void disable_kernel_altivec(void)
+{
+ WARN_ON_ONCE(!this_cpu_read(in_kernel_fpu));
+ this_cpu_write(in_kernel_fpu, false);
+ msr_check_and_clear(MSR_VEC);
+}
+EXPORT_SYMBOL(disable_kernel_altivec);
+
/*
* Make sure the VMX/Altivec register state in the
* the thread_struct is up to date for task tsk.
@@ -346,35 +419,6 @@ static int restore_altivec(struct task_struct *tsk)
return 0;
}
-/*
- * Were we in user mode when we were
- * interrupted?
- *
- * Doing kernel_altivec/vsx_begin/end() is ok if we are running
- * in an interrupt context from user mode - we'll just
- * save the FPU state as required.
- */
-static bool interrupted_user_mode(void)
-{
- struct pt_regs *regs = get_irq_regs();
-
- return regs && user_mode(regs);
-}
-
-/*
- * Can we use FPU in kernel mode with the
- * whole "kernel_fpu/altivec/vsx_begin/end()" sequence?
- *
- * It's always ok in process context (ie "not interrupt")
- * but it is sometimes ok even from an irq.
- */
-bool may_use_simd(void)
-{
- return !in_interrupt() ||
- interrupted_user_mode();
-}
-EXPORT_SYMBOL(may_use_simd);
-
#else
#define loadvec(thr) 0
static inline int restore_altivec(struct task_struct *tsk) { return 0; }
@@ -411,7 +455,8 @@ void enable_kernel_vsx(void)
{
unsigned long cpumsr;
- WARN_ON(preemptible());
+ WARN_ON_ONCE(this_cpu_read(in_kernel_fpu));
+ this_cpu_write(in_kernel_fpu, true);
cpumsr = msr_check_and_set(MSR_FP|MSR_VEC|MSR_VSX);
@@ -433,6 +478,14 @@ void enable_kernel_vsx(void)
}
EXPORT_SYMBOL(enable_kernel_vsx);
+void disable_kernel_vsx(void)
+{
+ WARN_ON_ONCE(!this_cpu_read(in_kernel_fpu));
+ this_cpu_write(in_kernel_fpu, false);
+ msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
+}
+EXPORT_SYMBOL(disable_kernel_vsx);
+
void flush_vsx_to_thread(struct task_struct *tsk)
{
if (tsk->thread.regs) {
--
2.21.0.1020.gf2820cf01a
^ permalink raw reply related
* [v4 PATCH 1/2] [PowerPC] Add simd.h implementation
From: Shawn Landden @ 2019-05-15 1:37 UTC (permalink / raw)
Cc: Paul Mackerras, Shawn Landden, linuxppc-dev
In-Reply-To: <20190513005104.20140-1-shawn@git.icu>
Based off the x86 one.
WireGuard really wants to be able to do SIMD in interrupts,
so it can accelerate its in-bound path.
Signed-off-by: Shawn Landden <shawn@git.icu>
---
arch/powerpc/include/asm/simd.h | 17 +++++++++++++++++
arch/powerpc/kernel/process.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
create mode 100644 arch/powerpc/include/asm/simd.h
diff --git a/arch/powerpc/include/asm/simd.h b/arch/powerpc/include/asm/simd.h
new file mode 100644
index 000000000..2fe26f258
--- /dev/null
+++ b/arch/powerpc/include/asm/simd.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+/*
+ * may_use_simd - whether it is allowable at this time to issue SIMD
+ * instructions or access the SIMD register file
+ *
+ * It's always ok in process context (ie "not interrupt")
+ * but it is sometimes ok even from an irq.
+ */
+#ifdef CONFIG_PPC_FPU
+extern bool may_use_simd(void);
+#else
+static inline bool may_use_simd(void)
+{
+ return false;
+}
+#endif
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index dd9e0d538..ef534831f 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -345,6 +345,36 @@ static int restore_altivec(struct task_struct *tsk)
}
return 0;
}
+
+/*
+ * Were we in user mode when we were
+ * interrupted?
+ *
+ * Doing kernel_altivec/vsx_begin/end() is ok if we are running
+ * in an interrupt context from user mode - we'll just
+ * save the FPU state as required.
+ */
+static bool interrupted_user_mode(void)
+{
+ struct pt_regs *regs = get_irq_regs();
+
+ return regs && user_mode(regs);
+}
+
+/*
+ * Can we use FPU in kernel mode with the
+ * whole "kernel_fpu/altivec/vsx_begin/end()" sequence?
+ *
+ * It's always ok in process context (ie "not interrupt")
+ * but it is sometimes ok even from an irq.
+ */
+bool may_use_simd(void)
+{
+ return !in_interrupt() ||
+ interrupted_user_mode();
+}
+EXPORT_SYMBOL(may_use_simd);
+
#else
#define loadvec(thr) 0
static inline int restore_altivec(struct task_struct *tsk) { return 0; }
--
2.21.0.1020.gf2820cf01a
^ permalink raw reply related
* [PATCH 1/2] [PowerPC] Add simd.h implementation
From: Shawn Landden @ 2019-05-15 1:36 UTC (permalink / raw)
Cc: Paul Mackerras, Shawn Landden, linuxppc-dev
In-Reply-To: <20190513005104.20140-1-shawn@git.icu>
Based off the x86 one.
WireGuard really wants to be able to do SIMD in interrupts,
so it can accelerate its in-bound path.
Signed-off-by: Shawn Landden <shawn@git.icu>
---
arch/powerpc/include/asm/simd.h | 17 +++++++++++++++++
arch/powerpc/kernel/process.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
create mode 100644 arch/powerpc/include/asm/simd.h
diff --git a/arch/powerpc/include/asm/simd.h b/arch/powerpc/include/asm/simd.h
new file mode 100644
index 000000000..2fe26f258
--- /dev/null
+++ b/arch/powerpc/include/asm/simd.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+/*
+ * may_use_simd - whether it is allowable at this time to issue SIMD
+ * instructions or access the SIMD register file
+ *
+ * It's always ok in process context (ie "not interrupt")
+ * but it is sometimes ok even from an irq.
+ */
+#ifdef CONFIG_PPC_FPU
+extern bool may_use_simd(void);
+#else
+static inline bool may_use_simd(void)
+{
+ return false;
+}
+#endif
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index dd9e0d538..ef534831f 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -345,6 +345,36 @@ static int restore_altivec(struct task_struct *tsk)
}
return 0;
}
+
+/*
+ * Were we in user mode when we were
+ * interrupted?
+ *
+ * Doing kernel_altivec/vsx_begin/end() is ok if we are running
+ * in an interrupt context from user mode - we'll just
+ * save the FPU state as required.
+ */
+static bool interrupted_user_mode(void)
+{
+ struct pt_regs *regs = get_irq_regs();
+
+ return regs && user_mode(regs);
+}
+
+/*
+ * Can we use FPU in kernel mode with the
+ * whole "kernel_fpu/altivec/vsx_begin/end()" sequence?
+ *
+ * It's always ok in process context (ie "not interrupt")
+ * but it is sometimes ok even from an irq.
+ */
+bool may_use_simd(void)
+{
+ return !in_interrupt() ||
+ interrupted_user_mode();
+}
+EXPORT_SYMBOL(may_use_simd);
+
#else
#define loadvec(thr) 0
static inline int restore_altivec(struct task_struct *tsk) { return 0; }
--
2.21.0.1020.gf2820cf01a
^ permalink raw reply related
* [PATCH] powerpc/mm/book3s64: Implement STRICT_MODULE_RWX
From: Russell Currey @ 2019-05-15 1:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Russell Currey
Strict module RWX is just like strict kernel RWX, but for modules - so
loadable modules aren't marked both writable and executable at the same
time. This is handled by the generic code in kernel/module.c, and
simply requires the architecture to implement the set_memory() set of
functions, declared with ARCH_HAS_SET_MEMORY.
The set_memory() family of functions are implemented for book3s64
MMUs (so Hash and Radix), however they could likely be adapted to work
for other platforms as well and made more generic. I did it this way
since they're the platforms I have the most understanding of and ability
to test.
There's nothing other than these functions required to turn
ARCH_HAS_STRICT_MODULE_RWX on, so turn that on too.
With STRICT_MODULE_RWX enabled, there are as many W+X pages at runtime
as there are with CONFIG_MODULES=n (none), so in my testing it works
well on both Hash and Radix.
There's a TODO in the code for also applying the page permission changes
to the backing pages in the linear mapping: this is pretty simple for
Radix and (seemingly) a lot harder for Hash, so I've left it for now
since there's still a notable security benefit for the patch as-is.
Technically can be enabled without STRICT_KERNEL_RWX, but I don't think
that gets you a whole lot, so I think we should leave it off by default
until we can get STRICT_KERNEL_RWX to the point where it's enabled by
default.
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
arch/powerpc/Kconfig | 2 +
arch/powerpc/include/asm/set_memory.h | 12 +++
arch/powerpc/mm/book3s64/Makefile | 2 +-
arch/powerpc/mm/book3s64/pageattr.c | 106 ++++++++++++++++++++++++++
4 files changed, 121 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/include/asm/set_memory.h
create mode 100644 arch/powerpc/mm/book3s64/pageattr.c
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d7996cfaceca..9e1bfa81bc5a 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -131,7 +131,9 @@ config PPC
select ARCH_HAS_PTE_SPECIAL
select ARCH_HAS_MEMBARRIER_CALLBACKS
select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC64
+ select ARCH_HAS_SET_MEMORY if PPC_BOOK3S_64
select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !RELOCATABLE && !HIBERNATION)
+ select ARCH_HAS_STRICT_MODULE_RWX if PPC_BOOK3S_64
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_HAS_UACCESS_FLUSHCACHE if PPC64
select ARCH_HAS_UBSAN_SANITIZE_ALL
diff --git a/arch/powerpc/include/asm/set_memory.h b/arch/powerpc/include/asm/set_memory.h
new file mode 100644
index 000000000000..5323a8b06f98
--- /dev/null
+++ b/arch/powerpc/include/asm/set_memory.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+#ifndef _ASM_POWERPC_SET_MEMORY_H
+#define _ASM_POWERPC_SET_MEMORY_H
+
+#ifdef CONFIG_PPC_BOOK3S_64
+int set_memory_ro(unsigned long addr, int numpages);
+int set_memory_rw(unsigned long addr, int numpages);
+int set_memory_nx(unsigned long addr, int numpages);
+int set_memory_x(unsigned long addr, int numpages);
+#endif
+
+#endif
diff --git a/arch/powerpc/mm/book3s64/Makefile b/arch/powerpc/mm/book3s64/Makefile
index 974b4fc19f4f..09c5afadf235 100644
--- a/arch/powerpc/mm/book3s64/Makefile
+++ b/arch/powerpc/mm/book3s64/Makefile
@@ -5,7 +5,7 @@ ccflags-y := $(NO_MINIMAL_TOC)
CFLAGS_REMOVE_slb.o = $(CC_FLAGS_FTRACE)
obj-y += hash_pgtable.o hash_utils.o slb.o \
- mmu_context.o pgtable.o hash_tlb.o
+ mmu_context.o pgtable.o hash_tlb.o pageattr.o
obj-$(CONFIG_PPC_NATIVE) += hash_native.o
obj-$(CONFIG_PPC_RADIX_MMU) += radix_pgtable.o radix_tlb.o
obj-$(CONFIG_PPC_4K_PAGES) += hash_4k.o
diff --git a/arch/powerpc/mm/book3s64/pageattr.c b/arch/powerpc/mm/book3s64/pageattr.c
new file mode 100644
index 000000000000..d6afa89fb407
--- /dev/null
+++ b/arch/powerpc/mm/book3s64/pageattr.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Page attribute and set_memory routines for Radix and Hash MMUs
+ *
+ * Derived from the arm64 implementation.
+ *
+ * Author: Russell Currey <ruscur@russell.cc>
+ *
+ * Copyright 2019, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/mm.h>
+#include <linux/vmalloc.h>
+
+#include <asm/mmu.h>
+#include <asm/page.h>
+#include <asm/pgtable.h>
+
+// we need this to have a single pointer to pass into apply_to_page_range()
+struct page_change_data {
+ pgprot_t set_mask;
+ pgprot_t clear_mask;
+};
+
+static pte_t clear_pte_bit(pte_t pte, pgprot_t prot)
+{
+ return __pte(pte_val(pte) & ~pgprot_val(prot));
+}
+
+static pte_t set_pte_bit(pte_t pte, pgprot_t prot)
+{
+ return __pte(pte_val(pte) | pgprot_val(prot));
+}
+
+static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long addr,
+ void *data)
+{
+ struct page_change_data *cdata = data;
+ pte_t pte = READ_ONCE(*ptep);
+
+ pte = clear_pte_bit(pte, cdata->clear_mask);
+ pte = set_pte_bit(pte, cdata->set_mask);
+
+ set_pte_at(&init_mm, addr, ptep, pte);
+ return 0;
+}
+
+static int change_memory(unsigned long addr, int numpages, pgprot_t set_mask,
+ pgprot_t clear_mask)
+{
+ unsigned long size = numpages * PAGE_SIZE;
+ unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
+ unsigned long end = PAGE_ALIGN(start + size);
+ struct page_change_data data;
+ struct vm_struct *area;
+ int ret;
+
+ if (!numpages)
+ return 0;
+
+ // only operate on VM areas for now
+ area = find_vm_area((void *)addr);
+ if (!area || end > (unsigned long)area->addr + area->size ||
+ !(area->flags & VM_ALLOC))
+ return -EINVAL;
+
+ // TODO: also apply change to the backing pages in the linear mapping
+ data.set_mask = set_mask;
+ data.clear_mask = clear_mask;
+
+ ret = apply_to_page_range(&init_mm, start, size, change_page_range,
+ &data);
+
+ flush_tlb_kernel_range(start, end);
+ return ret;
+}
+
+int set_memory_ro(unsigned long addr, int numpages)
+{
+ return change_memory(addr, numpages,
+ __pgprot(0), __pgprot(_PAGE_WRITE));
+}
+
+int set_memory_rw(unsigned long addr, int numpages)
+{
+ return change_memory(addr, numpages,
+ __pgprot(_PAGE_RW), __pgprot(0));
+}
+
+int set_memory_nx(unsigned long addr, int numpages)
+{
+ return change_memory(addr, numpages,
+ __pgprot(0), __pgprot(_PAGE_EXEC));
+}
+
+int set_memory_x(unsigned long addr, int numpages)
+{
+ return change_memory(addr, numpages,
+ __pgprot(_PAGE_EXEC), __pgprot(0));
+}
--
2.21.0
^ permalink raw reply related
* Re: [v3 2/2] [PowerPC] Allow use of SIMD in interrupts from kernel code
From: kbuild test robot @ 2019-05-15 1:03 UTC (permalink / raw)
To: Shawn Landden; +Cc: linuxppc-dev, Shawn Landden, Paul Mackerras, kbuild-all
In-Reply-To: <20190514154609.23976-2-shawn@git.icu>
[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]
Hi Shawn,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.1 next-20190514]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Shawn-Landden/Add-simd-h-implementation/20190515-062235
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-mpc836x_mds_defconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
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
GCC_VERSION=7.2.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> arch/powerpc/kernel/process.c:194:13: error: 'interrupted_kernel_fpu_idle' defined but not used [-Werror=unused-function]
static bool interrupted_kernel_fpu_idle(void)
^~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
vim +/interrupted_kernel_fpu_idle +194 arch/powerpc/kernel/process.c
193
> 194 static bool interrupted_kernel_fpu_idle(void)
195 {
196 return !kernel_fpu_disabled();
197 }
198
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 15420 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc: Allow may_use_simd() to function as feature detection
From: Shawn Landden @ 2019-05-14 19:00 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20190514180607.GD8599@gate.crashing.org>
[-- Attachment #1: Type: text/plain, Size: 884 bytes --]
Wireguard is not in the kernel (yet) and uses these symbols. (It is
IS_ENABLED so doesn't need it, but I think it is reasonable) I think these
enable/disable symbols should not be marked GPL-only.
El mar., 14 may. 2019 13:06, Segher Boessenkool <segher@kernel.crashing.org>
escribió:
> On Tue, May 14, 2019 at 09:49:18AM -0300, Shawn Landden wrote:
> > ARM does this, so we might as well too.
> > I am a bit confused however as CONFIG_ALTIVEC does not select
> > CONFIG_PPC_FPU. Would you ever have altivec without a fpu?
>
> There is no hardware like that, none supported anyway. It does not make
> very much sense, and it cannot happen with VSX, so no hardware like it
> will ever show up most likely.
>
> It is much simpler to just make a Kconfig dependency (or a select) between
> the symbols than to have to add code like this patch.
>
>
> Segher
>
[-- Attachment #2: Type: text/html, Size: 1437 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] [PowerPC] Add simd.h implementation
From: Shawn Landden @ 2019-05-14 15:44 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <a9f1e109e0b6de8abfabc62375403754ab22dcfc.camel@kernel.crashing.org>
On Tue, May 14, 2019 at 12:43 AM Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> On Mon, 2019-05-13 at 22:44 -0300, Shawn Landden wrote:
> > +
> > +/*
> > + * Were we in user mode when we were
> > + * interrupted?
> > + *
> > + * Doing kernel_altivec/vsx_begin/end() is ok if we are running
> > + * in an interrupt context from user mode - we'll just
> > + * save the FPU state as required.
> > + */
> > +static bool interrupted_user_mode(void)
> > +{
> > + struct pt_regs *regs = get_irq_regs();
> > +
> > + return regs && user_mode(regs);
> > +}
> > +
>
> That's interesting .... it *could* work but we'll have to careful audit
> the code to make sure thats ok.
>
> We probably also want to handle the case where the CPU is in the idle
> loop.
That is the next patch. It is best to split these up because then git
bisect works better, and these are higher-risk changes.
>
> Do we always save the user state when switching out these days ? If
> yes, then there's no "live" state to worry about...
>
> Cheers,
> Ben.
>
>
^ permalink raw reply
* Re: [v2 2/2] [PowerPC] Allow use of SIMD in interrupts from kernel code
From: Shawn Landden @ 2019-05-14 15:35 UTC (permalink / raw)
To: Russell Currey; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <5f31b4964d64aea3fc31165b9cbd0b9d16fd2aa0.camel@russell.cc>
On Tue, May 14, 2019 at 2:22 AM Russell Currey <ruscur@russell.cc> wrote:
>
> On Mon, 2019-05-13 at 23:23 -0300, Shawn Landden wrote:
> > This second patch is separate because it could be wrong,
> > like I am not sure about how kernel thread migration works,
> > and it is even allowing simd in preemptible kernel code.
> >
> > Signed-off-by: Shawn Landden <shawn@git.icu>
> > ---
>
> Hi Shawn,
>
> This patch doesn't build on 64-bit embedded (ppc64e_defconfig):
>
> arch/powerpc/kernel/process.c:194:13: error: 'interrupted_kernel_fpu_idle' defined but not used [-Werror=unused-function]
> static bool interrupted_kernel_fpu_idle(void)
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~
Thanks for noticing this. I knew what I needed to do, and this was
just sloppiness on my end.
>
> and otherwise adds two sparse warnings:
>
> +arch/powerpc/kernel/process.c:356:13: warning: function 'disable_kernel_altivec' with external linkage has definition
> +arch/powerpc/kernel/process.c:416:6: warning: symbol 'may_use_simd' was not declared. Should it be static?
This is the same problem as above.
>
> There's also some style issues (spaces instead of tabs).
Yes, I have to be careful using nano on a VPS.
New patch coming in 1 sec.
>
> Reported by snowpatch (see https://patchwork.ozlabs.org/patch/1099181/)
>
> - Russell
>
^ permalink raw reply
* Re: [RFC PATCH 1/3] powerpc/mm: Handle page table allocation failures
From: Tyrel Datwyler @ 2019-05-14 22:35 UTC (permalink / raw)
To: Aneesh Kumar K.V, npiggin, paulus, mpe; +Cc: linuxppc-dev
In-Reply-To: <20190514145041.7836-1-aneesh.kumar@linux.ibm.com>
On 05/14/2019 07:50 AM, Aneesh Kumar K.V wrote:
> This fixes the below crash that arises due to not handling page table allocation
> failures while allocating hugetlb page table.
Was there supposed to be a oops stack trace attached here in the commit log?
-Tyrel
>
> Fixes: e2b3d202d1db ("powerpc: Switch 16GB and 16MB explicit hugepages to a different page table format")
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/mm/hugetlbpage.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
> index c5c9ff2d7afc..ae9d71da5219 100644
> --- a/arch/powerpc/mm/hugetlbpage.c
> +++ b/arch/powerpc/mm/hugetlbpage.c
> @@ -130,6 +130,8 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, unsigned long addr, unsigned long sz
> } else {
> pdshift = PUD_SHIFT;
> pu = pud_alloc(mm, pg, addr);
> + if (!pu)
> + return NULL;
> if (pshift == PUD_SHIFT)
> return (pte_t *)pu;
> else if (pshift > PMD_SHIFT) {
> @@ -138,6 +140,8 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, unsigned long addr, unsigned long sz
> } else {
> pdshift = PMD_SHIFT;
> pm = pmd_alloc(mm, pu, addr);
> + if (!pm)
> + return NULL;
> if (pshift == PMD_SHIFT)
> /* 16MB hugepage */
> return (pte_t *)pm;
> @@ -154,12 +158,16 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, unsigned long addr, unsigned long sz
> } else {
> pdshift = PUD_SHIFT;
> pu = pud_alloc(mm, pg, addr);
> + if (!pu)
> + return NULL;
> if (pshift >= PUD_SHIFT) {
> ptl = pud_lockptr(mm, pu);
> hpdp = (hugepd_t *)pu;
> } else {
> pdshift = PMD_SHIFT;
> pm = pmd_alloc(mm, pu, addr);
> + if (!pm)
> + return NULL;
> ptl = pmd_lockptr(mm, pm);
> hpdp = (hugepd_t *)pm;
> }
>
^ permalink raw reply
* Re: [PATCH] vsprintf: Do not break early boot with probing addresses
From: Steven Rostedt @ 2019-05-14 19:35 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Petr Mladek, linux-arch@vger.kernel.org, Sergey Senozhatsky,
Heiko Carstens, linux-s390@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, Rasmus Villemoes,
linux-kernel@vger.kernel.org, Michal Hocko, Sergey Senozhatsky,
David Laight, Stephen Rothwell, Andy Shevchenko, Linus Torvalds,
Martin Schwidefsky, Tobin C . Harding
In-Reply-To: <CAMuHMdUhy3uB+G23uXh__F2Y_Jsam5uS1Q5jJC95kWAOEM8WRA@mail.gmail.com>
On Tue, 14 May 2019 21:13:06 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > Do we care about the value? "(-E%u)"?
> >
> > That too could be confusing. What would (-E22) be considered by a user
> > doing an sprintf() on some string. I know that would confuse me, or I
> > would think that it was what the %pX displayed, and wonder why it
> > displayed it that way. Whereas "(fault)" is quite obvious for any %p
> > use case.
>
> I would immediately understand there's a missing IS_ERR() check in a
> function that can return -EINVAL, without having to add a new printk()
> to find out what kind of bogus value has been received, and without
> having to reboot, and trying to reproduce...
Hi Geert,
I have to ask. Has there actually been a case that you used a %pX and
it faulted, and you had to go back to find what the value of the
failure was?
IMO, sprintf() should not be a tool to do this, because then people
will not add their IS_ERR() and just let sprintf() do the job for them.
I don't think that would be wise to allow.
-- Steve
^ permalink raw reply
* Re: [PATCH] vsprintf: Do not break early boot with probing addresses
From: Geert Uytterhoeven @ 2019-05-14 19:13 UTC (permalink / raw)
To: Steven Rostedt
Cc: Petr Mladek, linux-arch@vger.kernel.org, Sergey Senozhatsky,
Heiko Carstens, linux-s390@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, Rasmus Villemoes,
linux-kernel@vger.kernel.org, Michal Hocko, Sergey Senozhatsky,
David Laight, Stephen Rothwell, Andy Shevchenko, Linus Torvalds,
Martin Schwidefsky, Tobin C . Harding
In-Reply-To: <20190514143751.48e81e05@oasis.local.home>
Hi Steve,
On Tue, May 14, 2019 at 8:37 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> On Tue, 14 May 2019 11:02:17 +0200
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, May 14, 2019 at 10:29 AM David Laight <David.Laight@aculab.com> wrote:
> > > > And I like Steven's "(fault)" idea.
> > > > How about this:
> > > >
> > > > if ptr < PAGE_SIZE -> "(null)"
> > > > if IS_ERR_VALUE(ptr) -> "(fault)"
> > > >
> > > > -ss
> > >
> > > Or:
> > > if (ptr < PAGE_SIZE)
> > > return ptr ? "(null+)" : "(null)";
>
> Hmm, that is useful.
>
> > > if IS_ERR_VALUE(ptr)
> > > return "(errno)"
>
> I still prefer "(fault)" as is pretty much all I would expect from a
> pointer dereference, even if it is just bad parsing of, say, a parsing
> an MAC address. "fault" is generic enough. "errno" will be confusing,
> because that's normally a variable not a output.
>
> >
> > Do we care about the value? "(-E%u)"?
>
> That too could be confusing. What would (-E22) be considered by a user
> doing an sprintf() on some string. I know that would confuse me, or I
> would think that it was what the %pX displayed, and wonder why it
> displayed it that way. Whereas "(fault)" is quite obvious for any %p
> use case.
I would immediately understand there's a missing IS_ERR() check in a
function that can return -EINVAL, without having to add a new printk()
to find out what kind of bogus value has been received, and without
having to reboot, and trying to reproduce...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Patch "x86/speculation: Support 'mitigations=' cmdline option" has been added to the 4.4-stable tree
From: gregkh @ 2019-05-14 18:30 UTC (permalink / raw)
To: 6616d0ae169308516cfdf5216bedd169f8a8291b.1555085500.git.jpoimboe,
aarcange, ben, benh, bp, catalin.marinas, gregkh, heiko.carstens,
hpa, jcm, jikos, jkosina, jpoimboe, linux-arm-kernel,
linuxppc-dev, longman, luto, mpe, pauld, paulus, peterz, rdunlap,
schwidefsky, steven.price, tglx, torvalds, tyhicks, will.deacon
Cc: stable-commits
This is a note to let you know that I've just added the patch titled
x86/speculation: Support 'mitigations=' cmdline option
to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
x86-speculation-support-mitigations-cmdline-option.patch
and it can be found in the queue-4.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From foo@baz Tue 14 May 2019 08:29:35 PM CEST
From: Josh Poimboeuf <jpoimboe@redhat.com>
Date: Fri, 12 Apr 2019 15:39:29 -0500
Subject: x86/speculation: Support 'mitigations=' cmdline option
From: Josh Poimboeuf <jpoimboe@redhat.com>
commit d68be4c4d31295ff6ae34a8ddfaa4c1a8ff42812 upstream.
Configure x86 runtime CPU speculation bug mitigations in accordance with
the 'mitigations=' cmdline option. This affects Meltdown, Spectre v2,
Speculative Store Bypass, and L1TF.
The default behavior is unchanged.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Jiri Kosina <jkosina@suse.cz> (on x86)
Reviewed-by: Jiri Kosina <jkosina@suse.cz>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H . Peter Anvin" <hpa@zytor.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Jon Masters <jcm@redhat.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-arch@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Tyler Hicks <tyhicks@canonical.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Steven Price <steven.price@arm.com>
Cc: Phil Auld <pauld@redhat.com>
Link: https://lkml.kernel.org/r/6616d0ae169308516cfdf5216bedd169f8a8291b.1555085500.git.jpoimboe@redhat.com
[bwh: Backported to 4.4:
- Drop the auto,nosmt option and the l1tf mitigation selection, which we can't
support
- Adjust filenames, context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Documentation/kernel-parameters.txt | 14 +++++++++-----
arch/x86/kernel/cpu/bugs.c | 6 ++++--
arch/x86/mm/kaiser.c | 4 +++-
3 files changed, 16 insertions(+), 8 deletions(-)
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2174,15 +2174,19 @@ bytes respectively. Such letter suffixes
http://repo.or.cz/w/linux-2.6/mini2440.git
mitigations=
- Control optional mitigations for CPU vulnerabilities.
- This is a set of curated, arch-independent options, each
- of which is an aggregation of existing arch-specific
- options.
+ [X86] Control optional mitigations for CPU
+ vulnerabilities. This is a set of curated,
+ arch-independent options, each of which is an
+ aggregation of existing arch-specific options.
off
Disable all optional CPU mitigations. This
improves system performance, but it may also
expose users to several CPU vulnerabilities.
+ Equivalent to: nopti [X86]
+ nospectre_v2 [X86]
+ spectre_v2_user=off [X86]
+ spec_store_bypass_disable=off [X86]
auto (default)
Mitigate all CPU vulnerabilities, but leave SMT
@@ -2190,7 +2194,7 @@ bytes respectively. Such letter suffixes
users who don't want to be surprised by SMT
getting disabled across kernel upgrades, or who
have other ways of avoiding SMT-based attacks.
- This is the default behavior.
+ Equivalent to: (default behavior)
mminit_loglevel=
[KNL] When CONFIG_DEBUG_MEMORY_INIT is set, this
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -479,7 +479,8 @@ static enum spectre_v2_mitigation_cmd __
char arg[20];
int ret, i;
- if (cmdline_find_option_bool(boot_command_line, "nospectre_v2"))
+ if (cmdline_find_option_bool(boot_command_line, "nospectre_v2") ||
+ cpu_mitigations_off())
return SPECTRE_V2_CMD_NONE;
ret = cmdline_find_option(boot_command_line, "spectre_v2", arg, sizeof(arg));
@@ -743,7 +744,8 @@ static enum ssb_mitigation_cmd __init ss
char arg[20];
int ret, i;
- if (cmdline_find_option_bool(boot_command_line, "nospec_store_bypass_disable")) {
+ if (cmdline_find_option_bool(boot_command_line, "nospec_store_bypass_disable") ||
+ cpu_mitigations_off()) {
return SPEC_STORE_BYPASS_CMD_NONE;
} else {
ret = cmdline_find_option(boot_command_line, "spec_store_bypass_disable",
--- a/arch/x86/mm/kaiser.c
+++ b/arch/x86/mm/kaiser.c
@@ -10,6 +10,7 @@
#include <linux/mm.h>
#include <linux/uaccess.h>
#include <linux/ftrace.h>
+#include <linux/cpu.h>
#undef pr_fmt
#define pr_fmt(fmt) "Kernel/User page tables isolation: " fmt
@@ -297,7 +298,8 @@ void __init kaiser_check_boottime_disabl
goto skip;
}
- if (cmdline_find_option_bool(boot_command_line, "nopti"))
+ if (cmdline_find_option_bool(boot_command_line, "nopti") ||
+ cpu_mitigations_off())
goto disable;
skip:
Patches currently in stable-queue which might be from jpoimboe@redhat.com are
queue-4.4/x86-speculation-mds-fix-documentation-typo.patch
queue-4.4/x86-speculation-mds-add-mitigations-support-for-mds.patch
queue-4.4/x86-speculation-rework-smt-state-change.patch
queue-4.4/cpu-speculation-add-mitigations-cmdline-option.patch
queue-4.4/x86-speculation-mds-fix-comment.patch
queue-4.4/x86-speculation-reorder-the-spec_v2-code.patch
queue-4.4/x86-speculation-add-prctl-control-for-indirect-branch-speculation.patch
queue-4.4/x86-speculation-provide-ibpb-always-command-line-options.patch
queue-4.4/x86-kconfig-select-sched_smt-if-smp-enabled.patch
queue-4.4/x86-speculation-mds-add-smt-warning-message.patch
queue-4.4/x86-process-consolidate-and-simplify-switch_to_xtra-code.patch
queue-4.4/x86-speculation-reorganize-speculation-control-msrs-update.patch
queue-4.4/x86-speculation-update-the-tif_ssbd-comment.patch
queue-4.4/x86-speculation-propagate-information-about-rsb-filling-mitigation-to-sysfs.patch
queue-4.4/x86-speculation-unify-conditional-spectre-v2-print-functions.patch
queue-4.4/x86-speculation-disable-stibp-when-enhanced-ibrs-is-in-use.patch
queue-4.4/x86-speculation-rename-ssbd-update-functions.patch
queue-4.4/x86-speculation-enable-prctl-mode-for-spectre_v2_user.patch
queue-4.4/x86-speculation-mark-string-arrays-const-correctly.patch
queue-4.4/x86-speculation-move-arch_smt_update-call-to-after-mitigation-decisions.patch
queue-4.4/x86-speculation-add-seccomp-spectre-v2-user-space-protection-mode.patch
queue-4.4/x86-speculation-clean-up-spectre_v2_parse_cmdline.patch
queue-4.4/x86-mm-use-write_once-when-setting-ptes.patch
queue-4.4/x86-speculation-apply-ibpb-more-strictly-to-avoid-cross-process-data-leak.patch
queue-4.4/x86-speculation-add-command-line-control-for-indirect-branch-speculation.patch
queue-4.4/x86-speculation-move-stipb-ibpb-string-conditionals-out-of-cpu_show_common.patch
queue-4.4/x86-speculation-support-mitigations-cmdline-option.patch
queue-4.4/x86-speculation-remove-unnecessary-ret-variable-in-cpu_show_common.patch
queue-4.4/x86-speculation-prepare-for-per-task-indirect-branch-speculation-control.patch
queue-4.4/x86-speculation-avoid-__switch_to_xtra-calls.patch
queue-4.4/x86-speculation-mds-print-smt-vulnerable-on-msbds-with-mitigations-off.patch
queue-4.4/x86-speculation-split-out-tif-update.patch
queue-4.4/x86-speculation-prepare-for-conditional-ibpb-in-switch_mm.patch
queue-4.4/x86-speculation-prepare-arch_smt_update-for-prctl-mode.patch
queue-4.4/x86-speculation-enable-cross-hyperthread-spectre-v2-stibp-mitigation.patch
queue-4.4/x86-speculataion-mark-command-line-parser-data-__initdata.patch
queue-4.4/x86-speculation-prevent-stale-spec_ctrl-msr-content.patch
^ permalink raw reply
* Patch "cpu/speculation: Add 'mitigations=' cmdline option" has been added to the 4.4-stable tree
From: gregkh @ 2019-05-14 18:30 UTC (permalink / raw)
To: aarcange,
b07a8ef9b7c5055c3a4637c87d07c296d5016fe0.1555085500.git.jpoimboe,
ben, benh, bp, catalin.marinas, gregkh, heiko.carstens, hpa, jcm,
jikos, jkosina, jpoimboe, linux-arm-kernel, linuxppc-dev, longman,
luto, mpe, pauld, paulus, peterz, rdunlap, schwidefsky,
steven.price, tglx, torvalds, tyhicks, will.deacon
Cc: stable-commits
This is a note to let you know that I've just added the patch titled
cpu/speculation: Add 'mitigations=' cmdline option
to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
cpu-speculation-add-mitigations-cmdline-option.patch
and it can be found in the queue-4.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From foo@baz Tue 14 May 2019 08:29:35 PM CEST
From: Josh Poimboeuf <jpoimboe@redhat.com>
Date: Fri, 12 Apr 2019 15:39:28 -0500
Subject: cpu/speculation: Add 'mitigations=' cmdline option
From: Josh Poimboeuf <jpoimboe@redhat.com>
commit 98af8452945c55652de68536afdde3b520fec429 upstream.
Keeping track of the number of mitigations for all the CPU speculation
bugs has become overwhelming for many users. It's getting more and more
complicated to decide which mitigations are needed for a given
architecture. Complicating matters is the fact that each arch tends to
have its own custom way to mitigate the same vulnerability.
Most users fall into a few basic categories:
a) they want all mitigations off;
b) they want all reasonable mitigations on, with SMT enabled even if
it's vulnerable; or
c) they want all reasonable mitigations on, with SMT disabled if
vulnerable.
Define a set of curated, arch-independent options, each of which is an
aggregation of existing options:
- mitigations=off: Disable all mitigations.
- mitigations=auto: [default] Enable all the default mitigations, but
leave SMT enabled, even if it's vulnerable.
- mitigations=auto,nosmt: Enable all the default mitigations, disabling
SMT if needed by a mitigation.
Currently, these options are placeholders which don't actually do
anything. They will be fleshed out in upcoming patches.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Jiri Kosina <jkosina@suse.cz> (on x86)
Reviewed-by: Jiri Kosina <jkosina@suse.cz>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H . Peter Anvin" <hpa@zytor.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Jon Masters <jcm@redhat.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-arch@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Tyler Hicks <tyhicks@canonical.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Steven Price <steven.price@arm.com>
Cc: Phil Auld <pauld@redhat.com>
Link: https://lkml.kernel.org/r/b07a8ef9b7c5055c3a4637c87d07c296d5016fe0.1555085500.git.jpoimboe@redhat.com
[bwh: Backported to 4.4:
- Drop the auto,nosmt option which we can't support
- Adjust filename]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Documentation/kernel-parameters.txt | 19 +++++++++++++++++++
include/linux/cpu.h | 17 +++++++++++++++++
kernel/cpu.c | 13 +++++++++++++
3 files changed, 49 insertions(+)
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2173,6 +2173,25 @@ bytes respectively. Such letter suffixes
in the "bleeding edge" mini2440 support kernel at
http://repo.or.cz/w/linux-2.6/mini2440.git
+ mitigations=
+ Control optional mitigations for CPU vulnerabilities.
+ This is a set of curated, arch-independent options, each
+ of which is an aggregation of existing arch-specific
+ options.
+
+ off
+ Disable all optional CPU mitigations. This
+ improves system performance, but it may also
+ expose users to several CPU vulnerabilities.
+
+ auto (default)
+ Mitigate all CPU vulnerabilities, but leave SMT
+ enabled, even if it's vulnerable. This is for
+ users who don't want to be surprised by SMT
+ getting disabled across kernel upgrades, or who
+ have other ways of avoiding SMT-based attacks.
+ This is the default behavior.
+
mminit_loglevel=
[KNL] When CONFIG_DEBUG_MEMORY_INIT is set, this
parameter allows control of the logging verbosity for
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -296,4 +296,21 @@ bool cpu_wait_death(unsigned int cpu, in
bool cpu_report_death(void);
#endif /* #ifdef CONFIG_HOTPLUG_CPU */
+/*
+ * These are used for a global "mitigations=" cmdline option for toggling
+ * optional CPU mitigations.
+ */
+enum cpu_mitigations {
+ CPU_MITIGATIONS_OFF,
+ CPU_MITIGATIONS_AUTO,
+};
+
+extern enum cpu_mitigations cpu_mitigations;
+
+/* mitigations=off */
+static inline bool cpu_mitigations_off(void)
+{
+ return cpu_mitigations == CPU_MITIGATIONS_OFF;
+}
+
#endif /* _LINUX_CPU_H_ */
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -842,3 +842,16 @@ void init_cpu_online(const struct cpumas
{
cpumask_copy(to_cpumask(cpu_online_bits), src);
}
+
+enum cpu_mitigations cpu_mitigations = CPU_MITIGATIONS_AUTO;
+
+static int __init mitigations_parse_cmdline(char *arg)
+{
+ if (!strcmp(arg, "off"))
+ cpu_mitigations = CPU_MITIGATIONS_OFF;
+ else if (!strcmp(arg, "auto"))
+ cpu_mitigations = CPU_MITIGATIONS_AUTO;
+
+ return 0;
+}
+early_param("mitigations", mitigations_parse_cmdline);
Patches currently in stable-queue which might be from jpoimboe@redhat.com are
queue-4.4/x86-speculation-mds-fix-documentation-typo.patch
queue-4.4/x86-speculation-mds-add-mitigations-support-for-mds.patch
queue-4.4/x86-speculation-rework-smt-state-change.patch
queue-4.4/cpu-speculation-add-mitigations-cmdline-option.patch
queue-4.4/x86-speculation-mds-fix-comment.patch
queue-4.4/x86-speculation-reorder-the-spec_v2-code.patch
queue-4.4/x86-speculation-add-prctl-control-for-indirect-branch-speculation.patch
queue-4.4/x86-speculation-provide-ibpb-always-command-line-options.patch
queue-4.4/x86-kconfig-select-sched_smt-if-smp-enabled.patch
queue-4.4/x86-speculation-mds-add-smt-warning-message.patch
queue-4.4/x86-process-consolidate-and-simplify-switch_to_xtra-code.patch
queue-4.4/x86-speculation-reorganize-speculation-control-msrs-update.patch
queue-4.4/x86-speculation-update-the-tif_ssbd-comment.patch
queue-4.4/x86-speculation-propagate-information-about-rsb-filling-mitigation-to-sysfs.patch
queue-4.4/x86-speculation-unify-conditional-spectre-v2-print-functions.patch
queue-4.4/x86-speculation-disable-stibp-when-enhanced-ibrs-is-in-use.patch
queue-4.4/x86-speculation-rename-ssbd-update-functions.patch
queue-4.4/x86-speculation-enable-prctl-mode-for-spectre_v2_user.patch
queue-4.4/x86-speculation-mark-string-arrays-const-correctly.patch
queue-4.4/x86-speculation-move-arch_smt_update-call-to-after-mitigation-decisions.patch
queue-4.4/x86-speculation-add-seccomp-spectre-v2-user-space-protection-mode.patch
queue-4.4/x86-speculation-clean-up-spectre_v2_parse_cmdline.patch
queue-4.4/x86-mm-use-write_once-when-setting-ptes.patch
queue-4.4/x86-speculation-apply-ibpb-more-strictly-to-avoid-cross-process-data-leak.patch
queue-4.4/x86-speculation-add-command-line-control-for-indirect-branch-speculation.patch
queue-4.4/x86-speculation-move-stipb-ibpb-string-conditionals-out-of-cpu_show_common.patch
queue-4.4/x86-speculation-support-mitigations-cmdline-option.patch
queue-4.4/x86-speculation-remove-unnecessary-ret-variable-in-cpu_show_common.patch
queue-4.4/x86-speculation-prepare-for-per-task-indirect-branch-speculation-control.patch
queue-4.4/x86-speculation-avoid-__switch_to_xtra-calls.patch
queue-4.4/x86-speculation-mds-print-smt-vulnerable-on-msbds-with-mitigations-off.patch
queue-4.4/x86-speculation-split-out-tif-update.patch
queue-4.4/x86-speculation-prepare-for-conditional-ibpb-in-switch_mm.patch
queue-4.4/x86-speculation-prepare-arch_smt_update-for-prctl-mode.patch
queue-4.4/x86-speculation-enable-cross-hyperthread-spectre-v2-stibp-mitigation.patch
queue-4.4/x86-speculataion-mark-command-line-parser-data-__initdata.patch
queue-4.4/x86-speculation-prevent-stale-spec_ctrl-msr-content.patch
^ permalink raw reply
* Re: [PATCH] vsprintf: Do not break early boot with probing addresses
From: Steven Rostedt @ 2019-05-14 18:37 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Petr Mladek, linux-arch@vger.kernel.org, Sergey Senozhatsky,
Heiko Carstens, linux-s390@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, Rasmus Villemoes,
linux-kernel@vger.kernel.org, Michal Hocko, Sergey Senozhatsky,
David Laight, Stephen Rothwell, Andy Shevchenko, Linus Torvalds,
Martin Schwidefsky, Tobin C . Harding
In-Reply-To: <CAMuHMdXaMObq9h2Sb49PW1-HUysPeaWXB7wJmKFz=xLmSoUDZg@mail.gmail.com>
[ Purple is a nice shade on the bike shed. ;-) ]
On Tue, 14 May 2019 11:02:17 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, May 14, 2019 at 10:29 AM David Laight <David.Laight@aculab.com> wrote:
> > > And I like Steven's "(fault)" idea.
> > > How about this:
> > >
> > > if ptr < PAGE_SIZE -> "(null)"
> > > if IS_ERR_VALUE(ptr) -> "(fault)"
> > >
> > > -ss
> >
> > Or:
> > if (ptr < PAGE_SIZE)
> > return ptr ? "(null+)" : "(null)";
Hmm, that is useful.
> > if IS_ERR_VALUE(ptr)
> > return "(errno)"
I still prefer "(fault)" as is pretty much all I would expect from a
pointer dereference, even if it is just bad parsing of, say, a parsing
an MAC address. "fault" is generic enough. "errno" will be confusing,
because that's normally a variable not a output.
>
> Do we care about the value? "(-E%u)"?
That too could be confusing. What would (-E22) be considered by a user
doing an sprintf() on some string. I know that would confuse me, or I
would think that it was what the %pX displayed, and wonder why it
displayed it that way. Whereas "(fault)" is quite obvious for any %p
use case.
-- Steve
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox