2021-12-02 12:01:58

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 01/11] powerpc/code-patching: Remove pr_debug()/pr_devel() messages and fix check()

code-patching has been working for years now, time has come to
remove debugging messages.

Change useful message to KERN_INFO and remove other ones.

Also add KERN_ERR to check() macro and change it into a do/while
to make checkpatch happy.

Signed-off-by: Christophe Leroy <[email protected]>
---
This series applies on top of series "[v5,1/5] powerpc/inst: Refactor ___get_user_instr()" https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=274258

arch/powerpc/lib/code-patching.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 312324a26df3..4fe4464b7a84 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -95,7 +95,6 @@ static int map_patch_area(void *addr, unsigned long text_poke_addr)

err = map_kernel_page(text_poke_addr, (pfn << PAGE_SHIFT), PAGE_KERNEL);

- pr_devel("Mapped addr %lx with pfn %lx:%d\n", text_poke_addr, pfn, err);
if (err)
return -1;

@@ -130,8 +129,6 @@ static inline int unmap_patch_area(unsigned long addr)
if (unlikely(!ptep))
return -EINVAL;

- pr_devel("clearing mm %p, pte %p, addr %lx\n", &init_mm, ptep, addr);
-
/*
* In hash, pte_clear flushes the tlb, in radix, we have to
*/
@@ -190,10 +187,9 @@ static int do_patch_instruction(u32 *addr, ppc_inst_t instr)
int patch_instruction(u32 *addr, ppc_inst_t instr)
{
/* Make sure we aren't patching a freed init section */
- if (init_mem_is_free && init_section_contains(addr, 4)) {
- pr_debug("Skipping init section patching addr: 0x%px\n", addr);
+ if (init_mem_is_free && init_section_contains(addr, 4))
return 0;
- }
+
return do_patch_instruction(addr, instr);
}
NOKPROBE_SYMBOL(patch_instruction);
@@ -411,8 +407,10 @@ static void __init test_trampoline(void)
asm ("nop;\n");
}

-#define check(x) \
- if (!(x)) printk("code-patching: test failed at line %d\n", __LINE__);
+#define check(x) do { \
+ if (!(x)) \
+ pr_err("code-patching: test failed at line %d\n", __LINE__); \
+} while (0)

static void __init test_branch_iform(void)
{
@@ -737,7 +735,7 @@ static inline void test_prefixed_patching(void) {}

static int __init test_code_patching(void)
{
- printk(KERN_DEBUG "Running code patching self-tests ...\n");
+ pr_info("Running code patching self-tests ...\n");

test_branch_iform();
test_branch_bform();
--
2.33.1



2021-12-02 12:02:00

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 07/11] powerpc/code-patching: Use test_trampoline for prefixed patch test

Use the dedicated test_trampoline function for testing prefixed
patching like other tests and remove the hand coded assembly stuff.

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/lib/Makefile | 2 +-
arch/powerpc/lib/code-patching.c | 24 +++++++++---------------
arch/powerpc/lib/test_code-patching.S | 20 --------------------
3 files changed, 10 insertions(+), 36 deletions(-)
delete mode 100644 arch/powerpc/lib/test_code-patching.S

diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 9e5d0f413b71..c2654894b468 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -19,7 +19,7 @@ CFLAGS_code-patching.o += -DDISABLE_BRANCH_PROFILING
CFLAGS_feature-fixups.o += -DDISABLE_BRANCH_PROFILING
endif

-obj-y += alloc.o code-patching.o feature-fixups.o pmem.o test_code-patching.o
+obj-y += alloc.o code-patching.o feature-fixups.o pmem.o

ifndef CONFIG_KASAN
obj-y += string.o memcmp_$(BITS).o
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index e7a2a41ae8eb..2d878e67df3f 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -399,7 +399,7 @@ static int instr_is_branch_to_addr(const u32 *instr, unsigned long addr)

static void __init test_trampoline(void)
{
- asm ("nop;\n");
+ asm ("nop;nop;\n");
}

#define check(x) do { \
@@ -708,25 +708,19 @@ static void __init test_translate_branch(void)
vfree(buf);
}

-#ifdef CONFIG_PPC64
static void __init test_prefixed_patching(void)
{
- extern unsigned int code_patching_test1[];
- extern unsigned int code_patching_test1_expected[];
- extern unsigned int end_code_patching_test1[];
+ u32 *iptr = (u32 *)ppc_function_entry(test_trampoline);
+ u32 expected[2] = {OP_PREFIX << 26, 0};
+ ppc_inst_t inst = ppc_inst_prefix(OP_PREFIX << 26, 0);

- __patch_instruction(code_patching_test1,
- ppc_inst_prefix(OP_PREFIX << 26, 0x00000000),
- code_patching_test1);
+ if (!IS_ENABLED(CONFIG_PPC64))
+ return;
+
+ patch_instruction(iptr, inst);

- check(!memcmp(code_patching_test1,
- code_patching_test1_expected,
- sizeof(unsigned int) *
- (end_code_patching_test1 - code_patching_test1)));
+ check(!memcmp(iptr, expected, sizeof(expected)));
}
-#else
-static inline void test_prefixed_patching(void) {}
-#endif

