[PATCH 0/4] arm64: Fix instrumentation escapes from noinstr path

[PATCH 0/4] arm64: Fix instrumentation escapes from noinstr path

[Jisheng Zhang]

Code marked with noinstr can only safely call other noinstr or
__always_inline functions. When reading arm64 source code, I found
necessary noinstr markers is missing somewhere. So I asked AI to scan
the arm64, this series is the result.

Jisheng Zhang (4):
  arm64: percpu: Always inline helpers used by noinstr code
  arm64: entry: Always inline helpers used by non-instrumentable paths
  arm64: stacktrace: Always inline internal stack boundary helpers
  arm64: alternatives: Make branch and ADRP offset helpers noinst

 arch/arm64/include/asm/cpufeature.h        |  8 +--
 arch/arm64/include/asm/entry-common.h      |  2 +-
 arch/arm64/include/asm/fpsimd.h            |  4 +-
 arch/arm64/include/asm/mte-kasan.h         |  4 +-
 arch/arm64/include/asm/mte.h               |  6 +--
 arch/arm64/include/asm/percpu.h            |  6 +--
 arch/arm64/include/asm/stacktrace.h        | 16 +++---
 arch/arm64/include/asm/stacktrace/common.h | 12 ++---
 arch/arm64/lib/insn.c                      | 57 ++++++++++++++--------
 9 files changed, 66 insertions(+), 49 deletions(-)

-- 
2.55.0

[PATCH 1/4] arm64: percpu: Always inline helpers used by noinstr code

[Jisheng Zhang]

The non-instrumentable entry and stack unwinding paths use the per-CPU
offset and read helpers. Plain inline does not guarantee inlining, so
these helpers can be emitted out of line without inheriting the
caller's instrumentation restrictions.

In a GCC build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y,
__kern_my_cpu_offset() was emitted in .text and called from
.noinstr.text.

Mark the per-CPU offset and read helpers __always_inline to keep their
instructions within their callers.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Assisted-by: LLM
---
 arch/arm64/include/asm/percpu.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/percpu.h b/arch/arm64/include/asm/percpu.h
index 3cc316091d66..7c344de5052d 100644
--- a/arch/arm64/include/asm/percpu.h
+++ b/arch/arm64/include/asm/percpu.h
@@ -23,7 +23,7 @@ static inline void set_my_cpu_offset(unsigned long off)
 			:: "r" (off) : "memory");
 }
 
-static inline unsigned long __hyp_my_cpu_offset(void)
+static __always_inline unsigned long __hyp_my_cpu_offset(void)
 {
 	/*
 	 * Non-VHE hyp code runs with preemption disabled. No need to hazard
@@ -37,7 +37,7 @@ static inline unsigned long __hyp_my_cpu_offset(void)
 		    "mrs " dst ", tpidr_el2",				\
 		       ARM64_HAS_VIRT_HOST_EXTN)
 
-static inline unsigned long __kern_my_cpu_offset(void)
+static __always_inline unsigned long __kern_my_cpu_offset(void)
 {
 	unsigned long off;
 
@@ -129,7 +129,7 @@ static inline unsigned long __kern_my_cpu_offset(void)
 #endif
 
 #define PERCPU_RW_OPS(w, sfx, sz)						\
-static inline unsigned long __percpu_read_##sz(void __percpu *pcp)		\
+static __always_inline unsigned long __percpu_read_##sz(void __percpu *pcp)		\
 {										\
 	u16 *gprs = &current_thread_info()->pcpu_gprs;				\
 	unsigned long off;							\
-- 
2.55.0

Re: [PATCH 1/4] arm64: percpu: Always inline helpers used by noinstr code

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] arm64: percpu: __PERCPU_OP_CASE() and sibling helpers not marked
  __always_inline
--

commit 3a2df0916b5200b227df21487897e171275b29f4
Author: Jisheng Zhang <jszhang@kernel.org>

arm64: percpu: Always inline helpers used by noinstr code

This commit marks __hyp_my_cpu_offset(), __kern_my_cpu_offset(), and
__percpu_read_##sz() as __always_inline so they are not emitted out of line
when called from non-instrumentable code.

> diff --git a/arch/arm64/include/asm/percpu.h b/arch/arm64/include/asm/percpu.h
> index 3cc316091d665..7c344de5052d5 100644
> --- a/arch/arm64/include/asm/percpu.h
> +++ b/arch/arm64/include/asm/percpu.h

[ ... ]

> @@ -129,7 +129,7 @@ static inline unsigned long __kern_my_cpu_offset(void)
>  #endif
>  
>  #define PERCPU_RW_OPS(w, sfx, sz)						\
> -static inline unsigned long __percpu_read_##sz(void __percpu *pcp)		\
> +static __always_inline unsigned long __percpu_read_##sz(void __percpu *pcp)		\
>  {										\
>  	u16 *gprs = &current_thread_info()->pcpu_gprs;				\
>  	unsigned long off;							\

[Severity: Low]
Should __PERCPU_OP_CASE() and the remaining __percpu_* helpers in this header
(__percpu_write_##sz(), __PERCPU_RET_OP_CASE(), PERCPU_XCHG_OP(),
PERCPU_CMPXCHG_OP(), and __percpu_cmpxchg_128()) also be marked
__always_inline?

The noinstr function lockdep_hardirqs_on() calls
__debug_atomic_inc(redundant_hardirqs_on), which under CONFIG_DEBUG_LOCKDEP=y
expands to this_cpu_inc(lockdep_stats.redundant_hardirqs_on) ->
__percpu_add_case_64().  Because __PERCPU_OP_CASE() still uses plain static
inline and is also called from instrumentable lockdep functions, builds with
CONFIG_CC_OPTIMIZE_FOR_SIZE=y can emit __percpu_add_case_64() out of line in
.text and call it from .noinstr.text before RCU and lockdep state are ready
for instrumentation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107071339.17467-1-jszhang@kernel.org?part=1

[PATCH 2/4] arm64: entry: Always inline helpers used by non-instrumentable paths

[Jisheng Zhang]

The noinstr entry and exit paths call MTE and SME helpers, CPU feature
checks, and the architecture-specific rescheduling check. Several of
these helpers use plain inline, which permits the compiler to emit
instrumentable out-of-line copies.

A GCC build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y emitted
mte_disable_tco_entry(), sme_enter_from_user_mode() and
sme_exit_to_user_mode() in .text.

Use __always_inline for these helpers and their nested feature checks
so they inherit the instrumentation restrictions of their callers

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Assisted-by: LLM
---
 arch/arm64/include/asm/cpufeature.h   | 8 ++++----
 arch/arm64/include/asm/entry-common.h | 2 +-
 arch/arm64/include/asm/fpsimd.h       | 4 ++--
 arch/arm64/include/asm/mte-kasan.h    | 4 ++--
 arch/arm64/include/asm/mte.h          | 6 +++---
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index e3c8a1493dc8..c62949693b92 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -774,7 +774,7 @@ static __always_inline bool system_supports_fpsimd(void)
 	return alternative_has_cap_likely(ARM64_HAS_FPSIMD);
 }
 
-static inline bool system_uses_hw_pan(void)
+static __always_inline bool system_uses_hw_pan(void)
 {
 	return alternative_has_cap_unlikely(ARM64_HAS_PAN);
 }
@@ -840,12 +840,12 @@ static __always_inline bool system_uses_irq_prio_masking(void)
 	return alternative_has_cap_unlikely(ARM64_HAS_GIC_PRIO_MASKING);
 }
 
-static inline bool system_supports_mte(void)
+static __always_inline bool system_supports_mte(void)
 {
 	return alternative_has_cap_unlikely(ARM64_MTE);
 }
 
-static inline bool system_has_prio_mask_debugging(void)
+static __always_inline bool system_has_prio_mask_debugging(void)
 {
 	return IS_ENABLED(CONFIG_ARM64_DEBUG_PRIORITY_MASKING) &&
 	       system_uses_irq_prio_masking();
@@ -950,7 +950,7 @@ static inline bool cpu_has_hw_af(void)
 						ID_AA64MMFR1_EL1_HAFDBS_SHIFT);
 }
 
-static inline bool cpu_has_pan(void)
+static __always_inline bool cpu_has_pan(void)
 {
 	u64 mmfr1 = read_cpuid(ID_AA64MMFR1_EL1);
 	return cpuid_feature_extract_unsigned_field(mmfr1,
diff --git a/arch/arm64/include/asm/entry-common.h b/arch/arm64/include/asm/entry-common.h
index 1905765159aa..4856bc453c01 100644
--- a/arch/arm64/include/asm/entry-common.h
+++ b/arch/arm64/include/asm/entry-common.h
@@ -27,7 +27,7 @@ static __always_inline void arch_exit_to_user_mode_work(struct pt_regs *regs,
 
 #define arch_exit_to_user_mode_work arch_exit_to_user_mode_work
 
-static inline bool arch_irqentry_exit_need_resched(void)
+static __always_inline bool arch_irqentry_exit_need_resched(void)
 {
 	/*
 	 * DAIF.DA are cleared at the start of IRQ/FIQ handling, and when GIC
diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
index a67d5774e672..157d0551f627 100644
--- a/arch/arm64/include/asm/fpsimd.h
+++ b/arch/arm64/include/asm/fpsimd.h
@@ -749,14 +749,14 @@ void sme_enable_dvmsync(void);
 void sme_set_active(void);
 void sme_clear_active(void);
 
-static inline void sme_enter_from_user_mode(void)
+static __always_inline void sme_enter_from_user_mode(void)
 {
 	if (alternative_has_cap_unlikely(ARM64_WORKAROUND_4193714) &&
 	    test_thread_flag(TIF_SME))
 		sme_clear_active();
 }
 
-static inline void sme_exit_to_user_mode(void)
+static __always_inline void sme_exit_to_user_mode(void)
 {
 	if (alternative_has_cap_unlikely(ARM64_WORKAROUND_4193714) &&
 	    test_thread_flag(TIF_SME))
diff --git a/arch/arm64/include/asm/mte-kasan.h b/arch/arm64/include/asm/mte-kasan.h
index de910f1c19e1..cc2151325eb0 100644
--- a/arch/arm64/include/asm/mte-kasan.h
+++ b/arch/arm64/include/asm/mte-kasan.h
@@ -18,14 +18,14 @@
 /* Whether the MTE asynchronous mode is enabled. */
 DECLARE_STATIC_KEY_FALSE(mte_async_or_asymm_mode);
 
-static inline bool system_uses_mte_async_or_asymm_mode(void)
+static __always_inline bool system_uses_mte_async_or_asymm_mode(void)
 {
 	return static_branch_unlikely(&mte_async_or_asymm_mode);
 }
 
 #else /* CONFIG_KASAN_HW_TAGS */
 
-static inline bool system_uses_mte_async_or_asymm_mode(void)
+static __always_inline bool system_uses_mte_async_or_asymm_mode(void)
 {
 	return false;
 }
diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index 83f3b05fc784..b2bf2e70c9ae 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -222,7 +222,7 @@ static inline bool folio_try_hugetlb_mte_tagging(struct folio *folio)
 }
 #endif
 
