[PATCH v8 0/6] Use per-CPU temporary mappings for patching

This is a revision of Chris and Jordan's series to introduces a per cpu temporary
mm to be used for patching with strict rwx on radix mmus.

It is just rebased on powerpc/next. I am aware there are several code patching
patches on the list and can rebase when necessary. For now I figure this'll get
changes requested for a v9 either way.

v8:	* Merge the temp mm 'introduction' and usage into one patch.
	  x86 split it because their temp MMU swap mechanism may be
	  used for other purposes, but ours cannot (it is local to
	  code-patching.c).
	* Shuffle v7,3/5 cpu_patching_addr usage to the end (v8,4/4)
	  after cpu_patching_addr is actually introduced.
	* Clearer formatting of the cpuhp_setup_state arguments
	* Only allocate patching resources as CPU comes online. Free
	  them when CPU goes offline or if an error occurs during allocation.
	* Refactored the random address calculation to make the page
	  alignment more obvious.
	* Manually perform the allocation page walk to avoid taking locks
	  (which, given they are not necessary to take, is misleading) and
	  prevent memory leaks if page tree allocation fails.
	* Cache the pte pointer.
	* Stop using the patching mm first, then clear the patching PTE & TLB.
	* Only clear the VA with the writable mapping from the TLB. Leaving
	  the other TLB entries helps performance, especially when patching
	  many times in a row (e.g., ftrace activation).
	* Instruction patch verification moved to it's own patch onto shared
	  path with existing mechanism.
	* Detect missing patching_mm and return an error for the caller to
	  decide what to do.
	* Comment the purposes of each synchronisation, and why it is safe to
	  omit some at certain points.

Previous versions:
v7: https://lore.kernel.org/all/20211110003717.1150965-1-jniethe5@gmail.com/
v6: https://lore.kernel.org/all/20210911022904.30962-1-cmr@bluescreens.de/
v5: https://lore.kernel.org/all/20210713053113.4632-1-cmr@linux.ibm.com/
v4: https://lore.kernel.org/all/20210429072057.8870-1-cmr@bluescreens.de/
v3: https://lore.kernel.org/all/20200827052659.24922-1-cmr@codefail.de/
v2: https://lore.kernel.org/all/20200709040316.12789-1-cmr@informatik.wtf/
v1: https://lore.kernel.org/all/20200603051912.23296-1-cmr@informatik.wtf/
RFC: https://lore.kernel.org/all/20200323045205.20314-1-cmr@informatik.wtf/
x86: https://lore.kernel.org/kernel-hardening/20190426232303.28381-1-nadav.amit@gmail.com/

Benjamin Gray (5):
  powerpc/code-patching: Use WARN_ON and fix check in poking_init
  powerpc/code-patching: Verify instruction patch succeeded
  powerpc/tlb: Add local flush for page given mm_struct and psize
  powerpc/code-patching: Use temporary mm for Radix MMU
  powerpc/code-patching: Use CPU local patch address directly

Jordan Niethe (1):
  powerpc: Allow clearing and restoring registers independent of saved
    breakpoint state

 arch/powerpc/include/asm/book3s/32/tlbflush.h |   9 +
 .../include/asm/book3s/64/tlbflush-hash.h     |   5 +
 arch/powerpc/include/asm/book3s/64/tlbflush.h |   8 +
 arch/powerpc/include/asm/debug.h              |   2 +
 arch/powerpc/include/asm/nohash/tlbflush.h    |   1 +
 arch/powerpc/kernel/process.c                 |  36 ++-
 arch/powerpc/lib/code-patching.c              | 236 +++++++++++++++++-
 7 files changed, 284 insertions(+), 13 deletions(-)


base-commit: 8636df94ec917019c4cb744ba0a1f94cf9057790
prerequisite-patch-id: b8387303be6478fdf94264d485d5e08994f305c7
prerequisite-patch-id: 06e54849e6c9e45a9b24668fa12cc0ece3f831a7
prerequisite-patch-id: f4be9e7d613761fba33fb2f7a81839cef36fe0fe
prerequisite-patch-id: 4ea0e36de5c393f9f6ae6243cb21a0ddb364c263
prerequisite-patch-id: 47a1294f0a5d5531ec5c32a761269cb5a1158515
prerequisite-patch-id: d72e371d3d820fdf529f03d2544c7f7f8bb6327a
prerequisite-patch-id: 3024e700433cb6a20dc1e1c6476ea1e98409d8b7
prerequisite-patch-id: f136637f7a8fe92dc4f60b908e2e7aa24aac3f43
--
2.37.3

From: Jordan Niethe <redacted>

For the coming temporary mm used for instruction patching, the
breakpoint registers need to be cleared to prevent them from
accidentally being triggered. As soon as the patching is done, the
breakpoints will be restored. The breakpoint state is stored in the per
cpu variable current_brk[]. Add a pause_breakpoints() function which will
clear the breakpoint registers without touching the state in
current_bkr[]. Add a pair function unpause_breakpoints() which will move
the state in current_brk[] back to the registers.

Signed-off-by: Jordan Niethe <redacted>
Signed-off-by: Benjamin Gray <redacted>
---
 arch/powerpc/include/asm/debug.h |  2 ++
 arch/powerpc/kernel/process.c    | 36 +++++++++++++++++++++++++++++---
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
index 86a14736c76c..83f2dc3785e8 100644
--- a/arch/powerpc/include/asm/debug.h
+++ b/arch/powerpc/include/asm/debug.h

@@ -46,6 +46,8 @@ static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
 #endif
 
 void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk);
+void pause_breakpoints(void);
+void unpause_breakpoints(void);
 bool ppc_breakpoint_available(void);
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS
 extern void do_send_trap(struct pt_regs *regs, unsigned long address,

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 67da147fe34d..7aee1b30e73c 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c

@@ -685,6 +685,7 @@ DEFINE_INTERRUPT_HANDLER(do_break)
 
 static DEFINE_PER_CPU(struct arch_hw_breakpoint, current_brk[HBP_NUM_MAX]);
 
+
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS
 /*
  * Set the debug registers back to their default "safe" values.

@@ -862,10 +863,8 @@ static inline int set_breakpoint_8xx(struct arch_hw_breakpoint *brk)
 	return 0;
 }
 
-void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk)
+static void ____set_breakpoint(int nr, struct arch_hw_breakpoint *brk)
 {
-	memcpy(this_cpu_ptr(&current_brk[nr]), brk, sizeof(*brk));
-
 	if (dawr_enabled())
 		// Power8 or later
 		set_dawr(nr, brk);

@@ -879,6 +878,12 @@ void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk)
 		WARN_ON_ONCE(1);
 }
 
+void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk)
+{
+	memcpy(this_cpu_ptr(&current_brk[nr]), brk, sizeof(*brk));
+	____set_breakpoint(nr, brk);
+}
+
 /* Check if we have DAWR or DABR hardware */
 bool ppc_breakpoint_available(void)
 {

@@ -891,6 +896,31 @@ bool ppc_breakpoint_available(void)
 }
 EXPORT_SYMBOL_GPL(ppc_breakpoint_available);
 
+/* Disable the breakpoint in hardware without touching current_brk[] */
+void pause_breakpoints(void)
+{
+	struct arch_hw_breakpoint brk = {0};
+	int i;
+
+	if (!ppc_breakpoint_available())
+		return;
+
+	for (i = 0; i < nr_wp_slots(); i++)
+		____set_breakpoint(i, &brk);
+}
+
+/* Renable the breakpoint in hardware from current_brk[] */
+void unpause_breakpoints(void)
+{
+	int i;
+
+	if (!ppc_breakpoint_available())
+		return;
+
+	for (i = 0; i < nr_wp_slots(); i++)
+		____set_breakpoint(i, this_cpu_ptr(&current_brk[i]));
+}
+
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 
 static inline bool tm_enabled(struct task_struct *tsk)

-- 
2.37.3

From: "Christopher M. Riedl" <redacted>

The latest kernel docs list BUG_ON() as 'deprecated' and that they
should be replaced with WARN_ON() (or pr_warn()) when possible. The
BUG_ON() in poking_init() warrants a WARN_ON() rather than a pr_warn()
since the error condition is deemed "unreachable".

Also take this opportunity to fix the failure check in the WARN_ON():
cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, ...) returns a positive integer
on success and a negative integer on failure.

Signed-off-by: Benjamin Gray <redacted>
---
 arch/powerpc/lib/code-patching.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index ad0cf3108dd0..34fc7ac34d91 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c

@@ -81,16 +81,13 @@ static int text_area_cpu_down(unsigned int cpu)
 
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(poking_init_done);
 
-/*
- * 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));
+	WARN_ON(cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
+				  "powerpc/text_poke:online",
+				  text_area_cpu_up,
+				  text_area_cpu_down) < 0);
+
 	static_branch_enable(&poking_init_done);
 }
 

-- 
2.37.3

Verifies that if the instruction patching did not return an error then
the value stored at the given address to patch is now equal to the
instruction we patched it to.

Signed-off-by: Benjamin Gray <redacted>
---
 arch/powerpc/lib/code-patching.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 34fc7ac34d91..9b9eba574d7e 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c

@@ -186,6 +186,8 @@ static int do_patch_instruction(u32 *addr, ppc_inst_t instr)
 	err = __do_patch_instruction(addr, instr);
 	local_irq_restore(flags);
 
+	WARN_ON(!err && !ppc_inst_equal(instr, ppc_inst_read(addr)));
+
 	return err;
 }
 #else /* !CONFIG_STRICT_KERNEL_RWX */

-- 
2.37.3

Adds a local TLB flush operation that works given an mm_struct, VA to
flush, and page size representation.

This removes the need to create a vm_area_struct, which the temporary
patching mm work does not need.

Signed-off-by: Benjamin Gray <redacted>
---
 arch/powerpc/include/asm/book3s/32/tlbflush.h      | 9 +++++++++
 arch/powerpc/include/asm/book3s/64/tlbflush-hash.h | 5 +++++
 arch/powerpc/include/asm/book3s/64/tlbflush.h      | 8 ++++++++
 arch/powerpc/include/asm/nohash/tlbflush.h         | 1 +
 4 files changed, 23 insertions(+)

diff --git a/arch/powerpc/include/asm/book3s/32/tlbflush.h b/arch/powerpc/include/asm/book3s/32/tlbflush.h
index ba1743c52b56..e5a688cebf69 100644
--- a/arch/powerpc/include/asm/book3s/32/tlbflush.h
+++ b/arch/powerpc/include/asm/book3s/32/tlbflush.h

@@ -2,6 +2,8 @@
 #ifndef _ASM_POWERPC_BOOK3S_32_TLBFLUSH_H
 #define _ASM_POWERPC_BOOK3S_32_TLBFLUSH_H
 
