These fixes have been triggered by [0]: basically, if you do not recompile the kernel first, and are running on an old kernel, vmlinux.h doesn't have the required symbols and the compilation fails.
The tests will fail if you run them on that very same machine, of course, but the binary should compile.
And while I was sorting out why it was failing, I realized I could do a couple of improvements on the Makefile.
[0] https://lore.kernel.org/linux-input/56ba8125-2c6f-a9c9-d498-0ca1c153dcb2@red...
Signed-off-by: Benjamin Tissoires bentiss@kernel.org --- Benjamin Tissoires (3): selftests/hid: ensure we can compile the tests on kernels pre-6.3 selftests/hid: do not manually call headers_install selftests/hid: force using our compiled libbpf headers
tools/testing/selftests/hid/Makefile | 10 ++++------ tools/testing/selftests/hid/progs/hid.c | 3 --- tools/testing/selftests/hid/progs/hid_bpf_helpers.h | 20 ++++++++++++++++++++ 3 files changed, 24 insertions(+), 9 deletions(-) --- base-commit: 1d7546042f8fdc4bc39ab91ec966203e2d64f8bd change-id: 20230825-wip-selftests-9a7502b56542
Best regards,
For the hid-bpf tests to compile, we need to have the definition of struct hid_bpf_ctx. This definition is an internal one from the kernel and it is supposed to be defined in the generated vmlinux.h.
This vmlinux.h header is generated based on the currently running kernel or if the kernel was already compiled in the tree. If you just compile the selftests without compiling the kernel beforehand and you are running on a 6.2 kernel, you'll end up with a vmlinux.h without the hid_bpf_ctx definition.
Use the clever trick from tools/testing/selftests/bpf/progs/bpf_iter.h to force the definition of that symbol in case we don't find it in the BTF.
Signed-off-by: Benjamin Tissoires bentiss@kernel.org --- tools/testing/selftests/hid/progs/hid.c | 3 --- tools/testing/selftests/hid/progs/hid_bpf_helpers.h | 20 ++++++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/hid/progs/hid.c b/tools/testing/selftests/hid/progs/hid.c index 88c593f753b5..1e558826b809 100644 --- a/tools/testing/selftests/hid/progs/hid.c +++ b/tools/testing/selftests/hid/progs/hid.c @@ -1,8 +1,5 @@ // SPDX-License-Identifier: GPL-2.0 /* Copyright (c) 2022 Red hat */ -#include "vmlinux.h" -#include <bpf/bpf_helpers.h> -#include <bpf/bpf_tracing.h> #include "hid_bpf_helpers.h"
char _license[] SEC("license") = "GPL"; diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h index 4fff31dbe0e7..749097f8f4d9 100644 --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h @@ -5,6 +5,26 @@ #ifndef __HID_BPF_HELPERS_H #define __HID_BPF_HELPERS_H
+/* "undefine" structs in vmlinux.h, because we "override" them below */ +#define hid_bpf_ctx hid_bpf_ctx___not_used +#include "vmlinux.h" +#undef hid_bpf_ctx + +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> + + +struct hid_bpf_ctx { + __u32 index; + const struct hid_device *hid; + __u32 allocated_size; + enum hid_report_type report_type; + union { + __s32 retval; + __s32 size; + }; +}; + /* following are kfuncs exported by HID for HID-BPF */ extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset,
"make headers" is a requirement before calling make on the selftests dir, so we should not have to manually install those headers
Signed-off-by: Benjamin Tissoires bentiss@kernel.org --- tools/testing/selftests/hid/Makefile | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/hid/Makefile b/tools/testing/selftests/hid/Makefile index 01c0491d64da..c5522088ece4 100644 --- a/tools/testing/selftests/hid/Makefile +++ b/tools/testing/selftests/hid/Makefile @@ -21,7 +21,7 @@ CXX ?= $(CROSS_COMPILE)g++
HOSTPKG_CONFIG := pkg-config
-CFLAGS += -g -O0 -rdynamic -Wall -Werror -I$(KHDR_INCLUDES) -I$(OUTPUT) +CFLAGS += -g -O0 -rdynamic -Wall -Werror -I$(OUTPUT) LDLIBS += -lelf -lz -lrt -lpthread
# Silence some warnings when compiled with clang @@ -65,7 +65,6 @@ BPFTOOLDIR := $(TOOLSDIR)/bpf/bpftool SCRATCH_DIR := $(OUTPUT)/tools BUILD_DIR := $(SCRATCH_DIR)/build INCLUDE_DIR := $(SCRATCH_DIR)/include -KHDR_INCLUDES := $(SCRATCH_DIR)/uapi/include BPFOBJ := $(BUILD_DIR)/libbpf/libbpf.a ifneq ($(CROSS_COMPILE),) HOST_BUILD_DIR := $(BUILD_DIR)/host @@ -151,9 +150,6 @@ else $(Q)cp "$(VMLINUX_H)" $@ endif
-$(KHDR_INCLUDES)/linux/hid.h: $(top_srcdir)/include/uapi/linux/hid.h - $(MAKE) -C $(top_srcdir) INSTALL_HDR_PATH=$(SCRATCH_DIR)/uapi headers_install - $(RESOLVE_BTFIDS): $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/resolve_btfids \ $(TOOLSDIR)/bpf/resolve_btfids/main.c \ $(TOOLSDIR)/lib/rbtree.c \ @@ -231,7 +227,7 @@ $(BPF_SKELS): %.skel.h: %.bpf.o $(BPFTOOL) | $(OUTPUT) $(Q)$(BPFTOOL) gen object $(<:.o=.linked1.o) $< $(Q)$(BPFTOOL) gen skeleton $(<:.o=.linked1.o) name $(notdir $(<:.bpf.o=)) > $@
-$(OUTPUT)/%.o: %.c $(BPF_SKELS) $(KHDR_INCLUDES)/linux/hid.h +$(OUTPUT)/%.o: %.c $(BPF_SKELS) $(call msg,CC,,$@) $(Q)$(CC) $(CFLAGS) -c $(filter %.c,$^) $(LDLIBS) -o $@
Turns out that we were relying on the globally installed headers, not the ones we freshly compiled. Add a manual include in CFLAGS to sort this out.
Signed-off-by: Benjamin Tissoires bentiss@kernel.org --- tools/testing/selftests/hid/Makefile | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/hid/Makefile b/tools/testing/selftests/hid/Makefile index c5522088ece4..b01c14077c5d 100644 --- a/tools/testing/selftests/hid/Makefile +++ b/tools/testing/selftests/hid/Makefile @@ -22,6 +22,8 @@ CXX ?= $(CROSS_COMPILE)g++ HOSTPKG_CONFIG := pkg-config
CFLAGS += -g -O0 -rdynamic -Wall -Werror -I$(OUTPUT) +CFLAGS += -I$(OUTPUT)/tools/include + LDLIBS += -lelf -lz -lrt -lpthread
# Silence some warnings when compiled with clang
On Fri, Aug 25, 2023 at 10:36:30AM +0200, Benjamin Tissoires wrote:
These fixes have been triggered by [0]: basically, if you do not recompile the kernel first, and are running on an old kernel, vmlinux.h doesn't have the required symbols and the compilation fails.
The tests will fail if you run them on that very same machine, of course, but the binary should compile.
And while I was sorting out why it was failing, I realized I could do a couple of improvements on the Makefile.
[0] https://lore.kernel.org/linux-input/56ba8125-2c6f-a9c9-d498-0ca1c153dcb2@red...
Signed-off-by: Benjamin Tissoires bentiss@kernel.org
Benjamin Tissoires (3): selftests/hid: ensure we can compile the tests on kernels pre-6.3 selftests/hid: do not manually call headers_install selftests/hid: force using our compiled libbpf headers
tools/testing/selftests/hid/Makefile | 10 ++++------ tools/testing/selftests/hid/progs/hid.c | 3 --- tools/testing/selftests/hid/progs/hid_bpf_helpers.h | 20 ++++++++++++++++++++ 3 files changed, 24 insertions(+), 9 deletions(-)
base-commit: 1d7546042f8fdc4bc39ab91ec966203e2d64f8bd change-id: 20230825-wip-selftests-9a7502b56542
Best regards,
Benjamin Tissoires bentiss@kernel.org
Benjamin, thanks for the work here. Your series fixed up _some_ of the errors I had while building on my 6.3.11 kernel. I'm proposing a single patch that should be applied on top of your series that fully fixes _all_ of the build errors I'm experiencing.
Can you let me know if it works and potentially formulate a new series so that `b4 shazam` applies it cleanly?
PATCH BELOW --- From 5378d70e1b3f7f75656332f9bff65a37122bb288 Mon Sep 17 00:00:00 2001 From: Justin Stitt justinstitt@google.com Date: Fri, 25 Aug 2023 18:10:33 +0000 Subject: [PATCH 4/3] selftests/hid: more fixes to build with older kernel
I had to use the clever trick [1] on some other symbols to get my builds working.
Apply this patch on top of Benjamin's series [2].
This is now a n=4 patch series which has fixed my builds when running: | $ make LLVM=1 -j128 ARCH=x86_64 mrproper headers | $ make LLVM=1 -j128 ARCH=x86_64 -C tools/testing/selftests TARGETS=hid
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tool... [2]: https://lore.kernel.org/all/20230825-wip-selftests-v1-0-c862769020a8@kernel.... Signed-off-by: Justin Stitt justinstitt@google.com --- .../selftests/hid/progs/hid_bpf_helpers.h | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h index 749097f8f4d9..e2eace2c0029 100644 --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h @@ -7,12 +7,26 @@
/* "undefine" structs in vmlinux.h, because we "override" them below */ #define hid_bpf_ctx hid_bpf_ctx___not_used +#define hid_report_type hid_report_type___not_used +#define hid_class_request hid_class_request___not_used +#define hid_bpf_attach_flags hid_bpf_attach_flags___not_used #include "vmlinux.h" #undef hid_bpf_ctx +#undef hid_report_type +#undef hid_class_request +#undef hid_bpf_attach_flags
#include <bpf/bpf_helpers.h> #include <bpf/bpf_tracing.h> +#include <linux/const.h>
+enum hid_report_type { + HID_INPUT_REPORT = 0, + HID_OUTPUT_REPORT = 1, + HID_FEATURE_REPORT = 2, + + HID_REPORT_TYPES, +};
struct hid_bpf_ctx { __u32 index; @@ -25,6 +39,21 @@ struct hid_bpf_ctx { }; };
+enum hid_class_request { + HID_REQ_GET_REPORT = 0x01, + HID_REQ_GET_IDLE = 0x02, + HID_REQ_GET_PROTOCOL = 0x03, + HID_REQ_SET_REPORT = 0x09, + HID_REQ_SET_IDLE = 0x0A, + HID_REQ_SET_PROTOCOL = 0x0B, +}; + +enum hid_bpf_attach_flags { + HID_BPF_FLAG_NONE = 0, + HID_BPF_FLAG_INSERT_HEAD = _BITUL(0), + HID_BPF_FLAG_MAX, +}; + /* following are kfuncs exported by HID for HID-BPF */ extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, -- 2.42.0.rc1.204.g551eb34607-goog
On Fri, 2023-08-25 at 18:23 +0000, Justin Stitt wrote:
On Fri, Aug 25, 2023 at 10:36:30AM +0200, Benjamin Tissoires wrote:
These fixes have been triggered by [0]: basically, if you do not recompile the kernel first, and are running on an old kernel, vmlinux.h doesn't have the required symbols and the compilation fails.
The tests will fail if you run them on that very same machine, of course, but the binary should compile.
And while I was sorting out why it was failing, I realized I could do a couple of improvements on the Makefile.
[0] https://lore.kernel.org/linux-input/56ba8125-2c6f-a9c9-d498-0ca1c153dcb2@red...
Signed-off-by: Benjamin Tissoires bentiss@kernel.org
Benjamin Tissoires (3): selftests/hid: ensure we can compile the tests on kernels pre-6.3 selftests/hid: do not manually call headers_install selftests/hid: force using our compiled libbpf headers
tools/testing/selftests/hid/Makefile | 10 ++++------ tools/testing/selftests/hid/progs/hid.c | 3 --- tools/testing/selftests/hid/progs/hid_bpf_helpers.h | 20 ++++++++++++++++++++ 3 files changed, 24 insertions(+), 9 deletions(-)
base-commit: 1d7546042f8fdc4bc39ab91ec966203e2d64f8bd change-id: 20230825-wip-selftests-9a7502b56542
Best regards,
Benjamin Tissoires bentiss@kernel.org
Benjamin, thanks for the work here. Your series fixed up _some_ of the errors I had while building on my 6.3.11 kernel. I'm proposing a single patch that should be applied on top of your series that fully fixes _all_ of the build errors I'm experiencing.
Can you let me know if it works and potentially formulate a new series so that `b4 shazam` applies it cleanly?
PATCH BELOW
From 5378d70e1b3f7f75656332f9bff65a37122bb288 Mon Sep 17 00:00:00 2001 From: Justin Stitt justinstitt@google.com Date: Fri, 25 Aug 2023 18:10:33 +0000 Subject: [PATCH 4/3] selftests/hid: more fixes to build with older kernel
I had to use the clever trick [1] on some other symbols to get my builds working.
Apply this patch on top of Benjamin's series [2].
This is now a n=4 patch series which has fixed my builds when running:
$ make LLVM=1 -j128 ARCH=x86_64 mrproper headers $ make LLVM=1 -j128 ARCH=x86_64 -C tools/testing/selftests TARGETS=hid
Signed-off-by: Justin Stitt justinstitt@google.com
.../selftests/hid/progs/hid_bpf_helpers.h | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h index 749097f8f4d9..e2eace2c0029 100644 --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h @@ -7,12 +7,26 @@
/* "undefine" structs in vmlinux.h, because we "override" them below */ #define hid_bpf_ctx hid_bpf_ctx___not_used +#define hid_report_type hid_report_type___not_used +#define hid_class_request hid_class_request___not_used +#define hid_bpf_attach_flags hid_bpf_attach_flags___not_used #include "vmlinux.h" #undef hid_bpf_ctx +#undef hid_report_type +#undef hid_class_request +#undef hid_bpf_attach_flags
#include <bpf/bpf_helpers.h> #include <bpf/bpf_tracing.h> +#include <linux/const.h>
+enum hid_report_type {
- HID_INPUT_REPORT = 0,
- HID_OUTPUT_REPORT = 1,
- HID_FEATURE_REPORT = 2,
- HID_REPORT_TYPES,
+};
struct hid_bpf_ctx { __u32 index; @@ -25,6 +39,21 @@ struct hid_bpf_ctx { }; };
Note, vmlinux.h has the following preamble/postamble:
#ifndef BPF_NO_PRESERVE_ACCESS_INDEX #pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record) #endif ... #ifndef BPF_NO_PRESERVE_ACCESS_INDEX #pragma clang attribute pop #endif
You might want to use it or add __attribute__((preserve_access_index)) to structure definitions, depending on whether or not you need CO-RE functionality for these tests.
+enum hid_class_request {
- HID_REQ_GET_REPORT = 0x01,
- HID_REQ_GET_IDLE = 0x02,
- HID_REQ_GET_PROTOCOL = 0x03,
- HID_REQ_SET_REPORT = 0x09,
- HID_REQ_SET_IDLE = 0x0A,
- HID_REQ_SET_PROTOCOL = 0x0B,
+};
+enum hid_bpf_attach_flags {
- HID_BPF_FLAG_NONE = 0,
- HID_BPF_FLAG_INSERT_HEAD = _BITUL(0),
- HID_BPF_FLAG_MAX,
+};
/* following are kfuncs exported by HID for HID-BPF */ extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, -- 2.42.0.rc1.204.g551eb34607-goog
Eduard,
On Fri, Aug 25, 2023 at 11:54 AM Eduard Zingerman eddyz87@gmail.com wrote:
On Fri, 2023-08-25 at 18:23 +0000, Justin Stitt wrote:
On Fri, Aug 25, 2023 at 10:36:30AM +0200, Benjamin Tissoires wrote:
These fixes have been triggered by [0]: basically, if you do not recompile the kernel first, and are running on an old kernel, vmlinux.h doesn't have the required symbols and the compilation fails.
The tests will fail if you run them on that very same machine, of course, but the binary should compile.
And while I was sorting out why it was failing, I realized I could do a couple of improvements on the Makefile.
[0] https://lore.kernel.org/linux-input/56ba8125-2c6f-a9c9-d498-0ca1c153dcb2@red...
Signed-off-by: Benjamin Tissoires bentiss@kernel.org
Benjamin Tissoires (3): selftests/hid: ensure we can compile the tests on kernels pre-6.3 selftests/hid: do not manually call headers_install selftests/hid: force using our compiled libbpf headers
tools/testing/selftests/hid/Makefile | 10 ++++------ tools/testing/selftests/hid/progs/hid.c | 3 --- tools/testing/selftests/hid/progs/hid_bpf_helpers.h | 20 ++++++++++++++++++++ 3 files changed, 24 insertions(+), 9 deletions(-)
base-commit: 1d7546042f8fdc4bc39ab91ec966203e2d64f8bd change-id: 20230825-wip-selftests-9a7502b56542
Best regards,
Benjamin Tissoires bentiss@kernel.org
Benjamin, thanks for the work here. Your series fixed up _some_ of the errors I had while building on my 6.3.11 kernel. I'm proposing a single patch that should be applied on top of your series that fully fixes _all_ of the build errors I'm experiencing.
Can you let me know if it works and potentially formulate a new series so that `b4 shazam` applies it cleanly?
PATCH BELOW
From 5378d70e1b3f7f75656332f9bff65a37122bb288 Mon Sep 17 00:00:00 2001 From: Justin Stitt justinstitt@google.com Date: Fri, 25 Aug 2023 18:10:33 +0000 Subject: [PATCH 4/3] selftests/hid: more fixes to build with older kernel
I had to use the clever trick [1] on some other symbols to get my builds working.
Apply this patch on top of Benjamin's series [2].
This is now a n=4 patch series which has fixed my builds when running:
$ make LLVM=1 -j128 ARCH=x86_64 mrproper headers $ make LLVM=1 -j128 ARCH=x86_64 -C tools/testing/selftests TARGETS=hid
Signed-off-by: Justin Stitt justinstitt@google.com
.../selftests/hid/progs/hid_bpf_helpers.h | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h index 749097f8f4d9..e2eace2c0029 100644 --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h @@ -7,12 +7,26 @@
/* "undefine" structs in vmlinux.h, because we "override" them below */ #define hid_bpf_ctx hid_bpf_ctx___not_used +#define hid_report_type hid_report_type___not_used +#define hid_class_request hid_class_request___not_used +#define hid_bpf_attach_flags hid_bpf_attach_flags___not_used #include "vmlinux.h" #undef hid_bpf_ctx +#undef hid_report_type +#undef hid_class_request +#undef hid_bpf_attach_flags
#include <bpf/bpf_helpers.h> #include <bpf/bpf_tracing.h> +#include <linux/const.h>
+enum hid_report_type {
HID_INPUT_REPORT = 0,HID_OUTPUT_REPORT = 1,HID_FEATURE_REPORT = 2,HID_REPORT_TYPES,+};
struct hid_bpf_ctx { __u32 index; @@ -25,6 +39,21 @@ struct hid_bpf_ctx { }; };
Note, vmlinux.h has the following preamble/postamble:
#ifndef BPF_NO_PRESERVE_ACCESS_INDEX #pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record) #endif ... #ifndef BPF_NO_PRESERVE_ACCESS_INDEX #pragma clang attribute pop #endifYou might want to use it or add __attribute__((preserve_access_index)) to structure definitions, depending on whether or not you need CO-RE functionality for these tests.
I have no idea whether or not CO-RE is needed for these tests or not. My main motivation is getting these selftests building with LLVM=1 [1].
Perhaps Benjamin could provide some more insight on whether this is needed or not and roll out a v2 containing my patch on top + any CO-RE semantics -- if deemed necessary.
+enum hid_class_request {
HID_REQ_GET_REPORT = 0x01,HID_REQ_GET_IDLE = 0x02,HID_REQ_GET_PROTOCOL = 0x03,HID_REQ_SET_REPORT = 0x09,HID_REQ_SET_IDLE = 0x0A,HID_REQ_SET_PROTOCOL = 0x0B,+};
+enum hid_bpf_attach_flags {
HID_BPF_FLAG_NONE = 0,HID_BPF_FLAG_INSERT_HEAD = _BITUL(0),HID_BPF_FLAG_MAX,+};
/* following are kfuncs exported by HID for HID-BPF */ extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, -- 2.42.0.rc1.204.g551eb34607-goog
[1]: https://github.com/ClangBuiltLinux/linux/issues/1698
Thanks Justin
linux-kselftest-mirror@lists.linaro.org
-
Benjamin Tissoires -
Eduard Zingerman -
Justin Stitt