Re: [PATCH] powerpc/64s: POWER10 CPU Kconfig build option

["Nicholas Piggin" <npiggin@gmail.com>]

On Fri Oct 7, 2022 at 4:07 AM AEST, Christophe Leroy wrote:
>
>
> Le 23/09/2022 à 08:23, Nicholas Piggin a écrit :
> > On Fri Sep 23, 2022 at 3:46 PM AEST, Christophe Leroy wrote:
> >>
> >>
> >> Le 23/09/2022 à 05:30, Nicholas Piggin a écrit :
> >>> This adds basic POWER10_CPU option, which builds with -mcpu=power10.
> >>>
> >>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >>> ---
> >>> There's quite a lot of asm and linker changes slated for the next merge
> >>> window already so I may leave the pcrel patch for next time. I think we
> >>> can add the basic POWER10 build option though.
> >>>
> >>> Thanks,
> >>> Nick
> >>>
> >>>    arch/powerpc/Makefile                  | 7 ++++++-
> >>>    arch/powerpc/platforms/Kconfig.cputype | 8 +++++++-
> >>>    2 files changed, 13 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> >>> index 8a3d69b02672..ea88af26f8c6 100644
> >>> --- a/arch/powerpc/Makefile
> >>> +++ b/arch/powerpc/Makefile
> >>> @@ -192,9 +192,14 @@ ifdef CONFIG_476FPE_ERR46
> >>>    		-T $(srctree)/arch/powerpc/platforms/44x/ppc476_modules.lds
> >>>    endif
> >>>    
> >>> -# No AltiVec or VSX instructions when building kernel
> >>> +# No prefix or pcrel
> >>> +KBUILD_CFLAGS += $(call cc-option,-mno-prefixed)
> >>
> >> We have lots of code to handle prefixed instructions in code_patching,
> >> and that code complexifies stuff and has a performance impact.
> >> And it is only partially taken into account, areas like ftrace don't
> >> properly take care of prefixed instructions.
> >>
> >> Should we get rid of prefixed instruction support completely in the
> >> kernel, and come back to more simple code ?
> > 
> > I would rather complete prefixed support in the kernel and use pcrel
> > addressing. Actually even if we don't compile with pcrel or prefixed,
> > there are some instructions and we will probably get more that require
> > prefixed, possible we might want to use them in kernel. Some of it is
> > required to handle user mode instructions too. So I think removing
> > it is premature, but I guess it's up for debate.
>
> Well ok, in fact I only had code_patching in mind.
>
> Code patching is only for kernel text. Today code patching is used for 
> things like kprobe, ftrace, etc .... which really do not seems to be 
> prepared for prefixed instructions.
>
> If you are adding -mno-prefixed, it is worth keeping that code which 
> sometimes gives us some headacke ?
>
> Of course if there are plans to get real prefixed instruction in kernel 
> code anytime soon, lets live with it, in that case the support should 
> get completed. But otherwise I think it would be better to get rid of it 
> for now, and implement it completely when we need it in years.

I have a series to enable it again, just not ready for upstream yet
but it's not all that far off.

https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-September/248521.html

> When I see the following, I'm having hard time believing it would work 
> with prefixed instructions in the kernel text:
>
> 	typedef u32 kprobe_opcode_t;
>
> 	struct kprobe {
> 	...
> 		/* Saved opcode (which has been replaced with breakpoint) */
> 		kprobe_opcode_t opcode;
>
>
> 	void arch_disarm_kprobe(struct kprobe *p)
> 	{
> 		WARN_ON_ONCE(patch_instruction(p->addr, ppc_inst(p->opcode)));
> 	}

Yeah that needs work for sure.

Thanks,
Nick