This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [PATCH 2/5] x86: Add -mindirect-branch-loop=


Hi all,

> -----Original Message-----
> From: H.J. Lu [mailto:hjl.tools@gmail.com]
> Sent: Friday, January 12, 2018 7:36 PM
> To: Martin Jambor <mjambor@suse.cz>
> Cc: Nagarajan, Muthu kumar raj <Muthukumarraj.Nagarajan@amd.com>;
> Kumar, Venkataramanan <Venkataramanan.Kumar@amd.com>; GCC
> Patches <gcc-patches@gcc.gnu.org>; Jeff Law <law@redhat.com>
> Subject: Re: [PATCH 2/5] x86: Add -mindirect-branch-loop=
> 
> On Fri, Jan 12, 2018 at 4:38 AM, Martin Jambor <mjambor@suse.cz> wrote:
> > Hi,
> >
> > On Thu, Jan 11 2018, Jeff Law wrote:
> >> On 01/07/2018 03:59 PM, H.J. Lu wrote:
> >>> Add -mindirect-branch-loop= option to control loop filler in call
> >>> and return thunks generated by -mindirect-branch=.  'lfence' uses
> "lfence"
> >>> as loop filler.  'pause' uses "pause" as loop filler.  'nop' uses "nop"
> >>> as loop filler.  The default is 'lfence'.
> >>>
> >>> gcc/
> >>>
> >>>      * config/i386/i386-opts.h (indirect_branch_loop): New.
> >>>      * config/i386/i386.c (output_indirect_thunk): Support
> >>>      -mindirect-branch-loop=.
> >>>      * config/i386/i386.opt (mindirect-branch-loop=): New option.
> >>>      (indirect_branch_loop): New.
> >>>      (lfence): Likewise.
> >>>      (pause): Likewise.
> >>>      (nop): Likewise.
> >>>      * doc/invoke.texi: Document -mindirect-branch-loop= option.
> >>>
> >>> gcc/testsuite/
> >>>
> >>>      * gcc.target/i386/indirect-thunk-loop-1.c: New test.
> >>>      * gcc.target/i386/indirect-thunk-loop-2.c: Likewise.
> >>>      * gcc.target/i386/indirect-thunk-loop-3.c: Likewise.
> >>>      * gcc.target/i386/indirect-thunk-loop-4.c: Likewise.
> >>>      * gcc.target/i386/indirect-thunk-loop-5.c: Likewise.
> >> I think we should drop the ability to change the filler until such
> >> time as we really need it.  Just pick one and go with it.  I think
> >> David suggested that they wanted "pause".  I'm obviously fine with that.
> >>
> >
> > unless I am mistaken (which is frankly quite possible, I am still not
> > quite up to speed about the nuances), AMD strongly prefers the lfence
> > variant.  OTOH, IIUC, in kernel this will be run-time patched but so
> > it does not matter in the most pressing case and we might want to have
> > a mechanism doing something similar for protecting userspace later on.
> > But perhaps it is enough to keep the option?
> >
> > Muthu and/or Venkat, can you please comment?
> 
> If we do want it, I will submit a separate patch AFTER the current patch set
> has been approved and checked into GCC 8.
> 

As per AMD architects, using “lfence” in “retpoline” is better than “pause” for our targets.
So please allow filler to use "lfence". 

Regards.
Venkat.
> --
> H.J.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]