All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] selftests/hid: assorted fixes
@ 2023-08-25  8:36 Benjamin Tissoires
  2023-08-25  8:36 ` [PATCH 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3 Benjamin Tissoires
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Benjamin Tissoires @ 2023-08-25  8:36 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Justin Stitt,
	Nick Desaulniers
  Cc: linux-input, linux-kselftest, linux-kernel, bpf, Benjamin Tissoires

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>
---
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>


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

* [PATCH 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3
  2023-08-25  8:36 [PATCH 0/3] selftests/hid: assorted fixes Benjamin Tissoires
@ 2023-08-25  8:36 ` Benjamin Tissoires
  2023-08-25  8:36 ` [PATCH 2/3] selftests/hid: do not manually call headers_install Benjamin Tissoires
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Benjamin Tissoires @ 2023-08-25  8:36 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Justin Stitt,
	Nick Desaulniers
  Cc: linux-input, linux-kselftest, linux-kernel, bpf, 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.

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,

-- 
2.39.1


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

* [PATCH 2/3] selftests/hid: do not manually call headers_install
  2023-08-25  8:36 [PATCH 0/3] selftests/hid: assorted fixes Benjamin Tissoires
  2023-08-25  8:36 ` [PATCH 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3 Benjamin Tissoires
@ 2023-08-25  8:36 ` Benjamin Tissoires
  2023-08-25  8:36 ` [PATCH 3/3] selftests/hid: force using our compiled libbpf headers Benjamin Tissoires
  2023-08-25 18:23 ` [PATCH 4/3] selftests/hid: more fixes to build with older kernel Justin Stitt
  3 siblings, 0 replies; 7+ messages in thread
From: Benjamin Tissoires @ 2023-08-25  8:36 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Justin Stitt,
	Nick Desaulniers
  Cc: linux-input, linux-kselftest, linux-kernel, bpf, Benjamin Tissoires

"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.39.1


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

* [PATCH 3/3] selftests/hid: force using our compiled libbpf headers
  2023-08-25  8:36 [PATCH 0/3] selftests/hid: assorted fixes Benjamin Tissoires
  2023-08-25  8:36 ` [PATCH 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3 Benjamin Tissoires
  2023-08-25  8:36 ` [PATCH 2/3] selftests/hid: do not manually call headers_install Benjamin Tissoires
@ 2023-08-25  8:36 ` Benjamin Tissoires
  2023-08-25 18:23 ` [PATCH 4/3] selftests/hid: more fixes to build with older kernel Justin Stitt
  3 siblings, 0 replies; 7+ messages in thread
From: Benjamin Tissoires @ 2023-08-25  8:36 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Justin Stitt,
	Nick Desaulniers
  Cc: linux-input, linux-kselftest, linux-kernel, bpf, 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.

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.39.1


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

* [PATCH 4/3] selftests/hid: more fixes to build with older kernel
  2023-08-25  8:36 [PATCH 0/3] selftests/hid: assorted fixes Benjamin Tissoires
                   ` (2 preceding siblings ...)
  2023-08-25  8:36 ` [PATCH 3/3] selftests/hid: force using our compiled libbpf headers Benjamin Tissoires
@ 2023-08-25 18:23 ` Justin Stitt
  2023-08-25 18:53   ` Eduard Zingerman
  3 siblings, 1 reply; 7+ messages in thread
From: Justin Stitt @ 2023-08-25 18:23 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Nick Desaulniers,
	linux-input, linux-kselftest, linux-kernel, bpf

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@redhat.com/T/#t
>
> 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/tools/testing/selftests/bpf/progs/bpf_iter.h#n3
[2]: https://lore.kernel.org/all/20230825-wip-selftests-v1-0-c862769020a8@kernel.org/
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

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

* Re: [PATCH 4/3] selftests/hid: more fixes to build with older kernel
  2023-08-25 18:23 ` [PATCH 4/3] selftests/hid: more fixes to build with older kernel Justin Stitt
@ 2023-08-25 18:53   ` Eduard Zingerman
  2023-08-28 17:47     ` Justin Stitt
  0 siblings, 1 reply; 7+ messages in thread
From: Eduard Zingerman @ 2023-08-25 18:53 UTC (permalink / raw)
  To: Justin Stitt, Benjamin Tissoires
  Cc: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Nick Desaulniers,
	linux-input, linux-kselftest, linux-kernel, bpf

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@redhat.com/T/#t
> > 
> > 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/tools/testing/selftests/bpf/progs/bpf_iter.h#n3
> [2]: https://lore.kernel.org/all/20230825-wip-selftests-v1-0-c862769020a8@kernel.org/
> 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
> 


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

* Re: [PATCH 4/3] selftests/hid: more fixes to build with older kernel
  2023-08-25 18:53   ` Eduard Zingerman
@ 2023-08-28 17:47     ` Justin Stitt
  0 siblings, 0 replies; 7+ messages in thread
From: Justin Stitt @ 2023-08-28 17:47 UTC (permalink / raw)
  To: Eduard Zingerman
  Cc: Benjamin Tissoires, Jiri Kosina, Benjamin Tissoires, Shuah Khan,
	Nick Desaulniers, linux-input, linux-kselftest, linux-kernel,
	bpf

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@redhat.com/T/#t
> > >
> > > 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/tools/testing/selftests/bpf/progs/bpf_iter.h#n3
> > [2]: https://lore.kernel.org/all/20230825-wip-selftests-v1-0-c862769020a8@kernel.org/
> > 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.

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

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

end of thread, other threads:[~2023-08-28 17:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-25  8:36 [PATCH 0/3] selftests/hid: assorted fixes Benjamin Tissoires
2023-08-25  8:36 ` [PATCH 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3 Benjamin Tissoires
2023-08-25  8:36 ` [PATCH 2/3] selftests/hid: do not manually call headers_install Benjamin Tissoires
2023-08-25  8:36 ` [PATCH 3/3] selftests/hid: force using our compiled libbpf headers Benjamin Tissoires
2023-08-25 18:23 ` [PATCH 4/3] selftests/hid: more fixes to build with older kernel Justin Stitt
2023-08-25 18:53   ` Eduard Zingerman
2023-08-28 17:47     ` Justin Stitt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.