[PATCH v3 0/3] selftests/hid: assorted fixes

[PATCH v3 0/3] selftests/hid: assorted fixes

[Benjamin Tissoires]

And this is the last(?) revision of this series which should now compile
with or without CONFIG_HID_BPF set.

I had to do changes because [1] was failing

Nick, I kept your Tested-by, even if I made small changes in 1/3. Feel
free to shout if you don't want me to keep it.

Eduard, You helped us a lot in the review of v1 but never sent your
Reviewed-by or Acked-by. Do you want me to add one?

Cheers,
Benjamin

[1] https://gitlab.freedesktop.org/bentiss/hid/-/jobs/49754306

For reference, the v2 cover letter:

| Hi, I am sending this series on behalf of myself and Benjamin Tissoires. There
| existed an initial n=3 patch series which was later expanded to n=4 and
| is now back to n=3 with some fixes added in and rebased against
| mainline.
|
| This patch series aims to ensure that the hid/bpf selftests can be built
| without errors.
|
| Here's Benjamin's initial cover letter for context:
| |  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@redhat.com/T/#t

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
Changes in v3:
- Also overwrite all of the enum symbols in patch 1/3
- Link to v2: https://lore.kernel.org/r/20230908-kselftest-09-08-v2-0-0def978a4c1b@google.com

Changes in v2:
- roll Justin's fix into patch 1/3
- add __attribute__((preserve_access_index)) (thanks Eduard)
- rebased onto mainline (2dde18cd1d8fac735875f2e4987f11817cc0bc2c)
- Link to v1: https://lore.kernel.org/r/20230825-wip-selftests-v1-0-c862769020a8@kernel.org

Link: https://github.com/ClangBuiltLinux/linux/issues/1698
Link: https://github.com/ClangBuiltLinux/continuous-integration2/issues/61

---
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 -
 .../testing/selftests/hid/progs/hid_bpf_helpers.h  | 77 ++++++++++++++++++++++
 3 files changed, 81 insertions(+), 9 deletions(-)
---
base-commit: 29aa98d0fe013e2ab62aae4266231b7fb05d47a2
change-id: 20230825-wip-selftests-9a7502b56542

Best regards,
-- 
Benjamin Tissoires <bentiss@kernel.org>

