Date: Mon, 15 Jan 2018 15:38:23 +0800
From: Alan Kao <nonerkao@gmail.com>
To: patches@groups.riscv.org
Cc: Palmer Dabbelt <palmer@sifive.com>, Albert Ou <albert@sifive.com>,
        Christoph Hellwig <hch@lst.de>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ingo Molnar <mingo@redhat.com>,
        Masahiro Yamada <yamada.masahiro@socionext.com>,
        Kamil Rytarowski <n54@gmx.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org, Alan Kao <alankao@andestech.com>,
        Greentime Hu <greentime@andestech.com>
Subject: Re: [patches] [PATCH v2 2/6] riscv/ftrace: Add dynamic function
 tracer support
Message-ID: <20180115073821.GA9496@93d8b218059b>
References: <20180115064742.25324-1-alankao@andestech.com>
 <20180115064742.25324-3-alankao@andestech.com>
 <CADJ6UvOZz=WZ18wYVhZcH0u58u_snDxmgbwsfTOS2ski-4O7tg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CADJ6UvOZz=WZ18wYVhZcH0u58u_snDxmgbwsfTOS2ski-4O7tg@mail.gmail.com>
User-Agent: Mutt/1.9.2 (2017-12-15)
Sender: linux-kernel-owner@vger.kernel.org

On Sun, Jan 14, 2018 at 11:24:53PM -0800, Stefan O'Rear wrote:
> On Sun, Jan 14, 2018 at 10:47 PM, Alan Kao <nonerkao@gmail.com> wrote:
> > +       /*
> > +        * For the dynamic ftrace to work, here we should reserve at least
> > +        * 8 bytes for a functional auipc-jalr pair. Pseudo inst nop may be
> > +        * interpreted as different length in different models, so we manually
> > +        * *expand* two 4-byte nops here.
> > +        *
> > +        * Calling ftrace_update_ftrace_func would overwrite the nops below.
> > +        * Check ftrace_modify_all_code for details.
> > +        */
> > +       addi    x0, x0, 0
> > +       addi    x0, x0, 0
> 
> This relies on behavior of the assembler which is undocumented and, if
> my reading of the specification is correct, a bug.
> 
> The documented way to assemble an sequence of 2 4-byte NOPs regardless
> of subtarget is as follows:
> 
> .option push
> .option norvc
>     nop
>     nop
> .option pop
> 
> I have filed https://github.com/riscv/riscv-binutils-gdb/issues/135 to
> get clarity on the assembler behavior; the explicit approach may be
> preferable even if the assembler behavior turns out to be correct.
> 
> -s
> 

Thanks for pointing this out.

After checking the way other architectures have done this, I think we can 
just put a 

	call ftrace_stub

here.  Currently we don't support linker relaxing with ftrace, so this
macro will be expand to 8-byte inst-pair.

Alan

> -- 
> You received this message because you are subscribed to the Google Groups "RISC-V Patches" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to patches+unsubscribe@groups.riscv.org.
> To post to this group, send email to patches@groups.riscv.org.
> Visit this group at https://groups.google.com/a/groups.riscv.org/group/patches/.
> To view this discussion on the web visit https://groups.google.com/a/groups.riscv.org/d/msgid/patches/CADJ6UvOZz%3DWZ18wYVhZcH0u58u_snDxmgbwsfTOS2ski-4O7tg%40mail.gmail.com.
> For more options, visit https://groups.google.com/a/groups.riscv.org/d/optout.