static int __init test_code_patching(void)
{
diff --git a/arch/powerpc/lib/test_code-patching.S b/arch/powerpc/lib/test_code-patching.S
deleted file mode 100644
index a9be6107844e..000000000000
--- a/arch/powerpc/lib/test_code-patching.S
+++ /dev/null
@@ -1,20 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Copyright (C) 2020 IBM Corporation
- */
-#include <asm/ppc-opcode.h>
-
- .text
-
-#define globl(x) \
- .globl x; \
-x:
-
-globl(code_patching_test1)
- nop
- nop
-globl(end_code_patching_test1)
-
-globl(code_patching_test1_expected)
- .long OP_PREFIX << 26
- .long 0x0000000
--
2.33.1


2021-12-02 12:02:03

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 05/11] powerpc/code-patching: Reorganise do_patch_instruction() to ease error handling

Split do_patch_instruction() in two functions, the caller doing the
spin locking and the callee doing everything else.

And remove a few unnecessary initialisations and intermediate
variables.

This allows the callee to return from anywhere in the function.

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/lib/code-patching.c | 37 ++++++++++++++++++--------------
1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 5fa719a4ee69..a43ca22313ee 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -129,13 +129,30 @@ static void unmap_patch_area(unsigned long addr)
flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
}

+static int __do_patch_instruction(u32 *addr, ppc_inst_t instr)
+{
+ int err;
+ u32 *patch_addr;
+ unsigned long text_poke_addr;
+
+ text_poke_addr = (unsigned long)__this_cpu_read(text_poke_area)->addr;
+ patch_addr = (u32 *)(text_poke_addr + offset_in_page(addr));
+
+ err = map_patch_area(addr, text_poke_addr);
+ if (err)
+ return err;
+
+ err = __patch_instruction(addr, instr, patch_addr);
+
+ unmap_patch_area(text_poke_addr);
+
+ return err;
+}
+
static int do_patch_instruction(u32 *addr, ppc_inst_t instr)
{
int err;
- u32 *patch_addr = NULL;
unsigned long flags;
- unsigned long text_poke_addr;
- unsigned long kaddr = (unsigned long)addr;

/*
* During early early boot patch_instruction is called
@@ -146,19 +163,7 @@ static int do_patch_instruction(u32 *addr, ppc_inst_t instr)
return raw_patch_instruction(addr, instr);

local_irq_save(flags);
-
- text_poke_addr = (unsigned long)__this_cpu_read(text_poke_area)->addr;
- err = map_patch_area(addr, text_poke_addr);
- if (err)
- goto out;
-
- patch_addr = (u32 *)(text_poke_addr + (kaddr & ~PAGE_MASK));
-
- err = __patch_instruction(addr, instr, patch_addr);
-
- unmap_patch_area(text_poke_addr);
-
-out:
+ err = __do_patch_instruction(addr, instr);
local_irq_restore(flags);

return err;
--
2.33.1


2021-12-02 12:02:06

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 09/11] powerpc/code-patching: Move instr_is_branch_{i/b}form() in code-patching.h

To enable moving selftests in their own C file in following patch,
move instr_is_branch_iform() and instr_is_branch_bform()
to code-patching.h

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/include/asm/code-patching.h | 15 +++++++++++++++
arch/powerpc/lib/code-patching.c | 15 ---------------
2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
index 275061c3c977..e26080539c31 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -58,6 +58,21 @@ static inline int modify_instruction_site(s32 *site, unsigned int clr, unsigned
return modify_instruction((unsigned int *)patch_site_addr(site), clr, set);
}

+static inline unsigned int branch_opcode(ppc_inst_t instr)
+{
+ return ppc_inst_primary_opcode(instr) & 0x3F;
+}
+
+static inline int instr_is_branch_iform(ppc_inst_t instr)
+{
+ return branch_opcode(instr) == 18;
+}
+
+static inline int instr_is_branch_bform(ppc_inst_t instr)
+{
+ return branch_opcode(instr) == 16;
+}
+
int instr_is_relative_branch(ppc_inst_t instr);
int instr_is_relative_link_branch(ppc_inst_t instr);
unsigned long branch_target(const u32 *instr);
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 17e6443eb6c8..e07de5db06c0 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -285,21 +285,6 @@ int create_cond_branch(ppc_inst_t *instr, const u32 *addr,
return 0;
}

-static unsigned int branch_opcode(ppc_inst_t instr)
-{
- return ppc_inst_primary_opcode(instr) & 0x3F;
-}
-
-static int instr_is_branch_iform(ppc_inst_t instr)
-{
- return branch_opcode(instr) == 18;
-}
-
-static int instr_is_branch_bform(ppc_inst_t instr)
-{
- return branch_opcode(instr) == 16;
-}
-
int instr_is_relative_branch(ppc_inst_t instr)
{
if (ppc_inst_val(instr) & BRANCH_ABSOLUTE)
--
2.33.1


2021-12-02 12:02:13

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 10/11] powerpc/code-patching: Move code patching selftests in its own file

Code patching selftests are half of code-patching.c.
As they are guarded by CONFIG_CODE_PATCHING_SELFTESTS,
they'd be better in their own file.

Also add a missing __init for instr_is_branch_to_addr()

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/lib/Makefile | 2 +
arch/powerpc/lib/code-patching.c | 355 ------------------
.../{code-patching.c => test-code-patching.c} | 353 +----------------
3 files changed, 3 insertions(+), 707 deletions(-)
copy arch/powerpc/lib/{code-patching.c => test-code-patching.c} (56%)

diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index c2654894b468..3e183f4b4bda 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -21,6 +21,8 @@ endif

obj-y += alloc.o code-patching.o feature-fixups.o pmem.o

+obj-$(CONFIG_CODE_PATCHING_SELFTEST) += test-code-patching.o
+
ifndef CONFIG_KASAN
obj-y += string.o memcmp_$(BITS).o
obj-$(CONFIG_PPC32) += strlen_32.o
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index e07de5db06c0..906d43463366 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -3,13 +3,10 @@
* Copyright 2008 Michael Ellerman, IBM Corporation.
*/

-#include <linux/kernel.h>
#include <linux/kprobes.h>
#include <linux/vmalloc.h>
#include <linux/init.h>
-#include <linux/mm.h>
#include <linux/cpuhotplug.h>
-#include <linux/slab.h>
#include <linux/uaccess.h>

#include <asm/tlbflush.h>
@@ -354,355 +351,3 @@ int translate_branch(ppc_inst_t *instr, const u32 *dest, const u32 *src)

return 1;
}
-
-#ifdef CONFIG_CODE_PATCHING_SELFTEST
-
-static int instr_is_branch_to_addr(const u32 *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;nop;\n");
-}
-
-#define check(x) do { \
- if (!(x)) \
- pr_err("code-patching: test failed at line %d\n", __LINE__); \
-} while (0)
-
-static void __init test_branch_iform(void)
-{
- int err;
- ppc_inst_t instr;
- u32 tmp[2];
- u32 *iptr = tmp;
- unsigned long addr = (unsigned long)tmp;
-
- /* The simplest case, branch to self, no flags */
- check(instr_is_branch_iform(ppc_inst(0x48000000)));
- /* All bits of target set, and flags */
- check(instr_is_branch_iform(ppc_inst(0x4bffffff)));
- /* High bit of opcode set, which is wrong */
- check(!instr_is_branch_iform(ppc_inst(0xcbffffff)));
- /* Middle bits of opcode set, which is wrong */
- check(!instr_is_branch_iform(ppc_inst(0x7bffffff)));
-
- /* Simplest case, branch to self with link */
- check(instr_is_branch_iform(ppc_inst(0x48000001)));
- /* All bits of targets set */
- check(instr_is_branch_iform(ppc_inst(0x4bfffffd)));
- /* Some bits of targets set */
- check(instr_is_branch_iform(ppc_inst(0x4bff00fd)));
- /* Must be a valid branch to start with */
- check(!instr_is_branch_iform(ppc_inst(0x7bfffffd)));
-
- /* Absolute branch to 0x100 */
- patch_instruction(iptr, ppc_inst(0x48000103));
- check(instr_is_branch_to_addr(iptr, 0x100));
- /* Absolute branch to 0x420fc */
- patch_instruction(iptr, ppc_inst(0x480420ff));
- check(instr_is_branch_to_addr(iptr, 0x420fc));
- /* Maximum positive relative branch, + 20MB - 4B */
- patch_instruction(iptr, ppc_inst(0x49fffffc));
- check(instr_is_branch_to_addr(iptr, addr + 0x1FFFFFC));
- /* Smallest negative relative branch, - 4B */
- patch_instruction(iptr, ppc_inst(0x4bfffffc));
- check(instr_is_branch_to_addr(iptr, addr - 4));
- /* Largest negative relative branch, - 32 MB */
- patch_instruction(iptr, ppc_inst(0x4a000000));
- check(instr_is_branch_to_addr(iptr, addr - 0x2000000));
-
- /* Branch to self, with link */
- 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, 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, 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, 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, iptr, addr - 0x2000004, BRANCH_SET_LINK);
- check(err);
-
- /* Out of range relative positive offset, + 32 MB */
- err = create_branch(&instr, iptr, addr + 0x2000000, BRANCH_SET_LINK);
- check(err);
-
- /* Unaligned target */
- err = create_branch(&instr, iptr, addr + 3, BRANCH_SET_LINK);
- check(err);
-
- /* Check flags are masked correctly */
- 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)));
-}
-
-static void __init test_create_function_call(void)
-{
- u32 *iptr;
- unsigned long dest;
- ppc_inst_t instr;
-
- /* Check we can create a function call */
- iptr = (u32 *)ppc_function_entry(test_trampoline);
- dest = ppc_function_entry(test_create_function_call);
- create_branch(&instr, iptr, dest, BRANCH_SET_LINK);
- patch_instruction(iptr, instr);
- check(instr_is_branch_to_addr(iptr, dest));
-}
-
-static void __init test_branch_bform(void)
-{
- int err;
- unsigned long addr;
- ppc_inst_t instr;
- u32 tmp[2];
- u32 *iptr = tmp;
- unsigned int flags;
-
- addr = (unsigned long)iptr;
-
- /* The simplest case, branch to self, no flags */
- check(instr_is_branch_bform(ppc_inst(0x40000000)));
- /* All bits of target set, and flags */
- check(instr_is_branch_bform(ppc_inst(0x43ffffff)));
- /* High bit of opcode set, which is wrong */
- check(!instr_is_branch_bform(ppc_inst(0xc3ffffff)));
- /* Middle bits of opcode set, which is wrong */
- check(!instr_is_branch_bform(ppc_inst(0x7bffffff)));
-
- /* Absolute conditional branch to 0x100 */
- patch_instruction(iptr, ppc_inst(0x43ff0103));
- check(instr_is_branch_to_addr(iptr, 0x100));
- /* Absolute conditional branch to 0x20fc */
- patch_instruction(iptr, ppc_inst(0x43ff20ff));
- check(instr_is_branch_to_addr(iptr, 0x20fc));
- /* Maximum positive relative conditional branch, + 32 KB - 4B */
- patch_instruction(iptr, ppc_inst(0x43ff7ffc));
- check(instr_is_branch_to_addr(iptr, addr + 0x7FFC));
- /* Smallest negative relative conditional branch, - 4B */
- patch_instruction(iptr, ppc_inst(0x43fffffc));
- check(instr_is_branch_to_addr(iptr, addr - 4));
- /* Largest negative relative conditional branch, - 32 KB */
- 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);
- 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);
- 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);
- 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);
- 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);
- check(err);
-
- /* Out of range relative positive offset, + 32 KB */
- err = create_cond_branch(&instr, iptr, addr + 0x8000, flags);
- check(err);
-
- /* Unaligned target */
- err = create_cond_branch(&instr, iptr, addr + 3, flags);
- check(err);
-
- /* Check flags are masked correctly */
- err = create_cond_branch(&instr, iptr, addr, 0xFFFFFFFC);
- patch_instruction(iptr, instr);
- check(instr_is_branch_to_addr(iptr, addr));
- check(ppc_inst_equal(instr, ppc_inst(0x43FF0000)));
-}
-
-static void __init test_translate_branch(void)
-{
- unsigned long addr;
- void *p, *q;
- ppc_inst_t instr;
- void *buf;
-
- buf = vmalloc(PAGE_ALIGN(0x2000000 + 1));
- check(buf);
- if (!buf)
- return;
-
- /* Simple case, branch to self moved a little */
- p = buf;
- addr = (unsigned long)p;
- patch_branch(p, addr, 0);
- check(instr_is_branch_to_addr(p, addr));
- q = p + 4;
- translate_branch(&instr, q, p);
- patch_instruction(q, instr);
- check(instr_is_branch_to_addr(q, addr));
-
- /* Maximum negative case, move b . to addr + 32 MB */
- p = buf;
- addr = (unsigned long)p;
- patch_branch(p, addr, 0);
- q = buf + 0x2000000;
- translate_branch(&instr, q, p);
- patch_instruction(q, instr);
- check(instr_is_branch_to_addr(p, addr));
- check(instr_is_branch_to_addr(q, addr));
- check(ppc_inst_equal(ppc_inst_read(q), ppc_inst(0x4a000000)));
-
- /* Maximum positive case, move x to x - 32 MB + 4 */
- p = buf + 0x2000000;
- addr = (unsigned long)p;
- patch_branch(p, addr, 0);
- q = buf + 4;
- translate_branch(&instr, q, p);
- patch_instruction(q, instr);
- check(instr_is_branch_to_addr(p, addr));
- check(instr_is_branch_to_addr(q, addr));
- check(ppc_inst_equal(ppc_inst_read(q), ppc_inst(0x49fffffc)));
-
- /* Jump to x + 16 MB moved to x + 20 MB */
- p = buf;
- addr = 0x1000000 + (unsigned long)buf;
- patch_branch(p, addr, BRANCH_SET_LINK);
- q = buf + 0x1400000;
- translate_branch(&instr, q, p);
- patch_instruction(q, instr);
- check(instr_is_branch_to_addr(p, addr));
- check(instr_is_branch_to_addr(q, addr));
-
- /* Jump to x + 16 MB moved to x - 16 MB + 4 */
- p = buf + 0x1000000;
- addr = 0x2000000 + (unsigned long)buf;
- patch_branch(p, addr, 0);
- q = buf + 4;
- translate_branch(&instr, q, p);
- patch_instruction(q, instr);
- check(instr_is_branch_to_addr(p, addr));
- check(instr_is_branch_to_addr(q, addr));
-
-
- /* Conditional branch tests */
-
- /* Simple case, branch to self moved a little */
- p = buf;
- addr = (unsigned long)p;
- create_cond_branch(&instr, p, addr, 0);
- patch_instruction(p, instr);
- check(instr_is_branch_to_addr(p, addr));
- q = buf + 4;
- translate_branch(&instr, q, p);
- patch_instruction(q, instr);
- check(instr_is_branch_to_addr(q, addr));
-
- /* Maximum negative case, move b . to addr + 32 KB */
- p = buf;
- addr = (unsigned long)p;
- create_cond_branch(&instr, p, addr, 0xFFFFFFFC);
- patch_instruction(p, instr);
- q = buf + 0x8000;
- translate_branch(&instr, q, p);
- patch_instruction(q, instr);
- check(instr_is_branch_to_addr(p, addr));
- check(instr_is_branch_to_addr(q, addr));
- check(ppc_inst_equal(ppc_inst_read(q), ppc_inst(0x43ff8000)));
-
- /* Maximum positive case, move x to x - 32 KB + 4 */
- p = buf + 0x8000;
- addr = (unsigned long)p;
- create_cond_branch(&instr, p, addr, 0xFFFFFFFC);
- patch_instruction(p, instr);
- q = buf + 4;
- translate_branch(&instr, q, p);
- patch_instruction(q, instr);
- check(instr_is_branch_to_addr(p, addr));
- check(instr_is_branch_to_addr(q, addr));
- check(ppc_inst_equal(ppc_inst_read(q), ppc_inst(0x43ff7ffc)));
-
- /* Jump to x + 12 KB moved to x + 20 KB */
- p = buf;
- addr = 0x3000 + (unsigned long)buf;
- create_cond_branch(&instr, p, addr, BRANCH_SET_LINK);
- patch_instruction(p, instr);
- q = buf + 0x5000;
- translate_branch(&instr, q, p);
- patch_instruction(q, instr);
- check(instr_is_branch_to_addr(p, addr));
- check(instr_is_branch_to_addr(q, addr));
-
- /* Jump to x + 8 KB moved to x - 8 KB + 4 */
- p = buf + 0x2000;
- addr = 0x4000 + (unsigned long)buf;
- create_cond_branch(&instr, p, addr, 0);
- patch_instruction(p, instr);
- q = buf + 4;
- translate_branch(&instr, q, p);
- patch_instruction(q, instr);
- check(instr_is_branch_to_addr(p, addr));
- check(instr_is_branch_to_addr(q, addr));
-
- /* Free the buffer we were using */
- vfree(buf);
-}
-
-static void __init test_prefixed_patching(void)
-{
- u32 *iptr = (u32 *)ppc_function_entry(test_trampoline);
- u32 expected[2] = {OP_PREFIX << 26, 0};
- ppc_inst_t inst = ppc_inst_prefix(OP_PREFIX << 26, 0);
-
- if (!IS_ENABLED(CONFIG_PPC64))
- return;
-
- patch_instruction(iptr, inst);
-
- check(!memcmp(iptr, expected, sizeof(expected)));
-}
-
-static int __init test_code_patching(void)
-{
- pr_info("Running code patching self-tests ...\n");
-
- test_branch_iform();
- test_branch_bform();
- test_create_function_call();
- test_translate_branch();
- test_prefixed_patching();
-
- return 0;
-}
-late_initcall(test_code_patching);
-
-#endif /* CONFIG_CODE_PATCHING_SELFTEST */
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/test-code-patching.c
similarity index 56%
copy from arch/powerpc/lib/code-patching.c
copy to arch/powerpc/lib/test-code-patching.c
index e07de5db06c0..e358c9d8a03e 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/test-code-patching.c
@@ -3,361 +3,12 @@
* Copyright 2008 Michael Ellerman, IBM Corporation.
*/