-static inline void mte_disable_tco_entry(struct task_struct *task)
+static __always_inline void mte_disable_tco_entry(struct task_struct *task)
 {
 	if (!system_supports_mte())
 		return;
@@ -245,7 +245,7 @@ static inline void mte_disable_tco_entry(struct task_struct *task)
 #ifdef CONFIG_KASAN_HW_TAGS
 void mte_check_tfsr_el1(void);
 
-static inline void mte_check_tfsr_entry(void)
+static __always_inline void mte_check_tfsr_entry(void)
 {
 	if (!kasan_hw_tags_enabled())
 		return;
@@ -256,7 +256,7 @@ static inline void mte_check_tfsr_entry(void)
 	mte_check_tfsr_el1();
 }
 
-static inline void mte_check_tfsr_exit(void)
+static __always_inline void mte_check_tfsr_exit(void)
 {
 	if (!kasan_hw_tags_enabled())
 		return;
-- 
2.55.0

Re: [PATCH 2/4] arm64: entry: Always inline helpers used by non-instrumentable paths

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Low] arm64: entry: kasan_hw_tags_enabled() not marked __always_inline
  for noinstr
- [Low] arm64: entry: sme_{enter,exit} helpers call non-noinstr
  test_thread_flag()
- [Low] arm64: entry: #else stubs for MTE and SME helpers not marked
  __always_inline
--

commit 36003a6dfa6a3ccb4d813e94b64d8956098d7ce8
Author: Jisheng Zhang <jszhang@kernel.org>

arm64: entry: Always inline helpers used by non-instrumentable paths

This commit changes several MTE, SME, CPU feature, and entry helpers from
plain inline to __always_inline so they are not emitted out-of-line in
.text when called from noinstr entry and exit paths.

> diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
> index a67d5774e6728..157d0551f6279 100644
> --- a/arch/arm64/include/asm/fpsimd.h
> +++ b/arch/arm64/include/asm/fpsimd.h
> @@ -749,14 +749,14 @@ void sme_enable_dvmsync(void);
>  void sme_set_active(void);
>  void sme_clear_active(void);
>  
> -static inline void sme_enter_from_user_mode(void)
> +static __always_inline void sme_enter_from_user_mode(void)
>  {
>  	if (alternative_has_cap_unlikely(ARM64_WORKAROUND_4193714) &&
>  	    test_thread_flag(TIF_SME))
>  		sme_clear_active();
>  }
>  
> -static inline void sme_exit_to_user_mode(void)
> +static __always_inline void sme_exit_to_user_mode(void)
>  {
>  	if (alternative_has_cap_unlikely(ARM64_WORKAROUND_4193714) &&
>  	    test_thread_flag(TIF_SME))

[Severity: Low]
Should sme_enter_from_user_mode() and sme_exit_to_user_mode() use
tif_test_bit(TIF_SME) instead of test_thread_flag(TIF_SME)?

test_thread_flag() expands to test_ti_thread_flag(), which is plain static
inline and calls instrumented test_bit():

include/linux/thread_info.h:test_ti_thread_flag() {
	return test_bit(flag, (unsigned long *)&ti->flags);
}

Inlining these SME helpers across the noinstr EL0 entry and exit paths in
arch/arm64/kernel/entry-common.c allows the compiler to outline
test_ti_thread_flag() into .text under CONFIG_CC_OPTIMIZE_FOR_SIZE=y or emit
bitop instrumentation in .noinstr.text, whereas tif_test_bit() is already
marked __always_inline and avoids bitop instrumentation for noinstr callers.

[ ... ]

> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> index 83f3b05fc7849..b2bf2e70c9aec 100644
> --- a/arch/arm64/include/asm/mte.h
> +++ b/arch/arm64/include/asm/mte.h
> @@ -222,7 +222,7 @@ static inline bool folio_try_hugetlb_mte_tagging(struct folio *folio)
>  }
>  #endif
>  
> -static inline void mte_disable_tco_entry(struct task_struct *task)
> +static __always_inline void mte_disable_tco_entry(struct task_struct *task)
>  {
>  	if (!system_supports_mte())
>  		return;
> @@ -245,7 +245,7 @@ static inline void mte_disable_tco_entry(struct task_struct *task)
>  #ifdef CONFIG_KASAN_HW_TAGS
>  void mte_check_tfsr_el1(void);
>  
> -static inline void mte_check_tfsr_entry(void)
> +static __always_inline void mte_check_tfsr_entry(void)
>  {
>  	if (!kasan_hw_tags_enabled())
>  		return;

[Severity: Low]
Does kasan_hw_tags_enabled() also need to be marked __always_inline?

mte_disable_tco_entry(), mte_check_tfsr_entry(), and mte_check_tfsr_exit()
all call kasan_hw_tags_enabled(), which is still defined as plain static
inline in include/linux/kasan-enabled.h (even though the underlying
kasan_enabled() helper is already __always_inline). Force-inlining the MTE
entry and exit helpers into arch/arm64/kernel/entry-common.c expands
kasan_hw_tags_enabled() across dozens of noinstr call sites, so GCC with
CONFIG_CC_OPTIMIZE_FOR_SIZE=y and CONFIG_KASAN_HW_TAGS=y can still emit an
out-of-line copy of kasan_hw_tags_enabled() in .text.

> @@ -256,7 +256,7 @@ static inline void mte_check_tfsr_entry(void)
>  	mte_check_tfsr_el1();
>  }
>  
> -static inline void mte_check_tfsr_exit(void)
> +static __always_inline void mte_check_tfsr_exit(void)
>  {
>  	if (!kasan_hw_tags_enabled())
>  		return;

[Severity: Low]
Should the #else stubs for mte_check_tfsr_entry() and mte_check_tfsr_exit()
(when !CONFIG_KASAN_HW_TAGS) and for sme_enter_from_user_mode() and
sme_exit_to_user_mode() in arch/arm64/include/asm/fpsimd.h (when
!CONFIG_ARM64_SME) also be marked __always_inline to match
system_uses_mte_async_or_asymm_mode()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107071339.17467-1-jszhang@kernel.org?part=2

[PATCH 3/4] arm64: stacktrace: Always inline internal stack boundary helpers

[Jisheng Zhang]

The noinstr stack walkers reach the stack boundary and frame-record
helpers through an __always_inline call chain. However, these shared
helpers do not themselves guarantee inlining.

With CONFIG_CC_OPTIMIZE_FOR_SIZE=y, GCC emitted several stack
information helpers and unwind_find_stack() out of line in .text.

Mark the shared stack boundary and frame-record helpers
__always_inline to keep their instructions in the non-instrumentable
walkers

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Assisted-by: LLM
---
 arch/arm64/include/asm/stacktrace.h        | 16 ++++++++--------
 arch/arm64/include/asm/stacktrace/common.h | 12 ++++++------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h
index 6d3280932bf5..b370c0e1e337 100644
--- a/arch/arm64/include/asm/stacktrace.h
+++ b/arch/arm64/include/asm/stacktrace.h
@@ -22,7 +22,7 @@ extern void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk,
 
 DECLARE_PER_CPU(unsigned long *, irq_stack_ptr);
 
-static inline struct stack_info stackinfo_get_irq(void)
+static __always_inline struct stack_info stackinfo_get_irq(void)
 {
 	unsigned long low = (unsigned long)raw_cpu_read(irq_stack_ptr);
 	unsigned long high = low + IRQ_STACK_SIZE;
@@ -33,13 +33,13 @@ static inline struct stack_info stackinfo_get_irq(void)
 	};
 }
 
-static inline bool on_irq_stack(unsigned long sp, unsigned long size)
+static __always_inline bool on_irq_stack(unsigned long sp, unsigned long size)
 {
 	struct stack_info info = stackinfo_get_irq();
 	return stackinfo_on_stack(&info, sp, size);
 }
 
-static inline struct stack_info stackinfo_get_task(const struct task_struct *tsk)
+static __always_inline struct stack_info stackinfo_get_task(const struct task_struct *tsk)
 {
 	unsigned long low = (unsigned long)task_stack_page(tsk);
 	unsigned long high = low + THREAD_SIZE;
@@ -50,7 +50,7 @@ static inline struct stack_info stackinfo_get_task(const struct task_struct *tsk
 	};
 }
 
-static inline bool on_task_stack(const struct task_struct *tsk,
+static __always_inline bool on_task_stack(const struct task_struct *tsk,
 				 unsigned long sp, unsigned long size)
 {
 	struct stack_info info = stackinfo_get_task(tsk);
@@ -61,7 +61,7 @@ static inline bool on_task_stack(const struct task_struct *tsk,
 
 DECLARE_PER_CPU(unsigned long [OVERFLOW_STACK_SIZE/sizeof(long)], overflow_stack);
 
-static inline struct stack_info stackinfo_get_overflow(void)
+static __always_inline struct stack_info stackinfo_get_overflow(void)
 {
 	unsigned long low = (unsigned long)raw_cpu_ptr(overflow_stack);
 	unsigned long high = low + OVERFLOW_STACK_SIZE;
@@ -76,7 +76,7 @@ static inline struct stack_info stackinfo_get_overflow(void)
 DECLARE_PER_CPU(unsigned long *, sdei_stack_normal_ptr);
 DECLARE_PER_CPU(unsigned long *, sdei_stack_critical_ptr);
 
-static inline struct stack_info stackinfo_get_sdei_normal(void)
+static __always_inline struct stack_info stackinfo_get_sdei_normal(void)
 {
 	unsigned long low = (unsigned long)raw_cpu_read(sdei_stack_normal_ptr);
 	unsigned long high = low + SDEI_STACK_SIZE;
@@ -87,7 +87,7 @@ static inline struct stack_info stackinfo_get_sdei_normal(void)
 	};
 }
 
-static inline struct stack_info stackinfo_get_sdei_critical(void)
+static __always_inline struct stack_info stackinfo_get_sdei_critical(void)
 {
 	unsigned long low = (unsigned long)raw_cpu_read(sdei_stack_critical_ptr);
 	unsigned long high = low + SDEI_STACK_SIZE;
@@ -105,7 +105,7 @@ static inline struct stack_info stackinfo_get_sdei_critical(void)
 #ifdef CONFIG_EFI
 extern u64 *efi_rt_stack_top;
 
-static inline struct stack_info stackinfo_get_efi(void)
+static __always_inline struct stack_info stackinfo_get_efi(void)
 {
 	unsigned long high = (u64)efi_rt_stack_top;
 	unsigned long low = high - THREAD_SIZE;
diff --git a/arch/arm64/include/asm/stacktrace/common.h b/arch/arm64/include/asm/stacktrace/common.h
index 821a8fdd31af..7c502ae276b9 100644
--- a/arch/arm64/include/asm/stacktrace/common.h
+++ b/arch/arm64/include/asm/stacktrace/common.h
@@ -35,7 +35,7 @@ struct unwind_state {
 	int nr_stacks;
 };
 
-static inline struct stack_info stackinfo_get_unknown(void)
+static __always_inline struct stack_info stackinfo_get_unknown(void)
 {
 	return (struct stack_info) {
 		.low = 0,
@@ -43,7 +43,7 @@ static inline struct stack_info stackinfo_get_unknown(void)
 	};
 }
 
-static inline bool stackinfo_on_stack(const struct stack_info *info,
+static __always_inline bool stackinfo_on_stack(const struct stack_info *info,
 				      unsigned long sp, unsigned long size)
 {
 	if (!info->low)
@@ -55,7 +55,7 @@ static inline bool stackinfo_on_stack(const struct stack_info *info,
 	return true;
 }
 
-static inline void unwind_init_common(struct unwind_state *state)
+static __always_inline void unwind_init_common(struct unwind_state *state)
 {
 	state->stack = stackinfo_get_unknown();
 }
@@ -70,7 +70,7 @@ static inline void unwind_init_common(struct unwind_state *state)
  *
  * Return: a pointer to the relevant stack_info if found; NULL otherwise.
  */
-static struct stack_info *unwind_find_stack(struct unwind_state *state,
+static __always_inline struct stack_info *unwind_find_stack(struct unwind_state *state,
 					    unsigned long sp,
 					    unsigned long size)
 {
@@ -99,7 +99,7 @@ static struct stack_info *unwind_find_stack(struct unwind_state *state,
  *
  * Return: 0 upon success, an error code otherwise.
  */
-static inline void unwind_consume_stack(struct unwind_state *state,
+static __always_inline void unwind_consume_stack(struct unwind_state *state,
 					struct stack_info *info,
 					unsigned long sp,
 					unsigned long size)
@@ -142,7 +142,7 @@ static inline void unwind_consume_stack(struct unwind_state *state,
  *
  * Return: 0 upon success, an error code otherwise.
  */
-static inline int
+static __always_inline int
 unwind_next_frame_record(struct unwind_state *state)
 {
 	struct stack_info *info;
-- 
2.55.0

[PATCH 4/4] arm64: alternatives: Make branch and ADRP offset helpers noinst

[Jisheng Zhang]

patch_alternative() must avoid instrumented code while alternatives
are being modified, before cache maintenance completes. However, its
branch and ADRP offset helpers are instrumentable. A GCC build with
function tracing enabled emitted ftrace patchable entries for these
helpers.

Mark the four offset helpers noinstr. Introduce private
__always_inline immediate encoding and decoding implementations for
their use, and force inline the immediate mask helper.

The offset helpers pass constant encoding types, allowing the compiler
to eliminate invalid-type diagnostic paths. Keep the public immediate
helpers as wrappers to preserve diagnostics for other callers.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Assisted-by: LLM
---
 arch/arm64/lib/insn.c | 57 ++++++++++++++++++++++++++++---------------
 1 file changed, 37 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/lib/insn.c b/arch/arm64/lib/insn.c
index e70ac0238515..b98610fcab32 100644
--- a/arch/arm64/lib/insn.c
+++ b/arch/arm64/lib/insn.c
@@ -21,7 +21,7 @@
 #define AARCH64_INSN_N_BIT	BIT(22)
 #define AARCH64_INSN_LSL_12	BIT(22)
 
-static int __kprobes aarch64_get_imm_shift_mask(enum aarch64_insn_imm_type type,
+static __always_inline int aarch64_get_imm_shift_mask(enum aarch64_insn_imm_type type,
 						u32 *maskp, int *shiftp)
 {
 	u32 mask;
@@ -86,7 +86,12 @@ static int __kprobes aarch64_get_imm_shift_mask(enum aarch64_insn_imm_type type,
 #define ADR_IMM_LOSHIFT		29
 #define ADR_IMM_HISHIFT		5
 
-u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn)
+/*
+ * Keep the immediate helpers inline so noinstr callers using constant encoding
+ * types do not call instrumentable code, including the invalid-type diagnostics.
+ */
+static __always_inline u64
+__aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn)
 {
 	u32 immlo, immhi, mask;
 	int shift;
@@ -101,8 +106,8 @@ u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn)
 		break;
 	default:
 		if (aarch64_get_imm_shift_mask(type, &mask, &shift) < 0) {
-			pr_err("%s: unknown immediate encoding %d\n", __func__,
-			       type);
+			pr_err("%s: unknown immediate encoding %d\n",
+			       "aarch64_insn_decode_immediate", type);
 			return 0;
 		}
 	}
@@ -110,8 +115,14 @@ u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn)
 	return (insn >> shift) & mask;
 }
 
-u32 __kprobes aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
-				  u32 insn, u64 imm)
+u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn)
+{
+	return __aarch64_insn_decode_immediate(type, insn);
+}
+
+static __always_inline u32
+__aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
+			      u32 insn, u64 imm)
 {
 	u32 immlo, immhi, mask;
 	int shift;
@@ -131,8 +142,8 @@ u32 __kprobes aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
 		break;
 	default:
 		if (aarch64_get_imm_shift_mask(type, &mask, &shift) < 0) {
-			pr_err("%s: unknown immediate encoding %d\n", __func__,
-			       type);
+			pr_err("%s: unknown immediate encoding %d\n",
+			       "aarch64_insn_encode_immediate", type);
 			return AARCH64_BREAK_FAULT;
 		}
 	}
@@ -144,6 +155,12 @@ u32 __kprobes aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
 	return insn;
 }
 
+u32 __kprobes aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
+					  u32 insn, u64 imm)
+{
+	return __aarch64_insn_encode_immediate(type, insn, imm);
+}
+
 u32 aarch64_insn_decode_register(enum aarch64_insn_register_type type,
 					u32 insn)
 {
@@ -1299,23 +1316,23 @@ u32 aarch64_insn_gen_adr(unsigned long pc, unsigned long addr,
  * signed value (so it can be used when computing a new branch
  * target).
  */
-s32 aarch64_get_branch_offset(u32 insn)
+s32 noinstr aarch64_get_branch_offset(u32 insn)
 {
 	s32 imm;
 
 	if (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn)) {
-		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_26, insn);
+		imm = __aarch64_insn_decode_immediate(AARCH64_INSN_IMM_26, insn);
 		return (imm << 6) >> 4;
 	}
 
 	if (aarch64_insn_is_cbz(insn) || aarch64_insn_is_cbnz(insn) ||
 	    aarch64_insn_is_bcond(insn)) {
-		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_19, insn);
+		imm = __aarch64_insn_decode_immediate(AARCH64_INSN_IMM_19, insn);
 		return (imm << 13) >> 11;
 	}
 
 	if (aarch64_insn_is_tbz(insn) || aarch64_insn_is_tbnz(insn)) {
-		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_14, insn);
+		imm = __aarch64_insn_decode_immediate(AARCH64_INSN_IMM_14, insn);
 		return (imm << 18) >> 16;
 	}
 
@@ -1327,35 +1344,35 @@ s32 aarch64_get_branch_offset(u32 insn)
  * Encode the displacement of a branch in the imm field and return the
  * updated instruction.
  */
-u32 aarch64_set_branch_offset(u32 insn, s32 offset)
+u32 noinstr aarch64_set_branch_offset(u32 insn, s32 offset)
 {
 	if (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn))
-		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_26, insn,
+		return __aarch64_insn_encode_immediate(AARCH64_INSN_IMM_26, insn,
 						     offset >> 2);
 
 	if (aarch64_insn_is_cbz(insn) || aarch64_insn_is_cbnz(insn) ||
 	    aarch64_insn_is_bcond(insn))
-		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_19, insn,
+		return __aarch64_insn_encode_immediate(AARCH64_INSN_IMM_19, insn,
 						     offset >> 2);
 
 	if (aarch64_insn_is_tbz(insn) || aarch64_insn_is_tbnz(insn))
-		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_14, insn,
+		return __aarch64_insn_encode_immediate(AARCH64_INSN_IMM_14, insn,
 						     offset >> 2);
 
 	/* Unhandled instruction */
 	BUG();
 }
 
-s32 aarch64_insn_adrp_get_offset(u32 insn)
+s32 noinstr aarch64_insn_adrp_get_offset(u32 insn)
 {
 	BUG_ON(!aarch64_insn_is_adrp(insn));
-	return aarch64_insn_decode_immediate(AARCH64_INSN_IMM_ADR, insn) << 12;
+	return __aarch64_insn_decode_immediate(AARCH64_INSN_IMM_ADR, insn) << 12;
 }
 
-u32 aarch64_insn_adrp_set_offset(u32 insn, s32 offset)
+u32 noinstr aarch64_insn_adrp_set_offset(u32 insn, s32 offset)
 {
 	BUG_ON(!aarch64_insn_is_adrp(insn));
-	return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_ADR, insn,
+	return __aarch64_insn_encode_immediate(AARCH64_INSN_IMM_ADR, insn,
 						offset >> 12);
 }
 
-- 
2.55.0

Re: [PATCH 4/4] arm64: alternatives: Make branch and ADRP offset helpers noinst

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] arm64: insn: branch profiling instrumentation in noinstr
  offset helpers
--

commit b08429609214cdffaa257c9ef953b28e179973db
Author: Jisheng Zhang <jszhang@kernel.org>

arm64: alternatives: Make branch and ADRP offset helpers noinst

This commit marks the branch and ADRP offset helpers noinstr and introduces
__always_inline immediate encoding and decoding helpers so that
patch_alternative() does not call instrumentable code while modifying
instructions.

> diff --git a/arch/arm64/lib/insn.c b/arch/arm64/lib/insn.c
> index e70ac02385153..b98610fcab32e 100644
> --- a/arch/arm64/lib/insn.c
> +++ b/arch/arm64/lib/insn.c

[ ... ]

> @@ -1299,23 +1316,23 @@ u32 aarch64_insn_gen_adr(unsigned long pc, unsigned long addr,
>   * signed value (so it can be used when computing a new branch
>   * target).
>   */
> -s32 aarch64_get_branch_offset(u32 insn)
> +s32 noinstr aarch64_get_branch_offset(u32 insn)
>  {
>  	s32 imm;
>  
>  	if (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn)) {
> -		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_26, insn);
> +		imm = __aarch64_insn_decode_immediate(AARCH64_INSN_IMM_26, insn);
>  		return (imm << 6) >> 4;
>  	}
>  
>  	if (aarch64_insn_is_cbz(insn) || aarch64_insn_is_cbnz(insn) ||
>  	    aarch64_insn_is_bcond(insn)) {
> -		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_19, insn);
> +		imm = __aarch64_insn_decode_immediate(AARCH64_INSN_IMM_19, insn);
>  		return (imm << 13) >> 11;
>  	}
>  
>  	if (aarch64_insn_is_tbz(insn) || aarch64_insn_is_tbnz(insn)) {
> -		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_14, insn);
> +		imm = __aarch64_insn_decode_immediate(AARCH64_INSN_IMM_14, insn);
>  		return (imm << 18) >> 16;
>  	}
>  
> @@ -1327,35 +1344,35 @@ s32 aarch64_get_branch_offset(u32 insn)
>   * Encode the displacement of a branch in the imm field and return the
>   * updated instruction.
>   */
> -u32 aarch64_set_branch_offset(u32 insn, s32 offset)
> +u32 noinstr aarch64_set_branch_offset(u32 insn, s32 offset)
>  {
>  	if (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn))
> -		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_26, insn,
> +		return __aarch64_insn_encode_immediate(AARCH64_INSN_IMM_26, insn,
>  						     offset >> 2);
>  
>  	if (aarch64_insn_is_cbz(insn) || aarch64_insn_is_cbnz(insn) ||
>  	    aarch64_insn_is_bcond(insn))
> -		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_19, insn,
> +		return __aarch64_insn_encode_immediate(AARCH64_INSN_IMM_19, insn,
>  						     offset >> 2);
>  
>  	if (aarch64_insn_is_tbz(insn) || aarch64_insn_is_tbnz(insn))
> -		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_14, insn,
> +		return __aarch64_insn_encode_immediate(AARCH64_INSN_IMM_14, insn,
>  						     offset >> 2);
>  
>  	/* Unhandled instruction */
>  	BUG();
>  }
>  
> -s32 aarch64_insn_adrp_get_offset(u32 insn)
> +s32 noinstr aarch64_insn_adrp_get_offset(u32 insn)
>  {
>  	BUG_ON(!aarch64_insn_is_adrp(insn));
> -	return aarch64_insn_decode_immediate(AARCH64_INSN_IMM_ADR, insn) << 12;
> +	return __aarch64_insn_decode_immediate(AARCH64_INSN_IMM_ADR, insn) << 12;
>  }
>  
> -u32 aarch64_insn_adrp_set_offset(u32 insn, s32 offset)
> +u32 noinstr aarch64_insn_adrp_set_offset(u32 insn, s32 offset)
>  {
>  	BUG_ON(!aarch64_insn_is_adrp(insn));
> -	return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_ADR, insn,
> +	return __aarch64_insn_encode_immediate(AARCH64_INSN_IMM_ADR, insn,
>  						offset >> 12);
>  }

[Severity: Medium]
Can these four noinstr helpers still call into ftrace_likely_update() when
CONFIG_TRACE_BRANCH_PROFILING is enabled?

Although f22c81bebf8b ("arm64: Disable branch profiling for all arm64 code")
added -DDISABLE_BRANCH_PROFILING to arch/arm64/Kbuild, arch/arm64/lib/ is
built via libs-y in arch/arm64/Makefile rather than arch/arm64/Kbuild, so
arch/arm64/lib/insn.o is compiled without DISABLE_BRANCH_PROFILING.

As a result, the generic BUG_ON() in aarch64_insn_adrp_get_offset() and
aarch64_insn_adrp_set_offset() expands to unlikely(), and the if statements
in aarch64_get_branch_offset() and aarch64_set_branch_offset() are
instrumented under CONFIG_PROFILE_ALL_BRANCHES.  When patch_alternative()
calls these helpers while patching live instructions before cache
maintenance completes, they call the instrumentable ftrace_likely_update().

Passing -DDISABLE_BRANCH_PROFILING in arch/arm64/lib/Makefile (or defining
DISABLE_BRANCH_PROFILING in arch/arm64/lib/insn.c) would prevent branch
profiling instrumentation here.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107071339.17467-1-jszhang@kernel.org?part=4