+#include <linux/build_bug.h>
+
 #define MMU_NO_CONTEXT      (0)
 /*
  * TLB flushing for "classic" hash-MMU 32-bit CPUs, 6xx, 7xx, 7xxx

@@ -74,6 +76,13 @@ static inline void local_flush_tlb_page(struct vm_area_struct *vma,
 {
 	flush_tlb_page(vma, vmaddr);
 }
+
+static inline void local_flush_tlb_page_psize(struct mm_struct *mm, unsigned long vmaddr, int psize)
+{
+	BUILD_BUG_ON(psize != MMU_PAGE_4K);
+	flush_range(mm, vmaddr, vmaddr + PAGE_SIZE);
+}
+
 static inline void local_flush_tlb_mm(struct mm_struct *mm)
 {
 	flush_tlb_mm(mm);

diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h b/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h
index fab8332fe1ad..8fd9dc49b2a1 100644
--- a/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h
+++ b/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h

@@ -94,6 +94,11 @@ static inline void hash__local_flush_tlb_page(struct vm_area_struct *vma,
 {
 }
 
+static inline void hash__local_flush_tlb_page_psize(struct mm_struct *mm,
+						    unsigned long vmaddr, int psize)
+{
+}
+
 static inline void hash__flush_tlb_page(struct vm_area_struct *vma,
 				    unsigned long vmaddr)
 {

diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush.h b/arch/powerpc/include/asm/book3s/64/tlbflush.h
index 67655cd60545..2d839dd5c08c 100644
--- a/arch/powerpc/include/asm/book3s/64/tlbflush.h
+++ b/arch/powerpc/include/asm/book3s/64/tlbflush.h

@@ -92,6 +92,14 @@ static inline void local_flush_tlb_page(struct vm_area_struct *vma,
 	return hash__local_flush_tlb_page(vma, vmaddr);
 }
 
+static inline void local_flush_tlb_page_psize(struct mm_struct *mm,
+					      unsigned long vmaddr, int psize)
+{
+	if (radix_enabled())
+		return radix__local_flush_tlb_page_psize(mm, vmaddr, psize);
+	return hash__local_flush_tlb_page_psize(mm, vmaddr, psize);
+}
+
 static inline void local_flush_all_mm(struct mm_struct *mm)
 {
 	if (radix_enabled())

diff --git a/arch/powerpc/include/asm/nohash/tlbflush.h b/arch/powerpc/include/asm/nohash/tlbflush.h
index bdaf34ad41ea..59bce0ebdcf4 100644
--- a/arch/powerpc/include/asm/nohash/tlbflush.h
+++ b/arch/powerpc/include/asm/nohash/tlbflush.h

@@ -58,6 +58,7 @@ static inline void flush_tlb_kernel_range(unsigned long start, unsigned long end
 extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
 extern void local_flush_tlb_mm(struct mm_struct *mm);
 extern void local_flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
+extern void local_flush_tlb_page_psize(struct mm_struct *mm, unsigned long vmaddr, int psize);
 
 extern void __local_flush_tlb_page(struct mm_struct *mm, unsigned long vmaddr,
 				   int tsize, int ind);

-- 
2.37.3

From: "Christopher M. Riedl" <redacted>

x86 supports the notion of a temporary mm which restricts access to
temporary PTEs to a single CPU. A temporary mm is useful for situations
where a CPU needs to perform sensitive operations (such as patching a
STRICT_KERNEL_RWX kernel) requiring temporary mappings without exposing
said mappings to other CPUs. Another benefit is that other CPU TLBs do
not need to be flushed when the temporary mm is torn down.

Mappings in the temporary mm can be set in the userspace portion of the
address-space.

Interrupts must be disabled while the temporary mm is in use. HW
breakpoints, which may have been set by userspace as watchpoints on
addresses now within the temporary mm, are saved and disabled when
loading the temporary mm. The HW breakpoints are restored when unloading
the temporary mm. All HW breakpoints are indiscriminately disabled while
the temporary mm is in use - this may include breakpoints set by perf.

Use the `poking_init` init hook to prepare a temporary mm and patching
address. Initialize the temporary mm by copying the init mm. Choose a
randomized patching address inside the temporary mm userspace address
space. The patching address is randomized between PAGE_SIZE and
DEFAULT_MAP_WINDOW-PAGE_SIZE.

Bits of entropy with 64K page size on BOOK3S_64:

	bits of entropy = log2(DEFAULT_MAP_WINDOW_USER64 / PAGE_SIZE)

	PAGE_SIZE=64K, DEFAULT_MAP_WINDOW_USER64=128TB
	bits of entropy = log2(128TB / 64K)
	bits of entropy = 31

The upper limit is DEFAULT_MAP_WINDOW due to how the Book3s64 Hash MMU
operates - by default the space above DEFAULT_MAP_WINDOW is not
available. Currently the Hash MMU does not use a temporary mm so
technically this upper limit isn't necessary; however, a larger
randomization range does not further "harden" this overall approach and
future work may introduce patching with a temporary mm on Hash as well.

Randomization occurs only once during initialization for each CPU as it
comes online.

The patching page is mapped with PAGE_KERNEL to set EAA[0] for the PTE
which ignores the AMR (so no need to unlock/lock KUAP) according to
PowerISA v3.0b Figure 35 on Radix.

Based on x86 implementation:

commit 4fc19708b165
("x86/alternatives: Initialize temporary mm for patching")

and:

commit b3fd8e83ada0
("x86/alternatives: Use temporary mm for text poking")

---

Synchronisation is done according to Book 3 Chapter 13 "Synchronization
Requirements for Context Alterations". Switching the mm is a change to
the PID, which requires a context synchronising instruction before and
after the change, and a hwsync between the last instruction that
performs address translation for an associated storage access.

Instruction fetch is an associated storage access, but the instruction
address mappings are not being changed, so it should not matter which
context they use. We must still perform a hwsync to guard arbitrary
prior code that may have access a userspace address.

TLB invalidation is local and VA specific. Local because only this core
used the patching mm, and VA specific because we only care that the
writable mapping is purged. Leaving the other mappings intact is more
efficient, especially when performing many code patches in a row (e.g.,
as ftrace would).

Signed-off-by: Benjamin Gray <redacted>
---
 arch/powerpc/lib/code-patching.c | 226 ++++++++++++++++++++++++++++++-
 1 file changed, 221 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 9b9eba574d7e..eabdd74a26c0 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c

@@ -4,12 +4,17 @@
  */
 
 #include <linux/kprobes.h>
+#include <linux/mmu_context.h>
+#include <linux/random.h>
 #include <linux/vmalloc.h>
 #include <linux/init.h>
 #include <linux/cpuhotplug.h>
 #include <linux/uaccess.h>
 #include <linux/jump_label.h>
 
+#include <asm/debug.h>
+#include <asm/pgalloc.h>
+#include <asm/tlb.h>
 #include <asm/tlbflush.h>
 #include <asm/page.h>
 #include <asm/code-patching.h>

@@ -42,11 +47,59 @@ int raw_patch_instruction(u32 *addr, ppc_inst_t instr)
 }
 
 #ifdef CONFIG_STRICT_KERNEL_RWX
+
 static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
+static DEFINE_PER_CPU(struct mm_struct *, cpu_patching_mm);
+static DEFINE_PER_CPU(unsigned long, cpu_patching_addr);
+static DEFINE_PER_CPU(pte_t *, cpu_patching_pte);
 
 static int map_patch_area(void *addr, unsigned long text_poke_addr);
 static void unmap_patch_area(unsigned long addr);
 
+struct temp_mm_state {
+	struct mm_struct *mm;
+};
+
+static bool mm_patch_enabled(void)
+{
+	return IS_ENABLED(CONFIG_SMP) && radix_enabled();
+}
+
+/*
+ * The following applies for Radix MMU. Hash MMU has different requirements,
+ * and so is not supported.
+ *
+ * Changing mm requires context synchronising instructions on both sides of
+ * the context switch, as well as a hwsync between the last instruction for
+ * which the address of an associated storage access was translated using
+ * the current context.
+ *
+ * switch_mm_irqs_off performs an isync after the context switch. It is
+ * the responsibility of the caller to perform the CSI and hwsync before
+ * starting/stopping the temp mm.
+ */
+static struct temp_mm_state start_using_temp_mm(struct mm_struct *mm)
+{
+	struct temp_mm_state temp_state;
+
+	lockdep_assert_irqs_disabled();
+	temp_state.mm = current->active_mm;
+	switch_mm_irqs_off(temp_state.mm, mm, current);
+
+	WARN_ON(!mm_is_thread_local(mm));
+
+	pause_breakpoints();
+	return temp_state;
+}
+
+static void stop_using_temp_mm(struct mm_struct *temp_mm,
+			       struct temp_mm_state prev_state)
+{
+	lockdep_assert_irqs_disabled();
+	switch_mm_irqs_off(temp_mm, prev_state.mm, current);
+	unpause_breakpoints();
+}
+
 static int text_area_cpu_up(unsigned int cpu)
 {
 	struct vm_struct *area;

@@ -79,14 +132,127 @@ static int text_area_cpu_down(unsigned int cpu)
 	return 0;
 }
 
+static int text_area_cpu_up_mm(unsigned int cpu)
+{
+	struct mm_struct *mm;
+	unsigned long addr;
+	pgd_t *pgdp;
+	p4d_t *p4dp;
+	pud_t *pudp;
+	pmd_t *pmdp;
+	pte_t *ptep;
+
+	mm = copy_init_mm();
+	if (WARN_ON(!mm))
+		goto fail_no_mm;
+
+	/*
+	 * Choose a random page-aligned address from the interval
+	 * [PAGE_SIZE .. DEFAULT_MAP_WINDOW - PAGE_SIZE].
+	 * The lower address bound is PAGE_SIZE to avoid the zero-page.
+	 */
+	addr = (1 + (get_random_long() % (DEFAULT_MAP_WINDOW / PAGE_SIZE - 2))) << PAGE_SHIFT;
+
+	/*
+	 * PTE allocation uses GFP_KERNEL which means we need to
+	 * pre-allocate the PTE here because we cannot do the
+	 * allocation during patching when IRQs are disabled.
+	 */
+	pgdp = pgd_offset(mm, addr);
+
+	p4dp = p4d_alloc(mm, pgdp, addr);
+	if (WARN_ON(!p4dp))
+		goto fail_no_p4d;
+
+	pudp = pud_alloc(mm, p4dp, addr);
+	if (WARN_ON(!pudp))
+		goto fail_no_pud;
+
+	pmdp = pmd_alloc(mm, pudp, addr);
+	if (WARN_ON(!pmdp))
+		goto fail_no_pmd;
+
+	ptep = pte_alloc_map(mm, pmdp, addr);
+	if (WARN_ON(!ptep))
+		goto fail_no_pte;
+
+	this_cpu_write(cpu_patching_mm, mm);
+	this_cpu_write(cpu_patching_addr, addr);
+	this_cpu_write(cpu_patching_pte, ptep);
+
+	return 0;
+
+fail_no_pte:
+	pmd_free(mm, pmdp);
+	mm_dec_nr_pmds(mm);
+fail_no_pmd:
+	pud_free(mm, pudp);
+	mm_dec_nr_puds(mm);
+fail_no_pud:
+	p4d_free(patching_mm, p4dp);
+fail_no_p4d:
+	mmput(mm);
+fail_no_mm:
+	return -ENOMEM;
+}
+
+static int text_area_cpu_down_mm(unsigned int cpu)
+{
+	struct mm_struct *mm;
+	unsigned long addr;
+	pte_t *ptep;
+	pmd_t *pmdp;
+	pud_t *pudp;
+	p4d_t *p4dp;
+	pgd_t *pgdp;
+
+	mm = this_cpu_read(cpu_patching_mm);
+	addr = this_cpu_read(cpu_patching_addr);
+
+	pgdp = pgd_offset(mm, addr);
+	p4dp = p4d_offset(pgdp, addr);
+	pudp = pud_offset(p4dp, addr);
+	pmdp = pmd_offset(pudp, addr);
+	ptep = pte_offset_map(pmdp, addr);
+
+	pte_free(mm, ptep);
+	pmd_free(mm, pmdp);
+	pud_free(mm, pudp);
+	p4d_free(mm, p4dp);
+	/* pgd is dropped in mmput */
+
+	mm_dec_nr_ptes(mm);
+	mm_dec_nr_pmds(mm);
+	mm_dec_nr_puds(mm);
+
+	mmput(mm);
+
+	this_cpu_write(cpu_patching_mm, NULL);
+	this_cpu_write(cpu_patching_addr, 0);
+	this_cpu_write(cpu_patching_pte, NULL);
+
+	return 0;
+}
+
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(poking_init_done);
 
 void __init poking_init(void)
 {
-	WARN_ON(cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
-				  "powerpc/text_poke:online",
-				  text_area_cpu_up,
-				  text_area_cpu_down) < 0);
+	int ret;
+
+	if (mm_patch_enabled())
+		ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
+					"powerpc/text_poke_mm:online",
+					text_area_cpu_up_mm,
+					text_area_cpu_down_mm);
+	else
+		ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
+					"powerpc/text_poke:online",
+					text_area_cpu_up,
+					text_area_cpu_down);
+
+	/* cpuhp_setup_state returns >= 0 on success */
+	WARN_ON(ret < 0);
 
 	static_branch_enable(&poking_init_done);
 }

@@ -144,6 +310,53 @@ static void unmap_patch_area(unsigned long addr)
 	flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
 }
 
+static int __do_patch_instruction_mm(u32 *addr, ppc_inst_t instr)
+{
+	int err;
+	u32 *patch_addr;
+	unsigned long text_poke_addr;
+	pte_t *pte;
+	unsigned long pfn = get_patch_pfn(addr);
+	struct mm_struct *patching_mm;
+	struct temp_mm_state prev;
+
+	patching_mm = __this_cpu_read(cpu_patching_mm);
+	pte = __this_cpu_read(cpu_patching_pte);
+	text_poke_addr = __this_cpu_read(cpu_patching_addr);
+	patch_addr = (u32 *)(text_poke_addr + offset_in_page(addr));
+
+	if (unlikely(!patching_mm))
+		return -ENOMEM;
+
+	set_pte_at(patching_mm, text_poke_addr, pte, pfn_pte(pfn, PAGE_KERNEL));
+
+	/* order PTE update before use, also serves as the hwsync */
+	asm volatile("ptesync": : :"memory");
+
+	/* order context switch after arbitrary prior code */
+	isync();
+
+	prev = start_using_temp_mm(patching_mm);
+
+	err = __patch_instruction(addr, instr, patch_addr);
+
+	/* hwsync performed by __patch_instruction (sync) if successful */
+	if (err)
+		mb();  /* sync */
+
+	/* context synchronisation performed by __patch_instruction (isync or exception) */
+	stop_using_temp_mm(patching_mm, prev);
+
+	pte_clear(patching_mm, text_poke_addr, pte);
+	/*
+	 * ptesync to order PTE update before TLB invalidation done
+	 * by radix__local_flush_tlb_page_psize (in _tlbiel_va)
+	 */
+	local_flush_tlb_page_psize(patching_mm, text_poke_addr, mmu_virtual_psize);
+
+	return err;
+}
+
 static int __do_patch_instruction(u32 *addr, ppc_inst_t instr)
 {
 	int err;

@@ -183,7 +396,10 @@ static int do_patch_instruction(u32 *addr, ppc_inst_t instr)
 		return raw_patch_instruction(addr, instr);
 
 	local_irq_save(flags);
-	err = __do_patch_instruction(addr, instr);
+	if (mm_patch_enabled())
+		err = __do_patch_instruction_mm(addr, instr);
+	else
+		err = __do_patch_instruction(addr, instr);
 	local_irq_restore(flags);
 
 	WARN_ON(!err && !ppc_inst_equal(instr, ppc_inst_read(addr)));

-- 
2.37.3

With the isolated mm context support, there is a CPU local variable that
can hold the patch address. Use it instead of adding a level of
indirection through the text_poke_area vm_struct.

Signed-off-by: Benjamin Gray <redacted>
---
 arch/powerpc/lib/code-patching.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index eabdd74a26c0..ce58c1b3fcf1 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c

@@ -122,6 +122,7 @@ static int text_area_cpu_up(unsigned int cpu)
 	unmap_patch_area(addr);
 
 	this_cpu_write(text_poke_area, area);
+	this_cpu_write(cpu_patching_addr, addr);
 
 	return 0;
 }

@@ -365,7 +366,7 @@ static int __do_patch_instruction(u32 *addr, ppc_inst_t instr)
 	pte_t *pte;
 	unsigned long pfn = get_patch_pfn(addr);
 
-	text_poke_addr = (unsigned long)__this_cpu_read(text_poke_area)->addr & PAGE_MASK;
+	text_poke_addr = (unsigned long)__this_cpu_read(cpu_patching_addr) & PAGE_MASK;
 	patch_addr = (u32 *)(text_poke_addr + offset_in_page(addr));
 
 	pte = virt_to_kpte(text_poke_addr);

-- 
2.37.3

On Fri, 2022-10-21 at 16:22 +1100, Benjamin Gray wrote:

From: Jordan Niethe <redacted>

Hi Ben,

For the coming temporary mm used for instruction patching, the
breakpoint registers need to be cleared to prevent them from
accidentally being triggered. As soon as the patching is done, the
breakpoints will be restored. The breakpoint state is stored in the
per
cpu variable current_brk[]. Add a pause_breakpoints() function which
will
clear the breakpoint registers without touching the state in
current_bkr[]. Add a pair function unpause_breakpoints() which will

 
typo here ^

Nitpick, would (clear/suspend)/restore be clearer than pause/unpause?

some bonus whitespace here

Is there a way to refactor this?  The quad underscore is pretty cursed.

On Fri, 2022-10-21 at 16:22 +1100, Benjamin Gray wrote:

From: "Christopher M. Riedl" <redacted>

The latest kernel docs list BUG_ON() as 'deprecated' and that they
should be replaced with WARN_ON() (or pr_warn()) when possible. The
BUG_ON() in poking_init() warrants a WARN_ON() rather than a
pr_warn()
since the error condition is deemed "unreachable".

Also take this opportunity to fix the failure check in the WARN_ON():
cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, ...) returns a positive
integer
on success and a negative integer on failure.

Signed-off-by: Benjamin Gray <redacted>

Reviewed-by: Russell Currey <redacted>

On Fri, 2022-10-21 at 16:22 +1100, Benjamin Gray wrote:

As a side note, I had a look at test-code-patching.c and it doesn't
look like we don't have a test for ppc_inst_equal() with prefixed
instructions.  We should fix that.

        return err;
 }
 #else /* !CONFIG_STRICT_KERNEL_RWX */

On Fri, 2022-10-21 at 16:22 +1100, Benjamin Gray wrote:

Is there any utility in adding this for 32bit if the following patches
are only for Radix?

On Fri, 2022-10-21 at 16:22 +1100, Benjamin Gray wrote:

From: "Christopher M. Riedl" <redacted>

x86 supports the notion of a temporary mm which restricts access to
temporary PTEs to a single CPU. A temporary mm is useful for
situations
where a CPU needs to perform sensitive operations (such as patching a
STRICT_KERNEL_RWX kernel) requiring temporary mappings without
exposing
said mappings to other CPUs. Another benefit is that other CPU TLBs
do
not need to be flushed when the temporary mm is torn down.

Mappings in the temporary mm can be set in the userspace portion of
the
address-space.

Interrupts must be disabled while the temporary mm is in use. HW
breakpoints, which may have been set by userspace as watchpoints on
addresses now within the temporary mm, are saved and disabled when
loading the temporary mm. The HW breakpoints are restored when
unloading
the temporary mm. All HW breakpoints are indiscriminately disabled
while
the temporary mm is in use - this may include breakpoints set by
perf.

Use the `poking_init` init hook to prepare a temporary mm and
patching
address. Initialize the temporary mm by copying the init mm. Choose a
randomized patching address inside the temporary mm userspace address
space. The patching address is randomized between PAGE_SIZE and
DEFAULT_MAP_WINDOW-PAGE_SIZE.

Bits of entropy with 64K page size on BOOK3S_64:

        bits of entropy = log2(DEFAULT_MAP_WINDOW_USER64 / PAGE_SIZE)

        PAGE_SIZE=64K, DEFAULT_MAP_WINDOW_USER64=128TB
        bits of entropy = log2(128TB / 64K)
        bits of entropy = 31

The upper limit is DEFAULT_MAP_WINDOW due to how the Book3s64 Hash
MMU
operates - by default the space above DEFAULT_MAP_WINDOW is not
available. Currently the Hash MMU does not use a temporary mm so
technically this upper limit isn't necessary; however, a larger
randomization range does not further "harden" this overall approach
and
future work may introduce patching with a temporary mm on Hash as
well.

Randomization occurs only once during initialization for each CPU as
it
comes online.

The patching page is mapped with PAGE_KERNEL to set EAA[0] for the
PTE
which ignores the AMR (so no need to unlock/lock KUAP) according to
PowerISA v3.0b Figure 35 on Radix.

Based on x86 implementation:

commit 4fc19708b165
("x86/alternatives: Initialize temporary mm for patching")

and:

commit b3fd8e83ada0
("x86/alternatives: Use temporary mm for text poking")

---

Is the section following the --- your addendum to Chris' patch?  That
cuts it off from git, including your signoff.  It'd be better to have
it together as one commit message and note the bits you contributed
below the --- after your signoff.

Commits where you're modifying someone else's previous work should
include their signoff above yours, as well.

Synchronisation is done according to Book 3 Chapter 13

might want to mention the ISA version alongside this, since chapter
numbering can change

Is this a useful abstraction?  This looks like a struct that used to
have more in it but is no longer necessary.