-#include <linux/kernel.h>
-#include <linux/kprobes.h>
#include <linux/vmalloc.h>
#include <linux/init.h>
-#include <linux/mm.h>
-#include <linux/cpuhotplug.h>
-#include <linux/slab.h>
-#include <linux/uaccess.h>

-#include <asm/tlbflush.h>
-#include <asm/page.h>
#include <asm/code-patching.h>
-#include <asm/inst.h>

-static int __patch_instruction(u32 *exec_addr, ppc_inst_t instr, u32 *patch_addr)
-{
- if (!ppc_inst_prefixed(instr)) {
- u32 val = ppc_inst_val(instr);
-
- __put_kernel_nofault(patch_addr, &val, u32, failed);
- } else {
- u64 val = ppc_inst_as_ulong(instr);
-
- __put_kernel_nofault(patch_addr, &val, u64, failed);
- }
-
- asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr),
- "r" (exec_addr));
-
- return 0;
-
-failed:
- return -EFAULT;
-}
-
-int raw_patch_instruction(u32 *addr, ppc_inst_t instr)
-{
- return __patch_instruction(addr, instr, addr);
-}
-
-#ifdef CONFIG_STRICT_KERNEL_RWX
-static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
-
-static int text_area_cpu_up(unsigned int cpu)
-{
- struct vm_struct *area;
-
- area = get_vm_area(PAGE_SIZE, VM_ALLOC);
- if (!area) {
- WARN_ONCE(1, "Failed to create text area for cpu %d\n",
- cpu);
- return -1;
- }
- this_cpu_write(text_poke_area, area);
-
- return 0;
-}
-
-static int text_area_cpu_down(unsigned int cpu)
-{
- free_vm_area(this_cpu_read(text_poke_area));
- return 0;
-}
-
-/*
- * Although BUG_ON() is rude, in this case it should only happen if ENOMEM, and
- * we judge it as being preferable to a kernel that will crash later when
- * someone tries to use patch_instruction().
- */
-void __init poking_init(void)
-{
- BUG_ON(!cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
- "powerpc/text_poke:online", text_area_cpu_up,
- text_area_cpu_down));
-}
-
-/*
- * This can be called for kernel text or a module.
- */
-static int map_patch_area(void *addr, unsigned long text_poke_addr)
-{
- unsigned long pfn;
-
- if (is_vmalloc_or_module_addr(addr))
- pfn = vmalloc_to_pfn(addr);
- else
- pfn = __pa_symbol(addr) >> PAGE_SHIFT;
-
- return map_kernel_page(text_poke_addr, (pfn << PAGE_SHIFT), PAGE_KERNEL);
-}
-
-static void unmap_patch_area(unsigned long addr)
-{
- pte_t *ptep;
- pmd_t *pmdp;
- pud_t *pudp;
- p4d_t *p4dp;
- pgd_t *pgdp;
-
- pgdp = pgd_offset_k(addr);
- if (WARN_ON(pgd_none(*pgdp)))
- return;
-
- p4dp = p4d_offset(pgdp, addr);
- if (WARN_ON(p4d_none(*p4dp)))
- return;
-
- pudp = pud_offset(p4dp, addr);
- if (WARN_ON(pud_none(*pudp)))
- return;
-
- pmdp = pmd_offset(pudp, addr);
- if (WARN_ON(pmd_none(*pmdp)))
- return;
-
- ptep = pte_offset_kernel(pmdp, addr);
- if (WARN_ON(pte_none(*ptep)))
- return;
-
- /*
- * In hash, pte_clear flushes the tlb, in radix, we have to
- */
- pte_clear(&init_mm, addr, ptep);
- flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
-}
-
-static int __do_patch_instruction(u32 *addr, ppc_inst_t instr)
-{
- int err;
- u32 *patch_addr;
- unsigned long text_poke_addr;
-
- text_poke_addr = (unsigned long)__this_cpu_read(text_poke_area)->addr;
- patch_addr = (u32 *)(text_poke_addr + offset_in_page(addr));
-
- err = map_patch_area(addr, text_poke_addr);
- if (err)
- return err;
-
- err = __patch_instruction(addr, instr, patch_addr);
-
- unmap_patch_area(text_poke_addr);
-
- return err;
-}
-
-static int do_patch_instruction(u32 *addr, ppc_inst_t instr)
-{
- int err;
- unsigned long flags;
-
- /*
- * During early early boot patch_instruction is called
- * when text_poke_area is not ready, but we still need
- * to allow patching. We just do the plain old patching
- */
- if (!this_cpu_read(text_poke_area))
- return raw_patch_instruction(addr, instr);
-
- local_irq_save(flags);
- err = __do_patch_instruction(addr, instr);
- local_irq_restore(flags);
-
- return err;
-}
-#else /* !CONFIG_STRICT_KERNEL_RWX */
-
-static int do_patch_instruction(u32 *addr, ppc_inst_t instr)
-{
- return raw_patch_instruction(addr, instr);
-}
-
-#endif /* CONFIG_STRICT_KERNEL_RWX */
-
-int patch_instruction(u32 *addr, ppc_inst_t instr)
-{
- /* Make sure we aren't patching a freed init section */
- if (system_state >= SYSTEM_FREEING_INITMEM && init_section_contains(addr, 4))
- return 0;
-
- return do_patch_instruction(addr, instr);
-}
-NOKPROBE_SYMBOL(patch_instruction);
-
-int patch_branch(u32 *addr, unsigned long target, int flags)
-{
- ppc_inst_t instr;
-
- if (create_branch(&instr, addr, target, flags))
- return -ERANGE;
-
- return patch_instruction(addr, instr);
-}
-
-bool is_offset_in_branch_range(long offset)
-{
- /*
- * Powerpc branch instruction is :
- *
- * 0 6 30 31
- * +---------+----------------+---+---+
- * | opcode | LI |AA |LK |
- * +---------+----------------+---+---+
- * Where AA = 0 and LK = 0
- *
- * LI is a signed 24 bits integer. The real branch offset is computed
- * by: imm32 = SignExtend(LI:'0b00', 32);
- *
- * So the maximum forward branch should be:
- * (0x007fffff << 2) = 0x01fffffc = 0x1fffffc
- * The maximum backward branch should be:
- * (0xff800000 << 2) = 0xfe000000 = -0x2000000
- */
- return (offset >= -0x2000000 && offset <= 0x1fffffc && !(offset & 0x3));
-}
-
-bool is_offset_in_cond_branch_range(long offset)
-{
- return offset >= -0x8000 && offset <= 0x7fff && !(offset & 0x3);
-}
-
-/*
- * Helper to check if a given instruction is a conditional branch
- * Derived from the conditional checks in analyse_instr()
- */
-bool is_conditional_branch(ppc_inst_t instr)
-{
- unsigned int opcode = ppc_inst_primary_opcode(instr);
-
- if (opcode == 16) /* bc, bca, bcl, bcla */
- return true;
- if (opcode == 19) {
- switch ((ppc_inst_val(instr) >> 1) & 0x3ff) {
- case 16: /* bclr, bclrl */
- case 528: /* bcctr, bcctrl */
- case 560: /* bctar, bctarl */
- return true;
- }
- }
- return false;
-}
-NOKPROBE_SYMBOL(is_conditional_branch);
-
-int create_branch(ppc_inst_t *instr, const u32 *addr,
- unsigned long target, int flags)
-{
- long offset;
-
- *instr = ppc_inst(0);
- offset = target;
- if (! (flags & BRANCH_ABSOLUTE))
- offset = offset - (unsigned long)addr;
-
- /* Check we can represent the target in the instruction format */
- if (!is_offset_in_branch_range(offset))
- return 1;
-
- /* Mask out the flags and target, so they don't step on each other. */
- *instr = ppc_inst(0x48000000 | (flags & 0x3) | (offset & 0x03FFFFFC));
-
- return 0;
-}
-
-int create_cond_branch(ppc_inst_t *instr, const u32 *addr,
- unsigned long target, int flags)
-{
- long offset;
-
- offset = target;
- if (! (flags & BRANCH_ABSOLUTE))
- offset = offset - (unsigned long)addr;
-
- /* Check we can represent the target in the instruction format */
- if (!is_offset_in_cond_branch_range(offset))
- return 1;
-
- /* Mask out the flags and target, so they don't step on each other. */
- *instr = ppc_inst(0x40000000 | (flags & 0x3FF0003) | (offset & 0xFFFC));
-
- return 0;
-}
-
-int instr_is_relative_branch(ppc_inst_t instr)
-{
- if (ppc_inst_val(instr) & BRANCH_ABSOLUTE)
- return 0;
-
- return instr_is_branch_iform(instr) || instr_is_branch_bform(instr);
-}
-
-int instr_is_relative_link_branch(ppc_inst_t instr)
-{
- return instr_is_relative_branch(instr) && (ppc_inst_val(instr) & BRANCH_SET_LINK);
-}
-
-static unsigned long branch_iform_target(const u32 *instr)
-{
- signed long imm;
-
- 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(ppc_inst_read(instr)) & BRANCH_ABSOLUTE) == 0)
- imm += (unsigned long)instr;
-
- return (unsigned long)imm;
-}
-
-static unsigned long branch_bform_target(const u32 *instr)
-{
- signed long imm;
-
- 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(ppc_inst_read(instr)) & BRANCH_ABSOLUTE) == 0)
- imm += (unsigned long)instr;
-
- return (unsigned long)imm;
-}
-
-unsigned long branch_target(const u32 *instr)
-{
- if (instr_is_branch_iform(ppc_inst_read(instr)))
- return branch_iform_target(instr);
- else if (instr_is_branch_bform(ppc_inst_read(instr)))
- return branch_bform_target(instr);
-
- return 0;
-}
-
-int translate_branch(ppc_inst_t *instr, const u32 *dest, const u32 *src)
-{
- unsigned long target;
- target = branch_target(src);
-
- if (instr_is_branch_iform(ppc_inst_read(src)))
- return create_branch(instr, dest, target,
- ppc_inst_val(ppc_inst_read(src)));
- else if (instr_is_branch_bform(ppc_inst_read(src)))
- return create_cond_branch(instr, dest, target,
- ppc_inst_val(ppc_inst_read(src)));
-
- return 1;
-}
-
-#ifdef CONFIG_CODE_PATCHING_SELFTEST
-
-static int instr_is_branch_to_addr(const u32 *instr, unsigned long addr)
+static int __init instr_is_branch_to_addr(const u32 *instr, unsigned long addr)
{
if (instr_is_branch_iform(ppc_inst_read(instr)) ||
instr_is_branch_bform(ppc_inst_read(instr)))
@@ -704,5 +355,3 @@ static int __init test_code_patching(void)
return 0;
}
late_initcall(test_code_patching);
-
-#endif /* CONFIG_CODE_PATCHING_SELFTEST */
--
2.33.1


