[PATCH v2 1/6] powerpc/code-patching: Implement generic text patching function

[Benjamin Gray]

On Tue, 2022-09-27 at 06:40 +0000, Christophe Leroy wrote:
> > +       /* Flush on the EA that may be executed in case of a non-
> > coherent icache */
> > +       icbi(prog_addr);
> 
> prog_addr is a misleading name ? Is that the address at which you 
> program it ? Is that the address the programs runs at ?
> 
> exec_addr was a lot more explicit as it clearly defines the address
> at 
> which the code is executed.

I'm not sure what it could be confused for other than "the address the
program uses" (be it uses for executing, or uses as data). I just
called it that because it's not necessarily executed, so 'exec_addr' is
misleading (to the extent it matters in the first place...).

> > +       if (IS_ENABLED(CONFIG_PPC64) && L1_CACHE_BYTES < 64)
> > +               icbi(prog_addr + size - 1);
> 
> This doesn't exist today.
> 
> I'd rather have:
> 
>         BUILD_BUG_ON(IS_ENABLED(CONFIG_PPC64) && L1_CACHE_BYTES <
> 64);

Sure, I can adjust the style.

> > +static int __always_inline __do_patch_memory(void *dest, unsigned
> > long src, size_t size)
> >   {
> >         int err;
> >         u32 *patch_addr;
> > -       unsigned long text_poke_addr;
> >         pte_t *pte;
> > -       unsigned long pfn = get_patch_pfn(addr);
> > -
> > -       text_poke_addr = (unsigned
> > long)__this_cpu_read(text_poke_area)->addr & PAGE_MASK;
> > -       patch_addr = (u32 *)(text_poke_addr +
> > offset_in_page(addr));
> > +       unsigned long text_poke_addr = (unsigned
> > long)__this_cpu_read(text_poke_area)->addr & PAGE_MASK;
> > +       unsigned long pfn = get_patch_pfn(dest);
> > 
> > +       patch_addr = (u32 *)(text_poke_addr +
> > offset_in_page(dest));
> 
> Can we avoid this churn ?
> Ok, you want to change 'addr' to 'dest', can we leave everything else
> as 
> is ?

'addr' was only renamed because the v1 used a pointer to the data, so
'addr' was ambiguous. I'll restore it to 'addr' for v3.

I'll also restore the formatting.