[PATCH v1 01/11] powerpc/code-patching: Remove pr_debug()/pr

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v1 01/11] powerpc/code-patching: Remove pr_debug()/pr_devel() messages and fix check()
@ 2021-12-02 12:00 ` Christophe Leroy
  0 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Christophe Leroy, linux-kernel, linuxppc-dev

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 <christophe.leroy@csgroup.eu>
---
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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 01/11] powerpc/code-patching: Remove pr_debug()/pr_devel() messages and fix check()
@ 2021-12-02 12:00 ` Christophe Leroy
  0 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

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 <christophe.leroy@csgroup.eu>
---
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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 02/11] powerpc/code-patching: Remove init_mem_is_free
  2021-12-02 12:00 ` Christophe Leroy
@ 2021-12-02 12:00   ` Christophe Leroy
  -1 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Christophe Leroy, linux-kernel, linuxppc-dev

A new state has been added by commit d2635f2012a4 ("mm: create a new
system state and fix core_kernel_text()"). That state tells when
initmem is about to be released and is redundant with init_mem_is_free.

Remove init_mem_is_free.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/setup.h | 1 -
 arch/powerpc/lib/code-patching.c | 3 +--
 arch/powerpc/mm/mem.c            | 2 --
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/setup.h b/arch/powerpc/include/asm/setup.h
index 6c1a7d217d1a..426a2d8d028f 100644
--- a/arch/powerpc/include/asm/setup.h
+++ b/arch/powerpc/include/asm/setup.h
@@ -9,7 +9,6 @@ extern void ppc_printk_progress(char *s, unsigned short hex);
 
 extern unsigned int rtas_data;
 extern unsigned long long memory_limit;
-extern bool init_mem_is_free;
 extern void *zalloc_maybe_bootmem(size_t size, gfp_t mask);
 
 struct device_node;
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 4fe4464b7a84..7bb8dd2dc19e 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -15,7 +15,6 @@
 #include <asm/tlbflush.h>
 #include <asm/page.h>
 #include <asm/code-patching.h>
-#include <asm/setup.h>
 #include <asm/inst.h>
 
 static int __patch_instruction(u32 *exec_addr, ppc_inst_t instr, u32 *patch_addr)
@@ -187,7 +186,7 @@ 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))
+	if (system_state >= SYSTEM_FREEING_INITMEM && init_section_contains(addr, 4))
 		return 0;
 
 	return do_patch_instruction(addr, instr);
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index bd5d91a31183..8e301cd8925b 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -26,7 +26,6 @@
 #include <mm/mmu_decl.h>
 
 unsigned long long memory_limit;
-bool init_mem_is_free;
 
 unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)] __page_aligned_bss;
 EXPORT_SYMBOL(empty_zero_page);
@@ -312,7 +311,6 @@ void free_initmem(void)
 {
 	ppc_md.progress = ppc_printk_progress;
 	mark_initmem_nx();
-	init_mem_is_free = true;
 	free_initmem_default(POISON_FREE_INITMEM);
 }
 
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 02/11] powerpc/code-patching: Remove init_mem_is_free
@ 2021-12-02 12:00   ` Christophe Leroy
  0 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

A new state has been added by commit d2635f2012a4 ("mm: create a new
system state and fix core_kernel_text()"). That state tells when
initmem is about to be released and is redundant with init_mem_is_free.

Remove init_mem_is_free.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/setup.h | 1 -
 arch/powerpc/lib/code-patching.c | 3 +--
 arch/powerpc/mm/mem.c            | 2 --
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/setup.h b/arch/powerpc/include/asm/setup.h
index 6c1a7d217d1a..426a2d8d028f 100644
--- a/arch/powerpc/include/asm/setup.h
+++ b/arch/powerpc/include/asm/setup.h
@@ -9,7 +9,6 @@ extern void ppc_printk_progress(char *s, unsigned short hex);
 
 extern unsigned int rtas_data;
 extern unsigned long long memory_limit;
-extern bool init_mem_is_free;
 extern void *zalloc_maybe_bootmem(size_t size, gfp_t mask);
 
 struct device_node;
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 4fe4464b7a84..7bb8dd2dc19e 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -15,7 +15,6 @@
 #include <asm/tlbflush.h>
 #include <asm/page.h>
 #include <asm/code-patching.h>
-#include <asm/setup.h>
 #include <asm/inst.h>
 
 static int __patch_instruction(u32 *exec_addr, ppc_inst_t instr, u32 *patch_addr)
@@ -187,7 +186,7 @@ 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))
+	if (system_state >= SYSTEM_FREEING_INITMEM && init_section_contains(addr, 4))
 		return 0;
 
 	return do_patch_instruction(addr, instr);
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index bd5d91a31183..8e301cd8925b 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -26,7 +26,6 @@
 #include <mm/mmu_decl.h>
 
 unsigned long long memory_limit;
-bool init_mem_is_free;
 
 unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)] __page_aligned_bss;
 EXPORT_SYMBOL(empty_zero_page);
@@ -312,7 +311,6 @@ void free_initmem(void)
 {
 	ppc_md.progress = ppc_printk_progress;
 	mark_initmem_nx();
-	init_mem_is_free = true;
 	free_initmem_default(POISON_FREE_INITMEM);
 }
 
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 03/11] powerpc/code-patching: Fix error handling in do_patch_instruction()
  2021-12-02 12:00 ` Christophe Leroy
