[PATCH 0/2] HID-BPF: Kconfig fix

[PATCH 0/2] HID-BPF: Kconfig fix

[Benjamin Tissoires]

Hi,

As discussed in [1], we need to add some more dependencies to HID-BPF in
the Kconfig. This way, when fmod_ret will be available on arm64, HID-BPF
will get enabled automatically.

Jon, would you mind giving a shot at that patch on your test system and
confirm the error disappeard?

Cheers,
Benjamin



[1] https://lore.kernel.org/r/CABRcYmKyRchQhabi1Vd9RcMQFCcb=EtWyEbFDFRTc-L-U8WhgA@mail.gmail.com

Benjamin Tissoires (2):
  HID: bpf: enforce HID_BPF dependencies
  selftests: hid: ensures we have the proper requirements in config

 drivers/hid/bpf/Kconfig            | 4 +++-
 tools/testing/selftests/hid/config | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

-- 
2.38.1

[PATCH 1/2] HID: bpf: enforce HID_BPF dependencies

[Benjamin Tissoires]

As mentioned in the link below, having JIT and BPF is not enough to
have fmod_ret and error injection APIs. This resolves the error that
happens on a system without tracing enabled when hid-bpf tries to
load itself.

Link: https://lore.kernel.org/r/CABRcYmKyRchQhabi1Vd9RcMQFCcb=EtWyEbFDFRTc-L-U8WhgA@mail.gmail.com
Fixes: f5c27da4e3c8 ("HID: initial BPF implementation")
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/bpf/Kconfig | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/bpf/Kconfig b/drivers/hid/bpf/Kconfig
index 298634fc3335..498232f9faa9 100644
--- a/drivers/hid/bpf/Kconfig
+++ b/drivers/hid/bpf/Kconfig
@@ -4,7 +4,9 @@ menu "HID-BPF support"
 config HID_BPF
 	bool "HID-BPF support"
 	default HID_SUPPORT
-	depends on BPF && BPF_SYSCALL
+	depends on BPF && BPF_SYSCALL && \
+		   DYNAMIC_FTRACE_WITH_DIRECT_CALLS && \
+		   FUNCTION_ERROR_INJECTION
 	help
 	This option allows to support eBPF programs on the HID subsystem.
 	eBPF programs can fix HID devices in a lighter way than a full
-- 
2.38.1

[PATCH 2/2] selftests: hid: ensures we have the proper requirements in config

[Benjamin Tissoires]

They are implicit right now but might get defaulted to N in the future,
so force them in the config so we don't get any surprises.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 tools/testing/selftests/hid/config | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/hid/config b/tools/testing/selftests/hid/config
index d4130489c1b1..e3595b4a3708 100644
--- a/tools/testing/selftests/hid/config
+++ b/tools/testing/selftests/hid/config
@@ -11,8 +11,10 @@ CONFIG_BPF_SYSCALL=y
 CONFIG_BPF=y
 CONFIG_CGROUP_BPF=y
 CONFIG_DEBUG_INFO_BTF=y
+CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS=y
 CONFIG_FPROBE=y
 CONFIG_FTRACE_SYSCALLS=y
+CONFIG_FUNCTION_ERROR_INJECTION=y
 CONFIG_FUNCTION_TRACER=y
 CONFIG_HIDRAW=y
 CONFIG_HID=y
-- 
2.38.1

Re: [PATCH 0/2] HID-BPF: Kconfig fix

[Florent Revest]

On Wed, Nov 30, 2022 at 11:10 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> Hi,
>
> As discussed in [1], we need to add some more dependencies to HID-BPF in
> the Kconfig. This way, when fmod_ret will be available on arm64, HID-BPF
> will get enabled automatically.
>
> Jon, would you mind giving a shot at that patch on your test system and
> confirm the error disappeard?
>
> Cheers,
> Benjamin
>
>
>
> [1] https://lore.kernel.org/r/CABRcYmKyRchQhabi1Vd9RcMQFCcb=EtWyEbFDFRTc-L-U8WhgA@mail.gmail.com
>
> Benjamin Tissoires (2):
>   HID: bpf: enforce HID_BPF dependencies
>   selftests: hid: ensures we have the proper requirements in config
>
>  drivers/hid/bpf/Kconfig            | 4 +++-
>  tools/testing/selftests/hid/config | 2 ++
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> --
> 2.38.1

