diff mbox series

[4/3] selftests/hid: more fixes to build with older kernel

Message ID 20230825182316.m2ksjoxe4s7dsapn@google.com (mailing list archive)
State Not Applicable
Delegated to: BPF
Headers show
Series selftests/hid: assorted fixes | expand

Commit Message

Justin Stitt Aug. 25, 2023, 6:23 p.m. UTC
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(+)

--
2.42.0.rc1.204.g551eb34607-goog

Comments

Eduard Zingerman Aug. 25, 2023, 6:53 p.m. UTC | #1
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
>
Justin Stitt Aug. 28, 2023, 5:47 p.m. UTC | #2
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
diff mbox series

Patch

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,