[2/2] HID: bpf: use __bpf_kfunc instead of noinline
Commit Message
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(-)
Comments
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
> >
> >
>
@@ -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)
@@ -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
*/