This series contains mainly fixes/improvements for cross-compilation but not only, tested for arm, arm64, but intended for any arch. Also verified on native build (not cross compilation) for x86_64 and arm. Initial RFC link: https://lkml.org/lkml/2019/8/29/1665 Prev. version: https://lkml.org/lkml/2019/9/10/331 Besides the patches given here, the RFC also contains couple patches related to llvm clang arm: include: asm: swab: mask rev16 instruction for clang arm: include: asm: unified: mask .syntax unified for clang They are necessarily to verify arm build. The change touches not only cross-compilation and can have impact on other archs and build environments, so might be good idea to verify it in order to add appropriate changes, some warn options could be tuned also. All is tested on x86-64 with clang installed (has to be built containing targets for arm, arm64..., see llc --version, usually it's present already) Instructions to test native on x86_64 ================================================= Native build on x86_64 is done in usual way and shouldn't have difference except HOSTCC is now printed as CC wile building the samples. Instructions to test cross compilation on arm64 ================================================= #Toolchain used for test: gcc version 8.3.0 (GNU Toolchain for the A-profile Architecture 8.3-2019.03 (arm-rel-8.36)) # Get some arm64 FS, containing at least libelf I've used sdk for TI am65x got here: http://downloads.ti.com/processor-sdk-linux/esd/AM65X/latest/exports/\ ti-processor-sdk-linux-am65xx-evm-06.00.00.07-Linux-x86-Install.bin # Install this binary to some dir, say "sdk". # Configure kernel (use defconfig as no matter), but clean everything # before. make ARCH=arm64 -C tools/ clean make ARCH=arm64 -C samples/bpf clean make ARCH=arm64 clean make ARCH=arm64 defconfig # The kernel version used in sdk doesn't correspond to checked one, # but for this verification only headers need to be syched, # so install them: make ARCH=arm64 headers_install # or on SDK if need keep them in sync (not necessarily to verify): make ARCH=arm64 INSTALL_HDR_PATH=/../sdk/\ ti-processor-sdk-linux-am65xx-evm-06.00.00.07/linux-devkit/sysroots/\ aarch64-linux/usr headers_install # Build samples make samples/bpf/ ARCH=arm64 CROSS_COMPILE="aarch64-linux-gnu-"\ SYSROOT="/../sdk/ti-processor-sdk-linux-am65xx-evm-06.00.00.07/\ linux-devkit/sysroots/aarch64-linux" Instructions to test cross compilation on arm ================================================= #Toolchains used for test: arm-linux-gnueabihf-gcc (Linaro GCC 7.2-2017.11) 7.2.1 20171011 or arm-linux-gnueabihf-gcc (GNU Toolchain for the A-profile Architecture 8.3-2019.03 \ (arm-rel-8.36)) 8.3.0 # Get some FS, I've used sdk for TI am52xx got here: http://downloads.ti.com/processor-sdk-linux/esd/AM57X/05_03_00_07/exports/\ ti-processor-sdk-linux-am57xx-evm-05.03.00.07-Linux-x86-Install.bin # Install this binary to some dir, say "sdk". # Configure kernel, but clean everything before. make ARCH=arm -C tools/ clean make ARCH=arm -C samples/bpf clean make ARCH=arm clean make ARCH=arm omap2plus_defconfig # The kernel version used in sdk doesn't correspond to checked one, but headers only should be synched, so install them: make ARCH=arm64 headers_install # or on SDK if need keep them in sync (not necessarily): make ARCH=arm INSTALL_HDR_PATH=/../sdk/\ ti-processor-sdk-linux-am57xx-evm-05.03.00.07/linux-devkit/sysroots/\ armv7ahf-neon-linux-gnueabi/usr headers_install # Build samples make samples/bpf/ ARCH=arm CROSS_COMPILE="arm-linux-gnueabihf-"\ SYSROOT="/../sdk/ti-processor-sdk-linux-am57xx-evm-05.03\ .00.07/linux-devkit/sysroots/armv7ahf-neon-linux-gnueabi" Based on bpf-next/master v3..v2: - renamed makefile.progs to makeifle.target, as more appropriate - left only __LINUX_ARM_ARCH__ for D options for arm - for host build - left options from KBUILD_HOST for compatibility reasons - split patch adding c/cxx/ld flags to libbpf by modules - moved readme change to separate patch - added patch setting options for cross-compile - fixed issue with option error for syscall_nrs.S, avoiding overlap for ccflags-y. v2..v1: - restructured patches order - split "samples: bpf: Makefile: base progs build on Makefile.progs" to make change more readable. It added couple nice extra patches. - removed redundant patch: "samples: bpf: Makefile: remove target for native build" - added fix: "samples: bpf: makefile: fix cookie_uid_helper_example obj build" - limited -D option filter only for arm - improved comments - added couple instructions to verify cross compilation for arm and arm64 arches based on TI am57xx and am65xx sdks. - corrected include a little order Ivan Khoronzhuk (14): samples: bpf: makefile: fix HDR_PROBE "echo" samples: bpf: makefile: fix cookie_uid_helper_example obj build samples: bpf: makefile: use --target from cross-compile samples: bpf: use own EXTRA_CFLAGS for clang commands samples: bpf: makefile: use __LINUX_ARM_ARCH__ selector for arm samples: bpf: makefile: drop unnecessarily inclusion for bpf_load samples: bpf: add makefile.target for separate CC target build samples: bpf: makefile: base target programs rules on Makefile.target samples: bpf: makefile: use own flags but not host when cross compile samples: bpf: makefile: use target CC environment for HDR_PROBE libbpf: makefile: add C/CXX/LDFLAGS to libbpf.so and test_libpf targets samples: bpf: makefile: provide C/CXX/LD flags to libbpf samples: bpf: makefile: add sysroot support samples: bpf: README: add preparation steps and sysroot info samples/bpf/Makefile | 179 +++++++++++++++++++++--------------- samples/bpf/Makefile.target | 75 +++++++++++++++ samples/bpf/README.rst | 41 ++++++++- tools/lib/bpf/Makefile | 11 ++- 4 files changed, 225 insertions(+), 81 deletions(-) create mode 100644 samples/bpf/Makefile.target -- 2.17.1
echo should be replaced with echo -e to handle '\n' correctly, but instead, replace it with printf as some systems can't handle echo -e. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- samples/bpf/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index 1d9be26b4edd..f50ca852c2a8 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -201,7 +201,7 @@ endif # Don't evaluate probes and warnings if we need to run make recursively ifneq ($(src),) -HDR_PROBE := $(shell echo "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \ +HDR_PROBE := $(shell printf "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \ $(HOSTCC) $(KBUILD_HOSTCFLAGS) -x c - -o /dev/null 2>/dev/null && \ echo okay) -- 2.17.1
Don't list userspace "cookie_uid_helper_example" object in list for bpf objects. 'always' target is used for listing bpf programs, but 'cookie_uid_helper_example.o' is a user space ELF file, and covered by rule `per_socket_stats_example`, so shouldn't be in 'always'. Let us remove `always += cookie_uid_helper_example.o`, which avoids breaking cross compilation due to mismatched includes. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- samples/bpf/Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index f50ca852c2a8..43dee90dffa4 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -145,7 +145,6 @@ always += sampleip_kern.o always += lwt_len_hist_kern.o always += xdp_tx_iptunnel_kern.o always += test_map_in_map_kern.o -always += cookie_uid_helper_example.o always += tcp_synrto_kern.o always += tcp_rwnd_kern.o always += tcp_bufs_kern.o -- 2.17.1
For cross compiling the target triple can be inherited from cross-compile prefix as it's done in CLANG_FLAGS from kernel makefile. So copy-paste this decision from kernel Makefile. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- samples/bpf/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index 43dee90dffa4..b59e77e2250e 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -195,7 +195,7 @@ BTF_PAHOLE ?= pahole # Detect that we're cross compiling and use the cross compiler ifdef CROSS_COMPILE HOSTCC = $(CROSS_COMPILE)gcc -CLANG_ARCH_ARGS = -target $(ARCH) +CLANG_ARCH_ARGS = --target=$(notdir $(CROSS_COMPILE:%-=%)) endif # Don't evaluate probes and warnings if we need to run make recursively -- 2.17.1
It can overlap with CFLAGS used for libraries built with gcc if not now then in next patches. Correct it here for simplicity. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- samples/bpf/Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index b59e77e2250e..8ecc5d0c2d5b 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -218,10 +218,10 @@ BTF_LLVM_PROBE := $(shell echo "int main() { return 0; }" | \ /bin/rm -f ./llvm_btf_verify.o) ifneq ($(BTF_LLVM_PROBE),) - EXTRA_CFLAGS += -g + CLANG_EXTRA_CFLAGS += -g else ifneq ($(and $(BTF_LLC_PROBE),$(BTF_PAHOLE_PROBE),$(BTF_OBJCOPY_PROBE)),) - EXTRA_CFLAGS += -g + CLANG_EXTRA_CFLAGS += -g LLC_FLAGS += -mattr=dwarfris DWARF2BTF = y endif @@ -280,8 +280,8 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h # useless for BPF samples. $(obj)/%.o: $(src)/%.c @echo " CLANG-bpf " $@ - $(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \ - -I$(srctree)/tools/testing/selftests/bpf/ \ + $(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(CLANG_EXTRA_CFLAGS) \ + -I$(obj) -I$(srctree)/tools/testing/selftests/bpf/ \ -D__KERNEL__ -D__BPF_TRACING__ -Wno-unused-value -Wno-pointer-sign \ -D__TARGET_ARCH_$(SRCARCH) -Wno-compare-distinct-pointer-types \ -Wno-gnu-variable-sized-type-not-at-end \ -- 2.17.1
For arm, -D__LINUX_ARM_ARCH__=X is min version used as instruction set selector and is absolutely required while parsing some parts of headers. It's present in KBUILD_CFLAGS but not in autoconf.h, so let's retrieve it from and add to programs cflags. In another case errors like "SMP is not supported" for armv7 and bunch of other errors are issued resulting to incorrect final object. --- samples/bpf/Makefile | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index 8ecc5d0c2d5b..d3c8db3df560 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -185,6 +185,16 @@ HOSTLDLIBS_map_perf_test += -lrt HOSTLDLIBS_test_overhead += -lrt HOSTLDLIBS_xdpsock += -pthread +ifeq ($(ARCH), arm) +# Strip all except -D__LINUX_ARM_ARCH__ option needed to handle linux +# headers when arm instruction set identification is requested. +ARM_ARCH_SELECTOR = $(shell echo "$(KBUILD_CFLAGS) " | \ + sed 's/[[:blank:]]/\n/g' | sed '/^-D__LINUX_ARM_ARCH__/!d') + +CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR) +KBUILD_HOSTCFLAGS := $(ARM_ARCH_SELECTOR) +endif + # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline: # make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang LLC ?= llc -- 2.17.1
Drop inclusion for bpf_load -I$(objtree)/usr/include as it is included for all objects anyway, with above line: KBUILD_HOSTCFLAGS += -I$(objtree)/usr/include Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- samples/bpf/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index d3c8db3df560..9d923546e087 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -176,7 +176,7 @@ KBUILD_HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include KBUILD_HOSTCFLAGS += -I$(srctree)/tools/perf -HOSTCFLAGS_bpf_load.o += -I$(objtree)/usr/include -Wno-unused-variable +HOSTCFLAGS_bpf_load.o += -Wno-unused-variable KBUILD_HOSTLDLIBS += $(LIBBPF) -lelf HOSTLDLIBS_tracex4 += -lrt -- 2.17.1
The makefile.target is added only and will be used in sample/bpf/Makefile later in order to switch cross-compiling on CC from HOSTCC environment. The HOSTCC is supposed to build binaries and tools running on the host afterwards, in order to simplify build or so, like "fixdep" or else. In case of cross compiling "fixdep" is executed on host when the rest samples should run on target arch. In order to build binaries for target arch with CC and tools running on host with HOSTCC, lets add Makefile.target for simplicity, having definition and routines similar to ones, used in script/Makefile.host. This allows later add cross-compilation to samples/bpf with minimum changes. The tprog stands for target programs built with CC. Makefile.target contains only stuff needed for samples/bpf, potentially can be reused later and now needed only for unblocking tricky samples/bpf cross compilation. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- samples/bpf/Makefile.target | 75 +++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 samples/bpf/Makefile.target diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target new file mode 100644 index 000000000000..fb6de63f7d2f --- /dev/null +++ b/samples/bpf/Makefile.target @@ -0,0 +1,75 @@ +# SPDX-License-Identifier: GPL-2.0 +# ========================================================================== +# Building binaries on the host system +# Binaries are not used during the compilation of the kernel, and intendent +# to be build for target board, target board can be host ofc. Added to build +# binaries to run not on host system. +# +# Sample syntax (see Documentation/kbuild/makefiles.rst for reference) +# tprogs-y := xsk_example +# Will compile xdpsock_example.c and create an executable named xsk_example +# +# tprogs-y := xdpsock +# xdpsock-objs := xdpsock_1.o xdpsock_2.o +# Will compile xdpsock_1.c and xdpsock_2.c, and then link the executable +# xdpsock, based on xdpsock_1.o and xdpsock_2.o +# +# Inherited from scripts/Makefile.host +# +__tprogs := $(sort $(tprogs-y)) + +# C code +# Executables compiled from a single .c file +tprog-csingle := $(foreach m,$(__tprogs), \ + $(if $($(m)-objs),,$(m))) + +# C executables linked based on several .o files +tprog-cmulti := $(foreach m,$(__tprogs),\ + $(if $($(m)-objs),$(m))) + +# Object (.o) files compiled from .c files +tprog-cobjs := $(sort $(foreach m,$(__tprogs),$($(m)-objs))) + +tprog-csingle := $(addprefix $(obj)/,$(tprog-csingle)) +tprog-cmulti := $(addprefix $(obj)/,$(tprog-cmulti)) +tprog-cobjs := $(addprefix $(obj)/,$(tprog-cobjs)) + +##### +# Handle options to gcc. Support building with separate output directory + +_tprogc_flags = $(TPROGS_CFLAGS) \ + $(TPROGCFLAGS_$(basetarget).o) + +# $(objtree)/$(obj) for including generated headers from checkin source files +ifeq ($(KBUILD_EXTMOD),) +ifdef building_out_of_srctree +_tprogc_flags += -I $(objtree)/$(obj) +endif +endif + +tprogc_flags = -Wp,-MD,$(depfile) $(_tprogc_flags) + +# Create executable from a single .c file +# tprog-csingle -> Executable +quiet_cmd_tprog-csingle = CC $@ + cmd_tprog-csingle = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ $< \ + $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F)) +$(tprog-csingle): $(obj)/%: $(src)/%.c FORCE + $(call if_changed_dep,tprog-csingle) + +# Link an executable based on list of .o files, all plain c +# tprog-cmulti -> executable +quiet_cmd_tprog-cmulti = LD $@ + cmd_tprog-cmulti = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ \ + $(addprefix $(obj)/,$($(@F)-objs)) \ + $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F)) +$(tprog-cmulti): $(tprog-cobjs) FORCE + $(call if_changed,tprog-cmulti) +$(call multi_depend, $(tprog-cmulti), , -objs) + +# Create .o file from a single .c file +# tprog-cobjs -> .o +quiet_cmd_tprog-cobjs = CC $@ + cmd_tprog-cobjs = $(CC) $(tprogc_flags) -c -o $@ $< +$(tprog-cobjs): $(obj)/%.o: $(src)/%.c FORCE + $(call if_changed_dep,tprog-cobjs) -- 2.17.1
The main reason for that - HOSTCC and CC have different aims. HOSTCC is used to build programs running on host, that can cross-comple target programs with CC. It was tested for arm and arm64 cross compilation, based on linaro toolchain, but should work for others. So, in order to split cross compilation (CC) with host build (HOSTCC), lets base samples on Makefile.target. It allows to cross-compile samples/bpf programs with CC while auxialry tools running on host built with HOSTCC. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- samples/bpf/Makefile | 135 ++++++++++++++++++++++--------------------- 1 file changed, 69 insertions(+), 66 deletions(-) diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index 9d923546e087..1579cc16a1c2 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -4,55 +4,53 @@ BPF_SAMPLES_PATH ?= $(abspath $(srctree)/$(src)) TOOLS_PATH := $(BPF_SAMPLES_PATH)/../../tools # List of programs to build -hostprogs-y := test_lru_dist -hostprogs-y += sock_example -hostprogs-y += fds_example -hostprogs-y += sockex1 -hostprogs-y += sockex2 -hostprogs-y += sockex3 -hostprogs-y += tracex1 -hostprogs-y += tracex2 -hostprogs-y += tracex3 -hostprogs-y += tracex4 -hostprogs-y += tracex5 -hostprogs-y += tracex6 -hostprogs-y += tracex7 -hostprogs-y += test_probe_write_user -hostprogs-y += trace_output -hostprogs-y += lathist -hostprogs-y += offwaketime -hostprogs-y += spintest -hostprogs-y += map_perf_test -hostprogs-y += test_overhead -hostprogs-y += test_cgrp2_array_pin -hostprogs-y += test_cgrp2_attach -hostprogs-y += test_cgrp2_sock -hostprogs-y += test_cgrp2_sock2 -hostprogs-y += xdp1 -hostprogs-y += xdp2 -hostprogs-y += xdp_router_ipv4 -hostprogs-y += test_current_task_under_cgroup -hostprogs-y += trace_event -hostprogs-y += sampleip -hostprogs-y += tc_l2_redirect -hostprogs-y += lwt_len_hist -hostprogs-y += xdp_tx_iptunnel -hostprogs-y += test_map_in_map -hostprogs-y += per_socket_stats_example -hostprogs-y += xdp_redirect -hostprogs-y += xdp_redirect_map -hostprogs-y += xdp_redirect_cpu -hostprogs-y += xdp_monitor -hostprogs-y += xdp_rxq_info -hostprogs-y += syscall_tp -hostprogs-y += cpustat -hostprogs-y += xdp_adjust_tail -hostprogs-y += xdpsock -hostprogs-y += xdp_fwd -hostprogs-y += task_fd_query -hostprogs-y += xdp_sample_pkts -hostprogs-y += ibumad -hostprogs-y += hbm +tprogs-y := test_lru_dist +tprogs-y += sock_example +tprogs-y += fds_example +tprogs-y += sockex1 +tprogs-y += sockex2 +tprogs-y += sockex3 +tprogs-y += tracex1 +tprogs-y += tracex2 +tprogs-y += tracex3 +tprogs-y += tracex4 +tprogs-y += tracex5 +tprogs-y += tracex6 +tprogs-y += tracex7 +tprogs-y += test_probe_write_user +tprogs-y += trace_output +tprogs-y += lathist +tprogs-y += offwaketime +tprogs-y += spintest +tprogs-y += map_perf_test +tprogs-y += test_overhead +tprogs-y += test_cgrp2_array_pin +tprogs-y += test_cgrp2_attach +tprogs-y += test_cgrp2_sock +tprogs-y += test_cgrp2_sock2 +tprogs-y += xdp1 +tprogs-y += xdp2 +tprogs-y += xdp_router_ipv4 +tprogs-y += test_current_task_under_cgroup +tprogs-y += trace_event +tprogs-y += sampleip +tprogs-y += tc_l2_redirect +tprogs-y += lwt_len_hist +tprogs-y += xdp_tx_iptunnel +tprogs-y += test_map_in_map +tprogs-y += xdp_redirect_map +tprogs-y += xdp_redirect_cpu +tprogs-y += xdp_monitor +tprogs-y += xdp_rxq_info +tprogs-y += syscall_tp +tprogs-y += cpustat +tprogs-y += xdp_adjust_tail +tprogs-y += xdpsock +tprogs-y += xdp_fwd +tprogs-y += task_fd_query +tprogs-y += xdp_sample_pkts +tprogs-y += ibumad +tprogs-y += hbm # Libbpf dependencies LIBBPF = $(TOOLS_PATH)/lib/bpf/libbpf.a @@ -111,7 +109,7 @@ ibumad-objs := bpf_load.o ibumad_user.o $(TRACE_HELPERS) hbm-objs := bpf_load.o hbm.o $(CGROUP_HELPERS) # Tell kbuild to always build the programs -always := $(hostprogs-y) +always := $(tprogs-y) always += sockex1_kern.o always += sockex2_kern.o always += sockex3_kern.o @@ -170,21 +168,6 @@ always += ibumad_kern.o always += hbm_out_kern.o always += hbm_edt_kern.o -KBUILD_HOSTCFLAGS += -I$(objtree)/usr/include -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/bpf/ -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/perf - -HOSTCFLAGS_bpf_load.o += -Wno-unused-variable - -KBUILD_HOSTLDLIBS += $(LIBBPF) -lelf -HOSTLDLIBS_tracex4 += -lrt -HOSTLDLIBS_trace_output += -lrt -HOSTLDLIBS_map_perf_test += -lrt -HOSTLDLIBS_test_overhead += -lrt -HOSTLDLIBS_xdpsock += -pthread - ifeq ($(ARCH), arm) # Strip all except -D__LINUX_ARM_ARCH__ option needed to handle linux # headers when arm instruction set identification is requested. @@ -192,9 +175,27 @@ ARM_ARCH_SELECTOR = $(shell echo "$(KBUILD_CFLAGS) " | \ sed 's/[[:blank:]]/\n/g' | sed '/^-D__LINUX_ARM_ARCH__/!d') CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR) -KBUILD_HOSTCFLAGS := $(ARM_ARCH_SELECTOR) +TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR) endif +TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS) +TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) +TPROGS_CFLAGS += -I$(objtree)/usr/include +TPROGS_CFLAGS += -I$(srctree)/tools/lib/bpf/ +TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ +TPROGS_CFLAGS += -I$(srctree)/tools/lib/ +TPROGS_CFLAGS += -I$(srctree)/tools/include +TPROGS_CFLAGS += -I$(srctree)/tools/perf + +TPROGCFLAGS_bpf_load.o += -Wno-unused-variable + +TPROGS_LDLIBS += $(LIBBPF) -lelf +TPROGLDLIBS_tracex4 += -lrt +TPROGLDLIBS_trace_output += -lrt +TPROGLDLIBS_map_perf_test += -lrt +TPROGLDLIBS_test_overhead += -lrt +TPROGLDLIBS_xdpsock += -pthread + # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline: # make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang LLC ?= llc @@ -285,6 +286,8 @@ $(obj)/hbm_out_kern.o: $(src)/hbm.h $(src)/hbm_kern.h $(obj)/hbm.o: $(src)/hbm.h $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h +-include $(BPF_SAMPLES_PATH)/Makefile.target + # asm/sysreg.h - inline assembly used by it is incompatible with llvm. # But, there is no easy way to fix it, so just exclude it since it is # useless for BPF samples. -- 2.17.1
While compile natively, the hosts cflags and ldflags are equal to ones used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should have own, used for target arch. While verification, for arm, arm64 and x86_64 the following flags were used alsways: -Wall -O2 -fomit-frame-pointer -Wmissing-prototypes -Wstrict-prototypes So, add them as they were verified and used before adding Makefile.target, but anyway limit it only for cross compile options as for host can be some configurations when another options can be used, So, for host arch samples left all as is, it allows to avoid potential option mistmatches for existent environments. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- samples/bpf/Makefile | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index 1579cc16a1c2..b5c87a8b8b51 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -178,8 +178,17 @@ CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR) TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR) endif +ifdef CROSS_COMPILE +TPROGS_CFLAGS += -Wall +TPROGS_CFLAGS += -O2 +TPROGS_CFLAGS += -fomit-frame-pointer +TPROGS_CFLAGS += -Wmissing-prototypes +TPROGS_CFLAGS += -Wstrict-prototypes +else TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS) TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) +endif + TPROGS_CFLAGS += -I$(objtree)/usr/include TPROGS_CFLAGS += -I$(srctree)/tools/lib/bpf/ TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ -- 2.17.1
No need in hacking HOSTCC to be cross-compiler any more, so drop this trick and use target CC for HDR_PROBE. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- samples/bpf/Makefile | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index b5c87a8b8b51..18ec22e7b444 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -214,15 +214,14 @@ BTF_PAHOLE ?= pahole # Detect that we're cross compiling and use the cross compiler ifdef CROSS_COMPILE -HOSTCC = $(CROSS_COMPILE)gcc CLANG_ARCH_ARGS = --target=$(notdir $(CROSS_COMPILE:%-=%)) endif # Don't evaluate probes and warnings if we need to run make recursively ifneq ($(src),) HDR_PROBE := $(shell printf "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \ - $(HOSTCC) $(KBUILD_HOSTCFLAGS) -x c - -o /dev/null 2>/dev/null && \ - echo okay) + $(CC) $(TPROGS_CFLAGS) $(TPROGS_LDFLAGS) -x c - \ + -o /dev/null 2>/dev/null && echo okay) ifeq ($(HDR_PROBE),) $(warning WARNING: Detected possible issues with include path.) -- 2.17.1
In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them correctly to build command, for instance when --sysroot is used or external libraries are used, like -lelf, wich can be absent in toolchain. This can be used for samples/bpf cross-compiling allowing to get elf lib from sysroot. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- tools/lib/bpf/Makefile | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile index c6f94cffe06e..bccfa556ef4e 100644 --- a/tools/lib/bpf/Makefile +++ b/tools/lib/bpf/Makefile @@ -94,6 +94,10 @@ else CFLAGS := -g -Wall endif +ifdef EXTRA_CXXFLAGS + CXXFLAGS := $(EXTRA_CXXFLAGS) +endif + ifeq ($(feature-libelf-mmap), 1) override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT endif @@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION) $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN) - $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \ - -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@ + $(QUIET_LINK)$(CC) $(LDFLAGS) \ + --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \ + -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@ @ln -sf $(@F) $(OUTPUT)libbpf.so @ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION) @@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN) $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^ $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a - $(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@ + $(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@ $(OUTPUT)libbpf.pc: $(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \ -- 2.17.1
In order to build libs using C/CXX/LD flags of target arch, provide them to libbpf make. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- samples/bpf/Makefile | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index 18ec22e7b444..133123d4c7d7 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -182,8 +182,6 @@ ifdef CROSS_COMPILE TPROGS_CFLAGS += -Wall TPROGS_CFLAGS += -O2 TPROGS_CFLAGS += -fomit-frame-pointer -TPROGS_CFLAGS += -Wmissing-prototypes -TPROGS_CFLAGS += -Wstrict-prototypes else TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS) TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) @@ -196,6 +194,14 @@ TPROGS_CFLAGS += -I$(srctree)/tools/lib/ TPROGS_CFLAGS += -I$(srctree)/tools/include TPROGS_CFLAGS += -I$(srctree)/tools/perf +EXTRA_CXXFLAGS := $(TPROGS_CFLAGS) + +# options not valid for C++ +ifdef CROSS_COMPILE +$(TPROGS_CFLAGS) += -Wmissing-prototypes +$(TPROGS_CFLAGS) += -Wstrict-prototypes +endif + TPROGCFLAGS_bpf_load.o += -Wno-unused-variable TPROGS_LDLIBS += $(LIBBPF) -lelf @@ -257,7 +263,9 @@ clean: $(LIBBPF): FORCE # Fix up variables inherited from Kbuild that tools/ build system won't like - $(MAKE) -C $(dir $@) RM='rm -rf' LDFLAGS= srctree=$(BPF_SAMPLES_PATH)/../../ O= + $(MAKE) -C $(dir $@) RM='rm -rf' EXTRA_CFLAGS="$(TPROGS_CFLAGS)" \ + EXTRA_CXXFLAGS="$(EXTRA_CXXFLAGS)" LDFLAGS=$(TPROGS_LDFLAGS) \ + srctree=$(BPF_SAMPLES_PATH)/../../ O= $(obj)/syscall_nrs.h: $(obj)/syscall_nrs.s FORCE $(call filechk,offsets,__SYSCALL_NRS_H__) -- 2.17.1
Basically it only enables that was added by previous couple fixes. Sysroot contains correct libs installed and its headers ofc. Useful when working with NFC or virtual machine. Usage: clean (on demand) make ARCH=arm -C samples/bpf clean make ARCH=arm -C tools clean make ARCH=arm clean configure and install headers: make ARCH=arm defconfig make ARCH=arm headers_install build samples/bpf: make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- samples/bpf/ \ SYSROOT="path/to/sysroot" Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- samples/bpf/Makefile | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index 133123d4c7d7..57ddf055d6c3 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -194,6 +194,11 @@ TPROGS_CFLAGS += -I$(srctree)/tools/lib/ TPROGS_CFLAGS += -I$(srctree)/tools/include TPROGS_CFLAGS += -I$(srctree)/tools/perf +ifdef SYSROOT +TPROGS_CFLAGS += --sysroot=${SYSROOT} +TPROGS_LDFLAGS := -L${SYSROOT}/usr/lib +endif + EXTRA_CXXFLAGS := $(TPROGS_CFLAGS) # options not valid for C++ -- 2.17.1
Add couple preparation steps: clean and configuration. Also add newly added sysroot support info to cross-compile section. --- samples/bpf/README.rst | 41 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/samples/bpf/README.rst b/samples/bpf/README.rst index 5f27e4faca50..d5845d73ab7d 100644 --- a/samples/bpf/README.rst +++ b/samples/bpf/README.rst @@ -14,6 +14,20 @@ Compiling requires having installed: Note that LLVM's tool 'llc' must support target 'bpf', list version and supported targets with command: ``llc --version`` +Clean and configuration +----------------------- + +It can be needed to clean tools, samples or kernel before trying new arch or +after some changes (on demand):: + + make -C tools clean + make -C samples/bpf clean + make clean + +Configure kernel, defconfig for instance:: + + make defconfig + Kernel headers -------------- @@ -68,9 +82,26 @@ It is also possible to point make to the newly compiled 'llc' or Cross compiling samples ----------------------- In order to cross-compile, say for arm64 targets, export CROSS_COMPILE and ARCH -environment variables before calling make. This will direct make to build -samples for the cross target. +environment variables before calling make. But do this before clean, +cofiguration and header install steps described above. This will direct make to +build samples for the cross target:: + + export ARCH=arm64 + export CROSS_COMPILE="aarch64-linux-gnu-" + +Headers can be also installed on RFC of target board if need to keep them in +sync (not necessarily and it creates a local "usr/include" directory also):: + + make INSTALL_HDR_PATH=~/some_sysroot/usr headers_install + +Pointing LLC and CLANG is not necessarily if it's installed on HOST and have +in its targets appropriate arm64 arch (usually it has several arches). +Build samples:: + + make samples/bpf/ + +Or build samples with SYSROOT if some header or library is absent in toolchain, +say libelf, providing address to file system containing headers and libs, +can be RFS of target board:: -export ARCH=arm64 -export CROSS_COMPILE="aarch64-linux-gnu-" -make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang + make samples/bpf/ SYSROOT=~/some_sysroot -- 2.17.1
On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > echo should be replaced with echo -e to handle '\n' correctly, but > instead, replace it with printf as some systems can't handle echo -e. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- > samples/bpf/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > index 1d9be26b4edd..f50ca852c2a8 100644 > --- a/samples/bpf/Makefile > +++ b/samples/bpf/Makefile > @@ -201,7 +201,7 @@ endif > > # Don't evaluate probes and warnings if we need to run make recursively > ifneq ($(src),) > -HDR_PROBE := $(shell echo "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \ > +HDR_PROBE := $(shell printf "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \ printf change is fine, but I'm confused about \# at the beginning of the string. Not sure what was the intent, but it seems like it should work with just #include at the beginning. > $(HOSTCC) $(KBUILD_HOSTCFLAGS) -x c - -o /dev/null 2>/dev/null && \ > echo okay) > > -- > 2.17.1 >
On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > Don't list userspace "cookie_uid_helper_example" object in list for > bpf objects. > > 'always' target is used for listing bpf programs, but > 'cookie_uid_helper_example.o' is a user space ELF file, and covered > by rule `per_socket_stats_example`, so shouldn't be in 'always'. > Let us remove `always += cookie_uid_helper_example.o`, which avoids > breaking cross compilation due to mismatched includes. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- Acked-by: Andrii Nakryiko <andriin@fb.com> > samples/bpf/Makefile | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > index f50ca852c2a8..43dee90dffa4 100644 > --- a/samples/bpf/Makefile > +++ b/samples/bpf/Makefile > @@ -145,7 +145,6 @@ always += sampleip_kern.o > always += lwt_len_hist_kern.o > always += xdp_tx_iptunnel_kern.o > always += test_map_in_map_kern.o > -always += cookie_uid_helper_example.o > always += tcp_synrto_kern.o > always += tcp_rwnd_kern.o > always += tcp_bufs_kern.o > -- > 2.17.1 >
On Mon, Sep 16, 2019 at 4:01 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > For cross compiling the target triple can be inherited from > cross-compile prefix as it's done in CLANG_FLAGS from kernel makefile. > So copy-paste this decision from kernel Makefile. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- Acked-by: Andrii Nakryiko <andriin@fb.com> > samples/bpf/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > index 43dee90dffa4..b59e77e2250e 100644 > --- a/samples/bpf/Makefile > +++ b/samples/bpf/Makefile > @@ -195,7 +195,7 @@ BTF_PAHOLE ?= pahole > # Detect that we're cross compiling and use the cross compiler > ifdef CROSS_COMPILE > HOSTCC = $(CROSS_COMPILE)gcc > -CLANG_ARCH_ARGS = -target $(ARCH) > +CLANG_ARCH_ARGS = --target=$(notdir $(CROSS_COMPILE:%-=%)) > endif > > # Don't evaluate probes and warnings if we need to run make recursively > -- > 2.17.1 >
On Mon, Sep 16, 2019 at 4:01 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > It can overlap with CFLAGS used for libraries built with gcc if > not now then in next patches. Correct it here for simplicity. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- With GCC BPF front-end recently added, we should probably generalize this to something like BPF_EXTRA_CFLAGS or something like that, eventually. But for now: Acked-by: Andrii Nakryiko <andriin@fb.com> > samples/bpf/Makefile | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > index b59e77e2250e..8ecc5d0c2d5b 100644 > --- a/samples/bpf/Makefile > +++ b/samples/bpf/Makefile > @@ -218,10 +218,10 @@ BTF_LLVM_PROBE := $(shell echo "int main() { return 0; }" | \ > /bin/rm -f ./llvm_btf_verify.o) > > ifneq ($(BTF_LLVM_PROBE),) > - EXTRA_CFLAGS += -g > + CLANG_EXTRA_CFLAGS += -g > else > ifneq ($(and $(BTF_LLC_PROBE),$(BTF_PAHOLE_PROBE),$(BTF_OBJCOPY_PROBE)),) > - EXTRA_CFLAGS += -g > + CLANG_EXTRA_CFLAGS += -g > LLC_FLAGS += -mattr=dwarfris > DWARF2BTF = y > endif > @@ -280,8 +280,8 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h > # useless for BPF samples. > $(obj)/%.o: $(src)/%.c > @echo " CLANG-bpf " $@ > - $(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \ > - -I$(srctree)/tools/testing/selftests/bpf/ \ > + $(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(CLANG_EXTRA_CFLAGS) \ > + -I$(obj) -I$(srctree)/tools/testing/selftests/bpf/ \ > -D__KERNEL__ -D__BPF_TRACING__ -Wno-unused-value -Wno-pointer-sign \ > -D__TARGET_ARCH_$(SRCARCH) -Wno-compare-distinct-pointer-types \ > -Wno-gnu-variable-sized-type-not-at-end \ > -- > 2.17.1 >
On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > For arm, -D__LINUX_ARM_ARCH__=X is min version used as instruction > set selector and is absolutely required while parsing some parts of > headers. It's present in KBUILD_CFLAGS but not in autoconf.h, so let's > retrieve it from and add to programs cflags. In another case errors > like "SMP is not supported" for armv7 and bunch of other errors are > issued resulting to incorrect final object. > --- > samples/bpf/Makefile | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > index 8ecc5d0c2d5b..d3c8db3df560 100644 > --- a/samples/bpf/Makefile > +++ b/samples/bpf/Makefile > @@ -185,6 +185,16 @@ HOSTLDLIBS_map_perf_test += -lrt > HOSTLDLIBS_test_overhead += -lrt > HOSTLDLIBS_xdpsock += -pthread > > +ifeq ($(ARCH), arm) > +# Strip all except -D__LINUX_ARM_ARCH__ option needed to handle linux > +# headers when arm instruction set identification is requested. > +ARM_ARCH_SELECTOR = $(shell echo "$(KBUILD_CFLAGS) " | \ > + sed 's/[[:blank:]]/\n/g' | sed '/^-D__LINUX_ARM_ARCH__/!d') Does the following work exactly like that without shelling out (and being arguably simpler)? ARM_ARCH_SELECTOR = $(filter -D__LINUX_ARM_ARCH__%, $(KBUILD_CFLAGS)) > + > +CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR) > +KBUILD_HOSTCFLAGS := $(ARM_ARCH_SELECTOR) Isn't this clearing out previous value of KBUILD_HOSTCFLAGS? Is that intentional, or it was supposed to be += here? > +endif > + > # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline: > # make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang > LLC ?= llc > -- > 2.17.1 >
On Mon, Sep 16, 2019 at 4:01 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > Drop inclusion for bpf_load -I$(objtree)/usr/include as it is > included for all objects anyway, with above line: > KBUILD_HOSTCFLAGS += -I$(objtree)/usr/include > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- Acked-by: Andrii Nakryiko <andriin@fb.com> > samples/bpf/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > index d3c8db3df560..9d923546e087 100644 > --- a/samples/bpf/Makefile > +++ b/samples/bpf/Makefile > @@ -176,7 +176,7 @@ KBUILD_HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ > KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include > KBUILD_HOSTCFLAGS += -I$(srctree)/tools/perf > > -HOSTCFLAGS_bpf_load.o += -I$(objtree)/usr/include -Wno-unused-variable > +HOSTCFLAGS_bpf_load.o += -Wno-unused-variable > > KBUILD_HOSTLDLIBS += $(LIBBPF) -lelf > HOSTLDLIBS_tracex4 += -lrt > -- > 2.17.1 >
On Mon, Sep 16, 2019 at 01:13:23PM -0700, Andrii Nakryiko wrote: >On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk ><ivan.khoronzhuk@linaro.org> wrote: >> >> echo should be replaced with echo -e to handle '\n' correctly, but >> instead, replace it with printf as some systems can't handle echo -e. >> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> >> --- >> samples/bpf/Makefile | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile >> index 1d9be26b4edd..f50ca852c2a8 100644 >> --- a/samples/bpf/Makefile >> +++ b/samples/bpf/Makefile >> @@ -201,7 +201,7 @@ endif >> >> # Don't evaluate probes and warnings if we need to run make recursively >> ifneq ($(src),) >> -HDR_PROBE := $(shell echo "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \ >> +HDR_PROBE := $(shell printf "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \ > >printf change is fine, but I'm confused about \# at the beginning of >the string. Not sure what was the intent, but it seems like it should >work with just #include at the beginning. At least no warns, but looks like should work. Will try it in next v. > >> $(HOSTCC) $(KBUILD_HOSTCFLAGS) -x c - -o /dev/null 2>/dev/null && \ >> echo okay) >> >> -- >> 2.17.1 >> -- Regards, Ivan Khoronzhuk
On Sep 16 2019, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
> <ivan.khoronzhuk@linaro.org> wrote:
>>
>> echo should be replaced with echo -e to handle '\n' correctly, but
>> instead, replace it with printf as some systems can't handle echo -e.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>> samples/bpf/Makefile | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 1d9be26b4edd..f50ca852c2a8 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -201,7 +201,7 @@ endif
>>
>> # Don't evaluate probes and warnings if we need to run make recursively
>> ifneq ($(src),)
>> -HDR_PROBE := $(shell echo "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \
>> +HDR_PROBE := $(shell printf "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \
>
> printf change is fine, but I'm confused about \# at the beginning of
> the string.
From the NEWS of make 4.3:
* WARNING: Backward-incompatibility!
Number signs (#) appearing inside a macro reference or function invocation
no longer introduce comments and should not be escaped with backslashes:
thus a call such as:
foo := $(shell echo '#')
is legal. Previously the number sign needed to be escaped, for example:
foo := $(shell echo '\#')
Now this latter will resolve to "\#". If you want to write makefiles
portable to both versions, assign the number sign to a variable:
H := \#
foo := $(shell echo '$H')
This was claimed to be fixed in 3.81, but wasn't, for some reason.
To detect this change search for 'nocomment' in the .FEATURES variable.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
On Mon, Sep 16, 2019 at 01:35:21PM -0700, Andrii Nakryiko wrote: >On Mon, Sep 16, 2019 at 4:01 AM Ivan Khoronzhuk ><ivan.khoronzhuk@linaro.org> wrote: >> >> It can overlap with CFLAGS used for libraries built with gcc if >> not now then in next patches. Correct it here for simplicity. >> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> >> --- > >With GCC BPF front-end recently added, we should probably generalize >this to something like BPF_EXTRA_CFLAGS or something like that, >eventually. But for now: > >Acked-by: Andrii Nakryiko <andriin@fb.com> I can replace with BPF_EXTRA_CFLAGS in next v. > >> samples/bpf/Makefile | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile >> index b59e77e2250e..8ecc5d0c2d5b 100644 >> --- a/samples/bpf/Makefile >> +++ b/samples/bpf/Makefile >> @@ -218,10 +218,10 @@ BTF_LLVM_PROBE := $(shell echo "int main() { return 0; }" | \ >> /bin/rm -f ./llvm_btf_verify.o) >> >> ifneq ($(BTF_LLVM_PROBE),) >> - EXTRA_CFLAGS += -g >> + CLANG_EXTRA_CFLAGS += -g >> else >> ifneq ($(and $(BTF_LLC_PROBE),$(BTF_PAHOLE_PROBE),$(BTF_OBJCOPY_PROBE)),) >> - EXTRA_CFLAGS += -g >> + CLANG_EXTRA_CFLAGS += -g >> LLC_FLAGS += -mattr=dwarfris >> DWARF2BTF = y >> endif >> @@ -280,8 +280,8 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h >> # useless for BPF samples. >> $(obj)/%.o: $(src)/%.c >> @echo " CLANG-bpf " $@ >> - $(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \ >> - -I$(srctree)/tools/testing/selftests/bpf/ \ >> + $(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(CLANG_EXTRA_CFLAGS) \ >> + -I$(obj) -I$(srctree)/tools/testing/selftests/bpf/ \ >> -D__KERNEL__ -D__BPF_TRACING__ -Wno-unused-value -Wno-pointer-sign \ >> -D__TARGET_ARCH_$(SRCARCH) -Wno-compare-distinct-pointer-types \ >> -Wno-gnu-variable-sized-type-not-at-end \ >> -- >> 2.17.1 >> -- Regards, Ivan Khoronzhuk
On Mon, Sep 16, 2019 at 2:35 PM Andreas Schwab <schwab@linux-m68k.org> wrote: > > On Sep 16 2019, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > > On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk > > <ivan.khoronzhuk@linaro.org> wrote: > >> > >> echo should be replaced with echo -e to handle '\n' correctly, but > >> instead, replace it with printf as some systems can't handle echo -e. > >> > >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > >> --- > >> samples/bpf/Makefile | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > >> index 1d9be26b4edd..f50ca852c2a8 100644 > >> --- a/samples/bpf/Makefile > >> +++ b/samples/bpf/Makefile > >> @@ -201,7 +201,7 @@ endif > >> > >> # Don't evaluate probes and warnings if we need to run make recursively > >> ifneq ($(src),) > >> -HDR_PROBE := $(shell echo "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \ > >> +HDR_PROBE := $(shell printf "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \ > > > > printf change is fine, but I'm confused about \# at the beginning of > > the string. > > From the NEWS of make 4.3: > > * WARNING: Backward-incompatibility! > Number signs (#) appearing inside a macro reference or function invocation > no longer introduce comments and should not be escaped with backslashes: > thus a call such as: > foo := $(shell echo '#') > is legal. Previously the number sign needed to be escaped, for example: > foo := $(shell echo '\#') > Now this latter will resolve to "\#". If you want to write makefiles > portable to both versions, assign the number sign to a variable: > H := \# > foo := $(shell echo '$H') > This was claimed to be fixed in 3.81, but wasn't, for some reason. > To detect this change search for 'nocomment' in the .FEATURES variable. > > Andreas. Oh, subtle... Thanks for explaining! > > -- > Andreas Schwab, schwab@linux-m68k.org > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > "And now for something completely different."
On Mon, Sep 16, 2019 at 01:44:23PM -0700, Andrii Nakryiko wrote:
>On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>> For arm, -D__LINUX_ARM_ARCH__=X is min version used as instruction
>> set selector and is absolutely required while parsing some parts of
>> headers. It's present in KBUILD_CFLAGS but not in autoconf.h, so let's
>> retrieve it from and add to programs cflags. In another case errors
>> like "SMP is not supported" for armv7 and bunch of other errors are
>> issued resulting to incorrect final object.
>> ---
>> samples/bpf/Makefile | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 8ecc5d0c2d5b..d3c8db3df560 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -185,6 +185,16 @@ HOSTLDLIBS_map_perf_test += -lrt
>> HOSTLDLIBS_test_overhead += -lrt
>> HOSTLDLIBS_xdpsock += -pthread
>>
>> +ifeq ($(ARCH), arm)
>> +# Strip all except -D__LINUX_ARM_ARCH__ option needed to handle linux
>> +# headers when arm instruction set identification is requested.
>> +ARM_ARCH_SELECTOR = $(shell echo "$(KBUILD_CFLAGS) " | \
>> + sed 's/[[:blank:]]/\n/g' | sed '/^-D__LINUX_ARM_ARCH__/!d')
>
>Does the following work exactly like that without shelling out (and
>being arguably simpler)?
>
>ARM_ARCH_SELECTOR = $(filter -D__LINUX_ARM_ARCH__%, $(KBUILD_CFLAGS))
>
>> +
>> +CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
>> +KBUILD_HOSTCFLAGS := $(ARM_ARCH_SELECTOR)
>
>Isn't this clearing out previous value of KBUILD_HOSTCFLAGS? Is that
>intentional, or it was supposed to be += here?
>
>> +endif
>> +
>> # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
>> # make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
>> LLC ?= llc
>> --
>> 2.17.1
>>
Just left from previous version filtering all -D options.
Will update in next v., SELECTOR also.
--
Regards,
Ivan Khoronzhuk
Hello! On 16.09.2019 13:54, Ivan Khoronzhuk wrote: > While compile natively, the hosts cflags and ldflags are equal to ones Compiling. Host's. > used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should > have own, used for target arch. While verification, for arm, arm64 and > x86_64 the following flags were used alsways: > > -Wall > -O2 > -fomit-frame-pointer > -Wmissing-prototypes > -Wstrict-prototypes > > So, add them as they were verified and used before adding > Makefile.target, but anyway limit it only for cross compile options as > for host can be some configurations when another options can be used, > So, for host arch samples left all as is, it allows to avoid potential > option mistmatches for existent environments. Mismatches. > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> [...] MBR, Sergei
On Mon, Sep 16, 2019 at 3:58 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > The makefile.target is added only and will be used in typo: Makefile > sample/bpf/Makefile later in order to switch cross-compiling on CC on -> to > from HOSTCC environment. > > The HOSTCC is supposed to build binaries and tools running on the host > afterwards, in order to simplify build or so, like "fixdep" or else. > In case of cross compiling "fixdep" is executed on host when the rest > samples should run on target arch. In order to build binaries for > target arch with CC and tools running on host with HOSTCC, lets add > Makefile.target for simplicity, having definition and routines similar > to ones, used in script/Makefile.host. This allows later add > cross-compilation to samples/bpf with minimum changes. > > The tprog stands for target programs built with CC. Why tprog? Could we just use prog: hostprog vs prog. > > Makefile.target contains only stuff needed for samples/bpf, potentially > can be reused later and now needed only for unblocking tricky > samples/bpf cross compilation. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- > samples/bpf/Makefile.target | 75 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 75 insertions(+) > create mode 100644 samples/bpf/Makefile.target > > diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target > new file mode 100644 > index 000000000000..fb6de63f7d2f > --- /dev/null > +++ b/samples/bpf/Makefile.target > @@ -0,0 +1,75 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# ========================================================================== > +# Building binaries on the host system > +# Binaries are not used during the compilation of the kernel, and intendent typo: intended > +# to be build for target board, target board can be host ofc. Added to build What's ofc, is it "of course"? > +# binaries to run not on host system. > +# > +# Sample syntax (see Documentation/kbuild/makefiles.rst for reference) > +# tprogs-y := xsk_example > +# Will compile xdpsock_example.c and create an executable named xsk_example You mix references to xsk_example and xdpsock_example, which is very confusing. I'm guessing you meant to use xdpsock_example consistently. > +# > +# tprogs-y := xdpsock > +# xdpsock-objs := xdpsock_1.o xdpsock_2.o > +# Will compile xdpsock_1.c and xdpsock_2.c, and then link the executable > +# xdpsock, based on xdpsock_1.o and xdpsock_2.o > +# > +# Inherited from scripts/Makefile.host "Inspired by" or "Derived from" would be probably more appropriate term :) > +# > +__tprogs := $(sort $(tprogs-y)) > + > +# C code > +# Executables compiled from a single .c file > +tprog-csingle := $(foreach m,$(__tprogs), \ > + $(if $($(m)-objs),,$(m))) > + > +# C executables linked based on several .o files > +tprog-cmulti := $(foreach m,$(__tprogs),\ > + $(if $($(m)-objs),$(m))) > + > +# Object (.o) files compiled from .c files > +tprog-cobjs := $(sort $(foreach m,$(__tprogs),$($(m)-objs))) > + > +tprog-csingle := $(addprefix $(obj)/,$(tprog-csingle)) > +tprog-cmulti := $(addprefix $(obj)/,$(tprog-cmulti)) > +tprog-cobjs := $(addprefix $(obj)/,$(tprog-cobjs)) > + > +##### > +# Handle options to gcc. Support building with separate output directory > + > +_tprogc_flags = $(TPROGS_CFLAGS) \ > + $(TPROGCFLAGS_$(basetarget).o) > + > +# $(objtree)/$(obj) for including generated headers from checkin source files > +ifeq ($(KBUILD_EXTMOD),) > +ifdef building_out_of_srctree > +_tprogc_flags += -I $(objtree)/$(obj) > +endif > +endif > + > +tprogc_flags = -Wp,-MD,$(depfile) $(_tprogc_flags) > + > +# Create executable from a single .c file > +# tprog-csingle -> Executable > +quiet_cmd_tprog-csingle = CC $@ > + cmd_tprog-csingle = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ $< \ > + $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F)) > +$(tprog-csingle): $(obj)/%: $(src)/%.c FORCE > + $(call if_changed_dep,tprog-csingle) > + > +# Link an executable based on list of .o files, all plain c > +# tprog-cmulti -> executable > +quiet_cmd_tprog-cmulti = LD $@ > + cmd_tprog-cmulti = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ \ > + $(addprefix $(obj)/,$($(@F)-objs)) \ > + $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F)) > +$(tprog-cmulti): $(tprog-cobjs) FORCE > + $(call if_changed,tprog-cmulti) > +$(call multi_depend, $(tprog-cmulti), , -objs) > + > +# Create .o file from a single .c file > +# tprog-cobjs -> .o > +quiet_cmd_tprog-cobjs = CC $@ > + cmd_tprog-cobjs = $(CC) $(tprogc_flags) -c -o $@ $< > +$(tprog-cobjs): $(obj)/%.o: $(src)/%.c FORCE > + $(call if_changed_dep,tprog-cobjs) > -- > 2.17.1 > tprogs is quite confusing, but overall looks good to me.
On Mon, Sep 16, 2019 at 3:58 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > Please don't prepend "samples: bpf: makefile:" to patches, "samples/bpf: " is a typical we've used for BPF samples changes. > The main reason for that - HOSTCC and CC have different aims. > HOSTCC is used to build programs running on host, that can > cross-comple target programs with CC. It was tested for arm and arm64 > cross compilation, based on linaro toolchain, but should work for > others. > > So, in order to split cross compilation (CC) with host build (HOSTCC), > lets base samples on Makefile.target. It allows to cross-compile > samples/bpf programs with CC while auxialry tools running on host > built with HOSTCC. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- > samples/bpf/Makefile | 135 ++++++++++++++++++++++--------------------- > 1 file changed, 69 insertions(+), 66 deletions(-) > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > index 9d923546e087..1579cc16a1c2 100644 > --- a/samples/bpf/Makefile > +++ b/samples/bpf/Makefile > @@ -4,55 +4,53 @@ BPF_SAMPLES_PATH ?= $(abspath $(srctree)/$(src)) > TOOLS_PATH := $(BPF_SAMPLES_PATH)/../../tools > > # List of programs to build > -hostprogs-y := test_lru_dist > -hostprogs-y += sock_example > -hostprogs-y += fds_example > -hostprogs-y += sockex1 > -hostprogs-y += sockex2 > -hostprogs-y += sockex3 > -hostprogs-y += tracex1 > -hostprogs-y += tracex2 > -hostprogs-y += tracex3 > -hostprogs-y += tracex4 > -hostprogs-y += tracex5 > -hostprogs-y += tracex6 > -hostprogs-y += tracex7 > -hostprogs-y += test_probe_write_user > -hostprogs-y += trace_output > -hostprogs-y += lathist > -hostprogs-y += offwaketime > -hostprogs-y += spintest > -hostprogs-y += map_perf_test > -hostprogs-y += test_overhead > -hostprogs-y += test_cgrp2_array_pin > -hostprogs-y += test_cgrp2_attach > -hostprogs-y += test_cgrp2_sock > -hostprogs-y += test_cgrp2_sock2 > -hostprogs-y += xdp1 > -hostprogs-y += xdp2 > -hostprogs-y += xdp_router_ipv4 > -hostprogs-y += test_current_task_under_cgroup > -hostprogs-y += trace_event > -hostprogs-y += sampleip > -hostprogs-y += tc_l2_redirect > -hostprogs-y += lwt_len_hist > -hostprogs-y += xdp_tx_iptunnel > -hostprogs-y += test_map_in_map > -hostprogs-y += per_socket_stats_example > -hostprogs-y += xdp_redirect > -hostprogs-y += xdp_redirect_map > -hostprogs-y += xdp_redirect_cpu > -hostprogs-y += xdp_monitor > -hostprogs-y += xdp_rxq_info > -hostprogs-y += syscall_tp > -hostprogs-y += cpustat > -hostprogs-y += xdp_adjust_tail > -hostprogs-y += xdpsock > -hostprogs-y += xdp_fwd > -hostprogs-y += task_fd_query > -hostprogs-y += xdp_sample_pkts > -hostprogs-y += ibumad > -hostprogs-y += hbm > +tprogs-y := test_lru_dist > +tprogs-y += sock_example > +tprogs-y += fds_example > +tprogs-y += sockex1 > +tprogs-y += sockex2 > +tprogs-y += sockex3 > +tprogs-y += tracex1 > +tprogs-y += tracex2 > +tprogs-y += tracex3 > +tprogs-y += tracex4 > +tprogs-y += tracex5 > +tprogs-y += tracex6 > +tprogs-y += tracex7 > +tprogs-y += test_probe_write_user > +tprogs-y += trace_output > +tprogs-y += lathist > +tprogs-y += offwaketime > +tprogs-y += spintest > +tprogs-y += map_perf_test > +tprogs-y += test_overhead > +tprogs-y += test_cgrp2_array_pin > +tprogs-y += test_cgrp2_attach > +tprogs-y += test_cgrp2_sock > +tprogs-y += test_cgrp2_sock2 > +tprogs-y += xdp1 > +tprogs-y += xdp2 > +tprogs-y += xdp_router_ipv4 > +tprogs-y += test_current_task_under_cgroup > +tprogs-y += trace_event > +tprogs-y += sampleip > +tprogs-y += tc_l2_redirect > +tprogs-y += lwt_len_hist > +tprogs-y += xdp_tx_iptunnel > +tprogs-y += test_map_in_map > +tprogs-y += xdp_redirect_map > +tprogs-y += xdp_redirect_cpu > +tprogs-y += xdp_monitor > +tprogs-y += xdp_rxq_info > +tprogs-y += syscall_tp > +tprogs-y += cpustat > +tprogs-y += xdp_adjust_tail > +tprogs-y += xdpsock > +tprogs-y += xdp_fwd > +tprogs-y += task_fd_query > +tprogs-y += xdp_sample_pkts > +tprogs-y += ibumad > +tprogs-y += hbm > > # Libbpf dependencies > LIBBPF = $(TOOLS_PATH)/lib/bpf/libbpf.a > @@ -111,7 +109,7 @@ ibumad-objs := bpf_load.o ibumad_user.o $(TRACE_HELPERS) > hbm-objs := bpf_load.o hbm.o $(CGROUP_HELPERS) > > # Tell kbuild to always build the programs > -always := $(hostprogs-y) > +always := $(tprogs-y) > always += sockex1_kern.o > always += sockex2_kern.o > always += sockex3_kern.o > @@ -170,21 +168,6 @@ always += ibumad_kern.o > always += hbm_out_kern.o > always += hbm_edt_kern.o > > -KBUILD_HOSTCFLAGS += -I$(objtree)/usr/include > -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/bpf/ > -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ > -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include > -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/perf > - > -HOSTCFLAGS_bpf_load.o += -Wno-unused-variable > - > -KBUILD_HOSTLDLIBS += $(LIBBPF) -lelf > -HOSTLDLIBS_tracex4 += -lrt > -HOSTLDLIBS_trace_output += -lrt > -HOSTLDLIBS_map_perf_test += -lrt > -HOSTLDLIBS_test_overhead += -lrt > -HOSTLDLIBS_xdpsock += -pthread > - > ifeq ($(ARCH), arm) > # Strip all except -D__LINUX_ARM_ARCH__ option needed to handle linux > # headers when arm instruction set identification is requested. > @@ -192,9 +175,27 @@ ARM_ARCH_SELECTOR = $(shell echo "$(KBUILD_CFLAGS) " | \ > sed 's/[[:blank:]]/\n/g' | sed '/^-D__LINUX_ARM_ARCH__/!d') > > CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR) > -KBUILD_HOSTCFLAGS := $(ARM_ARCH_SELECTOR) > +TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR) > endif > > +TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS) Please group TPROGS_LDLIBS definition together with the one below, there doesn't seem to be a reason to split them this way. But also, it's kind of weird to use host libraries as cross-compiled libraries as well. Is that intentional? > +TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) Same here, is it right to use HOSTCFLAGS and HOST_EXTRACFLAGS as a base for cross-compiled cflags? > +TPROGS_CFLAGS += -I$(objtree)/usr/include > +TPROGS_CFLAGS += -I$(srctree)/tools/lib/bpf/ > +TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ > +TPROGS_CFLAGS += -I$(srctree)/tools/lib/ > +TPROGS_CFLAGS += -I$(srctree)/tools/include > +TPROGS_CFLAGS += -I$(srctree)/tools/perf > + > +TPROGCFLAGS_bpf_load.o += -Wno-unused-variable > + > +TPROGS_LDLIBS += $(LIBBPF) -lelf > +TPROGLDLIBS_tracex4 += -lrt > +TPROGLDLIBS_trace_output += -lrt > +TPROGLDLIBS_map_perf_test += -lrt > +TPROGLDLIBS_test_overhead += -lrt > +TPROGLDLIBS_xdpsock += -pthread > + > # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline: > # make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang > LLC ?= llc > @@ -285,6 +286,8 @@ $(obj)/hbm_out_kern.o: $(src)/hbm.h $(src)/hbm_kern.h > $(obj)/hbm.o: $(src)/hbm.h > $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h > > +-include $(BPF_SAMPLES_PATH)/Makefile.target > + > # asm/sysreg.h - inline assembly used by it is incompatible with llvm. > # But, there is no easy way to fix it, so just exclude it since it is > # useless for BPF samples. > -- > 2.17.1 >
On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > While compile natively, the hosts cflags and ldflags are equal to ones > used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should > have own, used for target arch. While verification, for arm, arm64 and > x86_64 the following flags were used alsways: > > -Wall > -O2 > -fomit-frame-pointer > -Wmissing-prototypes > -Wstrict-prototypes > > So, add them as they were verified and used before adding > Makefile.target, but anyway limit it only for cross compile options as > for host can be some configurations when another options can be used, > So, for host arch samples left all as is, it allows to avoid potential > option mistmatches for existent environments. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- > samples/bpf/Makefile | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > index 1579cc16a1c2..b5c87a8b8b51 100644 > --- a/samples/bpf/Makefile > +++ b/samples/bpf/Makefile > @@ -178,8 +178,17 @@ CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR) > TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR) > endif > > +ifdef CROSS_COMPILE > +TPROGS_CFLAGS += -Wall > +TPROGS_CFLAGS += -O2 Specifying one arg per line seems like overkill, put them in one line? > +TPROGS_CFLAGS += -fomit-frame-pointer Why this one? > +TPROGS_CFLAGS += -Wmissing-prototypes > +TPROGS_CFLAGS += -Wstrict-prototypes Are these in some way special that we want them in cross-compile mode only? All of those flags seem useful regardless of cross-compilation or not, shouldn't they be common? I'm a bit lost about the intent here... > +else > TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS) > TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) > +endif > + > TPROGS_CFLAGS += -I$(objtree)/usr/include > TPROGS_CFLAGS += -I$(srctree)/tools/lib/bpf/ > TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ > -- > 2.17.1 >
On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> No need in hacking HOSTCC to be cross-compiler any more, so drop
> this trick and use target CC for HDR_PROBE.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
Acked-by: Andrii Nakryiko <andriin@fb.com>
[...]
On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them > correctly to build command, for instance when --sysroot is used or > external libraries are used, like -lelf, wich can be absent in > toolchain. This can be used for samples/bpf cross-compiling allowing > to get elf lib from sysroot. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- > tools/lib/bpf/Makefile | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile > index c6f94cffe06e..bccfa556ef4e 100644 > --- a/tools/lib/bpf/Makefile > +++ b/tools/lib/bpf/Makefile > @@ -94,6 +94,10 @@ else > CFLAGS := -g -Wall > endif > > +ifdef EXTRA_CXXFLAGS > + CXXFLAGS := $(EXTRA_CXXFLAGS) > +endif > + > ifeq ($(feature-libelf-mmap), 1) > override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT > endif > @@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep > $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION) > > $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN) > - $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \ > - -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@ > + $(QUIET_LINK)$(CC) $(LDFLAGS) \ > + --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \ > + -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@ > @ln -sf $(@F) $(OUTPUT)libbpf.so > @ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION) > > @@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN) > $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^ > > $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a > - $(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@ > + $(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@ Instead of doing ifdef EXTRA_CXXFLAGS bit above, you can just include both $(CXXFLAGS) and $(EXTRA_CXXFLAGS), which will do the right thing (and is actually recommended my make documentation way to do this). But actually, there is no need to use C++ compiler here, test_libbpf.cpp can just be plain C. Do you mind renaming it to .c and using C compiler instead? > > $(OUTPUT)libbpf.pc: > $(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \ > -- > 2.17.1 >
On Mon, Sep 16, 2019 at 3:58 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > In order to build libs using C/CXX/LD flags of target arch, > provide them to libbpf make. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- > samples/bpf/Makefile | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > index 18ec22e7b444..133123d4c7d7 100644 > --- a/samples/bpf/Makefile > +++ b/samples/bpf/Makefile > @@ -182,8 +182,6 @@ ifdef CROSS_COMPILE > TPROGS_CFLAGS += -Wall > TPROGS_CFLAGS += -O2 > TPROGS_CFLAGS += -fomit-frame-pointer > -TPROGS_CFLAGS += -Wmissing-prototypes > -TPROGS_CFLAGS += -Wstrict-prototypes > else > TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS) > TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) > @@ -196,6 +194,14 @@ TPROGS_CFLAGS += -I$(srctree)/tools/lib/ > TPROGS_CFLAGS += -I$(srctree)/tools/include > TPROGS_CFLAGS += -I$(srctree)/tools/perf > > +EXTRA_CXXFLAGS := $(TPROGS_CFLAGS) > + > +# options not valid for C++ > +ifdef CROSS_COMPILE > +$(TPROGS_CFLAGS) += -Wmissing-prototypes > +$(TPROGS_CFLAGS) += -Wstrict-prototypes > +endif > + ugh, let's really get rid of dependency on C++ compiler, as suggested for previous patch. > TPROGCFLAGS_bpf_load.o += -Wno-unused-variable > > TPROGS_LDLIBS += $(LIBBPF) -lelf > @@ -257,7 +263,9 @@ clean: > > $(LIBBPF): FORCE > # Fix up variables inherited from Kbuild that tools/ build system won't like > - $(MAKE) -C $(dir $@) RM='rm -rf' LDFLAGS= srctree=$(BPF_SAMPLES_PATH)/../../ O= > + $(MAKE) -C $(dir $@) RM='rm -rf' EXTRA_CFLAGS="$(TPROGS_CFLAGS)" \ > + EXTRA_CXXFLAGS="$(EXTRA_CXXFLAGS)" LDFLAGS=$(TPROGS_LDFLAGS) \ > + srctree=$(BPF_SAMPLES_PATH)/../../ O= > > $(obj)/syscall_nrs.h: $(obj)/syscall_nrs.s FORCE > $(call filechk,offsets,__SYSCALL_NRS_H__) > -- > 2.17.1 >
On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > Basically it only enables that was added by previous couple fixes. > Sysroot contains correct libs installed and its headers ofc. Useful Please, let's not use unnecessary abbreviations/slang. "Of course" is not too long and is a proper English, let's stick to it. > when working with NFC or virtual machine. > > Usage: > > clean (on demand) > make ARCH=arm -C samples/bpf clean > make ARCH=arm -C tools clean > make ARCH=arm clean > > configure and install headers: > > make ARCH=arm defconfig > make ARCH=arm headers_install > > build samples/bpf: > make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- samples/bpf/ \ > SYSROOT="path/to/sysroot" > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- > samples/bpf/Makefile | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > index 133123d4c7d7..57ddf055d6c3 100644 > --- a/samples/bpf/Makefile > +++ b/samples/bpf/Makefile > @@ -194,6 +194,11 @@ TPROGS_CFLAGS += -I$(srctree)/tools/lib/ > TPROGS_CFLAGS += -I$(srctree)/tools/include > TPROGS_CFLAGS += -I$(srctree)/tools/perf > > +ifdef SYSROOT > +TPROGS_CFLAGS += --sysroot=${SYSROOT} > +TPROGS_LDFLAGS := -L${SYSROOT}/usr/lib Please stay consistent: $() instead of ${}? > +endif > + > EXTRA_CXXFLAGS := $(TPROGS_CFLAGS) > > # options not valid for C++ > -- > 2.17.1 >
On Mon, Sep 16, 2019 at 4:02 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > Thanks for these changes, they look good overall. It would be great if someone else could test and validate that cross-compilation works not just in your environment and generated binaries successfully run on target machines, though... [...] > > Ivan Khoronzhuk (14): > samples: bpf: makefile: fix HDR_PROBE "echo" > samples: bpf: makefile: fix cookie_uid_helper_example obj build > samples: bpf: makefile: use --target from cross-compile > samples: bpf: use own EXTRA_CFLAGS for clang commands > samples: bpf: makefile: use __LINUX_ARM_ARCH__ selector for arm > samples: bpf: makefile: drop unnecessarily inclusion for bpf_load > samples: bpf: add makefile.target for separate CC target build > samples: bpf: makefile: base target programs rules on Makefile.target > samples: bpf: makefile: use own flags but not host when cross compile > samples: bpf: makefile: use target CC environment for HDR_PROBE > libbpf: makefile: add C/CXX/LDFLAGS to libbpf.so and test_libpf > targets > samples: bpf: makefile: provide C/CXX/LD flags to libbpf > samples: bpf: makefile: add sysroot support > samples: bpf: README: add preparation steps and sysroot info > Prefixes like "samples: bpf: makefile: " are very verbose without adding much value. We've been converging to essentially this set of prefixes: - "libbpf:" for libbpf changes - "bpftool:" for bpftool changes - "selftests/bpf:" for bpf selftests - "samples/bpf:" for bpf samples There is no need to prefix with "makefile: " either. Please update your patch subjects in the next version. Thanks! > samples/bpf/Makefile | 179 +++++++++++++++++++++--------------- > samples/bpf/Makefile.target | 75 +++++++++++++++ > samples/bpf/README.rst | 41 ++++++++- > tools/lib/bpf/Makefile | 11 ++- > 4 files changed, 225 insertions(+), 81 deletions(-) > create mode 100644 samples/bpf/Makefile.target > > -- > 2.17.1 >
On Tue, Sep 17, 2019 at 04:19:40PM -0700, Andrii Nakryiko wrote: >On Mon, Sep 16, 2019 at 3:58 AM Ivan Khoronzhuk ><ivan.khoronzhuk@linaro.org> wrote: >> >> The makefile.target is added only and will be used in > >typo: Makefile > >> sample/bpf/Makefile later in order to switch cross-compiling on CC > >on -> to > >> from HOSTCC environment. >> >> The HOSTCC is supposed to build binaries and tools running on the host >> afterwards, in order to simplify build or so, like "fixdep" or else. >> In case of cross compiling "fixdep" is executed on host when the rest >> samples should run on target arch. In order to build binaries for >> target arch with CC and tools running on host with HOSTCC, lets add >> Makefile.target for simplicity, having definition and routines similar >> to ones, used in script/Makefile.host. This allows later add >> cross-compilation to samples/bpf with minimum changes. >> >> The tprog stands for target programs built with CC. > >Why tprog? Could we just use prog: hostprog vs prog. Prev. version was with prog, but Yonghong Song found it ambiguous. As prog can be bpf also. So, decision was made to follow logic: * target prog - non bpf progs * bpf prog = bpf prog, that can be later smth similar, providing build options for each bpf object separately. Details here: https://lkml.org/lkml/2019/9/13/1037 > >> >> Makefile.target contains only stuff needed for samples/bpf, potentially >> can be reused later and now needed only for unblocking tricky >> samples/bpf cross compilation. >> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> >> --- >> samples/bpf/Makefile.target | 75 +++++++++++++++++++++++++++++++++++++ >> 1 file changed, 75 insertions(+) >> create mode 100644 samples/bpf/Makefile.target >> >> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target >> new file mode 100644 >> index 000000000000..fb6de63f7d2f >> --- /dev/null >> +++ b/samples/bpf/Makefile.target >> @@ -0,0 +1,75 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +# ========================================================================== >> +# Building binaries on the host system >> +# Binaries are not used during the compilation of the kernel, and intendent > >typo: intended > >> +# to be build for target board, target board can be host ofc. Added to build > >What's ofc, is it "of course"? yes, ofc ) > >> +# binaries to run not on host system. >> +# >> +# Sample syntax (see Documentation/kbuild/makefiles.rst for reference) >> +# tprogs-y := xsk_example >> +# Will compile xdpsock_example.c and create an executable named xsk_example > >You mix references to xsk_example and xdpsock_example, which is very >confusing. I'm guessing you meant to use xdpsock_example consistently. Oh, yes. Thanks. > >> +# >> +# tprogs-y := xdpsock >> +# xdpsock-objs := xdpsock_1.o xdpsock_2.o >> +# Will compile xdpsock_1.c and xdpsock_2.c, and then link the executable >> +# xdpsock, based on xdpsock_1.o and xdpsock_2.o >> +# >> +# Inherited from scripts/Makefile.host > >"Inspired by" or "Derived from" would be probably more appropriate term :) I will replace with "Derived from", looks better. > >> +# >> +__tprogs := $(sort $(tprogs-y)) >> + >> +# C code >> +# Executables compiled from a single .c file >> +tprog-csingle := $(foreach m,$(__tprogs), \ >> + $(if $($(m)-objs),,$(m))) >> + >> +# C executables linked based on several .o files >> +tprog-cmulti := $(foreach m,$(__tprogs),\ >> + $(if $($(m)-objs),$(m))) >> + >> +# Object (.o) files compiled from .c files >> +tprog-cobjs := $(sort $(foreach m,$(__tprogs),$($(m)-objs))) >> + >> +tprog-csingle := $(addprefix $(obj)/,$(tprog-csingle)) >> +tprog-cmulti := $(addprefix $(obj)/,$(tprog-cmulti)) >> +tprog-cobjs := $(addprefix $(obj)/,$(tprog-cobjs)) >> + >> +##### >> +# Handle options to gcc. Support building with separate output directory >> + >> +_tprogc_flags = $(TPROGS_CFLAGS) \ >> + $(TPROGCFLAGS_$(basetarget).o) >> + >> +# $(objtree)/$(obj) for including generated headers from checkin source files >> +ifeq ($(KBUILD_EXTMOD),) >> +ifdef building_out_of_srctree >> +_tprogc_flags += -I $(objtree)/$(obj) >> +endif >> +endif >> + >> +tprogc_flags = -Wp,-MD,$(depfile) $(_tprogc_flags) >> + >> +# Create executable from a single .c file >> +# tprog-csingle -> Executable >> +quiet_cmd_tprog-csingle = CC $@ >> + cmd_tprog-csingle = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ $< \ >> + $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F)) >> +$(tprog-csingle): $(obj)/%: $(src)/%.c FORCE >> + $(call if_changed_dep,tprog-csingle) >> + >> +# Link an executable based on list of .o files, all plain c >> +# tprog-cmulti -> executable >> +quiet_cmd_tprog-cmulti = LD $@ >> + cmd_tprog-cmulti = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ \ >> + $(addprefix $(obj)/,$($(@F)-objs)) \ >> + $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F)) >> +$(tprog-cmulti): $(tprog-cobjs) FORCE >> + $(call if_changed,tprog-cmulti) >> +$(call multi_depend, $(tprog-cmulti), , -objs) >> + >> +# Create .o file from a single .c file >> +# tprog-cobjs -> .o >> +quiet_cmd_tprog-cobjs = CC $@ >> + cmd_tprog-cobjs = $(CC) $(tprogc_flags) -c -o $@ $< >> +$(tprog-cobjs): $(obj)/%.o: $(src)/%.c FORCE >> + $(call if_changed_dep,tprog-cobjs) >> -- >> 2.17.1 >> > >tprogs is quite confusing, but overall looks good to me. I tend to leave it as tprogs, unless it's going to be progs and agreed with Yonghong. It follows logic: - tprogs for bins - bpfprogs or bojs or bprogs (could be) for bpf obj -- Regards, Ivan Khoronzhuk
On Tue, Sep 17, 2019 at 04:28:01PM -0700, Andrii Nakryiko wrote: >On Mon, Sep 16, 2019 at 3:58 AM Ivan Khoronzhuk ><ivan.khoronzhuk@linaro.org> wrote: >> > >Please don't prepend "samples: bpf: makefile:" to patches, >"samples/bpf: " is a typical we've used for BPF samples changes. Ok. > > >> The main reason for that - HOSTCC and CC have different aims. >> HOSTCC is used to build programs running on host, that can >> cross-comple target programs with CC. It was tested for arm and arm64 >> cross compilation, based on linaro toolchain, but should work for >> others. >> >> So, in order to split cross compilation (CC) with host build (HOSTCC), >> lets base samples on Makefile.target. It allows to cross-compile >> samples/bpf programs with CC while auxialry tools running on host >> built with HOSTCC. >> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> >> --- >> samples/bpf/Makefile | 135 ++++++++++++++++++++++--------------------- >> 1 file changed, 69 insertions(+), 66 deletions(-) >> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile >> index 9d923546e087..1579cc16a1c2 100644 >> --- a/samples/bpf/Makefile >> +++ b/samples/bpf/Makefile >> @@ -4,55 +4,53 @@ BPF_SAMPLES_PATH ?= $(abspath $(srctree)/$(src)) >> TOOLS_PATH := $(BPF_SAMPLES_PATH)/../../tools >> >> # List of programs to build >> -hostprogs-y := test_lru_dist [...] >> -KBUILD_HOSTCFLAGS := $(ARM_ARCH_SELECTOR) >> +TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR) >> endif >> >> +TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS) > >Please group TPROGS_LDLIBS definition together with the one below, >there doesn't seem to be a reason to split them this way. No. It's used in Makefile.target and should be here, following hostprog logic. > >But also, it's kind of weird to use host libraries as cross-compiled >libraries as well. Is that intentional? No cross-compile split yet. This patch replace only KBUILD on TPROGS. It's done in following patches. > >> +TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) > >Same here, is it right to use HOSTCFLAGS and HOST_EXTRACFLAGS as a >base for cross-compiled cflags? same [...] -- Regards, Ivan Khoronzhuk
On Tue, Sep 17, 2019 at 04:42:07PM -0700, Andrii Nakryiko wrote: >On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk ><ivan.khoronzhuk@linaro.org> wrote: >> >> While compile natively, the hosts cflags and ldflags are equal to ones >> used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should >> have own, used for target arch. While verification, for arm, arm64 and >> x86_64 the following flags were used alsways: >> >> -Wall >> -O2 >> -fomit-frame-pointer >> -Wmissing-prototypes >> -Wstrict-prototypes >> >> So, add them as they were verified and used before adding >> Makefile.target, but anyway limit it only for cross compile options as >> for host can be some configurations when another options can be used, >> So, for host arch samples left all as is, it allows to avoid potential >> option mistmatches for existent environments. >> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> >> --- >> samples/bpf/Makefile | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile >> index 1579cc16a1c2..b5c87a8b8b51 100644 >> --- a/samples/bpf/Makefile >> +++ b/samples/bpf/Makefile >> @@ -178,8 +178,17 @@ CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR) >> TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR) >> endif >> >> +ifdef CROSS_COMPILE >> +TPROGS_CFLAGS += -Wall >> +TPROGS_CFLAGS += -O2 > >Specifying one arg per line seems like overkill, put them in one line? Will combine. > >> +TPROGS_CFLAGS += -fomit-frame-pointer > >Why this one? I've explained in commit msg. The logic is to have as much as close options to have smiliar binaries. As those options are used before for hosts and kinda cross builds - better follow same way. > >> +TPROGS_CFLAGS += -Wmissing-prototypes >> +TPROGS_CFLAGS += -Wstrict-prototypes > >Are these in some way special that we want them in cross-compile mode only? > >All of those flags seem useful regardless of cross-compilation or not, >shouldn't they be common? I'm a bit lost about the intent here... They are common but split is needed to expose it at least. Also host for different arches can have some own opts already used that shouldn't be present for cross, better not mix it for safety. > >> +else >> TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS) >> TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) >> +endif >> + >> TPROGS_CFLAGS += -I$(objtree)/usr/include >> TPROGS_CFLAGS += -I$(srctree)/tools/lib/bpf/ >> TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ >> -- >> 2.17.1 >> -- Regards, Ivan Khoronzhuk
On Tue, Sep 17, 2019 at 10:19:22PM -0700, Andrii Nakryiko wrote: >On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk ><ivan.khoronzhuk@linaro.org> wrote: >> >> In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them >> correctly to build command, for instance when --sysroot is used or >> external libraries are used, like -lelf, wich can be absent in >> toolchain. This can be used for samples/bpf cross-compiling allowing >> to get elf lib from sysroot. >> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> >> --- >> tools/lib/bpf/Makefile | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile >> index c6f94cffe06e..bccfa556ef4e 100644 >> --- a/tools/lib/bpf/Makefile >> +++ b/tools/lib/bpf/Makefile >> @@ -94,6 +94,10 @@ else >> CFLAGS := -g -Wall >> endif >> >> +ifdef EXTRA_CXXFLAGS >> + CXXFLAGS := $(EXTRA_CXXFLAGS) >> +endif >> + >> ifeq ($(feature-libelf-mmap), 1) >> override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT >> endif >> @@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep >> $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION) >> >> $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN) >> - $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \ >> - -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@ >> + $(QUIET_LINK)$(CC) $(LDFLAGS) \ >> + --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \ >> + -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@ >> @ln -sf $(@F) $(OUTPUT)libbpf.so >> @ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION) >> >> @@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN) >> $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^ >> >> $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a >> - $(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@ >> + $(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@ > >Instead of doing ifdef EXTRA_CXXFLAGS bit above, you can just include >both $(CXXFLAGS) and $(EXTRA_CXXFLAGS), which will do the right thing >(and is actually recommended my make documentation way to do this). It's good practice to follow existent style, I've done similar way as for CFLAGS + EXTRACFLAGS here, didn't want to verify it can impact on smth else. And my goal is not to correct everything but embed my functionality, series tool large w/o it. > >But actually, there is no need to use C++ compiler here, >test_libbpf.cpp can just be plain C. Do you mind renaming it to .c and >using C compiler instead? Seems like, will try in next v. > >> >> $(OUTPUT)libbpf.pc: >> $(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \ >> -- >> 2.17.1 >> -- Regards, Ivan Khoronzhuk
On Tue, Sep 17, 2019 at 10:23:57PM -0700, Andrii Nakryiko wrote: >On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk ><ivan.khoronzhuk@linaro.org> wrote: >> >> Basically it only enables that was added by previous couple fixes. >> Sysroot contains correct libs installed and its headers ofc. Useful > >Please, let's not use unnecessary abbreviations/slang. "Of course" is >not too long and is a proper English, let's stick to it. > >> when working with NFC or virtual machine. >> >> Usage: >> >> clean (on demand) >> make ARCH=arm -C samples/bpf clean >> make ARCH=arm -C tools clean >> make ARCH=arm clean >> >> configure and install headers: >> >> make ARCH=arm defconfig >> make ARCH=arm headers_install >> >> build samples/bpf: >> make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- samples/bpf/ \ >> SYSROOT="path/to/sysroot" >> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> >> --- >> samples/bpf/Makefile | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile >> index 133123d4c7d7..57ddf055d6c3 100644 >> --- a/samples/bpf/Makefile >> +++ b/samples/bpf/Makefile >> @@ -194,6 +194,11 @@ TPROGS_CFLAGS += -I$(srctree)/tools/lib/ >> TPROGS_CFLAGS += -I$(srctree)/tools/include >> TPROGS_CFLAGS += -I$(srctree)/tools/perf >> >> +ifdef SYSROOT >> +TPROGS_CFLAGS += --sysroot=${SYSROOT} >> +TPROGS_LDFLAGS := -L${SYSROOT}/usr/lib > >Please stay consistent: $() instead of ${}? Yes, thanks. > >> +endif >> + >> EXTRA_CXXFLAGS := $(TPROGS_CFLAGS) >> >> # options not valid for C++ >> -- >> 2.17.1 >> -- Regards, Ivan Khoronzhuk
On Wed, Sep 18, 2019 at 3:12 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > On Tue, Sep 17, 2019 at 04:19:40PM -0700, Andrii Nakryiko wrote: > >On Mon, Sep 16, 2019 at 3:58 AM Ivan Khoronzhuk > ><ivan.khoronzhuk@linaro.org> wrote: > >> > >> The makefile.target is added only and will be used in > > > >typo: Makefile > > > >> sample/bpf/Makefile later in order to switch cross-compiling on CC > > > >on -> to > > > >> from HOSTCC environment. > >> > >> The HOSTCC is supposed to build binaries and tools running on the host > >> afterwards, in order to simplify build or so, like "fixdep" or else. > >> In case of cross compiling "fixdep" is executed on host when the rest > >> samples should run on target arch. In order to build binaries for > >> target arch with CC and tools running on host with HOSTCC, lets add > >> Makefile.target for simplicity, having definition and routines similar > >> to ones, used in script/Makefile.host. This allows later add > >> cross-compilation to samples/bpf with minimum changes. > >> > >> The tprog stands for target programs built with CC. > > > >Why tprog? Could we just use prog: hostprog vs prog. > Prev. version was with prog, but Yonghong Song found it ambiguous. > As prog can be bpf also. So, decision was made to follow logic: > * target prog - non bpf progs > * bpf prog = bpf prog, that can be later smth similar, providing build options > for each bpf object separately. > Well, I'm not going to insist, but BPF program is a C function, compiled BPF .o file is BPF object, so I don't think there is going to be too much confusion to have progs and hostprogs in Makefile. But I'm fine with tprog. > Details here: > https://lkml.org/lkml/2019/9/13/1037 > > > > >> > >> Makefile.target contains only stuff needed for samples/bpf, potentially > >> can be reused later and now needed only for unblocking tricky > >> samples/bpf cross compilation. > >> > >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > >> --- > >> samples/bpf/Makefile.target | 75 +++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 75 insertions(+) > >> create mode 100644 samples/bpf/Makefile.target > >> > >> diff --git a/samples/bpf/Makefile.target b/samples/bpf/Makefile.target > >> new file mode 100644 > >> index 000000000000..fb6de63f7d2f > >> --- /dev/null > >> +++ b/samples/bpf/Makefile.target > >> @@ -0,0 +1,75 @@ > >> +# SPDX-License-Identifier: GPL-2.0 > >> +# ========================================================================== > >> +# Building binaries on the host system > >> +# Binaries are not used during the compilation of the kernel, and intendent > > > >typo: intended > > > >> +# to be build for target board, target board can be host ofc. Added to build > > > >What's ofc, is it "of course"? > yes, ofc ) Alright, let's not try to save 5 letters, it's quite confusing. > > > > >> +# binaries to run not on host system. > >> +# > >> +# Sample syntax (see Documentation/kbuild/makefiles.rst for reference) > >> +# tprogs-y := xsk_example > >> +# Will compile xdpsock_example.c and create an executable named xsk_example > > > >You mix references to xsk_example and xdpsock_example, which is very > >confusing. I'm guessing you meant to use xdpsock_example consistently. > Oh, yes. Thanks. > > > > >> +# > >> +# tprogs-y := xdpsock > >> +# xdpsock-objs := xdpsock_1.o xdpsock_2.o > >> +# Will compile xdpsock_1.c and xdpsock_2.c, and then link the executable > >> +# xdpsock, based on xdpsock_1.o and xdpsock_2.o > >> +# > >> +# Inherited from scripts/Makefile.host > > > >"Inspired by" or "Derived from" would be probably more appropriate term :) > I will replace with "Derived from", looks better. > sounds good > > > >> +# > >> +__tprogs := $(sort $(tprogs-y)) > >> + > >> +# C code > >> +# Executables compiled from a single .c file > >> +tprog-csingle := $(foreach m,$(__tprogs), \ > >> + $(if $($(m)-objs),,$(m))) > >> + > >> +# C executables linked based on several .o files > >> +tprog-cmulti := $(foreach m,$(__tprogs),\ > >> + $(if $($(m)-objs),$(m))) > >> + > >> +# Object (.o) files compiled from .c files > >> +tprog-cobjs := $(sort $(foreach m,$(__tprogs),$($(m)-objs))) > >> + > >> +tprog-csingle := $(addprefix $(obj)/,$(tprog-csingle)) > >> +tprog-cmulti := $(addprefix $(obj)/,$(tprog-cmulti)) > >> +tprog-cobjs := $(addprefix $(obj)/,$(tprog-cobjs)) > >> + > >> +##### > >> +# Handle options to gcc. Support building with separate output directory > >> + > >> +_tprogc_flags = $(TPROGS_CFLAGS) \ > >> + $(TPROGCFLAGS_$(basetarget).o) > >> + > >> +# $(objtree)/$(obj) for including generated headers from checkin source files > >> +ifeq ($(KBUILD_EXTMOD),) > >> +ifdef building_out_of_srctree > >> +_tprogc_flags += -I $(objtree)/$(obj) > >> +endif > >> +endif > >> + > >> +tprogc_flags = -Wp,-MD,$(depfile) $(_tprogc_flags) > >> + > >> +# Create executable from a single .c file > >> +# tprog-csingle -> Executable > >> +quiet_cmd_tprog-csingle = CC $@ > >> + cmd_tprog-csingle = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ $< \ > >> + $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F)) > >> +$(tprog-csingle): $(obj)/%: $(src)/%.c FORCE > >> + $(call if_changed_dep,tprog-csingle) > >> + > >> +# Link an executable based on list of .o files, all plain c > >> +# tprog-cmulti -> executable > >> +quiet_cmd_tprog-cmulti = LD $@ > >> + cmd_tprog-cmulti = $(CC) $(tprogc_flags) $(TPROGS_LDFLAGS) -o $@ \ > >> + $(addprefix $(obj)/,$($(@F)-objs)) \ > >> + $(TPROGS_LDLIBS) $(TPROGLDLIBS_$(@F)) > >> +$(tprog-cmulti): $(tprog-cobjs) FORCE > >> + $(call if_changed,tprog-cmulti) > >> +$(call multi_depend, $(tprog-cmulti), , -objs) > >> + > >> +# Create .o file from a single .c file > >> +# tprog-cobjs -> .o > >> +quiet_cmd_tprog-cobjs = CC $@ > >> + cmd_tprog-cobjs = $(CC) $(tprogc_flags) -c -o $@ $< > >> +$(tprog-cobjs): $(obj)/%.o: $(src)/%.c FORCE > >> + $(call if_changed_dep,tprog-cobjs) > >> -- > >> 2.17.1 > >> > > > >tprogs is quite confusing, but overall looks good to me. > I tend to leave it as tprogs, unless it's going to be progs and agreed with > Yonghong. > > It follows logic: > - tprogs for bins > - bpfprogs or bojs or bprogs (could be) for bpf obj as mentioned above, we never build "BPF programs", they are always part of BPF objects. But as I mentioned, I'm fine with sticking to tprog. > > -- > Regards, > Ivan Khoronzhuk
On Wed, Sep 18, 2019 at 3:35 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > On Tue, Sep 17, 2019 at 04:42:07PM -0700, Andrii Nakryiko wrote: > >On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk > ><ivan.khoronzhuk@linaro.org> wrote: > >> > >> While compile natively, the hosts cflags and ldflags are equal to ones > >> used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should > >> have own, used for target arch. While verification, for arm, arm64 and > >> x86_64 the following flags were used alsways: > >> > >> -Wall > >> -O2 > >> -fomit-frame-pointer > >> -Wmissing-prototypes > >> -Wstrict-prototypes > >> > >> So, add them as they were verified and used before adding > >> Makefile.target, but anyway limit it only for cross compile options as > >> for host can be some configurations when another options can be used, > >> So, for host arch samples left all as is, it allows to avoid potential > >> option mistmatches for existent environments. > >> > >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > >> --- > >> samples/bpf/Makefile | 9 +++++++++ > >> 1 file changed, 9 insertions(+) > >> > >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > >> index 1579cc16a1c2..b5c87a8b8b51 100644 > >> --- a/samples/bpf/Makefile > >> +++ b/samples/bpf/Makefile > >> @@ -178,8 +178,17 @@ CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR) > >> TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR) > >> endif > >> > >> +ifdef CROSS_COMPILE > >> +TPROGS_CFLAGS += -Wall > >> +TPROGS_CFLAGS += -O2 > > > >Specifying one arg per line seems like overkill, put them in one line? > Will combine. > > > > >> +TPROGS_CFLAGS += -fomit-frame-pointer > > > >Why this one? > I've explained in commit msg. The logic is to have as much as close options > to have smiliar binaries. As those options are used before for hosts and kinda > cross builds - better follow same way. I'm just asking why omit frame pointers and make it harder to do stuff like profiling? What performance benefits are we seeking for in BPF samples? > > > > >> +TPROGS_CFLAGS += -Wmissing-prototypes > >> +TPROGS_CFLAGS += -Wstrict-prototypes > > > >Are these in some way special that we want them in cross-compile mode only? > > > >All of those flags seem useful regardless of cross-compilation or not, > >shouldn't they be common? I'm a bit lost about the intent here... > They are common but split is needed to expose it at least. Also host for > different arches can have some own opts already used that shouldn't be present > for cross, better not mix it for safety. We want -Wmissing-prototypes and -Wstrict-prototypes for cross-compile and non-cross-compile cases, right? So let's specify them as common set of options, instead of relying on KBUILD_HOSTCFLAGS or HOST_EXTRACFLAGS to have them. Otherwise we'll be getting extra warnings for just cross-compile case, which is not good. If you are worrying about having duplicate -W flags, seems like it's handled by GCC already, so shouldn't be a problem. > > > > >> +else > >> TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS) > >> TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) > >> +endif > >> + > >> TPROGS_CFLAGS += -I$(objtree)/usr/include > >> TPROGS_CFLAGS += -I$(srctree)/tools/lib/bpf/ > >> TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ > >> -- > >> 2.17.1 > >> > > -- > Regards, > Ivan Khoronzhuk
On Wed, Sep 18, 2019 at 4:05 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > On Tue, Sep 17, 2019 at 10:19:22PM -0700, Andrii Nakryiko wrote: > >On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk > ><ivan.khoronzhuk@linaro.org> wrote: > >> > >> In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them > >> correctly to build command, for instance when --sysroot is used or > >> external libraries are used, like -lelf, wich can be absent in > >> toolchain. This can be used for samples/bpf cross-compiling allowing > >> to get elf lib from sysroot. > >> > >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > >> --- > >> tools/lib/bpf/Makefile | 11 ++++++++--- > >> 1 file changed, 8 insertions(+), 3 deletions(-) > >> > >> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile > >> index c6f94cffe06e..bccfa556ef4e 100644 > >> --- a/tools/lib/bpf/Makefile > >> +++ b/tools/lib/bpf/Makefile > >> @@ -94,6 +94,10 @@ else > >> CFLAGS := -g -Wall > >> endif > >> > >> +ifdef EXTRA_CXXFLAGS > >> + CXXFLAGS := $(EXTRA_CXXFLAGS) > >> +endif > >> + > >> ifeq ($(feature-libelf-mmap), 1) > >> override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT > >> endif > >> @@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep > >> $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION) > >> > >> $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN) > >> - $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \ > >> - -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@ > >> + $(QUIET_LINK)$(CC) $(LDFLAGS) \ > >> + --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \ > >> + -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@ > >> @ln -sf $(@F) $(OUTPUT)libbpf.so > >> @ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION) > >> > >> @@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN) > >> $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^ > >> > >> $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a > >> - $(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@ > >> + $(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@ > > > >Instead of doing ifdef EXTRA_CXXFLAGS bit above, you can just include > >both $(CXXFLAGS) and $(EXTRA_CXXFLAGS), which will do the right thing > >(and is actually recommended my make documentation way to do this). > It's good practice to follow existent style, I've done similar way as for > CFLAGS + EXTRACFLAGS here, didn't want to verify it can impact on > smth else. And my goal is not to correct everything but embed my > functionality, series tool large w/o it. Alright, we'll have to eventually clean up this Makefile. What we do with EXTRA_CFLAGS is not exactly correct, as in this Makefile EXTRA_CFLAGS are overriding CFLAGS, instead of extending them, which doesn't seem correct to me. BTW, bpftool does += instead of :=. All this is avoided by just keeping CFLAGS and EXTRA_CFLAGS separate and specifying both of them in $(CC)/$(CLANG) invocations. But feel free to ignore this for now. > > > > >But actually, there is no need to use C++ compiler here, > >test_libbpf.cpp can just be plain C. Do you mind renaming it to .c and > >using C compiler instead? > Seems like, will try in next v. Thanks! > > > > >> > >> $(OUTPUT)libbpf.pc: > >> $(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \ > >> -- > >> 2.17.1 > >> > > -- > Regards, > Ivan Khoronzhuk
On Wed, Sep 18, 2019 at 02:29:53PM -0700, Andrii Nakryiko wrote:
>On Wed, Sep 18, 2019 at 3:35 AM Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>> On Tue, Sep 17, 2019 at 04:42:07PM -0700, Andrii Nakryiko wrote:
>> >On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk
>> ><ivan.khoronzhuk@linaro.org> wrote:
>> >>
>> >> While compile natively, the hosts cflags and ldflags are equal to ones
>> >> used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should
>> >> have own, used for target arch. While verification, for arm, arm64 and
>> >> x86_64 the following flags were used alsways:
>> >>
>> >> -Wall
>> >> -O2
>> >> -fomit-frame-pointer
>> >> -Wmissing-prototypes
>> >> -Wstrict-prototypes
>> >>
>> >> So, add them as they were verified and used before adding
>> >> Makefile.target, but anyway limit it only for cross compile options as
>> >> for host can be some configurations when another options can be used,
>> >> So, for host arch samples left all as is, it allows to avoid potential
>> >> option mistmatches for existent environments.
>> >>
>> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> >> ---
>> >> samples/bpf/Makefile | 9 +++++++++
>> >> 1 file changed, 9 insertions(+)
>> >>
>> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> >> index 1579cc16a1c2..b5c87a8b8b51 100644
>> >> --- a/samples/bpf/Makefile
>> >> +++ b/samples/bpf/Makefile
>> >> @@ -178,8 +178,17 @@ CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
>> >> TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR)
>> >> endif
>> >>
>> >> +ifdef CROSS_COMPILE
>> >> +TPROGS_CFLAGS += -Wall
>> >> +TPROGS_CFLAGS += -O2
>> >
>> >Specifying one arg per line seems like overkill, put them in one line?
>> Will combine.
>>
>> >
>> >> +TPROGS_CFLAGS += -fomit-frame-pointer
>> >
>> >Why this one?
>> I've explained in commit msg. The logic is to have as much as close options
>> to have smiliar binaries. As those options are used before for hosts and kinda
>> cross builds - better follow same way.
>
>I'm just asking why omit frame pointers and make it harder to do stuff
>like profiling? What performance benefits are we seeking for in BPF
>samples?
>
>>
>> >
>> >> +TPROGS_CFLAGS += -Wmissing-prototypes
>> >> +TPROGS_CFLAGS += -Wstrict-prototypes
>> >
>> >Are these in some way special that we want them in cross-compile mode only?
>> >
>> >All of those flags seem useful regardless of cross-compilation or not,
>> >shouldn't they be common? I'm a bit lost about the intent here...
>> They are common but split is needed to expose it at least. Also host for
>> different arches can have some own opts already used that shouldn't be present
>> for cross, better not mix it for safety.
>
>We want -Wmissing-prototypes and -Wstrict-prototypes for cross-compile
>and non-cross-compile cases, right? So let's specify them as common
>set of options, instead of relying on KBUILD_HOSTCFLAGS or
>HOST_EXTRACFLAGS to have them. Otherwise we'll be getting extra
>warnings for just cross-compile case, which is not good. If you are
>worrying about having duplicate -W flags, seems like it's handled by
>GCC already, so shouldn't be a problem.
Ok, lets drop omit-frame-pointer.
But then, lets do more radical step and drop
KBUILD_HOSTCFLAGS & HOST_EXTRACFLAG in this patch:
-ifdef CROSS_COMPILE
+TPROGS_CFLAGS += -Wall -O2
+TPROGS_CFLAGS += -Wmissing-prototypes
+TPROGS_CFLAGS += -Wstrict-prototypes
-else
-TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS)
-TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS)
-endif
At least it allows to use same options always for both, native and cross.
I verified on native x86_64, arm64 and arm and cross for arm and arm64,
but should work for others, at least it can be tuned explicitly and
no need to depend on KBUILD and use "cross" fork here.
--
Regards,
Ivan Khoronzhuk
On Thu, Sep 19, 2019 at 7:18 AM Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > On Wed, Sep 18, 2019 at 02:29:53PM -0700, Andrii Nakryiko wrote: > >On Wed, Sep 18, 2019 at 3:35 AM Ivan Khoronzhuk > ><ivan.khoronzhuk@linaro.org> wrote: > >> > >> On Tue, Sep 17, 2019 at 04:42:07PM -0700, Andrii Nakryiko wrote: > >> >On Mon, Sep 16, 2019 at 3:59 AM Ivan Khoronzhuk > >> ><ivan.khoronzhuk@linaro.org> wrote: > >> >> > >> >> While compile natively, the hosts cflags and ldflags are equal to ones > >> >> used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should > >> >> have own, used for target arch. While verification, for arm, arm64 and > >> >> x86_64 the following flags were used alsways: > >> >> > >> >> -Wall > >> >> -O2 > >> >> -fomit-frame-pointer > >> >> -Wmissing-prototypes > >> >> -Wstrict-prototypes > >> >> > >> >> So, add them as they were verified and used before adding > >> >> Makefile.target, but anyway limit it only for cross compile options as > >> >> for host can be some configurations when another options can be used, > >> >> So, for host arch samples left all as is, it allows to avoid potential > >> >> option mistmatches for existent environments. > >> >> > >> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > >> >> --- > >> >> samples/bpf/Makefile | 9 +++++++++ > >> >> 1 file changed, 9 insertions(+) > >> >> > >> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > >> >> index 1579cc16a1c2..b5c87a8b8b51 100644 > >> >> --- a/samples/bpf/Makefile > >> >> +++ b/samples/bpf/Makefile > >> >> @@ -178,8 +178,17 @@ CLANG_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR) > >> >> TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR) > >> >> endif > >> >> > >> >> +ifdef CROSS_COMPILE > >> >> +TPROGS_CFLAGS += -Wall > >> >> +TPROGS_CFLAGS += -O2 > >> > > >> >Specifying one arg per line seems like overkill, put them in one line? > >> Will combine. > >> > >> > > >> >> +TPROGS_CFLAGS += -fomit-frame-pointer > >> > > >> >Why this one? > >> I've explained in commit msg. The logic is to have as much as close options > >> to have smiliar binaries. As those options are used before for hosts and kinda > >> cross builds - better follow same way. > > > >I'm just asking why omit frame pointers and make it harder to do stuff > >like profiling? What performance benefits are we seeking for in BPF > >samples? > > > >> > >> > > >> >> +TPROGS_CFLAGS += -Wmissing-prototypes > >> >> +TPROGS_CFLAGS += -Wstrict-prototypes > >> > > >> >Are these in some way special that we want them in cross-compile mode only? > >> > > >> >All of those flags seem useful regardless of cross-compilation or not, > >> >shouldn't they be common? I'm a bit lost about the intent here... > >> They are common but split is needed to expose it at least. Also host for > >> different arches can have some own opts already used that shouldn't be present > >> for cross, better not mix it for safety. > > > >We want -Wmissing-prototypes and -Wstrict-prototypes for cross-compile > >and non-cross-compile cases, right? So let's specify them as common > >set of options, instead of relying on KBUILD_HOSTCFLAGS or > >HOST_EXTRACFLAGS to have them. Otherwise we'll be getting extra > >warnings for just cross-compile case, which is not good. If you are > >worrying about having duplicate -W flags, seems like it's handled by > >GCC already, so shouldn't be a problem. > > Ok, lets drop omit-frame-pointer. > > But then, lets do more radical step and drop > KBUILD_HOSTCFLAGS & HOST_EXTRACFLAG in this patch: Yeah, let's do this, if you confirmed that everything still works (and I don't see a reason why it shouldn't). Thanks. > > -ifdef CROSS_COMPILE > +TPROGS_CFLAGS += -Wall -O2 > +TPROGS_CFLAGS += -Wmissing-prototypes > +TPROGS_CFLAGS += -Wstrict-prototypes > -else > -TPROGS_LDLIBS := $(KBUILD_HOSTLDLIBS) > -TPROGS_CFLAGS += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) > -endif > > At least it allows to use same options always for both, native and cross. > > I verified on native x86_64, arm64 and arm and cross for arm and arm64, > but should work for others, at least it can be tuned explicitly and > no need to depend on KBUILD and use "cross" fork here. Yep, I like it. > > -- > Regards, > Ivan Khoronzhuk