On 7/28/22 16:19, Greg KH wrote:
On Thu, Jul 21, 2022 at 05:36:14PM +0200, Benjamin Tissoires wrote:
--- /dev/null +++ b/include/linux/hid_bpf.h @@ -0,0 +1,102 @@ +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
This is not a uapi .h file, so the "WITH Linux-syscall-note" should not be here, right?
thanks, dropping this syscall note from the series.
+#ifndef __HID_BPF_H +#define __HID_BPF_H
+#include <linux/spinlock.h> +#include <uapi/linux/hid.h> +#include <uapi/linux/hid_bpf.h>
+struct hid_device;
+/*
- The following is the HID BPF API.
- It should be treated as UAPI, so extra care is required
- when making change to this file.
So is this uapi? If so, shouldn't it go into a uapi include directory so we know this and properly track it and maintain it that way?
IMO it's a grey area. It is not "uapi" because it doesn't export anything that userspace can use. A userspace program can not include that and use it in other words.
So strictly speaking, it's a normal part of a kernel header file, because it's a description of what other kernel users (though here, eBPF programs) can use.
But I really want that part of the API to be considered as "stable" and give some guarantees to the users that I won't change it at every release. Thus the "uapi-like".
- */
+/**
- struct hid_bpf_ctx - User accessible data for all HID programs
- ``data`` is not directly accessible from the context. We need to issue
- a call to ``hid_bpf_get_data()`` in order to get a pointer to that field.
- All of these fields are currently read-only.
- @index: program index in the jump table. No special meaning (a smaller index
doesn't mean the program will be executed before another program with
a bigger index).
- @hid: the ``struct hid_device`` representing the device itself
- @report_type: used for ``hid_bpf_device_event()``
- @size: Valid data in the data field.
Programs can get the available valid size in data by fetching this field.- */
+struct hid_bpf_ctx {
- __u32 index;
- const struct hid_device *hid;
- enum hid_report_type report_type;
- __s32 size;
+};
+/* Following functions are tracepoints that BPF programs can attach to */ +int hid_bpf_device_event(struct hid_bpf_ctx *ctx);
+/* Following functions are kfunc that we export to BPF programs */ +/* only available in tracing */ +__u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t __sz);
+/* only available in syscall */ +int hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags);
+/*
- Below is HID internal
- */
+/* internal function to call eBPF programs, not to be used by anybody */ +int __hid_bpf_tail_call(struct hid_bpf_ctx *ctx);
+#define HID_BPF_MAX_PROGS_PER_DEV 64 +#define HID_BPF_FLAG_MASK (((HID_BPF_FLAG_MAX - 1) << 1) - 1)
+/* types of HID programs to attach to */ +enum hid_bpf_prog_type {
- HID_BPF_PROG_TYPE_UNDEF = -1,
- HID_BPF_PROG_TYPE_DEVICE_EVENT, /* an event is emitted from the device */
- HID_BPF_PROG_TYPE_MAX,
+};
+struct hid_bpf_ops {
- struct module *owner;
- struct bus_type *bus_type;
+};
+extern struct hid_bpf_ops *hid_bpf_ops;
+struct hid_bpf_prog_list {
- u16 prog_idx[HID_BPF_MAX_PROGS_PER_DEV];
- u8 prog_cnt;
+};
+/* stored in each device */ +struct hid_bpf {
- struct hid_bpf_prog_list __rcu *progs[HID_BPF_PROG_TYPE_MAX]; /* attached BPF progs */
- bool destroyed; /* prevents the assignment of any progs */
- spinlock_t progs_lock; /* protects RCU update of progs */
+};
+#ifdef CONFIG_HID_BPF +int dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
u32 size, int interrupt);+void hid_bpf_destroy_device(struct hid_device *hid); +void hid_bpf_device_init(struct hid_device *hid); +#else /* CONFIG_HID_BPF */ +static inline int dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
u32 size, int interrupt) { return 0; }+static inline void hid_bpf_destroy_device(struct hid_device *hid) {} +static inline void hid_bpf_device_init(struct hid_device *hid) {} +#endif /* CONFIG_HID_BPF */
+#endif /* __HID_BPF_H */ diff --git a/include/uapi/linux/hid_bpf.h b/include/uapi/linux/hid_bpf.h new file mode 100644 index 000000000000..ba8caf9b60ee --- /dev/null +++ b/include/uapi/linux/hid_bpf.h @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
This is fine, it is in include/uapi/
Other than those minor comments, this all looks good to me!
Reviewed-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
Great! And thanks a lot for the other reviews.
I finally managed to get some time to work on it after some time off and urgent sh**t happening, so I'll send a new version of the series today.
Cheers, Benjamin