2021-12-02 12:02:22

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 08/11] powerpc/code-patching: Move patch_exception() outside code-patching.c

patch_exception() is dedicated to book3e/64 is nothing more than
a normal use of patch_branch(), so move it into a place dedicated
to book3e/64.

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/include/asm/code-patching.h | 7 -------
arch/powerpc/include/asm/exception-64e.h | 4 ++++
arch/powerpc/include/asm/nohash/64/pgtable.h | 6 ++++++
arch/powerpc/lib/code-patching.c | 16 ----------------
arch/powerpc/mm/nohash/book3e_pgtable.c | 14 ++++++++++++++
5 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
index 46e8c5a8ce51..275061c3c977 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -63,13 +63,6 @@ int instr_is_relative_link_branch(ppc_inst_t instr);
unsigned long branch_target(const u32 *instr);
int translate_branch(ppc_inst_t *instr, const u32 *dest, const u32 *src);
bool is_conditional_branch(ppc_inst_t instr);
-#ifdef CONFIG_PPC_BOOK3E_64
-void __patch_exception(int exc, unsigned long addr);
-#define patch_exception(exc, name) do { \
- extern unsigned int name; \
- __patch_exception((exc), (unsigned long)&name); \
-} while (0)
-#endif

#define OP_RT_RA_MASK 0xffff0000UL
#define LIS_R2 (PPC_RAW_LIS(_R2, 0))
diff --git a/arch/powerpc/include/asm/exception-64e.h b/arch/powerpc/include/asm/exception-64e.h
index 40cdcb2fb057..b1ef1e92c34a 100644
--- a/arch/powerpc/include/asm/exception-64e.h
+++ b/arch/powerpc/include/asm/exception-64e.h
@@ -149,6 +149,10 @@ exc_##label##_book3e:
addi r11,r13,PACA_EXTLB; \
TLB_MISS_RESTORE(r11)

+#ifndef __ASSEMBLY__
+extern unsigned int interrupt_base_book3e;
+#endif
+
#define SET_IVOR(vector_number, vector_offset) \
LOAD_REG_ADDR(r3,interrupt_base_book3e);\
ori r3,r3,vector_offset@l; \
diff --git a/arch/powerpc/include/asm/nohash/64/pgtable.h b/arch/powerpc/include/asm/nohash/64/pgtable.h
index 9d2905a47410..a3313e853e5e 100644
--- a/arch/powerpc/include/asm/nohash/64/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/64/pgtable.h
@@ -313,6 +313,12 @@ extern int __meminit vmemmap_create_mapping(unsigned long start,
unsigned long phys);
extern void vmemmap_remove_mapping(unsigned long start,
unsigned long page_size);
+void __patch_exception(int exc, unsigned long addr);
+#define patch_exception(exc, name) do { \
+ extern unsigned int name; \
+ __patch_exception((exc), (unsigned long)&name); \
+} while (0)
+
#endif /* __ASSEMBLY__ */

#endif /* _ASM_POWERPC_NOHASH_64_PGTABLE_H */
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 2d878e67df3f..17e6443eb6c8 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -370,22 +370,6 @@ int translate_branch(ppc_inst_t *instr, const u32 *dest, const u32 *src)
return 1;
}

