MIME-Version: 1.0
In-Reply-To: <20171212070758.GA26787@1a3c108b4593>
References: <20171207023110.2622-1-alankao@andestech.com> <mhng-2ff6fcb7-db72-482b-b70b-c925ae43c220@palmer-si-x1c4>
 <20171212070758.GA26787@1a3c108b4593>
From: Jim Wilson <jimw@sifive.com>
Date: Tue, 12 Dec 2017 09:47:03 -0800
Message-ID: <CAFyWVaYkca64qZzZ45PRmjHm46JzQOmVh+fxYKOMvkgwOi1aVQ@mail.gmail.com>
Subject: Re: [PATCH v2] riscv/ftrace: Add basic support
To: Alan Kao <nonerkao@gmail.com>
Cc: Palmer Dabbelt <palmer@sifive.com>, Albert Ou <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
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1853
Lines: 36

On Mon, Dec 11, 2017 at 11:08 PM, Alan Kao <nonerkao@gmail.com> wrote:
> On Mon, Dec 11, 2017 at 10:15:58AM -0800, Palmer Dabbelt wrote:
>> It's not a big deal, though -- we can fix these later.  The more interesting
>> thing here is that this code means our `-pg` stuff is now part of the GCC
>> ABI, which is something I'd never though of before.  I've added Jim, our GCC
>> guy.
>>
>> Jim: do you mind checking to make sure the GCC profiling support is sane?
>> Specifically, I'm thinking:
>>
>> * Are there any profiling features we don't support that would require an
>> ABI break?
>> * Is there a way to add future ISA extensions without breaking the ABI?
>> * Should we document this as part of the ELF psABI specification?
>>
>> Even though this isn't user-visible as far an Linux is concerned, it'd be a
>> bit of a pain to have to break this ABI because we did something brain-dead.
>> Since there's a bit of time before 7.3.0, I think it'd be OK to consider
>> breaking the profiling ABI if there's a good reason.

It looks sane to me.  I don't have a proper linux environment to test
in, but simple statically linked binaries are working on the spike
simulator, and doing what I expect.  The call is after the prologue,
so no need to worry about mcount overwriting registers that the
prologue needs.

As Alan mentioned, all gcc does is call mcount with two args, parent
pc and self pc, same as most other linux targets.  Most of the
interesting features of prof/gprof profiling happen inside glibc, with
the special start files provided by glibc, and some special functions
like profil(3).  I think this is more of a glibc API issue than a gcc
ABI issue.  If the glibc API changes, then the kernel support will
have to change too.  I checked half a dozen different processor ABIs,
and I didn't find that one documents how mcount works.

Jim