@ 2021-12-02 12:00   ` Christophe Leroy
  -1 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Christophe Leroy, linux-kernel, linuxppc-dev

Use real errors instead of using -1 as error, so that errors
returned by callees can be used towards callers.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/lib/code-patching.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 7bb8dd2dc19e..4ce2b6457757 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -85,19 +85,13 @@ void __init poking_init(void)
 static int map_patch_area(void *addr, unsigned long text_poke_addr)
 {
 	unsigned long pfn;
-	int err;
 
 	if (is_vmalloc_or_module_addr(addr))
 		pfn = vmalloc_to_pfn(addr);
 	else
 		pfn = __pa_symbol(addr) >> PAGE_SHIFT;
 
-	err = map_kernel_page(text_poke_addr, (pfn << PAGE_SHIFT), PAGE_KERNEL);
-
-	if (err)
-		return -1;
-
-	return 0;
+	return map_kernel_page(text_poke_addr, (pfn << PAGE_SHIFT), PAGE_KERNEL);
 }
 
 static inline int unmap_patch_area(unsigned long addr)
@@ -156,10 +150,9 @@ static int do_patch_instruction(u32 *addr, ppc_inst_t instr)
 	local_irq_save(flags);
 
 	text_poke_addr = (unsigned long)__this_cpu_read(text_poke_area)->addr;
-	if (map_patch_area(addr, text_poke_addr)) {
-		err = -1;
+	err = map_patch_area(addr, text_poke_addr);
+	if (err)
 		goto out;
-	}
 
 	patch_addr = (u32 *)(text_poke_addr + (kaddr & ~PAGE_MASK));
 
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 03/11] powerpc/code-patching: Fix error handling in do_patch_instruction()
@ 2021-12-02 12:00   ` Christophe Leroy
  0 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

Use real errors instead of using -1 as error, so that errors
returned by callees can be used towards callers.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/lib/code-patching.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 7bb8dd2dc19e..4ce2b6457757 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -85,19 +85,13 @@ void __init poking_init(void)
 static int map_patch_area(void *addr, unsigned long text_poke_addr)
 {
 	unsigned long pfn;
-	int err;
 
 	if (is_vmalloc_or_module_addr(addr))
 		pfn = vmalloc_to_pfn(addr);
 	else
 		pfn = __pa_symbol(addr) >> PAGE_SHIFT;
 
-	err = map_kernel_page(text_poke_addr, (pfn << PAGE_SHIFT), PAGE_KERNEL);
-
-	if (err)
-		return -1;
-
-	return 0;
+	return map_kernel_page(text_poke_addr, (pfn << PAGE_SHIFT), PAGE_KERNEL);
 }
 
 static inline int unmap_patch_area(unsigned long addr)
@@ -156,10 +150,9 @@ static int do_patch_instruction(u32 *addr, ppc_inst_t instr)
 	local_irq_save(flags);
 
 	text_poke_addr = (unsigned long)__this_cpu_read(text_poke_area)->addr;
-	if (map_patch_area(addr, text_poke_addr)) {
-		err = -1;
+	err = map_patch_area(addr, text_poke_addr);
+	if (err)
 		goto out;
-	}
 
 	patch_addr = (u32 *)(text_poke_addr + (kaddr & ~PAGE_MASK));
 
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 04/11] powerpc/code-patching: Fix unmap_patch_area() error handling
  2021-12-02 12:00 ` Christophe Leroy
@ 2021-12-02 12:00   ` Christophe Leroy
  -1 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Christophe Leroy, linux-kernel, linuxppc-dev

pXd_offset() doesn't return NULL. When the base is NULL, it
still adds the offset.

Use pXd_none() to check validity instead. It also improves
performance by folding out none existing levels as pXd_none()
always returns 0 in that case.

Such an error is unexpected, use WARN_ON() so that the caller
doesn't have to worry about it, and drop the returned value.

And now that unmap_patch_area() doesn't return error, we can
take into account the error returned by __patch_instruction().

While at it, remove the 'inline' property which is useless.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/lib/code-patching.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 4ce2b6457757..5fa719a4ee69 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -94,7 +94,7 @@ static int map_patch_area(void *addr, unsigned long text_poke_addr)
 	return map_kernel_page(text_poke_addr, (pfn << PAGE_SHIFT), PAGE_KERNEL);
 }
 
-static inline int unmap_patch_area(unsigned long addr)
+static void unmap_patch_area(unsigned long addr)
 {
 	pte_t *ptep;
 	pmd_t *pmdp;
@@ -103,32 +103,30 @@ static inline int unmap_patch_area(unsigned long addr)
 	pgd_t *pgdp;
 
 	pgdp = pgd_offset_k(addr);
-	if (unlikely(!pgdp))
-		return -EINVAL;
+	if (WARN_ON(pgd_none(*pgdp)))
+		return;
 
 	p4dp = p4d_offset(pgdp, addr);
-	if (unlikely(!p4dp))
-		return -EINVAL;
+	if (WARN_ON(p4d_none(*p4dp)))
+		return;
 
 	pudp = pud_offset(p4dp, addr);
-	if (unlikely(!pudp))
-		return -EINVAL;
+	if (WARN_ON(pud_none(*pudp)))
+		return;
 
 	pmdp = pmd_offset(pudp, addr);
-	if (unlikely(!pmdp))
-		return -EINVAL;
+	if (WARN_ON(pmd_none(*pmdp)))
+		return;
 
 	ptep = pte_offset_kernel(pmdp, addr);
-	if (unlikely(!ptep))
-		return -EINVAL;
+	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);
-
-	return 0;
 }
 
 static int do_patch_instruction(u32 *addr, ppc_inst_t instr)
