Re: [PATCH v3 2/7] arm64/runtime-const: Use aarch64_insn_patch_text_nosync() for patching

From: Catalin Marinas <catalin.marinas@arm.com>
To: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: "Thomas Gleixner" <tglx@kernel.org>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Sebastian Andrzej Siewior" <bigeasy@linutronix.de>,
	"Will Deacon" <will@kernel.org>,
	"David Laight" <david.laight.linux@gmail.com>,
	"Darren Hart" <dvhart@infradead.org>,
	"Davidlohr Bueso" <dave@stgolabs.net>,
	"André Almeida" <andrealmeid@igalia.com>,
	linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-s390@vger.kernel.org, linux-riscv@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	"Jisheng Zhang" <jszhang@kernel.org>
Subject: Re: [PATCH v3 2/7] arm64/runtime-const: Use aarch64_insn_patch_text_nosync() for patching
Date: Mon, 13 Apr 2026 12:01:05 +0100	[thread overview]
Message-ID: <adzM8RtjbOy0kLOC@arm.com> (raw)
In-Reply-To: <c45ede13-f8d0-4b6a-b2ed-f06af4882ebc@amd.com>

On Sun, Apr 12, 2026 at 01:24:45AM +0530, K Prateek Nayak wrote:
> On 4/10/2026 3:07 PM, Catalin Marinas wrote:
> > https://sashiko.dev/#/patchset/20260402112250.2138-1-kprateek.nayak@amd.com
> > 
> > In short, aarch64_insn_patch_text_nosync() does not expect a linear map
> > address but rather a kernel text one (or vmalloc/modules). The other
> > valid point is on aliasing I-caches.
> > 
> > I think dropping the lm_alias() and just use 'where' directly would do
> > but I haven't tried.
> 
> Ack! I completely missed that subtlety of passing "where" to
> caches_clean_inval_pou(). I'm still surprised that it didn't
> blow up in my testing.

For the first part (passing a linear map address), I suspect we get away
with this in vmalloc_to_page() as it just walks the page tables and
VIRTUAL_BUG_ON() is a no-op with defconfig.

For the I-cache aliasing, you may not have the right hardware but even
if you did, it's harder to hit.

> Anyhow, following diff, on top of the full series builds and
> tests fine and has been blessed by review-prompts:
> 
> diff --git a/arch/arm64/include/asm/runtime-const.h b/arch/arm64/include/asm/runtime-const.h
> index 21f817eb5951..d3f0dfa7ced0 100644
> --- a/arch/arm64/include/asm/runtime-const.h
> +++ b/arch/arm64/include/asm/runtime-const.h
> @@ -57,21 +57,21 @@
>  } while (0)
>  
>  /* 16-bit immediate for wide move (movz and movk) in bits 5..20 */
> -static inline void __runtime_fixup_16(__le32 *p, unsigned int val)
> +static inline void __runtime_fixup_16(void *where, unsigned int val)
>  {
> +	__le32 *p = lm_alias(where);
>  	u32 insn = le32_to_cpu(*p);
>  	insn &= 0xffe0001f;
>  	insn |= (val & 0xffff) << 5;
> -	aarch64_insn_patch_text_nosync(p, insn);
> +	aarch64_insn_patch_text_nosync(where, insn);
>  }

You might as well keep the __le32 *p argument and just dereference it
directly, no need for lm_alias(). The kernel text is readable and you
avoid having to change all the other functions.

>  static inline void __runtime_fixup_ptr(void *where, unsigned long val)
>  {
> -	__le32 *p = lm_alias(where);
> -	__runtime_fixup_16(p, val);
> -	__runtime_fixup_16(p+1, val >> 16);
> -	__runtime_fixup_16(p+2, val >> 32);
> -	__runtime_fixup_16(p+3, val >> 48);
> +	__runtime_fixup_16(where, val);
> +	__runtime_fixup_16(where + 4, val >> 16);
> +	__runtime_fixup_16(where + 8, val >> 32);
> +	__runtime_fixup_16(where + 12, val >> 48);
>  }

And here change the argument to '__le32 *p' (and in other places where
you changed p to where + 4 etc.).

-- 
Catalin