lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aK3O6kOMqgDb6zZj@antec>
Date: Tue, 26 Aug 2025 16:12:42 +0100
From: Stafford Horne <shorne@...il.com>
To: ChenMiao <chenmiao.ku@...il.com>
Cc: Linux Kernel <linux-kernel@...r.kernel.org>,
	Linux OpenRISC <linux-openrisc@...r.kernel.org>,
	Jonathan Corbet <corbet@....net>, Jonas Bonn <jonas@...thpole.se>,
	Stefan Kristiansson <stefan.kristiansson@...nalahti.fi>,
	Peter Zijlstra <peterz@...radead.org>,
	Josh Poimboeuf <jpoimboe@...nel.org>,
	Jason Baron <jbaron@...mai.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Ard Biesheuvel <ardb@...nel.org>,
	Masahiro Yamada <masahiroy@...nel.org>,
	Johannes Berg <johannes@...solutions.net>,
	Nicolas Schier <nicolas.schier@...ux.dev>,
	Sahil Siddiq <sahilcdq0@...il.com>, linux-doc@...r.kernel.org
Subject: Re: [PATCH v3 2/2] openrisc: Add jump label support

On Thu, Aug 14, 2025 at 03:27:02AM +0000, ChenMiao wrote:
> From: chenmiao <chenmiao.ku@...il.com>
> 
> Implemented the full functionality of jump_label, of course,
> with text patching supported by just one API.

I am not sure the second part of the statement is relevant.  Perhaps we can
mention that we use relative labels.

Do you have any details about how this improves performance?

How have you tested this?

We should try to measure boot time or various syscalls or scheduling operations.
I would like to get some numbers before we queue this for upstream.  I may do
myself as I think its beneficial.

Other than that I think this is ok, one small comment below.

> Link: https://lore.kernel.org/openrisc/aJIC8o1WmVHol9RY@antec/T/#t
> 
> Signed-off-by: chenmiao <chenmiao.ku@...il.com>
> 
> ---
> Changes in V3:
>   - Ensure the two defconfig using the make savedefconfig.
>   - modify the __ASSEMBLY__ to __ASSEMBLER__, modify the
>     __ASM_JUMP_LABEL_H to __ASM_OPENRISC_JUMP_LABEL_H and remove
>     invalid comment.
> 
> Changes in V2:
>   - using the patch_insn_write(void *addr, u32 insn) not the
>     const void *insn.
>   - add new macro OPENRISC_INSN_NOP in insn-def.h to use.
> 
> Signed-off-by: chenmiao <chenmiao.ku@...il.com>
> ---
>  .../core/jump-labels/arch-support.txt         |  2 +-
>  arch/openrisc/Kconfig                         |  2 +
>  arch/openrisc/configs/or1ksim_defconfig       | 19 ++----
>  arch/openrisc/configs/virt_defconfig          |  2 +-
>  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             | 52 ++++++++++++++
>  arch/openrisc/kernel/setup.c                  |  2 +
>  9 files changed, 137 insertions(+), 14 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..a93a3e1e4f87 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
> @@ -55,7 +56,6 @@ CONFIG_DRM=y
>  # CONFIG_DRM_FBDEV_EMULATION is not set
>  CONFIG_DRM_VIRTIO_GPU=y
>  CONFIG_FB=y
> -CONFIG_FIRMWARE_EDID=y
>  CONFIG_FRAMEBUFFER_CONSOLE=y
>  CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
>  CONFIG_LOGO=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..28dd6c78f8ce
> --- /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_OPENRISC_JUMP_LABEL_H
> +#define __ASM_OPENRISC_JUMP_LABEL_H
> +
> +#ifndef __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 - .
> + */

Perhaps this comment could be expanded a bit?

> +#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 /* __ASSEMBLER__ */
> +#endif /* __ASM_OPENRISC_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..071dacad885c
> --- /dev/null
> +++ b/arch/openrisc/kernel/jump_label.c
> @@ -0,0 +1,52 @@
> +// 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)
> +{
> +	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
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