| Message ID | 20240123-b4-hid-bpf-fixes-v1-2-aa1fac734377@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | HID: bpf: couple of upstream fixes | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Tue, Jan 23, 2024 at 8:46 AM Benjamin Tissoires <bentiss@kernel.org> wrote: > > Follow the docs at Documentation/bpf/kfuncs.rst: > - declare the function with `__bpf_kfunc` > - disables missing prototype warnings, which allows to remove them from > include/linux/hid-bpf.h > > Removing the prototypes is not an issue because we currently have to > redeclare them when writing the BPF program. They will eventually be > generated by bpftool directly AFAIU. > > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org> > --- > drivers/hid/bpf/hid_bpf_dispatch.c | 22 +++++++++++++++++----- > include/linux/hid_bpf.h | 11 ----------- > 2 files changed, 17 insertions(+), 16 deletions(-) > > diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c > index 5111d1fef0d3..119e4f03df55 100644 > --- a/drivers/hid/bpf/hid_bpf_dispatch.c > +++ b/drivers/hid/bpf/hid_bpf_dispatch.c > @@ -143,6 +143,11 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s > } > EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup); > > +/* Disables missing prototype warnings */ > +__diag_push(); > +__diag_ignore_all("-Wmissing-prototypes", > + "Global kfuncs as their definitions will be in BTF"); > + would it be possible to use __bpf_kfunc_start_defs() and __bpf_kfunc_end_defs() macros instead of explicit __diag push/pop pairs? It's defined in include/linux/btf.h > /** > * hid_bpf_get_data - Get the kernel memory pointer associated with the context @ctx > * > @@ -152,7 +157,7 @@ EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup); > * > * @returns %NULL on error, an %__u8 memory pointer on success > */ > -noinline __u8 * > +__bpf_kfunc __u8 * > hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr_buf_size) > { > struct hid_bpf_ctx_kern *ctx_kern; > @@ -167,6 +172,7 @@ hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr > > return ctx_kern->data + offset; > } > +__diag_pop(); /* missing prototype warnings */ > > /* > * The following set contains all functions we agree BPF programs > @@ -274,6 +280,11 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b > return fd; > } > > +/* Disables missing prototype warnings */ > +__diag_push(); > +__diag_ignore_all("-Wmissing-prototypes", > + "Global kfuncs as their definitions will be in BTF"); > + > /** > * hid_bpf_attach_prog - Attach the given @prog_fd to the given HID device > * > @@ -286,7 +297,7 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b > * is pinned to the BPF file system). > */ > /* called from syscall */ > -noinline int > +__bpf_kfunc int > hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags) > { > struct hid_device *hdev; > @@ -328,7 +339,7 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags) > * > * @returns A pointer to &struct hid_bpf_ctx on success, %NULL on error. > */ > -noinline struct hid_bpf_ctx * > +__bpf_kfunc struct hid_bpf_ctx * > hid_bpf_allocate_context(unsigned int hid_id) > { > struct hid_device *hdev; > @@ -359,7 +370,7 @@ hid_bpf_allocate_context(unsigned int hid_id) > * @ctx: the HID-BPF context to release > * > */ > -noinline void > +__bpf_kfunc void > hid_bpf_release_context(struct hid_bpf_ctx *ctx) > { > struct hid_bpf_ctx_kern *ctx_kern; > @@ -380,7 +391,7 @@ hid_bpf_release_context(struct hid_bpf_ctx *ctx) > * > * @returns %0 on success, a negative error code otherwise. > */ > -noinline int > +__bpf_kfunc int > hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz, > enum hid_report_type rtype, enum hid_class_request reqtype) > { > @@ -448,6 +459,7 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz, > kfree(dma_data); > return ret; > } > +__diag_pop(); /* missing prototype warnings */ > > /* our HID-BPF entrypoints */ > BTF_SET8_START(hid_bpf_fmodret_ids) > diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h > index 840cd254172d..7118ac28d468 100644 > --- a/include/linux/hid_bpf.h > +++ b/include/linux/hid_bpf.h > @@ -77,17 +77,6 @@ enum hid_bpf_attach_flags { > int hid_bpf_device_event(struct hid_bpf_ctx *ctx); > int hid_bpf_rdesc_fixup(struct hid_bpf_ctx *ctx); > > -/* Following functions are kfunc that we export to BPF programs */ > -/* available everywhere in HID-BPF */ > -__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); > -int hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz, > - enum hid_report_type rtype, enum hid_class_request reqtype); > -struct hid_bpf_ctx *hid_bpf_allocate_context(unsigned int hid_id); > -void hid_bpf_release_context(struct hid_bpf_ctx *ctx); > - > /* > * Below is HID internal > */ > > -- > 2.43.0 > >
On Tue, Jan 23, 2024 at 8:57 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Tue, Jan 23, 2024 at 8:46 AM Benjamin Tissoires <bentiss@kernel.org> wrote: > > > > Follow the docs at Documentation/bpf/kfuncs.rst: > > - declare the function with `__bpf_kfunc` > > - disables missing prototype warnings, which allows to remove them from > > include/linux/hid-bpf.h > > > > Removing the prototypes is not an issue because we currently have to > > redeclare them when writing the BPF program. They will eventually be > > generated by bpftool directly AFAIU. > > > > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org> > > --- > > drivers/hid/bpf/hid_bpf_dispatch.c | 22 +++++++++++++++++----- > > include/linux/hid_bpf.h | 11 ----------- > > 2 files changed, 17 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c > > index 5111d1fef0d3..119e4f03df55 100644 > > --- a/drivers/hid/bpf/hid_bpf_dispatch.c > > +++ b/drivers/hid/bpf/hid_bpf_dispatch.c > > @@ -143,6 +143,11 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s > > } > > EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup); > > > > +/* Disables missing prototype warnings */ > > +__diag_push(); > > +__diag_ignore_all("-Wmissing-prototypes", > > + "Global kfuncs as their definitions will be in BTF"); > > + > > would it be possible to use __bpf_kfunc_start_defs() and > __bpf_kfunc_end_defs() macros instead of explicit __diag push/pop > pairs? It's defined in include/linux/btf.h Sure, I'll use them in v2. I also realized that I had some memory leaks because I never called put_device() after bus_find_device(), so I need to add another fix to this series. Cheers, Benjamin > > > /** > > * hid_bpf_get_data - Get the kernel memory pointer associated with the context @ctx > > * > > @@ -152,7 +157,7 @@ EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup); > > * > > * @returns %NULL on error, an %__u8 memory pointer on success > > */ > > -noinline __u8 * > > +__bpf_kfunc __u8 * > > hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr_buf_size) > > { > > struct hid_bpf_ctx_kern *ctx_kern; > > @@ -167,6 +172,7 @@ hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr > > > > return ctx_kern->data + offset; > > } > > +__diag_pop(); /* missing prototype warnings */ > > > > /* > > * The following set contains all functions we agree BPF programs > > @@ -274,6 +280,11 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b > > return fd; > > } > > > > +/* Disables missing prototype warnings */ > > +__diag_push(); > > +__diag_ignore_all("-Wmissing-prototypes", > > + "Global kfuncs as their definitions will be in BTF"); > > + > > /** > > * hid_bpf_attach_prog - Attach the given @prog_fd to the given HID device > > * > > @@ -286,7 +297,7 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b > > * is pinned to the BPF file system). > > */ > > /* called from syscall */ > > -noinline int > > +__bpf_kfunc int > > hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags) > > { > > struct hid_device *hdev; > > @@ -328,7 +339,7 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags) > > * > > * @returns A pointer to &struct hid_bpf_ctx on success, %NULL on error. > > */ > > -noinline struct hid_bpf_ctx * > > +__bpf_kfunc struct hid_bpf_ctx * > > hid_bpf_allocate_context(unsigned int hid_id) > > { > > struct hid_device *hdev; > > @@ -359,7 +370,7 @@ hid_bpf_allocate_context(unsigned int hid_id) > > * @ctx: the HID-BPF context to release > > * > > */ > > -noinline void > > +__bpf_kfunc void > > hid_bpf_release_context(struct hid_bpf_ctx *ctx) > > { > > struct hid_bpf_ctx_kern *ctx_kern; > > @@ -380,7 +391,7 @@ hid_bpf_release_context(struct hid_bpf_ctx *ctx) > > * > > * @returns %0 on success, a negative error code otherwise. > > */ > > -noinline int > > +__bpf_kfunc int > > hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz, > > enum hid_report_type rtype, enum hid_class_request reqtype) > > { > > @@ -448,6 +459,7 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz, > > kfree(dma_data); > > return ret; > > } > > +__diag_pop(); /* missing prototype warnings */ > > > > /* our HID-BPF entrypoints */ > > BTF_SET8_START(hid_bpf_fmodret_ids) > > diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h > > index 840cd254172d..7118ac28d468 100644 > > --- a/include/linux/hid_bpf.h > > +++ b/include/linux/hid_bpf.h > > @@ -77,17 +77,6 @@ enum hid_bpf_attach_flags { > > int hid_bpf_device_event(struct hid_bpf_ctx *ctx); > > int hid_bpf_rdesc_fixup(struct hid_bpf_ctx *ctx); > > > > -/* Following functions are kfunc that we export to BPF programs */ > > -/* available everywhere in HID-BPF */ > > -__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); > > -int hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz, > > - enum hid_report_type rtype, enum hid_class_request reqtype); > > -struct hid_bpf_ctx *hid_bpf_allocate_context(unsigned int hid_id); > > -void hid_bpf_release_context(struct hid_bpf_ctx *ctx); > > - > > /* > > * Below is HID internal > > */ > > > > -- > > 2.43.0 > > > > >
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c index 5111d1fef0d3..119e4f03df55 100644 --- a/drivers/hid/bpf/hid_bpf_dispatch.c +++ b/drivers/hid/bpf/hid_bpf_dispatch.c @@ -143,6 +143,11 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s } EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup); +/* Disables missing prototype warnings */ +__diag_push(); +__diag_ignore_all("-Wmissing-prototypes", + "Global kfuncs as their definitions will be in BTF"); + /** * hid_bpf_get_data - Get the kernel memory pointer associated with the context @ctx * @@ -152,7 +157,7 @@ EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup); * * @returns %NULL on error, an %__u8 memory pointer on success */ -noinline __u8 * +__bpf_kfunc __u8 * hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr_buf_size) { struct hid_bpf_ctx_kern *ctx_kern; @@ -167,6 +172,7 @@ hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr return ctx_kern->data + offset; } +__diag_pop(); /* missing prototype warnings */ /* * The following set contains all functions we agree BPF programs @@ -274,6 +280,11 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b return fd; } +/* Disables missing prototype warnings */ +__diag_push(); +__diag_ignore_all("-Wmissing-prototypes", + "Global kfuncs as their definitions will be in BTF"); + /** * hid_bpf_attach_prog - Attach the given @prog_fd to the given HID device * @@ -286,7 +297,7 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b * is pinned to the BPF file system). */ /* called from syscall */ -noinline int +__bpf_kfunc int hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags) { struct hid_device *hdev; @@ -328,7 +339,7 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags) * * @returns A pointer to &struct hid_bpf_ctx on success, %NULL on error. */ -noinline struct hid_bpf_ctx * +__bpf_kfunc struct hid_bpf_ctx * hid_bpf_allocate_context(unsigned int hid_id) { struct hid_device *hdev; @@ -359,7 +370,7 @@ hid_bpf_allocate_context(unsigned int hid_id) * @ctx: the HID-BPF context to release * */ -noinline void +__bpf_kfunc void hid_bpf_release_context(struct hid_bpf_ctx *ctx) { struct hid_bpf_ctx_kern *ctx_kern; @@ -380,7 +391,7 @@ hid_bpf_release_context(struct hid_bpf_ctx *ctx) * * @returns %0 on success, a negative error code otherwise. */ -noinline int +__bpf_kfunc int hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz, enum hid_report_type rtype, enum hid_class_request reqtype) { @@ -448,6 +459,7 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz, kfree(dma_data); return ret; } +__diag_pop(); /* missing prototype warnings */ /* our HID-BPF entrypoints */ BTF_SET8_START(hid_bpf_fmodret_ids) diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h index 840cd254172d..7118ac28d468 100644 --- a/include/linux/hid_bpf.h +++ b/include/linux/hid_bpf.h @@ -77,17 +77,6 @@ enum hid_bpf_attach_flags { int hid_bpf_device_event(struct hid_bpf_ctx *ctx); int hid_bpf_rdesc_fixup(struct hid_bpf_ctx *ctx); -/* Following functions are kfunc that we export to BPF programs */ -/* available everywhere in HID-BPF */ -__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); -int hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz, - enum hid_report_type rtype, enum hid_class_request reqtype); -struct hid_bpf_ctx *hid_bpf_allocate_context(unsigned int hid_id); -void hid_bpf_release_context(struct hid_bpf_ctx *ctx); - /* * Below is HID internal */
Follow the docs at Documentation/bpf/kfuncs.rst: - declare the function with `__bpf_kfunc` - disables missing prototype warnings, which allows to remove them from include/linux/hid-bpf.h Removing the prototypes is not an issue because we currently have to redeclare them when writing the BPF program. They will eventually be generated by bpftool directly AFAIU. Signed-off-by: Benjamin Tissoires <bentiss@kernel.org> --- drivers/hid/bpf/hid_bpf_dispatch.c | 22 +++++++++++++++++----- include/linux/hid_bpf.h | 11 ----------- 2 files changed, 17 insertions(+), 16 deletions(-)