-#ifdef CONFIG_PPC_BOOK3E_64
-void __patch_exception(int exc, unsigned long addr)
-{
- extern unsigned int interrupt_base_book3e;
- unsigned int *ibase = &interrupt_base_book3e;
-
- /* Our exceptions vectors start with a NOP and -then- a branch
- * to deal with single stepping from userspace which stops on
- * the second instruction. Thus we need to patch the second
- * instruction of the exception, not the first one
- */
-
- patch_branch(ibase + (exc / 4) + 1, addr, 0);
-}
-#endif
-
#ifdef CONFIG_CODE_PATCHING_SELFTEST

static int instr_is_branch_to_addr(const u32 *instr, unsigned long addr)
diff --git a/arch/powerpc/mm/nohash/book3e_pgtable.c b/arch/powerpc/mm/nohash/book3e_pgtable.c
index 77884e24281d..7b6db97c2bdc 100644
--- a/arch/powerpc/mm/nohash/book3e_pgtable.c
+++ b/arch/powerpc/mm/nohash/book3e_pgtable.c
@@ -10,6 +10,7 @@
#include <asm/pgalloc.h>
#include <asm/tlb.h>
#include <asm/dma.h>
+#include <asm/code-patching.h>

#include <mm/mmu_decl.h>

@@ -115,3 +116,16 @@ int __ref map_kernel_page(unsigned long ea, unsigned long pa, pgprot_t prot)
smp_wmb();
return 0;
}
+
+void __patch_exception(int exc, unsigned long addr)
+{
+ unsigned int *ibase = &interrupt_base_book3e;
+
+ /* Our exceptions vectors start with a NOP and -then- a branch
+ * to deal with single stepping from userspace which stops on
+ * the second instruction. Thus we need to patch the second
+ * instruction of the exception, not the first one
+ */
+
+ patch_branch(ibase + (exc / 4) + 1, addr, 0);
+}
--
2.33.1


2021-12-26 21:54:10

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v1 01/11] powerpc/code-patching: Remove pr_debug()/pr_devel() messages and fix check()

On Thu, 2 Dec 2021 13:00:17 +0100, Christophe Leroy wrote:
> code-patching has been working for years now, time has come to
> remove debugging messages.
>
> Change useful message to KERN_INFO and remove other ones.
>
> Also add KERN_ERR to check() macro and change it into a do/while
> to make checkpatch happy.
>
> [...]

Applied to powerpc/next.

[01/11] powerpc/code-patching: Remove pr_debug()/pr_devel() messages and fix check()
https://git.kernel.org/powerpc/c/edecd2d6d6f4a122dd62bce654b4f63301e8ad9a
[02/11] powerpc/code-patching: Remove init_mem_is_free
https://git.kernel.org/powerpc/c/af5304a7506588221d8317ef3f76585eb4483506
[03/11] powerpc/code-patching: Fix error handling in do_patch_instruction()
https://git.kernel.org/powerpc/c/285672f99327d5b8febdf83cadba61a68abe5d69
[04/11] powerpc/code-patching: Fix unmap_patch_area() error handling
https://git.kernel.org/powerpc/c/a3483c3dd18c136785a31406fe27210649fc4fba
[05/11] powerpc/code-patching: Reorganise do_patch_instruction() to ease error handling
https://git.kernel.org/powerpc/c/6b21af74495b556f9d496d97d74e7a3d0ab16d7c
[06/11] powerpc/code-patching: Fix patch_branch() return on out-of-range failure
https://git.kernel.org/powerpc/c/d5937db114e4b6446c62809484729955f1aeb108
[07/11] powerpc/code-patching: Use test_trampoline for prefixed patch test
https://git.kernel.org/powerpc/c/ff14a9c09fe91a70bfc6381809877e5a19e38cdb
[08/11] powerpc/code-patching: Move patch_exception() outside code-patching.c
https://git.kernel.org/powerpc/c/29562a9da29478834e57f81e3804e9ec7a6b350b
[09/11] powerpc/code-patching: Move instr_is_branch_{i/b}form() in code-patching.h
https://git.kernel.org/powerpc/c/31acc599564120fa41f9df2c567842d003728dab
[10/11] powerpc/code-patching: Move code patching selftests in its own file
https://git.kernel.org/powerpc/c/f30a578d7653f7dbb253a20daad4bcd9f881d6c9
[11/11] powerpc/code-patching: Replace patch_instruction() by ppc_inst_write() in selftests
https://git.kernel.org/powerpc/c/309a0a601864831510209531dd72da486225d8ae

cheers