Date: Wed, 13 Dec 2017 08:57:39 +0800
From: Alan Kao <nonerkao@gmail.com>
To: Palmer Dabbelt <palmer@sifive.com>
Cc: Jim Wilson <jimw@sifive.com>, albert@sifive.com, rostedt@goodmis.org,
        mingo@redhat.com, patches@groups.riscv.org,
        linux-kernel@vger.kernel.org, greentime@andestech.com,
        alankao@andestech.com, pombredanne@nexb.com, kito@andestech.com
Subject: Re: [patches] [PATCH v2] riscv/ftrace: Add basic support
Message-ID: <20171213005735.GB26787@1a3c108b4593>
References: <20171207023110.2622-1-alankao@andestech.com>
 <mhng-2ff6fcb7-db72-482b-b70b-c925ae43c220@palmer-si-x1c4>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <mhng-2ff6fcb7-db72-482b-b70b-c925ae43c220@palmer-si-x1c4>
User-Agent: Mutt/1.9.1 (2017-09-22)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1268
Lines: 39

On Mon, Dec 11, 2017 at 10:15:58AM -0800, Palmer Dabbelt wrote:
> On Wed, 06 Dec 2017 18:31:10 PST (-0800), nonerkao@gmail.com wrote:

Hi Palmer, 

I forgot to explain this section in the previous reply:

> > +ENTRY(_mcount)
> > +	la	t4, ftrace_stub
> > +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> > +	la	t0, ftrace_graph_return
> > +	ld	t1, 0(t0)
> > +	bne	t1, t4, do_ftrace_graph_caller
> > +
> > +	la	t3, ftrace_graph_entry
> > +	ld	t2, 0(t3)
> > +	la	t6, ftrace_graph_entry_stub
> > +	bne	t2, t6, do_ftrace_graph_caller
> > +#endif
> > +	la	t3, ftrace_trace_function
> > +	ld	t5, 0(t3)
> > +	bne	t5, t4, do_trace
> > +	ret
> 
> * You can save an instruction when addressing by using somethingl like "ld
> t1,  ftrace_graph_return" instead of "la t0, ftrace_graph_return; ld t1
> 0(t0)".
> 

There are three "la-ld" instruction pairs for loading ftrace_graph_return,
ftrace_graph_entry, and ftrace_trace_function.  All of them are function
pointers in C.  The problem here is that, if we applied an "ld" inst. to a
function pointer, we would have loaded the content, which would be the 
first 8 bytes in the function, rather than the address of the target function
that the function pointer stored before.

In brief, the logic of the "la-ld" pairs should be fine.

Alan