Reviewed-by: Florent Revest <revest@chromium.org>

Re: [PATCH 0/2] HID-BPF: Kconfig fix

[Jon Hunter]

Hi Benjamin,

On 30/11/2022 10:10, Benjamin Tissoires wrote:
> Hi,
> 
> As discussed in [1], we need to add some more dependencies to HID-BPF in
> the Kconfig. This way, when fmod_ret will be available on arm64, HID-BPF
> will get enabled automatically.
> 
> Jon, would you mind giving a shot at that patch on your test system and
> confirm the error disappeard?

Yes will test today. Thanks!

Jon
-- 
nvpublic

Re: [PATCH 1/2] HID: bpf: enforce HID_BPF dependencies

[Jon Hunter]

On 30/11/2022 10:10, Benjamin Tissoires wrote:
> As mentioned in the link below, having JIT and BPF is not enough to
> have fmod_ret and error injection APIs. This resolves the error that
> happens on a system without tracing enabled when hid-bpf tries to
> load itself.
> 
> Link: https://lore.kernel.org/r/CABRcYmKyRchQhabi1Vd9RcMQFCcb=EtWyEbFDFRTc-L-U8WhgA@mail.gmail.com
> Fixes: f5c27da4e3c8 ("HID: initial BPF implementation")
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>   drivers/hid/bpf/Kconfig | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/bpf/Kconfig b/drivers/hid/bpf/Kconfig
> index 298634fc3335..498232f9faa9 100644
> --- a/drivers/hid/bpf/Kconfig
> +++ b/drivers/hid/bpf/Kconfig
> @@ -4,7 +4,9 @@ menu "HID-BPF support"
>   config HID_BPF
>   	bool "HID-BPF support"
>   	default HID_SUPPORT
> -	depends on BPF && BPF_SYSCALL
> +	depends on BPF && BPF_SYSCALL && \
> +		   DYNAMIC_FTRACE_WITH_DIRECT_CALLS && \
> +		   FUNCTION_ERROR_INJECTION
>   	help
>   	This option allows to support eBPF programs on the HID subsystem.
>   	eBPF programs can fix HID devices in a lighter way than a full


This fixes it for me!

Tested-by: Jon Hunter <jonathanh@nvidia.com>

Thanks!
Jon

-- 
nvpublic

Re: [PATCH 1/2] HID: bpf: enforce HID_BPF dependencies

[Jiri Kosina]

On Wed, 30 Nov 2022, Benjamin Tissoires wrote:

> As mentioned in the link below, having JIT and BPF is not enough to
> have fmod_ret and error injection APIs. This resolves the error that
> happens on a system without tracing enabled when hid-bpf tries to
> load itself.
> 
> Link: https://lore.kernel.org/r/CABRcYmKyRchQhabi1Vd9RcMQFCcb=EtWyEbFDFRTc-L-U8WhgA@mail.gmail.com
> Fixes: f5c27da4e3c8 ("HID: initial BPF implementation")
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>  drivers/hid/bpf/Kconfig | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/bpf/Kconfig b/drivers/hid/bpf/Kconfig
> index 298634fc3335..498232f9faa9 100644
> --- a/drivers/hid/bpf/Kconfig
> +++ b/drivers/hid/bpf/Kconfig
> @@ -4,7 +4,9 @@ menu "HID-BPF support"
>  config HID_BPF
>  	bool "HID-BPF support"
>  	default HID_SUPPORT
> -	depends on BPF && BPF_SYSCALL
> +	depends on BPF && BPF_SYSCALL && \
> +		   DYNAMIC_FTRACE_WITH_DIRECT_CALLS && \
> +		   FUNCTION_ERROR_INJECTION

FUNCTION_ERROR_INJECTION is a purely debugging feature, and not something 
we want to have enabled in production kernels (which is where HID-BPF 
should, on the other hand, be enabled). I am afraid this needs to go back 
to the drawing board.

JFTR, to make sure this gets properly discussed before 6.2 merge window 
opens up

	Nacked-by: Jiri Kosina <jkosina@suse.cz>

Which unfortunately means the whole hid-bpf implementation would need to 
be revised and postponed for 6.3 at least.

Sorry,

-- 
Jiri Kosina
SUSE Labs