* Re: [PATCH] powerpc: Fix instruction encoding for lis in ppc_function_entry()
2021-03-04 2:04 [PATCH] powerpc: Fix instruction encoding for lis in ppc_function_entry() Naveen N. Rao
@ 2021-03-05 11:37 ` Christophe Leroy
2021-03-08 10:40 ` Naveen N. Rao
2021-03-05 13:15 ` Jiri Olsa
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2021-03-05 11:37 UTC (permalink / raw)
To: Naveen N. Rao, linuxppc-dev; +Cc: Jiri Olsa
Le 04/03/2021 à 03:04, Naveen N. Rao a écrit :
> 'lis r2,N' is 'addis r2,0,N' and the instruction encoding in the macro
> LIS_R2 is incorrect (it currently maps to 'addis 0,r2,N'). Fix the same.
>
> Fixes: c71b7eff426fa7 ("powerpc: Add ABIv2 support to ppc_function_entry")
> Reported-by: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/code-patching.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
> index eacc9102c2515c..d5b3c3bb95b400 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -73,7 +73,7 @@ void __patch_exception(int exc, unsigned long addr);
> #endif
>
> #define OP_RT_RA_MASK 0xffff0000UL
> -#define LIS_R2 0x3c020000UL
> +#define LIS_R2 0x3c400000UL
> #define ADDIS_R2_R12 0x3c4c0000UL
> #define ADDI_R2_R2 0x38420000UL
That probably goes beyond the scope of this patch, but it would be more readable and less error
prone to use macros defined in ppc-opcode.h just like kernel/module_64.c does for instance:
#define LIS_R2 (PPC_INST_ADDIS | __PPC_RT(R2))
#define ADDIS_R2_R12 (PPC_INST_ADDIS | __PPC_RT(R2) | __PPC_RA(R12))
#define ADDI_R2_R2 (PPC_INST_ADDI | __PPC_RT(R2) | __PPC_RA(R2))
>
>
> base-commit: 91966823812efbd175f904599e5cf2a854b39809
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] powerpc: Fix instruction encoding for lis in ppc_function_entry()
2021-03-05 11:37 ` Christophe Leroy
@ 2021-03-08 10:40 ` Naveen N. Rao
0 siblings, 0 replies; 6+ messages in thread
From: Naveen N. Rao @ 2021-03-08 10:40 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Jiri Olsa, linuxppc-dev
On 2021/03/05 12:37PM, Christophe Leroy wrote:
>
>
> Le 04/03/2021 à 03:04, Naveen N. Rao a écrit :
> > 'lis r2,N' is 'addis r2,0,N' and the instruction encoding in the macro
> > LIS_R2 is incorrect (it currently maps to 'addis 0,r2,N'). Fix the same.
> >
> > Fixes: c71b7eff426fa7 ("powerpc: Add ABIv2 support to ppc_function_entry")
> > Reported-by: Jiri Olsa <jolsa@redhat.com>
> > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/include/asm/code-patching.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
> > index eacc9102c2515c..d5b3c3bb95b400 100644
> > --- a/arch/powerpc/include/asm/code-patching.h
> > +++ b/arch/powerpc/include/asm/code-patching.h
> > @@ -73,7 +73,7 @@ void __patch_exception(int exc, unsigned long addr);
> > #endif
> > #define OP_RT_RA_MASK 0xffff0000UL
> > -#define LIS_R2 0x3c020000UL
> > +#define LIS_R2 0x3c400000UL
> > #define ADDIS_R2_R12 0x3c4c0000UL
> > #define ADDI_R2_R2 0x38420000UL
>
> That probably goes beyond the scope of this patch, but it would be more
> readable and less error prone to use macros defined in ppc-opcode.h just
> like kernel/module_64.c does for instance:
>
> #define LIS_R2 (PPC_INST_ADDIS | __PPC_RT(R2))
> #define ADDIS_R2_R12 (PPC_INST_ADDIS | __PPC_RT(R2) | __PPC_RA(R12))
> #define ADDI_R2_R2 (PPC_INST_ADDI | __PPC_RT(R2) | __PPC_RA(R2))
Good point. While that would have made it harder to spot the error, it
probably would have avoided the error in the first place.
Your change looks good to me.
Thanks,
Naveen
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: Fix instruction encoding for lis in ppc_function_entry()
2021-03-04 2:04 [PATCH] powerpc: Fix instruction encoding for lis in ppc_function_entry() Naveen N. Rao
2021-03-05 11:37 ` Christophe Leroy
@ 2021-03-05 13:15 ` Jiri Olsa
2021-03-05 18:41 ` Segher Boessenkool
2021-03-14 10:01 ` Michael Ellerman
3 siblings, 0 replies; 6+ messages in thread
From: Jiri Olsa @ 2021-03-05 13:15 UTC (permalink / raw)
To: Naveen N. Rao; +Cc: linuxppc-dev
On Thu, Mar 04, 2021 at 07:34:11AM +0530, Naveen N. Rao wrote:
> 'lis r2,N' is 'addis r2,0,N' and the instruction encoding in the macro
> LIS_R2 is incorrect (it currently maps to 'addis 0,r2,N'). Fix the same.
>
> Fixes: c71b7eff426fa7 ("powerpc: Add ABIv2 support to ppc_function_entry")
> Reported-by: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/code-patching.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
> index eacc9102c2515c..d5b3c3bb95b400 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -73,7 +73,7 @@ void __patch_exception(int exc, unsigned long addr);
> #endif
>
> #define OP_RT_RA_MASK 0xffff0000UL
> -#define LIS_R2 0x3c020000UL
> +#define LIS_R2 0x3c400000UL
> #define ADDIS_R2_R12 0x3c4c0000UL
> #define ADDI_R2_R2 0x38420000UL
>
>
> base-commit: 91966823812efbd175f904599e5cf2a854b39809
> --
> 2.25.1
>
I tested the new define value with uprobe issue we were dealing with
and now it matches the instruction, so it works for me
thanks a lot for the fix,
jirka
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] powerpc: Fix instruction encoding for lis in ppc_function_entry()
2021-03-04 2:04 [PATCH] powerpc: Fix instruction encoding for lis in ppc_function_entry() Naveen N. Rao
2021-03-05 11:37 ` Christophe Leroy
2021-03-05 13:15 ` Jiri Olsa
@ 2021-03-05 18:41 ` Segher Boessenkool
2021-03-14 10:01 ` Michael Ellerman
3 siblings, 0 replies; 6+ messages in thread
From: Segher Boessenkool @ 2021-03-05 18:41 UTC (permalink / raw)
To: Naveen N. Rao; +Cc: Jiri Olsa, linuxppc-dev
On Thu, Mar 04, 2021 at 07:34:11AM +0530, Naveen N. Rao wrote:
> 'lis r2,N' is 'addis r2,0,N' and the instruction encoding in the macro
> LIS_R2 is incorrect (it currently maps to 'addis 0,r2,N'). Fix the same.
That is written "addis r0,r2,N" even. Your patch looks fine otherwise,
so with that fix:
Acked-by: Segher Boessenkool <segher@kernel.crashing.org>
Segher
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: Fix instruction encoding for lis in ppc_function_entry()
2021-03-04 2:04 [PATCH] powerpc: Fix instruction encoding for lis in ppc_function_entry() Naveen N. Rao
` (2 preceding siblings ...)
2021-03-05 18:41 ` Segher Boessenkool
@ 2021-03-14 10:01 ` Michael Ellerman
3 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2021-03-14 10:01 UTC (permalink / raw)
To: linuxppc-dev, Naveen N. Rao; +Cc: Jiri Olsa
On Thu, 4 Mar 2021 07:34:11 +0530, Naveen N. Rao wrote:
> 'lis r2,N' is 'addis r2,0,N' and the instruction encoding in the macro
> LIS_R2 is incorrect (it currently maps to 'addis 0,r2,N'). Fix the same.
Applied to powerpc/fixes.
[1/1] powerpc/64s: Fix instruction encoding for lis in ppc_function_entry()
https://git.kernel.org/powerpc/c/cea15316ceee2d4a51dfdecd79e08a438135416c
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread