| Message ID | 20230825-wip-selftests-v1-0-c862769020a8@kernel.org |
|---|---|
| Headers | show |
| Series | selftests/hid: assorted fixes | expand |
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 ---
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
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,