On Fri, Dec 13, 2024 at 03:22:53PM -0800, Namhyung Kim wrote:
> Hello,
>
> On Thu, Dec 12, 2024 at 04:32:51PM -0800, Charlie Jenkins wrote:
> > Currently each architecture in perf independently generates syscall
> > headers. Adapt the work that has gone into unifying syscall header
> > implementations in the kernel to work with perf tools. Introduce this
> > framework with riscv at first. riscv previously relied on libaudit, but
> > with this change, perf tools for riscv no longer needs this external
> > dependency.
>
> Nice work!
>
> >
> > Signed-off-by: Charlie Jenkins <charlie@xxxxxxxxxxxx>
> > ---
> > tools/perf/Makefile.config | 11 +-
> > tools/perf/Makefile.perf | 4 +
> > tools/perf/arch/riscv/Makefile | 22 --
> > tools/perf/arch/riscv/entry/syscalls/Kbuild | 2 +
> > .../arch/riscv/entry/syscalls/Makefile.syscalls | 4 +
> > tools/perf/arch/riscv/entry/syscalls/mksyscalltbl | 47 ---
> > tools/perf/arch/riscv/include/syscall_table.h | 8 +
> > tools/perf/check-headers.sh | 1 +
> > tools/perf/scripts/Makefile.syscalls | 60 +++
> > tools/perf/scripts/syscalltbl.sh | 86 +++++
> > tools/perf/util/syscalltbl.c | 8 +-
> > tools/scripts/syscall.tbl | 409 +++++++++++++++++++++
> > 12 files changed, 585 insertions(+), 77 deletions(-)
> >
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index 2916d59c88cd08b299202a48ddb42fa32aac04a6..a72f25162714f0117a88d94474da336814d4f030 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -35,6 +35,13 @@ ifneq ($(NO_SYSCALL_TABLE),1)
> > NO_SYSCALL_TABLE := 0
> > endif
> >
> > + # architectures that use the generic syscall table scripts
> > + ifeq ($(SRCARCH),riscv)
> > + NO_SYSCALL_TABLE := 0
> > + CFLAGS += -DGENERIC_SYSCALL_TABLE
> > + CFLAGS += -I$(OUTPUT)/tools/perf/arch/$(SRCARCH)/include/generated
> > + endif
> > +
> > ifneq ($(NO_SYSCALL_TABLE),1)
> > CFLAGS += -DHAVE_SYSCALL_TABLE_SUPPORT
> > endif
> > @@ -83,10 +90,6 @@ ifeq ($(ARCH),mips)
> > LIBUNWIND_LIBS = -lunwind -lunwind-mips
> > endif
> >
> > -ifeq ($(ARCH),riscv)
> > - CFLAGS += -I$(OUTPUT)arch/riscv/include/generated
> > -endif
> > -
> > # So far there's only x86 and arm libdw unwind support merged in perf.
> > # Disable it on all other architectures in case libdw unwind
> > # support is detected in system. Add supported architectures
> > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > index d74241a151313bd09101aabb5d765a5a0a6efc84..f5278ed9f778f928436693a14e016c5c3c5171c1 100644
> > --- a/tools/perf/Makefile.perf
> > +++ b/tools/perf/Makefile.perf
> > @@ -310,6 +310,10 @@ ifeq ($(filter feature-dump,$(MAKECMDGOALS)),feature-dump)
> > FEATURE_TESTS := all
> > endif
> > endif
> > +# architectures that use the generic syscall table
> > +ifeq ($(SRCARCH),riscv)
> > +include $(srctree)/tools/perf/scripts/Makefile.syscalls
> > +endif
> > include Makefile.config
> > endif
> >
> > diff --git a/tools/perf/arch/riscv/Makefile b/tools/perf/arch/riscv/Makefile
> > index 18ad078000e2bba595f92efc5d97a63fdb83ef45..087e099fb453a9236db34878077a51f711881ce0 100644
> > --- a/tools/perf/arch/riscv/Makefile
> > +++ b/tools/perf/arch/riscv/Makefile
> > @@ -1,25 +1,3 @@
> > # SPDX-License-Identifier: GPL-2.0
> > PERF_HAVE_JITDUMP := 1
> > HAVE_KVM_STAT_SUPPORT := 1
> > -
> > -#
> > -# Syscall table generation for perf
> > -#
> > -
> > -out := $(OUTPUT)arch/riscv/include/generated/asm
> > -header := $(out)/syscalls.c
> > -incpath := $(srctree)/tools
> > -sysdef := $(srctree)/tools/arch/riscv/include/uapi/asm/unistd.h
> > -sysprf := $(srctree)/tools/perf/arch/riscv/entry/syscalls/
> > -systbl := $(sysprf)/mksyscalltbl
> > -
> > -# Create output directory if not already present
> > -$(shell [ -d '$(out)' ] || mkdir -p '$(out)')
> > -
> > -$(header): $(sysdef) $(systbl)
> > - $(Q)$(SHELL) '$(systbl)' '$(CC)' '$(HOSTCC)' $(incpath) $(sysdef) > $@
> > -
> > -clean::
> > - $(call QUIET_CLEAN, riscv) $(RM) $(header)
> > -
> > -archheaders: $(header)
> > diff --git a/tools/perf/arch/riscv/entry/syscalls/Kbuild b/tools/perf/arch/riscv/entry/syscalls/Kbuild
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..9a41e3572c3afd4f202321fd9e492714540e8fd3
> > --- /dev/null
> > +++ b/tools/perf/arch/riscv/entry/syscalls/Kbuild
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +syscall-y += syscalls_64.h
> > diff --git a/tools/perf/arch/riscv/entry/syscalls/Makefile.syscalls b/tools/perf/arch/riscv/entry/syscalls/Makefile.syscalls
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..9668fd1faf60e828ed2786c2ee84739ac1f153fc
> > --- /dev/null
> > +++ b/tools/perf/arch/riscv/entry/syscalls/Makefile.syscalls
> > @@ -0,0 +1,4 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +syscall_abis_32 += riscv memfd_secret
> > +syscall_abis_64 += riscv rlimit memfd_secret
> > diff --git a/tools/perf/arch/riscv/entry/syscalls/mksyscalltbl b/tools/perf/arch/riscv/entry/syscalls/mksyscalltbl
> > deleted file mode 100755
> > index c59f5e852b97712a9a879b89e6ef6999ed4b6cd7..0000000000000000000000000000000000000000
> > --- a/tools/perf/arch/riscv/entry/syscalls/mksyscalltbl
> > +++ /dev/null
> > @@ -1,47 +0,0 @@
> > -#!/bin/sh
> > -# SPDX-License-Identifier: GPL-2.0
> > -#
> > -# Generate system call table for perf. Derived from
> > -# powerpc script.
> > -#
> > -# Copyright IBM Corp. 2017
> > -# Author(s): Hendrik Brueckner <brueckner@xxxxxxxxxxxxxxxxxx>
> > -# Changed by: Ravi Bangoria <ravi.bangoria@xxxxxxxxxxxxxxxxxx>
> > -# Changed by: Kim Phillips <kim.phillips@xxxxxxx>
> > -# Changed by: Björn Töpel <bjorn@xxxxxxxxxxxx>
> > -
> > -gcc=$1
> > -hostcc=$2
> > -incpath=$3
> > -input=$4
> > -
> > -if ! test -r $input; then
> > - echo "Could not read input file" >&2
> > - exit 1
> > -fi
> > -
> > -create_sc_table()
> > -{
> > - local sc nr max_nr
> > -
> > - while read sc nr; do
> > - printf "%s\n" " [$nr] = \"$sc\","
> > - max_nr=$nr
> > - done
> > -
> > - echo "#define SYSCALLTBL_RISCV_MAX_ID $max_nr"
> > -}
> > -
> > -create_table()
> > -{
> > - echo "#include \"$input\""
> > - echo "static const char *const syscalltbl_riscv[] = {"
> > - create_sc_table
> > - echo "};"
> > -}
> > -
> > -$gcc -E -dM -x c -I $incpath/include/uapi $input \
> > - |awk '$2 ~ "__NR" && $3 !~ "__NR3264_" {
> > - sub("^#define __NR(3264)?_", "");
> > - print | "sort -k2 -n"}' \
> > - |create_table
> > diff --git a/tools/perf/arch/riscv/include/syscall_table.h b/tools/perf/arch/riscv/include/syscall_table.h
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..7ff51b783000d727ec48be960730b81ecdb05575
> > --- /dev/null
> > +++ b/tools/perf/arch/riscv/include/syscall_table.h
> > @@ -0,0 +1,8 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#include <asm/bitsperlong.h>
> > +
> > +#if __BITS_PER_LONG == 64
> > +#include <asm/syscalls_64.h>
> > +#else
> > +#include <asm/syscalls_32.h>
> > +#endif
> > diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
> > index a05c1c105c51bf1bd18a59195220894598eb7461..692f48db810ccbef229e240db29261f0c60db632 100755
> > --- a/tools/perf/check-headers.sh
> > +++ b/tools/perf/check-headers.sh
> > @@ -71,6 +71,7 @@ FILES=(
> > "include/uapi/asm-generic/ioctls.h"
> > "include/uapi/asm-generic/mman-common.h"
> > "include/uapi/asm-generic/unistd.h"
> > + "scripts/syscall.tbl"
> > )
> >
> > declare -a SYNC_CHECK_FILES
> > diff --git a/tools/perf/scripts/Makefile.syscalls b/tools/perf/scripts/Makefile.syscalls
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..e2b0c4b513d5c42c8e9ac51a9ea774c34a1311b7
> > --- /dev/null
> > +++ b/tools/perf/scripts/Makefile.syscalls
> > @@ -0,0 +1,60 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +# This Makefile generates tools/perf/arch/$(SRCARCH)/include/generated/asm from
> > +# the generic syscall table if that is supported by the architecture, otherwise
> > +# from the architecture's defined syscall table.
> > +
> > +PHONY := all
> > +all:
> > +
> > +obj := $(OUTPUT)/tools/perf/arch/$(SRCARCH)/include/generated/asm
> > +
> > +syscall_abis_32 += common,32
> > +syscall_abis_64 += common,64
>
> Couldn't it be := instead of += ?
Yes good point.
>
>
> > +syscalltbl := $(srctree)/tools/scripts/syscall.tbl
> > +
> > +# let architectures override $(syscall_abis_%) and $(syscalltbl)
> > +-include $(srctree)/tools/perf/arch/$(SRCARCH)/entry/syscalls/Makefile.syscalls
> > +include $(srctree)/scripts/Kbuild.include
> > +-include $(srctree)/tools/perf/arch/$(SRCARCH)/entry/syscalls/Kbuild
> > +
> > +systbl := $(srctree)/tools/perf/scripts/syscalltbl.sh
> > +
> > +syscall-y := $(addprefix $(obj)/, $(syscall-y))
> > +
> > +# Remove stale wrappers when the corresponding files are removed from generic-y
> > +old-headers := $(wildcard $(obj)/*.c)
> > +unwanted := $(filter-out $(syscall-y),$(old-headers))
>
> Can you elaborate? What's the stable wrappers?
>
> I think syscall-y and old-headers have .h files but the wildcard above
> is called for .c files?
Oh yes! I meant to change that to .h, thank you for mentioning that, I
had .c at some point during development but changed it to .h files.
- Charlie
>
> Thanks,
> Namhyung
>
> > +
> > +quiet_cmd_remove = REMOVE $(unwanted)
> > + cmd_remove = rm -f $(unwanted)
> > +
> > +quiet_cmd_systbl = SYSTBL $@
> > + cmd_systbl = $(CONFIG_SHELL) $(systbl) \
> > + $(if $(systbl-args-$*),$(systbl-args-$*),$(systbl-args)) \
> > + --abis $(subst $(space),$(comma),$(strip $(syscall_abis_$*))) \
> > + $< $@
> > +
> > +all: $(syscall-y)
> > + $(if $(unwanted),$(call cmd,remove))
> > + @:
> > +
> > +$(obj)/syscalls_%.h: $(syscalltbl) $(systbl) FORCE
> > + $(call if_changed,systbl)
> > +
> > +targets := $(syscall-y)
> > +
> > +# Create output directory. Skip it if at least one old header exists
> > +# since we know the output directory already exists.
> > +ifeq ($(old-headers),)
> > +$(shell mkdir -p $(obj))
> > +endif
> > +
> > +PHONY += FORCE
> > +
> > +FORCE:
> > +
> > +existing-targets := $(wildcard $(sort $(targets)))
> > +
> > +-include $(foreach f,$(existing-targets),$(dir $(f)).$(notdir $(f)).cmd)
> > +
> > +.PHONY: $(PHONY)
