linux-kernel - Re: [PATCH 1/3] riscv: Add perf callchain support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190429083940.GA22718@vmh-VirtualBox>
Date:   Mon, 29 Apr 2019 16:39:40 +0800
From:   Mao Han <han_mao@...ky.com>
To:     Palmer Dabbelt <palmer@...ive.com>
Cc:     linux-riscv@...ts.infradead.org, guoren@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] riscv: Add perf callchain support

On Thu, Apr 25, 2019 at 02:11:00PM -0700, Palmer Dabbelt wrote:
> On Thu, 11 Apr 2019 00:53:48 PDT (-0700), han_mao@...ky.com wrote:
> >+	if (!kstack_end((void *)frame->fp) &&
> >+	    !((unsigned long)frame->fp & 0x3) &&
> >+	    ((unsigned long)frame->fp >= TASK_SIZE)) {
> >+		frame->ra = ((struct stackframe *)frame->fp - 1)->ra;
> >+		frame->fp = ((struct stackframe *)frame->fp - 1)->fp;
> 
> It looks like this depends on having a frame pointer?  In that case, shouldn't
> we add some Kconfig logic to make CONFIG_PERF_EVENTS select
> -fno-omit-frame-pointer?  Frame pointers aren't enabled by default on RISC-V
> and therefor are unlikely to exist at all.
>

Yes, frame pointer is required for kernel backtrace, -fno-omit-frame-pointer
should be added if frame pointer is not enabled by default on RISC-V.
When I testing the callchain support with buildroot chunk
git://git.busybox.net/buildroot
cbf1d861fadb491eee6e3686019d8f67d7f4ad85
both kernel and user program have fp without adding any extra option,
so I thought frame pointer was enabled by default.

ffffffe0000009ee <kernel_init_freeable>:
ffffffe0000009ee:       711d                    addi    sp,sp,-96
ffffffe0000009f0:       ec86                    sd      ra,88(sp)
ffffffe0000009f2:       e8a2                    sd      s0,80(sp)
ffffffe0000009f4:       e4a6                    sd      s1,72(sp)

void test_3(void)
{
   10498:       fe010113                addi    sp,sp,-32
   1049c:       00113c23                sd      ra,24(sp)
   104a0:       00813823                sd      s0,16(sp)
   104a4:       02010413                addi    s0,sp,32

> >+		/* make sure CONFIG_FUNCTION_GRAPH_TRACER is turned on */
> 
> Should that also be mandated by a Kconfig?
>

Yes, it is required for ftrace_graph_ret_addr support.
It's already default for ARCH_RV64I in Kconfig.
 
> >+/*
> >+ * This will be called when the target is in user mode
> >+ * This function will only be called when we use
> >+ * "PERF_SAMPLE_CALLCHAIN" in
> >+ * kernel/events/core.c:perf_prepare_sample()
> >+ *
> >+ * How to trigger perf_callchain_[user/kernel] :
> >+ * $ perf record -e cpu-clock --call-graph fp ./program
> >+ * $ perf report --call-graph
> >+ *
> >+ * On RISC-V platform, the program being sampled and the C library
> >+ * need to be compiled with * -mbacktrace, otherwise the user
> 
> What is "-mbacktrace"?  I don't remember that ever being a RISC-V GCC option,
> and my compiler doesn't undersand it.  It understands "-fbacktrace" but that
> doesn't produce a frame pointer.
>

It's a csky specific option, I forget to modify the comment.
So it should be -fno-omit-frame-pointer here.

Thanks,
Mao Han