* [PATCH v2 0/2] Support fixmap and jump_label for openrisc
@ 2025-08-06 2:05 ChenMiao
2025-08-06 2:05 ` [PATCH v2 1/2] openrisc: Add text patching API support ChenMiao
2025-08-06 2:05 ` [PATCH v2 2/2] openrisc: Add jump label support ChenMiao
0 siblings, 2 replies; 7+ messages in thread
From: ChenMiao @ 2025-08-06 2:05 UTC (permalink / raw)
To: Linux Kernel; +Cc: Linux OpenRISC, chenmiao
From: chenmiao <chenmiao.ku@gmail.com>
Hello everyone,
This implements a basic text patching mechanism and a complete jump_label
implementation for OpenRISC.
1. Implement the patch_map interface and the patch_insn_write single insn
write API by providing FIX_TEXT_POKE0. And create a new insn-def.h to record
openrisc's instruction macro definition which only have OPENRISC_INSN_SIZE
now.
- V2: modify the patch_insn_write API from const void* to u32 insn.
2. Based on the previously implemented patch_insn_write, achieve a complete
jump_label implementation, directly supporting arch_jump_label_transform_queue.
And add a new macro OPENRISC_INSN_NOP in insn-def.h.
- V2: using the modified patch_insn_write API in
arch_jump_label_transform_queue.
Link: https://lore.kernel.org/openrisc/20250805084926.4125564-1-chenmiao.ku@gmail.com/T/#u
Thanks,
Chen Miao
chenmiao (2):
openrisc: Add text patching API support
openrisc: Add jump label support
.../core/jump-labels/arch-support.txt | 2 +-
arch/openrisc/Kconfig | 2 +
arch/openrisc/configs/or1ksim_defconfig | 19 ++---
arch/openrisc/configs/virt_defconfig | 1 +
arch/openrisc/include/asm/Kbuild | 1 -
arch/openrisc/include/asm/fixmap.h | 1 +
arch/openrisc/include/asm/insn-def.h | 15 ++++
arch/openrisc/include/asm/jump_label.h | 68 ++++++++++++++++
arch/openrisc/include/asm/text-patching.h | 13 +++
arch/openrisc/kernel/Makefile | 2 +
arch/openrisc/kernel/jump_label.c | 53 +++++++++++++
arch/openrisc/kernel/patching.c | 79 +++++++++++++++++++
arch/openrisc/kernel/setup.c | 2 +
arch/openrisc/mm/init.c | 2 +-
14 files changed, 245 insertions(+), 15 deletions(-)
create mode 100644 arch/openrisc/include/asm/insn-def.h
create mode 100644 arch/openrisc/include/asm/jump_label.h
create mode 100644 arch/openrisc/include/asm/text-patching.h
create mode 100644 arch/openrisc/kernel/jump_label.c
create mode 100644 arch/openrisc/kernel/patching.c
--
2.45.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] openrisc: Add text patching API support
2025-08-06 2:05 [PATCH v2 0/2] Support fixmap and jump_label for openrisc ChenMiao
@ 2025-08-06 2:05 ` ChenMiao
2025-08-09 8:25 ` Stafford Horne
2025-08-06 2:05 ` [PATCH v2 2/2] openrisc: Add jump label support ChenMiao
1 sibling, 1 reply; 7+ messages in thread
From: ChenMiao @ 2025-08-06 2:05 UTC (permalink / raw)
To: Linux Kernel
Cc: Linux OpenRISC, chenmiao, Jonas Bonn, Stefan Kristiansson,
Stafford Horne, Mike Rapoport (Microsoft), Geert Uytterhoeven,
Arnd Bergmann, Luis Chamberlain, Sahil Siddiq, Johannes Berg,
Nicolas Schier, Masahiro Yamada, Dave Hansen, Andrew Morton
From: chenmiao <chenmiao.ku@gmail.com>
We need a text patching mechanism to ensure that in the subsequent
implementation of jump_label, the code can be modified to the correct
location. Therefore, FIX_TEXT_POKE0 has been added as a mapping area.
And, I create a new file named insn-def.h to define the or1k insn macro
size and more define in the future.
Among these changes, we implement patch_map and support the
patch_insn_write API for single instruction writing.
- V2: We modify the patch_insn_write(void *addr, const void *insn) API to
patch_insn_write(void *addr, u32 insn), derectly support a single u32
instruction write to map memory.
Link: https://lore.kernel.org/openrisc/aJIC8o1WmVHol9RY@antec/T/#t
Signed-off-by: chenmiao <chenmiao.ku@gmail.com>
---
arch/openrisc/include/asm/Kbuild | 1 -
arch/openrisc/include/asm/fixmap.h | 1 +
arch/openrisc/include/asm/insn-def.h | 12 ++++
arch/openrisc/include/asm/text-patching.h | 13 ++++
arch/openrisc/kernel/Makefile | 1 +
arch/openrisc/kernel/patching.c | 79 +++++++++++++++++++++++
arch/openrisc/mm/init.c | 2 +-
7 files changed, 107 insertions(+), 2 deletions(-)
create mode 100644 arch/openrisc/include/asm/insn-def.h
create mode 100644 arch/openrisc/include/asm/text-patching.h
create mode 100644 arch/openrisc/kernel/patching.c
diff --git a/arch/openrisc/include/asm/Kbuild b/arch/openrisc/include/asm/Kbuild
index 2b1a6b00cdac..cef49d60d74c 100644
--- a/arch/openrisc/include/asm/Kbuild
+++ b/arch/openrisc/include/asm/Kbuild
@@ -9,4 +9,3 @@ generic-y += spinlock.h
generic-y += qrwlock_types.h
generic-y += qrwlock.h
generic-y += user.h
-generic-y += text-patching.h
diff --git a/arch/openrisc/include/asm/fixmap.h b/arch/openrisc/include/asm/fixmap.h
index aaa6a26a3e92..74000215064d 100644
--- a/arch/openrisc/include/asm/fixmap.h
+++ b/arch/openrisc/include/asm/fixmap.h
@@ -28,6 +28,7 @@
enum fixed_addresses {
FIX_EARLYCON_MEM_BASE,
+ FIX_TEXT_POKE0,
__end_of_fixed_addresses
};
diff --git a/arch/openrisc/include/asm/insn-def.h b/arch/openrisc/include/asm/insn-def.h
new file mode 100644
index 000000000000..dc8d16db1579
--- /dev/null
+++ b/arch/openrisc/include/asm/insn-def.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2025 Chen Miao
+ */
+
+#ifndef __ASM_INSN_DEF_H
+#define __ASM_INSN_DEF_H
+
+/* or1k instructions are always 32 bits. */
+#define OPENRISC_INSN_SIZE 4
+
+#endif /* __ASM_INSN_DEF_H */
diff --git a/arch/openrisc/include/asm/text-patching.h b/arch/openrisc/include/asm/text-patching.h
new file mode 100644
index 000000000000..bffe828288c3
--- /dev/null
+++ b/arch/openrisc/include/asm/text-patching.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2025 Chen Miao
+ */
+
+#ifndef _ASM_PATCHING_H_
+#define _ASM_PATCHING_H_
+
+#include <linux/types.h>
+
+int patch_insn_write(void *addr, u32 insn);
+
+#endif /* _ASM_PATCHING_H_ */
diff --git a/arch/openrisc/kernel/Makefile b/arch/openrisc/kernel/Makefile
index 58e6a1b525b7..f0957ce16d6b 100644
--- a/arch/openrisc/kernel/Makefile
+++ b/arch/openrisc/kernel/Makefile
@@ -13,5 +13,6 @@ obj-$(CONFIG_SMP) += smp.o sync-timer.o
obj-$(CONFIG_STACKTRACE) += stacktrace.o
obj-$(CONFIG_MODULES) += module.o
obj-$(CONFIG_OF) += prom.o
+obj-y += patching.o
clean:
diff --git a/arch/openrisc/kernel/patching.c b/arch/openrisc/kernel/patching.c
new file mode 100644
index 000000000000..c9a30f0d1193
--- /dev/null
+++ b/arch/openrisc/kernel/patching.c
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (C) 2020 SiFive
+ * Copyright (C) 2025 Chen Miao
+ */
+
+#include <linux/mm.h>
+#include <linux/kernel.h>
+#include <linux/spinlock.h>
+#include <linux/uaccess.h>
+
+#include <asm/insn-def.h>
+#include <asm/cacheflush.h>
+#include <asm/page.h>
+#include <asm/fixmap.h>
+#include <asm/text-patching.h>
+#include <asm/sections.h>
+
+static DEFINE_RAW_SPINLOCK(patch_lock);
+
+static inline bool is_exit_text(uintptr_t addr)
+{
+ /* Now Have NO Mechanism to do */
+ return true;
+}
+
+static __always_inline void *patch_map(void *addr, int fixmap)
+{
+ uintptr_t uaddr = (uintptr_t) addr;
+ phys_addr_t phys;
+
+ if (core_kernel_text(uaddr) || is_exit_text(uaddr)) {
+ phys = __pa_symbol(addr);
+ } else {
+ struct page *page = vmalloc_to_page(addr);
+ BUG_ON(!page);
+ phys = page_to_phys(page) + offset_in_page(addr);
+ }
+
+ return (void *)set_fixmap_offset(fixmap, phys);
+}
+
+static void patch_unmap(int fixmap)
+{
+ clear_fixmap(fixmap);
+}
+
+static int __patch_insn_write(void *addr, u32 insn)
+{
+ void *waddr = addr;
+ unsigned long flags = 0;
+ int ret;
+
+ raw_spin_lock_irqsave(&patch_lock, flags);
+
+ waddr = patch_map(addr, FIX_TEXT_POKE0);
+
+ ret = copy_to_kernel_nofault(waddr, &insn, OPENRISC_INSN_SIZE);
+ local_icache_range_inv((unsigned long)waddr,
+ (unsigned long)waddr + OPENRISC_INSN_SIZE);
+
+ patch_unmap(FIX_TEXT_POKE0);
+
+ raw_spin_unlock_irqrestore(&patch_lock, flags);
+
+ return ret;
+}
+
+int patch_insn_write(void *addr, u32 insn)
+{
+ u32 *tp = addr;
+ int ret;
+
+ if ((uintptr_t) tp & 0x3)
+ return -EINVAL;
+
+ ret = __patch_insn_write(tp, insn);
+
+ return ret;
+}
diff --git a/arch/openrisc/mm/init.c b/arch/openrisc/mm/init.c
index e4904ca6f0a0..b5925710f954 100644
--- a/arch/openrisc/mm/init.c
+++ b/arch/openrisc/mm/init.c
@@ -226,7 +226,7 @@ static int __init map_page(unsigned long va, phys_addr_t pa, pgprot_t prot)
return 0;
}
-void __init __set_fixmap(enum fixed_addresses idx,
+void __set_fixmap(enum fixed_addresses idx,
phys_addr_t phys, pgprot_t prot)
{
unsigned long address = __fix_to_virt(idx);
--
2.45.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] openrisc: Add jump label support
2025-08-06 2:05 [PATCH v2 0/2] Support fixmap and jump_label for openrisc ChenMiao
2025-08-06 2:05 ` [PATCH v2 1/2] openrisc: Add text patching API support ChenMiao
@ 2025-08-06 2:05 ` ChenMiao
2025-08-09 8:37 ` Stafford Horne
1 sibling, 1 reply; 7+ messages in thread
From: ChenMiao @ 2025-08-06 2:05 UTC (permalink / raw)
To: Linux Kernel
Cc: Linux OpenRISC, chenmiao, Jonathan Corbet, Jonas Bonn,
Stefan Kristiansson, Stafford Horne, Peter Zijlstra,
Josh Poimboeuf, Jason Baron, Steven Rostedt, Ard Biesheuvel,
Masahiro Yamada, Sahil Siddiq, Nicolas Schier, Johannes Berg,
linux-doc
From: chenmiao <chenmiao.ku@gmail.com>
Implemented the full functionality of jump_label, of course,
with text patching supported by just one API.
By the way, add new macro OPENRISC_INSN_NOP in insn-def.h to use.
- V2: using the patch_insn_write(void *addr, u32 insn) not the
const void *insn.
Link: https://lore.kernel.org/openrisc/aJIC8o1WmVHol9RY@antec/T/#t
Signed-off-by: chenmiao <chenmiao.ku@gmail.com>
---
.../core/jump-labels/arch-support.txt | 2 +-
arch/openrisc/Kconfig | 2 +
arch/openrisc/configs/or1ksim_defconfig | 19 ++----
arch/openrisc/configs/virt_defconfig | 1 +
arch/openrisc/include/asm/insn-def.h | 3 +
arch/openrisc/include/asm/jump_label.h | 68 +++++++++++++++++++
arch/openrisc/kernel/Makefile | 1 +
arch/openrisc/kernel/jump_label.c | 53 +++++++++++++++
arch/openrisc/kernel/setup.c | 2 +
9 files changed, 138 insertions(+), 13 deletions(-)
create mode 100644 arch/openrisc/include/asm/jump_label.h
create mode 100644 arch/openrisc/kernel/jump_label.c
diff --git a/Documentation/features/core/jump-labels/arch-support.txt b/Documentation/features/core/jump-labels/arch-support.txt
index ccada815569f..683de7c15058 100644
--- a/Documentation/features/core/jump-labels/arch-support.txt
+++ b/Documentation/features/core/jump-labels/arch-support.txt
@@ -17,7 +17,7 @@
| microblaze: | TODO |
| mips: | ok |
| nios2: | TODO |
- | openrisc: | TODO |
+ | openrisc: | ok |
| parisc: | ok |
| powerpc: | ok |
| riscv: | ok |
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index b38fee299bc4..9156635dd264 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -24,6 +24,8 @@ config OPENRISC
select GENERIC_PCI_IOMAP
select GENERIC_IOREMAP
select GENERIC_CPU_DEVICES
+ select HAVE_ARCH_JUMP_LABEL
+ select HAVE_ARCH_JUMP_LABEL_RELATIVE
select HAVE_PCI
select HAVE_UID16
select HAVE_PAGE_SIZE_8KB
diff --git a/arch/openrisc/configs/or1ksim_defconfig b/arch/openrisc/configs/or1ksim_defconfig
index 59fe33cefba2..769705ac24d5 100644
--- a/arch/openrisc/configs/or1ksim_defconfig
+++ b/arch/openrisc/configs/or1ksim_defconfig
@@ -3,26 +3,23 @@ CONFIG_LOG_BUF_SHIFT=14
CONFIG_BLK_DEV_INITRD=y
# CONFIG_RD_GZIP is not set
CONFIG_EXPERT=y
-# CONFIG_KALLSYMS is not set
# CONFIG_EPOLL is not set
# CONFIG_TIMERFD is not set
# CONFIG_EVENTFD is not set
# CONFIG_AIO is not set
-# CONFIG_VM_EVENT_COUNTERS is not set
-# CONFIG_COMPAT_BRK is not set
-CONFIG_SLUB=y
-CONFIG_SLUB_TINY=y
-CONFIG_MODULES=y
-# CONFIG_BLOCK is not set
+# CONFIG_KALLSYMS is not set
CONFIG_BUILTIN_DTB_NAME="or1ksim"
CONFIG_HZ_100=y
+CONFIG_JUMP_LABEL=y
+CONFIG_MODULES=y
+# CONFIG_BLOCK is not set
+CONFIG_SLUB_TINY=y
+# CONFIG_COMPAT_BRK is not set
+# CONFIG_VM_EVENT_COUNTERS is not set
CONFIG_NET=y
CONFIG_PACKET=y
CONFIG_UNIX=y
CONFIG_INET=y
-# CONFIG_INET_XFRM_MODE_TRANSPORT is not set
-# CONFIG_INET_XFRM_MODE_TUNNEL is not set
-# CONFIG_INET_XFRM_MODE_BEET is not set
# CONFIG_INET_DIAG is not set
CONFIG_TCP_CONG_ADVANCED=y
# CONFIG_TCP_CONG_BIC is not set
@@ -35,7 +32,6 @@ CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y
# CONFIG_PREVENT_FIRMWARE_BUILD is not set
# CONFIG_FW_LOADER is not set
-CONFIG_PROC_DEVICETREE=y
CONFIG_NETDEVICES=y
CONFIG_ETHOC=y
CONFIG_MICREL_PHY=y
@@ -53,4 +49,3 @@ CONFIG_SERIAL_OF_PLATFORM=y
# CONFIG_DNOTIFY is not set
CONFIG_TMPFS=y
CONFIG_NFS_FS=y
-# CONFIG_ENABLE_MUST_CHECK is not set
diff --git a/arch/openrisc/configs/virt_defconfig b/arch/openrisc/configs/virt_defconfig
index c1b69166c500..4a80c5794877 100644
--- a/arch/openrisc/configs/virt_defconfig
+++ b/arch/openrisc/configs/virt_defconfig
@@ -12,6 +12,7 @@ CONFIG_NR_CPUS=8
CONFIG_SMP=y
CONFIG_HZ_100=y
# CONFIG_OPENRISC_NO_SPR_SR_DSX is not set
+CONFIG_JUMP_LABEL=y
# CONFIG_COMPAT_BRK is not set
CONFIG_NET=y
CONFIG_PACKET=y
diff --git a/arch/openrisc/include/asm/insn-def.h b/arch/openrisc/include/asm/insn-def.h
index dc8d16db1579..2ccdbb37c27c 100644
--- a/arch/openrisc/include/asm/insn-def.h
+++ b/arch/openrisc/include/asm/insn-def.h
@@ -9,4 +9,7 @@
/* or1k instructions are always 32 bits. */
#define OPENRISC_INSN_SIZE 4
+/* or1k nop instruction code */
+#define OPENRISC_INSN_NOP 0x15000000U
+
#endif /* __ASM_INSN_DEF_H */
diff --git a/arch/openrisc/include/asm/jump_label.h b/arch/openrisc/include/asm/jump_label.h
new file mode 100644
index 000000000000..03afca9c3a1f
--- /dev/null
+++ b/arch/openrisc/include/asm/jump_label.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2025 Chen Miao
+ *
+ * Based on arch/arm/include/asm/jump_label.h
+ */
+#ifndef __ASM_JUMP_LABEL_H
+#define __ASM_JUMP_LABEL_H
+
+#ifndef __ASSEMBLY__
+
+#include <linux/types.h>
+#include <asm/insn-def.h>
+
+#define HAVE_JUMP_LABEL_BATCH
+
+#define JUMP_LABEL_NOP_SIZE OPENRISC_INSN_SIZE
+
+/*
+ * should aligned 4
+ * for jump_label relative
+ * entry.code = nop.addr - . -> return false
+ * entry.target = l_yes - . -> return true
+ * entry.key = key - .
+ */
+#define JUMP_TABLE_ENTRY(key, label) \
+ ".pushsection __jump_table, \"aw\" \n\t" \
+ ".align 4 \n\t" \
+ ".long 1b - ., " label " - . \n\t" \
+ ".long " key " - . \n\t" \
+ ".popsection \n\t"
+
+#define ARCH_STATIC_BRANCH_ASM(key, label) \
+ ".align 4 \n\t" \
+ "1: l.nop \n\t" \
+ " l.nop \n\t" \
+ JUMP_TABLE_ENTRY(key, label)
+
+static __always_inline bool arch_static_branch(struct static_key *const key,
+ const bool branch)
+{
+ asm goto (ARCH_STATIC_BRANCH_ASM("%0", "%l[l_yes]")
+ ::"i"(&((char *)key)[branch])::l_yes);
+
+ return false;
+l_yes:
+ return true;
+}
+
+#define ARCH_STATIC_BRANCH_JUMP_ASM(key, label) \
+ ".align 4 \n\t" \
+ "1: l.j " label " \n\t" \
+ " l.nop \n\t" \
+ JUMP_TABLE_ENTRY(key, label)
+
+static __always_inline bool
+arch_static_branch_jump(struct static_key *const key, const bool branch)
+{
+ asm goto (ARCH_STATIC_BRANCH_JUMP_ASM("%0", "%l[l_yes]")
+ ::"i"(&((char *)key)[branch])::l_yes);
+
+ return false;
+l_yes:
+ return true;
+}
+
+#endif /* __ASSEMBLY__ */
+#endif /* __ASM_JUMP_LABEL_H */
diff --git a/arch/openrisc/kernel/Makefile b/arch/openrisc/kernel/Makefile
index f0957ce16d6b..19e0eb94f2eb 100644
--- a/arch/openrisc/kernel/Makefile
+++ b/arch/openrisc/kernel/Makefile
@@ -9,6 +9,7 @@ obj-y := head.o setup.o or32_ksyms.o process.o dma.o \
traps.o time.o irq.o entry.o ptrace.o signal.o \
sys_call_table.o unwinder.o cacheinfo.o
+obj-$(CONFIG_JUMP_LABEL) += jump_label.o
obj-$(CONFIG_SMP) += smp.o sync-timer.o
obj-$(CONFIG_STACKTRACE) += stacktrace.o
obj-$(CONFIG_MODULES) += module.o
diff --git a/arch/openrisc/kernel/jump_label.c b/arch/openrisc/kernel/jump_label.c
new file mode 100644
index 000000000000..ce259ba30258
--- /dev/null
+++ b/arch/openrisc/kernel/jump_label.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025 Chen Miao
+ *
+ * Based on arch/arm/kernel/jump_label.c
+ */
+#include <linux/jump_label.h>
+#include <linux/kernel.h>
+#include <linux/memory.h>
+#include <asm/bug.h>
+#include <asm/cacheflush.h>
+#include <asm/text-patching.h>
+
+bool arch_jump_label_transform_queue(struct jump_entry *entry,
+ enum jump_label_type type)
+{
+ void *addr = (void *)jump_entry_code(entry);
+ u32 insn;
+
+ if (type == JUMP_LABEL_JMP) {
+ long offset;
+
+ offset = jump_entry_target(entry) - jump_entry_code(entry);
+ /*
+ * The actual maximum range of the l.j instruction's offset is -134,217,728
+ * ~ 134,217,724 (sign 26-bit imm).
+ * For the original jump range, we need to right-shift N by 2 to obtain the
+ * instruction's offset.
+ */
+ if (unlikely(offset < -134217728 || offset > 134217724)) {
+ WARN_ON_ONCE(true);
+ }
+ /* 26bit imm mask */
+ offset = (offset >> 2) & 0x03ffffff;
+
+ insn = offset;
+ } else {
+ insn = OPENRISC_INSN_NOP;
+ }
+
+ if (early_boot_irqs_disabled) {
+ copy_to_kernel_nofault(addr, &insn, sizeof(insn));
+ } else {
+ patch_insn_write(addr, insn);
+ }
+ return true;
+}
+
+void arch_jump_label_transform_apply(void)
+{
+ // flush
+ kick_all_cpus_sync();
+}
diff --git a/arch/openrisc/kernel/setup.c b/arch/openrisc/kernel/setup.c
index a9fb9cc6779e..000a9cc10e6f 100644
--- a/arch/openrisc/kernel/setup.c
+++ b/arch/openrisc/kernel/setup.c
@@ -249,6 +249,8 @@ void __init setup_arch(char **cmdline_p)
initrd_below_start_ok = 1;
}
#endif
+ /* perform jump_table sorting before paging_init locks down read only memory */
+ jump_label_init();
/* paging_init() sets up the MMU and marks all pages as reserved */
paging_init();
--
2.45.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] openrisc: Add text patching API support
2025-08-06 2:05 ` [PATCH v2 1/2] openrisc: Add text patching API support ChenMiao
@ 2025-08-09 8:25 ` Stafford Horne
2025-08-10 14:52 ` Miao Chen
0 siblings, 1 reply; 7+ messages in thread
From: Stafford Horne @ 2025-08-09 8:25 UTC (permalink / raw)
To: ChenMiao
Cc: Linux Kernel, Linux OpenRISC, Jonas Bonn, Stefan Kristiansson,
Mike Rapoport (Microsoft), Geert Uytterhoeven, Arnd Bergmann,
Luis Chamberlain, Sahil Siddiq, Johannes Berg, Nicolas Schier,
Masahiro Yamada, Dave Hansen, Andrew Morton
On Wed, Aug 06, 2025 at 02:05:03AM +0000, ChenMiao wrote:
> From: chenmiao <chenmiao.ku@gmail.com>
>
> We need a text patching mechanism to ensure that in the subsequent
> implementation of jump_label, the code can be modified to the correct
> location. Therefore, FIX_TEXT_POKE0 has been added as a mapping area.
>
> And, I create a new file named insn-def.h to define the or1k insn macro
> size and more define in the future.
>
> Among these changes, we implement patch_map and support the
> patch_insn_write API for single instruction writing.
>
> - V2: We modify the patch_insn_write(void *addr, const void *insn) API to
> patch_insn_write(void *addr, u32 insn), derectly support a single u32
> instruction write to map memory.
>
> Link: https://lore.kernel.org/openrisc/aJIC8o1WmVHol9RY@antec/T/#t
>
> Signed-off-by: chenmiao <chenmiao.ku@gmail.com>
> ---
The v2, notes should go in this part of the patch (after ---), this way
it does not show up when I queue the patches.
> arch/openrisc/include/asm/Kbuild | 1 -
> arch/openrisc/include/asm/fixmap.h | 1 +
> arch/openrisc/include/asm/insn-def.h | 12 ++++
> arch/openrisc/include/asm/text-patching.h | 13 ++++
> arch/openrisc/kernel/Makefile | 1 +
> arch/openrisc/kernel/patching.c | 79 +++++++++++++++++++++++
> arch/openrisc/mm/init.c | 2 +-
> 7 files changed, 107 insertions(+), 2 deletions(-)
> create mode 100644 arch/openrisc/include/asm/insn-def.h
> create mode 100644 arch/openrisc/include/asm/text-patching.h
> create mode 100644 arch/openrisc/kernel/patching.c
>
> diff --git a/arch/openrisc/include/asm/Kbuild b/arch/openrisc/include/asm/Kbuild
> index 2b1a6b00cdac..cef49d60d74c 100644
> --- a/arch/openrisc/include/asm/Kbuild
> +++ b/arch/openrisc/include/asm/Kbuild
> @@ -9,4 +9,3 @@ generic-y += spinlock.h
> generic-y += qrwlock_types.h
> generic-y += qrwlock.h
> generic-y += user.h
> -generic-y += text-patching.h
> diff --git a/arch/openrisc/include/asm/fixmap.h b/arch/openrisc/include/asm/fixmap.h
> index aaa6a26a3e92..74000215064d 100644
> --- a/arch/openrisc/include/asm/fixmap.h
> +++ b/arch/openrisc/include/asm/fixmap.h
> @@ -28,6 +28,7 @@
>
> enum fixed_addresses {
> FIX_EARLYCON_MEM_BASE,
> + FIX_TEXT_POKE0,
> __end_of_fixed_addresses
> };
>
> diff --git a/arch/openrisc/include/asm/insn-def.h b/arch/openrisc/include/asm/insn-def.h
> new file mode 100644
> index 000000000000..dc8d16db1579
> --- /dev/null
> +++ b/arch/openrisc/include/asm/insn-def.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2025 Chen Miao
> + */
> +
> +#ifndef __ASM_INSN_DEF_H
> +#define __ASM_INSN_DEF_H
> +
> +/* or1k instructions are always 32 bits. */
> +#define OPENRISC_INSN_SIZE 4
Is this used outside of the usage below? Why do we need a header for it?
> +#endif /* __ASM_INSN_DEF_H */
> diff --git a/arch/openrisc/include/asm/text-patching.h b/arch/openrisc/include/asm/text-patching.h
> new file mode 100644
> index 000000000000..bffe828288c3
> --- /dev/null
> +++ b/arch/openrisc/include/asm/text-patching.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2025 Chen Miao
> + */
> +
> +#ifndef _ASM_PATCHING_H_
> +#define _ASM_PATCHING_H_
> +
> +#include <linux/types.h>
> +
> +int patch_insn_write(void *addr, u32 insn);
> +
> +#endif /* _ASM_PATCHING_H_ */
> diff --git a/arch/openrisc/kernel/Makefile b/arch/openrisc/kernel/Makefile
> index 58e6a1b525b7..f0957ce16d6b 100644
> --- a/arch/openrisc/kernel/Makefile
> +++ b/arch/openrisc/kernel/Makefile
> @@ -13,5 +13,6 @@ obj-$(CONFIG_SMP) += smp.o sync-timer.o
> obj-$(CONFIG_STACKTRACE) += stacktrace.o
> obj-$(CONFIG_MODULES) += module.o
> obj-$(CONFIG_OF) += prom.o
> +obj-y += patching.o
>
> clean:
> diff --git a/arch/openrisc/kernel/patching.c b/arch/openrisc/kernel/patching.c
> new file mode 100644
> index 000000000000..c9a30f0d1193
> --- /dev/null
> +++ b/arch/openrisc/kernel/patching.c
> @@ -0,0 +1,79 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (C) 2020 SiFive
> + * Copyright (C) 2025 Chen Miao
> + */
> +
> +#include <linux/mm.h>
> +#include <linux/kernel.h>
> +#include <linux/spinlock.h>
> +#include <linux/uaccess.h>
> +
> +#include <asm/insn-def.h>
> +#include <asm/cacheflush.h>
> +#include <asm/page.h>
> +#include <asm/fixmap.h>
> +#include <asm/text-patching.h>
> +#include <asm/sections.h>
> +
> +static DEFINE_RAW_SPINLOCK(patch_lock);
> +
> +static inline bool is_exit_text(uintptr_t addr)
> +{
> + /* Now Have NO Mechanism to do */
> + return true;
> +}
Is this needed? OpenRISC doesn't seem to have any sections to support __exit
annotated cleanup functions.
Ref: https://kernelnewbies.org/FAQ/InitExitMacros
Returning true here means that all of the patch_map calls will be treated as
kernel pages below though, which is wrong, please remove this.
> +
> +static __always_inline void *patch_map(void *addr, int fixmap)
> +{
> + uintptr_t uaddr = (uintptr_t) addr;
> + phys_addr_t phys;
> +
> + if (core_kernel_text(uaddr) || is_exit_text(uaddr)) {
> + phys = __pa_symbol(addr);
> + } else {
> + struct page *page = vmalloc_to_page(addr);
> + BUG_ON(!page);
> + phys = page_to_phys(page) + offset_in_page(addr);
> + }
> +
> + return (void *)set_fixmap_offset(fixmap, phys);
> +}
> +
> +static void patch_unmap(int fixmap)
> +{
> + clear_fixmap(fixmap);
> +}
> +
> +static int __patch_insn_write(void *addr, u32 insn)
> +{
> + void *waddr = addr;
> + unsigned long flags = 0;
> + int ret;
> +
> + raw_spin_lock_irqsave(&patch_lock, flags);
> +
> + waddr = patch_map(addr, FIX_TEXT_POKE0);
> +
> + ret = copy_to_kernel_nofault(waddr, &insn, OPENRISC_INSN_SIZE);
> + local_icache_range_inv((unsigned long)waddr,
> + (unsigned long)waddr + OPENRISC_INSN_SIZE);
Instead of OPENRISC_INSN_SIZE, can you use sizeof(insn)?
> + patch_unmap(FIX_TEXT_POKE0);
> +
> + raw_spin_unlock_irqrestore(&patch_lock, flags);
> +
> + return ret;
> +}
> +
> +int patch_insn_write(void *addr, u32 insn)
> +{
> + u32 *tp = addr;
> + int ret;
> +
> + if ((uintptr_t) tp & 0x3)
> + return -EINVAL;
> +
> + ret = __patch_insn_write(tp, insn);
> +
> + return ret;
> +}
> diff --git a/arch/openrisc/mm/init.c b/arch/openrisc/mm/init.c
> index e4904ca6f0a0..b5925710f954 100644
> --- a/arch/openrisc/mm/init.c
> +++ b/arch/openrisc/mm/init.c
> @@ -226,7 +226,7 @@ static int __init map_page(unsigned long va, phys_addr_t pa, pgprot_t prot)
> return 0;
> }
>
> -void __init __set_fixmap(enum fixed_addresses idx,
> +void __set_fixmap(enum fixed_addresses idx,
Can you put a comment about this bit in the commit description?
> phys_addr_t phys, pgprot_t prot)
> {
> unsigned long address = __fix_to_virt(idx);
> --
> 2.45.2
-Stafford
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] openrisc: Add jump label support
2025-08-06 2:05 ` [PATCH v2 2/2] openrisc: Add jump label support ChenMiao
@ 2025-08-09 8:37 ` Stafford Horne
2025-08-10 15:09 ` Miao Chen
0 siblings, 1 reply; 7+ messages in thread
From: Stafford Horne @ 2025-08-09 8:37 UTC (permalink / raw)
To: ChenMiao
Cc: Linux Kernel, Linux OpenRISC, Jonathan Corbet, Jonas Bonn,
Stefan Kristiansson, Peter Zijlstra, Josh Poimboeuf, Jason Baron,
Steven Rostedt, Ard Biesheuvel, Masahiro Yamada, Sahil Siddiq,
Nicolas Schier, Johannes Berg, linux-doc
On Wed, Aug 06, 2025 at 02:05:04AM +0000, ChenMiao wrote:
> From: chenmiao <chenmiao.ku@gmail.com>
>
> Implemented the full functionality of jump_label, of course,
> with text patching supported by just one API.
>
> By the way, add new macro OPENRISC_INSN_NOP in insn-def.h to use.
>
> - V2: using the patch_insn_write(void *addr, u32 insn) not the
> const void *insn.
>
> Link: https://lore.kernel.org/openrisc/aJIC8o1WmVHol9RY@antec/T/#t
>
> Signed-off-by: chenmiao <chenmiao.ku@gmail.com>
> ---
> .../core/jump-labels/arch-support.txt | 2 +-
> arch/openrisc/Kconfig | 2 +
> arch/openrisc/configs/or1ksim_defconfig | 19 ++----
> arch/openrisc/configs/virt_defconfig | 1 +
> arch/openrisc/include/asm/insn-def.h | 3 +
> arch/openrisc/include/asm/jump_label.h | 68 +++++++++++++++++++
> arch/openrisc/kernel/Makefile | 1 +
> arch/openrisc/kernel/jump_label.c | 53 +++++++++++++++
> arch/openrisc/kernel/setup.c | 2 +
> 9 files changed, 138 insertions(+), 13 deletions(-)
> create mode 100644 arch/openrisc/include/asm/jump_label.h
> create mode 100644 arch/openrisc/kernel/jump_label.c
>
> diff --git a/Documentation/features/core/jump-labels/arch-support.txt b/Documentation/features/core/jump-labels/arch-support.txt
> index ccada815569f..683de7c15058 100644
> --- a/Documentation/features/core/jump-labels/arch-support.txt
> +++ b/Documentation/features/core/jump-labels/arch-support.txt
> @@ -17,7 +17,7 @@
> | microblaze: | TODO |
> | mips: | ok |
> | nios2: | TODO |
> - | openrisc: | TODO |
> + | openrisc: | ok |
> | parisc: | ok |
> | powerpc: | ok |
> | riscv: | ok |
> diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
> index b38fee299bc4..9156635dd264 100644
> --- a/arch/openrisc/Kconfig
> +++ b/arch/openrisc/Kconfig
> @@ -24,6 +24,8 @@ config OPENRISC
> select GENERIC_PCI_IOMAP
> select GENERIC_IOREMAP
> select GENERIC_CPU_DEVICES
> + select HAVE_ARCH_JUMP_LABEL
> + select HAVE_ARCH_JUMP_LABEL_RELATIVE
> select HAVE_PCI
> select HAVE_UID16
> select HAVE_PAGE_SIZE_8KB
> diff --git a/arch/openrisc/configs/or1ksim_defconfig b/arch/openrisc/configs/or1ksim_defconfig
> index 59fe33cefba2..769705ac24d5 100644
> --- a/arch/openrisc/configs/or1ksim_defconfig
> +++ b/arch/openrisc/configs/or1ksim_defconfig
> @@ -3,26 +3,23 @@ CONFIG_LOG_BUF_SHIFT=14
> CONFIG_BLK_DEV_INITRD=y
> # CONFIG_RD_GZIP is not set
> CONFIG_EXPERT=y
> -# CONFIG_KALLSYMS is not set
> # CONFIG_EPOLL is not set
> # CONFIG_TIMERFD is not set
> # CONFIG_EVENTFD is not set
> # CONFIG_AIO is not set
> -# CONFIG_VM_EVENT_COUNTERS is not set
> -# CONFIG_COMPAT_BRK is not set
> -CONFIG_SLUB=y
> -CONFIG_SLUB_TINY=y
> -CONFIG_MODULES=y
> -# CONFIG_BLOCK is not set
> +# CONFIG_KALLSYMS is not set
> CONFIG_BUILTIN_DTB_NAME="or1ksim"
> CONFIG_HZ_100=y
> +CONFIG_JUMP_LABEL=y
> +CONFIG_MODULES=y
> +# CONFIG_BLOCK is not set
> +CONFIG_SLUB_TINY=y
> +# CONFIG_COMPAT_BRK is not set
> +# CONFIG_VM_EVENT_COUNTERS is not set
> CONFIG_NET=y
> CONFIG_PACKET=y
> CONFIG_UNIX=y
> CONFIG_INET=y
> -# CONFIG_INET_XFRM_MODE_TRANSPORT is not set
> -# CONFIG_INET_XFRM_MODE_TUNNEL is not set
> -# CONFIG_INET_XFRM_MODE_BEET is not set
> # CONFIG_INET_DIAG is not set
> CONFIG_TCP_CONG_ADVANCED=y
> # CONFIG_TCP_CONG_BIC is not set
> @@ -35,7 +32,6 @@ CONFIG_DEVTMPFS=y
> CONFIG_DEVTMPFS_MOUNT=y
> # CONFIG_PREVENT_FIRMWARE_BUILD is not set
> # CONFIG_FW_LOADER is not set
> -CONFIG_PROC_DEVICETREE=y
> CONFIG_NETDEVICES=y
> CONFIG_ETHOC=y
> CONFIG_MICREL_PHY=y
> @@ -53,4 +49,3 @@ CONFIG_SERIAL_OF_PLATFORM=y
> # CONFIG_DNOTIFY is not set
> CONFIG_TMPFS=y
> CONFIG_NFS_FS=y
> -# CONFIG_ENABLE_MUST_CHECK is not set
> diff --git a/arch/openrisc/configs/virt_defconfig b/arch/openrisc/configs/virt_defconfig
> index c1b69166c500..4a80c5794877 100644
> --- a/arch/openrisc/configs/virt_defconfig
> +++ b/arch/openrisc/configs/virt_defconfig
> @@ -12,6 +12,7 @@ CONFIG_NR_CPUS=8
> CONFIG_SMP=y
> CONFIG_HZ_100=y
> # CONFIG_OPENRISC_NO_SPR_SR_DSX is not set
> +CONFIG_JUMP_LABEL=y
> # CONFIG_COMPAT_BRK is not set
> CONFIG_NET=y
> CONFIG_PACKET=y
The defconfig updates look quite different. Did you use savedefconfig for both
of them?
> diff --git a/arch/openrisc/include/asm/insn-def.h b/arch/openrisc/include/asm/insn-def.h
> index dc8d16db1579..2ccdbb37c27c 100644
> --- a/arch/openrisc/include/asm/insn-def.h
> +++ b/arch/openrisc/include/asm/insn-def.h
> @@ -9,4 +9,7 @@
> /* or1k instructions are always 32 bits. */
> #define OPENRISC_INSN_SIZE 4
>
> +/* or1k nop instruction code */
> +#define OPENRISC_INSN_NOP 0x15000000U
I see you use this header again here, note that in
arch/openrisc/kernel/signal.c. We write instructions to memory too for the
sigreturn trampoline.
Also, you use OPENRISC_INSN_SIZE below. Could you move this header to this
patch completely? I don't think its needed in the patching patch.
> #endif /* __ASM_INSN_DEF_H */
> diff --git a/arch/openrisc/include/asm/jump_label.h b/arch/openrisc/include/asm/jump_label.h
> new file mode 100644
> index 000000000000..03afca9c3a1f
> --- /dev/null
> +++ b/arch/openrisc/include/asm/jump_label.h
> @@ -0,0 +1,68 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2025 Chen Miao
> + *
> + * Based on arch/arm/include/asm/jump_label.h
> + */
> +#ifndef __ASM_JUMP_LABEL_H
> +#define __ASM_JUMP_LABEL_H
Can this be __ASM_OPENRISC_JUMP_LABEL_H?
> +#ifndef __ASSEMBLY__
Note upstream this is now __ASSEMBLER__.
> +#include <linux/types.h>
> +#include <asm/insn-def.h>
> +
> +#define HAVE_JUMP_LABEL_BATCH
> +
> +#define JUMP_LABEL_NOP_SIZE OPENRISC_INSN_SIZE
> +
> +/*
> + * should aligned 4
> + * for jump_label relative
> + * entry.code = nop.addr - . -> return false
> + * entry.target = l_yes - . -> return true
> + * entry.key = key - .
> + */
> +#define JUMP_TABLE_ENTRY(key, label) \
> + ".pushsection __jump_table, \"aw\" \n\t" \
> + ".align 4 \n\t" \
> + ".long 1b - ., " label " - . \n\t" \
> + ".long " key " - . \n\t" \
> + ".popsection \n\t"
> +
> +#define ARCH_STATIC_BRANCH_ASM(key, label) \
> + ".align 4 \n\t" \
> + "1: l.nop \n\t" \
> + " l.nop \n\t" \
> + JUMP_TABLE_ENTRY(key, label)
> +
> +static __always_inline bool arch_static_branch(struct static_key *const key,
> + const bool branch)
> +{
> + asm goto (ARCH_STATIC_BRANCH_ASM("%0", "%l[l_yes]")
> + ::"i"(&((char *)key)[branch])::l_yes);
> +
> + return false;
> +l_yes:
> + return true;
> +}
> +
> +#define ARCH_STATIC_BRANCH_JUMP_ASM(key, label) \
> + ".align 4 \n\t" \
> + "1: l.j " label " \n\t" \
> + " l.nop \n\t" \
> + JUMP_TABLE_ENTRY(key, label)
> +
> +static __always_inline bool
> +arch_static_branch_jump(struct static_key *const key, const bool branch)
> +{
> + asm goto (ARCH_STATIC_BRANCH_JUMP_ASM("%0", "%l[l_yes]")
> + ::"i"(&((char *)key)[branch])::l_yes);
> +
> + return false;
> +l_yes:
> + return true;
> +}
> +
> +#endif /* __ASSEMBLY__ */
> +#endif /* __ASM_JUMP_LABEL_H */
> diff --git a/arch/openrisc/kernel/Makefile b/arch/openrisc/kernel/Makefile
> index f0957ce16d6b..19e0eb94f2eb 100644
> --- a/arch/openrisc/kernel/Makefile
> +++ b/arch/openrisc/kernel/Makefile
> @@ -9,6 +9,7 @@ obj-y := head.o setup.o or32_ksyms.o process.o dma.o \
> traps.o time.o irq.o entry.o ptrace.o signal.o \
> sys_call_table.o unwinder.o cacheinfo.o
>
> +obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> obj-$(CONFIG_SMP) += smp.o sync-timer.o
> obj-$(CONFIG_STACKTRACE) += stacktrace.o
> obj-$(CONFIG_MODULES) += module.o
> diff --git a/arch/openrisc/kernel/jump_label.c b/arch/openrisc/kernel/jump_label.c
> new file mode 100644
> index 000000000000..ce259ba30258
> --- /dev/null
> +++ b/arch/openrisc/kernel/jump_label.c
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2025 Chen Miao
> + *
> + * Based on arch/arm/kernel/jump_label.c
> + */
> +#include <linux/jump_label.h>
> +#include <linux/kernel.h>
> +#include <linux/memory.h>
> +#include <asm/bug.h>
> +#include <asm/cacheflush.h>
> +#include <asm/text-patching.h>
> +
> +bool arch_jump_label_transform_queue(struct jump_entry *entry,
> + enum jump_label_type type)
> +{
> + void *addr = (void *)jump_entry_code(entry);
> + u32 insn;
> +
> + if (type == JUMP_LABEL_JMP) {
> + long offset;
> +
> + offset = jump_entry_target(entry) - jump_entry_code(entry);
> + /*
> + * The actual maximum range of the l.j instruction's offset is -134,217,728
> + * ~ 134,217,724 (sign 26-bit imm).
> + * For the original jump range, we need to right-shift N by 2 to obtain the
> + * instruction's offset.
> + */
> + if (unlikely(offset < -134217728 || offset > 134217724)) {
> + WARN_ON_ONCE(true);
> + }
> + /* 26bit imm mask */
> + offset = (offset >> 2) & 0x03ffffff;
> +
> + insn = offset;
> + } else {
> + insn = OPENRISC_INSN_NOP;
> + }
> +
> + if (early_boot_irqs_disabled) {
> + copy_to_kernel_nofault(addr, &insn, sizeof(insn));
> + } else {
> + patch_insn_write(addr, insn);
> + }
> + return true;
> +}
> +
> +void arch_jump_label_transform_apply(void)
> +{
> + // flush
Could you use the /* */ comment style? Is this really flushing?
> + kick_all_cpus_sync();
> +}
> diff --git a/arch/openrisc/kernel/setup.c b/arch/openrisc/kernel/setup.c
> index a9fb9cc6779e..000a9cc10e6f 100644
> --- a/arch/openrisc/kernel/setup.c
> +++ b/arch/openrisc/kernel/setup.c
> @@ -249,6 +249,8 @@ void __init setup_arch(char **cmdline_p)
> initrd_below_start_ok = 1;
> }
> #endif
> + /* perform jump_table sorting before paging_init locks down read only memory */
> + jump_label_init();
>
> /* paging_init() sets up the MMU and marks all pages as reserved */
> paging_init();
> --
> 2.45.2
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] openrisc: Add text patching API support
2025-08-09 8:25 ` Stafford Horne
@ 2025-08-10 14:52 ` Miao Chen
0 siblings, 0 replies; 7+ messages in thread
From: Miao Chen @ 2025-08-10 14:52 UTC (permalink / raw)
To: Stafford Horne
Cc: Linux Kernel, Linux OpenRISC, Jonas Bonn, Stefan Kristiansson,
Mike Rapoport (Microsoft), Geert Uytterhoeven, Arnd Bergmann,
Luis Chamberlain, Sahil Siddiq, Johannes Berg, Nicolas Schier,
Masahiro Yamada, Dave Hansen, Andrew Morton
Stafford Horne <shorne@gmail.com> 于2025年8月9日周六 16:25写道：
>
> On Wed, Aug 06, 2025 at 02:05:03AM +0000, ChenMiao wrote:
> > From: chenmiao <chenmiao.ku@gmail.com>
> >
> > We need a text patching mechanism to ensure that in the subsequent
> > implementation of jump_label, the code can be modified to the correct
> > location. Therefore, FIX_TEXT_POKE0 has been added as a mapping area.
> >
> > And, I create a new file named insn-def.h to define the or1k insn macro
> > size and more define in the future.
> >
> > Among these changes, we implement patch_map and support the
> > patch_insn_write API for single instruction writing.
> >
> > - V2: We modify the patch_insn_write(void *addr, const void *insn) API to
> > patch_insn_write(void *addr, u32 insn), derectly support a single u32
> > instruction write to map memory.
> >
> > Link: https://lore.kernel.org/openrisc/aJIC8o1WmVHol9RY@antec/T/#t
> >
> > Signed-off-by: chenmiao <chenmiao.ku@gmail.com>
> > ---
>
> The v2, notes should go in this part of the patch (after ---), this way
> it does not show up when I queue the patches.
well, I'll be care of the next version
>
> > arch/openrisc/include/asm/Kbuild | 1 -
> > arch/openrisc/include/asm/fixmap.h | 1 +
> > arch/openrisc/include/asm/insn-def.h | 12 ++++
> > arch/openrisc/include/asm/text-patching.h | 13 ++++
> > arch/openrisc/kernel/Makefile | 1 +
> > arch/openrisc/kernel/patching.c | 79 +++++++++++++++++++++++
> > arch/openrisc/mm/init.c | 2 +-
> > 7 files changed, 107 insertions(+), 2 deletions(-)
> > create mode 100644 arch/openrisc/include/asm/insn-def.h
> > create mode 100644 arch/openrisc/include/asm/text-patching.h
> > create mode 100644 arch/openrisc/kernel/patching.c
> >
> > diff --git a/arch/openrisc/include/asm/Kbuild b/arch/openrisc/include/asm/Kbuild
> > index 2b1a6b00cdac..cef49d60d74c 100644
> > --- a/arch/openrisc/include/asm/Kbuild
> > +++ b/arch/openrisc/include/asm/Kbuild
> > @@ -9,4 +9,3 @@ generic-y += spinlock.h
> > generic-y += qrwlock_types.h
> > generic-y += qrwlock.h
> > generic-y += user.h
> > -generic-y += text-patching.h
> > diff --git a/arch/openrisc/include/asm/fixmap.h b/arch/openrisc/include/asm/fixmap.h
> > index aaa6a26a3e92..74000215064d 100644
> > --- a/arch/openrisc/include/asm/fixmap.h
> > +++ b/arch/openrisc/include/asm/fixmap.h
> > @@ -28,6 +28,7 @@
> >
> > enum fixed_addresses {
> > FIX_EARLYCON_MEM_BASE,
> > + FIX_TEXT_POKE0,
> > __end_of_fixed_addresses
> > };
> >
> > diff --git a/arch/openrisc/include/asm/insn-def.h b/arch/openrisc/include/asm/insn-def.h
> > new file mode 100644
> > index 000000000000..dc8d16db1579
> > --- /dev/null
> > +++ b/arch/openrisc/include/asm/insn-def.h
> > @@ -0,0 +1,12 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2025 Chen Miao
> > + */
> > +
> > +#ifndef __ASM_INSN_DEF_H
> > +#define __ASM_INSN_DEF_H
> > +
> > +/* or1k instructions are always 32 bits. */
> > +#define OPENRISC_INSN_SIZE 4
>
> Is this used outside of the usage below? Why do we need a header for it?
I think we need the header, although it seems redundant. Although we
only added one line here, I think as development progresses, we might
need more macro definitions related to the instruction set, possibly
including jump_label, ftrace, and potentially bpf.
>
> > +#endif /* __ASM_INSN_DEF_H */
> > diff --git a/arch/openrisc/include/asm/text-patching.h b/arch/openrisc/include/asm/text-patching.h
> > new file mode 100644
> > index 000000000000..bffe828288c3
> > --- /dev/null
> > +++ b/arch/openrisc/include/asm/text-patching.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2025 Chen Miao
> > + */
> > +
> > +#ifndef _ASM_PATCHING_H_
> > +#define _ASM_PATCHING_H_
> > +
> > +#include <linux/types.h>
> > +
> > +int patch_insn_write(void *addr, u32 insn);
> > +
> > +#endif /* _ASM_PATCHING_H_ */
> > diff --git a/arch/openrisc/kernel/Makefile b/arch/openrisc/kernel/Makefile
> > index 58e6a1b525b7..f0957ce16d6b 100644
> > --- a/arch/openrisc/kernel/Makefile
> > +++ b/arch/openrisc/kernel/Makefile
> > @@ -13,5 +13,6 @@ obj-$(CONFIG_SMP) += smp.o sync-timer.o
> > obj-$(CONFIG_STACKTRACE) += stacktrace.o
> > obj-$(CONFIG_MODULES) += module.o
> > obj-$(CONFIG_OF) += prom.o
> > +obj-y += patching.o
> >
> > clean:
> > diff --git a/arch/openrisc/kernel/patching.c b/arch/openrisc/kernel/patching.c
> > new file mode 100644
> > index 000000000000..c9a30f0d1193
> > --- /dev/null
> > +++ b/arch/openrisc/kernel/patching.c
> > @@ -0,0 +1,79 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/* Copyright (C) 2020 SiFive
> > + * Copyright (C) 2025 Chen Miao
> > + */
> > +
> > +#include <linux/mm.h>
> > +#include <linux/kernel.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/uaccess.h>
> > +
> > +#include <asm/insn-def.h>
> > +#include <asm/cacheflush.h>
> > +#include <asm/page.h>
> > +#include <asm/fixmap.h>
> > +#include <asm/text-patching.h>
> > +#include <asm/sections.h>
> > +
> > +static DEFINE_RAW_SPINLOCK(patch_lock);
> > +
> > +static inline bool is_exit_text(uintptr_t addr)
> > +{
> > + /* Now Have NO Mechanism to do */
> > + return true;
> > +}
>
> Is this needed? OpenRISC doesn't seem to have any sections to support __exit
> annotated cleanup functions.
>
> Ref: https://kernelnewbies.org/FAQ/InitExitMacros
>
> Returning true here means that all of the patch_map calls will be treated as
> kernel pages below though, which is wrong, please remove this.
>
I'll remove it.
> > +
> > +static __always_inline void *patch_map(void *addr, int fixmap)
> > +{
> > + uintptr_t uaddr = (uintptr_t) addr;
> > + phys_addr_t phys;
> > +
> > + if (core_kernel_text(uaddr) || is_exit_text(uaddr)) {
> > + phys = __pa_symbol(addr);
> > + } else {
> > + struct page *page = vmalloc_to_page(addr);
> > + BUG_ON(!page);
> > + phys = page_to_phys(page) + offset_in_page(addr);
> > + }
> > +
> > + return (void *)set_fixmap_offset(fixmap, phys);
> > +}
> > +
> > +static void patch_unmap(int fixmap)
> > +{
> > + clear_fixmap(fixmap);
> > +}
> > +
> > +static int __patch_insn_write(void *addr, u32 insn)
> > +{
> > + void *waddr = addr;
> > + unsigned long flags = 0;
> > + int ret;
> > +
> > + raw_spin_lock_irqsave(&patch_lock, flags);
> > +
> > + waddr = patch_map(addr, FIX_TEXT_POKE0);
> > +
> > + ret = copy_to_kernel_nofault(waddr, &insn, OPENRISC_INSN_SIZE);
> > + local_icache_range_inv((unsigned long)waddr,
> > + (unsigned long)waddr + OPENRISC_INSN_SIZE);
>
> Instead of OPENRISC_INSN_SIZE, can you use sizeof(insn)?
I think not using sizeof(insn)here would be better. In fact, using the
OPENRISC_INSN_SIZE macro might be more appropriate.
Here we are modifying a single instruction, so it must be 4 bytes—the
length of one OPENRISC INSN.
>
> > + patch_unmap(FIX_TEXT_POKE0);
> > +
> > + raw_spin_unlock_irqrestore(&patch_lock, flags);
> > +
> > + return ret;
> > +}
> > +
> > +int patch_insn_write(void *addr, u32 insn)
> > +{
> > + u32 *tp = addr;
> > + int ret;
> > +
> > + if ((uintptr_t) tp & 0x3)
> > + return -EINVAL;
> > +
> > + ret = __patch_insn_write(tp, insn);
> > +
> > + return ret;
> > +}
> > diff --git a/arch/openrisc/mm/init.c b/arch/openrisc/mm/init.c
> > index e4904ca6f0a0..b5925710f954 100644
> > --- a/arch/openrisc/mm/init.c
> > +++ b/arch/openrisc/mm/init.c
> > @@ -226,7 +226,7 @@ static int __init map_page(unsigned long va, phys_addr_t pa, pgprot_t prot)
> > return 0;
> > }
> >
> > -void __init __set_fixmap(enum fixed_addresses idx,
> > +void __set_fixmap(enum fixed_addresses idx,
>
> Can you put a comment about this bit in the commit description?
I'll do the next version
>
> > phys_addr_t phys, pgprot_t prot)
> > {
> > unsigned long address = __fix_to_virt(idx);
> > --
> > 2.45.2
>
> -Stafford
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] openrisc: Add jump label support
2025-08-09 8:37 ` Stafford Horne
@ 2025-08-10 15:09 ` Miao Chen
0 siblings, 0 replies; 7+ messages in thread
From: Miao Chen @ 2025-08-10 15:09 UTC (permalink / raw)
To: Stafford Horne
Cc: Linux Kernel, Linux OpenRISC, Jonathan Corbet, Jonas Bonn,
Stefan Kristiansson, Peter Zijlstra, Josh Poimboeuf, Jason Baron,
Steven Rostedt, Ard Biesheuvel, Masahiro Yamada, Sahil Siddiq,
Nicolas Schier, Johannes Berg, linux-doc
Stafford Horne <shorne@gmail.com> 于2025年8月9日周六 16:37写道：
>
> On Wed, Aug 06, 2025 at 02:05:04AM +0000, ChenMiao wrote:
> > From: chenmiao <chenmiao.ku@gmail.com>
> >
> > Implemented the full functionality of jump_label, of course,
> > with text patching supported by just one API.
> >
> > By the way, add new macro OPENRISC_INSN_NOP in insn-def.h to use.
> >
> > - V2: using the patch_insn_write(void *addr, u32 insn) not the
> > const void *insn.
> >
> > Link: https://lore.kernel.org/openrisc/aJIC8o1WmVHol9RY@antec/T/#t
> >
> > Signed-off-by: chenmiao <chenmiao.ku@gmail.com>
> > ---
> > .../core/jump-labels/arch-support.txt | 2 +-
> > arch/openrisc/Kconfig | 2 +
> > arch/openrisc/configs/or1ksim_defconfig | 19 ++----
> > arch/openrisc/configs/virt_defconfig | 1 +
> > arch/openrisc/include/asm/insn-def.h | 3 +
> > arch/openrisc/include/asm/jump_label.h | 68 +++++++++++++++++++
> > arch/openrisc/kernel/Makefile | 1 +
> > arch/openrisc/kernel/jump_label.c | 53 +++++++++++++++
> > arch/openrisc/kernel/setup.c | 2 +
> > 9 files changed, 138 insertions(+), 13 deletions(-)
> > create mode 100644 arch/openrisc/include/asm/jump_label.h
> > create mode 100644 arch/openrisc/kernel/jump_label.c
> >
> > diff --git a/Documentation/features/core/jump-labels/arch-support.txt b/Documentation/features/core/jump-labels/arch-support.txt
> > index ccada815569f..683de7c15058 100644
> > --- a/Documentation/features/core/jump-labels/arch-support.txt
> > +++ b/Documentation/features/core/jump-labels/arch-support.txt
> > @@ -17,7 +17,7 @@
> > | microblaze: | TODO |
> > | mips: | ok |
> > | nios2: | TODO |
> > - | openrisc: | TODO |
> > + | openrisc: | ok |
> > | parisc: | ok |
> > | powerpc: | ok |
> > | riscv: | ok |
> > diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
> > index b38fee299bc4..9156635dd264 100644
> > --- a/arch/openrisc/Kconfig
> > +++ b/arch/openrisc/Kconfig
> > @@ -24,6 +24,8 @@ config OPENRISC
> > select GENERIC_PCI_IOMAP
> > select GENERIC_IOREMAP
> > select GENERIC_CPU_DEVICES
> > + select HAVE_ARCH_JUMP_LABEL
> > + select HAVE_ARCH_JUMP_LABEL_RELATIVE
> > select HAVE_PCI
> > select HAVE_UID16
> > select HAVE_PAGE_SIZE_8KB
> > diff --git a/arch/openrisc/configs/or1ksim_defconfig b/arch/openrisc/configs/or1ksim_defconfig
> > index 59fe33cefba2..769705ac24d5 100644
> > --- a/arch/openrisc/configs/or1ksim_defconfig
> > +++ b/arch/openrisc/configs/or1ksim_defconfig
> > @@ -3,26 +3,23 @@ CONFIG_LOG_BUF_SHIFT=14
> > CONFIG_BLK_DEV_INITRD=y
> > # CONFIG_RD_GZIP is not set
> > CONFIG_EXPERT=y
> > -# CONFIG_KALLSYMS is not set
> > # CONFIG_EPOLL is not set
> > # CONFIG_TIMERFD is not set
> > # CONFIG_EVENTFD is not set
> > # CONFIG_AIO is not set
> > -# CONFIG_VM_EVENT_COUNTERS is not set
> > -# CONFIG_COMPAT_BRK is not set
> > -CONFIG_SLUB=y
> > -CONFIG_SLUB_TINY=y
> > -CONFIG_MODULES=y
> > -# CONFIG_BLOCK is not set
> > +# CONFIG_KALLSYMS is not set
> > CONFIG_BUILTIN_DTB_NAME="or1ksim"
> > CONFIG_HZ_100=y
> > +CONFIG_JUMP_LABEL=y
> > +CONFIG_MODULES=y
> > +# CONFIG_BLOCK is not set
> > +CONFIG_SLUB_TINY=y
> > +# CONFIG_COMPAT_BRK is not set
> > +# CONFIG_VM_EVENT_COUNTERS is not set
> > CONFIG_NET=y
> > CONFIG_PACKET=y
> > CONFIG_UNIX=y
> > CONFIG_INET=y
> > -# CONFIG_INET_XFRM_MODE_TRANSPORT is not set
> > -# CONFIG_INET_XFRM_MODE_TUNNEL is not set
> > -# CONFIG_INET_XFRM_MODE_BEET is not set
> > # CONFIG_INET_DIAG is not set
> > CONFIG_TCP_CONG_ADVANCED=y
> > # CONFIG_TCP_CONG_BIC is not set
> > @@ -35,7 +32,6 @@ CONFIG_DEVTMPFS=y
> > CONFIG_DEVTMPFS_MOUNT=y
> > # CONFIG_PREVENT_FIRMWARE_BUILD is not set
> > # CONFIG_FW_LOADER is not set
> > -CONFIG_PROC_DEVICETREE=y
> > CONFIG_NETDEVICES=y
> > CONFIG_ETHOC=y
> > CONFIG_MICREL_PHY=y
> > @@ -53,4 +49,3 @@ CONFIG_SERIAL_OF_PLATFORM=y
> > # CONFIG_DNOTIFY is not set
> > CONFIG_TMPFS=y
> > CONFIG_NFS_FS=y
> > -# CONFIG_ENABLE_MUST_CHECK is not set
> > diff --git a/arch/openrisc/configs/virt_defconfig b/arch/openrisc/configs/virt_defconfig
> > index c1b69166c500..4a80c5794877 100644
> > --- a/arch/openrisc/configs/virt_defconfig
> > +++ b/arch/openrisc/configs/virt_defconfig
> > @@ -12,6 +12,7 @@ CONFIG_NR_CPUS=8
> > CONFIG_SMP=y
> > CONFIG_HZ_100=y
> > # CONFIG_OPENRISC_NO_SPR_SR_DSX is not set
> > +CONFIG_JUMP_LABEL=y
> > # CONFIG_COMPAT_BRK is not set
> > CONFIG_NET=y
> > CONFIG_PACKET=y
>
> The defconfig updates look quite different. Did you use savedefconfig for both
> of them?
Sure, it will do in the next version.
>
> > diff --git a/arch/openrisc/include/asm/insn-def.h b/arch/openrisc/include/asm/insn-def.h
> > index dc8d16db1579..2ccdbb37c27c 100644
> > --- a/arch/openrisc/include/asm/insn-def.h
> > +++ b/arch/openrisc/include/asm/insn-def.h
> > @@ -9,4 +9,7 @@
> > /* or1k instructions are always 32 bits. */
> > #define OPENRISC_INSN_SIZE 4
> >
> > +/* or1k nop instruction code */
> > +#define OPENRISC_INSN_NOP 0x15000000U
>
> I see you use this header again here, note that in
> arch/openrisc/kernel/signal.c. We write instructions to memory too for the
> sigreturn trampoline.
>
> Also, you use OPENRISC_INSN_SIZE below. Could you move this header to this
> patch completely? I don't think its needed in the patching patch.
>
maybe... this patch we no need this header, but I think this header is
meaningful, we can do this in another patch? modify the hard code insn
to macro.
> > #endif /* __ASM_INSN_DEF_H */
> > diff --git a/arch/openrisc/include/asm/jump_label.h b/arch/openrisc/include/asm/jump_label.h
> > new file mode 100644
> > index 000000000000..03afca9c3a1f
> > --- /dev/null
> > +++ b/arch/openrisc/include/asm/jump_label.h
> > @@ -0,0 +1,68 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2025 Chen Miao
> > + *
> > + * Based on arch/arm/include/asm/jump_label.h
> > + */
> > +#ifndef __ASM_JUMP_LABEL_H
> > +#define __ASM_JUMP_LABEL_H
>
> Can this be __ASM_OPENRISC_JUMP_LABEL_H?
>
> > +#ifndef __ASSEMBLY__
>
> Note upstream this is now __ASSEMBLER__.
copy that.
>
> > +#include <linux/types.h>
> > +#include <asm/insn-def.h>
> > +
> > +#define HAVE_JUMP_LABEL_BATCH
> > +
> > +#define JUMP_LABEL_NOP_SIZE OPENRISC_INSN_SIZE
> > +
> > +/*
> > + * should aligned 4
> > + * for jump_label relative
> > + * entry.code = nop.addr - . -> return false
> > + * entry.target = l_yes - . -> return true
> > + * entry.key = key - .
> > + */
> > +#define JUMP_TABLE_ENTRY(key, label) \
> > + ".pushsection __jump_table, \"aw\" \n\t" \
> > + ".align 4 \n\t" \
> > + ".long 1b - ., " label " - . \n\t" \
> > + ".long " key " - . \n\t" \
> > + ".popsection \n\t"
> > +
> > +#define ARCH_STATIC_BRANCH_ASM(key, label) \
> > + ".align 4 \n\t" \
> > + "1: l.nop \n\t" \
> > + " l.nop \n\t" \
> > + JUMP_TABLE_ENTRY(key, label)
> > +
> > +static __always_inline bool arch_static_branch(struct static_key *const key,
> > + const bool branch)
> > +{
> > + asm goto (ARCH_STATIC_BRANCH_ASM("%0", "%l[l_yes]")
> > + ::"i"(&((char *)key)[branch])::l_yes);
> > +
> > + return false;
> > +l_yes:
> > + return true;
> > +}
> > +
> > +#define ARCH_STATIC_BRANCH_JUMP_ASM(key, label) \
> > + ".align 4 \n\t" \
> > + "1: l.j " label " \n\t" \
> > + " l.nop \n\t" \
> > + JUMP_TABLE_ENTRY(key, label)
> > +
> > +static __always_inline bool
> > +arch_static_branch_jump(struct static_key *const key, const bool branch)
> > +{
> > + asm goto (ARCH_STATIC_BRANCH_JUMP_ASM("%0", "%l[l_yes]")
> > + ::"i"(&((char *)key)[branch])::l_yes);
> > +
> > + return false;
> > +l_yes:
> > + return true;
> > +}
> > +
> > +#endif /* __ASSEMBLY__ */
> > +#endif /* __ASM_JUMP_LABEL_H */
> > diff --git a/arch/openrisc/kernel/Makefile b/arch/openrisc/kernel/Makefile
> > index f0957ce16d6b..19e0eb94f2eb 100644
> > --- a/arch/openrisc/kernel/Makefile
> > +++ b/arch/openrisc/kernel/Makefile
> > @@ -9,6 +9,7 @@ obj-y := head.o setup.o or32_ksyms.o process.o dma.o \
> > traps.o time.o irq.o entry.o ptrace.o signal.o \
> > sys_call_table.o unwinder.o cacheinfo.o
> >
> > +obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> > obj-$(CONFIG_SMP) += smp.o sync-timer.o
> > obj-$(CONFIG_STACKTRACE) += stacktrace.o
> > obj-$(CONFIG_MODULES) += module.o
> > diff --git a/arch/openrisc/kernel/jump_label.c b/arch/openrisc/kernel/jump_label.c
> > new file mode 100644
> > index 000000000000..ce259ba30258
> > --- /dev/null
> > +++ b/arch/openrisc/kernel/jump_label.c
> > @@ -0,0 +1,53 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2025 Chen Miao
> > + *
> > + * Based on arch/arm/kernel/jump_label.c
> > + */
> > +#include <linux/jump_label.h>
> > +#include <linux/kernel.h>
> > +#include <linux/memory.h>
> > +#include <asm/bug.h>
> > +#include <asm/cacheflush.h>
> > +#include <asm/text-patching.h>
> > +
> > +bool arch_jump_label_transform_queue(struct jump_entry *entry,
> > + enum jump_label_type type)
> > +{
> > + void *addr = (void *)jump_entry_code(entry);
> > + u32 insn;
> > +
> > + if (type == JUMP_LABEL_JMP) {
> > + long offset;
> > +
> > + offset = jump_entry_target(entry) - jump_entry_code(entry);
> > + /*
> > + * The actual maximum range of the l.j instruction's offset is -134,217,728
> > + * ~ 134,217,724 (sign 26-bit imm).
> > + * For the original jump range, we need to right-shift N by 2 to obtain the
> > + * instruction's offset.
> > + */
> > + if (unlikely(offset < -134217728 || offset > 134217724)) {
> > + WARN_ON_ONCE(true);
> > + }
> > + /* 26bit imm mask */
> > + offset = (offset >> 2) & 0x03ffffff;
> > +
> > + insn = offset;
> > + } else {
> > + insn = OPENRISC_INSN_NOP;
> > + }
> > +
> > + if (early_boot_irqs_disabled) {
> > + copy_to_kernel_nofault(addr, &insn, sizeof(insn));
> > + } else {
> > + patch_insn_write(addr, insn);
> > + }
> > + return true;
> > +}
> > +
> > +void arch_jump_label_transform_apply(void)
> > +{
> > + // flush
>
> Could you use the /* */ comment style? Is this really flushing?
sure, really flushing.
>
> > + kick_all_cpus_sync();
> > +}
> > diff --git a/arch/openrisc/kernel/setup.c b/arch/openrisc/kernel/setup.c
> > index a9fb9cc6779e..000a9cc10e6f 100644
> > --- a/arch/openrisc/kernel/setup.c
> > +++ b/arch/openrisc/kernel/setup.c
> > @@ -249,6 +249,8 @@ void __init setup_arch(char **cmdline_p)
> > initrd_below_start_ok = 1;
> > }
> > #endif
> > + /* perform jump_table sorting before paging_init locks down read only memory */
> > + jump_label_init();
> >
> > /* paging_init() sets up the MMU and marks all pages as reserved */
> > paging_init();
> > --
> > 2.45.2
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-08-10 15:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-06 2:05 [PATCH v2 0/2] Support fixmap and jump_label for openrisc ChenMiao
2025-08-06 2:05 ` [PATCH v2 1/2] openrisc: Add text patching API support ChenMiao
2025-08-09 8:25 ` Stafford Horne
2025-08-10 14:52 ` Miao Chen
2025-08-06 2:05 ` [PATCH v2 2/2] openrisc: Add jump label support ChenMiao
2025-08-09 8:37 ` Stafford Horne
2025-08-10 15:09 ` Miao Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).