>From: Vineet Gupta <vgupta@xxxxxxxxxxxx>
>Sent: Wednesday, March 11, 2020 20:38
>To: Eugeniy Paltsev; linux-snps-arc@xxxxxxxxxxxxxxxxxxx
>Cc: linux-kernel@xxxxxxxxxxxxxxx; Alexey Brodkin
>Subject: Re: [PATCH 2/2] ARC: don't align ret_from_exception function
>
>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.
Do you know some exact numbers? I'm not an expert in ARC700 (fortunately =)
>
>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.
I really don't insist about applying this patch but I don't understand your
argumentation about ARC HS like at all. Could you please explain in more detail what
130 instructions you are talking about?
>What's weird is I never hit the original problem you ran into - are you using a
>toolchain with the branch relaxation stuff ?
Looks like we just were lucky enough and didn't change this code a lot.
I faced with this issue when I was developing DSP-related stuff. Initially I
triggered it when I replaced 'mov r10, 0' instruction with 'mov_s r10, 0' and got
weird kernel crush.
It can be reproduced with any toolchain (it's not related to branch relaxation stuff).
>I faked it using a nop_s and the SYM_FUNC_START_NOALIGN( ) annotation and can see
>all instructions getting unaligned.
What is the problem with it? It's totally valid and fine for ARC HS to have instructions
aligned by 2 byte. Or are you talking about ARC700 again?
>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
>> -
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
