* [PATCH] arm64/alternatives: don't patch up internal branches
@ 2020-07-09 12:59 Ard Biesheuvel
2020-07-09 13:14 ` Alexandru Elisei
2020-07-09 15:49 ` Will Deacon
0 siblings, 2 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2020-07-09 12:59 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: catalin.marinas, will, Ard Biesheuvel, alexandru.elisei
Commit f7b93d429 ("arm64/alternatives: use subsections for replacement
sequences") moved the alternatives replacement sequences into subsections,
in order to keep the as close as possible to the code that they replace.
Unfortunately, this broke the logic in branch_insn_requires_update,
which assumed that any branch into kernel executable code was a branch
that required updating, which is no longer the case now that the code
sequences that are patched in are in the same section as the patch site
itself.
So the only way to discriminate branches that require updating and ones
that don't is to check whether the branch targets the replacement sequence
itself, and so we can drop the call to kernel_text_address() entirely.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm64/kernel/alternative.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c
index d1757ef1b1e7..5711adda14bb 100644
--- a/arch/arm64/kernel/alternative.c
+++ b/arch/arm64/kernel/alternative.c
@@ -45,18 +45,8 @@ static bool branch_insn_requires_update(struct alt_instr *alt, unsigned long pc)
{
unsigned long replptr;
- if (kernel_text_address(pc))
- return true;
-
replptr = (unsigned long)ALT_REPL_PTR(alt);
- if (pc >= replptr && pc <= (replptr + alt->alt_len))
- return false;
-
- /*
- * Branching into *another* alternate sequence is doomed, and
- * we're not even trying to fix it up.
- */
- BUG();
+ return !(pc >= replptr && pc <= (replptr + alt->alt_len));
}
#define align_down(x, a) ((unsigned long)(x) & ~(((unsigned long)(a)) - 1))
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] arm64/alternatives: don't patch up internal branches
2020-07-09 12:59 [PATCH] arm64/alternatives: don't patch up internal branches Ard Biesheuvel
@ 2020-07-09 13:14 ` Alexandru Elisei
2020-07-09 15:49 ` Will Deacon
1 sibling, 0 replies; 3+ messages in thread
From: Alexandru Elisei @ 2020-07-09 13:14 UTC (permalink / raw)
To: Ard Biesheuvel, linux-arm-kernel; +Cc: catalin.marinas, will
Hi Ard,
On 7/9/20 1:59 PM, Ard Biesheuvel wrote:
> Commit f7b93d429 ("arm64/alternatives: use subsections for replacement
> sequences") moved the alternatives replacement sequences into subsections,
> in order to keep the as close as possible to the code that they replace.
>
> Unfortunately, this broke the logic in branch_insn_requires_update,
> which assumed that any branch into kernel executable code was a branch
> that required updating, which is no longer the case now that the code
> sequences that are patched in are in the same section as the patch site
> itself.
>
> So the only way to discriminate branches that require updating and ones
> that don't is to check whether the branch targets the replacement sequence
> itself, and so we can drop the call to kernel_text_address() entirely.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/arm64/kernel/alternative.c | 12 +-----------
> 1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c
> index d1757ef1b1e7..5711adda14bb 100644
> --- a/arch/arm64/kernel/alternative.c
> +++ b/arch/arm64/kernel/alternative.c
> @@ -45,18 +45,8 @@ static bool branch_insn_requires_update(struct alt_instr *alt, unsigned long pc)
> {
> unsigned long replptr;
>
> - if (kernel_text_address(pc))
> - return true;
> -
> replptr = (unsigned long)ALT_REPL_PTR(alt);
> - if (pc >= replptr && pc <= (replptr + alt->alt_len))
> - return false;
> -
> - /*
> - * Branching into *another* alternate sequence is doomed, and
> - * we're not even trying to fix it up.
> - */
> - BUG();
> + return !(pc >= replptr && pc <= (replptr + alt->alt_len));
> }
>
> #define align_down(x, a) ((unsigned long)(x) & ~(((unsigned long)(a)) - 1))
Ran some perf tests on the model and rockpro64 using the PMU interrupt as an NMI,
everything works as expected:
Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
Thanks,
Alex
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] arm64/alternatives: don't patch up internal branches
2020-07-09 12:59 [PATCH] arm64/alternatives: don't patch up internal branches Ard Biesheuvel
2020-07-09 13:14 ` Alexandru Elisei
@ 2020-07-09 15:49 ` Will Deacon
1 sibling, 0 replies; 3+ messages in thread
From: Will Deacon @ 2020-07-09 15:49 UTC (permalink / raw)
To: linux-arm-kernel, Ard Biesheuvel
Cc: catalin.marinas, kernel-team, Will Deacon, alexandru.elisei
On Thu, 9 Jul 2020 15:59:53 +0300, Ard Biesheuvel wrote:
> Commit f7b93d429 ("arm64/alternatives: use subsections for replacement
> sequences") moved the alternatives replacement sequences into subsections,
> in order to keep the as close as possible to the code that they replace.
>
> Unfortunately, this broke the logic in branch_insn_requires_update,
> which assumed that any branch into kernel executable code was a branch
> that required updating, which is no longer the case now that the code
> sequences that are patched in are in the same section as the patch site
> itself.
>
> [...]
Applied to arm64 (for-next/fixes), thanks!
[1/1] arm64/alternatives: don't patch up internal branches
https://git.kernel.org/arm64/c/5679b2814219
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-07-09 15:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-09 12:59 [PATCH] arm64/alternatives: don't patch up internal branches Ard Biesheuvel
2020-07-09 13:14 ` Alexandru Elisei
2020-07-09 15:49 ` Will Deacon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.