@@ -156,11 +154,9 @@ static int do_patch_instruction(u32 *addr, ppc_inst_t instr)
 
 	patch_addr = (u32 *)(text_poke_addr + (kaddr & ~PAGE_MASK));
 
-	__patch_instruction(addr, instr, patch_addr);
+	err = __patch_instruction(addr, instr, patch_addr);
 
-	err = unmap_patch_area(text_poke_addr);
-	if (err)
-		pr_warn("failed to unmap %lx\n", text_poke_addr);
+	unmap_patch_area(text_poke_addr);
 
 out:
 	local_irq_restore(flags);
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 04/11] powerpc/code-patching: Fix unmap_patch_area() error handling
@ 2021-12-02 12:00   ` Christophe Leroy
  0 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

pXd_offset() doesn't return NULL. When the base is NULL, it
still adds the offset.

Use pXd_none() to check validity instead. It also improves
performance by folding out none existing levels as pXd_none()
always returns 0 in that case.

Such an error is unexpected, use WARN_ON() so that the caller
doesn't have to worry about it, and drop the returned value.

And now that unmap_patch_area() doesn't return error, we can
take into account the error returned by __patch_instruction().

While at it, remove the 'inline' property which is useless.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/lib/code-patching.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 4ce2b6457757..5fa719a4ee69 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -94,7 +94,7 @@ static int map_patch_area(void *addr, unsigned long text_poke_addr)
 	return map_kernel_page(text_poke_addr, (pfn << PAGE_SHIFT), PAGE_KERNEL);
 }
 
-static inline int unmap_patch_area(unsigned long addr)
+static void unmap_patch_area(unsigned long addr)
 {
 	pte_t *ptep;
 	pmd_t *pmdp;
@@ -103,32 +103,30 @@ static inline int unmap_patch_area(unsigned long addr)
 	pgd_t *pgdp;
 
 	pgdp = pgd_offset_k(addr);
-	if (unlikely(!pgdp))
-		return -EINVAL;
+	if (WARN_ON(pgd_none(*pgdp)))
+		return;
 
 	p4dp = p4d_offset(pgdp, addr);
-	if (unlikely(!p4dp))
-		return -EINVAL;
+	if (WARN_ON(p4d_none(*p4dp)))
+		return;
 
 	pudp = pud_offset(p4dp, addr);
-	if (unlikely(!pudp))
-		return -EINVAL;
+	if (WARN_ON(pud_none(*pudp)))
+		return;
 
 	pmdp = pmd_offset(pudp, addr);
-	if (unlikely(!pmdp))
-		return -EINVAL;
+	if (WARN_ON(pmd_none(*pmdp)))
+		return;
 
 	ptep = pte_offset_kernel(pmdp, addr);
-	if (unlikely(!ptep))
-		return -EINVAL;
+	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);
-
-	return 0;
 }
 
 static int do_patch_instruction(u32 *addr, ppc_inst_t instr)
@@ -156,11 +154,9 @@ static int do_patch_instruction(u32 *addr, ppc_inst_t instr)
 
 	patch_addr = (u32 *)(text_poke_addr + (kaddr & ~PAGE_MASK));
 
-	__patch_instruction(addr, instr, patch_addr);
+	err = __patch_instruction(addr, instr, patch_addr);
 
-	err = unmap_patch_area(text_poke_addr);
-	if (err)
-		pr_warn("failed to unmap %lx\n", text_poke_addr);
+	unmap_patch_area(text_poke_addr);
 
 out:
 	local_irq_restore(flags);
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 05/11] powerpc/code-patching: Reorganise do_patch_instruction() to ease error handling
  2021-12-02 12:00 ` Christophe Leroy
@ 2021-12-02 12:00   ` Christophe Leroy
  -1 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Christophe Leroy, linux-kernel, linuxppc-dev

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 <christophe.leroy@csgroup.eu>
---
 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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 05/11] powerpc/code-patching: Reorganise do_patch_instruction() to ease error handling
@ 2021-12-02 12:00   ` Christophe Leroy
  0 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

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 <christophe.leroy@csgroup.eu>
---
 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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 06/11] powerpc/code-patching: Fix patch_branch() return on out-of-range failure
  2021-12-02 12:00 ` Christophe Leroy
@ 2021-12-02 12:00   ` Christophe Leroy
  -1 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Christophe Leroy, linux-kernel, linuxppc-dev

