On Thu, 9 Jul 2020 at 15:31, Ard Biesheuvel [off-list ref] wrote:

On Thu, 9 Jul 2020 at 14:11, Alexandru Elisei [off-list ref] wrote:

quoted

Hi,

Hi Alex,

Apologies for the breakage.

quoted

I should post the entire boot log:

...

quoted

[    0.002204] pc : work_pending+0x32c/0x348

This suggests that you ended executing directly from the alternatives
replacement section that gets appended to the end of work_pending (and
therefore gets mistaken for being part of it)

It appears that the following code in alternatives.c

static bool branch_insn_requires_update(struct alt_instr *alt, unsigned long pc)
{
    unsigned long replptr;

    if (kernel_text_address(pc))
       return true;

returns true inadvertently for the branch in this piece of code in entry.S

alternative_if ARM64_HAS_IRQ_PRIO_MASKING
    ldr x20, [sp, #S_PMR_SAVE]
    msr_s SYS_ICC_PMR_EL1, x20
    mrs_s x21, SYS_ICC_CTLR_EL1
    tbz x21, #6, .L__skip_pmr_sync\@ // Check for ICC_CTLR_EL1.PMHE
    dsb sy // Ensure priority change is seen by redistributor
.L__skip_pmr_sync\@:


due to the fact that kernel_text_address() has no way of
distinguishing branches inside the subsection from branches that
require updating. So the alternatives patching code dutifully updates
the tbz opcode and points it to its original target in the subsection.

This is going to be rather tricky to fix, unless we special case
tbz/cbz branches and other branches with limited range that would
never have worked before anyway.

For now, better to just revert it and revisit it later.

... unless we decide to fix up all branches pointing outside the
replacement sequence, which is not an entirely unreasonable thing to
do:

diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c
index d1757ef1b1e7..7c205f9202a3 100644
--- a/arch/arm64/kernel/alternative.c
+++ b/arch/arm64/kernel/alternative.c

@@ -45,18 +45,11 @@
 {
        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 true;
 }

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel