bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] selftests/hid: fix building for older kernels
@ 2023-09-08 22:22 Justin Stitt
  2023-09-08 22:22 ` [PATCH v2 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3 Justin Stitt
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Justin Stitt @ 2023-09-08 22:22 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Shuah Khan
  Cc: Eduard Zingerman, linux-input, linux-kselftest, linux-kernel,
	bpf, Benjamin Tissoires, Justin Stitt

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

Changes from v1 -> 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/all/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  | 49 ++++++++++++++++++++++
 3 files changed, 53 insertions(+), 9 deletions(-)
---
base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
change-id: 20230908-kselftest-09-08-56d7f4a8d5c4

Best regards,
--
Justin Stitt <justinstitt@google.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3
  2023-09-08 22:22 [PATCH v2 0/3] selftests/hid: fix building for older kernels Justin Stitt
@ 2023-09-08 22:22 ` Justin Stitt
  2023-09-11 13:19   ` Eduard Zingerman
  2023-09-08 22:22 ` [PATCH v2 2/3] selftests/hid: do not manually call headers_install Justin Stitt
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Justin Stitt @ 2023-09-08 22:22 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Shuah Khan
  Cc: Eduard Zingerman, linux-input, linux-kselftest, linux-kernel,
	bpf, Benjamin Tissoires, Justin Stitt

From: Benjamin Tissoires <bentiss@kernel.org>

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>
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 tools/testing/selftests/hid/progs/hid.c            |  3 --
 .../testing/selftests/hid/progs/hid_bpf_helpers.h  | 49 ++++++++++++++++++++++
 2 files changed, 49 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..ab3b18ba48c4 100644
--- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
+++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
@@ -5,6 +5,55 @@
 #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
+#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;
+	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.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/3] selftests/hid: do not manually call headers_install
  2023-09-08 22:22 [PATCH v2 0/3] selftests/hid: fix building for older kernels Justin Stitt
  2023-09-08 22:22 ` [PATCH v2 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3 Justin Stitt
@ 2023-09-08 22:22 ` Justin Stitt
  2023-09-18 17:24   ` Shuah Khan
  2023-09-08 22:22 ` [PATCH v2 3/3] selftests/hid: force using our compiled libbpf headers Justin Stitt
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Justin Stitt @ 2023-09-08 22:22 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Shuah Khan
  Cc: Eduard Zingerman, linux-input, linux-kselftest, linux-kernel,
	bpf, Benjamin Tissoires

From: Benjamin Tissoires <bentiss@kernel.org>

"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 $@
 

-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/3] selftests/hid: force using our compiled libbpf headers
  2023-09-08 22:22 [PATCH v2 0/3] selftests/hid: fix building for older kernels Justin Stitt
  2023-09-08 22:22 ` [PATCH v2 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3 Justin Stitt
  2023-09-08 22:22 ` [PATCH v2 2/3] selftests/hid: do not manually call headers_install Justin Stitt
