Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Vineet Gupta <Vineet.Gupta1@synopsys.com>
To:     Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>,
        Cupertino Miranda <Cupertino.Miranda@synopsys.com>
CC:     Alexey Brodkin <Alexey.Brodkin@synopsys.com>,
        Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com>,
        arcml <linux-snps-arc@lists.infradead.org>,
        lkml <linux-kernel@vger.kernel.org>
Subject: Re: ARC Assembler: bundle_align_mode directive support
Thread-Topic: ARC Assembler: bundle_align_mode directive support
Thread-Index: AQHVHicA2N+x/AVgjkyWYOIr/ofXig==
Date:   Mon, 10 Jun 2019 15:55:41 +0000
Message-ID: <C2D7FE5348E1B147BCA15975FBA2307501A2525686@us01wembx1.internal.synopsys.com>
References: <3962a9ad199cea45b1cfadb80be551aab83b7028.camel@synopsys.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 6/8/19 11:21 AM, Eugeniy Paltsev wrote:=0A=
> Hi Cupertino,=0A=
>=0A=
> I tried to use ".bundle_align_mode" directive in ARC assembly, but I got =
following error:=0A=
> ----------------->8--------------=0A=
> Assembler messages:=0A=
> Error: unknown pseudo-op: `.bundle_align_mode'=0A=
> ----------------->8--------------=0A=
>=0A=
> Is it possible to implement it in ARC assembler?=0A=
> There is some context about the reason we want to have it:=0A=
>=0A=
> I'm trying to add support of jump labels for ARC in linux kernel. Jump la=
bels=0A=
> provide an interface to generate static branches using self-modifying cod=
e.=0A=
> This allows us to implement conditional branches where changing branch=0A=
> direction is expensive but branch selection is basically 'free'.=0A=
>=0A=
> There is nuance in current implementation:=0A=
> We need to patch code by rewriting 32-bit NOP by 32-bit BRANCH instructio=
n (or vice versa).=0A=
> It can be easily done with following code:=0A=
> ----------------->8--------------=0A=
> write_32_bit(new_instruction)=0A=
> flush_l1_dcache_range_this_cpu=0A=
> invalidate_l1_icache_range_all_cpu=0A=
> ----------------->8--------------=0A=
>=0A=
> I$ update will be atomic in most of cases except the patched instruction =
share=0A=
> two L1 cache lines (so first 16 bits of instruction are in the one cache =
line and=0A=
> last 16 bit are in another cache line).=0A=
> In such case we can execute half-updated instruction if we are patching l=
ive code (and we are unlucky enough :)=0A=
=0A=
While I understand your need for alignment, I don't see how you can possibl=
y=0A=
execute stray lines.=0A=
dcache flush will be propagated by hardware (SCU) to all cores (as applicab=
le) and=0A=
the icache cache flush xcall is synchronous and will have to finish on all =
cores=0A=
before we proceed to execute the cod eitself.=0A=
=0A=
>=0A=
> As of today I simply align by 4 byte instruction which can be patched wit=
h ".balign 4" directive:=0A=
> ----------------->8--------------=0A=
> static __always_inline bool arch_static_branch_jump(struct static_key *ke=
y,=0A=
>     bool branch)=0A=
> {=0A=
> asm_volatile_goto(".balign 4\n"=0A=
>  "1:\n"=0A=
>  "b %l[l_yes]\n" // <- instruction which can be patched=0A=
>  ".pushsection __jump_table, \"aw\"\n"=0A=
>  ".word 1b, %l[l_yes], %c0\n"=0A=
>  ".popsection\n"=0A=
>  : : "i" (&((char *)key)[branch]) : : l_yes);=0A=
>=0A=
> return false;=0A=
> l_yes:=0A=
> return true;=0A=
> }=0A=
> ----------------->8--------------=0A=
>=0A=
> In that case patched instruction is aligned with one 16-bit NOP if this i=
s required.=0A=
> However 'align by 4' directive is much stricter than it actually required=
. =0A=
=0A=
I don't quite understand. Can u write a couple of lines of pseudo assembly =
to show=0A=
what the issue is.=0A=
=0A=
> It's enough=0A=
> that our 32-bit instruction don't cross l1 cache line boundary.=0A=
> That will save us from adding useless NOP padding in most of the cases.=
=0A=
> It can be implemented with ".bundle_align_mode" directive which isn't sup=
ported by ARC AS unfortunately.=0A=
=0A=
This seems like a reasonable request (contingent to the difficulty of=0A=
implementation in binutils). but I can't comprehend why you would need it.=
=0A=