linux-kernel - Re: [PATCH v1 0/4] perf parse-regs: Cleanup config and building

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: <CAP-5=fXqdFrTDY2a2dXK1ehP2AbwPfPF8vYWXgeSMcKh4mS+ow@mail.gmail.com>
Date: Wed, 14 Feb 2024 14:42:55 -0800
From: Ian Rogers <irogers@...gle.com>
To: Leo Yan <leo.yan@...ux.dev>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>, Jiri Olsa <jolsa@...nel.org>, 
	Adrian Hunter <adrian.hunter@...el.com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, John Garry <john.g.garry@...cle.com>, 
	Will Deacon <will@...nel.org>, James Clark <james.clark@....com>, 
	Mike Leach <mike.leach@...aro.org>, Guo Ren <guoren@...nel.org>, 
	Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, 
	Albert Ou <aou@...s.berkeley.edu>, Huacai Chen <chenhuacai@...nel.org>, 
	Ming Wang <wangming01@...ngson.cn>, Kan Liang <kan.liang@...ux.intel.com>, 
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-csky@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v1 0/4] perf parse-regs: Cleanup config and building

On Wed, Feb 14, 2024 at 3:40 AM Leo Yan <leo.yan@...ux.dev> wrote:
>
> Currently, the perf building enables register parsing based on the
> target architecture has supported register feature.
>
> Furthermore, the perf building system needs to maintain a variable
> 'NO_PERF_REGS' and defines macro 'HAVE_PERF_REGS_SUPPORT' for statically
> compiling the tool.
>
> As a result, the perf has no flexibilty for parsing register if an
> architecture doesn't support it. And the source files use the macro
> 'HAVE_PERF_REGS_SUPPORT' to switch on and off the register parsing
> related code, which is not a good practice.
>
> This series is to remove the static building for register parsing. In
> theory, we should can dynamically detect if an arch has support this
> feature and functions can return errors when the feature is not
> supported.
>
> The first patch is to remove unused build configuration
> CONFIG_PERF_REGS.
>
> The second patch is to build perf register functions, without using the
> macro 'HAVE_PERF_REGS_SUPPORT' to statically turn on or off code.
>
> The third patch is to introduce a weak function arch__sample_reg_masks(),
> this function can allow the target arch to return its sample register
> list.  With this change, we can totally remove the macro
> 'HAVE_PERF_REGS_SUPPORT' in the source file.
>
> The forth patch is to clean up the Makefile for removing relevant
> configuration and macro definition, as they are not useful anymore.
>
> I tested this patch set on Arm64 and x86 for building and did a cross
> register parsing ('perf record' on Arm64 and 'perf report' on x86).
>
>
> Leo Yan (4):
>   perf build: Remove unused CONFIG_PERF_REGS
>   perf parse-regs: Always build perf register functions
>   perf parse-regs: Introduce a weak function arch__sample_reg_masks()
>   perf build: Cleanup perf register configuration

Thanks Leo, this is great cleanup! Series:
Reviewed-by: Ian Rogers <irogers@...gle.com>

Ian

>  tools/perf/Makefile.config                    | 25 --------------
>  tools/perf/arch/arm/util/perf_regs.c          |  7 +++-
>  tools/perf/arch/arm64/util/machine.c          |  2 ++
>  tools/perf/arch/arm64/util/perf_regs.c        |  7 +++-
>  tools/perf/arch/csky/util/perf_regs.c         |  7 +++-
>  tools/perf/arch/loongarch/util/perf_regs.c    |  7 +++-
>  tools/perf/arch/mips/util/perf_regs.c         |  7 +++-
>  tools/perf/arch/powerpc/util/perf_regs.c      |  7 +++-
>  tools/perf/arch/riscv/util/perf_regs.c        |  7 +++-
>  tools/perf/arch/s390/util/perf_regs.c         |  7 +++-
>  tools/perf/arch/x86/util/perf_regs.c          |  7 +++-
>  tools/perf/util/parse-regs-options.c          |  8 ++---
>  .../util/perf-regs-arch/perf_regs_aarch64.c   |  4 ---
>  .../perf/util/perf-regs-arch/perf_regs_arm.c  |  4 ---
>  .../perf/util/perf-regs-arch/perf_regs_csky.c |  4 ---
>  .../util/perf-regs-arch/perf_regs_loongarch.c |  4 ---
>  .../perf/util/perf-regs-arch/perf_regs_mips.c |  4 ---
>  .../util/perf-regs-arch/perf_regs_powerpc.c   |  4 ---
>  .../util/perf-regs-arch/perf_regs_riscv.c     |  4 ---
>  .../perf/util/perf-regs-arch/perf_regs_s390.c |  4 ---
>  .../perf/util/perf-regs-arch/perf_regs_x86.c  |  4 ---
>  tools/perf/util/perf_regs.c                   | 11 ++++--
>  tools/perf/util/perf_regs.h                   | 34 +------------------
>  23 files changed, 67 insertions(+), 112 deletions(-)
>
> --
> 2.34.1
>