@ 2023-09-08 22:22 ` Justin Stitt
  2023-09-08 23:02 ` [PATCH v2 0/3] selftests/hid: fix building for older kernels Nick Desaulniers
  2023-09-26  7:26 ` Justin Stitt
  4 siblings, 0 replies; 12+ messages in thread
From: Justin Stitt @ 2023-09-08 22:22 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Shuah Khan
  Cc: Eduard Zingerman, linux-input, linux-kselftest, linux-kernel,
	bpf, Benjamin Tissoires

From: Benjamin Tissoires <bentiss@kernel.org>

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

-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/3] selftests/hid: fix building for older kernels
  2023-09-08 22:22 [PATCH v2 0/3] selftests/hid: fix building for older kernels Justin Stitt
                   ` (2 preceding siblings ...)
  2023-09-08 22:22 ` [PATCH v2 3/3] selftests/hid: force using our compiled libbpf headers Justin Stitt
@ 2023-09-08 23:02 ` Nick Desaulniers
  2023-09-26  7:26 ` Justin Stitt
  4 siblings, 0 replies; 12+ messages in thread
From: Nick Desaulniers @ 2023-09-08 23:02 UTC (permalink / raw)
  To: Justin Stitt
  Cc: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Eduard Zingerman,
	linux-input, linux-kselftest, linux-kernel, bpf,
	Benjamin Tissoires, llvm

On Fri, Sep 08, 2023 at 10:22:37PM +0000, Justin Stitt wrote:
> 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
> 
> Changes from v1 -> 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/all/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

Thanks to you and Benjamin for sorting all of this out! With this series
applied, I was able to build the hid selftests now without the previous
-Wvisibility diagnostics failing the build.

Tested-by: Nick Desaulniers <ndesaulniers@google.com> # Build

> ---
> 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  | 49 ++++++++++++++++++++++
>  3 files changed, 53 insertions(+), 9 deletions(-)
> ---
> base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
> change-id: 20230908-kselftest-09-08-56d7f4a8d5c4
> 
> Best regards,
> --
> Justin Stitt <justinstitt@google.com>
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3
  2023-09-08 22:22 ` [PATCH v2 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3 Justin Stitt
@ 2023-09-11 13:19   ` Eduard Zingerman
  2023-09-11 13:39     ` Benjamin Tissoires
  0 siblings, 1 reply; 12+ messages in thread
From: Eduard Zingerman @ 2023-09-11 13:19 UTC (permalink / raw)
  To: Justin Stitt, Jiri Kosina, Benjamin Tissoires, Shuah Khan
  Cc: linux-input, linux-kselftest, linux-kernel, bpf, Benjamin Tissoires

On Fri, 2023-09-08 at 22:22 +0000, Justin Stitt wrote:
> From: Benjamin Tissoires <bentiss@kernel.org>
> 
> 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>
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
>  tools/testing/selftests/hid/progs/hid.c            |  3 --
>  .../testing/selftests/hid/progs/hid_bpf_helpers.h  | 49 ++++++++++++++++++++++
>  2 files changed, 49 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..ab3b18ba48c4 100644
> --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> @@ -5,6 +5,55 @@
>  #ifndef __HID_BPF_HELPERS_H
>  #define __HID_BPF_HELPERS_H
>  
> +/* "undefine" structs in vmlinux.h, because we "override" them below */

Hi Justin,

What you have here should work, however I still think that the trick
with "___local" suffix I refer to in [1] is a bit less hacky, e.g.:

    enum hid_report_type___local { ... };
    struct hid_bpf_ctx___local {
       __u32 index;
       const struct hid_device *hid; // this one should be in vmlinux.h with any config
       __u32 allocated_size;
       enum hid_report_type___local report_type;
       union {
           __s32 retval;
           __s32 size;
       };
    } __attribute__((preserve_access_index));
    
    enum hid_class_request___local { ... };
    enum hid_bpf_attach_flags___local { ... };
    ...
    extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx___local *ctx,
                                  unsigned int offset,


(sorry for being a bore, won't bring this up anymore).

Thanks,
Eduard

[1] https://lore.kernel.org/bpf/e99b4226bd450fedfebd4eb5c37054f032432b4f.camel@gmail.com/

> +#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;
> +	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,
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3
  2023-09-11 13:19   ` Eduard Zingerman
@ 2023-09-11 13:39     ` Benjamin Tissoires
  2023-09-11 13:43       ` Eduard Zingerman
  0 siblings, 1 reply; 12+ messages in thread
From: Benjamin Tissoires @ 2023-09-11 13:39 UTC (permalink / raw)
  To: Eduard Zingerman
  Cc: Justin Stitt, Jiri Kosina, Benjamin Tissoires, Shuah Khan,
	linux-input, linux-kselftest, linux-kernel, bpf

On Sep 11 2023, Eduard Zingerman wrote:
> On Fri, 2023-09-08 at 22:22 +0000, Justin Stitt wrote:
> > From: Benjamin Tissoires <bentiss@kernel.org>
> > 
> > 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>
> > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> > ---
> >  tools/testing/selftests/hid/progs/hid.c            |  3 --
> >  .../testing/selftests/hid/progs/hid_bpf_helpers.h  | 49 ++++++++++++++++++++++
> >  2 files changed, 49 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..ab3b18ba48c4 100644
> > --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> > +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> > @@ -5,6 +5,55 @@
> >  #ifndef __HID_BPF_HELPERS_H
> >  #define __HID_BPF_HELPERS_H
> >  
> > +/* "undefine" structs in vmlinux.h, because we "override" them below */
> 
> Hi Justin,
> 
> What you have here should work, however I still think that the trick
> with "___local" suffix I refer to in [1] is a bit less hacky, e.g.:
> 
>     enum hid_report_type___local { ... };
>     struct hid_bpf_ctx___local {
>        __u32 index;
>        const struct hid_device *hid; // this one should be in vmlinux.h with any config
>        __u32 allocated_size;
>        enum hid_report_type___local report_type;
>        union {
>            __s32 retval;
>            __s32 size;
>        };
>     } __attribute__((preserve_access_index));
>     
>     enum hid_class_request___local { ... };
>     enum hid_bpf_attach_flags___local { ... };
>     ...
>     extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx___local *ctx,
>                                   unsigned int offset,
> 
> 
> (sorry for being a bore, won't bring this up anymore).

No need to apologies for trying to make the code better :)

I specifically asked Justin to not use this because I intend the
examples to be here to use the same API in the selftests dir than users
in the wild. So if your suggestion definitely makes the header code
much better, it also means that everybody will start using `___local`
annotations for anything HID-BPF related, which is not what I want.

This header file is supposed to be included in the BPF, and IMO it's not
that important that we have the cleanest code, as long as the users have
the proper API.

Feel free to share your concerns :)

Cheers,
Benjamin

> 
> Thanks,
> Eduard
> 
> [1] https://lore.kernel.org/bpf/e99b4226bd450fedfebd4eb5c37054f032432b4f.camel@gmail.com/
> 
> > +#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;
> > +	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,
> > 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3
  2023-09-11 13:39     ` Benjamin Tissoires
@ 2023-09-11 13:43       ` Eduard Zingerman
  0 siblings, 0 replies; 12+ messages in thread
From: Eduard Zingerman @ 2023-09-11 13:43 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Justin Stitt, Jiri Kosina, Benjamin Tissoires, Shuah Khan,
	linux-input, linux-kselftest, linux-kernel, bpf

On Mon, 2023-09-11 at 15:39 +0200, Benjamin Tissoires wrote:
> On Sep 11 2023, Eduard Zingerman wrote:
> > On Fri, 2023-09-08 at 22:22 +0000, Justin Stitt wrote:
> > > From: Benjamin Tissoires <bentiss@kernel.org>
> > > 
> > > 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>
> > > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> > > ---
> > >  tools/testing/selftests/hid/progs/hid.c            |  3 --
> > >  .../testing/selftests/hid/progs/hid_bpf_helpers.h  | 49 ++++++++++++++++++++++
> > >  2 files changed, 49 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..ab3b18ba48c4 100644
> > > --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> > > +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> > > @@ -5,6 +5,55 @@
> > >  #ifndef __HID_BPF_HELPERS_H
> > >  #define __HID_BPF_HELPERS_H
> > >  
> > > +/* "undefine" structs in vmlinux.h, because we "override" them below */
> > 
> > Hi Justin,
> > 
> > What you have here should work, however I still think that the trick
> > with "___local" suffix I refer to in [1] is a bit less hacky, e.g.:
> > 
> >     enum hid_report_type___local { ... };
> >     struct hid_bpf_ctx___local {
> >        __u32 index;
> >        const struct hid_device *hid; // this one should be in vmlinux.h with any config
> >        __u32 allocated_size;
> >        enum hid_report_type___local report_type;
> >        union {
> >            __s32 retval;
> >            __s32 size;
> >        };
> >     } __attribute__((preserve_access_index));
> >     
> >     enum hid_class_request___local { ... };
> >     enum hid_bpf_attach_flags___local { ... };
> >     ...
> >     extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx___local *ctx,
> >                                   unsigned int offset,
> > 
> > 
> > (sorry for being a bore, won't bring this up anymore).
> 
> No need to apologies for trying to make the code better :)
> 
> I specifically asked Justin to not use this because I intend the
> examples to be here to use the same API in the selftests dir than users
> in the wild. So if your suggestion definitely makes the header code
> much better, it also means that everybody will start using `___local`
> annotations for anything HID-BPF related, which is not what I want.
> 
> This header file is supposed to be included in the BPF, and IMO it's not
> that important that we have the cleanest code, as long as the users have
> the proper API.
> 
> Feel free to share your concerns :)

Got it, thank you for explanation :)

> 
> Cheers,
> Benjamin
> 
> > 
> > Thanks,
> > Eduard
> > 
> > [1] https://lore.kernel.org/bpf/e99b4226bd450fedfebd4eb5c37054f032432b4f.camel@gmail.com/
> > 
> > > +#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;
> > > +	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,
> > > 
> > 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/3] selftests/hid: do not manually call headers_install
  2023-09-08 22:22 ` [PATCH v2 2/3] selftests/hid: do not manually call headers_install Justin Stitt
@ 2023-09-18 17:24   ` Shuah Khan
  0 siblings, 0 replies; 12+ messages in thread
From: Shuah Khan @ 2023-09-18 17:24 UTC (permalink / raw)
  To: Justin Stitt, Jiri Kosina, Benjamin Tissoires, Shuah Khan
  Cc: Eduard Zingerman, linux-input, linux-kselftest, linux-kernel,
	bpf, Benjamin Tissoires, Shuah Khan

On 9/8/23 16:22, Justin Stitt wrote:
> From: Benjamin Tissoires <bentiss@kernel.org>
> 
> "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>

Thank for making this change. Just check bpf continues to
compile and run though.

Acked-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/3] selftests/hid: fix building for older kernels
  2023-09-08 22:22 [PATCH v2 0/3] selftests/hid: fix building for older kernels Justin Stitt
                   ` (3 preceding siblings ...)
  2023-09-08 23:02 ` [PATCH v2 0/3] selftests/hid: fix building for older kernels Nick Desaulniers
@ 2023-09-26  7:26 ` Justin Stitt
  2023-10-02 14:48   ` Benjamin Tissoires
  4 siblings, 1 reply; 12+ messages in thread
From: Justin Stitt @ 2023-09-26  7:26 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Shuah Khan
  Cc: Eduard Zingerman, linux-input, linux-kselftest, linux-kernel,
	bpf, Benjamin Tissoires

Hey all,

Gentle ping on this patch. Looking to get this patch and [1] slated
for 6.7 wherein we can start getting cleaner kselftests builds.

I do not think I am able to successfully run the hid/bpf selftests due
to my kernel version being too low (and an inability to upgrade it as
I'm on a corp rolling release). I'd appreciate some insight on how to
get the tests running or if someone could actually build+run the tests
with this patch applied.

On Sat, Sep 9, 2023 at 7:22 AM Justin Stitt <justinstitt@google.com> wrote:
>
> 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
>
> Changes from v1 -> 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/all/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  | 49 ++++++++++++++++++++++
>  3 files changed, 53 insertions(+), 9 deletions(-)
> ---
> base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
> change-id: 20230908-kselftest-09-08-56d7f4a8d5c4
>
> Best regards,
> --
> Justin Stitt <justinstitt@google.com>
>

[1]: https://lore.kernel.org/all/20230912-kselftest-param_test-c-v1-1-80a6cffc7374@google.com/

Thanks
Justin

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/3] selftests/hid: fix building for older kernels
  2023-09-26  7:26 ` Justin Stitt
@ 2023-10-02 14:48   ` Benjamin Tissoires
  2023-10-03 22:34     ` Justin Stitt
  0 siblings, 1 reply; 12+ messages in thread
From: Benjamin Tissoires @ 2023-10-02 14:48 UTC (permalink / raw)
  To: Justin Stitt
  Cc: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Eduard Zingerman,
	linux-input, linux-kselftest, linux-kernel, bpf

On Sep 26 2023, Justin Stitt wrote:
> Hey all,
> 
> Gentle ping on this patch. Looking to get this patch and [1] slated
> for 6.7 wherein we can start getting cleaner kselftests builds.
> 
> I do not think I am able to successfully run the hid/bpf selftests due
> to my kernel version being too low (and an inability to upgrade it as
> I'm on a corp rolling release). I'd appreciate some insight on how to
> get the tests running or if someone could actually build+run the tests
> with this patch applied.

I wanted to apply this series today, but it failed my own CI now with
the enums being already defined:
https://gitlab.freedesktop.org/bentiss/hid/-/jobs/49754306

I'll probably squash the following patch in 1/3, would you mind giving
it a test?

---
From 37feca6c0e84705ad65e621643206c287b63bb0a Mon Sep 17 00:00:00 2001
From: Benjamin Tissoires <bentiss@kernel.org>
Date: Mon, 2 Oct 2023 15:37:18 +0200
Subject: [PATCH] fix selftests/hid: ensure we can compile the tests on kernels
 pre-6.3

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 .../selftests/hid/progs/hid_bpf_helpers.h     | 30 ++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
index ab3b18ba48c4..feed5a991e05 100644
--- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
+++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
@@ -5,16 +5,44 @@
 #ifndef __HID_BPF_HELPERS_H
 #define __HID_BPF_HELPERS_H
 
-/* "undefine" structs in vmlinux.h, because we "override" them below */
+/* "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>
-- 
2.41.0
---

Cheers,
Benjamin

> 
> On Sat, Sep 9, 2023 at 7:22 AM Justin Stitt <justinstitt@google.com> wrote:
> >
> > 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
> >
> > Changes from v1 -> 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/all/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  | 49 ++++++++++++++++++++++
> >  3 files changed, 53 insertions(+), 9 deletions(-)
> > ---
> > base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
> > change-id: 20230908-kselftest-09-08-56d7f4a8d5c4
> >
> > Best regards,
> > --
> > Justin Stitt <justinstitt@google.com>
> >
> 
> [1]: https://lore.kernel.org/all/20230912-kselftest-param_test-c-v1-1-80a6cffc7374@google.com/
> 
> Thanks
> Justin

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/3] selftests/hid: fix building for older kernels
  2023-10-02 14:48   ` Benjamin Tissoires
@ 2023-10-03 22:34     ` Justin Stitt
  0 siblings, 0 replies; 12+ messages in thread
From: Justin Stitt @ 2023-10-03 22:34 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Eduard Zingerman,
	linux-input, linux-kselftest, linux-kernel, bpf

On Mon, Oct 2, 2023 at 7:48 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> On Sep 26 2023, Justin Stitt wrote:
> > Hey all,
> >
> > Gentle ping on this patch. Looking to get this patch and [1] slated
> > for 6.7 wherein we can start getting cleaner kselftests builds.
> >
> > I do not think I am able to successfully run the hid/bpf selftests due
> > to my kernel version being too low (and an inability to upgrade it as
> > I'm on a corp rolling release). I'd appreciate some insight on how to
> > get the tests running or if someone could actually build+run the tests
> > with this patch applied.
>
> I wanted to apply this series today, but it failed my own CI now with
> the enums being already defined:
> https://gitlab.freedesktop.org/bentiss/hid/-/jobs/49754306
>
> I'll probably squash the following patch in 1/3, would you mind giving
> it a test?

Works for me with this incantation:
$ make LLVM=1 -j128 ARCH=x86_64 mrproper headers && make LLVM=1 -j128
ARCH=x86_64 -C tools/testing/selftests TARGETS=hid
...
---> BINARY   hid_bpf

Although, the tests expectedly fail.

Looks good to me.

>
> ---
> From 37feca6c0e84705ad65e621643206c287b63bb0a Mon Sep 17 00:00:00 2001
> From: Benjamin Tissoires <bentiss@kernel.org>
> Date: Mon, 2 Oct 2023 15:37:18 +0200
> Subject: [PATCH] fix selftests/hid: ensure we can compile the tests on kernels
>  pre-6.3
>
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
>  .../selftests/hid/progs/hid_bpf_helpers.h     | 30 ++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> index ab3b18ba48c4..feed5a991e05 100644
> --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> @@ -5,16 +5,44 @@
>  #ifndef __HID_BPF_HELPERS_H
>  #define __HID_BPF_HELPERS_H
>
> -/* "undefine" structs in vmlinux.h, because we "override" them below */
> +/* "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>
> --
> 2.41.0
> ---
>
> Cheers,
> Benjamin
>
> >
> > On Sat, Sep 9, 2023 at 7:22 AM Justin Stitt <justinstitt@google.com> wrote:
> > >
> > > 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
> > >
> > > Changes from v1 -> 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/all/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  | 49 ++++++++++++++++++++++
> > >  3 files changed, 53 insertions(+), 9 deletions(-)
> > > ---
> > > base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
> > > change-id: 20230908-kselftest-09-08-56d7f4a8d5c4
> > >
> > > Best regards,
> > > --
> > > Justin Stitt <justinstitt@google.com>
> > >
> >
> > [1]: https://lore.kernel.org/all/20230912-kselftest-param_test-c-v1-1-80a6cffc7374@google.com/
> >
> > Thanks
> > Justin

Thanks
Justin

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-10-03 22:35 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-08 22:22 [PATCH v2 0/3] selftests/hid: fix building for older kernels Justin Stitt
2023-09-08 22:22 ` [PATCH v2 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3 Justin Stitt
2023-09-11 13:19   ` Eduard Zingerman
2023-09-11 13:39     ` Benjamin Tissoires
2023-09-11 13:43       ` Eduard Zingerman
2023-09-08 22:22 ` [PATCH v2 2/3] selftests/hid: do not manually call headers_install Justin Stitt
2023-09-18 17:24   ` Shuah Khan
2023-09-08 22:22 ` [PATCH v2 3/3] selftests/hid: force using our compiled libbpf headers Justin Stitt
2023-09-08 23:02 ` [PATCH v2 0/3] selftests/hid: fix building for older kernels Nick Desaulniers
2023-09-26  7:26 ` Justin Stitt
2023-10-02 14:48   ` Benjamin Tissoires
2023-10-03 22:34     ` Justin Stitt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).