| Message ID | 20220912082020.226755-3-sv@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | objtool: Enable and implement --mcount option on powerpc | expand |
On Mon, Sep 12, 2022 at 01:50:06PM +0530, Sathvika Vasireddy wrote: > In a subsequent patch, we would want to annotate powerpc assembly functions > with SYM_FUNC_START_LOCAL macro. This macro depends on __ALIGN macro. > > The default expansion of __ALIGN macro is: > #define __ALIGN .align 4,0x90 > > So, override __ALIGN and __ALIGN_STR macros to use the same alignment as > that of the existing _GLOBAL macro. Also, do not pad with 0x90, because > repeated 0x90s are not a nop or trap on powerpc. > > Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com> > --- > arch/powerpc/include/asm/linkage.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/powerpc/include/asm/linkage.h b/arch/powerpc/include/asm/linkage.h > index b71b9582e754..b88d1d2cf304 100644 > --- a/arch/powerpc/include/asm/linkage.h > +++ b/arch/powerpc/include/asm/linkage.h > @@ -4,6 +4,9 @@ > > #include <asm/types.h> > > +#define __ALIGN .align 2 > +#define __ALIGN_STR ".align 2" Like mentioned last time; I'm fixing this (but you're right to not wait on that), that said, would it make sense to write it like: #define __ALIGN .balign 4 #define __ALIGN_STR __stringify(__ALIGN) That said; with power instructions being 4 bytes, the above alignment is basically no-alignment, right?
Le 13/09/2022 à 14:03, Peter Zijlstra a écrit : > On Mon, Sep 12, 2022 at 01:50:06PM +0530, Sathvika Vasireddy wrote: >> In a subsequent patch, we would want to annotate powerpc assembly functions >> with SYM_FUNC_START_LOCAL macro. This macro depends on __ALIGN macro. >> >> The default expansion of __ALIGN macro is: >> #define __ALIGN .align 4,0x90 >> >> So, override __ALIGN and __ALIGN_STR macros to use the same alignment as >> that of the existing _GLOBAL macro. Also, do not pad with 0x90, because >> repeated 0x90s are not a nop or trap on powerpc. >> >> Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com> >> --- >> arch/powerpc/include/asm/linkage.h | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/arch/powerpc/include/asm/linkage.h b/arch/powerpc/include/asm/linkage.h >> index b71b9582e754..b88d1d2cf304 100644 >> --- a/arch/powerpc/include/asm/linkage.h >> +++ b/arch/powerpc/include/asm/linkage.h >> @@ -4,6 +4,9 @@ >> >> #include <asm/types.h> >> >> +#define __ALIGN .align 2 >> +#define __ALIGN_STR ".align 2" > > Like mentioned last time; I'm fixing this (but you're right to not wait > on that), that said, would it make sense to write it like: > > #define __ALIGN .balign 4 > #define __ALIGN_STR __stringify(__ALIGN) By the way, I commented to Sathvika to not use __stringify() in order to avoid having to include stringify.h as we are trying to minimise dependencies between headers. Several other architectures also do it that way. That being said, it could then be #define __ALIGN .balign 4 #define __ALIGN_STR ".balign 4" > > That said; with power instructions being 4 bytes, the above alignment is > basically no-alignment, right? > Yes indeed.
On Tue, Sep 13, 2022 at 12:21:51PM +0000, Christophe Leroy wrote: > > Like mentioned last time; I'm fixing this (but you're right to not wait > > on that), that said, would it make sense to write it like: > > > > #define __ALIGN .balign 4 > > #define __ALIGN_STR __stringify(__ALIGN) > > By the way, I commented to Sathvika to not use __stringify() in order to > avoid having to include stringify.h as we are trying to minimise > dependencies between headers. > > Several other architectures also do it that way. stringify.h is a trivial header and included by linux/linkage.h before it includes asm/linkage. Anyway, I was thinking of having: #ifndef __ALIGN_STR #define __ALIGN_STR __stringify(__ALIGN) #endif in linux/linkage.h, that avoids having to duplicate this all over the place.
diff --git a/arch/powerpc/include/asm/linkage.h b/arch/powerpc/include/asm/linkage.h index b71b9582e754..b88d1d2cf304 100644 --- a/arch/powerpc/include/asm/linkage.h +++ b/arch/powerpc/include/asm/linkage.h @@ -4,6 +4,9 @@ #include <asm/types.h> +#define __ALIGN .align 2 +#define __ALIGN_STR ".align 2" + #ifdef CONFIG_PPC64_ELF_ABI_V1 #define cond_syscall(x) \ asm ("\t.weak " #x "\n\t.set " #x ", sys_ni_syscall\n" \
In a subsequent patch, we would want to annotate powerpc assembly functions with SYM_FUNC_START_LOCAL macro. This macro depends on __ALIGN macro. The default expansion of __ALIGN macro is: #define __ALIGN .align 4,0x90 So, override __ALIGN and __ALIGN_STR macros to use the same alignment as that of the existing _GLOBAL macro. Also, do not pad with 0x90, because repeated 0x90s are not a nop or trap on powerpc. Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com> --- arch/powerpc/include/asm/linkage.h | 3 +++ 1 file changed, 3 insertions(+)