+
+static bool mm_patch_enabled(void)
+{
+       return IS_ENABLED(CONFIG_SMP) && radix_enabled();
+}
+
+/*
+ * The following applies for Radix MMU. Hash MMU has different
requirements,
+ * and so is not supported.
+ *
+ * Changing mm requires context synchronising instructions on both
sides of
+ * the context switch, as well as a hwsync between the last
instruction for
+ * which the address of an associated storage access was translated
using
+ * the current context.
+ *
+ * switch_mm_irqs_off performs an isync after the context switch. It
is

I'd prefer having parens here (switch_mm_irqs_off()) but I dunno if
that's actually a style guideline.

Reverse christmas tree?  If we care

Rest looks good to me.

On Fri, 2022-10-21 at 16:22 +1100, Benjamin Gray wrote:

This misses a definition for PPC_8xx which leads to a build failure as
found by snowpatch here:
https://github.com/ruscur/linux-ci/actions/runs/3295033018/jobs/5433162658#step:4:116

 
 extern void __local_flush_tlb_page(struct mm_struct *mm, unsigned
long vmaddr,
                                   int tsize, int ind);

On Mon, 2022-10-24 at 14:45 +1100, Russell Currey wrote:

On Fri, 2022-10-21 at 16:22 +1100, Benjamin Gray wrote:

quoted

From: "Christopher M. Riedl" <redacted>

x86 supports the notion of a temporary mm which restricts access to
temporary PTEs to a single CPU. A temporary mm is useful for
situations
where a CPU needs to perform sensitive operations (such as patching
a
STRICT_KERNEL_RWX kernel) requiring temporary mappings without
exposing
said mappings to other CPUs. Another benefit is that other CPU TLBs
do
not need to be flushed when the temporary mm is torn down.

Mappings in the temporary mm can be set in the userspace portion of
the
address-space.

Interrupts must be disabled while the temporary mm is in use. HW
breakpoints, which may have been set by userspace as watchpoints on
addresses now within the temporary mm, are saved and disabled when
loading the temporary mm. The HW breakpoints are restored when
unloading
the temporary mm. All HW breakpoints are indiscriminately disabled
while
the temporary mm is in use - this may include breakpoints set by
perf.

Use the `poking_init` init hook to prepare a temporary mm and
patching
address. Initialize the temporary mm by copying the init mm. Choose
a
randomized patching address inside the temporary mm userspace
address
space. The patching address is randomized between PAGE_SIZE and
DEFAULT_MAP_WINDOW-PAGE_SIZE.

Bits of entropy with 64K page size on BOOK3S_64:

        bits of entropy = log2(DEFAULT_MAP_WINDOW_USER64 /
PAGE_SIZE)

        PAGE_SIZE=64K, DEFAULT_MAP_WINDOW_USER64=128TB
        bits of entropy = log2(128TB / 64K)
        bits of entropy = 31

The upper limit is DEFAULT_MAP_WINDOW due to how the Book3s64 Hash
MMU
operates - by default the space above DEFAULT_MAP_WINDOW is not
available. Currently the Hash MMU does not use a temporary mm so
technically this upper limit isn't necessary; however, a larger
randomization range does not further "harden" this overall approach
and
future work may introduce patching with a temporary mm on Hash as
well.

Randomization occurs only once during initialization for each CPU
as
it
comes online.

The patching page is mapped with PAGE_KERNEL to set EAA[0] for the
PTE
which ignores the AMR (so no need to unlock/lock KUAP) according to
PowerISA v3.0b Figure 35 on Radix.

Based on x86 implementation:

commit 4fc19708b165
("x86/alternatives: Initialize temporary mm for patching")

and:

commit b3fd8e83ada0
("x86/alternatives: Use temporary mm for text poking")

---

Is the section following the --- your addendum to Chris' patch?  That
cuts it off from git, including your signoff.  It'd be better to have
it together as one commit message and note the bits you contributed
below the --- after your signoff.

Commits where you're modifying someone else's previous work should
include their signoff above yours, as well.

Addendum to his wording, to break it off from the "From..." section
(which is me splicing together his comments from previous patches with
some minor changes to account for the patch changes). I found out
earlier today that Git will treat it as a comment :(

I'll add the signed off by back, I wasn't sure whether to leave it
there after making changes (same in patch 2).
 

quoted

+static int __do_patch_instruction_mm(u32 *addr, ppc_inst_t instr)
+{
+       int err;
+       u32 *patch_addr;
+       unsigned long text_poke_addr;
+       pte_t *pte;
+       unsigned long pfn = get_patch_pfn(addr);
+       struct mm_struct *patching_mm;
+       struct temp_mm_state prev;

Reverse christmas tree?  If we care

Currently it's mirroring the __do_patch_instruction declarations, with
extra ones at the bottom.

On Mon, 2022-10-24 at 14:30 +1100, Russell Currey wrote:

On Fri, 2022-10-21 at 16:22 +1100, Benjamin Gray wrote:

quoted

Adds a local TLB flush operation that works given an mm_struct, VA
to
flush, and page size representation.

This removes the need to create a vm_area_struct, which the
temporary
patching mm work does not need.

Signed-off-by: Benjamin Gray <redacted>
---
 arch/powerpc/include/asm/book3s/32/tlbflush.h      | 9 +++++++++
 arch/powerpc/include/asm/book3s/64/tlbflush-hash.h | 5 +++++
 arch/powerpc/include/asm/book3s/64/tlbflush.h      | 8 ++++++++
 arch/powerpc/include/asm/nohash/tlbflush.h         | 1 +
 4 files changed, 23 insertions(+)

diff --git a/arch/powerpc/include/asm/book3s/32/tlbflush.h

b/arch/powerpc/include/asm/book3s/32/tlbflush.h
index ba1743c52b56..e5a688cebf69 100644

--- a/arch/powerpc/include/asm/book3s/32/tlbflush.h
+++ b/arch/powerpc/include/asm/book3s/32/tlbflush.h

@@ -2,6 +2,8 @@

 #ifndef _ASM_POWERPC_BOOK3S_32_TLBFLUSH_H
 #define _ASM_POWERPC_BOOK3S_32_TLBFLUSH_H
 
+#include <linux/build_bug.h>
+
 #define MMU_NO_CONTEXT      (0)
 /*
  * TLB flushing for "classic" hash-MMU 32-bit CPUs, 6xx, 7xx, 7xxx

@@ -74,6 +76,13 @@ static inline void local_flush_tlb_page(struct

vm_area_struct *vma,
 {
        flush_tlb_page(vma, vmaddr);
 }
+
+static inline void local_flush_tlb_page_psize(struct mm_struct
*mm,
unsigned long vmaddr, int psize)
+{
+       BUILD_BUG_ON(psize != MMU_PAGE_4K);

Is there any utility in adding this for 32bit if the following
patches
are only for Radix?

It needs some kind of definition to avoid #ifdef's. I figured I may as
well provide a correct implementation, given the functions around it
are implemented. The BUILD_BUG_ON specifically is just defensive in
case my assumptions are wrong. I don't know anything about these
machines, just what the kernel defines. I can remove the check, or
replace the whole implementation with a BUILD_BUG?

Hi Benjamin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on 8636df94ec917019c4cb744ba0a1f94cf9057790]

url:    https://github.com/intel-lab-lkp/linux/commits/Benjamin-Gray/Use-per-CPU-temporary-mappings-for-patching/20221021-133129
base:   8636df94ec917019c4cb744ba0a1f94cf9057790
patch link:    https://lore.kernel.org/r/20221021052238.580986-6-bgray%40linux.ibm.com
patch subject: [PATCH v8 5/6] powerpc/code-patching: Use temporary mm for Radix MMU
config: powerpc-tqm8xx_defconfig
compiler: powerpc-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/13c3eee268f72a6f6b47b978b3c472b8bed253d9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Benjamin-Gray/Use-per-CPU-temporary-mappings-for-patching/20221021-133129
        git checkout 13c3eee268f72a6f6b47b978b3c472b8bed253d9
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot [off-list ref]

All errors (new ones prefixed by >>):

   arch/powerpc/lib/code-patching.c: In function '__do_patch_instruction_mm':

quoted

arch/powerpc/lib/code-patching.c:355:9: error: implicit declaration of function 'local_flush_tlb_page_psize'; did you mean 'local_flush_tlb_page'? [-Werror=implicit-function-declaration]

     355 |         local_flush_tlb_page_psize(patching_mm, text_poke_addr, mmu_virtual_psize);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
         |         local_flush_tlb_page
   cc1: all warnings being treated as errors


vim +355 arch/powerpc/lib/code-patching.c

   312	
   313	static int __do_patch_instruction_mm(u32 *addr, ppc_inst_t instr)
   314	{
   315		int err;
   316		u32 *patch_addr;
   317		unsigned long text_poke_addr;
   318		pte_t *pte;
   319		unsigned long pfn = get_patch_pfn(addr);
   320		struct mm_struct *patching_mm;
   321		struct temp_mm_state prev;
   322	
   323		patching_mm = __this_cpu_read(cpu_patching_mm);
   324		pte = __this_cpu_read(cpu_patching_pte);
   325		text_poke_addr = __this_cpu_read(cpu_patching_addr);
   326		patch_addr = (u32 *)(text_poke_addr + offset_in_page(addr));
   327	
   328		if (unlikely(!patching_mm))
   329			return -ENOMEM;
   330	
   331		set_pte_at(patching_mm, text_poke_addr, pte, pfn_pte(pfn, PAGE_KERNEL));
   332	
   333		/* order PTE update before use, also serves as the hwsync */
   334		asm volatile("ptesync": : :"memory");
   335	
   336		/* order context switch after arbitrary prior code */
   337		isync();
   338	
   339		prev = start_using_temp_mm(patching_mm);
   340	
   341		err = __patch_instruction(addr, instr, patch_addr);
   342	
   343		/* hwsync performed by __patch_instruction (sync) if successful */
   344		if (err)
   345			mb();  /* sync */
   346	
   347		/* context synchronisation performed by __patch_instruction (isync or exception) */
   348		stop_using_temp_mm(patching_mm, prev);
   349	
   350		pte_clear(patching_mm, text_poke_addr, pte);
   351		/*
   352		 * ptesync to order PTE update before TLB invalidation done
   353		 * by radix__local_flush_tlb_page_psize (in _tlbiel_va)
   354		 */
 > 355		local_flush_tlb_page_psize(patching_mm, text_poke_addr, mmu_virtual_psize);
   356	
   357		return err;
   358	}
   359	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

On Mon Oct 24, 2022 at 12:17 AM CDT, Benjamin Gray wrote:

On Mon, 2022-10-24 at 14:45 +1100, Russell Currey wrote:

quoted

On Fri, 2022-10-21 at 16:22 +1100, Benjamin Gray wrote:

quoted

From: "Christopher M. Riedl" <redacted>

-----%<------

quoted

quoted

---

Is the section following the --- your addendum to Chris' patch?  That
cuts it off from git, including your signoff.  It'd be better to have
it together as one commit message and note the bits you contributed
below the --- after your signoff.

Commits where you're modifying someone else's previous work should
include their signoff above yours, as well.

Addendum to his wording, to break it off from the "From..." section
(which is me splicing together his comments from previous patches with
some minor changes to account for the patch changes). I found out
earlier today that Git will treat it as a comment :(

I'll add the signed off by back, I wasn't sure whether to leave it
there after making changes (same in patch 2).
 

This commit has lots of my words so should probably keep the sign-off - if only
to guarantee that blame is properly directed at me for any nonsense therein ^^.

Patch 2 probably doesn't need my sign-off any more - iirc, I actually defended
the BUG_ON()s (which are WARN_ON()s now) at some point.

On Mon, 2022-10-24 at 14:20 +1100, Russell Currey wrote:

On Fri, 2022-10-21 at 16:22 +1100, Benjamin Gray wrote:

quoted

diff --git a/arch/powerpc/lib/code-patching.c

b/arch/powerpc/lib/code-patching.c
index 34fc7ac34d91..9b9eba574d7e 100644

--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c

@@ -186,6 +186,8 @@ static int do_patch_instruction(u32 *addr,

ppc_inst_t instr)
        err = __do_patch_instruction(addr, instr);
        local_irq_restore(flags);
 
+       WARN_ON(!err && !ppc_inst_equal(instr,
ppc_inst_read(addr)));
+

As a side note, I had a look at test-code-patching.c and it doesn't
look like we don't have a test for ppc_inst_equal() with prefixed
instructions.  We should fix that.

Yeah, for a different series though I assume. And I think it would be
better suited in a suite dedicated to testing asm/inst.h functions.