Re: [PATCH 2/2] ARC: don't align ret_from_exception function

[Vineet Gupta]

On 3/11/20 9:26 AM, Eugeniy Paltsev wrote:
> ARC have a tricky implemented ret_from_exception function.
> It is written on ASM and can be called like regular function.
> However it has another 'entry point' as it can be called as a
> continuation of EV_Trap function.

It is not really intended / needed to be called form outside ASM - but has to be
spread across 2 asm files as it is mostly target isa independent, while the
callers are in separate target isa specific files.
The ENTRY/END annotations are simply for the dwarf unwinder to stop gracefully.

> As we declare "ret_from_exception" using ENTRY macro it may align
> "ret_from_exception" by 4 bytes by adding padding before it.
> "ret_from_exception" doesn't require alignment by 4 bytes
> (as it doesn't go to vector table, etc...) so let's avoid aligning
> it by switch from ENTRY (which is alias for SYM_FUNC_START) to
> SYM_FUNC_START_NOALIGN macro.

I would like to keep it aligned.

ARC700 definitely has penalty for unaligned branch targets, so it will definitely
suffer there.

For HS, unaligned branch targets have no penalty (for the general case atleast),
but if it does get unaligned, the entire entry prologue code will be - i.e. each
one of the subsequent 130 or so instructions. That doesn't seem like a good idea
in general to me.

What's weird is I never hit the original problem you ran into - are you using a
toolchain with the branch relaxation stuff ?
I faked it using a nop_s and the SYM_FUNC_START_NOALIGN( ) annotation and can see
all instructions getting unaligned.

Before;

921238e4 <ret_from_exception>:
921238e4:ÂÂÂ ldÂÂÂ r8,[sp,124]
921238e8:ÂÂÂ bbit0.ntÂÂÂ r8,0x7,212ÂÂÂ
...
92123ac8:ÂÂÂ rtie
92123acc <debug_marker_ds>:
92123acc:ÂÂÂ ldÂÂÂ r2,[0x927d81d8]
92123ad4:ÂÂÂ addÂÂÂ r2,r2,0x1
92123ad8:ÂÂÂ stÂÂÂ r2,[0x927d81d8]
92123ae0:ÂÂÂ bmsknÂÂÂ r11,r10,0xf
92123ae4:ÂÂÂ srÂÂÂ r11,[aux_irq_act]
92123ae8:ÂÂÂ bÂÂÂ -140ÂÂÂ ;92123a5c

After:

9212393c:ÂÂÂ nop_s
9212393e <ret_from_exception>:
9212393e:ÂÂÂ ldÂÂÂ r8,[sp,124]
92123942:ÂÂÂ bbit0.ntÂÂÂ r8,0x7,214
...
92123b22:ÂÂÂ rtie
92123b26 <debug_marker_ds>:
92123b26:ÂÂÂ ldÂÂÂ r2,[0x927d81d8]
92123b2e:ÂÂÂ addÂÂÂ r2,r2,0x1
92123b32:ÂÂÂ stÂÂÂ r2,[0x927d81d8]
92123b3a:ÂÂÂ bmsknÂÂÂ r11,r10,0xf
92123b3e:ÂÂÂ srÂÂÂ r11,[aux_irq_act]
92123b42:ÂÂÂ bÂÂÂ -138ÂÂÂ ;92123ab6 <debug_marker_syscall>
92123b46:ÂÂÂ nop_s

> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@xxxxxxxxxxxx>
> ---
>  arch/arc/kernel/entry-arcv2.S | 2 +-
>  arch/arc/kernel/entry.S       | 3 +--
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arc/kernel/entry-arcv2.S b/arch/arc/kernel/entry-arcv2.S
> index 12d5f12d10d2..d482e1507f56 100644
> --- a/arch/arc/kernel/entry-arcv2.S
> +++ b/arch/arc/kernel/entry-arcv2.S
> @@ -260,4 +260,4 @@ debug_marker_ds:
>  	sr	r11, [AUX_IRQ_ACT]
>  	b	.Lexcept_ret
>  
> -END(ret_from_exception)
> +SYM_FUNC_END(ret_from_exception)
> diff --git a/arch/arc/kernel/entry.S b/arch/arc/kernel/entry.S
> index 60406ec62eb8..79409b518a09 100644
> --- a/arch/arc/kernel/entry.S
> +++ b/arch/arc/kernel/entry.S
> @@ -280,7 +280,7 @@ END(EV_Trap)
>  ;
>  ; If ret to user mode do we need to handle signals, schedule() et al.
>  
> -ENTRY(ret_from_exception)
> +SYM_FUNC_START_NOALIGN(ret_from_exception)
>  
>  	; Pre-{IRQ,Trap,Exception} K/U mode from pt_regs->status32
>  	ld  r8, [sp, PT_status32]   ; returning to User/Kernel Mode
> @@ -373,4 +373,3 @@ resume_kernel_mode:
>  	b	.Lrestore_regs
>  
>  ##### DONT ADD CODE HERE - .Lrestore_regs actually follows in entry-<isa>.S
> -