On Wed, Nov 23, 2022 at 9:14 PM Alexei Starovoitov alexei.starovoitov@gmail.com wrote:
On Wed, Nov 23, 2022 at 6:53 AM Benjamin Tissoires benjamin.tissoires@redhat.com wrote:
Hi Jon,
On Wed, Nov 23, 2022 at 2:25 PM Jon Hunter jonathanh@nvidia.com wrote:
On 03/11/2022 15:57, Benjamin Tissoires wrote:
Declare an entry point that can use fmod_ret BPF programs, and also an API to access and change the incoming data.
A simpler implementation would consist in just calling hid_bpf_device_event() for any incoming event and let users deal with the fact that they will be called for any event of any device.
The goal of HID-BPF is to partially replace drivers, so this situation can be problematic because we might have programs which will step on each other toes.
For that, we add a new API hid_bpf_attach_prog() that can be called from a syscall and we manually deal with a jump table in hid-bpf.
Whenever we add a program to the jump table (in other words, when we attach a program to a HID device), we keep the number of time we added this program in the jump table so we can release it whenever there are no other users.
HID devices have an RCU protected list of available programs in the jump table, and those programs are called one after the other thanks to bpf_tail_call().
To achieve the detection of users losing their fds on the programs we attached, we add 2 tracing facilities on bpf_prog_release() (for when a fd is closed) and bpf_free_inode() (for when a pinned program gets unpinned).
Reviewed-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Benjamin Tissoires benjamin.tissoires@redhat.com
...
+static int __init hid_bpf_init(void) +{
int err;/* Note: if we exit with an error any time here, we would entirely break HID, which* is probably not something we want. So we log an error and return success.** This is not a big deal: the syscall allowing to attach a BPF program to a HID device* will not be available, so nobody will be able to use the functionality.*/err = register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &hid_bpf_kfunc_set);if (err) {pr_warn("error while setting HID BPF tracing kfuncs: %d", err);return 0;}err = hid_bpf_preload_skel();if (err) {pr_warn("error while preloading HID BPF dispatcher: %d", err);return 0;}/* register syscalls after we are sure we can load our preloaded bpf program */err = register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &hid_bpf_syscall_kfunc_set);if (err) {pr_warn("error while setting HID BPF syscall kfuncs: %d", err);return 0;}return 0;+}
We have a kernel test that checks for new warning and error messages on boot and with this change I am now seeing the following error message on our Tegra platforms ...
WARNING KERN hid_bpf: error while preloading HID BPF dispatcher: -13
I have a quick look at the code, but I can't say I am familiar with this. So I wanted to ask if a way to fix this or avoid this? I see the code returns 0, so one option would be to make this an informational or debug print.
I am not in favor of debug in that case, because I suspect it'll hide too much when getting a bug report. Informational could do, yes.
However, before that, I'd like to dig a little bit more on why it is failing. I thought arm64 now has support of tracing bpf programs, so I would not expect this to fail.
Unfortunately the patches to add support for such tracing bpf progs got stuck. Florent/Mark can probably share the latest status.
Oh... I noticed Jon's config was lacking CONFIG_FTRACE. So should I also add a depends on CONFIG_FTRACE to enable HID-BPF?
Cheers, Benjamin