[PATCH v3 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3

[Benjamin Tissoires]

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 and also add __attribute__((preserve_access_index)) to further
support CO-RE functionality for these tests.

Signed-off-by: Justin Stitt <justinstitt@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com> # Build
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 tools/testing/selftests/hid/progs/hid.c            |  3 -
 .../testing/selftests/hid/progs/hid_bpf_helpers.h  | 77 ++++++++++++++++++++++
 2 files changed, 77 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..65e657ac1198 100644
--- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
+++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
@@ -5,6 +5,83 @@
 #ifndef __HID_BPF_HELPERS_H
 #define __HID_BPF_HELPERS_H
 
+/* "undefine" structs and enums 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
+#define HID_INPUT_REPORT         HID_INPUT_REPORT___not_used
+#define HID_OUTPUT_REPORT        HID_OUTPUT_REPORT___not_used
+#define HID_FEATURE_REPORT       HID_FEATURE_REPORT___not_used
+#define HID_REPORT_TYPES         HID_REPORT_TYPES___not_used
+#define HID_REQ_GET_REPORT       HID_REQ_GET_REPORT___not_used
+#define HID_REQ_GET_IDLE         HID_REQ_GET_IDLE___not_used
+#define HID_REQ_GET_PROTOCOL     HID_REQ_GET_PROTOCOL___not_used
+#define HID_REQ_SET_REPORT       HID_REQ_SET_REPORT___not_used
+#define HID_REQ_SET_IDLE         HID_REQ_SET_IDLE___not_used
+#define HID_REQ_SET_PROTOCOL     HID_REQ_SET_PROTOCOL___not_used
+#define HID_BPF_FLAG_NONE        HID_BPF_FLAG_NONE___not_used
+#define HID_BPF_FLAG_INSERT_HEAD HID_BPF_FLAG_INSERT_HEAD___not_used
+#define HID_BPF_FLAG_MAX         HID_BPF_FLAG_MAX___not_used
+
+#include "vmlinux.h"
+
+#undef hid_bpf_ctx
+#undef hid_report_type
+#undef hid_class_request
+#undef hid_bpf_attach_flags
+#undef HID_INPUT_REPORT
+#undef HID_OUTPUT_REPORT
+#undef HID_FEATURE_REPORT
+#undef HID_REPORT_TYPES
+#undef HID_REQ_GET_REPORT
+#undef HID_REQ_GET_IDLE
+#undef HID_REQ_GET_PROTOCOL
+#undef HID_REQ_SET_REPORT
+#undef HID_REQ_SET_IDLE
+#undef HID_REQ_SET_PROTOCOL
+#undef HID_BPF_FLAG_NONE
+#undef HID_BPF_FLAG_INSERT_HEAD
+#undef HID_BPF_FLAG_MAX
+
+#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;
+	const struct hid_device *hid;
+	__u32 allocated_size;
+	enum hid_report_type report_type;
+	union {
+		__s32 retval;
+		__s32 size;
+	};
+} __attribute__((preserve_access_index));
+
+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.39.1

[PATCH v3 2/3] selftests/hid: do not manually call headers_install

[Benjamin Tissoires]

"make headers" is a requirement before calling make on the selftests
dir, so we should not have to manually install those headers

Acked-by: Shuah Khan <skhan@linuxfoundation.org>
Tested-by: Nick Desaulniers <ndesaulniers@google.com> # Build
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 2e986cbf1a46..a28054113f47 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 $@
 

-- 
2.39.1

[PATCH v3 3/3] selftests/hid: force using our compiled libbpf headers

[Benjamin Tissoires]

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.

Tested-by: Nick Desaulniers <ndesaulniers@google.com> # Build
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 a28054113f47..2b5ea18bde38 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

-- 
2.39.1

Re: [PATCH v3 0/3] selftests/hid: assorted fixes

[Justin Stitt]

On Thu, Oct 5, 2023 at 8:55 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> And this is the last(?) revision of this series which should now compile
> with or without CONFIG_HID_BPF set.
>
> I had to do changes because [1] was failing
>
> Nick, I kept your Tested-by, even if I made small changes in 1/3. Feel
> free to shout if you don't want me to keep it.
>
> Eduard, You helped us a lot in the review of v1 but never sent your
> Reviewed-by or Acked-by. Do you want me to add one?
>
> Cheers,
> Benjamin
>
> [1] https://gitlab.freedesktop.org/bentiss/hid/-/jobs/49754306
>
> For reference, the v2 cover letter:
>
> | Hi, I am sending this series on behalf of myself and Benjamin Tissoires. There
> | existed an initial n=3 patch series which was later expanded to n=4 and
> | is now back to n=3 with some fixes added in and rebased against
> | mainline.
> |
> | This patch series aims to ensure that the hid/bpf selftests can be built
> | without errors.
> |
> | Here's Benjamin's initial cover letter for context:
> | |  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@redhat.com/T/#t
>
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> Changes in v3:
> - Also overwrite all of the enum symbols in patch 1/3
> - Link to v2: https://lore.kernel.org/r/20230908-kselftest-09-08-v2-0-0def978a4c1b@google.com
>
> Changes in v2:
> - roll Justin's fix into patch 1/3
> - add __attribute__((preserve_access_index)) (thanks Eduard)
> - rebased onto mainline (2dde18cd1d8fac735875f2e4987f11817cc0bc2c)
> - Link to v1: https://lore.kernel.org/r/20230825-wip-selftests-v1-0-c862769020a8@kernel.org
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1698
> Link: https://github.com/ClangBuiltLinux/continuous-integration2/issues/61
>
> ---
> 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 -
>  .../testing/selftests/hid/progs/hid_bpf_helpers.h  | 77 ++++++++++++++++++++++
>  3 files changed, 81 insertions(+), 9 deletions(-)
> ---
> base-commit: 29aa98d0fe013e2ab62aae4266231b7fb05d47a2
> change-id: 20230825-wip-selftests-9a7502b56542
>
> Best regards,
> --
> Benjamin Tissoires <bentiss@kernel.org>
>

Tested entire series.

 I can now build the tests using this command:

$ make LLVM=1 -j128 ARCH=x86_64 mrproper headers && make LLVM=1 -j128
ARCH=x86_64 -C tools/testing/selftests TARGETS=hid


Tested-by:  Justin Stitt <justinstitt@google.com>

Re: [PATCH v3 0/3] selftests/hid: assorted fixes

[Eduard Zingerman]

On Thu, 2023-10-05 at 17:55 +0200, Benjamin Tissoires wrote:
> > And this is the last(?) revision of this series which should now compile
> > with or without CONFIG_HID_BPF set.
> > 
> > I had to do changes because [1] was failing
> > 
> > Nick, I kept your Tested-by, even if I made small changes in 1/3. Feel
> > free to shout if you don't want me to keep it.
> > 
> > Eduard, You helped us a lot in the review of v1 but never sent your
> > Reviewed-by or Acked-by. Do you want me to add one?

Hi Benjamin,

I think there is no need, I just took part in the discussion and that's all.
Feel free to ping me if there is anything BPF related that needs clarification.

Thanks,
Eduard

> > 
> > Cheers,
> > Benjamin
> > 
> > [1] https://gitlab.freedesktop.org/bentiss/hid/-/jobs/49754306
> > 
> > For reference, the v2 cover letter:
> > 
> > > > Hi, I am sending this series on behalf of myself and Benjamin Tissoires. There
> > > > existed an initial n=3 patch series which was later expanded to n=4 and
> > > > is now back to n=3 with some fixes added in and rebased against
> > > > mainline.
> > > > 
> > > > This patch series aims to ensure that the hid/bpf selftests can be built
> > > > without errors.
> > > > 
> > > > Here's Benjamin's initial cover letter for context:
> > > > > >  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@redhat.com/T/#t
> > 
> > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> > ---
> > Changes in v3:
> > - Also overwrite all of the enum symbols in patch 1/3
> > - Link to v2: https://lore.kernel.org/r/20230908-kselftest-09-08-v2-0-0def978a4c1b@google.com
> > 
> > Changes in v2:
> > - roll Justin's fix into patch 1/3
> > - add __attribute__((preserve_access_index)) (thanks Eduard)
> > - rebased onto mainline (2dde18cd1d8fac735875f2e4987f11817cc0bc2c)
> > - Link to v1: https://lore.kernel.org/r/20230825-wip-selftests-v1-0-c862769020a8@kernel.org
> > 
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1698
> > Link: https://github.com/ClangBuiltLinux/continuous-integration2/issues/61
> > 
> > ---
> > 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 -
> >  .../testing/selftests/hid/progs/hid_bpf_helpers.h  | 77 ++++++++++++++++++++++
> >  3 files changed, 81 insertions(+), 9 deletions(-)
> > ---
> > base-commit: 29aa98d0fe013e2ab62aae4266231b7fb05d47a2
> > change-id: 20230825-wip-selftests-9a7502b56542
> > 
> > Best regards,

Re: [PATCH v3 0/3] selftests/hid: assorted fixes

[Benjamin Tissoires]

On Thu, 05 Oct 2023 17:55:31 +0200, Benjamin Tissoires wrote:
> And this is the last(?) revision of this series which should now compile
> with or without CONFIG_HID_BPF set.
> 
> I had to do changes because [1] was failing
> 
> Nick, I kept your Tested-by, even if I made small changes in 1/3. Feel
> free to shout if you don't want me to keep it.
> 
> [...]

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-6.7/selftests), thanks!

[1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3
      https://git.kernel.org/hid/hid/c/ae7487d112cf
[2/3] selftests/hid: do not manually call headers_install
      https://git.kernel.org/hid/hid/c/89d024a7ba02
[3/3] selftests/hid: force using our compiled libbpf headers
      https://git.kernel.org/hid/hid/c/91939636cac4

Cheers,
-- 
Benjamin Tissoires <bentiss@kernel.org>