Message-ID: <09d44d74-f3c9-cf99-8d14-74499485e956@gmail.com>
Date:   Sun, 12 Mar 2023 21:53:22 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH] perf tools riscv: Add support for riscv
 lookup_binutils_path
Content-Language: en-US
To:     Ian Rogers <irogers@google.com>, Namhyung Kim <namhyung@kernel.org>
References: <20230311112122.28894-1-p4ranlee@gmail.com>
 <CAP-5=fWkAqX+Uy_XhXHjEx6sj-wpOJ+WQf=vMtRzRBS-0Kj32Q@mail.gmail.com>
From:   Paran Lee <p4ranlee@gmail.com>
Cc:     linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-riscv@lists.infradead.org
In-Reply-To: <CAP-5=fWkAqX+Uy_XhXHjEx6sj-wpOJ+WQf=vMtRzRBS-0Kj32Q@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk


23. 3. 12. 15:27에 Ian Rogers 이(가) 쓴 글:
> On Sat, Mar 11, 2023 at 3:22 AM paranlee <p4ranlee@gmail.com> wrote:
>>
>> Add to know RISC-V binutils path.
>> Secondarily, edit the code block with alphabetical order.
>>
>> Signed-off-by: Paran Lee <p4ranlee@gmail.com>
>> ---
>>  tools/perf/arch/common.c | 51 +++++++++++++++++++++++++++-------------
>>  1 file changed, 35 insertions(+), 16 deletions(-)
>>
>> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
>> index 59dd875fd5e4..058527ededdd 100644
>> --- a/tools/perf/arch/common.c
>> +++ b/tools/perf/arch/common.c
>> @@ -29,11 +29,23 @@ const char *const arm_triplets[] = {
>>  };
>>
>>  const char *const arm64_triplets[] = {
>> +       "aarch64-unknown-linux-",
> 
> Modifying ARM64 behavior should be a separate change.
> 
>>         "aarch64-linux-android-",
>>         "aarch64-linux-gnu-",
>>         NULL
>>  };
>>
>> +const char *const mips_triplets[] = {
>> +       "mips-unknown-linux-gnu-",
>> +       "mipsel-linux-android-",
>> +       "mips-linux-gnu-",
>> +       "mips64-linux-gnu-",
>> +       "mips64el-linux-gnuabi64-",
>> +       "mips64-linux-gnuabi64-",
>> +       "mipsel-linux-gnu-",
>> +       NULL
>> +};
>> +
> 
> This will affect the blame history. It should probably be its own change too.

Thank you for review! I agree. So I would split the patch.

>> -
>>  static bool lookup_path(char *name)
>>  {
>>         bool found = false;
>> @@ -164,18 +179,22 @@ static int perf_env__lookup_binutils_path(struct perf_env *env,
>>                 path_list = arm_triplets;
>>         else if (!strcmp(arch, "arm64"))
>>                 path_list = arm64_triplets;
>> +       else if (!strcmp(arch, "mips"))
>> +               path_list = mips_triplets;
>>         else if (!strcmp(arch, "powerpc"))
>>                 path_list = powerpc_triplets;
>> -       else if (!strcmp(arch, "sh"))
>> -               path_list = sh_triplets;
>> +       else if (!strcmp(arch, "riscv32"))
>> +               path_list = riscv32_triplets;
>> +       else if (!strcmp(arch, "riscv64"))
>> +               path_list = riscv64_triplets;
>>         else if (!strcmp(arch, "s390"))
>> -               path_list = s390_triplets;
>> +               path_list = s390_triplets;
> 
> whitespace issue?

I tried to correct the alphabetical order because it was vaguely sorted.
And I'll try to work on blame history on each arch code block as well.

>> +       else if (!strcmp(arch, "sh"))
>> +               path_list = sh_triplets;
>>         else if (!strcmp(arch, "sparc"))
>>                 path_list = sparc_triplets;
>>         else if (!strcmp(arch, "x86"))
>>                 path_list = x86_triplets;
>> -       else if (!strcmp(arch, "mips"))
>> -               path_list = mips_triplets;
>>         else {
>>                 ui__error("binutils for %s not supported.\n", arch);
>>                 goto out_error;
> 
> I think in general we need to revamp this code. Two things that I see
> that are missing are (1) support for perf config and (2) addr2line
> should be configurable, you may want llvm-addr2line. Adding RISC-V is
> of course important too :-)
> 
> Thanks,
> Ian

May I ask documentation or hint that I can help work with?

P.S.

I'm interested in the Google Summer Of code perf category this year,
especially the part about risc-v architecture, I recently purchased a
development board and would like to be able to test perf on a Sifive U74
CPU based environment.
But I've only used perf with command tool and don't know much about the
internals, so if there is a roadmap for perf development or
contribution, I have interest in perf internals both kernel and user side.
May I ask information to apply?
I am developing Linux Security Driver drivers for a security company.

BR
Paran Lee