On Wed, Jan 25, 2023 at 8:46 AM Josh Poimboeuf [off-list ref] wrote:

On Tue, Jan 24, 2023 at 10:09:56PM -0800, Song Liu wrote:

quoted

quoted

@@ -514,9 +515,18 @@ static int restore_r2(const char *name, u32 *instruction, struct module *me)
        if (!instr_is_relative_link_branch(ppc_inst(*prev_insn)))
                return 0;

-       if (*instruction != PPC_RAW_NOP()) {
+       /*
+        * For livepatch, the restore r2 instruction might have already been
+        * written previously, if the referenced symbol is in a previously
+        * unloaded module which is now being loaded again.  In that case, skip
+        * the warning and the instruction write.
+        */
+       if (insn_val == PPC_INST_LD_TOC)
+               return 0;

Do we need "sym->st_shndx == SHN_LIVEPATCH" here?

My original patch had that check, but I dropped it for simplicity.

In the non-livepatch case, the condition should never be true, but it
doesn't hurt to check it anyway.

While this is the only place we use PPC_INST_LD_TOC, there is another
place we use "PPC_RAW_STD(_R2, _R1, R2_STACK_OFFSET)", which
is identical to PPC_INST_LD_TOC. So I am not quite sure whether this
happens for non-livepatch.

Thanks,
Song