* [PATCH -next v2 2/9] ASoC: fsl_aud2htx: Use devm_platform_get_and_ioremap_resource()
From: Yang Yingliang @ 2021-06-15 1:39 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, alsa-devel; +Cc: broonie, timur
In-Reply-To: <20210615013922.784296-1-yangyingliang@huawei.com>
Use devm_platform_get_and_ioremap_resource() to simplify
code.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
sound/soc/fsl/fsl_aud2htx.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_aud2htx.c b/sound/soc/fsl/fsl_aud2htx.c
index a328697511f7..99ab7f0241cf 100644
--- a/sound/soc/fsl/fsl_aud2htx.c
+++ b/sound/soc/fsl/fsl_aud2htx.c
@@ -196,8 +196,7 @@ static int fsl_aud2htx_probe(struct platform_device *pdev)
aud2htx->pdev = pdev;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- regs = devm_ioremap_resource(&pdev->dev, res);
+ regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
if (IS_ERR(regs))
return PTR_ERR(regs);
--
2.25.1
^ permalink raw reply related
* [PATCH -next v2 6/9] ASoC: fsl_sai: Use devm_platform_get_and_ioremap_resource()
From: Yang Yingliang @ 2021-06-15 1:39 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, alsa-devel; +Cc: broonie, timur
In-Reply-To: <20210615013922.784296-1-yangyingliang@huawei.com>
Use devm_platform_get_and_ioremap_resource() to simplify
code.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
sound/soc/fsl/fsl_sai.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 407a45e48eee..223fcd15bfcc 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -1017,8 +1017,7 @@ static int fsl_sai_probe(struct platform_device *pdev)
sai->is_lsb_first = of_property_read_bool(np, "lsb-first");
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- base = devm_ioremap_resource(&pdev->dev, res);
+ base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
if (IS_ERR(base))
return PTR_ERR(base);
--
2.25.1
^ permalink raw reply related
* [PATCH -next v2 9/9] ASoC: fsl_xcvr: check return value after calling platform_get_resource_byname()
From: Yang Yingliang @ 2021-06-15 1:39 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, alsa-devel; +Cc: broonie, timur
In-Reply-To: <20210615013922.784296-1-yangyingliang@huawei.com>
It will cause null-ptr-deref if platform_get_resource_byname() returns NULL,
we need check the return value.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
sound/soc/fsl/fsl_xcvr.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c
index df7c189d97dd..1330e190e1ff 100644
--- a/sound/soc/fsl/fsl_xcvr.c
+++ b/sound/soc/fsl/fsl_xcvr.c
@@ -1202,6 +1202,10 @@ static int fsl_xcvr_probe(struct platform_device *pdev)
rx_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rxfifo");
tx_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "txfifo");
+ if (!rx_res || !tx_res) {
+ dev_err(dev, "could not find rxfifo or txfifo resource\n");
+ return -EINVAL;
+ }
xcvr->dma_prms_rx.chan_name = "rx";
xcvr->dma_prms_tx.chan_name = "tx";
xcvr->dma_prms_rx.addr = rx_res->start;
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 07/11] powerpc: Add support for microwatt's hardware random number generator
From: Nicholas Piggin @ 2021-06-15 1:40 UTC (permalink / raw)
To: linuxppc-dev, Paul Mackerras
In-Reply-To: <YMff6iLDiCbFQmrW@thinks.paulus.ozlabs.org>
Excerpts from Paul Mackerras's message of June 15, 2021 9:02 am:
> This is accessed using the DARN instruction and should probably be
> done more generically.
>
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> ---
> arch/powerpc/include/asm/archrandom.h | 12 +++++-
> arch/powerpc/platforms/microwatt/Kconfig | 1 +
> arch/powerpc/platforms/microwatt/Makefile | 2 +-
> arch/powerpc/platforms/microwatt/rng.c | 48 +++++++++++++++++++++++
> 4 files changed, 61 insertions(+), 2 deletions(-)
> create mode 100644 arch/powerpc/platforms/microwatt/rng.c
>
> diff --git a/arch/powerpc/include/asm/archrandom.h b/arch/powerpc/include/asm/archrandom.h
> index 9a53e29680f4..e8ae0f7740f9 100644
> --- a/arch/powerpc/include/asm/archrandom.h
> +++ b/arch/powerpc/include/asm/archrandom.h
> @@ -8,12 +8,22 @@
>
> static inline bool __must_check arch_get_random_long(unsigned long *v)
> {
> + if (ppc_md.get_random_seed)
> + return ppc_md.get_random_seed(v);
> +
> return false;
> }
>
> static inline bool __must_check arch_get_random_int(unsigned int *v)
> {
> - return false;
> + unsigned long val;
> + bool rc;
> +
> + rc = arch_get_random_long(&val);
> + if (rc)
> + *v = val;
> +
> + return rc;
> }
>
I would be happier if you didn't change this (or at least put it in its
own patch explaining why it's not going to slow down other platforms).
I'm assuming the main problem you have is seeding the rngs at boot? It
should be enough to have ppc_md.get_random_seed for that.
(BTW I wonder should lib/random32.c be changed to call
arch_get_random_seed_long() for seeding)
> static inline bool __must_check arch_get_random_seed_long(unsigned long *v)
> diff --git a/arch/powerpc/platforms/microwatt/Kconfig b/arch/powerpc/platforms/microwatt/Kconfig
> index 50ed0cedb5f1..8f6a81978461 100644
> --- a/arch/powerpc/platforms/microwatt/Kconfig
> +++ b/arch/powerpc/platforms/microwatt/Kconfig
> @@ -7,6 +7,7 @@ config PPC_MICROWATT
> select PPC_ICP_NATIVE
> select PPC_NATIVE
> select PPC_UDBG_16550
> + select ARCH_RANDOM
> help
> This option enables support for FPGA-based Microwatt implementations.
>
> diff --git a/arch/powerpc/platforms/microwatt/Makefile b/arch/powerpc/platforms/microwatt/Makefile
> index e6885b3b2ee7..116d6d3ad3f0 100644
> --- a/arch/powerpc/platforms/microwatt/Makefile
> +++ b/arch/powerpc/platforms/microwatt/Makefile
> @@ -1 +1 @@
> -obj-y += setup.o
> +obj-y += setup.o rng.o
> diff --git a/arch/powerpc/platforms/microwatt/rng.c b/arch/powerpc/platforms/microwatt/rng.c
> new file mode 100644
> index 000000000000..3d8ee6eb7dad
> --- /dev/null
> +++ b/arch/powerpc/platforms/microwatt/rng.c
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Derived from arch/powerpc/platforms/powernv/rng.c, which is:
> + * Copyright 2013, Michael Ellerman, IBM Corporation.
> + */
> +
> +#define pr_fmt(fmt) "microwatt-rng: " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/smp.h>
> +#include <asm/archrandom.h>
> +#include <asm/cputable.h>
> +#include <asm/machdep.h>
> +
> +#define DARN_ERR 0xFFFFFFFFFFFFFFFFul
> +
> +int microwatt_get_random_darn(unsigned long *v)
> +{
> + unsigned long val;
> +
> + /* Using DARN with L=1 - 64-bit conditioned random number */
> + asm volatile(PPC_DARN(%0, 1) : "=r"(val));
> +
> + if (val == DARN_ERR)
> + return 0;
> +
> + *v = val;
> +
> + return 1;
> +}
> +
> +static __init int rng_init(void)
> +{
> + unsigned long val;
> + int i;
> +
> + for (i = 0; i < 10; i++) {
> + if (microwatt_get_random_darn(&val)) {
> + ppc_md.get_random_seed = microwatt_get_random_darn;
> + return 0;
> + }
> + }
> +
> + pr_warn("Unable to use DARN for get_random_seed()\n");
> +
> + return -EIO;
> +}
> +machine_subsys_initcall(, rng_init);
> --
> 2.31.1
>
>
^ permalink raw reply
* Re: [RFC PATCH 0/8] Add support for FORM2 associativity
From: Daniel Henrique Barboza @ 2021-06-15 1:47 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Nathan Lynch, David Gibson
In-Reply-To: <20210614164003.196094-1-aneesh.kumar@linux.ibm.com>
On 6/14/21 1:39 PM, Aneesh Kumar K.V wrote:
> Form2 associativity adds a much more flexible NUMA topology layout
> than what is provided by Form1. This also allows PAPR SCM device
> to use better associativity when using the device as DAX KMEM
> device. More details can be found in patch x
>
> $ ndctl list -N -v
> [
> {
> "dev":"namespace0.0",
> "mode":"devdax",
> "map":"dev",
> "size":1071644672,
> "uuid":"37dea198-ddb5-4e42-915a-99a915e24188",
> "raw_uuid":"148deeaa-4a2f-41d1-8d74-fd9a942d26ba",
> "daxregion":{
> "id":0,
> "size":1071644672,
> "devices":[
> {
> "chardev":"dax0.0",
> "size":1071644672,
> "target_node":4,
> "mode":"devdax"
> }
> ]
> },
> "align":2097152,
> "numa_node":1
> }
> ]
>
> $ numactl -H
> ...
> node distances:
> node 0 1 2 3
> 0: 10 11 222 33
> 1: 44 10 55 66
> 2: 77 88 10 99
> 3: 101 121 132 10
> $
>
> After DAX KMEM
> # numactl -H
> available: 5 nodes (0-4)
> ...
> node distances:
> node 0 1 2 3 4
> 0: 10 11 22 33 255
> 1: 44 10 55 66 255
> 2: 77 88 10 99 255
> 3: 101 121 132 10 255
> 4: 255 255 255 255 10
> #
>
> The above output is with a Qemu command line
For reference, this QEMU:
https://github.com/danielhb/qemu/tree/form2_affinity_v1
https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg03617.html
but ...
>
> -numa node,nodeid=4 \
> -numa dist,src=0,dst=1,val=11 -numa dist,src=0,dst=2,val=22 -numa dist,src=0,dst=3,val=33 -numa dist,src=0,dst=4,val=255 \
> -numa dist,src=1,dst=0,val=44 -numa dist,src=1,dst=2,val=55 -numa dist,src=1,dst=3,val=66 -numa dist,src=1,dst=4,val=255 \
> -numa dist,src=2,dst=0,val=77 -numa dist,src=2,dst=1,val=88 -numa dist,src=2,dst=3,val=99 -numa dist,src=2,dst=4,val=255 \
> -numa dist,src=3,dst=0,val=101 -numa dist,src=3,dst=1,val=121 -numa dist,src=3,dst=2,val=132 -numa dist,src=3,dst=4,val=255 \
> -numa dist,src=4,dst=0,val=255 -numa dist,src=4,dst=1,val=255 -numa dist,src=4,dst=2,val=255 -numa dist,src=4,dst=3,val=255 \
> -object memory-backend-file,id=memnvdimm1,prealloc=yes,mem-path=$PMEM_DISK,share=yes,size=${PMEM_SIZE} \
> -device nvdimm,label-size=128K,memdev=memnvdimm1,id=nvdimm1,slot=4,uuid=72511b67-0b3b-42fd-8d1d-5be3cae8bcaa,node=4,persistent-nodeid=1
with 'device-node=1' instead of 'persistent=nodeid=1' in the nvdimm parameter
up here.
>
>
>
> Aneesh Kumar K.V (8):
> powerpc/pseries: rename min_common_depth to primary_domain_index
> powerpc/pseries: rename distance_ref_points_depth to max_domain_index
> powerpc/pseries: Rename TYPE1_AFFINITY to FORM1_AFFINITY
> powerpc/pseries: Consolidate DLPAR NUMA distance update
> powerpc/pseries: Consolidate NUMA distance update during boot
> powerpc/pseries: Add a helper for form1 cpu distance
> powerpc/pseries: Add support for FORM2 associativity
> powerpc/papr_scm: Use FORM2 associativity details
Series:
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>
> Documentation/powerpc/associativity.rst | 139 ++++++
> arch/powerpc/include/asm/firmware.h | 7 +-
> arch/powerpc/include/asm/prom.h | 3 +-
> arch/powerpc/kernel/prom_init.c | 3 +-
> arch/powerpc/mm/numa.c | 436 ++++++++++++++----
> arch/powerpc/platforms/pseries/firmware.c | 3 +-
> arch/powerpc/platforms/pseries/hotplug-cpu.c | 2 +
> .../platforms/pseries/hotplug-memory.c | 2 +
> arch/powerpc/platforms/pseries/papr_scm.c | 26 +-
> arch/powerpc/platforms/pseries/pseries.h | 2 +
> 10 files changed, 522 insertions(+), 101 deletions(-)
> create mode 100644 Documentation/powerpc/associativity.rst
>
^ permalink raw reply
* [PATCH v12 0/6] KASAN core changes for ppc64 radix KASAN
From: Daniel Axtens @ 2021-06-15 1:46 UTC (permalink / raw)
To: linux-kernel, linux-mm, linuxppc-dev, kasan-dev, christophe.leroy,
aneesh.kumar, bsingharora
Cc: elver, Daniel Axtens
Building on the work of Christophe, Aneesh and Balbir, I've ported
KASAN to 64-bit Book3S kernels running on the Radix MMU.
I've been trying this for a while, but we keep having collisions
between the kasan code in the mm tree and the code I want to put in to
the ppc tree. So my aim here is for patches 1 through 4 or 1 through 5
to go in via the mm tree. I will then propose the powerpc changes for
a later cycle. (I have attached them to this series as an RFC, and
there are still outstanding review comments I need to attend to.)
v12 applies to next-20210611. There should be no noticable changes to
other platforms.
Kind regards,
Daniel
Daniel Axtens (6):
kasan: allow an architecture to disable inline instrumentation
kasan: allow architectures to provide an outline readiness check
kasan: define and use MAX_PTRS_PER_* for early shadow tables
kasan: Document support on 32-bit powerpc
powerpc/mm/kasan: rename kasan_init_32.c to init_32.c
[RFC] powerpc: Book3S 64-bit outline-only KASAN support
Documentation/dev-tools/kasan.rst | 7 +-
Documentation/powerpc/kasan.txt | 58 +++++++++++
arch/powerpc/Kconfig | 4 +-
arch/powerpc/Kconfig.debug | 3 +-
arch/powerpc/include/asm/book3s/64/hash.h | 4 +
arch/powerpc/include/asm/book3s/64/pgtable.h | 4 +
arch/powerpc/include/asm/book3s/64/radix.h | 13 ++-
arch/powerpc/include/asm/kasan.h | 22 +++++
arch/powerpc/kernel/Makefile | 11 +++
arch/powerpc/kernel/process.c | 16 ++--
arch/powerpc/kvm/Makefile | 5 +
arch/powerpc/mm/book3s64/Makefile | 9 ++
arch/powerpc/mm/kasan/Makefile | 3 +-
.../mm/kasan/{kasan_init_32.c => init_32.c} | 0
arch/powerpc/mm/kasan/init_book3s_64.c | 95 +++++++++++++++++++
arch/powerpc/mm/ptdump/ptdump.c | 20 +++-
arch/powerpc/platforms/Kconfig.cputype | 1 +
arch/powerpc/platforms/powernv/Makefile | 6 ++
arch/powerpc/platforms/pseries/Makefile | 3 +
include/linux/kasan.h | 18 +++-
lib/Kconfig.kasan | 14 +++
mm/kasan/common.c | 4 +
mm/kasan/generic.c | 3 +
mm/kasan/init.c | 6 +-
mm/kasan/kasan.h | 4 +
mm/kasan/shadow.c | 4 +
26 files changed, 316 insertions(+), 21 deletions(-)
create mode 100644 Documentation/powerpc/kasan.txt
rename arch/powerpc/mm/kasan/{kasan_init_32.c => init_32.c} (100%)
create mode 100644 arch/powerpc/mm/kasan/init_book3s_64.c
--
2.27.0
^ permalink raw reply
* [PATCH v12 1/6] kasan: allow an architecture to disable inline instrumentation
From: Daniel Axtens @ 2021-06-15 1:47 UTC (permalink / raw)
To: linux-kernel, linux-mm, linuxppc-dev, kasan-dev, christophe.leroy,
aneesh.kumar, bsingharora
Cc: elver, Daniel Axtens
In-Reply-To: <20210615014705.2234866-1-dja@axtens.net>
For annoying architectural reasons, it's very difficult to support inline
instrumentation on powerpc64.
Add a Kconfig flag to allow an arch to disable inline. (It's a bit
annoying to be 'backwards', but I'm not aware of any way to have
an arch force a symbol to be 'n', rather than 'y'.)
We also disable stack instrumentation in this case as it does things that
are functionally equivalent to inline instrumentation, namely adding
code that touches the shadow directly without going through a C helper.
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
lib/Kconfig.kasan | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index cffc2ebbf185..935814f332a7 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -12,6 +12,15 @@ config HAVE_ARCH_KASAN_HW_TAGS
config HAVE_ARCH_KASAN_VMALLOC
bool
+# Sometimes an architecture might not be able to support inline instrumentation
+# but might be able to support outline instrumentation. This option allows an
+# arch to prevent inline and stack instrumentation from being enabled.
+# ppc64 turns on virtual memory late in boot, after calling into generic code
+# like the device-tree parser, so it uses this in conjuntion with a hook in
+# outline mode to avoid invalid access early in boot.
+config ARCH_DISABLE_KASAN_INLINE
+ bool
+
config CC_HAS_KASAN_GENERIC
def_bool $(cc-option, -fsanitize=kernel-address)
@@ -130,6 +139,7 @@ config KASAN_OUTLINE
config KASAN_INLINE
bool "Inline instrumentation"
+ depends on !ARCH_DISABLE_KASAN_INLINE
help
Compiler directly inserts code checking shadow memory before
memory accesses. This is faster than outline (in some workloads
@@ -141,6 +151,7 @@ endchoice
config KASAN_STACK
bool "Enable stack instrumentation (unsafe)" if CC_IS_CLANG && !COMPILE_TEST
depends on KASAN_GENERIC || KASAN_SW_TAGS
+ depends on !ARCH_DISABLE_KASAN_INLINE
default y if CC_IS_GCC
help
The LLVM stack address sanitizer has a know problem that
@@ -154,6 +165,9 @@ config KASAN_STACK
but clang users can still enable it for builds without
CONFIG_COMPILE_TEST. On gcc it is assumed to always be safe
to use and enabled by default.
+ If the architecture disables inline instrumentation, this is
+ also disabled as it adds inline-style instrumentation that
+ is run unconditionally.
config KASAN_SW_TAGS_IDENTIFY
bool "Enable memory corruption identification"
--
2.27.0
^ permalink raw reply related
* [PATCH v12 2/6] kasan: allow architectures to provide an outline readiness check
From: Daniel Axtens @ 2021-06-15 1:47 UTC (permalink / raw)
To: linux-kernel, linux-mm, linuxppc-dev, kasan-dev, christophe.leroy,
aneesh.kumar, bsingharora
Cc: elver, Aneesh Kumar K . V, Daniel Axtens
In-Reply-To: <20210615014705.2234866-1-dja@axtens.net>
Allow architectures to define a kasan_arch_is_ready() hook that bails
out of any function that's about to touch the shadow unless the arch
says that it is ready for the memory to be accessed. This is fairly
uninvasive and should have a negligible performance penalty.
This will only work in outline mode, so an arch must specify
ARCH_DISABLE_KASAN_INLINE if it requires this.
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Daniel Axtens <dja@axtens.net>
--
I discuss the justfication for this later in the series. Also,
both previous RFCs for ppc64 - by 2 different people - have
needed this trick! See:
- https://lore.kernel.org/patchwork/patch/592820/ # ppc64 hash series
- https://patchwork.ozlabs.org/patch/795211/ # ppc radix series
---
mm/kasan/common.c | 4 ++++
mm/kasan/generic.c | 3 +++
mm/kasan/kasan.h | 4 ++++
mm/kasan/shadow.c | 4 ++++
4 files changed, 15 insertions(+)
diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index 10177cc26d06..0ad615f3801d 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -331,6 +331,10 @@ static inline bool ____kasan_slab_free(struct kmem_cache *cache, void *object,
u8 tag;
void *tagged_object;
+ /* Bail if the arch isn't ready */
+ if (!kasan_arch_is_ready())
+ return false;
+
tag = get_tag(object);
tagged_object = object;
object = kasan_reset_tag(object);
diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
index 53cbf28859b5..c3f5ba7a294a 100644
--- a/mm/kasan/generic.c
+++ b/mm/kasan/generic.c
@@ -163,6 +163,9 @@ static __always_inline bool check_region_inline(unsigned long addr,
size_t size, bool write,
unsigned long ret_ip)
{
+ if (!kasan_arch_is_ready())
+ return true;
+
if (unlikely(size == 0))
return true;
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 8f450bc28045..19323a3d5975 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -449,6 +449,10 @@ static inline void kasan_poison_last_granule(const void *address, size_t size) {
#endif /* CONFIG_KASAN_GENERIC */
+#ifndef kasan_arch_is_ready
+static inline bool kasan_arch_is_ready(void) { return true; }
+#endif
+
/*
* Exported functions for interfaces called from assembly or from generated
* code. Declarations here to avoid warning about missing declarations.
diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c
index 082ee5b6d9a1..74134b657d7d 100644
--- a/mm/kasan/shadow.c
+++ b/mm/kasan/shadow.c
@@ -73,6 +73,10 @@ void kasan_poison(const void *addr, size_t size, u8 value, bool init)
{
void *shadow_start, *shadow_end;
+ /* Don't touch the shadow memory if arch isn't ready */
+ if (!kasan_arch_is_ready())
+ return;
+
/*
* Perform shadow offset calculation based on untagged address, as
* some of the callers (e.g. kasan_poison_object_data) pass tagged
--
2.27.0
^ permalink raw reply related
* [PATCH v12 3/6] kasan: define and use MAX_PTRS_PER_* for early shadow tables
From: Daniel Axtens @ 2021-06-15 1:47 UTC (permalink / raw)
To: linux-kernel, linux-mm, linuxppc-dev, kasan-dev, christophe.leroy,
aneesh.kumar, bsingharora
Cc: elver, Daniel Axtens
In-Reply-To: <20210615014705.2234866-1-dja@axtens.net>
powerpc has a variable number of PTRS_PER_*, set at runtime based
on the MMU that the kernel is booted under.
This means the PTRS_PER_* are no longer constants, and therefore
breaks the build.
Define default MAX_PTRS_PER_*s in the same style as MAX_PTRS_PER_P4D.
As KASAN is the only user at the moment, just define them in the kasan
header, and have them default to PTRS_PER_* unless overridden in arch
code.
Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Suggested-by: Balbir Singh <bsingharora@gmail.com>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
include/linux/kasan.h | 18 +++++++++++++++---
mm/kasan/init.c | 6 +++---
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 768d7d342757..fd65f477ac92 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -40,10 +40,22 @@ struct kunit_kasan_expectation {
#define PTE_HWTABLE_PTRS 0
#endif
+#ifndef MAX_PTRS_PER_PTE
+#define MAX_PTRS_PER_PTE PTRS_PER_PTE
+#endif
+
+#ifndef MAX_PTRS_PER_PMD
+#define MAX_PTRS_PER_PMD PTRS_PER_PMD
+#endif
+
+#ifndef MAX_PTRS_PER_PUD
+#define MAX_PTRS_PER_PUD PTRS_PER_PUD
+#endif
+
extern unsigned char kasan_early_shadow_page[PAGE_SIZE];
-extern pte_t kasan_early_shadow_pte[PTRS_PER_PTE + PTE_HWTABLE_PTRS];
-extern pmd_t kasan_early_shadow_pmd[PTRS_PER_PMD];
-extern pud_t kasan_early_shadow_pud[PTRS_PER_PUD];
+extern pte_t kasan_early_shadow_pte[MAX_PTRS_PER_PTE + PTE_HWTABLE_PTRS];
+extern pmd_t kasan_early_shadow_pmd[MAX_PTRS_PER_PMD];
+extern pud_t kasan_early_shadow_pud[MAX_PTRS_PER_PUD];
extern p4d_t kasan_early_shadow_p4d[MAX_PTRS_PER_P4D];
int kasan_populate_early_shadow(const void *shadow_start,
diff --git a/mm/kasan/init.c b/mm/kasan/init.c
index 348f31d15a97..cc64ed6858c6 100644
--- a/mm/kasan/init.c
+++ b/mm/kasan/init.c
@@ -41,7 +41,7 @@ static inline bool kasan_p4d_table(pgd_t pgd)
}
#endif
#if CONFIG_PGTABLE_LEVELS > 3
-pud_t kasan_early_shadow_pud[PTRS_PER_PUD] __page_aligned_bss;
+pud_t kasan_early_shadow_pud[MAX_PTRS_PER_PUD] __page_aligned_bss;
static inline bool kasan_pud_table(p4d_t p4d)
{
return p4d_page(p4d) == virt_to_page(lm_alias(kasan_early_shadow_pud));
@@ -53,7 +53,7 @@ static inline bool kasan_pud_table(p4d_t p4d)
}
#endif
#if CONFIG_PGTABLE_LEVELS > 2
-pmd_t kasan_early_shadow_pmd[PTRS_PER_PMD] __page_aligned_bss;
+pmd_t kasan_early_shadow_pmd[MAX_PTRS_PER_PMD] __page_aligned_bss;
static inline bool kasan_pmd_table(pud_t pud)
{
return pud_page(pud) == virt_to_page(lm_alias(kasan_early_shadow_pmd));
@@ -64,7 +64,7 @@ static inline bool kasan_pmd_table(pud_t pud)
return false;
}
#endif
-pte_t kasan_early_shadow_pte[PTRS_PER_PTE + PTE_HWTABLE_PTRS]
+pte_t kasan_early_shadow_pte[MAX_PTRS_PER_PTE + PTE_HWTABLE_PTRS]
__page_aligned_bss;
static inline bool kasan_pte_table(pmd_t pmd)
--
2.27.0
^ permalink raw reply related
* [PATCH v12 4/6] kasan: Document support on 32-bit powerpc
From: Daniel Axtens @ 2021-06-15 1:47 UTC (permalink / raw)
To: linux-kernel, linux-mm, linuxppc-dev, kasan-dev, christophe.leroy,
aneesh.kumar, bsingharora
Cc: elver, Daniel Axtens
In-Reply-To: <20210615014705.2234866-1-dja@axtens.net>
KASAN is supported on 32-bit powerpc and the docs should reflect this.
Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
Documentation/dev-tools/kasan.rst | 8 ++++++--
Documentation/powerpc/kasan.txt | 12 ++++++++++++
2 files changed, 18 insertions(+), 2 deletions(-)
create mode 100644 Documentation/powerpc/kasan.txt
diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
index 83ec4a556c19..05d2d428a332 100644
--- a/Documentation/dev-tools/kasan.rst
+++ b/Documentation/dev-tools/kasan.rst
@@ -36,7 +36,8 @@ Both software KASAN modes work with SLUB and SLAB memory allocators,
while the hardware tag-based KASAN currently only supports SLUB.
Currently, generic KASAN is supported for the x86_64, arm, arm64, xtensa, s390,
-and riscv architectures, and tag-based KASAN modes are supported only for arm64.
+and riscv architectures. It is also supported on 32-bit powerpc kernels.
+Tag-based KASAN modes are supported only for arm64.
Usage
-----
@@ -343,7 +344,10 @@ CONFIG_KASAN_VMALLOC
With ``CONFIG_KASAN_VMALLOC``, KASAN can cover vmalloc space at the
cost of greater memory usage. Currently, this is supported on x86,
-riscv, s390, and powerpc.
+riscv, s390, and 32-bit powerpc.
+
+It is optional, except on 32-bit powerpc kernels with module support,
+where it is required.
This works by hooking into vmalloc and vmap and dynamically
allocating real shadow memory to back the mappings.
diff --git a/Documentation/powerpc/kasan.txt b/Documentation/powerpc/kasan.txt
new file mode 100644
index 000000000000..26bb0e8bb18c
--- /dev/null
+++ b/Documentation/powerpc/kasan.txt
@@ -0,0 +1,12 @@
+KASAN is supported on powerpc on 32-bit only.
+
+32 bit support
+==============
+
+KASAN is supported on both hash and nohash MMUs on 32-bit.
+
+The shadow area sits at the top of the kernel virtual memory space above the
+fixmap area and occupies one eighth of the total kernel virtual memory space.
+
+Instrumentation of the vmalloc area is optional, unless built with modules,
+in which case it is required.
--
2.27.0
^ permalink raw reply related
* [PATCH v12 5/6] powerpc/mm/kasan: rename kasan_init_32.c to init_32.c
From: Daniel Axtens @ 2021-06-15 1:47 UTC (permalink / raw)
To: linux-kernel, linux-mm, linuxppc-dev, kasan-dev, christophe.leroy,
aneesh.kumar, bsingharora
Cc: elver, Daniel Axtens
In-Reply-To: <20210615014705.2234866-1-dja@axtens.net>
kasan is already implied by the directory name, we don't need to
repeat it.
Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
arch/powerpc/mm/kasan/Makefile | 2 +-
arch/powerpc/mm/kasan/{kasan_init_32.c => init_32.c} | 0
2 files changed, 1 insertion(+), 1 deletion(-)
rename arch/powerpc/mm/kasan/{kasan_init_32.c => init_32.c} (100%)
diff --git a/arch/powerpc/mm/kasan/Makefile b/arch/powerpc/mm/kasan/Makefile
index bb1a5408b86b..42fb628a44fd 100644
--- a/arch/powerpc/mm/kasan/Makefile
+++ b/arch/powerpc/mm/kasan/Makefile
@@ -2,6 +2,6 @@
KASAN_SANITIZE := n
-obj-$(CONFIG_PPC32) += kasan_init_32.o
+obj-$(CONFIG_PPC32) += init_32.o
obj-$(CONFIG_PPC_8xx) += 8xx.o
obj-$(CONFIG_PPC_BOOK3S_32) += book3s_32.o
diff --git a/arch/powerpc/mm/kasan/kasan_init_32.c b/arch/powerpc/mm/kasan/init_32.c
similarity index 100%
rename from arch/powerpc/mm/kasan/kasan_init_32.c
rename to arch/powerpc/mm/kasan/init_32.c
--
2.27.0
^ permalink raw reply related
* [PATCH v12 6/6] [RFC] powerpc: Book3S 64-bit outline-only KASAN support
From: Daniel Axtens @ 2021-06-15 1:47 UTC (permalink / raw)
To: linux-kernel, linux-mm, linuxppc-dev, kasan-dev, christophe.leroy,
aneesh.kumar, bsingharora
Cc: elver, Daniel Axtens
In-Reply-To: <20210615014705.2234866-1-dja@axtens.net>
[I'm hoping to get this in a subsequent merge window after we get the core
changes in. I know there are still a few outstanding review comments, I just
wanted to make sure that I supplied a real use-case for the core changes I'm
proposing.]
Implement a limited form of KASAN for Book3S 64-bit machines running under
the Radix MMU, supporting only outline mode.
- Enable the compiler instrumentation to check addresses and maintain the
shadow region. (This is the guts of KASAN which we can easily reuse.)
- Require kasan-vmalloc support to handle modules and anything else in
vmalloc space.
- KASAN needs to be able to validate all pointer accesses, but we can't
instrument all kernel addresses - only linear map and vmalloc. On boot,
set up a single page of read-only shadow that marks all iomap and
vmemmap accesses as valid.
- Document KASAN in both generic and powerpc docs.
Background
----------
KASAN support on Book3S is a bit tricky to get right:
- It would be good to support inline instrumentation so as to be able to
catch stack issues that cannot be caught with outline mode.
- Inline instrumentation requires a fixed offset.
- Book3S runs code with translations off ("real mode") during boot,
including a lot of generic device-tree parsing code which is used to
determine MMU features.
[ppc64 mm note: The kernel installs a linear mapping at effective
address c000...-c008.... This is a one-to-one mapping with physical
memory from 0000... onward. Because of how memory accesses work on
powerpc 64-bit Book3S, a kernel pointer in the linear map accesses the
same memory both with translations on (accessing as an 'effective
address'), and with translations off (accessing as a 'real
address'). This works in both guests and the hypervisor. For more
details, see s5.7 of Book III of version 3 of the ISA, in particular
the Storage Control Overview, s5.7.3, and s5.7.5 - noting that this
KASAN implementation currently only supports Radix.]
- Some code - most notably a lot of KVM code - also runs with translations
off after boot.
- Therefore any offset has to point to memory that is valid with
translations on or off.
One approach is just to give up on inline instrumentation. This way
boot-time checks can be delayed until after the MMU is set is up, and we
can just not instrument any code that runs with translations off after
booting. Take this approach for now and require outline instrumentation.
Previous attempts allowed inline instrumentation. However, they came with
some unfortunate restrictions: only physically contiguous memory could be
used and it had to be specified at compile time. Maybe we can do better in
the future.
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> # ppc64 hash version
Cc: Christophe Leroy <christophe.leroy@csgroup.eu> # ppc32 version
Originally-by: Balbir Singh <bsingharora@gmail.com> # ppc64 out-of-line radix version
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
Documentation/dev-tools/kasan.rst | 11 +--
Documentation/powerpc/kasan.txt | 48 +++++++++-
arch/powerpc/Kconfig | 4 +-
arch/powerpc/Kconfig.debug | 3 +-
arch/powerpc/include/asm/book3s/64/hash.h | 4 +
arch/powerpc/include/asm/book3s/64/pgtable.h | 4 +
arch/powerpc/include/asm/book3s/64/radix.h | 13 ++-
arch/powerpc/include/asm/kasan.h | 22 +++++
arch/powerpc/kernel/Makefile | 11 +++
arch/powerpc/kernel/process.c | 16 ++--
arch/powerpc/kvm/Makefile | 5 ++
arch/powerpc/mm/book3s64/Makefile | 9 ++
arch/powerpc/mm/kasan/Makefile | 1 +
arch/powerpc/mm/kasan/init_book3s_64.c | 95 ++++++++++++++++++++
arch/powerpc/mm/ptdump/ptdump.c | 20 ++++-
arch/powerpc/platforms/Kconfig.cputype | 1 +
arch/powerpc/platforms/powernv/Makefile | 6 ++
arch/powerpc/platforms/pseries/Makefile | 3 +
18 files changed, 257 insertions(+), 19 deletions(-)
create mode 100644 arch/powerpc/mm/kasan/init_book3s_64.c
diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
index 05d2d428a332..f8d6048db1bb 100644
--- a/Documentation/dev-tools/kasan.rst
+++ b/Documentation/dev-tools/kasan.rst
@@ -36,8 +36,9 @@ Both software KASAN modes work with SLUB and SLAB memory allocators,
while the hardware tag-based KASAN currently only supports SLUB.
Currently, generic KASAN is supported for the x86_64, arm, arm64, xtensa, s390,
-and riscv architectures. It is also supported on 32-bit powerpc kernels.
-Tag-based KASAN modes are supported only for arm64.
+and riscv architectures. It is also supported on powerpc for 32-bit kernels and
+for 64-bit kernels running under the Radix MMU. Tag-based KASAN modes are
+supported only for arm64.
Usage
-----
@@ -344,10 +345,10 @@ CONFIG_KASAN_VMALLOC
With ``CONFIG_KASAN_VMALLOC``, KASAN can cover vmalloc space at the
cost of greater memory usage. Currently, this is supported on x86,
-riscv, s390, and 32-bit powerpc.
+riscv, s390, and powerpc.
-It is optional, except on 32-bit powerpc kernels with module support,
-where it is required.
+It is optional, except on 64-bit powerpc kernels, and on 32-bit
+powerpc kernels with module support, where it is required.
This works by hooking into vmalloc and vmap and dynamically
allocating real shadow memory to back the mappings.
diff --git a/Documentation/powerpc/kasan.txt b/Documentation/powerpc/kasan.txt
index 26bb0e8bb18c..f032b4eaf205 100644
--- a/Documentation/powerpc/kasan.txt
+++ b/Documentation/powerpc/kasan.txt
@@ -1,4 +1,4 @@
-KASAN is supported on powerpc on 32-bit only.
+KASAN is supported on powerpc on 32-bit and Radix 64-bit only.
32 bit support
==============
@@ -10,3 +10,49 @@ fixmap area and occupies one eighth of the total kernel virtual memory space.
Instrumentation of the vmalloc area is optional, unless built with modules,
in which case it is required.
+
+64 bit support
+==============
+
+Currently, only the radix MMU is supported. There have been versions for hash
+and Book3E processors floating around on the mailing list, but nothing has been
+merged.
+
+KASAN support on Book3S is a bit tricky to get right:
+
+ - It would be good to support inline instrumentation so as to be able to catch
+ stack issues that cannot be caught with outline mode.
+
+ - Inline instrumentation requires a fixed offset.
+
+ - Book3S runs code with translations off ("real mode") during boot, including a
+ lot of generic device-tree parsing code which is used to determine MMU
+ features.
+
+ - Some code - most notably a lot of KVM code - also runs with translations off
+ after boot.
+
+ - Therefore any offset has to point to memory that is valid with
+ translations on or off.
+
+One approach is just to give up on inline instrumentation. This way boot-time
+checks can be delayed until after the MMU is set is up, and we can just not
+instrument any code that runs with translations off after booting. This is the
+current approach.
+
+To avoid this limitiation, the KASAN shadow would have to be placed inside the
+linear mapping, using the same high-bits trick we use for the rest of the linear
+mapping. This is tricky:
+
+ - We'd like to place it near the start of physical memory. In theory we can do
+ this at run-time based on how much physical memory we have, but this requires
+ being able to arbitrarily relocate the kernel, which is basically the tricky
+ part of KASLR. Not being game to implement both tricky things at once, this
+ is hopefully something we can revisit once we get KASLR for Book3S.
+
+ - Alternatively, we can place the shadow at the _end_ of memory, but this
+ requires knowing how much contiguous physical memory a system has _at compile
+ time_. This is a big hammer, and has some unfortunate consequences: inablity
+ to handle discontiguous physical memory, total failure to boot on machines
+ with less memory than specified, and that machines with more memory than
+ specified can't use it. This was deemed unacceptable.
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index dbccb0676e48..ff16af7022b1 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -118,6 +118,7 @@ config PPC
# Please keep this list sorted alphabetically.
#
select ARCH_32BIT_OFF_T if PPC32
+ select ARCH_DISABLE_KASAN_INLINE if PPC_RADIX_MMU
select ARCH_ENABLE_MEMORY_HOTPLUG
select ARCH_ENABLE_MEMORY_HOTREMOVE
select ARCH_HAS_COPY_MC if PPC64
@@ -191,7 +192,8 @@ config PPC
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_JUMP_LABEL_RELATIVE
select HAVE_ARCH_KASAN if PPC32 && PPC_PAGE_SHIFT <= 14
- select HAVE_ARCH_KASAN_VMALLOC if PPC32 && PPC_PAGE_SHIFT <= 14
+ select HAVE_ARCH_KASAN if PPC_RADIX_MMU
+ select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN
select HAVE_ARCH_KFENCE if PPC32
select HAVE_ARCH_KGDB
select HAVE_ARCH_MMAP_RND_BITS
diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 6342f9da4545..ad5b776a96e7 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -399,4 +399,5 @@ config PPC_FAST_ENDIAN_SWITCH
config KASAN_SHADOW_OFFSET
hex
depends on KASAN
- default 0xe0000000
+ default 0xe0000000 if PPC32
+ default 0xa80e000000000000 if PPC64
diff --git a/arch/powerpc/include/asm/book3s/64/hash.h b/arch/powerpc/include/asm/book3s/64/hash.h
index d959b0195ad9..222669864ff6 100644
--- a/arch/powerpc/include/asm/book3s/64/hash.h
+++ b/arch/powerpc/include/asm/book3s/64/hash.h
@@ -18,6 +18,10 @@
#include <asm/book3s/64/hash-4k.h>
#endif
+#define H_PTRS_PER_PTE (1 << H_PTE_INDEX_SIZE)
+#define H_PTRS_PER_PMD (1 << H_PMD_INDEX_SIZE)
+#define H_PTRS_PER_PUD (1 << H_PUD_INDEX_SIZE)
+
/* Bits to set in a PMD/PUD/PGD entry valid bit*/
#define HASH_PMD_VAL_BITS (0x8000000000000000UL)
#define HASH_PUD_VAL_BITS (0x8000000000000000UL)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index a666d561b44d..49f2a2bbc0cf 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -232,6 +232,10 @@ extern unsigned long __pmd_frag_size_shift;
#define PTRS_PER_PUD (1 << PUD_INDEX_SIZE)
#define PTRS_PER_PGD (1 << PGD_INDEX_SIZE)
+#define MAX_PTRS_PER_PTE ((H_PTRS_PER_PTE > R_PTRS_PER_PTE) ? H_PTRS_PER_PTE : R_PTRS_PER_PTE)
+#define MAX_PTRS_PER_PMD ((H_PTRS_PER_PMD > R_PTRS_PER_PMD) ? H_PTRS_PER_PMD : R_PTRS_PER_PMD)
+#define MAX_PTRS_PER_PUD ((H_PTRS_PER_PUD > R_PTRS_PER_PUD) ? H_PTRS_PER_PUD : R_PTRS_PER_PUD)
+
/* PMD_SHIFT determines what a second-level page table entry can map */
#define PMD_SHIFT (PAGE_SHIFT + PTE_INDEX_SIZE)
#define PMD_SIZE (1UL << PMD_SHIFT)
diff --git a/arch/powerpc/include/asm/book3s/64/radix.h b/arch/powerpc/include/asm/book3s/64/radix.h
index 59cab558e2f0..191399143dc8 100644
--- a/arch/powerpc/include/asm/book3s/64/radix.h
+++ b/arch/powerpc/include/asm/book3s/64/radix.h
@@ -35,6 +35,11 @@
#define RADIX_PMD_SHIFT (PAGE_SHIFT + RADIX_PTE_INDEX_SIZE)
#define RADIX_PUD_SHIFT (RADIX_PMD_SHIFT + RADIX_PMD_INDEX_SIZE)
#define RADIX_PGD_SHIFT (RADIX_PUD_SHIFT + RADIX_PUD_INDEX_SIZE)
+
+#define R_PTRS_PER_PTE (1 << RADIX_PTE_INDEX_SIZE)
+#define R_PTRS_PER_PMD (1 << RADIX_PMD_INDEX_SIZE)
+#define R_PTRS_PER_PUD (1 << RADIX_PUD_INDEX_SIZE)
+
/*
* Size of EA range mapped by our pagetables.
*/
@@ -68,11 +73,11 @@
*
*
* 3rd quadrant expanded:
- * +------------------------------+
+ * +------------------------------+ Highest address (0xc010000000000000)
+ * +------------------------------+ KASAN shadow end (0xc00fc00000000000)
* | |
* | |
- * | |
- * +------------------------------+ Kernel vmemmap end (0xc010000000000000)
+ * +------------------------------+ Kernel vmemmap end/shadow start (0xc00e000000000000)
* | |
* | 512TB |
* | |
@@ -126,6 +131,8 @@
#define RADIX_VMEMMAP_SIZE RADIX_KERN_MAP_SIZE
#define RADIX_VMEMMAP_END (RADIX_VMEMMAP_START + RADIX_VMEMMAP_SIZE)
+/* For the sizes of the shadow area, see kasan.h */
+
#ifndef __ASSEMBLY__
#define RADIX_PTE_TABLE_SIZE (sizeof(pte_t) << RADIX_PTE_INDEX_SIZE)
#define RADIX_PMD_TABLE_SIZE (sizeof(pmd_t) << RADIX_PMD_INDEX_SIZE)
diff --git a/arch/powerpc/include/asm/kasan.h b/arch/powerpc/include/asm/kasan.h
index 3c478e5ef24c..6efc822e70fd 100644
--- a/arch/powerpc/include/asm/kasan.h
+++ b/arch/powerpc/include/asm/kasan.h
@@ -30,9 +30,31 @@
#define KASAN_SHADOW_OFFSET ASM_CONST(CONFIG_KASAN_SHADOW_OFFSET)
+#ifdef CONFIG_PPC32
#define KASAN_SHADOW_END (-(-KASAN_SHADOW_START >> KASAN_SHADOW_SCALE_SHIFT))
+#endif
#ifdef CONFIG_KASAN
+#ifdef CONFIG_PPC_BOOK3S_64
+/*
+ * The shadow ends before the highest accessible address
+ * because we don't need a shadow for the shadow. Instead:
+ * c00e000000000000 << 3 + a80e000000000000 = c00fc00000000000
+ */
+#define KASAN_SHADOW_END 0xc00fc00000000000UL
+
+DECLARE_STATIC_KEY_FALSE(powerpc_kasan_enabled_key);
+
+static __always_inline bool kasan_arch_is_ready(void)
+{
+ if (static_branch_likely(&powerpc_kasan_enabled_key))
+ return true;
+ return false;
+}
+
+#define kasan_arch_is_ready kasan_arch_is_ready
+#endif
+
void kasan_early_init(void);
void kasan_mmu_init(void);
void kasan_init(void);
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index f66b63e81c3b..aabac84106f1 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -32,6 +32,17 @@ KASAN_SANITIZE_early_32.o := n
KASAN_SANITIZE_cputable.o := n
KASAN_SANITIZE_prom_init.o := n
KASAN_SANITIZE_btext.o := n
+KASAN_SANITIZE_paca.o := n
+KASAN_SANITIZE_setup_64.o := n
+KASAN_SANITIZE_mce.o := n
+KASAN_SANITIZE_mce_power.o := n
+
+# we have to be particularly careful in ppc64 to exclude code that
+# runs with translations off, as we cannot access the shadow with
+# translations off. However, ppc32 can sanitize this.
+ifdef CONFIG_PPC64
+KASAN_SANITIZE_traps.o := n
+endif
ifdef CONFIG_KASAN
CFLAGS_early_32.o += -DDISABLE_BRANCH_PROFILING
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 89e34aa273e2..430cf06f9406 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2151,8 +2151,8 @@ void show_stack(struct task_struct *tsk, unsigned long *stack,
break;
stack = (unsigned long *) sp;
- newsp = stack[0];
- ip = stack[STACK_FRAME_LR_SAVE];
+ newsp = READ_ONCE_NOCHECK(stack[0]);
+ ip = READ_ONCE_NOCHECK(stack[STACK_FRAME_LR_SAVE]);
if (!firstframe || ip != lr) {
printk("%s["REG"] ["REG"] %pS",
loglvl, sp, ip, (void *)ip);
@@ -2170,17 +2170,19 @@ void show_stack(struct task_struct *tsk, unsigned long *stack,
* See if this is an exception frame.
* We look for the "regshere" marker in the current frame.
*/
- if (validate_sp(sp, tsk, STACK_FRAME_WITH_PT_REGS)
- && stack[STACK_FRAME_MARKER] == STACK_FRAME_REGS_MARKER) {
+ if (validate_sp(sp, tsk, STACK_FRAME_WITH_PT_REGS) &&
+ (READ_ONCE_NOCHECK(stack[STACK_FRAME_MARKER]) ==
+ STACK_FRAME_REGS_MARKER)) {
struct pt_regs *regs = (struct pt_regs *)
(sp + STACK_FRAME_OVERHEAD);
- lr = regs->link;
+ lr = READ_ONCE_NOCHECK(regs->link);
printk("%s--- interrupt: %lx at %pS\n",
- loglvl, regs->trap, (void *)regs->nip);
+ loglvl, READ_ONCE_NOCHECK(regs->trap),
+ (void *)READ_ONCE_NOCHECK(regs->nip));
__show_regs(regs);
printk("%s--- interrupt: %lx\n",
- loglvl, regs->trap);
+ loglvl, READ_ONCE_NOCHECK(regs->trap));
firstframe = 1;
}
diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
index 2bfeaa13befb..7f1592dacbeb 100644
--- a/arch/powerpc/kvm/Makefile
+++ b/arch/powerpc/kvm/Makefile
@@ -136,3 +136,8 @@ obj-$(CONFIG_KVM_BOOK3S_64_PR) += kvm-pr.o
obj-$(CONFIG_KVM_BOOK3S_64_HV) += kvm-hv.o
obj-y += $(kvm-book3s_64-builtin-objs-y)
+
+# KVM does a lot in real-mode, and 64-bit Book3S KASAN doesn't support that
+ifdef CONFIG_PPC_BOOK3S_64
+KASAN_SANITIZE := n
+endif
diff --git a/arch/powerpc/mm/book3s64/Makefile b/arch/powerpc/mm/book3s64/Makefile
index 1b56d3af47d4..a7d8a68bd2c5 100644
--- a/arch/powerpc/mm/book3s64/Makefile
+++ b/arch/powerpc/mm/book3s64/Makefile
@@ -21,3 +21,12 @@ obj-$(CONFIG_PPC_PKEY) += pkeys.o
# Instrumenting the SLB fault path can lead to duplicate SLB entries
KCOV_INSTRUMENT_slb.o := n
+
+# Parts of these can run in real mode and therefore are
+# not safe with the current outline KASAN implementation
+KASAN_SANITIZE_mmu_context.o := n
+KASAN_SANITIZE_pgtable.o := n
+KASAN_SANITIZE_radix_pgtable.o := n
+KASAN_SANITIZE_radix_tlb.o := n
+KASAN_SANITIZE_slb.o := n
+KASAN_SANITIZE_pkeys.o := n
diff --git a/arch/powerpc/mm/kasan/Makefile b/arch/powerpc/mm/kasan/Makefile
index 42fb628a44fd..07eef87abd6c 100644
--- a/arch/powerpc/mm/kasan/Makefile
+++ b/arch/powerpc/mm/kasan/Makefile
@@ -5,3 +5,4 @@ KASAN_SANITIZE := n
obj-$(CONFIG_PPC32) += init_32.o
obj-$(CONFIG_PPC_8xx) += 8xx.o
obj-$(CONFIG_PPC_BOOK3S_32) += book3s_32.o
+obj-$(CONFIG_PPC_BOOK3S_64) += init_book3s_64.o
diff --git a/arch/powerpc/mm/kasan/init_book3s_64.c b/arch/powerpc/mm/kasan/init_book3s_64.c
new file mode 100644
index 000000000000..ca913ed951a2
--- /dev/null
+++ b/arch/powerpc/mm/kasan/init_book3s_64.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KASAN for 64-bit Book3S powerpc
+ *
+ * Copyright (C) 2019-2020 IBM Corporation
+ * Author: Daniel Axtens <dja@axtens.net>
+ */
+
+#define DISABLE_BRANCH_PROFILING
+
+#include <linux/kasan.h>
+#include <linux/printk.h>
+#include <linux/sched/task.h>
+#include <linux/memblock.h>
+#include <asm/pgalloc.h>
+
+DEFINE_STATIC_KEY_FALSE(powerpc_kasan_enabled_key);
+
+static void __init kasan_init_phys_region(void *start, void *end)
+{
+ unsigned long k_start, k_end, k_cur;
+ void *va;
+
+ if (start >= end)
+ return;
+
+ k_start = ALIGN_DOWN((unsigned long)kasan_mem_to_shadow(start), PAGE_SIZE);
+ k_end = ALIGN((unsigned long)kasan_mem_to_shadow(end), PAGE_SIZE);
+
+ va = memblock_alloc(k_end - k_start, PAGE_SIZE);
+ for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE, va += PAGE_SIZE)
+ map_kernel_page(k_cur, __pa(va), PAGE_KERNEL);
+}
+
+void __init kasan_init(void)
+{
+ /*
+ * We want to do the following things:
+ * 1) Map real memory into the shadow for all physical memblocks
+ * This takes us from c000... to c008...
+ * 2) Leave a hole over the shadow of vmalloc space. KASAN_VMALLOC
+ * will manage this for us.
+ * This takes us from c008... to c00a...
+ * 3) Map the 'early shadow'/zero page over iomap and vmemmap space.
+ * This takes us up to where we start at c00e...
+ */
+
+ void *k_start = kasan_mem_to_shadow((void *)RADIX_VMALLOC_END);
+ void *k_end = kasan_mem_to_shadow((void *)RADIX_VMEMMAP_END);
+ phys_addr_t start, end;
+ u64 i;
+ pte_t zero_pte = pfn_pte(virt_to_pfn(kasan_early_shadow_page), PAGE_KERNEL);
+
+ if (!early_radix_enabled())
+ panic("KASAN requires radix!");
+
+ for_each_mem_range(i, &start, &end)
+ kasan_init_phys_region((void *)start, (void *)end);
+
+ for (i = 0; i < PTRS_PER_PTE; i++)
+ __set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
+ &kasan_early_shadow_pte[i], zero_pte, 0);
+
+ for (i = 0; i < PTRS_PER_PMD; i++)
+ pmd_populate_kernel(&init_mm, &kasan_early_shadow_pmd[i],
+ kasan_early_shadow_pte);
+
+ for (i = 0; i < PTRS_PER_PUD; i++)
+ pud_populate(&init_mm, &kasan_early_shadow_pud[i],
+ kasan_early_shadow_pmd);
+
+ /* map the early shadow over the iomap and vmemmap space */
+ kasan_populate_early_shadow(k_start, k_end);
+
+ /* mark early shadow region as RO and wipe it */
+ zero_pte = pfn_pte(virt_to_pfn(kasan_early_shadow_page), PAGE_KERNEL_RO);
+ for (i = 0; i < PTRS_PER_PTE; i++)
+ __set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
+ &kasan_early_shadow_pte[i], zero_pte, 0);
+
+ /*
+ * clear_page relies on some cache info that hasn't been set up yet.
+ * It ends up looping ~forever and blows up other data.
+ * Use memset instead.
+ */
+ memset(kasan_early_shadow_page, 0, PAGE_SIZE);
+
+ static_branch_inc(&powerpc_kasan_enabled_key);
+
+ /* Enable error messages */
+ init_task.kasan_depth = 0;
+ pr_info("KASAN init done (64-bit Book3S)\n");
+}
+
+void __init kasan_late_init(void) { }
diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
index aca354fb670b..63672aa656e8 100644
--- a/arch/powerpc/mm/ptdump/ptdump.c
+++ b/arch/powerpc/mm/ptdump/ptdump.c
@@ -20,6 +20,7 @@
#include <linux/seq_file.h>
#include <asm/fixmap.h>
#include <linux/const.h>
+#include <linux/kasan.h>
#include <asm/page.h>
#include <asm/hugetlb.h>
@@ -317,6 +318,23 @@ static void walk_pud(struct pg_state *st, p4d_t *p4d, unsigned long start)
unsigned long addr;
unsigned int i;
+#if defined(CONFIG_KASAN) && defined(CONFIG_PPC_BOOK3S_64)
+ /*
+ * On radix + KASAN, we want to check for the KASAN "early" shadow
+ * which covers huge quantities of memory with the same set of
+ * read-only PTEs. If it is, we want to note the first page (to see
+ * the status change), and then note the last page. This gives us good
+ * results without spending ages noting the exact same PTEs over 100s of
+ * terabytes of memory.
+ */
+ if (p4d_page(*p4d) == virt_to_page(lm_alias(kasan_early_shadow_pud))) {
+ walk_pmd(st, pud, start);
+ addr = start + (PTRS_PER_PUD - 1) * PUD_SIZE;
+ walk_pmd(st, pud, addr);
+ return;
+ }
+#endif
+
for (i = 0; i < PTRS_PER_PUD; i++, pud++) {
addr = start + i * PUD_SIZE;
if (!pud_none(*pud) && !pud_is_leaf(*pud))
@@ -387,11 +405,11 @@ static void populate_markers(void)
#endif
address_markers[i++].start_address = FIXADDR_START;
address_markers[i++].start_address = FIXADDR_TOP;
+#endif /* CONFIG_PPC64 */
#ifdef CONFIG_KASAN
address_markers[i++].start_address = KASAN_SHADOW_START;
address_markers[i++].start_address = KASAN_SHADOW_END;
#endif
-#endif /* CONFIG_PPC64 */
}
static int ptdump_show(struct seq_file *m, void *v)
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 113431604035..de70bfea2982 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -105,6 +105,7 @@ config PPC_BOOK3S_64
select PPC_MM_SLICES
select PPC_HAVE_KUEP
select PPC_HAVE_KUAP
+ select KASAN_VMALLOC if KASAN
config PPC_BOOK3E_64
bool "Embedded processors"
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index be2546b96816..d50f6fc71ac6 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -1,4 +1,10 @@
# SPDX-License-Identifier: GPL-2.0
+
+# nothing that deals with real mode is safe to KASAN
+# in particular, idle code runs a bunch of things in real mode
+KASAN_SANITIZE_idle.o := n
+KASAN_SANITIZE_pci-ioda.o := n
+
obj-y += setup.o opal-call.o opal-wrappers.o opal.o opal-async.o
obj-y += idle.o opal-rtc.o opal-nvram.o opal-lpc.o opal-flash.o
obj-y += rng.o opal-elog.o opal-dump.o opal-sysparam.o opal-sensor.o
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index c8a2b0b05ac0..202199ef9e5c 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -30,3 +30,6 @@ obj-$(CONFIG_PPC_SVM) += svm.o
obj-$(CONFIG_FA_DUMP) += rtas-fadump.o
obj-$(CONFIG_SUSPEND) += suspend.o
+
+# nothing that operates in real mode is safe for KASAN
+KASAN_SANITIZE_ras.o := n
--
2.27.0
^ permalink raw reply related
* Re: [PATCH v1 10/12] powerpc/lib/feature-fixups: Use PPC_RAW_xxx() macros
From: Michael Ellerman @ 2021-06-15 1:48 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
naveen.n.rao, jniethe5
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <e79cd8e111ca13bf8c61a384bac365aa7e207647.1621506159.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
> index fe26f2fa0f3f..f0fc521b82ae 100644
> --- a/arch/powerpc/lib/feature-fixups.c
> +++ b/arch/powerpc/lib/feature-fixups.c
> @@ -180,32 +180,31 @@ static void do_stf_exit_barrier_fixups(enum stf_barrier_type types)
> start = PTRRELOC(&__start___stf_exit_barrier_fixup);
> end = PTRRELOC(&__stop___stf_exit_barrier_fixup);
>
> - instrs[0] = 0x60000000; /* nop */
> - instrs[1] = 0x60000000; /* nop */
> - instrs[2] = 0x60000000; /* nop */
> - instrs[3] = 0x60000000; /* nop */
> - instrs[4] = 0x60000000; /* nop */
> - instrs[5] = 0x60000000; /* nop */
> + instrs[0] = PPC_RAW_NOP();
> + instrs[1] = PPC_RAW_NOP();
> + instrs[2] = PPC_RAW_NOP();
> + instrs[3] = PPC_RAW_NOP();
> + instrs[4] = PPC_RAW_NOP();
> + instrs[5] = PPC_RAW_NOP();
>
> i = 0;
> if (types & STF_BARRIER_FALLBACK || types & STF_BARRIER_SYNC_ORI) {
> if (cpu_has_feature(CPU_FTR_HVMODE)) {
> - instrs[i++] = 0x7db14ba6; /* mtspr 0x131, r13 (HSPRG1) */
> - instrs[i++] = 0x7db04aa6; /* mfspr r13, 0x130 (HSPRG0) */
> + instrs[i++] = PPC_RAW_MTSPR(SPRN_HSPRG1, _R13);
> + instrs[i++] = PPC_RAW_MFSPR(_R13, SPRN_HSPRG0);
> } else {
> - instrs[i++] = 0x7db243a6; /* mtsprg 2,r13 */
> - instrs[i++] = 0x7db142a6; /* mfsprg r13,1 */
> + instrs[i++] = PPC_RAW_MTSPR(SPRN_SPRG2, _R13);
> + instrs[i++] = PPC_RAW_MFSPR(_R13, SPRN_SPRG1);
> }
> - instrs[i++] = 0x7c0004ac; /* hwsync */
> - instrs[i++] = 0xe9ad0000; /* ld r13,0(r13) */
...
> + instrs[i++] = PPC_RAW_LD(_R10, _R13, 0);
This conversion was wrong, r13 became r10.
I fixed it up.
cheers
^ permalink raw reply
* Re: [PATCH] usb: gadget: fsl: properly remove remnant of MXC support
From: Joel Stanley @ 2021-06-15 1:51 UTC (permalink / raw)
To: Li Yang
Cc: Felipe Balbi, Arnd Bergmann, Greg Kroah-Hartman, linux-usb,
Linux Kernel Mailing List, Fabio Estevam, ran.wang_1,
linuxppc-dev
In-Reply-To: <20210612003128.372238-1-leoyang.li@nxp.com>
On Sat, 12 Jun 2021 at 00:31, Li Yang <leoyang.li@nxp.com> wrote:
>
> Commit a390bef7db1f ("usb: gadget: fsl_mxc_udc: Remove the driver")
> didn't remove all the MXC related stuff which can cause build problem
> for LS1021 when enabled again in Kconfig. This patch remove all the
> remnants.
>
> Signed-off-by: Li Yang <leoyang.li@nxp.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Will you re-submit the kconfig change once this is merged?
Cheers,
Joel
^ permalink raw reply
* Re: [PATCH -next] powerpc/spider-pci: Remove set but not used variable 'val'
From: libaokun (A) @ 2021-06-15 2:00 UTC (permalink / raw)
To: arnd, mpe, benh, paulus, linuxppc-dev, linux-kernel
Cc: yuehaibing, weiyongjun1, yukuai3, yangjihong1
In-Reply-To: <20210601085319.140461-1-libaokun1@huawei.com>
ping
在 2021/6/1 16:53, Baokun Li 写道:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> arch/powerpc/platforms/cell/spider-pci.c: In function 'spiderpci_io_flush':
> arch/powerpc/platforms/cell/spider-pci.c:28:6: warning:
> variable ‘val’ set but not used [-Wunused-but-set-variable]
>
> It never used since introduction.
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
> ---
> arch/powerpc/platforms/cell/spider-pci.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/cell/spider-pci.c b/arch/powerpc/platforms/cell/spider-pci.c
> index 93ea41680f54..a1c293f42a1f 100644
> --- a/arch/powerpc/platforms/cell/spider-pci.c
> +++ b/arch/powerpc/platforms/cell/spider-pci.c
> @@ -25,10 +25,9 @@ struct spiderpci_iowa_private {
> static void spiderpci_io_flush(struct iowa_bus *bus)
> {
> struct spiderpci_iowa_private *priv;
> - u32 val;
>
> priv = bus->private;
> - val = in_be32(priv->regs + SPIDER_PCI_DUMMY_READ);
> + in_be32(priv->regs + SPIDER_PCI_DUMMY_READ);
> iosync();
> }
>
^ permalink raw reply
* Re: [PATCH v2 08/12] powerpc: Don't use 'struct ppc_inst' to reference instruction location
From: Michael Ellerman @ 2021-06-15 2:01 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
naveen.n.rao, jniethe5
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <7062722b087228e42cbd896e39bfdf526d6a340a.1621516826.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> index 5a0740ebf132..32d318c3b180 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -139,7 +139,7 @@ static inline int ppc_inst_len(struct ppc_inst x)
> * Return the address of the next instruction, if the instruction @value was
> * located at @location.
> */
> -static inline struct ppc_inst *ppc_inst_next(void *location, struct ppc_inst *value)
> +static inline unsigned int *ppc_inst_next(unsigned int *location, unsigned int *value)
> {
> struct ppc_inst tmp;
>
It's not visible in the diff, but the rest of the function is:
tmp = ppc_inst_read(value);
return location + ppc_inst_len(tmp);
}
And so changing the type of location from void * to int * changes the
result of that addition, ie. previously it was in units of bytes, now
it's units of 4 bytes.
To fix it I've kept location as unsigned int *, and added a cast where
we do the addition. That way users of the function just see unsigned int *,
the cast to void * is an implementation detail.
We only have a handful of uses of ppc_inst_len(), so maybe that should
change name and return a result in units of int *. But that can be a
separate change.
> diff --git a/arch/powerpc/platforms/86xx/mpc86xx_smp.c b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
> index 87f524e4b09c..302f2a1e0361 100644
> --- a/arch/powerpc/platforms/86xx/mpc86xx_smp.c
> +++ b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
> @@ -83,7 +83,7 @@ smp_86xx_kick_cpu(int nr)
> mdelay(1);
>
> /* Restore the exception vector */
> - patch_instruction((struct ppc_inst *)vector, ppc_inst(save_vector));
> + patch_instruction(vector, ppc_inst(save_vector));
>
> local_irq_restore(flags);
>
There was another usage in here:
/* Setup fake reset vector to call __secondary_start_mpc86xx. */
target = (unsigned long) __secondary_start_mpc86xx;
- patch_branch((struct ppc_inst *)vector, target, BRANCH_SET_LINK);
+ patch_branch(vector, target, BRANCH_SET_LINK);
/* Kick that CPU */
smp_86xx_release_core(nr);
I fixed it up.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/signal64: Copy siginfo before changing regs->nip
From: Michael Ellerman @ 2021-06-15 2:05 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: dja, cmr
In-Reply-To: <1623631623.jvh0hlk56m.astroid@bobo.none>
Nicholas Piggin <npiggin@gmail.com> writes:
> Excerpts from Michael Ellerman's message of June 8, 2021 11:46 pm:
>> In commit 96d7a4e06fab ("powerpc/signal64: Rewrite handle_rt_signal64()
>> to minimise uaccess switches") the 64-bit signal code was rearranged to
>> use user_write_access_begin/end().
>>
>> As part of that change the call to copy_siginfo_to_user() was moved
>> later in the function, so that it could be done after the
>> user_write_access_end().
>>
>> In particular it was moved after we modify regs->nip to point to the
>> signal trampoline. That means if copy_siginfo_to_user() fails we exit
>> handle_rt_signal64() with an error but with regs->nip modified, whereas
>> previously we would not modify regs->nip until the copy succeeded.
>>
>> Returning an error from signal delivery but with regs->nip updated
>> leaves the process in a sort of half-delivered state. We do immediately
>> force a SEGV in signal_setup_done(), called from do_signal(), so the
>> process should never run in the half-delivered state.
>>
>> However that SEGV is not delivered until we've gone around to
>> do_notify_resume() again, so it's possible some tracing could observe
>> the half-delivered state.
>>
>> There are other cases where we fail signal delivery with regs partly
>> updated, eg. the write to newsp and SA_SIGINFO, but the latter at least
>> is very unlikely to fail as it reads back from the frame we just wrote
>> to.
>>
>> Looking at other arches they seem to be more careful about leaving regs
>> unchanged until the copy operations have succeeded, and in general that
>> seems like good hygenie.
>>
>> So although the current behaviour is not cleary buggy, it's also not
>> clearly correct. So move the call to copy_siginfo_to_user() up prior to
>> the modification of regs->nip, which is closer to the old behaviour, and
>> easier to reason about.
>
> Good catch, should it still have a Fixes: tag though? Even if it's not
> clearly buggy we want it to be patched.
Yeah I'll add one.
cheers
^ permalink raw reply
* Re: [PATCH v4 1/2] module: add elf_check_module_arch for module specific elf arch checks
From: Nicholas Piggin @ 2021-06-15 2:05 UTC (permalink / raw)
To: Jessica Yu; +Cc: Michal Suchánek, linuxppc-dev, linux-kernel
In-Reply-To: <YMdGWjBOmcstBwOl@p200300cbcf109700df096d564fe976c3.dip0.t-ipconnect.de>
Excerpts from Jessica Yu's message of June 14, 2021 10:06 pm:
> +++ Nicholas Piggin [11/06/21 19:39 +1000]:
>>The elf_check_arch() function is used to test usermode binaries, but
>>kernel modules may have more specific requirements. powerpc would like
>>to test for ABI version compatibility.
>>
>>Add an arch-overridable function elf_check_module_arch() that defaults
>>to elf_check_arch() and use it in elf_validity_check().
>>
>>Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>>[np: split patch, added changelog]
>>Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>---
>> include/linux/moduleloader.h | 5 +++++
>> kernel/module.c | 2 +-
>> 2 files changed, 6 insertions(+), 1 deletion(-)
>>
>>diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
>>index 9e09d11ffe5b..fdc042a84562 100644
>>--- a/include/linux/moduleloader.h
>>+++ b/include/linux/moduleloader.h
>>@@ -13,6 +13,11 @@
>> * must be implemented by each architecture.
>> */
>>
>>+// Allow arch to optionally do additional checking of module ELF header
>>+#ifndef elf_check_module_arch
>>+#define elf_check_module_arch elf_check_arch
>>+#endif
>
> Hi Nicholas,
>
> Why not make elf_check_module_arch() consistent with the other
> arch-specific functions? Please see module_frob_arch_sections(),
> module_{init,exit}_section(), etc in moduleloader.h. That is, they are
> all __weak functions that are overridable by arches. We can maybe make
> elf_check_module_arch() a weak symbol, available for arches to
> override if they want to perform additional elf checks. Then we don't
> have to have this one-off #define.
Like this? I like it. Good idea.
Thanks,
Nick
diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
index 9e09d11ffe5b..7b4587a19189 100644
--- a/include/linux/moduleloader.h
+++ b/include/linux/moduleloader.h
@@ -13,6 +13,9 @@
* must be implemented by each architecture.
*/
+/* arch may override to do additional checking of ELF header architecture */
+bool module_elf_check_arch(Elf_Ehdr *hdr);
+
/* Adjust arch-specific sections. Return 0 on success. */
int module_frob_arch_sections(Elf_Ehdr *hdr,
Elf_Shdr *sechdrs,
diff --git a/kernel/module.c b/kernel/module.c
index 7e78dfabca97..8b31c0b7c2a0 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3459,6 +3459,11 @@ static void flush_module_icache(const struct module *mod)
(unsigned long)mod->core_layout.base + mod->core_layout.size);
}
+bool __weak module_elf_check_arch(Elf_Ehdr *hdr)
+{
+ return elf_check_arch(hdr);
+}
+
int __weak module_frob_arch_sections(Elf_Ehdr *hdr,
Elf_Shdr *sechdrs,
char *secstrings,
^ permalink raw reply related
* Re: [PATCH] powerpc/signal64: Copy siginfo before changing regs->nip
From: Michael Ellerman @ 2021-06-15 2:07 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: dja, cmr
In-Reply-To: <1623633868.lnyugcilh9.astroid@bobo.none>
Nicholas Piggin <npiggin@gmail.com> writes:
> Excerpts from Nicholas Piggin's message of June 14, 2021 10:47 am:
>> Excerpts from Michael Ellerman's message of June 8, 2021 11:46 pm:
>>> In commit 96d7a4e06fab ("powerpc/signal64: Rewrite handle_rt_signal64()
>>> to minimise uaccess switches") the 64-bit signal code was rearranged to
>>> use user_write_access_begin/end().
>>>
>>> As part of that change the call to copy_siginfo_to_user() was moved
>>> later in the function, so that it could be done after the
>>> user_write_access_end().
>>>
>>> In particular it was moved after we modify regs->nip to point to the
>>> signal trampoline. That means if copy_siginfo_to_user() fails we exit
>>> handle_rt_signal64() with an error but with regs->nip modified, whereas
>>> previously we would not modify regs->nip until the copy succeeded.
>>>
>>> Returning an error from signal delivery but with regs->nip updated
>>> leaves the process in a sort of half-delivered state. We do immediately
>>> force a SEGV in signal_setup_done(), called from do_signal(), so the
>>> process should never run in the half-delivered state.
>>>
>>> However that SEGV is not delivered until we've gone around to
>>> do_notify_resume() again, so it's possible some tracing could observe
>>> the half-delivered state.
>>>
>>> There are other cases where we fail signal delivery with regs partly
>>> updated, eg. the write to newsp and SA_SIGINFO, but the latter at least
>>> is very unlikely to fail as it reads back from the frame we just wrote
>>> to.
>>>
>>> Looking at other arches they seem to be more careful about leaving regs
>>> unchanged until the copy operations have succeeded, and in general that
>>> seems like good hygenie.
>>>
>>> So although the current behaviour is not cleary buggy, it's also not
>>> clearly correct. So move the call to copy_siginfo_to_user() up prior to
>>> the modification of regs->nip, which is closer to the old behaviour, and
>>> easier to reason about.
>>
>> Good catch, should it still have a Fixes: tag though? Even if it's not
>> clearly buggy we want it to be patched.
>
> Also...
>
>>>
>>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>>> ---
>>> arch/powerpc/kernel/signal_64.c | 9 ++++-----
>>> 1 file changed, 4 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
>>> index dca66481d0c2..f9e1f5428b9e 100644
>>> --- a/arch/powerpc/kernel/signal_64.c
>>> +++ b/arch/powerpc/kernel/signal_64.c
>>> @@ -902,6 +902,10 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
>>> unsafe_copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set), badframe_block);
>>> user_write_access_end();
>>>
>>> + /* Save the siginfo outside of the unsafe block. */
>>> + if (copy_siginfo_to_user(&frame->info, &ksig->info))
>>> + goto badframe;
>>> +
>>> /* Make sure signal handler doesn't get spurious FP exceptions */
>>> tsk->thread.fp_state.fpscr = 0;
>>>
>>> @@ -915,11 +919,6 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
>>> regs->nip = (unsigned long) &frame->tramp[0];
>>> }
>>>
>>> -
>>> - /* Save the siginfo outside of the unsafe block. */
>>> - if (copy_siginfo_to_user(&frame->info, &ksig->info))
>>> - goto badframe;
>>> -
>>> /* Allocate a dummy caller frame for the signal handler. */
>>> newsp = ((unsigned long)frame) - __SIGNAL_FRAMESIZE;
>>> err |= put_user(regs->gpr[1], (unsigned long __user *)newsp);
>
> Does the same reasoning apply to this one and the ELF V1 function
> descriptor thing? It seems like you could move all of that block
> up instead. With your other SA_SIGINFO get_user patch, there would
> then be no possibility of error after you start modifying regs.
Yeah I think we should rework it further and eventually get to the point
were we leave regs untouched until we're guaranteed to return success.
It will need a bit more work though because of copy_siginfo_to_user().
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/signal64: Copy siginfo before changing regs->nip
From: Nicholas Piggin @ 2021-06-15 2:11 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev, Michael Ellerman; +Cc: dja, cmr
In-Reply-To: <a7739358-6dd3-2f5d-50c4-f6b908ab2718@csgroup.eu>
Excerpts from Christophe Leroy's message of June 14, 2021 5:22 pm:
>
>
> Le 14/06/2021 à 07:55, Nicholas Piggin a écrit :
>> Excerpts from Christophe Leroy's message of June 14, 2021 3:31 pm:
>>>
>>>
>>> Le 14/06/2021 à 03:29, Nicholas Piggin a écrit :
>>>> Excerpts from Nicholas Piggin's message of June 14, 2021 10:47 am:
>>>>> Excerpts from Michael Ellerman's message of June 8, 2021 11:46 pm:
>>>>>> In commit 96d7a4e06fab ("powerpc/signal64: Rewrite handle_rt_signal64()
>>>>>> to minimise uaccess switches") the 64-bit signal code was rearranged to
>>>>>> use user_write_access_begin/end().
>>>>>>
>>>>>> As part of that change the call to copy_siginfo_to_user() was moved
>>>>>> later in the function, so that it could be done after the
>>>>>> user_write_access_end().
>>>>>>
>>>>>> In particular it was moved after we modify regs->nip to point to the
>>>>>> signal trampoline. That means if copy_siginfo_to_user() fails we exit
>>>>>> handle_rt_signal64() with an error but with regs->nip modified, whereas
>>>>>> previously we would not modify regs->nip until the copy succeeded.
>>>>>>
>>>>>> Returning an error from signal delivery but with regs->nip updated
>>>>>> leaves the process in a sort of half-delivered state. We do immediately
>>>>>> force a SEGV in signal_setup_done(), called from do_signal(), so the
>>>>>> process should never run in the half-delivered state.
>>>>>>
>>>>>> However that SEGV is not delivered until we've gone around to
>>>>>> do_notify_resume() again, so it's possible some tracing could observe
>>>>>> the half-delivered state.
>>>>>>
>>>>>> There are other cases where we fail signal delivery with regs partly
>>>>>> updated, eg. the write to newsp and SA_SIGINFO, but the latter at least
>>>>>> is very unlikely to fail as it reads back from the frame we just wrote
>>>>>> to.
>>>>>>
>>>>>> Looking at other arches they seem to be more careful about leaving regs
>>>>>> unchanged until the copy operations have succeeded, and in general that
>>>>>> seems like good hygenie.
>>>>>>
>>>>>> So although the current behaviour is not cleary buggy, it's also not
>>>>>> clearly correct. So move the call to copy_siginfo_to_user() up prior to
>>>>>> the modification of regs->nip, which is closer to the old behaviour, and
>>>>>> easier to reason about.
>>>>>
>>>>> Good catch, should it still have a Fixes: tag though? Even if it's not
>>>>> clearly buggy we want it to be patched.
>>>>
>>>> Also...
>>>>
>>>>>>
>>>>>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>>>>>> ---
>>>>>> arch/powerpc/kernel/signal_64.c | 9 ++++-----
>>>>>> 1 file changed, 4 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
>>>>>> index dca66481d0c2..f9e1f5428b9e 100644
>>>>>> --- a/arch/powerpc/kernel/signal_64.c
>>>>>> +++ b/arch/powerpc/kernel/signal_64.c
>>>>>> @@ -902,6 +902,10 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
>>>>>> unsafe_copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set), badframe_block);
>>>>>> user_write_access_end();
>>>>>>
>>>>>> + /* Save the siginfo outside of the unsafe block. */
>>>>>> + if (copy_siginfo_to_user(&frame->info, &ksig->info))
>>>>>> + goto badframe;
>>>>>> +
>>>>>> /* Make sure signal handler doesn't get spurious FP exceptions */
>>>>>> tsk->thread.fp_state.fpscr = 0;
>>>>>>
>>>>>> @@ -915,11 +919,6 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
>>>>>> regs->nip = (unsigned long) &frame->tramp[0];
>>>>>> }
>>>>>>
>>>>>> -
>>>>>> - /* Save the siginfo outside of the unsafe block. */
>>>>>> - if (copy_siginfo_to_user(&frame->info, &ksig->info))
>>>>>> - goto badframe;
>>>>>> -
>>>>>> /* Allocate a dummy caller frame for the signal handler. */
>>>>>> newsp = ((unsigned long)frame) - __SIGNAL_FRAMESIZE;
>>>>>> err |= put_user(regs->gpr[1], (unsigned long __user *)newsp);
>>>>
>>>> Does the same reasoning apply to this one and the ELF V1 function
>>>> descriptor thing? It seems like you could move all of that block
>>>> up instead. With your other SA_SIGINFO get_user patch, there would
>>>> then be no possibility of error after you start modifying regs.
>>>>
>>>
>>> To move the above in the user access block, we need to open a larger window. At the time being the
>>> window opened only contains the 'frame'. 'newsp' points before the 'frame'.
>>>
>>
>> Only by 64/128 bytes though. Is that a problem? Not for 64s. Could it
>> cause more overhead than it saves on other platforms?
>
> No it is not a problem at all, just need to not be forgotten, on ppc64 it may go unnoticed, on 32s
> it will blew up if we forget to enlarge the access window and the access involves a different 256M
> segment (Very unlikely for sure but ...)
Okay, and it's a good point. Would be nice if there was some sanitizer
that could check this to byte granularity.
Thanks,
Nick
>> For protection, it looks like all the important control data is in the
>> signal frame anyway, this frame is just for stack unwinding?
>
> That's my understanding as well.
^ permalink raw reply
* Re: [PATCH v2 04/12] powerpc/inst: Avoid pointer dereferencing in ppc_inst_equal()
From: Jordan Niethe @ 2021-06-15 3:38 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, Paul Mackerras, Naveen N. Rao, linuxppc-dev
In-Reply-To: <d8b155e930b7a9708ca110e8ff0ace6713a7af75.1621516826.git.christophe.leroy@csgroup.eu>
On Thu, May 20, 2021 at 11:50 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
> Avoid casting/dereferencing ppc_inst() as u64* , check each member
> of the struct when relevant.
>
> And remove the 0xff initialisation of the suffix for non
> prefixed instruction. An instruction with 0xff as a suffix
> might be invalid, but still is a prefixed instruction and
> has to be considered as this.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/inst.h | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> index fc6adef528a5..5a0740ebf132 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -61,7 +61,7 @@ static inline int ppc_inst_primary_opcode(struct ppc_inst x)
> }
>
> #ifdef CONFIG_PPC64
> -#define ppc_inst(x) ((struct ppc_inst){ .val = (x), .suffix = 0xff })
> +#define ppc_inst(x) ((struct ppc_inst){ .val = (x) })
>
> #define ppc_inst_prefix(x, y) ((struct ppc_inst){ .val = (x), .suffix = (y) })
>
> @@ -72,7 +72,7 @@ static inline u32 ppc_inst_suffix(struct ppc_inst x)
>
> static inline bool ppc_inst_prefixed(struct ppc_inst x)
> {
> - return ppc_inst_primary_opcode(x) == OP_PREFIX && ppc_inst_suffix(x) != 0xff;
> + return ppc_inst_primary_opcode(x) == OP_PREFIX;
> }
>
> static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
> @@ -93,11 +93,6 @@ static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
> }
> }
>
> -static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
> -{
> - return *(u64 *)&x == *(u64 *)&y;
> -}
> -
> #else
>
> #define ppc_inst(x) ((struct ppc_inst){ .val = x })
> @@ -124,13 +119,17 @@ static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
> return *ptr;
> }
>
> +#endif /* CONFIG_PPC64 */
> +
> static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
> {
> - return ppc_inst_val(x) == ppc_inst_val(y);
> + if (ppc_inst_val(x) != ppc_inst_val(y))
> + return false;
> + if (!ppc_inst_prefixed(x))
> + return true;
> + return ppc_inst_suffix(x) == ppc_inst_suffix(y);
> }
>
> -#endif /* CONFIG_PPC64 */
> -
> static inline int ppc_inst_len(struct ppc_inst x)
> {
> return ppc_inst_prefixed(x) ? 8 : 4;
> --
> 2.25.0
>
Reviewed by: Jordan Niethe <jniethe5@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 05/12] powerpc: Do not dereference code as 'struct ppc_inst' (uprobe, code-patching, feature-fixups)
From: Jordan Niethe @ 2021-06-15 3:41 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, Paul Mackerras, Naveen N. Rao, linuxppc-dev
In-Reply-To: <c9a1201dd0a66b4a0f91f0fb46d9385cbf030feb.1621516826.git.christophe.leroy@csgroup.eu>
On Thu, May 20, 2021 at 11:50 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
> 'struct ppc_inst' is an internal structure to represent an instruction,
> it is not directly the representation of that instruction in text code.
> It is not meant to map and dereference code.
>
> Dereferencing code directly through 'struct ppc_inst' has two main issues:
> - On powerpc, structs are expected to be 8 bytes aligned while code is
> spread every 4 byte.
> - Should a non prefixed instruction lie at the end of the page and the
> following page not be mapped, it would generate a page fault.
>
> In-memory code must be accessed with ppc_inst_read().
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/kernel/uprobes.c | 2 +-
> arch/powerpc/lib/code-patching.c | 8 ++++----
> arch/powerpc/lib/feature-fixups.c | 2 +-
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
> index 186f69b11e94..46971bb41d05 100644
> --- a/arch/powerpc/kernel/uprobes.c
> +++ b/arch/powerpc/kernel/uprobes.c
> @@ -42,7 +42,7 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
> return -EINVAL;
>
> if (cpu_has_feature(CPU_FTR_ARCH_31) &&
> - ppc_inst_prefixed(auprobe->insn) &&
> + ppc_inst_prefixed(ppc_inst_read(&auprobe->insn)) &&
> (addr & 0x3f) == 60) {
> pr_info_ratelimited("Cannot register a uprobe on 64 byte unaligned prefixed instruction\n");
> return -EINVAL;
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index 870b30d9be2f..0308429b0d1a 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -329,13 +329,13 @@ static unsigned long branch_iform_target(const struct ppc_inst *instr)
> {
> signed long imm;
>
> - imm = ppc_inst_val(*instr) & 0x3FFFFFC;
> + imm = ppc_inst_val(ppc_inst_read(instr)) & 0x3FFFFFC;
>
> /* If the top bit of the immediate value is set this is negative */
> if (imm & 0x2000000)
> imm -= 0x4000000;
>
> - if ((ppc_inst_val(*instr) & BRANCH_ABSOLUTE) == 0)
> + if ((ppc_inst_val(ppc_inst_read(instr)) & BRANCH_ABSOLUTE) == 0)
> imm += (unsigned long)instr;
>
> return (unsigned long)imm;
> @@ -345,13 +345,13 @@ static unsigned long branch_bform_target(const struct ppc_inst *instr)
> {
> signed long imm;
>
> - imm = ppc_inst_val(*instr) & 0xFFFC;
> + imm = ppc_inst_val(ppc_inst_read(instr)) & 0xFFFC;
>
> /* If the top bit of the immediate value is set this is negative */
> if (imm & 0x8000)
> imm -= 0x10000;
>
> - if ((ppc_inst_val(*instr) & BRANCH_ABSOLUTE) == 0)
> + if ((ppc_inst_val(ppc_inst_read(instr)) & BRANCH_ABSOLUTE) == 0)
> imm += (unsigned long)instr;
>
> return (unsigned long)imm;
> diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
> index fe26f2fa0f3f..8905b53109bc 100644
> --- a/arch/powerpc/lib/feature-fixups.c
> +++ b/arch/powerpc/lib/feature-fixups.c
> @@ -51,7 +51,7 @@ static int patch_alt_instruction(struct ppc_inst *src, struct ppc_inst *dest,
>
> instr = ppc_inst_read(src);
>
> - if (instr_is_relative_branch(*src)) {
> + if (instr_is_relative_branch(ppc_inst_read(src))) {
The above variable instr could be used here, but that is not an issue
with this patch.
> struct ppc_inst *target = (struct ppc_inst *)branch_target(src);
>
> /* Branch within the section doesn't need translating */
> --
> 2.25.0
>
Reviewed by: Jordan Niethe <jniethe5@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 06/12] powerpc/lib/code-patching: Make instr_is_branch_to_addr() static
From: Jordan Niethe @ 2021-06-15 3:42 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, Paul Mackerras, Naveen N. Rao, linuxppc-dev
In-Reply-To: <5f6b9c8c83170ed310953eac2f5b14539bfc964a.1621516826.git.christophe.leroy@csgroup.eu>
On Thu, May 20, 2021 at 11:50 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
> instr_is_branch_to_addr() is only used in code-patching.c
>
> Make it static.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/code-patching.h | 1 -
> arch/powerpc/lib/code-patching.c | 18 +++++++++---------
> 2 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
> index f1d029bf906e..f9bd1397b696 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -59,7 +59,6 @@ static inline int modify_instruction_site(s32 *site, unsigned int clr, unsigned
>
> int instr_is_relative_branch(struct ppc_inst instr);
> int instr_is_relative_link_branch(struct ppc_inst instr);
> -int instr_is_branch_to_addr(const struct ppc_inst *instr, unsigned long addr);
> unsigned long branch_target(const struct ppc_inst *instr);
> int translate_branch(struct ppc_inst *instr, const struct ppc_inst *dest,
> const struct ppc_inst *src);
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index 0308429b0d1a..82f2c1edb498 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -367,15 +367,6 @@ unsigned long branch_target(const struct ppc_inst *instr)
> return 0;
> }
>
> -int instr_is_branch_to_addr(const struct ppc_inst *instr, unsigned long addr)
> -{
> - if (instr_is_branch_iform(ppc_inst_read(instr)) ||
> - instr_is_branch_bform(ppc_inst_read(instr)))
> - return branch_target(instr) == addr;
> -
> - return 0;
> -}
> -
> int translate_branch(struct ppc_inst *instr, const struct ppc_inst *dest,
> const struct ppc_inst *src)
> {
> @@ -410,6 +401,15 @@ void __patch_exception(int exc, unsigned long addr)
>
> #ifdef CONFIG_CODE_PATCHING_SELFTEST
>
> +static int instr_is_branch_to_addr(const struct ppc_inst *instr, unsigned long addr)
> +{
> + if (instr_is_branch_iform(ppc_inst_read(instr)) ||
> + instr_is_branch_bform(ppc_inst_read(instr)))
> + return branch_target(instr) == addr;
> +
> + return 0;
> +}
> +
> static void __init test_trampoline(void)
> {
> asm ("nop;\n");
> --
> 2.25.0
>
Reviewed by: Jordan Niethe <jniethe5@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 07/12] powerpc/lib/code-patching: Don't use struct 'ppc_inst' for runnable code in tests.
From: Jordan Niethe @ 2021-06-15 3:42 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, Paul Mackerras, Naveen N. Rao, linuxppc-dev
In-Reply-To: <d8425fb42a4adebc35b7509f121817eeb02fac31.1621516826.git.christophe.leroy@csgroup.eu>
On Thu, May 20, 2021 at 11:50 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
> 'struct ppc_inst' is meant to represent an instruction internally, it
> is not meant to dereference code in memory.
>
> For testing code patching, use patch_instruction() to properly
> write into memory the code to be tested.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/lib/code-patching.c | 95 ++++++++++++++++++--------------
> 1 file changed, 53 insertions(+), 42 deletions(-)
>
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index 82f2c1edb498..508e9511ca96 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -422,9 +422,9 @@ static void __init test_branch_iform(void)
> {
> int err;
> struct ppc_inst instr;
> - unsigned long addr;
> -
> - addr = (unsigned long)&instr;
> + unsigned int tmp[2];
> + struct ppc_inst *iptr = (struct ppc_inst *)tmp;
> + unsigned long addr = (unsigned long)tmp;
>
> /* The simplest case, branch to self, no flags */
> check(instr_is_branch_iform(ppc_inst(0x48000000)));
> @@ -445,52 +445,57 @@ static void __init test_branch_iform(void)
> check(!instr_is_branch_iform(ppc_inst(0x7bfffffd)));
>
> /* Absolute branch to 0x100 */
> - instr = ppc_inst(0x48000103);
> - check(instr_is_branch_to_addr(&instr, 0x100));
> + patch_instruction(iptr, ppc_inst(0x48000103));
> + check(instr_is_branch_to_addr(iptr, 0x100));
> /* Absolute branch to 0x420fc */
> - instr = ppc_inst(0x480420ff);
> - check(instr_is_branch_to_addr(&instr, 0x420fc));
> + patch_instruction(iptr, ppc_inst(0x480420ff));
> + check(instr_is_branch_to_addr(iptr, 0x420fc));
> /* Maximum positive relative branch, + 20MB - 4B */
> - instr = ppc_inst(0x49fffffc);
> - check(instr_is_branch_to_addr(&instr, addr + 0x1FFFFFC));
> + patch_instruction(iptr, ppc_inst(0x49fffffc));
> + check(instr_is_branch_to_addr(iptr, addr + 0x1FFFFFC));
> /* Smallest negative relative branch, - 4B */
> - instr = ppc_inst(0x4bfffffc);
> - check(instr_is_branch_to_addr(&instr, addr - 4));
> + patch_instruction(iptr, ppc_inst(0x4bfffffc));
> + check(instr_is_branch_to_addr(iptr, addr - 4));
> /* Largest negative relative branch, - 32 MB */
> - instr = ppc_inst(0x4a000000);
> - check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
> + patch_instruction(iptr, ppc_inst(0x4a000000));
> + check(instr_is_branch_to_addr(iptr, addr - 0x2000000));
>
> /* Branch to self, with link */
> - err = create_branch(&instr, &instr, addr, BRANCH_SET_LINK);
> - check(instr_is_branch_to_addr(&instr, addr));
> + err = create_branch(&instr, iptr, addr, BRANCH_SET_LINK);
> + patch_instruction(iptr, instr);
> + check(instr_is_branch_to_addr(iptr, addr));
>
> /* Branch to self - 0x100, with link */
> - err = create_branch(&instr, &instr, addr - 0x100, BRANCH_SET_LINK);
> - check(instr_is_branch_to_addr(&instr, addr - 0x100));
> + err = create_branch(&instr, iptr, addr - 0x100, BRANCH_SET_LINK);
> + patch_instruction(iptr, instr);
> + check(instr_is_branch_to_addr(iptr, addr - 0x100));
>
> /* Branch to self + 0x100, no link */
> - err = create_branch(&instr, &instr, addr + 0x100, 0);
> - check(instr_is_branch_to_addr(&instr, addr + 0x100));
> + err = create_branch(&instr, iptr, addr + 0x100, 0);
> + patch_instruction(iptr, instr);
> + check(instr_is_branch_to_addr(iptr, addr + 0x100));
>
> /* Maximum relative negative offset, - 32 MB */
> - err = create_branch(&instr, &instr, addr - 0x2000000, BRANCH_SET_LINK);
> - check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
> + err = create_branch(&instr, iptr, addr - 0x2000000, BRANCH_SET_LINK);
> + patch_instruction(iptr, instr);
> + check(instr_is_branch_to_addr(iptr, addr - 0x2000000));
>
> /* Out of range relative negative offset, - 32 MB + 4*/
> - err = create_branch(&instr, &instr, addr - 0x2000004, BRANCH_SET_LINK);
> + err = create_branch(&instr, iptr, addr - 0x2000004, BRANCH_SET_LINK);
> check(err);
>
> /* Out of range relative positive offset, + 32 MB */
> - err = create_branch(&instr, &instr, addr + 0x2000000, BRANCH_SET_LINK);
> + err = create_branch(&instr, iptr, addr + 0x2000000, BRANCH_SET_LINK);
> check(err);
>
> /* Unaligned target */
> - err = create_branch(&instr, &instr, addr + 3, BRANCH_SET_LINK);
> + err = create_branch(&instr, iptr, addr + 3, BRANCH_SET_LINK);
> check(err);
>
> /* Check flags are masked correctly */
> - err = create_branch(&instr, &instr, addr, 0xFFFFFFFC);
> - check(instr_is_branch_to_addr(&instr, addr));
> + err = create_branch(&instr, iptr, addr, 0xFFFFFFFC);
> + patch_instruction(iptr, instr);
> + check(instr_is_branch_to_addr(iptr, addr));
> check(ppc_inst_equal(instr, ppc_inst(0x48000000)));
> }
>
> @@ -513,9 +518,10 @@ static void __init test_branch_bform(void)
> int err;
> unsigned long addr;
> struct ppc_inst *iptr, instr;
> + unsigned int tmp[2];
> unsigned int flags;
>
> - iptr = &instr;
> + iptr = (struct ppc_inst *)tmp;
> addr = (unsigned long)iptr;
>
> /* The simplest case, branch to self, no flags */
> @@ -528,39 +534,43 @@ static void __init test_branch_bform(void)
> check(!instr_is_branch_bform(ppc_inst(0x7bffffff)));
>
> /* Absolute conditional branch to 0x100 */
> - instr = ppc_inst(0x43ff0103);
> - check(instr_is_branch_to_addr(&instr, 0x100));
> + patch_instruction(iptr, ppc_inst(0x43ff0103));
> + check(instr_is_branch_to_addr(iptr, 0x100));
> /* Absolute conditional branch to 0x20fc */
> - instr = ppc_inst(0x43ff20ff);
> - check(instr_is_branch_to_addr(&instr, 0x20fc));
> + patch_instruction(iptr, ppc_inst(0x43ff20ff));
> + check(instr_is_branch_to_addr(iptr, 0x20fc));
> /* Maximum positive relative conditional branch, + 32 KB - 4B */
> - instr = ppc_inst(0x43ff7ffc);
> - check(instr_is_branch_to_addr(&instr, addr + 0x7FFC));
> + patch_instruction(iptr, ppc_inst(0x43ff7ffc));
> + check(instr_is_branch_to_addr(iptr, addr + 0x7FFC));
> /* Smallest negative relative conditional branch, - 4B */
> - instr = ppc_inst(0x43fffffc);
> - check(instr_is_branch_to_addr(&instr, addr - 4));
> + patch_instruction(iptr, ppc_inst(0x43fffffc));
> + check(instr_is_branch_to_addr(iptr, addr - 4));
> /* Largest negative relative conditional branch, - 32 KB */
> - instr = ppc_inst(0x43ff8000);
> - check(instr_is_branch_to_addr(&instr, addr - 0x8000));
> + patch_instruction(iptr, ppc_inst(0x43ff8000));
> + check(instr_is_branch_to_addr(iptr, addr - 0x8000));
>
> /* All condition code bits set & link */
> flags = 0x3ff000 | BRANCH_SET_LINK;
>
> /* Branch to self */
> err = create_cond_branch(&instr, iptr, addr, flags);
> - check(instr_is_branch_to_addr(&instr, addr));
> + patch_instruction(iptr, instr);
> + check(instr_is_branch_to_addr(iptr, addr));
>
> /* Branch to self - 0x100 */
> err = create_cond_branch(&instr, iptr, addr - 0x100, flags);
> - check(instr_is_branch_to_addr(&instr, addr - 0x100));
> + patch_instruction(iptr, instr);
> + check(instr_is_branch_to_addr(iptr, addr - 0x100));
>
> /* Branch to self + 0x100 */
> err = create_cond_branch(&instr, iptr, addr + 0x100, flags);
> - check(instr_is_branch_to_addr(&instr, addr + 0x100));
> + patch_instruction(iptr, instr);
> + check(instr_is_branch_to_addr(iptr, addr + 0x100));
>
> /* Maximum relative negative offset, - 32 KB */
> err = create_cond_branch(&instr, iptr, addr - 0x8000, flags);
> - check(instr_is_branch_to_addr(&instr, addr - 0x8000));
> + patch_instruction(iptr, instr);
> + check(instr_is_branch_to_addr(iptr, addr - 0x8000));
>
> /* Out of range relative negative offset, - 32 KB + 4*/
> err = create_cond_branch(&instr, iptr, addr - 0x8004, flags);
> @@ -576,7 +586,8 @@ static void __init test_branch_bform(void)
>
> /* Check flags are masked correctly */
> err = create_cond_branch(&instr, iptr, addr, 0xFFFFFFFC);
> - check(instr_is_branch_to_addr(&instr, addr));
> + patch_instruction(iptr, instr);
> + check(instr_is_branch_to_addr(iptr, addr));
> check(ppc_inst_equal(instr, ppc_inst(0x43FF0000)));
> }
>
> --
> 2.25.0
>
Reviewed by: Jordan Niethe <jniethe5@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 08/12] powerpc: Don't use 'struct ppc_inst' to reference instruction location
From: Jordan Niethe @ 2021-06-15 3:47 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-kernel, Paul Mackerras, Naveen N. Rao, linuxppc-dev
In-Reply-To: <871r93vqcb.fsf@mpe.ellerman.id.au>
On Tue, Jun 15, 2021 at 12:01 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> > diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> > index 5a0740ebf132..32d318c3b180 100644
> > --- a/arch/powerpc/include/asm/inst.h
> > +++ b/arch/powerpc/include/asm/inst.h
> > @@ -139,7 +139,7 @@ static inline int ppc_inst_len(struct ppc_inst x)
> > * Return the address of the next instruction, if the instruction @value was
> > * located at @location.
> > */
> > -static inline struct ppc_inst *ppc_inst_next(void *location, struct ppc_inst *value)
> > +static inline unsigned int *ppc_inst_next(unsigned int *location, unsigned int *value)
> > {
> > struct ppc_inst tmp;
> >
>
> It's not visible in the diff, but the rest of the function is:
>
> tmp = ppc_inst_read(value);
>
> return location + ppc_inst_len(tmp);
> }
>
> And so changing the type of location from void * to int * changes the
> result of that addition, ie. previously it was in units of bytes, now
> it's units of 4 bytes.
>
> To fix it I've kept location as unsigned int *, and added a cast where
> we do the addition. That way users of the function just see unsigned int *,
> the cast to void * is an implementation detail.
>
> We only have a handful of uses of ppc_inst_len(), so maybe that should
> change name and return a result in units of int *. But that can be a
> separate change.
>
> > diff --git a/arch/powerpc/platforms/86xx/mpc86xx_smp.c b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
> > index 87f524e4b09c..302f2a1e0361 100644
> > --- a/arch/powerpc/platforms/86xx/mpc86xx_smp.c
> > +++ b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
> > @@ -83,7 +83,7 @@ smp_86xx_kick_cpu(int nr)
> > mdelay(1);
> >
> > /* Restore the exception vector */
> > - patch_instruction((struct ppc_inst *)vector, ppc_inst(save_vector));
> > + patch_instruction(vector, ppc_inst(save_vector));
> >
> > local_irq_restore(flags);
> >
>
> There was another usage in here:
>
> /* Setup fake reset vector to call __secondary_start_mpc86xx. */
> target = (unsigned long) __secondary_start_mpc86xx;
> - patch_branch((struct ppc_inst *)vector, target, BRANCH_SET_LINK);
> + patch_branch(vector, target, BRANCH_SET_LINK);
>
> /* Kick that CPU */
> smp_86xx_release_core(nr);
>
> I fixed it up.
>
> cheers
fwiw
Reviewed by: Jordan Niethe <jniethe5@gmail.com>
^ 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