| Message ID | 20230825182316.m2ksjoxe4s7dsapn@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | selftests/hid: assorted fixes | expand |
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 >
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 --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,