[RFC PATCH] arm64: jump_label: Ensure patched jump_labels are visible to all CPUs
Catalin Marinas
catalin.marinas at arm.com
Thu Aug 1 08:53:33 PDT 2024
On Wed, Jul 31, 2024 at 02:36:01PM +0100, Will Deacon wrote:
> Although the Arm architecture permits concurrent modification and
> execution of NOP and branch instructions, it still requires some
> synchronisation to ensure that other CPUs consistently execute the newly
> written instruction:
>
> > When the modified instructions are observable, each PE that is
> > executing the modified instructions must execute an ISB or perform a
> > context synchronizing event to ensure execution of the modified
> > instructions
>
> Prior to commit f6cc0c501649 ("arm64: Avoid calling stop_machine() when
> patching jump labels"), the arm64 jump_label patching machinery
> performed synchronisation using stop_machine() after each modification,
> however this was problematic when flipping static keys from atomic
> contexts (namely, the arm_arch_timer CPU hotplug startup notifier) and
> so we switched to the _nosync() patching routines to avoid "scheduling
> while atomic" BUG()s during boot.
>
> In hindsight, the analysis of the issue in f6cc0c501649 isn't quite
> right: it cites the use of IPIs in the default patching routines as the
> cause of the lockup, whereas stop_machine() does not rely on IPIs and
> the I-cache invalidation is performed using __flush_icache_range(),
> which elides the call to kick_all_cpus_sync(). In fact, the blocking
> wait for other CPUs is what triggers the BUG() and the problem remains
> even after f6cc0c501649, for example because we could block on the
> jump_label_mutex. Eventually, the arm_arch_timer driver was fixed to
> avoid the static key entirely in commit a862fc2254bd
> ("clocksource/arm_arch_timer: Remove use of workaround static key").
>
> This all leaves the jump_label patching code in a funny situation on
> arm64 as we do not synchronise with other CPUs to reduce the likelihood
> of a bug which no longer exists. Consequently, toggling a static key on
> one CPU cannot be assumed to take effect on other CPUs, leading to
> potential issues, for example with missing preempt notifiers.
>
> Rather than revert f6cc0c501649 and go back to stop_machine() for each
> patch site, implement arch_jump_label_transform_apply() and kick all
> the other CPUs with an IPI at the end of patching.
>
> Cc: Catalin Marinas <catalin.marinas at arm.com>
> Cc: Alexander Potapenko <glider at google.com>
> Cc: Mark Rutland <mark.rutland at arm.com>
> Cc: Marc Zyngier <maz at kernel.org>
> Fixes: f6cc0c501649 ("arm64: Avoid calling stop_machine() when patching jump labels")
> Signed-off-by: Will Deacon <will at kernel.org>
We need to keep an eye so that the patch is not picked up for 4.19 (the
fixed commit) as it doesn't have the arm_arch_timer fix, nor the batch
jump label support. LTS 5.4 is fine though, it has both. If we want it
in -stable, we can explicitly mention the version in the Cc line.
The patch looks good to me.
Reviewed-by: Catalin Marinas <catalin.marinas at arm.com>
More information about the linux-arm-kernel
mailing list