Do not silentely ignore a failure of create_branch() in
patch_branch(). Return -ERANGE.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/lib/code-patching.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index a43ca22313ee..e7a2a41ae8eb 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -191,7 +191,9 @@ int patch_branch(u32 *addr, unsigned long target, int flags)
 {
 	ppc_inst_t instr;
 
-	create_branch(&instr, addr, target, flags);
+	if (create_branch(&instr, addr, target, flags))
+		return -ERANGE;
+
 	return patch_instruction(addr, instr);
 }
 
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 06/11] powerpc/code-patching: Fix patch_branch() return on out-of-range failure
@ 2021-12-02 12:00   ` Christophe Leroy
  0 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

Do not silentely ignore a failure of create_branch() in
patch_branch(). Return -ERANGE.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/lib/code-patching.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index a43ca22313ee..e7a2a41ae8eb 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -191,7 +191,9 @@ int patch_branch(u32 *addr, unsigned long target, int flags)
 {
 	ppc_inst_t instr;
 
-	create_branch(&instr, addr, target, flags);
+	if (create_branch(&instr, addr, target, flags))
+		return -ERANGE;
+
 	return patch_instruction(addr, instr);
 }
 
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 07/11] powerpc/code-patching: Use test_trampoline for prefixed patch test
  2021-12-02 12:00 ` Christophe Leroy
@ 2021-12-02 12:00   ` Christophe Leroy
  -1 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Christophe Leroy, linux-kernel, linuxppc-dev

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 <christophe.leroy@csgroup.eu>
---
 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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 07/11] powerpc/code-patching: Use test_trampoline for prefixed patch test
@ 2021-12-02 12:00   ` Christophe Leroy
  0 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

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 <christophe.leroy@csgroup.eu>
---
 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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 08/11] powerpc/code-patching: Move patch_exception() outside code-patching.c
  2021-12-02 12:00 ` Christophe Leroy
@ 2021-12-02 12:00   ` Christophe Leroy
  -1 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Christophe Leroy, linux-kernel, linuxppc-dev

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 <christophe.leroy@csgroup.eu>
---
 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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 08/11] powerpc/code-patching: Move patch_exception() outside code-patching.c
@ 2021-12-02 12:00   ` Christophe Leroy
  0 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

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 <christophe.leroy@csgroup.eu>
---
 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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 09/11] powerpc/code-patching: Move instr_is_branch_{i/b}form() in code-patching.h
  2021-12-02 12:00 ` Christophe Leroy
@ 2021-12-02 12:00   ` Christophe Leroy
  -1 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Christophe Leroy, linux-kernel, linuxppc-dev

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 <christophe.leroy@csgroup.eu>
---
 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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 09/11] powerpc/code-patching: Move instr_is_branch_{i/b}form() in code-patching.h
@ 2021-12-02 12:00   ` Christophe Leroy
  0 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

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 <christophe.leroy@csgroup.eu>
---
 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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 10/11] powerpc/code-patching: Move code patching selftests in its own file
  2021-12-02 12:00 ` Christophe Leroy
@ 2021-12-02 12:00   ` Christophe Leroy
  -1 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Christophe Leroy, linux-kernel, linuxppc-dev

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 <christophe.leroy@csgroup.eu>
---
 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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 10/11] powerpc/code-patching: Move code patching selftests in its own file
@ 2021-12-02 12:00   ` Christophe Leroy
  0 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

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 <christophe.leroy@csgroup.eu>
---
 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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 11/11] powerpc/code-patching: Replace patch_instruction() by ppc_inst_write() in selftests
  2021-12-02 12:00 ` Christophe Leroy
@ 2021-12-02 12:00   ` Christophe Leroy
  -1 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Christophe Leroy, linux-kernel, linuxppc-dev

The purpose of selftests is to check that instructions are
properly formed. Not to check that they properly run.

For that test it uses normal memory, not special test
memory.

In preparation of a future patch enforcing patch_instruction()
to be used only on valid text areas, implement a ppc_inst_write()
instruction which is the complement of ppc_inst_read(). This
new function writes the formated instruction in valid kernel
memory and doesn't bother about icache.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/inst.h       |  8 +++
 arch/powerpc/lib/test-code-patching.c | 85 ++++++++++++++-------------
 2 files changed, 53 insertions(+), 40 deletions(-)

diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index 631436f3f5c3..21fe8594e078 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -131,6 +131,14 @@ static inline unsigned long ppc_inst_as_ulong(ppc_inst_t x)
 		return (u64)ppc_inst_val(x) << 32 | ppc_inst_suffix(x);
 }
 
+static inline void ppc_inst_write(u32 *ptr, ppc_inst_t x)
+{
+	if (!ppc_inst_prefixed(x))
+		*ptr = ppc_inst_val(x);
+	else
+		*(u64 *)ptr = ppc_inst_as_ulong(x);
+}
+
 #define PPC_INST_STR_LEN sizeof("00000000 00000000")
 
 static inline char *__ppc_inst_as_str(char str[PPC_INST_STR_LEN], ppc_inst_t x)
diff --git a/arch/powerpc/lib/test-code-patching.c b/arch/powerpc/lib/test-code-patching.c
index e358c9d8a03e..c44823292f73 100644
--- a/arch/powerpc/lib/test-code-patching.c
+++ b/arch/powerpc/lib/test-code-patching.c
@@ -54,39 +54,39 @@ static void __init test_branch_iform(void)
 	check(!instr_is_branch_iform(ppc_inst(0x7bfffffd)));
 
 	/* Absolute branch to 0x100 */
-	patch_instruction(iptr, ppc_inst(0x48000103));
+	ppc_inst_write(iptr, ppc_inst(0x48000103));
 	check(instr_is_branch_to_addr(iptr, 0x100));
 	/* Absolute branch to 0x420fc */
-	patch_instruction(iptr, ppc_inst(0x480420ff));
+	ppc_inst_write(iptr, ppc_inst(0x480420ff));
 	check(instr_is_branch_to_addr(iptr, 0x420fc));
 	/* Maximum positive relative branch, + 20MB - 4B */
-	patch_instruction(iptr, ppc_inst(0x49fffffc));
+	ppc_inst_write(iptr, ppc_inst(0x49fffffc));
 	check(instr_is_branch_to_addr(iptr, addr + 0x1FFFFFC));
 	/* Smallest negative relative branch, - 4B */
-	patch_instruction(iptr, ppc_inst(0x4bfffffc));
+	ppc_inst_write(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));
+	ppc_inst_write(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);
+	ppc_inst_write(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);
+	ppc_inst_write(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);
+	ppc_inst_write(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);
+	ppc_inst_write(iptr, instr);
 	check(instr_is_branch_to_addr(iptr, addr - 0x2000000));
 
 	/* Out of range relative negative offset, - 32 MB + 4*/
@@ -103,7 +103,7 @@ static void __init test_branch_iform(void)
 
 	/* Check flags are masked correctly */
 	err = create_branch(&instr, iptr, addr, 0xFFFFFFFC);
-	patch_instruction(iptr, instr);
+	ppc_inst_write(iptr, instr);
 	check(instr_is_branch_to_addr(iptr, addr));
 	check(ppc_inst_equal(instr, ppc_inst(0x48000000)));
 }
@@ -143,19 +143,19 @@ static void __init test_branch_bform(void)
 	check(!instr_is_branch_bform(ppc_inst(0x7bffffff)));
 
 	/* Absolute conditional branch to 0x100 */
-	patch_instruction(iptr, ppc_inst(0x43ff0103));
+	ppc_inst_write(iptr, ppc_inst(0x43ff0103));
 	check(instr_is_branch_to_addr(iptr, 0x100));
 	/* Absolute conditional branch to 0x20fc */
-	patch_instruction(iptr, ppc_inst(0x43ff20ff));
+	ppc_inst_write(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));
+	ppc_inst_write(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));
+	ppc_inst_write(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));
+	ppc_inst_write(iptr, ppc_inst(0x43ff8000));
 	check(instr_is_branch_to_addr(iptr, addr - 0x8000));
 
 	/* All condition code bits set & link */
@@ -163,22 +163,22 @@ static void __init test_branch_bform(void)
 
 	/* Branch to self */
 	err = create_cond_branch(&instr, iptr, addr, flags);
-	patch_instruction(iptr, instr);
+	ppc_inst_write(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);
+	ppc_inst_write(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);
+	ppc_inst_write(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);
+	ppc_inst_write(iptr, instr);
 	check(instr_is_branch_to_addr(iptr, addr - 0x8000));
 
 	/* Out of range relative negative offset, - 32 KB + 4*/
@@ -195,7 +195,7 @@ static void __init test_branch_bform(void)
 
 	/* Check flags are masked correctly */
 	err = create_cond_branch(&instr, iptr, addr, 0xFFFFFFFC);
-	patch_instruction(iptr, instr);
+	ppc_inst_write(iptr, instr);
 	check(instr_is_branch_to_addr(iptr, addr));
 	check(ppc_inst_equal(instr, ppc_inst(0x43FF0000)));
 }
@@ -215,20 +215,22 @@ static void __init test_translate_branch(void)
 	/* Simple case, branch to self moved a little */
 	p = buf;
 	addr = (unsigned long)p;
-	patch_branch(p, addr, 0);
+	create_branch(&instr, p, addr, 0);
+	ppc_inst_write(p, instr);
 	check(instr_is_branch_to_addr(p, addr));
 	q = p + 4;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(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);
+	create_branch(&instr, p, addr, 0);
+	ppc_inst_write(p, instr);
 	q = buf + 0x2000000;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(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)));
@@ -236,10 +238,11 @@ static void __init test_translate_branch(void)
 	/* Maximum positive case, move x to x - 32 MB + 4 */
 	p = buf + 0x2000000;
 	addr = (unsigned long)p;
-	patch_branch(p, addr, 0);
+	create_branch(&instr, p, addr, 0);
+	ppc_inst_write(p, instr);
 	q = buf + 4;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(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)));
@@ -247,20 +250,22 @@ static void __init test_translate_branch(void)
 	/* Jump to x + 16 MB moved to x + 20 MB */
 	p = buf;
 	addr = 0x1000000 + (unsigned long)buf;
-	patch_branch(p, addr, BRANCH_SET_LINK);
+	create_branch(&instr, p, addr, BRANCH_SET_LINK);
+	ppc_inst_write(p, instr);
 	q = buf + 0x1400000;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(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);
+	create_branch(&instr, p, addr, 0);
+	ppc_inst_write(p, instr);
 	q = buf + 4;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(q, instr);
 	check(instr_is_branch_to_addr(p, addr));
 	check(instr_is_branch_to_addr(q, addr));
 
@@ -271,21 +276,21 @@ static void __init test_translate_branch(void)
 	p = buf;
 	addr = (unsigned long)p;
 	create_cond_branch(&instr, p, addr, 0);
-	patch_instruction(p, instr);
+	ppc_inst_write(p, instr);
 	check(instr_is_branch_to_addr(p, addr));
 	q = buf + 4;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(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);
+	ppc_inst_write(p, instr);
 	q = buf + 0x8000;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(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)));
@@ -294,10 +299,10 @@ static void __init test_translate_branch(void)
 	p = buf + 0x8000;
 	addr = (unsigned long)p;
 	create_cond_branch(&instr, p, addr, 0xFFFFFFFC);
-	patch_instruction(p, instr);
+	ppc_inst_write(p, instr);
 	q = buf + 4;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(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)));
@@ -306,10 +311,10 @@ static void __init test_translate_branch(void)
 	p = buf;
 	addr = 0x3000 + (unsigned long)buf;
 	create_cond_branch(&instr, p, addr, BRANCH_SET_LINK);
-	patch_instruction(p, instr);
+	ppc_inst_write(p, instr);
 	q = buf + 0x5000;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(q, instr);
 	check(instr_is_branch_to_addr(p, addr));
 	check(instr_is_branch_to_addr(q, addr));
 
@@ -317,10 +322,10 @@ static void __init test_translate_branch(void)
 	p = buf + 0x2000;
 	addr = 0x4000 + (unsigned long)buf;
 	create_cond_branch(&instr, p, addr, 0);
-	patch_instruction(p, instr);
+	ppc_inst_write(p, instr);
 	q = buf + 4;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(q, instr);
 	check(instr_is_branch_to_addr(p, addr));
 	check(instr_is_branch_to_addr(q, addr));
 
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v1 11/11] powerpc/code-patching: Replace patch_instruction() by ppc_inst_write() in selftests
@ 2021-12-02 12:00   ` Christophe Leroy
  0 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-02 12:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

