[PATCH v9 4/7] powerpc/code-patching: Verify instruction patch succeeded

[Christophe Leroy]

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 <bgray at 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 */