Le 25/10/2022 à 06:44, Benjamin Gray a écrit : > Verifies that if the instruction patching did not return an error then > the value stored at the given address to patch is now equal to the > instruction we patched it to.
Why do we need that verification ? Until now it wasn't necessary, can you describe a failure that occurs because we don't have this verification ? Or is that until now it was reliable but the new method you are adding will not be as reliable as before ? What worries me with that new verification is that you are reading a memory address with is mostly only used for code execution. That means: - You will almost always take a DATA TLB Miss, hence performance impact. - If one day we want Exec-only text mappings, it will become problematic. So really the question is, is that patch really required ? > > Signed-off-by: Benjamin Gray <bg...@linux.ibm.com> > --- > arch/powerpc/lib/code-patching.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/powerpc/lib/code-patching.c > b/arch/powerpc/lib/code-patching.c > index 3b3b09d5d2e1..b0a12b2d5a9b 100644 > --- a/arch/powerpc/lib/code-patching.c > +++ b/arch/powerpc/lib/code-patching.c > @@ -192,6 +192,8 @@ static int do_patch_instruction(u32 *addr, ppc_inst_t > instr) > err = __do_patch_instruction(addr, instr); > local_irq_restore(flags); > > + WARN_ON(!err && !ppc_inst_equal(instr, ppc_inst_read(addr))); > + > return err; > } > #else /* !CONFIG_STRICT_KERNEL_RWX */