The purpose of selftests is to check that instructions are
properly formed. Not to check that they properly run.

For that test it uses normal memory, not special test
memory.

In preparation of a future patch enforcing patch_instruction()
to be used only on valid text areas, implement a ppc_inst_write()
instruction which is the complement of ppc_inst_read(). This
new function writes the formated instruction in valid kernel
memory and doesn't bother about icache.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/inst.h       |  8 +++
 arch/powerpc/lib/test-code-patching.c | 85 ++++++++++++++-------------
 2 files changed, 53 insertions(+), 40 deletions(-)

diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index 631436f3f5c3..21fe8594e078 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -131,6 +131,14 @@ static inline unsigned long ppc_inst_as_ulong(ppc_inst_t x)
 		return (u64)ppc_inst_val(x) << 32 | ppc_inst_suffix(x);
 }
 
+static inline void ppc_inst_write(u32 *ptr, ppc_inst_t x)
+{
+	if (!ppc_inst_prefixed(x))
+		*ptr = ppc_inst_val(x);
+	else
+		*(u64 *)ptr = ppc_inst_as_ulong(x);
+}
+
 #define PPC_INST_STR_LEN sizeof("00000000 00000000")
 
 static inline char *__ppc_inst_as_str(char str[PPC_INST_STR_LEN], ppc_inst_t x)
diff --git a/arch/powerpc/lib/test-code-patching.c b/arch/powerpc/lib/test-code-patching.c
index e358c9d8a03e..c44823292f73 100644
--- a/arch/powerpc/lib/test-code-patching.c
+++ b/arch/powerpc/lib/test-code-patching.c
@@ -54,39 +54,39 @@ static void __init test_branch_iform(void)
 	check(!instr_is_branch_iform(ppc_inst(0x7bfffffd)));
 
 	/* Absolute branch to 0x100 */
-	patch_instruction(iptr, ppc_inst(0x48000103));
+	ppc_inst_write(iptr, ppc_inst(0x48000103));
 	check(instr_is_branch_to_addr(iptr, 0x100));
 	/* Absolute branch to 0x420fc */
-	patch_instruction(iptr, ppc_inst(0x480420ff));
+	ppc_inst_write(iptr, ppc_inst(0x480420ff));
 	check(instr_is_branch_to_addr(iptr, 0x420fc));
 	/* Maximum positive relative branch, + 20MB - 4B */
-	patch_instruction(iptr, ppc_inst(0x49fffffc));
+	ppc_inst_write(iptr, ppc_inst(0x49fffffc));
 	check(instr_is_branch_to_addr(iptr, addr + 0x1FFFFFC));
 	/* Smallest negative relative branch, - 4B */
-	patch_instruction(iptr, ppc_inst(0x4bfffffc));
+	ppc_inst_write(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));
+	ppc_inst_write(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);
+	ppc_inst_write(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);
+	ppc_inst_write(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);
+	ppc_inst_write(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);
+	ppc_inst_write(iptr, instr);
 	check(instr_is_branch_to_addr(iptr, addr - 0x2000000));
 
 	/* Out of range relative negative offset, - 32 MB + 4*/
@@ -103,7 +103,7 @@ static void __init test_branch_iform(void)
 
 	/* Check flags are masked correctly */
 	err = create_branch(&instr, iptr, addr, 0xFFFFFFFC);
-	patch_instruction(iptr, instr);
+	ppc_inst_write(iptr, instr);
 	check(instr_is_branch_to_addr(iptr, addr));
 	check(ppc_inst_equal(instr, ppc_inst(0x48000000)));
 }
@@ -143,19 +143,19 @@ static void __init test_branch_bform(void)
 	check(!instr_is_branch_bform(ppc_inst(0x7bffffff)));
 
 	/* Absolute conditional branch to 0x100 */
-	patch_instruction(iptr, ppc_inst(0x43ff0103));
+	ppc_inst_write(iptr, ppc_inst(0x43ff0103));
 	check(instr_is_branch_to_addr(iptr, 0x100));
 	/* Absolute conditional branch to 0x20fc */
-	patch_instruction(iptr, ppc_inst(0x43ff20ff));
+	ppc_inst_write(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));
+	ppc_inst_write(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));
+	ppc_inst_write(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));
+	ppc_inst_write(iptr, ppc_inst(0x43ff8000));
 	check(instr_is_branch_to_addr(iptr, addr - 0x8000));
 
 	/* All condition code bits set & link */
@@ -163,22 +163,22 @@ static void __init test_branch_bform(void)
 
 	/* Branch to self */
 	err = create_cond_branch(&instr, iptr, addr, flags);
-	patch_instruction(iptr, instr);
+	ppc_inst_write(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);
+	ppc_inst_write(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);
+	ppc_inst_write(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);
+	ppc_inst_write(iptr, instr);
 	check(instr_is_branch_to_addr(iptr, addr - 0x8000));
 
 	/* Out of range relative negative offset, - 32 KB + 4*/
@@ -195,7 +195,7 @@ static void __init test_branch_bform(void)
 
 	/* Check flags are masked correctly */
 	err = create_cond_branch(&instr, iptr, addr, 0xFFFFFFFC);
-	patch_instruction(iptr, instr);
+	ppc_inst_write(iptr, instr);
 	check(instr_is_branch_to_addr(iptr, addr));
 	check(ppc_inst_equal(instr, ppc_inst(0x43FF0000)));
 }
@@ -215,20 +215,22 @@ static void __init test_translate_branch(void)
 	/* Simple case, branch to self moved a little */
 	p = buf;
 	addr = (unsigned long)p;
-	patch_branch(p, addr, 0);
+	create_branch(&instr, p, addr, 0);
+	ppc_inst_write(p, instr);
 	check(instr_is_branch_to_addr(p, addr));
 	q = p + 4;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(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);
+	create_branch(&instr, p, addr, 0);
+	ppc_inst_write(p, instr);
 	q = buf + 0x2000000;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(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)));
@@ -236,10 +238,11 @@ static void __init test_translate_branch(void)
 	/* Maximum positive case, move x to x - 32 MB + 4 */
 	p = buf + 0x2000000;
 	addr = (unsigned long)p;
-	patch_branch(p, addr, 0);
+	create_branch(&instr, p, addr, 0);
+	ppc_inst_write(p, instr);
 	q = buf + 4;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(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)));
@@ -247,20 +250,22 @@ static void __init test_translate_branch(void)
 	/* Jump to x + 16 MB moved to x + 20 MB */
 	p = buf;
 	addr = 0x1000000 + (unsigned long)buf;
-	patch_branch(p, addr, BRANCH_SET_LINK);
+	create_branch(&instr, p, addr, BRANCH_SET_LINK);
+	ppc_inst_write(p, instr);
 	q = buf + 0x1400000;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(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);
+	create_branch(&instr, p, addr, 0);
+	ppc_inst_write(p, instr);
 	q = buf + 4;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(q, instr);
 	check(instr_is_branch_to_addr(p, addr));
 	check(instr_is_branch_to_addr(q, addr));
 
@@ -271,21 +276,21 @@ static void __init test_translate_branch(void)
 	p = buf;
 	addr = (unsigned long)p;
 	create_cond_branch(&instr, p, addr, 0);
-	patch_instruction(p, instr);
+	ppc_inst_write(p, instr);
 	check(instr_is_branch_to_addr(p, addr));
 	q = buf + 4;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(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);
+	ppc_inst_write(p, instr);
 	q = buf + 0x8000;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(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)));
@@ -294,10 +299,10 @@ static void __init test_translate_branch(void)
 	p = buf + 0x8000;
 	addr = (unsigned long)p;
 	create_cond_branch(&instr, p, addr, 0xFFFFFFFC);
-	patch_instruction(p, instr);
+	ppc_inst_write(p, instr);
 	q = buf + 4;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(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)));
@@ -306,10 +311,10 @@ static void __init test_translate_branch(void)
 	p = buf;
 	addr = 0x3000 + (unsigned long)buf;
 	create_cond_branch(&instr, p, addr, BRANCH_SET_LINK);
-	patch_instruction(p, instr);
+	ppc_inst_write(p, instr);
 	q = buf + 0x5000;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(q, instr);
 	check(instr_is_branch_to_addr(p, addr));
 	check(instr_is_branch_to_addr(q, addr));
 
@@ -317,10 +322,10 @@ static void __init test_translate_branch(void)
 	p = buf + 0x2000;
 	addr = 0x4000 + (unsigned long)buf;
 	create_cond_branch(&instr, p, addr, 0);
-	patch_instruction(p, instr);
+	ppc_inst_write(p, instr);
 	q = buf + 4;
 	translate_branch(&instr, q, p);
-	patch_instruction(q, instr);
+	ppc_inst_write(q, instr);
 	check(instr_is_branch_to_addr(p, addr));
 	check(instr_is_branch_to_addr(q, addr));
 
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v1 06/11] powerpc/code-patching: Fix patch_branch() return on out-of-range failure
  2021-12-02 12:00   ` Christophe Leroy
@ 2021-12-22 13:16     ` Christophe Leroy
  -1 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-22 13:16 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev



Le 02/12/2021 à 13:00, Christophe Leroy a écrit :
> Do not silentely ignore a failure of create_branch() in
> patch_branch(). Return -ERANGE.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>

 From 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/4940b03de220d1dfe2c6b47a41e60925497ce125.1630657331.git.christophe.leroy@csgroup.eu/

Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

> ---
>   arch/powerpc/lib/code-patching.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index a43ca22313ee..e7a2a41ae8eb 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -191,7 +191,9 @@ int patch_branch(u32 *addr, unsigned long target, int flags)
>   {
>   	ppc_inst_t instr;
>   
> -	create_branch(&instr, addr, target, flags);
> +	if (create_branch(&instr, addr, target, flags))
> +		return -ERANGE;
> +
>   	return patch_instruction(addr, instr);
>   }
>   
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v1 06/11] powerpc/code-patching: Fix patch_branch() return on out-of-range failure
@ 2021-12-22 13:16     ` Christophe Leroy
  0 siblings, 0 replies; 26+ messages in thread
From: Christophe Leroy @ 2021-12-22 13:16 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel



Le 02/12/2021 à 13:00, Christophe Leroy a écrit :
> Do not silentely ignore a failure of create_branch() in
> patch_branch(). Return -ERANGE.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>

 From 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/4940b03de220d1dfe2c6b47a41e60925497ce125.1630657331.git.christophe.leroy@csgroup.eu/

Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

> ---
>   arch/powerpc/lib/code-patching.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index a43ca22313ee..e7a2a41ae8eb 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -191,7 +191,9 @@ int patch_branch(u32 *addr, unsigned long target, int flags)
>   {
>   	ppc_inst_t instr;
>   
> -	create_branch(&instr, addr, target, flags);
> +	if (create_branch(&instr, addr, target, flags))
> +		return -ERANGE;
> +
>   	return patch_instruction(addr, instr);
>   }
>   
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v1 01/11] powerpc/code-patching: Remove pr_debug()/pr_devel() messages and fix check()
  2021-12-02 12:00 ` Christophe Leroy
@ 2021-12-26 21:52   ` Michael Ellerman
  -1 siblings, 0 replies; 26+ messages in thread
From: Michael Ellerman @ 2021-12-26 21:52 UTC (permalink / raw)
  To: Paul Mackerras, Benjamin Herrenschmidt, Christophe Leroy,
	Michael Ellerman
  Cc: linux-kernel, linuxppc-dev

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v1 01/11] powerpc/code-patching: Remove pr_debug()/pr_devel() messages and fix check()
@ 2021-12-26 21:52   ` Michael Ellerman
  0 siblings, 0 replies; 26+ messages in thread
From: Michael Ellerman @ 2021-12-26 21:52 UTC (permalink / raw)
  To: Paul Mackerras, Benjamin Herrenschmidt, Christophe Leroy,
	Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2021-12-26 21:55 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-02 12:00 [PATCH v1 01/11] powerpc/code-patching: Remove pr_debug()/pr_devel() messages and fix check() Christophe Leroy
2021-12-02 12:00 ` Christophe Leroy
2021-12-02 12:00 ` [PATCH v1 02/11] powerpc/code-patching: Remove init_mem_is_free Christophe Leroy
2021-12-02 12:00   ` Christophe Leroy
2021-12-02 12:00 ` [PATCH v1 03/11] powerpc/code-patching: Fix error handling in do_patch_instruction() Christophe Leroy
2021-12-02 12:00   ` Christophe Leroy
2021-12-02 12:00 ` [PATCH v1 04/11] powerpc/code-patching: Fix unmap_patch_area() error handling Christophe Leroy
2021-12-02 12:00   ` Christophe Leroy
2021-12-02 12:00 ` [PATCH v1 05/11] powerpc/code-patching: Reorganise do_patch_instruction() to ease " Christophe Leroy
2021-12-02 12:00   ` Christophe Leroy
2021-12-02 12:00 ` [PATCH v1 06/11] powerpc/code-patching: Fix patch_branch() return on out-of-range failure Christophe Leroy
2021-12-02 12:00   ` Christophe Leroy
2021-12-22 13:16   ` Christophe Leroy
2021-12-22 13:16     ` Christophe Leroy
2021-12-02 12:00 ` [PATCH v1 07/11] powerpc/code-patching: Use test_trampoline for prefixed patch test Christophe Leroy
2021-12-02 12:00   ` Christophe Leroy
2021-12-02 12:00 ` [PATCH v1 08/11] powerpc/code-patching: Move patch_exception() outside code-patching.c Christophe Leroy
2021-12-02 12:00   ` Christophe Leroy
2021-12-02 12:00 ` [PATCH v1 09/11] powerpc/code-patching: Move instr_is_branch_{i/b}form() in code-patching.h Christophe Leroy
2021-12-02 12:00   ` Christophe Leroy
2021-12-02 12:00 ` [PATCH v1 10/11] powerpc/code-patching: Move code patching selftests in its own file Christophe Leroy
2021-12-02 12:00   ` Christophe Leroy
2021-12-02 12:00 ` [PATCH v1 11/11] powerpc/code-patching: Replace patch_instruction() by ppc_inst_write() in selftests Christophe Leroy
2021-12-02 12:00   ` Christophe Leroy
2021-12-26 21:52 ` [PATCH v1 01/11] powerpc/code-patching: Remove pr_debug()/pr_devel() messages and fix check() Michael Ellerman
2021-12-26 21:52   ` Michael Ellerman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.